Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated Amazon MQ functionality #16261

Merged
merged 10 commits into from
Mar 11, 2021

Conversation

lucastetreault
Copy link
Contributor

@lucastetreault lucastetreault commented Nov 17, 2020

  • Add support for RabbitMQ
  • Add support for LDAP Authentication
  • Add support for EBS storage type
  • Make security groups optional since it will use the default one if omitted

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

Relates #16108

Release note for CHANGELOG:

resource/aws_mq_broker: Support RabbitMQ, LDAP Authentication and EBS storage.

Output from acceptance testing:

$ 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_EBS
=== PAUSE TestAccAWSMqBroker_EBS
=== RUN   TestAccAWSMqBroker_basicRabbitMQ
=== PAUSE TestAccAWSMqBroker_basicRabbitMQ
=== RUN   TestAccAWSMqBroker_clusterRabbitMQ
=== PAUSE TestAccAWSMqBroker_clusterRabbitMQ
=== RUN   TestAccAWSMqBroker_ldap
=== PAUSE TestAccAWSMqBroker_ldap
=== RUN   TestAccAWSMqBroker_basicDefaultSecurityGroup
=== PAUSE TestAccAWSMqBroker_basicDefaultSecurityGroup
=== 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
=== CONT  TestAccAWSMqBroker_basic
=== CONT  TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
=== CONT  TestAccAWSMqBroker_updateSecurityGroup
=== CONT  TestAccAWSMqBroker_ldap
=== CONT  TestAccAWSMqBroker_basicDefaultSecurityGroup
=== CONT  TestAccAWSMqBroker_basicRabbitMQ
=== CONT  TestAccAWSMqBroker_allFieldsDefaultVpc
=== CONT  TestAccAWSMqBroker_clusterRabbitMQ
=== CONT  TestAccAWSMqBroker_allFieldsCustomVpc
=== CONT  TestAccAWSMqBroker_updateTags
=== CONT  TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
=== CONT  TestAccAWSMqBroker_updateUsers
=== CONT  TestAccAWSMqBroker_disappears
=== CONT  TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
=== CONT  TestAccAWSMqBroker_EBS
2020/11/17 13:28:50 [WARN] Truncating attribute path of 0 diagnostics for TypeSet
2020/11/17 13:28:50 [WARN] Truncating attribute path of 0 diagnostics for TypeSet
2020/11/17 13:28:50 [WARN] Truncating attribute path of 0 diagnostics for TypeSet
2020/11/17 13:28:53 [WARN] Truncating attribute path of 0 diagnostics for TypeSet
--- PASS: TestAccAWSMqBroker_basicRabbitMQ (628.99s)
--- PASS: TestAccAWSMqBroker_clusterRabbitMQ (734.59s)
2020/11/17 13:41:31 [WARN] Truncating attribute path of 0 diagnostics for TypeSet
--- PASS: TestAccAWSMqBroker_EBS (835.84s)
--- PASS: TestAccAWSMqBroker_basicDefaultSecurityGroup (1114.85s)
--- PASS: TestAccAWSMqBroker_disappears (1151.17s)
--- PASS: TestAccAWSMqBroker_ldap (1172.08s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1182.22s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1205.41s)
--- PASS: TestAccAWSMqBroker_basic (1213.00s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1253.00s)
--- PASS: TestAccAWSMqBroker_updateTags (1271.10s)
--- PASS: TestAccAWSMqBroker_updateUsers (1445.56s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1452.81s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1867.78s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1882.72s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1886.075s


...

@lucastetreault lucastetreault requested a review from a team as a code owner November 17, 2020 22:04
@ghost ghost added size/XL 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 17, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Nov 17, 2020
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome @lucastetreault 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@lucastetreault
Copy link
Contributor Author

I didn't actually look for open issues or PRs before starting work on this... just got bored on the weekend and didn't see these features. Looks like there's another PR for the RabbitMQ engine type #16108 and another for the EBS storage type #11325

@yardensachs
Copy link
Contributor

yardensachs commented Nov 18, 2020

@lucastetreault Once they merge PR #16108, you can pull and update

Base automatically changed from master to main January 23, 2021 00:59
@YakDriver YakDriver self-assigned this Mar 5, 2021
@YakDriver
Copy link
Member

@lucastetreault Thank you for this PR! We have performed significant reworking in #16108, which has been merged. While some of the same features were merged with that PR, you have excellent acceptance tests we'd love to be able to merge. If you have a chance, please rebase and give the code a once over to see if there are additional improvements.

@YakDriver YakDriver added waiting-response Maintainers are waiting on response from community or contributor. and removed needs-triage Waiting for first response or review from a maintainer. labels Mar 5, 2021
- Add support for RabbitMQ
- Add support for LDAP Authentication
- Add support for EBS storage type
- Make security groups optional since it will use the default one if omitted
@ghost ghost added size/L 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 10, 2021
@YakDriver YakDriver removed the waiting-response Maintainers are waiting on response from community or contributor. label Mar 11, 2021
@ghost ghost added the documentation Introduces or discusses updates to documentation. label Mar 11, 2021
Copy link
Member

@YakDriver YakDriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you.

Acceptance tests on commercial partition (us-west-2):

--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1587.35s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1649.70s)
--- PASS: TestAccAWSMqBroker_basic (1098.21s)
--- PASS: TestAccAWSMqBroker_clusterRabbitMQ (698.27s)
--- PASS: TestAccAWSMqBroker_disappears (1176.88s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1099.69s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1091.11s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1091.33s)
--- PASS: TestAccAWSMqBroker_ldap (1096.64s)
--- PASS: TestAccAWSMqBroker_rabbitmq (577.41s)
--- PASS: TestAccAWSMqBroker_tags (1109.23s)
--- PASS: TestAccAWSMqBroker_throughputOptimized (627.94s)
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1188.06s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1189.04s)
--- PASS: TestAccAWSMqBroker_updateUsers (1191.79s)

@YakDriver
Copy link
Member

YakDriver commented Mar 11, 2021

NOTE: (I added a note to the documentation also but for future travelers, I'll explain more here.) After trying both UpdateBroker and ForceNew, I could not get AWS to update LDAP server metadata information. Hopefully they will patch the issue and all will work as it should. In the meantime, you may or may not be able to change any LDAP server metadata for an MQ broker. The specific field I attempt to change was ldap_server_metadata.service_account_username.

@YakDriver YakDriver added this to the v3.32.0 milestone Mar 11, 2021
@YakDriver YakDriver merged commit 40f8371 into hashicorp:main Mar 11, 2021
@ghost
Copy link

ghost commented Mar 12, 2021

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
Copy link

ghost commented Apr 10, 2021

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 ghost locked as resolved and limited conversation to collaborators Apr 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/mq Issues and PRs that pertain to the mq service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants