s3/bucket policy: Improve diffs with policies

.changelog/28855.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_s3_bucket: Improve refresh to avoid unnecessary diffs in `policy`
+```
+
+```release-note:bug
+resource/aws_s3_bucket_policy: Improve refresh to avoid unnecessary diffs in `policy`
+```

internal/service/s3/bucket.go

@@ -136,12 +136,17 @@ func ResourceBucket() *schema.Resource {
 			},
 
 			"policy": {
-				Type:             schema.TypeString,
-				Optional:         true,
-				Computed:         true,
-				Deprecated:       "Use the aws_s3_bucket_policy resource instead",
-				ValidateFunc:     validation.StringIsJSON,
-				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				Type:                  schema.TypeString,
+				Optional:              true,
+				Computed:              true,
+				Deprecated:            "Use the aws_s3_bucket_policy resource instead",
+				ValidateFunc:          validation.StringIsJSON,
+				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
+				DiffSuppressOnRefresh: true,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 
 			"cors_rule": {
@@ -987,7 +992,12 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if output, ok := pol.(*s3.GetBucketPolicyOutput); ok {
-		d.Set("policy", output.Policy)
+		policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(output.Policy))
+		if err != nil {
+			return fmt.Errorf("while setting policy (%s), encountered: %w", aws.StringValue(output.Policy), err)
+		}
+
+		d.Set("policy", policyToSet)
 	} else {
 		d.Set("policy", nil)
 	}
@@ -1927,7 +1937,6 @@ func resourceBucketInternalObjectLockConfigurationUpdate(conn *s3.S3, d *schema.
 
 func resourceBucketInternalPolicyUpdate(conn *s3.S3, d *schema.ResourceData) error {
 	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
-
 	if err != nil {
 		return fmt.Errorf("policy (%s) is an invalid JSON: %w", policy, err)
 	}

internal/service/s3/bucket_policy.go

@@ -1,6 +1,7 @@
 package s3
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"time"
@@ -13,8 +14,10 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
+	"github.com/hashicorp/terraform-provider-aws/internal/create"
 	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
+	"github.com/hashicorp/terraform-provider-aws/names"
 )
 
 func ResourceBucketPolicy() *schema.Resource {
@@ -35,10 +38,15 @@ func ResourceBucketPolicy() *schema.Resource {
 			},
 
 			"policy": {
-				Type:             schema.TypeString,
-				Required:         true,
-				ValidateFunc:     validation.StringIsJSON,
-				DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
+				Type:                  schema.TypeString,
+				Required:              true,
+				ValidateFunc:          validation.StringIsJSON,
+				DiffSuppressFunc:      verify.SuppressEquivalentPolicyDiffs,
+				DiffSuppressOnRefresh: true,
+				StateFunc: func(v interface{}) string {
+					json, _ := structure.NormalizeJsonString(v)
+					return json
+				},
 			},
 		},
 	}
@@ -50,7 +58,6 @@ func resourceBucketPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	bucket := d.Get("bucket").(string)
 
 	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
-
 	if err != nil {
 		return fmt.Errorf("policy (%s) is an invalid JSON: %w", policy, err)
 	}
@@ -98,21 +105,17 @@ func resourceBucketPolicyRead(d *schema.ResourceData, meta interface{}) error {
 		return nil
 	}
 
-	v := ""
-	if err == nil && pol.Policy != nil {
-		v = aws.StringValue(pol.Policy)
-	}
-
-	policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), v)
-
 	if err != nil {
-		return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
+		return create.Error(names.S3, create.ErrActionReading, "Bucket Policy", d.Id(), err)
 	}
 
-	policyToSet, err = structure.NormalizeJsonString(policyToSet)
+	if pol.Policy == nil {
+		return create.Error(names.S3, create.ErrActionReading, "Bucket Policy", d.Id(), errors.New("empty policy returned"))
+	}
 
+	policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(pol.Policy))
 	if err != nil {
-		return fmt.Errorf("policy (%s) is an invalid JSON: %w", policyToSet, err)
+		return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
 	}
 
 	if err := d.Set("policy", policyToSet); err != nil {

internal/service/s3/bucket_test.go

@@ -409,7 +409,7 @@ func TestAccS3Bucket_Duplicate_UsEast1AltAccount(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config:      testAccBucketConfig_duplicateAltAccount(endpoints.UsEast1RegionID, bucketName),
-				ExpectError: regexp.MustCompile(s3.ErrCodeBucketAlreadyExists),
+				ExpectError: regexp.MustCompile(s3.ErrCodeBucketAlreadyOwnedByYou),
 			},
 		},
 	})