From 4b0c7f735b9d783b173ccc857062fcabecd4f6d7 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Tue, 1 Mar 2022 20:00:36 -0500 Subject: [PATCH 1/2] r/s3_bucket_lifecycle_configuration: make new_noncurrent_versions arguments nullable ints --- .../s3/bucket_lifecycle_configuration.go | 8 +- .../s3/bucket_lifecycle_configuration_test.go | 276 ++++++++++++++++++ internal/service/s3/flex.go | 29 +- ...cket_lifecycle_configuration.html.markdown | 8 +- 4 files changed, 300 insertions(+), 21 deletions(-) diff --git a/internal/service/s3/bucket_lifecycle_configuration.go b/internal/service/s3/bucket_lifecycle_configuration.go index 21535c22f342..58937e805ceb 100644 --- a/internal/service/s3/bucket_lifecycle_configuration.go +++ b/internal/service/s3/bucket_lifecycle_configuration.go @@ -164,9 +164,9 @@ func ResourceBucketLifecycleConfiguration() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "newer_noncurrent_versions": { - Type: schema.TypeInt, + Type: nullable.TypeNullableInt, Optional: true, - ValidateFunc: validation.IntAtLeast(1), + ValidateFunc: nullable.ValidateTypeStringNullableIntAtLeast(1), }, "noncurrent_days": { Type: schema.TypeInt, @@ -182,9 +182,9 @@ func ResourceBucketLifecycleConfiguration() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "newer_noncurrent_versions": { - Type: schema.TypeInt, + Type: nullable.TypeNullableInt, Optional: true, - ValidateFunc: validation.IntAtLeast(1), + ValidateFunc: nullable.ValidateTypeStringNullableIntAtLeast(1), }, "noncurrent_days": { Type: schema.TypeInt, diff --git a/internal/service/s3/bucket_lifecycle_configuration_test.go b/internal/service/s3/bucket_lifecycle_configuration_test.go index 97427452a2a9..d0993bc65973 100644 --- a/internal/service/s3/bucket_lifecycle_configuration_test.go +++ b/internal/service/s3/bucket_lifecycle_configuration_test.go @@ -128,6 +128,151 @@ func TestAccS3BucketLifecycleConfiguration_filterWithPrefix(t *testing.T) { }) } +func TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeGreaterThan(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_filterWithObjectSizeGreaterThanConfig(rName, date, 100), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketLifecycleConfigurationExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "expiration.#": "1", + "expiration.0.date": date, + "filter.#": "1", + "filter.0.object_size_greater_than": "100", + "id": rName, + "status": tfs3.LifecycleRuleStatusEnabled, + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeLessThan(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_filterWithObjectSizeLessThanConfig(rName, date, 500), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketLifecycleConfigurationExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "expiration.#": "1", + "expiration.0.date": date, + "filter.#": "1", + "filter.0.object_size_less_than": "500", + "id": rName, + "status": tfs3.LifecycleRuleStatusEnabled, + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRange(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_filterWithObjectSizeRangeConfig(rName, date, 500, 64000), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketLifecycleConfigurationExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "expiration.#": "1", + "expiration.0.date": date, + "filter.#": "1", + "filter.0.and.#": "1", + "filter.0.and.0.object_size_greater_than": "500", + "filter.0.and.0.object_size_less_than": "64000", + "id": rName, + "status": tfs3.LifecycleRuleStatusEnabled, + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccS3BucketLifecycleConfiguration_Filter_ObjectSizeRangeAndPrefix(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_filterWithObjectSizeRangeAndPrefixConfig(rName, date, 500, 64000), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketLifecycleConfigurationExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{ + "expiration.#": "1", + "expiration.0.date": date, + "filter.#": "1", + "filter.0.and.#": "1", + "filter.0.and.0.object_size_greater_than": "500", + "filter.0.and.0.object_size_less_than": "64000", + "filter.0.and.0.prefix": rName, + "id": rName, + "status": tfs3.LifecycleRuleStatusEnabled, + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccS3BucketLifecycleConfiguration_disableRule(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_bucket_lifecycle_configuration.test" @@ -1241,3 +1386,134 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" { } `, rName) } + +func testAccBucketLifecycleConfiguration_filterWithObjectSizeGreaterThanConfig(rName, date string, sizeGreaterThan int) 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 + + expiration { + date = %[2]q + } + + filter { + object_size_greater_than = %[3]d + } + + status = "Enabled" + } +} +`, rName, date, sizeGreaterThan) +} + +func testAccBucketLifecycleConfiguration_filterWithObjectSizeLessThanConfig(rName, date string, sizeLessThan int) 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 + + expiration { + date = %[2]q + } + + filter { + object_size_less_than = %[3]d + } + + status = "Enabled" + } +} +`, rName, date, sizeLessThan) +} + +func testAccBucketLifecycleConfiguration_filterWithObjectSizeRangeConfig(rName, date string, sizeGreaterThan, sizeLessThan int) 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 + + expiration { + date = %[2]q + } + + filter { + and { + object_size_greater_than = %[3]d + object_size_less_than = %[4]d + } + } + + status = "Enabled" + } +} +`, rName, date, sizeGreaterThan, sizeLessThan) +} + +func testAccBucketLifecycleConfiguration_filterWithObjectSizeRangeAndPrefixConfig(rName, date string, sizeGreaterThan, sizeLessThan int) 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 + + expiration { + date = %[2]q + } + + filter { + and { + object_size_greater_than = %[3]d + object_size_less_than = %[4]d + prefix = %[1]q + } + } + + status = "Enabled" + } +} +`, rName, date, sizeGreaterThan, sizeLessThan) +} diff --git a/internal/service/s3/flex.go b/internal/service/s3/flex.go index 4731f4688ab8..c1cc5a3ac69c 100644 --- a/internal/service/s3/flex.go +++ b/internal/service/s3/flex.go @@ -2,11 +2,13 @@ package s3 import ( "fmt" + "strconv" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-aws/internal/experimental/nullable" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" ) @@ -230,12 +232,12 @@ func ExpandLifecycleRuleFilter(l []interface{}) *s3.LifecycleRuleFilter { result.And = ExpandLifecycleRuleFilterAndOperator(v[0].(map[string]interface{})) } - if v, ok := m["object_size_greater_than"].(int); ok && v > 0 { - result.ObjectSizeGreaterThan = aws.Int64(int64(v)) + if v, null, _ := nullable.Int(m["object_size_greater_than"].(string)).Value(); !null && v > 0 { + result.ObjectSizeGreaterThan = aws.Int64(v) } - if v, ok := m["object_size_less_than"].(int); ok && v > 0 { - result.ObjectSizeLessThan = aws.Int64(int64(v)) + if v, null, _ := nullable.Int(m["object_size_less_than"].(string)).Value(); !null && v > 0 { + result.ObjectSizeLessThan = aws.Int64(v) } if v, ok := m["tag"].([]interface{}); ok && len(v) > 0 && v[0] != nil { @@ -244,7 +246,8 @@ func ExpandLifecycleRuleFilter(l []interface{}) *s3.LifecycleRuleFilter { // Per AWS S3 API, "A Filter must have exactly one of Prefix, Tag, or And specified"; // Specifying more than one of the listed parameters results in a MalformedXML error. - if v, ok := m["prefix"].(string); ok && result.And == nil && result.Tag == nil { + // In practice, this also includes ObjectSizeGreaterThan and ObjectSizeLessThan. + if v, ok := m["prefix"].(string); ok && result.And == nil && result.Tag == nil && result.ObjectSizeGreaterThan == nil && result.ObjectSizeLessThan == nil { result.Prefix = aws.String(v) } @@ -305,8 +308,8 @@ func ExpandLifecycleRuleNoncurrentVersionExpiration(m map[string]interface{}) *s result := &s3.NoncurrentVersionExpiration{} - if v, ok := m["newer_noncurrent_versions"].(int); ok && v > 0 { - result.NewerNoncurrentVersions = aws.Int64(int64(v)) + if v, null, _ := nullable.Int(m["newer_noncurrent_versions"].(string)).Value(); !null && v > 0 { + result.NewerNoncurrentVersions = aws.Int64(v) } if v, ok := m["noncurrent_days"].(int); ok { @@ -332,8 +335,8 @@ func ExpandLifecycleRuleNoncurrentVersionTransitions(l []interface{}) []*s3.Nonc transition := &s3.NoncurrentVersionTransition{} - if v, ok := tfMap["newer_noncurrent_versions"].(int); ok && v > 0 { - transition.NewerNoncurrentVersions = aws.Int64(int64(v)) + if v, null, _ := nullable.Int(tfMap["newer_noncurrent_versions"].(string)).Value(); !null && v > 0 { + transition.NewerNoncurrentVersions = aws.Int64(v) } if v, ok := tfMap["noncurrent_days"].(int); ok { @@ -910,11 +913,11 @@ func FlattenLifecycleRuleFilter(filter *s3.LifecycleRuleFilter) []interface{} { } if filter.ObjectSizeGreaterThan != nil { - m["object_size_greater_than"] = int(aws.Int64Value(filter.ObjectSizeGreaterThan)) + m["object_size_greater_than"] = strconv.FormatInt(aws.Int64Value(filter.ObjectSizeGreaterThan), 10) } if filter.ObjectSizeLessThan != nil { - m["object_size_less_than"] = int(aws.Int64Value(filter.ObjectSizeLessThan)) + m["object_size_less_than"] = strconv.FormatInt(aws.Int64Value(filter.ObjectSizeLessThan), 10) } if filter.Prefix != nil { @@ -980,7 +983,7 @@ func FlattenLifecycleRuleNoncurrentVersionExpiration(expiration *s3.NoncurrentVe m := make(map[string]interface{}) if expiration.NewerNoncurrentVersions != nil { - m["newer_noncurrent_versions"] = int(aws.Int64Value(expiration.NewerNoncurrentVersions)) + m["newer_noncurrent_versions"] = strconv.FormatInt(aws.Int64Value(expiration.NewerNoncurrentVersions), 10) } if expiration.NoncurrentDays != nil { @@ -1005,7 +1008,7 @@ func FlattenLifecycleRuleNoncurrentVersionTransitions(transitions []*s3.Noncurre m := make(map[string]interface{}) if transition.NewerNoncurrentVersions != nil { - m["newer_noncurrent_versions"] = int(aws.Int64Value(transition.NewerNoncurrentVersions)) + m["newer_noncurrent_versions"] = strconv.FormatInt(aws.Int64Value(transition.NewerNoncurrentVersions), 10) } if transition.NoncurrentDays != nil { diff --git a/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown b/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown index 0f5bf28a32a0..77fce8f6d562 100644 --- a/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown +++ b/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown @@ -365,13 +365,13 @@ The `abort_incomplete_multipart_upload` configuration block supports the followi The `expiration` configuration block supports the following arguments: -* `date` - (Optional) The date the object is to be moved or deleted. Should be in GMT ISO 8601 Format. +* `date` - (Optional) The date the object is to be moved or deleted. Should be in [RFC3339 format](https://tools.ietf.org/html/rfc3339#section-5.8). * `days` - (Optional) The lifetime, in days, of the objects that are subject to the rule. The value must be a non-zero positive integer. * `expired_object_delete_marker` - (Optional, Conflicts with `date` and `days`) Indicates whether Amazon S3 will remove a delete marker with no noncurrent versions. If set to `true`, the delete marker will be expired; if set to `false` the policy takes no action. ### filter -~> **NOTE:** The `filter` configuration block must have exactly one of `prefix`, `tag`, or `and` specified. +~> **NOTE:** The `filter` configuration block must have exactly one of `prefix`, `tag`, `and`, `object_size_greater_than` or `object_size_less_than` specified. The `filter` configuration block supports the following arguments: @@ -392,7 +392,7 @@ The `noncurrent_version_expiration` configuration block supports the following a The `noncurrent_version_transition` configuration block supports the following arguments: -* `newer_noncurrent_versions` - (Optional) The number of noncurrent versions Amazon S3 will retain. +* `newer_noncurrent_versions` - (Optional) The number of noncurrent versions Amazon S3 will retain. Must be a non-zero positive integer. * `noncurrent_days` - (Optional) The number of days an object is noncurrent before Amazon S3 can perform the associated action. * `storage_class` - (Required) The class of storage used to store the object. Valid Values: `GLACIER`, `STANDARD_IA`, `ONEZONE_IA`, `INTELLIGENT_TIERING`, `DEEP_ARCHIVE`, `GLACIER_IR`. @@ -402,7 +402,7 @@ The `transition` configuration block supports the following arguments: ~> **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`. +* `date` - (Optional, Conflicts with `days`) The date objects are transitioned to the specified storage class. The date value must be in [RFC3339 format](https://tools.ietf.org/html/rfc3339#section-5.8) 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`. From 57e2c5dc5be22db1bec5be106f35efee1746a17e Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Tue, 1 Mar 2022 22:20:19 -0500 Subject: [PATCH 2/2] Update CHANGELOG for #23441 --- .changelog/23441.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/23441.txt diff --git a/.changelog/23441.txt b/.changelog/23441.txt new file mode 100644 index 000000000000..f2525789a8e2 --- /dev/null +++ b/.changelog/23441.txt @@ -0,0 +1,7 @@ +```release-note:bug +resource/aws_s3_bucket_lifecycle_configuration: Correctly configure `rule.filter.object_size_greater_than` and `rule.filter.object_size_less_than` in API requests and terraform state +``` + +```release-note:bug +resource/aws_s3_bucket_lifecycle_configuration: Prevent drift when `rule.noncurrent_version_expiration.newer_noncurrent_versions` or `rule.noncurrent_version_transition.newer_noncurrent_versions` is not specified +``` \ No newline at end of file