Merge pull request #22217 from hashicorp/b-secretsmanager-secret-poli…

…cy-diffs

secretsmanager/secret: Fix equivalent policy diffs

.changelog/22217.txt

@@ -0,0 +1,11 @@
+```release-note:bug
+resource/aws_secretsmanager_secret: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
+```
+
+```release-note:bug
+resource/aws_secretsmanager_secret_policy: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
+```
+
+```release-note:bug
+resource/aws_iam_role: Fix eventual consistency problem with `arn` sometimes being a unique ID instead of the role ARN
+```

internal/acctest/acctest.go

@@ -620,6 +620,10 @@ func PreCheckMultipleRegion(t *testing.T, regions int) {
 	}
 
 	if regions >= 3 {
+		if thirdRegionPartition() == "aws-us-gov" || Partition() == "aws-us-gov" {
+			t.Skipf("wanted %d regions, partition (%s) only has 2 regions", regions, Partition())
+		}
+
 		if Region() == ThirdRegion() {
 			t.Fatalf("%s and %s must be set to different values for acceptance tests", conns.EnvVarDefaultRegion, conns.EnvVarThirdRegion)
 		}

internal/service/iam/wait.go

@@ -26,10 +26,12 @@ const (
 
 func waitRoleARNIsNotUniqueID(conn *iam.IAM, id string, role *iam.Role) (*iam.Role, error) {
 	stateConf := &resource.StateChangeConf{
-		Pending: []string{RoleStatusARNIsUniqueID, RoleStatusNotFound},
-		Target:  []string{RoleStatusARNIsARN},
-		Refresh: statusRoleCreate(conn, id, role),
-		Timeout: PropagationTimeout,
+		Pending:                   []string{RoleStatusARNIsUniqueID, RoleStatusNotFound},
+		Target:                    []string{RoleStatusARNIsARN},
+		Refresh:                   statusRoleCreate(conn, id, role),
+		Timeout:                   PropagationTimeout,
+		NotFoundChecks:            10,
+		ContinuousTargetOccurence: 5,
 	}
 
 	outputRaw, err := stateConf.WaitForState()

internal/service/secretsmanager/secret.go

@@ -71,6 +71,10 @@ func ResourceSecret() *schema.Resource {
 				Computed:         true,
 				ValidateFunc:     validation.StringIsJSON,
 				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 			"recovery_window_in_days": {
 				Type:     schema.TypeInt,
@@ -206,12 +210,18 @@ func resourceSecretCreate(d *schema.ResourceData, meta interface{}) error {
 	d.SetId(aws.StringValue(output.ARN))
 
 	if v, ok := d.GetOk("policy"); ok && v.(string) != "" {
+		policy, err := structure.NormalizeJsonString(v.(string))
+
+		if err != nil {
+			return fmt.Errorf("policy (%s) is invalid JSON: %w", v.(string), err)
+		}
+
 		input := &secretsmanager.PutResourcePolicyInput{
-			ResourcePolicy: aws.String(v.(string)),
+			ResourcePolicy: aws.String(policy),
 			SecretId:       aws.String(d.Id()),
 		}
 
-		err := resource.Retry(tfiam.PropagationTimeout, func() *resource.RetryError {
+		err = resource.Retry(tfiam.PropagationTimeout, func() *resource.RetryError {
 			_, err := conn.PutResourcePolicy(input)
 			if tfawserr.ErrMessageContains(err, secretsmanager.ErrCodeMalformedPolicyDocumentException,
 				"This resource policy contains an unsupported principal") {
@@ -269,28 +279,10 @@ func resourceSecretRead(d *schema.ResourceData, meta interface{}) error {
 		SecretId: aws.String(d.Id()),
 	}
 
-	var output *secretsmanager.DescribeSecretOutput
-
-	err := resource.Retry(PropagationTimeout, func() *resource.RetryError {
-		var err error
-
-		output, err = conn.DescribeSecret(input)
-
-		if d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) {
-			return resource.RetryableError(err)
-		}
-
-		if err != nil {
-			return resource.NonRetryableError(err)
-		}
-
-		return nil
+	outputRaw, err := tfresource.RetryWhenNotFound(PropagationTimeout, func() (interface{}, error) {
+		return conn.DescribeSecret(input)
 	})
 
-	if tfresource.TimedOut(err) {
-		output, err = conn.DescribeSecret(input)
-	}
-
 	if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) {
 		log.Printf("[WARN] Secrets Manager Secret (%s) not found, removing from state", d.Id())
 		d.SetId("")
@@ -301,6 +293,8 @@ func resourceSecretRead(d *schema.ResourceData, meta interface{}) error {
 		return fmt.Errorf("error reading Secrets Manager Secret (%s): %w", d.Id(), err)
 	}
 
+	output := outputRaw.(*secretsmanager.DescribeSecretOutput)
+
 	if output == nil {
 		return fmt.Errorf("error reading Secrets Manager Secret (%s): empty response", d.Id())
 	}
@@ -324,11 +318,15 @@ func resourceSecretRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if pOut.ResourcePolicy != nil {
-		policy, err := structure.NormalizeJsonString(aws.StringValue(pOut.ResourcePolicy))
+		policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(pOut.ResourcePolicy))
+
 		if err != nil {
-			return fmt.Errorf("policy contains an invalid JSON: %w", err)
+			return err
 		}
-		d.Set("policy", policy)
+
+		d.Set("policy", policyToSet)
+	} else {
+		d.Set("policy", "{}")
 	}
 
 	d.Set("rotation_enabled", output.RotationEnabled)
@@ -397,7 +395,7 @@ func resourceSecretUpdate(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if d.HasChange("policy") {
-		if v, ok := d.GetOk("policy"); ok && v.(string) != "" {
+		if v, ok := d.GetOk("policy"); ok && v.(string) != "" && v.(string) != "{}" {
 			policy, err := structure.NormalizeJsonString(v.(string))
 			if err != nil {
 				return fmt.Errorf("policy contains an invalid JSON: %w", err)

internal/service/secretsmanager/secret_policy.go

@@ -39,6 +39,10 @@ func ResourceSecretPolicy() *schema.Resource {
 				Required:         true,
 				ValidateFunc:     validation.StringIsJSON,
 				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 			"block_public_policy": {
 				Type:     schema.TypeBool,
@@ -51,8 +55,14 @@ func ResourceSecretPolicy() *schema.Resource {
 func resourceSecretPolicyCreate(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*conns.AWSClient).SecretsManagerConn
 
+	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
+
+	if err != nil {
+		return fmt.Errorf("policy (%s) is invalid JSON: %w", d.Get("policy").(string), err)
+	}
+
 	input := &secretsmanager.PutResourcePolicyInput{
-		ResourcePolicy: aws.String(d.Get("policy").(string)),
+		ResourcePolicy: aws.String(policy),
 		SecretId:       aws.String(d.Get("secret_arn").(string)),
 	}
 
@@ -61,11 +71,11 @@ func resourceSecretPolicyCreate(d *schema.ResourceData, meta interface{}) error
 	}
 
 	log.Printf("[DEBUG] Setting Secrets Manager Secret resource policy; %#v", input)
-	var res *secretsmanager.PutResourcePolicyOutput
+	var output *secretsmanager.PutResourcePolicyOutput
 
-	err := resource.Retry(tfiam.PropagationTimeout, func() *resource.RetryError {
+	err = resource.Retry(tfiam.PropagationTimeout, func() *resource.RetryError {
 		var err error
-		res, err = conn.PutResourcePolicy(input)
+		output, err = conn.PutResourcePolicy(input)
 		if tfawserr.ErrMessageContains(err, secretsmanager.ErrCodeMalformedPolicyDocumentException,
 			"This resource policy contains an unsupported principal") {
 			return resource.RetryableError(err)
@@ -76,13 +86,13 @@ func resourceSecretPolicyCreate(d *schema.ResourceData, meta interface{}) error
 		return nil
 	})
 	if tfresource.TimedOut(err) {
-		res, err = conn.PutResourcePolicy(input)
+		output, err = conn.PutResourcePolicy(input)
 	}
 	if err != nil {
 		return fmt.Errorf("error setting Secrets Manager Secret %q policy: %w", d.Id(), err)
 	}
 
-	d.SetId(aws.StringValue(res.ARN))
+	d.SetId(aws.StringValue(output.ARN))
 
 	return resourceSecretPolicyRead(d, meta)
 }
@@ -94,28 +104,10 @@ func resourceSecretPolicyRead(d *schema.ResourceData, meta interface{}) error {
 		SecretId: aws.String(d.Id()),
 	}
 
-	var res *secretsmanager.GetResourcePolicyOutput
-
-	err := resource.Retry(PropagationTimeout, func() *resource.RetryError {
-		var err error
-
-		res, err = conn.GetResourcePolicy(input)
-
-		if d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) {
-			return resource.RetryableError(err)
-		}
-
-		if err != nil {
-			return resource.NonRetryableError(err)
-		}
-
-		return nil
+	outputRaw, err := tfresource.RetryWhenNotFound(PropagationTimeout, func() (interface{}, error) {
+		return conn.GetResourcePolicy(input)
 	})
 
-	if tfresource.TimedOut(err) {
-		res, err = conn.GetResourcePolicy(input)
-	}
-
 	if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, secretsmanager.ErrCodeResourceNotFoundException) {
 		log.Printf("[WARN] Secrets Manager Secret Policy (%s) not found, removing from state", d.Id())
 		d.SetId("")
@@ -126,16 +118,20 @@ func resourceSecretPolicyRead(d *schema.ResourceData, meta interface{}) error {
 		return fmt.Errorf("error reading Secrets Manager Secret Policy (%s): %w", d.Id(), err)
 	}
 
-	if res == nil {
+	output := outputRaw.(*secretsmanager.GetResourcePolicyOutput)
+
+	if output == nil {
 		return fmt.Errorf("error reading Secrets Manager Secret Policy (%s): empty response", d.Id())
 	}
 
-	if res.ResourcePolicy != nil {
-		policy, err := structure.NormalizeJsonString(aws.StringValue(res.ResourcePolicy))
+	if output.ResourcePolicy != nil {
+		policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(output.ResourcePolicy))
+
 		if err != nil {
-			return fmt.Errorf("policy contains an invalid JSON: %w", err)
+			return err
 		}
-		d.Set("policy", policy)
+
+		d.Set("policy", policyToSet)
 	} else {
 		d.Set("policy", "")
 	}

internal/service/secretsmanager/secret_test.go

@@ -424,22 +424,24 @@ func TestAccSecretsManagerSecret_policy(t *testing.T) {
 		CheckDestroy: testAccCheckSecretDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccSecretConfig_Policy(rName),
+				Config: testAccSecretPolicyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckSecretExists(resourceName, &secret),
+					resource.TestCheckResourceAttr(resourceName, "description", "San Holo feat. Duskus"),
 					resource.TestMatchResourceAttr(resourceName, "policy",
 						regexp.MustCompile(`{"Action":"secretsmanager:GetSecretValue".+`)),
 				),
 			},
 			{
-				Config: testAccSecretConfig_Name(rName),
+				Config: testAccSecretPolicyEmptyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckSecretExists(resourceName, &secret),
-					resource.TestCheckResourceAttr(resourceName, "policy", ""),
+					resource.TestCheckResourceAttr(resourceName, "description", "Poliça"),
+					resource.TestCheckResourceAttr(resourceName, "policy", "{}"),
 				),
 			},
 			{
-				Config: testAccSecretConfig_Policy(rName),
+				Config: testAccSecretPolicyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckSecretExists(resourceName, &secret),
 					resource.TestMatchResourceAttr(resourceName, "policy",
@@ -801,47 +803,67 @@ resource "aws_secretsmanager_secret" "test" {
 `, rName)
 }
 
-func testAccSecretConfig_Policy(rName string) string {
+func testAccSecretPolicyConfig(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_iam_role" "test" {
   name = %[1]q
 
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "ec2.amazonaws.com"
+  assume_role_policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Principal = {
+        Service = "ec2.amazonaws.com"
       },
-      "Effect": "Allow",
-      "Sid": ""
-    }
-  ]
-}
-EOF
+      Effect = "Allow"
+      Sid    = ""
+    }]
+  })
 }
 
 resource "aws_secretsmanager_secret" "test" {
+  name        = %[1]q
+  description = "San Holo feat. Duskus"
+
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Sid    = "EnableAllPermissions"
+      Effect = "Allow"
+      Principal = {
+        AWS = aws_iam_role.test.arn
+      }
+      Action   = "secretsmanager:GetSecretValue"
+      Resource = "*"
+    }]
+  })
+}
+`, rName)
+}
+
+func testAccSecretPolicyEmptyConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_iam_role" "test" {
   name = %[1]q
 
-  policy = <<POLICY
-{
-  "Version": "2012-10-17",
-  "Statement": [
-    {
-      "Sid": "EnableAllPermissions",
-      "Effect": "Allow",
-      "Principal": {
-        "AWS": "${aws_iam_role.test.arn}"
+  assume_role_policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Action = "sts:AssumeRole"
+      Principal = {
+        Service = "ec2.amazonaws.com"
       },
-      "Action": "secretsmanager:GetSecretValue",
-      "Resource": "*"
-    }
-  ]
+      Effect = "Allow"
+      Sid    = ""
+    }]
+  })
 }
-POLICY
+
+resource "aws_secretsmanager_secret" "test" {
+  name        = %[1]q
+  description = "Poliça"
+
+  policy = "{}"
 }
 `, rName)
 }