Merge pull request #23120 from idanoo/s3_invalid_xml

Allow S3 lifecycle 'days' parameter in S3 transition lifecycle to be >= 0. Issue #23117
hashicorp · Feb 15, 2022 · 26f2311 · 26f2311
2 parents b8ddc20 + 670cce1
commit 26f2311
Show file tree

Hide file tree

Showing 4 changed files with 280 additions and 3 deletions.
diff --git a/.changelog/23120.txt b/.changelog/23120.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_s3_bucket_lifecycle_configuration: Correctly handle the `days` value of the `rule` `transition` configuration block when set to `0`
+```
diff --git a/internal/service/s3/bucket_lifecycle_configuration_test.go b/internal/service/s3/bucket_lifecycle_configuration_test.go
@@ -444,6 +444,165 @@ func TestAccS3BucketLifecycleConfiguration_RuleAbortIncompleteMultipartUpload(t
 	})
 }
 
+// TestAccS3BucketLifecycleConfiguration_TransitionDate_StandardIa validates the change to address
+// https://github.com/hashicorp/terraform-provider-aws/issues/23117
+// does not introduce a regression.
+func TestAccS3BucketLifecycleConfiguration_TransitionDate_StandardIa(t *testing.T) {
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_s3_bucket_lifecycle_configuration.test"
+
+	currTime := time.Now()
+	date := time.Date(currTime.Year(), currTime.Month()+1, currTime.Day(), 0, 0, 0, 0, time.UTC).Format(time.RFC3339)
+
+	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_DateTransitionConfig(rName, date, s3.TransitionStorageClassStandardIa),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+// TestAccS3BucketLifecycleConfiguration_TransitionDate_IntelligentTiering validates the change to address
+// https://github.com/hashicorp/terraform-provider-aws/issues/23117
+// does not introduce a regression.
+func TestAccS3BucketLifecycleConfiguration_TransitionDate_IntelligentTiering(t *testing.T) {
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_s3_bucket_lifecycle_configuration.test"
+
+	currTime := time.Now()
+	date := time.Date(currTime.Year(), currTime.Month()+1, currTime.Day(), 0, 0, 0, 0, time.UTC).Format(time.RFC3339)
+
+	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_DateTransitionConfig(rName, date, s3.StorageClassIntelligentTiering),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/23117
+func TestAccS3BucketLifecycleConfiguration_TransitionStorageClassOnly_IntelligentTiering(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_TransitionStorageClassOnlyConfig(rName, s3.StorageClassIntelligentTiering),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.0.transition.*", map[string]string{
+						"days":          "0",
+						"date":          "",
+						"storage_class": s3.StorageClassIntelligentTiering,
+					}),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/23117
+func TestAccS3BucketLifecycleConfiguration_TransitionZeroDays_IntelligentTiering(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_ZeroDaysTransitionConfig(rName, s3.StorageClassIntelligentTiering),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccS3BucketLifecycleConfiguration_TransitionUpdateBetweenDaysAndDate_IntelligentTiering(t *testing.T) {
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_s3_bucket_lifecycle_configuration.test"
+
+	currTime := time.Now()
+	date := time.Date(currTime.Year(), currTime.Month()+1, currTime.Day(), 0, 0, 0, 0, time.UTC).Format(time.RFC3339)
+
+	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_ZeroDaysTransitionConfig(rName, s3.StorageClassIntelligentTiering),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+				),
+			},
+			{
+				Config: testAccBucketLifecycleConfiguration_DateTransitionConfig(rName, date, s3.StorageClassIntelligentTiering),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccBucketLifecycleConfiguration_ZeroDaysTransitionConfig(rName, s3.StorageClassIntelligentTiering),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckBucketLifecycleConfigurationDestroy(s *terraform.State) error {
 	conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn
 
@@ -857,3 +1016,113 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
 }
 `, rName)
 }
+
+func testAccBucketLifecycleConfiguration_TransitionStorageClassOnlyConfig(rName, storageClass 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.bucket
+
+  rule {
+    id = %[1]q
+
+    abort_incomplete_multipart_upload {
+      days_after_initiation = 1
+    }
+
+    noncurrent_version_transition {
+      noncurrent_days = 0
+      storage_class   = "INTELLIGENT_TIERING"
+    }
+
+    transition {
+      storage_class = %[2]q
+    }
+
+    status = "Enabled"
+  }
+}
+`, rName, storageClass)
+}
+
+func testAccBucketLifecycleConfiguration_ZeroDaysTransitionConfig(rName, storageClass 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.bucket
+
+  rule {
+    id = %[1]q
+
+    abort_incomplete_multipart_upload {
+      days_after_initiation = 1
+    }
+
+    noncurrent_version_transition {
+      noncurrent_days = 0
+      storage_class   = "INTELLIGENT_TIERING"
+    }
+
+    transition {
+      days          = 0
+      storage_class = %[2]q
+    }
+
+    status = "Enabled"
+  }
+}
+`, rName, storageClass)
+}
+
+func testAccBucketLifecycleConfiguration_DateTransitionConfig(rName, transitionDate, storageClass 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.bucket
+
+  rule {
+    id = %[1]q
+
+    abort_incomplete_multipart_upload {
+      days_after_initiation = 1
+    }
+
+    noncurrent_version_transition {
+      noncurrent_days = 0
+      storage_class   = "INTELLIGENT_TIERING"
+    }
+
+    transition {
+      date          = %[2]q
+      storage_class = %[3]q
+    }
+
+    status = "Enabled"
+  }
+}
+`, rName, transitionDate, storageClass)
+}
diff --git a/internal/service/s3/flex.go b/internal/service/s3/flex.go
@@ -357,7 +357,10 @@ func ExpandLifecycleRuleTransitions(l []interface{}) ([]*s3.Transition, error) {
 			transition.Date = aws.Time(t)
 		}
 
-		if v, ok := tfMap["days"].(int); ok && v > 0 {
+		// Only one of "date" and "days" can be configured
+		// so only set the transition.Days value when transition.Date is nil
+		// By default, tfMap["days"] = 0 if not explicitly configured in terraform.
+		if v, ok := tfMap["days"].(int); ok && v >= 0 && transition.Date == nil {
 			transition.Days = aws.Int64(int64(v))
 		}
 

diff --git a/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown b/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown
@@ -202,8 +202,10 @@ The `noncurrent_version_transition` configuration block supports the following a
 
 The `transition` configuration block supports the following arguments:
 
-* `date` - (Optional) The date objects are transitioned to the specified storage class. The date value must be in ISO 8601 format. The time is always midnight UTC.
-* `days` - (Optional) The number of days after creation when objects are transitioned to the specified storage class. The value must be a positive integer.
+~> **Note:** Only one of `date` or `days` should be specified. If neither are specified, the `transition` will default to 0 `days`.
+
+* `date` - (Optional, Conflicts with `days`) The date objects are transitioned to the specified storage class. The date value must be in ISO 8601 format and set to midnight UTC e.g. `2023-01-13T00:00:00Z`.
+* `days` - (Optional, Conflicts with `date`) The number of days after creation when objects are transitioned to the specified storage class. The value must be a positive integer. If both `days` and `date` are not specified, defaults to `0`. Valid values depend on `storage_class`, see [Transition objects using Amazon S3 Lifecycle](https://docs.aws.amazon.com/AmazonS3/latest/userguide/lifecycle-transition-general-considerations.html) for more details.
 * `storage_class` - The class of storage used to store the object. Valid Values: `GLACIER`, `STANDARD_IA`, `ONEZONE_IA`, `INTELLIGENT_TIERING`, `DEEP_ARCHIVE`, `GLACIER_IR`.
 
 ### tag