Add support for MSK IAM client authentication

[hcourse-nydig]

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

Description

Resolves #19295.

Affects aws_msk_cluster resource

This PR adds support for the iam SASL client authentication configuration. Go is not my forté so I could really do with someone experienced in Go/writing providers to have a look over this. Thanks.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSMskCluster_ClientAuthentication_Sasl_' ...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMskCluster_ClientAuthentication_Sasl_ -timeout 180m
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (3640.96s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (3645.80s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       3647.501s

[github-actions]

Welcome @hcourse-nydig 👋

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! 😃

[hcourse-nydig]

Struggling to get one of the acceptance tests to pass, specifically resource.TestCheckResourceAttr(resourceName, "client_authentication.0.sasl.0.iam", "true"), fails even though I can see the cluster getting created with iam auth.

Otherwise I think this is pretty close to being ready to go.

[hcourse-nydig]

Tests passing, all ready for review.

[ewbankkit]

Needs documentation updates 😄.

[hcourse-nydig]

Want me to have a go? Apologies missed docs and changelog updates.

Update: Docs updated.

[raoim]

If everything in place, can someone merge it please. We are planning to use this feature.

[YakDriver]

I apologize for the delay. Several of the acceptance tests are failing. Although the failures seem unrelated to these changes, we want to make sure. Also, the acceptance tests take a very long time to run.

[hcourse-nydig]

Several of the acceptance tests are failing.

Ah, I only ran the ones in the comment. Let me know which are failing and I can take a look.

[YakDriver]

Thanks @hcourse-nydig ! Make sure to pull my latest changes if you push anything to your branch!

I've fixed the import verify issues. It looks like on commercial, AWS sometimes tweaks the version slightly (probably giving a minor or point version upgrade) so the import verify for current_version fails. It was already being ignored in many cases but not all. Lack of consistency on both sides...

However, the TestAccAWSMskCluster_ConfigurationInfo_Revision looks like eventual consistency a timeout problem. We should probably make a note of it, merge this PR, and work on a later fix.

Currently, everything is passing in GovCloud but not in commercial. (Usually it's the other way around.)

GovCloud:

--- PASS: TestAccAWSMskCluster_Tags (1491.89s)
--- PASS: TestAccAWSMskCluster_basic (1492.61s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1493.69s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1496.32s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1500.44s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1505.85s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1607.35s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1612.29s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1639.14s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (1986.12s)
--- PASS: TestAccAWSMskCluster_ConfigurationInfo_Revision (1996.89s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (2900.72s)
--- PASS: TestAccAWSMskCluster_KafkaVersionDowngrade (2911.52s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (2920.55s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgrade (3580.18s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo (3584.98s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType (3959.66s)

Commercial run (us-west-2):
TestAccAWSMskCluster_ConfigurationInfo_Revision looks like an eventual consistency too-short of timeout issue.

=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
    resource_aws_msk_cluster_test.go:468: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) (len=1) {
         (string) (len=15) "current_version": (string) (len=14) "K3P5ROKL5A1OLE"
        }
        
        
        (map[string]string) (len=1) {
         (string) (len=15) "current_version": (string) (len=14) "K3AEGXETSR30VB"
        }
=== CONT  TestAccAWSMskCluster_KafkaVersionUpgrade
    resource_aws_msk_cluster_test.go:685: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) (len=1) {
         (string) (len=15) "current_version": (string) (len=14) "K3P5ROKL5A1OLE"
        }
        
        
        (map[string]string) (len=1) {
         (string) (len=15) "current_version": (string) (len=14) "K3AEGXETSR30VB"
        }
--- PASS: TestAccAWSMskCluster_Tags (1862.27s)
--- FAIL: TestAccAWSMskCluster_KafkaVersionUpgrade (1873.17s)
--- FAIL: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1879.64s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1883.62s)
--- PASS: TestAccAWSMskCluster_basic (1883.95s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1885.08s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1960.59s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (2089.80s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (2137.21s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (2146.06s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (2615.50s)
--- PASS: TestAccAWSMskCluster_KafkaVersionDowngrade (3355.87s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (3356.76s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (3363.19s)
=== CONT  TestAccAWSMskCluster_ConfigurationInfo_Revision
    resource_aws_msk_cluster_test.go:366: Step 1/3 error: Error running apply: exit status 1
        
        Error: error waiting for MSK cluster creation (arn:aws:kafka:us-west-2:067819342479:cluster/tf-acc-test-26423674100300426/12536d42-524a-430b-8e44-0c4430fe808a-5): Error waiting for MSK cluster creation: "arn:aws:kafka:us-west-2:067819342479:cluster/tf-acc-test-26423674100300426/12536d42-524a-430b-8e44-0c4430fe808a-5": cluster still creating
        
          with aws_msk_cluster.test,
          on terraform_plugin_test.tf line 62, in resource "aws_msk_cluster" "test":
          62: resource "aws_msk_cluster" "test" {
        
    testing_new.go:63: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: failed deleting MSK cluster "arn:aws:kafka:us-west-2:067819342479:cluster/tf-acc-test-26423674100300426/12536d42-524a-430b-8e44-0c4430fe808a-5": BadRequestException: You can't delete cluster in CREATING state.
        {
          RespMetadata: {
            StatusCode: 400,
            RequestID: "b3a5f4bc-252f-4c3f-a84a-f4190b57463a"
          },
          Message_: "You can't delete cluster in CREATING state."
        }
        
--- FAIL: TestAccAWSMskCluster_ConfigurationInfo_Revision (3636.12s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo (6231.33s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType (6937.26s)

[YakDriver]

TL;DR: I increased the create timeout to 2 hours and am ignoring current_version when verifying the import state. I believe these fixes will fix all the current failures in us-west-2. Tests running now.

[YakDriver]

Note: aws_msk_cluster should be modernized with proper waiters. However, to reduce the impact of this PR, that will need to happen in a separate PR. See #19537 for tracking these improvements.

[YakDriver]

Besides the issue at hand (IAM client auth), we managed to fix some acceptance test issues and set timeouts to a better length. Looks great! 🎉

Output of acceptance tests (GovCloud):

--- PASS: TestAccAWSMskCluster_Tags (1491.89s)
--- PASS: TestAccAWSMskCluster_basic (1492.61s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1493.69s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1496.32s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1500.44s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1505.85s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1607.35s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1612.29s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1639.14s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (1986.12s)
--- PASS: TestAccAWSMskCluster_ConfigurationInfo_Revision (1996.89s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (2900.72s)
--- PASS: TestAccAWSMskCluster_KafkaVersionDowngrade (2911.52s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (2920.55s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgrade (3580.18s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo (3584.98s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType (3959.66s)

Output of acceptance tests (us-west-2):

--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1696.48s)
--- PASS: TestAccAWSMskCluster_basic (1700.72s)
--- PASS: TestAccAWSMskCluster_Tags (1727.76s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1728.05s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1738.26s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1806.86s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1937.45s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1941.49s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1969.42s)
--- PASS: TestAccAWSMskCluster_ConfigurationInfo_Revision (2417.66s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (3989.85s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (4000.16s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (4000.16s)
--- PASS: TestAccAWSMskCluster_KafkaVersionDowngrade (4000.91s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo (5929.39s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgrade (5988.22s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType (6581.33s)

[hcourse-nydig]

Thanks for getting into that. I ran out of time yesterday so really appreciate you doing that.

[ghost]

This has been released in version 3.43.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!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.