From 5c45fc13a28d111abf44afe16246073dbe93c10c Mon Sep 17 00:00:00 2001 From: Daniel Mason Date: Fri, 11 Feb 2022 18:45:47 +1300 Subject: [PATCH 1/3] Allow days parameter in S3 lifecycle to be >= 0 --- internal/service/s3/flex.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/s3/flex.go b/internal/service/s3/flex.go index 3465c420fce..fa0bf27f04d 100644 --- a/internal/service/s3/flex.go +++ b/internal/service/s3/flex.go @@ -357,7 +357,7 @@ func ExpandLifecycleRuleTransitions(l []interface{}) ([]*s3.Transition, error) { transition.Date = aws.Time(t) } - if v, ok := tfMap["days"].(int); ok && v > 0 { + if v, ok := tfMap["days"].(int); ok && v >= 0 { transition.Days = aws.Int64(int64(v)) } From 6013c75cbf82e5d96c6b768dd1301785a0a32295 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 11 Feb 2022 16:15:58 -0500 Subject: [PATCH 2/3] r/lifecycle_configuration: allow days to be set to 0 if applicable --- .../s3/bucket_lifecycle_configuration_test.go | 269 ++++++++++++++++++ internal/service/s3/flex.go | 5 +- ...cket_lifecycle_configuration.html.markdown | 6 +- 3 files changed, 277 insertions(+), 3 deletions(-) diff --git a/internal/service/s3/bucket_lifecycle_configuration_test.go b/internal/service/s3/bucket_lifecycle_configuration_test.go index 5e92ec7bc20..a327eaeb978 100644 --- 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 index fa0bf27f04d..ca1c1dd8229 100644 --- 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 index 31c7decfd3e..557410cb8c0 100644 --- 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 From 670cce1e518a0a6d7f55697b4b4fbd67a186a07b Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Mon, 14 Feb 2022 10:21:17 -0500 Subject: [PATCH 3/3] Update CHANGELOG for #23120 --- .changelog/23120.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/23120.txt diff --git a/.changelog/23120.txt b/.changelog/23120.txt new file mode 100644 index 00000000000..41b9da1a109 --- /dev/null +++ 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` +``` \ No newline at end of file