Skip to content

aws_elasticache_user_group : filter out already removed users during a user group update - #49948

Open
asungur wants to merge 3 commits into
hashicorp:mainfrom
asungur:b-aws_ec_user_group-dropped-user-additions
Open

aws_elasticache_user_group : filter out already removed users during a user group update#49948
asungur wants to merge 3 commits into
hashicorp:mainfrom
asungur:b-aws_ec_user_group-dropped-user-additions

Conversation

@asungur

@asungur asungur commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the library.

Changes to Security Controls

No changes to security controls in this pull request.

Description

resourceUserGroupUpdate sends additions and removals in a single ModifyUserGroup call. When an aws_elasticache_user is destroyed earlier in the same apply, ElastiCache detaches that user from its user groups automatically, so the removal the provider computed from state is already a no-op by the time the group update runs. AWS then rejects the entire request with InvalidParameterValueException: ... is not a member of user group.

The error suppression added in #43520 made removals idempotent, but it suppresses the failure of the whole request additions included. Two further things hide the failure:

  1. Because the call was rejected, the group never enters modifying. waitUserGroupUpdated (Pending: [modifying]Target: [active]) sees active after its 30s Delay and reports the update as successful.
  2. resourceUserGroupRead then writes back the real membership, so state and remote agree at the end of the apply.

The result is an apply that reports success while the group is missing the user it was told to add. The next plan proposes the addition again and a second apply converges, so the practical symptom is a converge-in-two-applies bug with no error surfaced in the first run.

Fix: reconcile the removal set against live membership before building the request, using the existing findUserGroupByID. Users AWS has already detached are dropped from UserIdsToRemove, so the request is one AWS will accept and the addition goes through.

With the removal set accurate, the blanket error suppression is no longer load-bearing and is removed. The #43520 scenario still behaves correctly: a removal for an already-detached user filters down to an empty set, and a new modified guard skips the API call and the waiter entirely rather than sending a no-op modification. That path now costs one fewer failed API call and skips a 30s waiter.

The add and the remove stay in a single atomic call.

One residual trade-off, called out for reviewers: because the membership read and the modify are separate calls, a removal that races between them (an out-of-band detach, or DescribeUserGroups lagging a DeleteUser) now errors instead of being tolerated. That window is much narrower than the one being fixed, erroring is the safer failure mode, and a re-apply converges.

Relations

Closes #49945

References

Introduced by 95705ee in #43520 , released in v6.14.0.

Output from Acceptance Testing

terraform-provider-aws git:(b-aws_ec_user_group-dropped-user-additions) make testacc TESTS=TestAccElastiCacheUserGroup_replaceMember PKG=elasticache
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
make: Validating schemas
ok      github.com/hashicorp/terraform-provider-aws/internal/provider/sdkv2     (cached)
ok      github.com/hashicorp/terraform-provider-aws/internal/provider/framework (cached)
make: Running acceptance tests on branch: 🌿 b-aws_ec_user_group-dropped-user-additions 🌿...
TF_ACC=1 go1.26.6 test ./internal/service/elasticache/... -v -count 1 -parallel 20 -run='TestAccElastiCacheUserGroup_replaceMember'  -timeout 360m -vet=off -buildvcs=false
2026/09/10 18:21:44 Creating Terraform AWS Provider (SDKv2-style)...
2026/09/10 18:21:44 Initializing Terraform AWS Provider (SDKv2-style)...
=== RUN   TestAccElastiCacheUserGroup_replaceMember
=== PAUSE TestAccElastiCacheUserGroup_replaceMember
=== CONT  TestAccElastiCacheUserGroup_replaceMember
--- PASS: TestAccElastiCacheUserGroup_replaceMember (394.07s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/elasticache        400.643s
...

@asungur
asungur requested a review from a team as a code owner September 10, 2026 17:33
@github-actions

Copy link
Copy Markdown
Contributor

Community Guidelines

This comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀

Voting for Prioritization

  • Please vote on this Pull Request by adding a 👍 reaction to the original post to help the community and maintainers prioritize it.
  • Please see our prioritization guide for additional information on how the maintainers handle prioritization.
  • Please do not leave +1 or other comments that do not add relevant new information or questions; they generate extra noise for others following the Pull Request and do not help prioritize the request.

Pull Request Authors

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • 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 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

✅ Thank you for correcting the previously detected issues! The maintainers appreciate your efforts to make the review process as smooth as possible.

@github-actions github-actions Bot added needs-triage Waiting for first response or review from a maintainer. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/elasticache Issues and PRs that pertain to the elasticache service. size/M Managed by automation to categorize the size of a PR. labels Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-triage Waiting for first response or review from a maintainer. service/elasticache Issues and PRs that pertain to the elasticache service. size/M 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.

aws_elasticache_user_group silently drops user additions when the same update removes an already-detached user

1 participant