From 01f3291c2acab6b96b43e9d00a4ae701deb84e0d Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Sun, 27 Mar 2022 20:16:51 -0400 Subject: [PATCH 1/2] r/s3_bucket_lifecycle_configuration: use custom diffsuppressfunc on rule.filter parameter --- .../s3/bucket_lifecycle_configuration.go | 26 ++++- .../s3/bucket_lifecycle_configuration_test.go | 99 +++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/internal/service/s3/bucket_lifecycle_configuration.go b/internal/service/s3/bucket_lifecycle_configuration.go index c84ac871fe4..5f10cb4580e 100644 --- a/internal/service/s3/bucket_lifecycle_configuration.go +++ b/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 +} diff --git a/internal/service/s3/bucket_lifecycle_configuration_test.go b/internal/service/s3/bucket_lifecycle_configuration_test.go index 41b9de2f269..c7633061ce2 100644 --- a/internal/service/s3/bucket_lifecycle_configuration_test.go +++ b/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) +} From 5bcb9cf72b811675b3bcd98832441b43f5488a4d Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Sun, 27 Mar 2022 20:26:02 -0400 Subject: [PATCH 2/2] Update CHANGELOG for #23893 --- .changelog/23893.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/23893.txt diff --git a/.changelog/23893.txt b/.changelog/23893.txt new file mode 100644 index 00000000000..3f8eab572f7 --- /dev/null +++ b/.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` +``` \ No newline at end of file