Merge pull request #43520 from asungur/b-aws_ec_user_group-skip-removed-user

jar-b · web-flow · commit bda755ce9004 · 2025-09-17T09:33:22.000-04:00
aws_elasticache_user_group: Skip deleted users during user group update
diff --git a/.changelog/43520.txt b/.changelog/43520.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_elasticache_user_group: Ignore `InvalidParameterValue: User xxx is not a member of user group xxx` errors during group modification
+```
diff --git a/internal/service/elasticache/user_group.go b/internal/service/elasticache/user_group.go
@@ -173,7 +173,7 @@ func resourceUserGroupUpdate(ctx context.Context, d *schema.ResourceData, meta a
 
 		_, err := conn.ModifyUserGroup(ctx, input)
 
-		if err != nil {
+		if err != nil && !errs.IsAErrorMessageContains[*awstypes.InvalidParameterValueException](err, "is not a member of user group") {
 			return sdkdiag.AppendErrorf(diags, "updating ElastiCache User Group (%q): %s", d.Id(), err)
 		}
 
diff --git a/internal/service/elasticache/user_group_test.go b/internal/service/elasticache/user_group_test.go
@@ -10,6 +10,7 @@ import (
 
 	awstypes "github.com/aws/aws-sdk-go-v2/service/elasticache/types"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-provider-aws/internal/acctest"
 	"github.com/hashicorp/terraform-provider-aws/internal/retry"
@@ -52,6 +53,8 @@ func TestAccElastiCacheUserGroup_update(t *testing.T) {
 	var userGroup awstypes.UserGroup
 	rName := acctest.RandomWithPrefix(t, "tf-acc")
 	resourceName := "aws_elasticache_user_group.test"
+	user1ResourceName := "aws_elasticache_user.test1"
+	user2ResourceName := "aws_elasticache_user.test2"
 
 	acctest.ParallelTest(ctx, t, resource.TestCase{
 		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
@@ -76,7 +79,105 @@ func TestAccElastiCacheUserGroup_update(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "user_group_id", rName),
 					resource.TestCheckResourceAttr(resourceName, names.AttrEngine, "redis"),
 				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
+						plancheck.ExpectResourceAction(user1ResourceName, plancheck.ResourceActionNoop),
+						plancheck.ExpectResourceAction(user2ResourceName, plancheck.ResourceActionNoop),
+					},
+				},
 			},
+			{
+				Config: testAccUserGroupConfig_basic(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckUserGroupExists(ctx, t, resourceName, &userGroup),
+					resource.TestCheckResourceAttr(resourceName, "user_ids.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "user_group_id", rName),
+					resource.TestCheckResourceAttr(resourceName, names.AttrEngine, "redis"),
+				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
+						plancheck.ExpectResourceAction(user1ResourceName, plancheck.ResourceActionNoop),
+						plancheck.ExpectResourceAction(user2ResourceName, plancheck.ResourceActionNoop),
+					},
+				},
+			},
+		},
+	})
+}
+
+func TestAccElastiCacheUserGroup_rotate(t *testing.T) {
+	ctx := acctest.Context(t)
+	var userGroup awstypes.UserGroup
+	rName := acctest.RandomWithPrefix(t, "tf-acc")
+	resourceName := "aws_elasticache_user_group.test"
+	user1ResourceName := "aws_elasticache_user.test1"
+	user2ResourceName := "aws_elasticache_user.test2"
+
+	acctest.ParallelTest(ctx, t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.ElastiCacheServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckUserGroupDestroy(ctx, t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccUserGroupConfig_basic(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckUserGroupExists(ctx, t, resourceName, &userGroup),
+					resource.TestCheckResourceAttr(resourceName, "user_ids.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "user_group_id", rName),
+					resource.TestCheckResourceAttr(resourceName, names.AttrEngine, "redis"),
+				),
+			},
+			{
+				Config: testAccUserGroupConfig_rotate(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckUserGroupExists(ctx, t, resourceName, &userGroup),
+					resource.TestCheckResourceAttr(resourceName, "user_ids.#", "2"),
+					resource.TestCheckResourceAttr(resourceName, "user_group_id", rName),
+					resource.TestCheckResourceAttr(resourceName, names.AttrEngine, "redis"),
+				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
+						plancheck.ExpectResourceAction(user1ResourceName, plancheck.ResourceActionNoop),
+						plancheck.ExpectResourceAction(user2ResourceName, plancheck.ResourceActionReplace),
+					},
+				},
+			},
+			{
+				Config: testAccUserGroupConfig_basic(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckUserGroupExists(ctx, t, resourceName, &userGroup),
+					resource.TestCheckResourceAttr(resourceName, "user_ids.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "user_group_id", rName),
+					resource.TestCheckResourceAttr(resourceName, names.AttrEngine, "redis"),
+				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
+						plancheck.ExpectResourceAction(user1ResourceName, plancheck.ResourceActionNoop),
+						plancheck.ExpectResourceAction(user2ResourceName, plancheck.ResourceActionReplace),
+					},
+				},
+			},
+		},
+	})
+}
+
+func TestAccElastiCacheUserGroup_engineValkey(t *testing.T) {
+	ctx := acctest.Context(t)
+	var userGroup awstypes.UserGroup
+	rName := acctest.RandomWithPrefix(t, "tf-acc")
+	resourceName := "aws_elasticache_user_group.test"
+
+	acctest.ParallelTest(ctx, t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.ElastiCacheServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckUserGroupDestroy(ctx, t),
+		Steps: []resource.TestStep{
 			{
 				Config: testAccUserGroupConfig_basic(rName),
 				Check: resource.ComposeTestCheckFunc(
@@ -168,6 +269,11 @@ func TestAccElastiCacheUserGroup_disappears(t *testing.T) {
 					acctest.CheckResourceDisappears(ctx, acctest.Provider, tfelasticache.ResourceUserGroup(), resourceName),
 				),
 				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+					},
+				},
 			},
 		},
 	})
@@ -246,6 +352,32 @@ resource "aws_elasticache_user_group" "test" {
 `, rName))
 }
 
+func testAccUserGroupConfig_rotate(rName string) string {
+	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
+resource "aws_elasticache_user" "test1" {
+  user_id       = "%[1]s-1"
+  user_name     = "default"
+  access_string = "on ~app::* -@all +@read +@hash +@bitmap +@geo -setbit -bitfield -hset -hsetnx -hmset -hincrby -hincrbyfloat -hdel -bitop -geoadd -georadius -georadiusbymember"
+  engine        = "REDIS"
+  passwords     = ["password123456789"]
+}
+
+resource "aws_elasticache_user" "test2" {
+  user_id       = "%[1]s-3"
+  user_name     = "username1"
+  access_string = "on ~app::* -@all +@read +@hash +@bitmap +@geo -setbit -bitfield -hset -hsetnx -hmset -hincrby -hincrbyfloat -hdel -bitop -geoadd -georadius -georadiusbymember"
+  engine        = "REDIS"
+  passwords     = ["password123456789"]
+}
+
+resource "aws_elasticache_user_group" "test" {
+  user_group_id = %[1]q
+  engine        = "REDIS"
+  user_ids      = [aws_elasticache_user.test1.user_id, aws_elasticache_user.test2.user_id]
+}
+`, rName))
+}
+
 func testAccUserGroupConfig_multiple(rName string) string {
 	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
 resource "aws_elasticache_user" "test1" {
@@ -282,6 +414,14 @@ resource "aws_elasticache_user" "test1" {
   passwords     = ["password123456789"]
 }
 
+resource "aws_elasticache_user" "test2" {
+  user_id       = "%[1]s-2"
+  user_name     = "username1"
+  access_string = "on ~app::* -@all +@read +@hash +@bitmap +@geo -setbit -bitfield -hset -hsetnx -hmset -hincrby -hincrbyfloat -hdel -bitop -geoadd -georadius -georadiusbymember"
+  engine        = "REDIS"
+  passwords     = ["password123456789"]
+}
+
 resource "aws_elasticache_user_group" "test" {
   user_group_id = %[1]q
   engine        = "VALKEY"

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+```release-note:bug
	`2`	+resource/aws_elasticache_user_group: Ignore `InvalidParameterValue: User xxx is not a member of user group xxx` errors during group modification
	`3`	+```
Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ func resourceUserGroupUpdate(ctx context.Context, d *schema.ResourceData, meta a`
`173`	`173`
`174`	`174`	`_, err := conn.ModifyUserGroup(ctx, input)`
`175`	`175`
`176`		`- if err != nil {`
	`176`	`+ if err != nil && !errs.IsAErrorMessageContains[*awstypes.InvalidParameterValueException](err, "is not a member of user group") {`
`177`	`177`	`return sdkdiag.AppendErrorf(diags, "updating ElastiCache User Group (%q): %s", d.Id(), err)`
`178`	`178`	`}`
`179`	`179`