resource/aws_mq_broker: Add RabbitMQ engine type #16108

yardensachs · 2020-11-10T03:22:31Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #16030
Closes #16027
Closes #16047
Relates #11459, #16261, #12758, #16751

Release note for CHANGELOG:

resource/aws_mq_broker: Added RabbitMQ engine type

Output from acceptance testing:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMqBroker_rabbitmq -timeout 120m
=== RUN   TestAccAWSMqBroker_rabbitmq
=== PAUSE TestAccAWSMqBroker_rabbitmq
=== CONT  TestAccAWSMqBroker_rabbitmq
--- PASS: TestAccAWSMqBroker_rabbitmq (514.93s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	518.598s

yardensachs · 2020-11-10T03:38:45Z

@DrFaust92 I was wondering if you might have a few minutes to help me with an issue related to AWS api:

It looks like for RabbitMQ engine type, AWS is not planning on sending the users list in the DescribeBroker API call.

The issue that I am getting is that if the users list does not come back - terraform will assume it needs to add them again, because the mq broker state has that user list empty:

make testacc TESTARGS='-run=TestAccAWSMqBroker_rabbitmq'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMqBroker_rabbitmq -timeout 120m
=== RUN   TestAccAWSMqBroker_rabbitmq
=== PAUSE TestAccAWSMqBroker_rabbitmq
=== CONT  TestAccAWSMqBroker_rabbitmq
    resource_aws_mq_broker_test.go:825: Step 1/2 error: After applying this test step and performing a `terraform refresh`, the plan was not empty.
        stdout


        An execution plan has been generated and is shown below.
        Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_mq_broker.test will be updated in-place
          ~ resource "aws_mq_broker" "test" {
                apply_immediately          = false
                arn                        = "arn:aws:mq:us-west-2:810560764000:broker:tf-acc-test-g6wq3:b-5d87a1ba-03da-4522-82de-77c0a7f8017d"
                auto_minor_version_upgrade = false
                broker_name                = "tf-acc-test-g6wq3"
                deployment_mode            = "SINGLE_INSTANCE"
                engine_type                = "RabbitMQ"
                engine_version             = "3.8.6"
                host_instance_type         = "mq.t3.micro"
                id                         = "b-5d87a1ba-03da-4522-82de-77c0a7f8017d"
                instances                  = [
                    {
                        console_url = "https://b-5d87a1ba-03da-4522-82de-77c0a7f8017d.mq.us-west-2.amazonaws.com"
                        endpoints   = [
                            "amqps://b-5d87a1ba-03da-4522-82de-77c0a7f8017d.mq.us-west-2.amazonaws.com:5671",
                        ]
                        ip_address  = ""
                    },
                ]
                publicly_accessible        = false
                security_groups            = [
                    "sg-0f0f5352a4f4b4810",
                ]
                subnet_ids                 = [
                    "subnet-f5064ede",
                ]
                tags                       = {}

                encryption_options {
                    use_aws_owned_key = true
                }

                logs {
                    audit   = false
                    general = false
                }

                maintenance_window_start_time {
                    day_of_week = "TUESDAY"
                    time_of_day = "12:00"
                    time_zone   = "UTC"
                }

              + user {
                  + console_access = false
                  + groups         = []
                  + password       = (sensitive value)
                  + username       = "Test"
                }
            }

        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccAWSMqBroker_rabbitmq (481.57s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	484.127s
FAIL
make: *** [testacc] Error 1
...

This only happens with RabbitMQ, It does return on the user list if the engine type is ApacheMQ.

How should I proceed?
Is there a way to ignore that user list? (based on the engine type)
Should username and password be only be effective on broker creation? (based on the engine type)

yardensachs · 2020-11-11T16:39:45Z

@gdavison can you help?

yardensachs · 2020-11-15T05:42:39Z

Ok I figured it out using DiffSuppressFunc

yardensachs · 2020-11-15T06:14:48Z

Also tested the whole module:

% make testacc TESTARGS='-run=TestAccAWSMqBroker_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMqBroker_ -timeout 120m
=== RUN   TestAccAWSMqBroker_basic
=== PAUSE TestAccAWSMqBroker_basic
=== RUN   TestAccAWSMqBroker_allFieldsDefaultVpc
=== PAUSE TestAccAWSMqBroker_allFieldsDefaultVpc
=== RUN   TestAccAWSMqBroker_allFieldsCustomVpc
=== PAUSE TestAccAWSMqBroker_allFieldsCustomVpc
=== RUN   TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
=== PAUSE TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
=== RUN   TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
=== PAUSE TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
=== RUN   TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
=== PAUSE TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
=== RUN   TestAccAWSMqBroker_updateUsers
=== PAUSE TestAccAWSMqBroker_updateUsers
=== RUN   TestAccAWSMqBroker_updateTags
=== PAUSE TestAccAWSMqBroker_updateTags
=== RUN   TestAccAWSMqBroker_updateSecurityGroup
=== PAUSE TestAccAWSMqBroker_updateSecurityGroup
=== RUN   TestAccAWSMqBroker_disappears
=== PAUSE TestAccAWSMqBroker_disappears
=== RUN   TestAccAWSMqBroker_rabbitmq
=== PAUSE TestAccAWSMqBroker_rabbitmq
=== CONT  TestAccAWSMqBroker_basic
=== CONT  TestAccAWSMqBroker_updateUsers
=== CONT  TestAccAWSMqBroker_updateTags
=== CONT  TestAccAWSMqBroker_disappears
=== CONT  TestAccAWSMqBroker_rabbitmq
=== CONT  TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
=== CONT  TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
=== CONT  TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
=== CONT  TestAccAWSMqBroker_updateSecurityGroup
=== CONT  TestAccAWSMqBroker_allFieldsCustomVpc
=== CONT  TestAccAWSMqBroker_allFieldsDefaultVpc
2020/11/14 21:42:23 [WARN] Truncating attribute path of 0 diagnostics for TypeSet
--- PASS: TestAccAWSMqBroker_rabbitmq (480.81s)
--- PASS: TestAccAWSMqBroker_disappears (1137.15s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1137.16s)
--- PASS: TestAccAWSMqBroker_basic (1171.67s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1185.68s)
--- PASS: TestAccAWSMqBroker_updateTags (1200.75s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1227.58s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1286.17s)
--- PASS: TestAccAWSMqBroker_updateUsers (1517.43s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1816.32s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1826.47s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1830.391s

yardensachs · 2020-11-16T16:47:36Z

@ewbankkit Is this waiting for a bot approval?

jonten · 2020-11-19T09:11:12Z

@yardensachs Just a question. Should'nt mq.EngineTypeRabbitmq be added here too?

terraform-provider-aws/aws/resource_aws_mq_configuration.go

Line 59 in be8f93c

mq.EngineTypeActivemq,

yardensachs · 2020-11-19T15:01:28Z

@yardensachs Just a question. Should'nt mq.EngineTypeRabbitmq be added here too?

terraform-provider-aws/aws/resource_aws_mq_configuration.go

Line 59 in be8f93c

mq.EngineTypeActivemq,

@jonten The configuration block/resource does not apply to RabbitMQ as far as I'm aware.

jonten · 2020-11-20T06:50:20Z

@yardensachs Just a question. Should'nt mq.EngineTypeRabbitmq be added here too?

terraform-provider-aws/aws/resource_aws_mq_configuration.go

Line 59 in be8f93c

mq.EngineTypeActivemq,

@jonten The configuration block/resource does not apply to RabbitMQ as far as I'm aware.

Ok, good 👍

one70six · 2020-12-01T22:40:58Z

I tried to use your branch to provision and came across 2 issues:

Enabling Cloudwatch logs with logs { general = true } results in a plan incuding:

+ logs {
          + audit   = false
          + general = true
        }

which is correct but it errors on the audit log even though you explicitly set audit = false or even leave it off:

aws_mq_broker.tf-rmq-broker: Creating...

Error: BadRequestException: Audit logging is not supported for RabbitMQ brokers.
{
  RespMetadata: {
    StatusCode: 400,
    RequestID: "4a17e8b5-8c0f-4ecd-a881-b058a610f959"
  },
  ErrorAttribute: "logs.audit",
  Message_: "Audit logging is not supported for RabbitMQ brokers."
}

Setting publicly_accessible = true results in:

aws_mq_broker.tf-rmq-broker: Creating...

Error: BadRequestException: Broker with [publiclyAccessible] set to true does not support specifying [securityGroups]
{
  RespMetadata: {
    StatusCode: 400,
    RequestID: "97343aef-5455-4f2a-94b3-1e49b28e40b8"
  },
  ErrorAttribute: "securityGroups",
  Message_: "Broker with [publiclyAccessible] set to true does not support specifying [securityGroups]"
}

and if you remove the security_groups argument:

Error: Missing required argument

  on main.tf line 6, in resource "aws_mq_broker" "tf-rmq-broker":
   6: resource "aws_mq_broker" "tf-rmq-broker" {

The argument "security_groups" is required, but no definition was found.

or if you try setting it to empty:

aws_mq_broker.tf-rmq-broker: Creating...

Error: BadRequestException: Empty list of security groups is invalid.
{
  RespMetadata: {
    StatusCode: 400,
    RequestID: "a8e6c4fb-05cb-4e06-8bc6-2e6d0243e373"
  },
  ErrorAttribute: "securityGroups",
  Message_: "Empty list of security groups is invalid."
}

Other than that it works great!

one70six · 2020-12-16T16:50:33Z

I found that for at least the security_groups issue, the aws-sdk-go that is being called by the hashicorp/aws Terraform module is enforcing that the SecurityGroups is NOT optional. This is most likely intentional for ActiveMQ but breaks in the case of Rabbit? I could be wrong..., bottom line is that there is a conflict between the enforcement on the SDK/API side vs. the Terraform provider side. Bummer...

See: https://github.com/aws/aws-sdk-go/blob/master/service/mq/api.go#L2789

alex-bes · 2020-12-23T06:17:45Z

hi there! thank you very much for the efforts on this!

is there an ETA on when this will be merged/released? Eager to try it out! :)

yardensachs · 2020-12-24T21:18:45Z

I don't know when this will be merged.
I have other PRs waiting for months, so this might take long as well.

I see people commented on this.
I am not very good at this yet, and not sure I can resolve the issues @one70six found.

I did this work a long time ago when I had the time to do it.
Now because of the slow response of: commit/review/merge - its hard to keep maintaining this PR and be responsive.

I can't imagine the amount of work the maintainers have, and the speed at which AWS is now releasing changes - I am sure its crazy.

lukaszkorecki · 2021-02-23T17:59:44Z

website/docs/r/mq_broker.html.markdown

-* `logs` - (Optional) Logging configuration of the broker. See below.
-* `user` - (Required) The list of all ActiveMQ usernames for the specified broker. See below.
+* `logs` - (Optional) Logging configuration of the broker. See below. (Applies to `ActiveMQ` only)
+* `user` - (Required) The list of all broker usernames for the specified broker. See below. RabbitMQ users can only be specified in the creation of the resource. Updates to users can only be in the RabbitMQ UI.


It might be helpful to add that RabbitMQ comes with the management plugin enabled, so it's also possible to the API to created/update/delete users (and more, /api/definitions endpoint is super handy for that).

We're using the cloud formation workaround at the moment, and also have a Lambda in place (triggered by Terraform) to set up a couple of exchanges and users.

Unrelated but you can use the rabbit provider to create exchanges and users :D

Ah yes, assuming that where the place where you run terraform apply has direct access to the RMQ server.

TheGreenSourcerer · 2021-02-23T20:37:35Z

Any chance to merge it this year? 😄

brunobraga-hotmart · 2021-03-02T14:41:39Z

Any merge estimate?

YakDriver · 2021-03-05T02:18:20Z

@yardensachs Thank you for your work on this PR! There are many limitations currently with using RabbitMQ in AWS. However, if we can provide the capability with the limitations and carefully document the limits, people can decide for themselves. It doesn't seem like a reason to hold up this PR.

I cannot guarantee how soon I'll get to this. However, to facilitate the process, here are the limitations I see in the documentation:

For BrokerInstance, a RabbitMQ broker instance can't have an associated IP address (IpAddress) of the ENI attached to the broker.
For CreateBrokerRequest, the list of information about broker configuration (Configuration) does not apply to RabbitMQ brokers.
For CreateBrokerRequest, LDAP server metadata (LdapServerMetadata) to authenticate and authorize connections to a broker is not supported for RabbitMQ.
For CreateBrokerRequest, the storage type (StorageType) cannot be EFS for RabbitMQ brokers.
For CreateConfiguration, CreateConfigurationRequest, the engine type can be RabbitMQ. Perhaps an error? See 2.
For Logs, audit logging (Audit) does not apply to RabbitMQ, while general logging (General) does appear to be applicable to RabbitMQ.
For UpdateBrokerRequest, the list of information about broker configuration (Configuration) does not apply to RabbitMQ brokers. See 2.
For UpdateBrokerRequest , LDAP server metadata (LdapServerMetadata) to authenticate and authorize connections to a broker is not supported for RabbitMQ. See 3.
For User, a RabbitMQ user cannot have console access (ConsoleAccess)
For User, a RabbitMQ user cannot be associated with a list of groups (Groups). (Username and Password do not list RabbitMQ limitations.)

YakDriver

Looks great! 🎉

Acceptance tests in commercial (us-west-2) (Amazon MQ is not supported in GovCloud):

--- PASS: TestAccAWSMqBroker_rabbitmq (540.89s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1127.84s)
--- PASS: TestAccAWSMqBroker_disappears (1130.68s)
--- PASS: TestAccAWSMqBroker_updateTags (1160.50s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1161.53s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1163.69s)
--- PASS: TestAccAWSMqBroker_basic (1188.40s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1454.75s)
--- PASS: TestAccAWSMqBroker_updateUsers (1506.68s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1717.77s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1755.67s)

YakDriver · 2021-03-06T00:06:09Z

This PR also adds enumeration validations per #14601

ghost · 2021-03-12T01:01:14Z

This has been released in version 3.32.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2021-04-05T17:10:19Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

ghost added size/M Managed by automation to categorize the size of a PR. service/mq Issues and PRs that pertain to the mq service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Nov 10, 2020

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Nov 10, 2020

yardensachs mentioned this pull request Nov 11, 2020

Support for RabbitMQ in AmazonMQ #16030

Closed

yardensachs marked this pull request as ready for review November 15, 2020 06:14

yardensachs requested a review from a team as a code owner November 15, 2020 06:14

yardensachs force-pushed the f-mq-rabbitmq-support branch from 962536d to 7123990 Compare November 15, 2020 06:17

grigorov approved these changes Nov 16, 2020

View reviewed changes

lucastetreault mentioned this pull request Nov 18, 2020

Updated Amazon MQ functionality #16261

Merged

yardensachs force-pushed the f-mq-rabbitmq-support branch from a2fd673 to 2cf1327 Compare November 19, 2020 15:07

ghost added the documentation Introduces or discusses updates to documentation. label Dec 24, 2020

Base automatically changed from master to main January 23, 2021 00:59

lukaszkorecki reviewed Feb 23, 2021

View reviewed changes

YakDriver self-assigned this Mar 5, 2021

YakDriver added 5 commits March 5, 2021 11:09

tests/data-source/mq_broker: Add storage and authentication arguments

97387c3

tests/resource/mq_broker: MQ capitalization issues

19dd6ed

resource/mq_broker: Rework with waiter

c78fc4e

docs/resource/mq_broker: Update docs with new attributes

73433fc

resource/mq_broker: Avoid nil pointer panic

7798234

ghost added size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Mar 5, 2021

YakDriver added 5 commits March 5, 2021 14:29

resource/mq_broker: Move state waiting to waiter

ed284c7

resource/mq_broker: Fix waiter

05a5bc0

resource/mq_broker: Update changelog

1a4e838

tests/resource/mq_broker: Allow sweeper to retry in some cases

785535a

resource/mq_broker: Use const for error code

b0df885

YakDriver approved these changes Mar 5, 2021

View reviewed changes

YakDriver changed the title ~~resource/aws_mq_broker: Added RabbitMQ engine type~~ resource/aws_mq_broker: Add RabbitMQ engine type Mar 5, 2021

YakDriver added this to the v3.32.0 milestone Mar 5, 2021

YakDriver merged commit 9ce03ed into hashicorp:main Mar 5, 2021

yardensachs deleted the f-mq-rabbitmq-support branch March 6, 2021 21:25

leriel mentioned this pull request Mar 8, 2021

engine_type seems to support only ActiveMQ cloudposse/terraform-aws-mq-broker#18

Closed

ghost locked as resolved and limited conversation to collaborators Apr 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_mq_broker: Add RabbitMQ engine type #16108

resource/aws_mq_broker: Add RabbitMQ engine type #16108

yardensachs commented Nov 10, 2020 •

edited by YakDriver

Loading

yardensachs commented Nov 10, 2020

yardensachs commented Nov 11, 2020

yardensachs commented Nov 15, 2020

yardensachs commented Nov 15, 2020

yardensachs commented Nov 16, 2020

jonten commented Nov 19, 2020

yardensachs commented Nov 19, 2020

jonten commented Nov 20, 2020

one70six commented Dec 1, 2020

one70six commented Dec 16, 2020

alex-bes commented Dec 23, 2020

yardensachs commented Dec 24, 2020

lukaszkorecki Feb 23, 2021

DrFaust92 Feb 23, 2021

lukaszkorecki Feb 23, 2021

TheGreenSourcerer commented Feb 23, 2021

brunobraga-hotmart commented Mar 2, 2021

YakDriver commented Mar 5, 2021 •

edited

Loading

YakDriver left a comment

YakDriver commented Mar 6, 2021

ghost commented Mar 12, 2021

ghost commented Apr 5, 2021

resource/aws_mq_broker: Add RabbitMQ engine type #16108

resource/aws_mq_broker: Add RabbitMQ engine type #16108

Conversation

yardensachs commented Nov 10, 2020 • edited by YakDriver Loading

Community Note

yardensachs commented Nov 10, 2020

yardensachs commented Nov 11, 2020

yardensachs commented Nov 15, 2020

yardensachs commented Nov 15, 2020

yardensachs commented Nov 16, 2020

jonten commented Nov 19, 2020

yardensachs commented Nov 19, 2020

jonten commented Nov 20, 2020

one70six commented Dec 1, 2020

one70six commented Dec 16, 2020

alex-bes commented Dec 23, 2020

yardensachs commented Dec 24, 2020

lukaszkorecki Feb 23, 2021

Choose a reason for hiding this comment

DrFaust92 Feb 23, 2021

Choose a reason for hiding this comment

lukaszkorecki Feb 23, 2021

Choose a reason for hiding this comment

TheGreenSourcerer commented Feb 23, 2021

brunobraga-hotmart commented Mar 2, 2021

YakDriver commented Mar 5, 2021 • edited Loading

YakDriver left a comment

Choose a reason for hiding this comment

YakDriver commented Mar 6, 2021

ghost commented Mar 12, 2021

ghost commented Apr 5, 2021

yardensachs commented Nov 10, 2020 •

edited by YakDriver

Loading

YakDriver commented Mar 5, 2021 •

edited

Loading