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.

Sign up for GitHub

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

Jump to bottom

Fix Kafka subnet ordering diff #14627

Merged
merged 17 commits into from
Sep 21, 2021
Merged

Fix Kafka subnet ordering diff #14627

merged 17 commits into from
Sep 21, 2021

Conversation

nijave
Copy link
Contributor

@nijave nijave commented Aug 13, 2020 •
edited by ewbankkit
Loading

Fixes

          ~ client_subnets  = [ # forces replacement
              + "subnet-5f6b1c30e",
                "subnet-1b00",
                "subnet-d1a10",
              - "subnet-5f6b1c30e",
            ]

where the subnets get passed in a different order.

Closes #15577.
Closes #19537.
Closes #17726.
Closes #11736.
Closes #15661.

@nijave nijave requested a review from a team August 13, 2020 15:45
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/kafka Issues and PRs that pertain to the kafka service. labels Aug 13, 2020
Base automatically changed from master to main January 23, 2021 00:58
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:58
@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Sep 15, 2021
@ewbankkit ewbankkit force-pushed the nv-fix-kafka-subnet-diff branch from c4e2b93 to c056120 Compare September 21, 2021 12:17
@github-actions github-actions bot added documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/XL Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels Sep 21, 2021
ewbankkit
ewbankkit approved these changes Sep 21, 2021
View reviewed changes
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

Commercial
% make testacc TESTARGS='-run=TestAccAWSMskCluster_\|TestAccAWSMskClusterDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMskCluster_\|TestAccAWSMskClusterDataSource_ -timeout 180m
=== RUN   TestAccAWSMskClusterDataSource_Name
=== PAUSE TestAccAWSMskClusterDataSource_Name
=== RUN   TestAccAWSMskCluster_basic
=== PAUSE TestAccAWSMskCluster_basic
=== RUN   TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize
=== PAUSE TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize
=== RUN   TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType
=== PAUSE TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
    provider_test.go:58: Requires the aws_acmpca_certificate_authority resource to support importing the root CA certificate
--- SKIP: TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns (0.00s)
=== RUN   TestAccAWSMskCluster_ConfigurationInfo_Revision
=== PAUSE TestAccAWSMskCluster_ConfigurationInfo_Revision
=== RUN   TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn
=== PAUSE TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn
=== RUN   TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker
=== PAUSE TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker
=== RUN   TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
=== PAUSE TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
=== RUN   TestAccAWSMskCluster_EnhancedMonitoring
=== PAUSE TestAccAWSMskCluster_EnhancedMonitoring
=== RUN   TestAccAWSMskCluster_NumberOfBrokerNodes
=== PAUSE TestAccAWSMskCluster_NumberOfBrokerNodes
=== RUN   TestAccAWSMskCluster_OpenMonitoring
=== PAUSE TestAccAWSMskCluster_OpenMonitoring
=== RUN   TestAccAWSMskCluster_LoggingInfo
=== PAUSE TestAccAWSMskCluster_LoggingInfo
=== RUN   TestAccAWSMskCluster_KafkaVersionUpgrade
=== PAUSE TestAccAWSMskCluster_KafkaVersionUpgrade
=== RUN   TestAccAWSMskCluster_KafkaVersionDowngrade
=== PAUSE TestAccAWSMskCluster_KafkaVersionDowngrade
=== RUN   TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo
=== PAUSE TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo
=== RUN   TestAccAWSMskCluster_Tags
=== PAUSE TestAccAWSMskCluster_Tags
=== CONT  TestAccAWSMskClusterDataSource_Name
=== CONT  TestAccAWSMskCluster_EnhancedMonitoring
=== CONT  TestAccAWSMskCluster_OpenMonitoring
=== CONT  TestAccAWSMskCluster_NumberOfBrokerNodes
=== CONT  TestAccAWSMskCluster_KafkaVersionDowngrade
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== CONT  TestAccAWSMskCluster_LoggingInfo
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn
=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker
=== CONT  TestAccAWSMskCluster_ConfigurationInfo_Revision
=== CONT  TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType
=== CONT  TestAccAWSMskCluster_KafkaVersionUpgrade
=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
=== CONT  TestAccAWSMskCluster_Tags
=== CONT  TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize
=== CONT  TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo
=== CONT  TestAccAWSMskCluster_basic
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1980.55s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1981.10s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1982.25s)
--- PASS: TestAccAWSMskCluster_Tags (1987.41s)
--- PASS: TestAccAWSMskCluster_basic (1989.03s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1990.55s)
--- PASS: TestAccAWSMskClusterDataSource_Name (1990.95s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (2121.01s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (2157.77s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (2204.65s)
--- PASS: TestAccAWSMskCluster_ConfigurationInfo_Revision (2721.42s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (2724.32s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (3146.42s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (3154.27s)
--- PASS: TestAccAWSMskCluster_KafkaVersionDowngrade (3157.41s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType (3914.71s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgrade (5943.75s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo (5973.44s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	5978.411s
GovCloud
% make testacc TESTARGS='-run=TestAccAWSMskCluster_\|TestAccAWSMskClusterDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMskCluster_\|TestAccAWSMskClusterDataSource_ -timeout 180m
=== RUN   TestAccAWSMskClusterDataSource_Name
=== PAUSE TestAccAWSMskClusterDataSource_Name
=== RUN   TestAccAWSMskCluster_basic
=== PAUSE TestAccAWSMskCluster_basic
=== RUN   TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize
=== PAUSE TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize
=== RUN   TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType
=== PAUSE TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
    provider_test.go:58: Requires the aws_acmpca_certificate_authority resource to support importing the root CA certificate
--- SKIP: TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns (0.00s)
=== RUN   TestAccAWSMskCluster_ConfigurationInfo_Revision
=== PAUSE TestAccAWSMskCluster_ConfigurationInfo_Revision
=== RUN   TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn
=== PAUSE TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn
=== RUN   TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker
=== PAUSE TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker
=== RUN   TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
=== PAUSE TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
=== RUN   TestAccAWSMskCluster_EnhancedMonitoring
=== PAUSE TestAccAWSMskCluster_EnhancedMonitoring
=== RUN   TestAccAWSMskCluster_NumberOfBrokerNodes
=== PAUSE TestAccAWSMskCluster_NumberOfBrokerNodes
=== RUN   TestAccAWSMskCluster_OpenMonitoring
=== PAUSE TestAccAWSMskCluster_OpenMonitoring
=== RUN   TestAccAWSMskCluster_LoggingInfo
=== PAUSE TestAccAWSMskCluster_LoggingInfo
=== RUN   TestAccAWSMskCluster_KafkaVersionUpgrade
=== PAUSE TestAccAWSMskCluster_KafkaVersionUpgrade
=== RUN   TestAccAWSMskCluster_KafkaVersionDowngrade
=== PAUSE TestAccAWSMskCluster_KafkaVersionDowngrade
=== RUN   TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo
=== PAUSE TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo
=== RUN   TestAccAWSMskCluster_Tags
=== PAUSE TestAccAWSMskCluster_Tags
=== CONT  TestAccAWSMskClusterDataSource_Name
=== CONT  TestAccAWSMskCluster_EnhancedMonitoring
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram
=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker
=== CONT  TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType
=== CONT  TestAccAWSMskCluster_ConfigurationInfo_Revision
=== CONT  TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize
=== CONT  TestAccAWSMskCluster_NumberOfBrokerNodes
=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster
=== CONT  TestAccAWSMskCluster_KafkaVersionDowngrade
=== CONT  TestAccAWSMskCluster_Tags
=== CONT  TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo
=== CONT  TestAccAWSMskCluster_basic
=== CONT  TestAccAWSMskCluster_LoggingInfo
=== CONT  TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn
=== CONT  TestAccAWSMskCluster_OpenMonitoring
=== CONT  TestAccAWSMskCluster_KafkaVersionUpgrade
--- PASS: TestAccAWSMskCluster_basic (1694.18s)
--- PASS: TestAccAWSMskClusterDataSource_Name (1694.32s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1698.12s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1698.51s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1699.50s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1700.54s)
--- PASS: TestAccAWSMskCluster_Tags (1703.18s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1775.07s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1849.42s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1854.50s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (2031.90s)
--- PASS: TestAccAWSMskCluster_ConfigurationInfo_Revision (2183.33s)
--- PASS: TestAccAWSMskCluster_KafkaVersionDowngrade (2957.75s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Scram (2966.01s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_InstanceType (2970.28s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgrade (2972.15s)
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Sasl_Iam (2976.66s)
--- PASS: TestAccAWSMskCluster_KafkaVersionUpgradeWithConfigurationInfo (2977.79s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	2987.632s

Carl Haferd and others added 17 commits September 21, 2021 15:13
@ewbankkit
Add broker_client_vpc_ip_addresses to the msk provider
87d05fe
@ewbankkit
Add and use 'internal/service/kafka/finder'.
dbb517a
@ewbankkit
r/aws_msk_cluster: Use 'BrokerAZDistribution_Values()'.
cffdacc
@ewbankkit
r/aws_msk_cluster: Remove 'schema.HashString'.
335de86
@nijave @ewbankkit
Fix Kafka subnet ordering diff
71b8967
@ewbankkit
Add 'finder.ClusterOperationByARN' and friends.
c7aecb6
@ewbankkit
Revert "Add broker_client_vpc_ip_addresses to the msk provider"
4f48e19 
This reverts commit f70779e.
@ewbankkit
Add 'SortEndpointsString'.
ae6967f
@ewbankkit
r/aws_msk_cluster: Use 'expandStringSet', not 'expandStringList' for …
c8e3447 
…'client_subnets' and 'security_groups'.
@ewbankkit
Fix 'panic: runtime error: invalid memory address or nil pointer dere…
00d7f57 
…ference'.
@ewbankkit
Use 'resource.TestCheckTypeSetElemAttrPair' to verify 'client_subnets…
0c3a99b 
…' and 'security_groups'.
@ewbankkit
Change '%s' to '%w' when returning errors (hashicorp#12991).
51136d3
@ewbankkit
Add CHANGELOG entry.
89a42b1
@ewbankkit
r/aws_msk_cluster: Document customizable timeouts.
999a728
@ewbankkit
r/aws_msk_broker: Add 'zookeeper_connect_string_tls' attribute.
deb4530
@ewbankkit
r/aws_msk_cluster: Activate CA for 'TestAccAWSMskCluster_ClientAuthen…
c250ac4 
…tication_Tls_CertificateAuthorityArns' (hashicorp#20973).

Acceptance test output:

% make testacc TESTARGS='-run=TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns -timeout 180m
=== RUN   TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
=== PAUSE TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
=== CONT  TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns
--- PASS: TestAccAWSMskCluster_ClientAuthentication_Tls_CertificateAuthorityArns (1616.46s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1619.884s
@ewbankkit
r/aws_msk_broker: Can now use 'BrokerInfo.StateInfo' in waiter.
9707159
@ewbankkit ewbankkit force-pushed the nv-fix-kafka-subnet-diff branch from 76cf446 to 9707159 Compare September 21, 2021 19:25
@ewbankkit
Copy link
Contributor

@nijave Thanks for the contribution 🎉 👏.

@ewbankkit ewbankkit merged commit cf1bbbc into hashicorp:main Sep 21, 2021
@github-actions github-actions bot added this to the v3.60.0 milestone Sep 21, 2021
@ewbankkit ewbankkit mentioned this pull request Sep 21, 2021
Closed
@nijave nijave deleted the nv-fix-kafka-subnet-diff branch September 21, 2021 21:16
@github-actions
Copy link

This functionality has been released in v3.60.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. Thank you!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ewbankkit ewbankkit ewbankkit approved these changes

Assignees
No one assigned
Labels
documentation Introduces or discusses updates to documentation. service/kafka Issues and PRs that pertain to the kafka service. size/XL 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
Milestone
v3.60.0
2 participants
@nijave @ewbankkit