Merge pull request #23893 from hashicorp/b-s3-bucket-lifecycle-config…

…uration-filter-diffsuppressfunc

r/s3_bucket_lifecycle_configuration: use custom `DiffSuppressFunc` on `rule.filter` parameter

.changelog/23893.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_s3_bucket_lifecycle_configuration: Prevent `MalformedXML` errors when handling diffs in `rule.filter`
+```

internal/service/s3/bucket_lifecycle_configuration.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"reflect"
+	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -95,7 +96,7 @@ func ResourceBucketLifecycleConfiguration() *schema.Resource {
 							// we apply the Default behavior from v3.x of the provider (Filter with empty string Prefix),
 							// which will thus return a Filter in the GetBucketLifecycleConfiguration request and
 							// require diff suppression.
-							DiffSuppressFunc: verify.SuppressMissingOptionalConfigurationBlock,
+							DiffSuppressFunc: suppressMissingFilterConfigurationBlock,
 							MaxItems:         1,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
@@ -420,3 +421,26 @@ func resourceBucketLifecycleConfigurationDelete(ctx context.Context, d *schema.R
 
 	return nil
 }
+
+// suppressMissingFilterConfigurationBlock suppresses the diff that results from an omitted
+// filter configuration block and one returned from the S3 API.
+// To work around the issue, https://github.com/hashicorp/terraform-plugin-sdk/issues/743,
+// this method only looks for changes in the "filter.#" value and not its nested fields
+// which are incorrectly suppressed when using the verify.SuppressMissingOptionalConfigurationBlock method.
+func suppressMissingFilterConfigurationBlock(k, old, new string, d *schema.ResourceData) bool {
+	if strings.HasSuffix(k, "filter.#") {
+		o, n := d.GetChange(k)
+		oVal, nVal := o.(int), n.(int)
+
+		if oVal == 1 && nVal == 0 {
+			return true
+		}
+
+		if oVal == 1 && nVal == 1 {
+			return old == "1" && new == "0"
+		}
+
+		return false
+	}
+	return false
+}

internal/service/s3/bucket_lifecycle_configuration_test.go

@@ -843,6 +843,42 @@ func TestAccS3BucketLifecycleConfiguration_EmptyFilter_NonCurrentVersions(t *tes
 	})
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/23884
+func TestAccS3BucketLifecycleConfiguration_Update_filterWithAndToFilterWithPrefix(t *testing.T) {
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_s3_bucket_lifecycle_configuration.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.PreCheck(t) },
+		ErrorCheck:   acctest.ErrorCheck(t, s3.EndpointsID),
+		Providers:    acctest.Providers,
+		CheckDestroy: testAccCheckBucketLifecycleConfigurationDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccBucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanAndPrefixConfig(rName, "prefix1"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "rule.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.0.and.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.0.and.0.object_size_greater_than", "300"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.0.and.0.prefix", "prefix1"),
+				),
+			},
+			{
+				Config: testAccBucketLifecycleConfiguration_Filter_PrefixConfig(rName, "prefix2"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "rule.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.0.and.#", "0"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.0.prefix", "prefix2"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckBucketLifecycleConfigurationDestroy(s *terraform.State) error {
 	conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn
 
@@ -1584,3 +1620,66 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
 }
 `, rName, date, sizeGreaterThan, sizeLessThan)
 }
+
+func testAccBucketLifecycleConfiguration_Filter_ObjectSizeGreaterThanAndPrefixConfig(rName, prefix string) string {
+	return fmt.Sprintf(`
+resource "aws_s3_bucket" "test" {
+  bucket = %[1]q
+}
+
+resource "aws_s3_bucket_acl" "test" {
+  bucket = aws_s3_bucket.test.id
+  acl    = "private"
+}
+
+resource "aws_s3_bucket_lifecycle_configuration" "test" {
+  bucket = aws_s3_bucket.test.id
+
+  rule {
+    id = %[1]q
+
+    expiration {
+      days = 90
+    }
+
+    filter {
+      and {
+        object_size_greater_than = 300
+        prefix                   = %[2]q
+      }
+    }
+
+    status = "Enabled"
+  }
+}`, rName, prefix)
+}
+
+func testAccBucketLifecycleConfiguration_Filter_PrefixConfig(rName, prefix string) string {
+	return fmt.Sprintf(`
+resource "aws_s3_bucket" "test" {
+  bucket = %[1]q
+}
+
+resource "aws_s3_bucket_acl" "test" {
+  bucket = aws_s3_bucket.test.id
+  acl    = "private"
+}
+
+resource "aws_s3_bucket_lifecycle_configuration" "test" {
+  bucket = aws_s3_bucket.test.id
+
+  rule {
+    id = %[1]q
+
+    expiration {
+      days = 90
+    }
+
+    filter {
+      prefix = %[2]q
+    }
+
+    status = "Enabled"
+  }
+}`, rName, prefix)
+}