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_elasticache_replication_group: add auth_token_update_strategy argument #34460

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Nov 17, 2023

Description

This optional argument will have a default value of ROTATE, as this was the previously hardcoded value sent during modify requests in which the auth_token was changed. Practitioners will now have the ability to explicitly use the SET and DELETE strategies as well. A future major version may remove the default ROTATE value once backwards compatibility can safely be broken.

Also adds an example to the replication group documentation which explicitly indicates that using the ROTATE update strategy when adding an initial auth_token to a group with no authentication will result in both the new password and no password being supported. This can be mitigated by using the SET strategy instead.

Relations

Closes #11524
Closes #33439
Closes #31710

References

Output from Acceptance Testing

% make testacc PKG=elasticache TESTS="TestAccElastiCacheReplicationGroup_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/elasticache/... -v -count 1 -parallel 20 -run='TestAccElastiCacheReplicationGroup_'  -timeout 360m

--- PASS: TestAccElastiCacheReplicationGroup_clusteringAndCacheNodesCausesError (37.89s)
=== CONT  TestAccElastiCacheReplicationGroup_Engine_Redis_LogDeliveryConfigurations_ClusterMode_Disabled
--- PASS: TestAccElastiCacheReplicationGroup_Validation_noNodeType (60.51s)
=== CONT  TestAccElastiCacheReplicationGroup_dataTiering
--- PASS: TestAccElastiCacheReplicationGroup_basic (947.34s)
=== CONT  TestAccElastiCacheReplicationGroup_GlobalReplicationGroupIDClusterModeValidation_numNodeGroupsOnSecondary
--- PASS: TestAccElastiCacheReplicationGroup_ClusterMode_singleNode (1004.69s)
=== CONT  TestAccElastiCacheReplicationGroup_GlobalReplicationGroupIDClusterMode_basic
--- PASS: TestAccElastiCacheReplicationGroup_enableSnapshotting (1032.43s)
=== CONT  TestAccElastiCacheReplicationGroup_GlobalReplicationGroupID_disappears
--- PASS: TestAccElastiCacheReplicationGroup_tags (1284.05s)
=== CONT  TestAccElastiCacheReplicationGroup_finalSnapshot
--- PASS: TestAccElastiCacheReplicationGroup_Validation_globalReplicationGroupIdAndNodeType (1292.54s)
=== CONT  TestAccElastiCacheReplicationGroup_autoMinorVersionUpgrade
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClustersFailover_autoFailoverEnabled (1889.39s)
=== CONT  TestAccElastiCacheReplicationGroup_tagWithOtherModification
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClustersMemberClusterDisappearsRemoveMemberCluster_atTargetSize (2055.15s)
=== CONT  TestAccElastiCacheReplicationGroup_multiAzNotInVPC
--- PASS: TestAccElastiCacheReplicationGroup_enableAuthTokenTransitEncryption (2062.61s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroupsAndReplicasPerNodeGroup_scaleDown
--- PASS: TestAccElastiCacheReplicationGroup_enableAtRestEncryption (2115.65s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroupsAndReplicasPerNodeGroup_scaleUp
--- PASS: TestAccElastiCacheReplicationGroup_dataTiering (2118.34s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterMode_updateReplicasPerNodeGroup
--- PASS: TestAccElastiCacheReplicationGroup_GlobalReplicationGroupIDClusterModeValidation_numNodeGroupsOnSecondary (1277.53s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroups_scaleDown
--- PASS: TestAccElastiCacheReplicationGroup_Engine_Redis_LogDeliveryConfigurations_ClusterMode_Disabled (2225.07s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroups_scaleUp
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClusters_multiAZEnabled (2322.03s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterMode_nonClusteredParameterGroup
--- PASS: TestAccElastiCacheReplicationGroup_autoMinorVersionUpgrade (1071.04s)
=== CONT  TestAccElastiCacheReplicationGroup_ClusterMode_basic
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClustersMemberClusterDisappears_addMemberCluster (2495.75s)
=== CONT  TestAccElastiCacheReplicationGroup_networkType
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClustersFailover_autoFailoverDisabled (2535.14s)
=== CONT  TestAccElastiCacheReplicationGroup_ipDiscovery
=== NAME  TestAccElastiCacheReplicationGroup_tagWithOtherModification
    replication_group_test.go:1945: Step 2/2 error: Error running apply: exit status 1

        Error: updating ElastiCache Replication Group (tf-acc-test-3083903035324591260): InvalidReplicationGroupState: Replication group must be in available state to modify.
                status code: 400, request id: cc1064fe-f4be-4690-a892-424cdb6a4344

          with aws_elasticache_replication_group.test,
          on terraform_plugin_test.tf line 8, in resource "aws_elasticache_replication_group" "test":
           8: resource "aws_elasticache_replication_group" "test" {

--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClustersMemberClusterDisappears_noChange (2684.81s)
=== CONT  TestAccElastiCacheReplicationGroup_ValidationMultiAz_noAutomaticFailover
--- PASS: TestAccElastiCacheReplicationGroup_ValidationMultiAz_noAutomaticFailover (3.88s)
=== CONT  TestAccElastiCacheReplicationGroup_deprecatedAvailabilityZones_multiAzInVPC
--- PASS: TestAccElastiCacheReplicationGroup_finalSnapshot (1414.98s)
=== CONT  TestAccElastiCacheReplicationGroup_multiAzInVPC
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClustersMemberClusterDisappearsRemoveMemberCluster_scaleDown (2848.80s)
=== CONT  TestAccElastiCacheReplicationGroup_multiAzNotInVPC_repeated
--- FAIL: TestAccElastiCacheReplicationGroup_tagWithOtherModification (1056.32s)
=== CONT  TestAccElastiCacheReplicationGroup_useCMKKMSKeyID
--- PASS: TestAccElastiCacheReplicationGroup_multiAzNotInVPC (1356.38s)
=== CONT  TestAccElastiCacheReplicationGroup_updateDescription
--- PASS: TestAccElastiCacheReplicationGroup_Engine_Redis_LogDeliveryConfigurations_ClusterMode_Enabled (3454.57s)
=== CONT  TestAccElastiCacheReplicationGroup_vpc
--- PASS: TestAccElastiCacheReplicationGroup_GlobalReplicationGroupID_basic (3505.68s)
=== CONT  TestAccElastiCacheReplicationGroup_authToken
--- PASS: TestAccElastiCacheReplicationGroup_multiAzInVPC (1103.79s)
=== CONT  TestAccElastiCacheReplicationGroup_updateParameterGroup
--- PASS: TestAccElastiCacheReplicationGroup_GlobalReplicationGroupID_disappears (2851.07s)
=== CONT  TestAccElastiCacheReplicationGroup_updateNodeSize
--- PASS: TestAccElastiCacheReplicationGroup_ClusterMode_nonClusteredParameterGroup (1650.31s)
=== CONT  TestAccElastiCacheReplicationGroup_updateUserGroups
--- PASS: TestAccElastiCacheReplicationGroup_NumberCacheClusters_basic (3993.21s)
=== CONT  TestAccElastiCacheReplicationGroup_updateMaintenanceWindow
--- PASS: TestAccElastiCacheReplicationGroup_ClusterMode_basic (1703.58s)
=== CONT  TestAccElastiCacheReplicationGroup_uppercase
--- PASS: TestAccElastiCacheReplicationGroup_useCMKKMSKeyID (1156.37s)
=== CONT  TestAccElastiCacheReplicationGroup_EngineVersion_v7
--- PASS: TestAccElastiCacheReplicationGroup_multiAzNotInVPC_repeated (1256.58s)
=== CONT  TestAccElastiCacheReplicationGroup_disappears
--- PASS: TestAccElastiCacheReplicationGroup_ipDiscovery (1573.86s)
=== CONT  TestAccElastiCacheReplicationGroup_basic_v5
--- PASS: TestAccElastiCacheReplicationGroup_GlobalReplicationGroupID_full (4175.23s)
=== CONT  TestAccElastiCacheReplicationGroup_EngineVersion_6xToRealVersion
--- PASS: TestAccElastiCacheReplicationGroup_updateDescription (1257.74s)
=== CONT  TestAccElastiCacheReplicationGroup_EngineVersion_update
--- PASS: TestAccElastiCacheReplicationGroup_networkType (2173.66s)
--- PASS: TestAccElastiCacheReplicationGroup_vpc (1279.68s)
--- PASS: TestAccElastiCacheReplicationGroup_updateMaintenanceWindow (795.80s)
--- PASS: TestAccElastiCacheReplicationGroup_deprecatedAvailabilityZones_multiAzInVPC (2103.31s)
--- PASS: TestAccElastiCacheReplicationGroup_uppercase (729.22s)
--- PASS: TestAccElastiCacheReplicationGroup_updateParameterGroup (1035.91s)
--- PASS: TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroupsAndReplicasPerNodeGroup_scaleDown (2782.05s)
--- PASS: TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroups_scaleDown (2622.27s)
--- PASS: TestAccElastiCacheReplicationGroup_ClusterMode_updateReplicasPerNodeGroup (2670.38s)
--- PASS: TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroups_scaleUp (2587.92s)
--- PASS: TestAccElastiCacheReplicationGroup_ClusterModeUpdateNumNodeGroupsAndReplicasPerNodeGroup_scaleUp (2754.33s)
--- PASS: TestAccElastiCacheReplicationGroup_disappears (829.48s)
--- PASS: TestAccElastiCacheReplicationGroup_basic_v5 (899.34s)
--- PASS: TestAccElastiCacheReplicationGroup_EngineVersion_v7 (920.23s)
--- PASS: TestAccElastiCacheReplicationGroup_updateUserGroups (1051.12s)
--- PASS: TestAccElastiCacheReplicationGroup_GlobalReplicationGroupIDClusterMode_basic (4442.92s)
--- PASS: TestAccElastiCacheReplicationGroup_EngineVersion_6xToRealVersion (1306.76s)
--- PASS: TestAccElastiCacheReplicationGroup_updateNodeSize (1862.76s)
--- PASS: TestAccElastiCacheReplicationGroup_authToken (2681.78s)
--- PASS: TestAccElastiCacheReplicationGroup_EngineVersion_update (6700.42s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-aws/internal/service/elasticache        11373.196s

Failure is unrelated to this change.

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 size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/elasticache Issues and PRs that pertain to the elasticache service. labels Nov 17, 2023
@terraform-aws-provider terraform-aws-provider bot added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Nov 17, 2023
…rgument

This optional argument will have a default value of `ROTATE`, as this was
the previously hardcoded value sent during modify requests in which the
`auth_token` was changed. Practitioners will now have the ability to explicitly
use the `SET` and `DELETE` strategies as well.
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=TestAccElastiCacheReplicationGroup_authToken' PKG=elasticache
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/elasticache/... -v -count 1 -parallel 20  -run=TestAccElastiCacheReplicationGroup_authToken -timeout 360m
=== RUN   TestAccElastiCacheReplicationGroup_authToken
=== PAUSE TestAccElastiCacheReplicationGroup_authToken
=== CONT  TestAccElastiCacheReplicationGroup_authToken
--- PASS: TestAccElastiCacheReplicationGroup_authToken (2039.51s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/elasticache	2046.126s

@jar-b jar-b merged commit 940196f into main Nov 21, 2023
53 checks passed
@jar-b jar-b deleted the f-elasticache-auth_token_update_strategy branch November 21, 2023 14:39
@github-actions github-actions bot added this to the v5.27.0 milestone Nov 21, 2023
github-actions bot pushed a commit that referenced this pull request Nov 21, 2023
Copy link

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

@kvaidas
Copy link

kvaidas commented Nov 28, 2023

It looks like this invalidates the description of auth_token_update_strategy which says

Can be specified only if transit_encryption_enabled = true.

Because even if transit_encryption_enabled is set to false, auth_token_update_strategy is being set.

@ut0mt8
Copy link

ut0mt8 commented Nov 28, 2023

It looks like this invalidates the description of auth_token_update_strategy which says

Can be specified only if transit_encryption_enabled = true.

Because even if transit_encryption_enabled is set to false, auth_token_update_strategy is being set.

this is indeed broken..

@jar-b
Copy link
Member Author

jar-b commented Nov 29, 2023

Thank you for the suggestion. The original intent was to indicate that when setting the auth_token and auth_token_update_strategy arguments, transit_encryption_enabled must be true (an AWS requirement). Because we had to include a default value to preserve backwards compatibility for the 5.X series, this may now be confusing since a value for auth_token_update_strategy will always be set in state (though not sent in a create/update call unless auth_token is also set).

For now we can remove this sentence from the documentation to avoid any confusion. Once we remove the default value in v6.0.0 (#34496), we can revisit how to properly describe the relationship between these three arguments.

@jar-b
Copy link
Member Author

jar-b commented Nov 29, 2023

Documentation has been updated in #34633. Thanks again!

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 Dec 30, 2023
@justinretzolk justinretzolk added the enhancement Requests to existing resources that expand the functionality or scope. label Feb 10, 2024
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. enhancement Requests to existing resources that expand the functionality or scope. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. service/elasticache Issues and PRs that pertain to the elasticache 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
5 participants