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

r/aws_msk_cluster: fix provisioned_throughput block update behavior #40910

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Jan 14, 2025

Description

This change includes two fixes for the broker_node_group_info.0.storage_info.0.ebs_storage_info.0.provisioned_throughput block:

  1. The update method now includes logic to handle the case where the block previously existed but has been removed. Removal will cause the nested enabled value to be set to false in the call to the Update API, disabling provisioned throughput.
  2. Differences between an unset provisioned_throughput block and the default value returned from AWS (a nested block with enabled set to false) will now be suppressed. This should prevent perpetual differences in configurations which choose to omit this block to disable it.
% make testacc PKG=kafka TESTS=TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/kafka/... -v -count 1 -parallel 20 -run='TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_'  -timeout 360m
2025/01/13 14:30:06 Initializing Terraform AWS Provider...

--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset (2195.72s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled (2200.75s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled (2207.31s)
PASS                                                                                                                                                                                    ok      github.com/hashicorp/terraform-provider-aws/internal/service/kafka      2213.740s

Relations

Closes #37043
Closes #26031

Output from Acceptance Testing

% make testacc PKG=kafka TESTS=TestAccKafkaCluster_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/kafka/... -v -count 1 -parallel 20 -run='TestAccKafkaCluster_'  -timeout 360m -vet=off
2025/01/13 15:31:15 Initializing Terraform AWS Provider...

--- PASS: TestAccKafkaCluster_EncryptionInfoEncryptionInTransit_inCluster (1903.67s)
=== CONT  TestAccKafkaCluster_EncryptionInfoEncryptionInTransit_clientBroker
--- PASS: TestAccKafkaCluster_basic (1918.03s)
=== CONT  TestAccKafkaCluster_EncryptionInfo_encryptionAtRestKMSKeyARN
--- PASS: TestAccKafkaCluster_enhancedMonitoring (2052.26s)
=== CONT  TestAccKafkaCluster_tags
--- PASS: TestAccKafkaCluster_ClientAuthenticationTLS_certificateAuthorityARNs (2115.51s)
=== CONT  TestAccKafkaCluster_disappears
--- PASS: TestAccKafkaCluster_openMonitoring (2158.66s)
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled
--- PASS: TestAccKafkaCluster_loggingInfo (2195.49s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled (2215.01s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset (2226.82s)
--- PASS: TestAccKafkaCluster_storageMode (2819.54s)
--- PASS: TestAccKafkaCluster_numberOfBrokerNodes (2823.48s)
--- PASS: TestAccKafkaCluster_Info_revision (2827.43s)
--- PASS: TestAccKafkaCluster_EncryptionInfo_encryptionAtRestKMSKeyARN (2089.17s)
--- PASS: TestAccKafkaCluster_EncryptionInfoEncryptionInTransit_clientBroker (2105.36s)
--- PASS: TestAccKafkaCluster_kafkaVersionDowngrade (4013.28s)
--- PASS: TestAccKafkaCluster_ClientAuthenticationSASL_iam (4014.32s)
--- PASS: TestAccKafkaCluster_tags (1962.95s)
--- PASS: TestAccKafkaCluster_ClientAuthenticationSASL_scram (4018.83s)
--- PASS: TestAccKafkaCluster_disappears (1907.78s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled (1878.63s)
--- PASS: TestAccKafkaCluster_ClientAuthenticationTLS_initiallyNoAuthentication (4289.27s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_vpcConnectivity (5800.89s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_publicAccessSASLIAM (5961.32s)
--- PASS: TestAccKafkaCluster_kafkaVersionUpgradeWithInfo (7131.54s)
--- PASS: TestAccKafkaCluster_kafkaVersionUpgrade (7131.72s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_instanceType (7585.60s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/kafka      7590.972s

Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/kafka Issues and PRs that pertain to the kafka service. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. labels Jan 14, 2025
@jar-b jar-b changed the title r/aws_msk_cluster: fix provisioned_throughput block updates r/aws_msk_cluster: fix provisioned_throughput block update behavior Jan 14, 2025
@jar-b jar-b force-pushed the b-msk_cluster-provisioned_throughput branch from 8839438 to b16e642 Compare January 14, 2025 01:08
This change includes two fixes for the `broker_node_group_info.0.storage_info.0.ebs_storage_info.0.provisioned_throughput` block:

1. The update method now includes logic to handle the case where the block previously existed but has been removed. Removal will cause the nested `enabled` value to be set to `false` in the call to the Update API, disabling provisioned throughput.
2. Differences between an unset `provisioned_throughput` block and the default value returned from AWS (a nested block with `enabled` set to `false`) will now be suppressed. This should prevent perpetual differences in configurations which choose to omit this block to disable it.

```console
% make testacc PKG=kafka TESTS=TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/kafka/... -v -count 1 -parallel 20 -run='TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_'  -timeout 360m
2025/01/13 14:30:06 Initializing Terraform AWS Provider...

--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset (2195.72s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled (2200.75s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled (2207.31s)
PASS                                                                                                                                                                                    ok      github.com/hashicorp/terraform-provider-aws/internal/service/kafka      2213.740s
```

```console
% make testacc PKG=kafka TESTS=TestAccKafkaCluster_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/kafka/... -v -count 1 -parallel 20 -run='TestAccKafkaCluster_'  -timeout 360m -vet=off
2025/01/13 15:31:15 Initializing Terraform AWS Provider...

--- PASS: TestAccKafkaCluster_EncryptionInfoEncryptionInTransit_inCluster (1903.67s)
=== CONT  TestAccKafkaCluster_EncryptionInfoEncryptionInTransit_clientBroker
--- PASS: TestAccKafkaCluster_basic (1918.03s)
=== CONT  TestAccKafkaCluster_EncryptionInfo_encryptionAtRestKMSKeyARN
--- PASS: TestAccKafkaCluster_enhancedMonitoring (2052.26s)
=== CONT  TestAccKafkaCluster_tags
--- PASS: TestAccKafkaCluster_ClientAuthenticationTLS_certificateAuthorityARNs (2115.51s)
=== CONT  TestAccKafkaCluster_disappears
--- PASS: TestAccKafkaCluster_openMonitoring (2158.66s)
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled
--- PASS: TestAccKafkaCluster_loggingInfo (2195.49s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled (2215.01s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset (2226.82s)
--- PASS: TestAccKafkaCluster_storageMode (2819.54s)
--- PASS: TestAccKafkaCluster_numberOfBrokerNodes (2823.48s)
--- PASS: TestAccKafkaCluster_Info_revision (2827.43s)
--- PASS: TestAccKafkaCluster_EncryptionInfo_encryptionAtRestKMSKeyARN (2089.17s)
--- PASS: TestAccKafkaCluster_EncryptionInfoEncryptionInTransit_clientBroker (2105.36s)
--- PASS: TestAccKafkaCluster_kafkaVersionDowngrade (4013.28s)
--- PASS: TestAccKafkaCluster_ClientAuthenticationSASL_iam (4014.32s)
--- PASS: TestAccKafkaCluster_tags (1962.95s)
--- PASS: TestAccKafkaCluster_ClientAuthenticationSASL_scram (4018.83s)
--- PASS: TestAccKafkaCluster_disappears (1907.78s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled (1878.63s)
--- PASS: TestAccKafkaCluster_ClientAuthenticationTLS_initiallyNoAuthentication (4289.27s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_vpcConnectivity (5800.89s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_publicAccessSASLIAM (5961.32s)
--- PASS: TestAccKafkaCluster_kafkaVersionUpgradeWithInfo (7131.54s)
--- PASS: TestAccKafkaCluster_kafkaVersionUpgrade (7131.72s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_instanceType (7585.60s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/kafka      7590.972s
```
@jar-b jar-b force-pushed the b-msk_cluster-provisioned_throughput branch from b16e642 to bff62de Compare January 14, 2025 01:08
@jar-b jar-b marked this pull request as ready for review January 14, 2025 14:08
@jar-b jar-b requested a review from a team as a code owner January 14, 2025 14:08
@ewbankkit ewbankkit added the bug Addresses a defect in current functionality. label Jan 14, 2025
@ewbankkit ewbankkit self-assigned this Jan 14, 2025
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 🚀.

 % make testacc TESTARGS='-run=TestAccKafkaCluster_BrokerNodeGroupInfo_\|TestAccKafkaCluster_basic' PKG=kafka ACCTEST_PARALLELISM=3
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/kafka/... -v -count 1 -parallel 3  -run=TestAccKafkaCluster_BrokerNodeGroupInfo_\|TestAccKafkaCluster_basic -timeout 360m -vet=off
2025/01/14 13:48:41 Initializing Terraform AWS Provider...
=== RUN   TestAccKafkaCluster_basic
=== PAUSE TestAccKafkaCluster_basic
=== RUN   TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled
=== PAUSE TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled
=== RUN   TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled
=== PAUSE TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled
=== RUN   TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset
=== PAUSE TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset
=== RUN   TestAccKafkaCluster_BrokerNodeGroupInfo_instanceType
=== PAUSE TestAccKafkaCluster_BrokerNodeGroupInfo_instanceType
=== RUN   TestAccKafkaCluster_BrokerNodeGroupInfo_publicAccessSASLIAM
=== PAUSE TestAccKafkaCluster_BrokerNodeGroupInfo_publicAccessSASLIAM
=== RUN   TestAccKafkaCluster_BrokerNodeGroupInfo_vpcConnectivity
=== PAUSE TestAccKafkaCluster_BrokerNodeGroupInfo_vpcConnectivity
=== CONT  TestAccKafkaCluster_basic
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_instanceType
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled
--- PASS: TestAccKafkaCluster_basic (2190.62s)
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToDisabled (2273.50s)
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_vpcConnectivity
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_disabledToEnabled (2470.80s)
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_StorageInfo_enabledToUnset (2426.64s)
=== CONT  TestAccKafkaCluster_BrokerNodeGroupInfo_publicAccessSASLIAM
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_instanceType (7593.81s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_vpcConnectivity (6366.20s)
--- PASS: TestAccKafkaCluster_BrokerNodeGroupInfo_publicAccessSASLIAM (2279.39s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/kafka	9372.862s

@jar-b jar-b merged commit 4066211 into main Jan 15, 2025
56 checks passed
@jar-b jar-b deleted the b-msk_cluster-provisioned_throughput branch January 15, 2025 14:24
@github-actions github-actions bot added this to the v5.84.0 milestone Jan 15, 2025
terraform-aws-provider bot pushed a commit that referenced this pull request Jan 15, 2025
@github-actions github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Jan 16, 2025
Copy link

This functionality has been released in v5.84.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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/kafka Issues and PRs that pertain to the kafka service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
2 participants