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

…uration-backwards-compatible r/s3_bucket_lifecycle_configuration: support the default filtering used in v3.x of the s3 bucket resource
hashicorp · Mar 18, 2022 · 327cb02 · 327cb02
2 parents 5fafa6b + 9bd37cf
commit 327cb02
Show file tree

Hide file tree

Showing 6 changed files with 204 additions and 20 deletions.
diff --git a/.changelog/23746.txt b/.changelog/23746.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_s3_bucket_lifecycle_configuration: Support empty string filtering (default behavior of the `aws_s3_bucket.lifecycle_rule` parameter in provider versions prior to v4.0)
+```
diff --git a/internal/service/s3/bucket_lifecycle_configuration.go b/internal/service/s3/bucket_lifecycle_configuration.go
@@ -91,7 +91,12 @@ func ResourceBucketLifecycleConfiguration() *schema.Resource {
 						"filter": {
 							Type:     schema.TypeList,
 							Optional: true,
-							MaxItems: 1,
+							// If neither the filter block nor the prefix parameter in the rule are specified,
+							// 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,
+							MaxItems:         1,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"and": {

diff --git a/internal/service/s3/bucket_lifecycle_configuration_test.go b/internal/service/s3/bucket_lifecycle_configuration_test.go
@@ -36,7 +36,8 @@ func TestAccS3BucketLifecycleConfiguration_basic(t *testing.T) {
 					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{
 						"expiration.#":      "1",
 						"expiration.0.days": "365",
-						"filter.#":          "0",
+						"filter.#":          "1",
+						"filter.0.prefix":   "",
 						"id":                rName,
 						"status":            tfs3.LifecycleRuleStatusEnabled,
 					}),
@@ -371,6 +372,37 @@ func TestAccS3BucketLifecycleConfiguration_multipleRules(t *testing.T) {
 	})
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/23730
+func TestAccS3BucketLifecycleConfiguration_multipleRules_noFilterOrPrefix(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_MultipleRules_noFilterOrPrefixConfig(rName, s3.ReplicationRuleStatusEnabled),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketLifecycleConfigurationExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "rule.#", "2"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.0.filter.0.prefix", ""),
+					resource.TestCheckResourceAttr(resourceName, "rule.1.filter.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "rule.1.filter.0.prefix", ""),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccS3BucketLifecycleConfiguration_nonCurrentVersionExpiration(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	resourceName := "aws_s3_bucket_lifecycle_configuration.test"
@@ -919,6 +951,41 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
 `, rName)
 }
 
+func testAccBucketLifecycleConfiguration_MultipleRules_noFilterOrPrefixConfig(rName, status 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]s-1"
+    status = %[2]q
+
+    expiration {
+      days = 365
+    }
+  }
+
+  rule {
+    id     = "%[1]s-2"
+    status = %[2]q
+
+    expiration {
+      expired_object_delete_marker = true
+    }
+  }
+}
+`, rName, status)
+}
+
 func testAccBucketLifecycleConfiguration_Basic_statusConfig(rName, status string) string {
 	return fmt.Sprintf(`
 resource "aws_s3_bucket" "test" {

diff --git a/internal/service/s3/flex.go b/internal/service/s3/flex.go
@@ -431,6 +431,19 @@ func ExpandLifecycleRules(l []interface{}) ([]*s3.LifecycleRule, error) {
 			result.Filter = ExpandLifecycleRuleFilter(v)
 		}
 
+		if v, ok := tfMap["prefix"].(string); ok && result.Filter == nil {
+			// If neither the filter block nor the prefix are specified,
+			// apply the Default behavior from v3.x of the provider;
+			// otherwise, set the prefix as specified in Terraform.
+			if v == "" {
+				result.SetFilter(&s3.LifecycleRuleFilter{
+					Prefix: aws.String(v),
+				})
+			} else {
+				result.Prefix = aws.String(v)
+			}
+		}
+
 		if v, ok := tfMap["id"].(string); ok {
 			result.ID = aws.String(v)
 		}
@@ -443,10 +456,6 @@ func ExpandLifecycleRules(l []interface{}) ([]*s3.LifecycleRule, error) {
 			result.NoncurrentVersionTransitions = ExpandLifecycleRuleNoncurrentVersionTransitions(v.List())
 		}
 
-		if v, ok := tfMap["prefix"].(string); ok && result.Filter == nil {
-			result.Prefix = aws.String(v)
-		}
-
 		if v, ok := tfMap["status"].(string); ok && v != "" {
 			result.Status = aws.String(v)
 		}

diff --git a/website/docs/guides/version-4-upgrade.html.md b/website/docs/guides/version-4-upgrade.html.md
@@ -540,7 +540,7 @@ resource and remove `lifecycle_rule` and its nested arguments in the `aws_s3_buc
 
 ~> **Note:** When configuring the `rule.filter` configuration block in the new `aws_s3_bucket_lifecycle_configuration` resource, use the AWS CLI s3api [get-bucket-lifecycle-configuration](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/s3api/get-bucket-lifecycle-configuration.html)
 to get the source bucket's lifecycle configuration and determine if the `Filter` is configured as `"Filter" : {}` or `"Filter" : { "Prefix": "" }`.
-If AWS returns the former, configure `rule.filter` as `filter {}`. Otherwise, configure `rule.filter` as `filter { prefix = "" }` as shown here:
+If AWS returns the former, configure `rule.filter` as `filter {}`. Otherwise, neither a `rule.filter` nor `rule.prefix` parameter should be configured as shown here:
 
 ```terraform
 resource "aws_s3_bucket" "example" {
@@ -556,10 +556,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "example" {
     id     = "Keep previous version 30 days, then in Glacier another 60"
     status = "Enabled"
 
-    filter {
-      prefix = ""
-    }
-
     noncurrent_version_transition {
       noncurrent_days = 30
       storage_class   = "GLACIER"
@@ -574,10 +570,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "example" {
     id     = "Delete old incomplete multi-part uploads"
     status = "Enabled"
 
-    filter {
-      prefix = ""
-    }
-
     abort_incomplete_multipart_upload {
       days_after_initiation = 7
     }
@@ -648,6 +640,89 @@ resource "aws_s3_bucket" "example" {
   # ... other configuration ...
 }
 
+resource "aws_s3_bucket_lifecycle_configuration" "example" {
+  bucket = aws_s3_bucket.example.id
+
+  rule {
+    id     = "log-expiration"
+    status = "Enabled"
+
+    transition {
+      days          = 30
+      storage_class = "STANDARD_IA"
+    }
+
+    transition {
+      days          = 180
+      storage_class = "GLACIER"
+    }
+  }
+}
+```
+
+Run `terraform import` on each new resource, _e.g._,
+
+```shell
+$ terraform import aws_s3_bucket_lifecycle_configuration.example yournamehere
+aws_s3_bucket_lifecycle_configuration.example: Importing from ID "yournamehere"...
+aws_s3_bucket_lifecycle_configuration.example: Import prepared!
+  Prepared aws_s3_bucket_lifecycle_configuration for import
+aws_s3_bucket_lifecycle_configuration.example: Refreshing state... [id=yournamehere]
+
+Import successful!
+
+The resources that were imported are shown above. These resources are now in
+your Terraform state and will henceforth be managed by Terraform.
+```
+
+#### For Lifecycle Rules with `prefix`
+
+For example, given this configuration:
+
+```terraform
+resource "aws_s3_bucket" "example" {
+  bucket = "yournamehere"
+
+  lifecycle_rule {
+    id      = "log-expiration"
+    enabled = true
+    prefix  = "foobar"
+
+    transition {
+      days          = 30
+      storage_class = "STANDARD_IA"
+    }
+
+    transition {
+      days          = 180
+      storage_class = "GLACIER"
+    }
+  }
+}
+```
+
+You will receive the following error after upgrading:
+
+```
+│ Error: Value for unconfigurable attribute
+│
+│   with aws_s3_bucket.example,
+│   on main.tf line 1, in resource "aws_s3_bucket" "example":
+│    1: resource "aws_s3_bucket" "example" {
+│
+│ Can't configure a value for "lifecycle_rule": its value will be decided automatically based on the result of applying this configuration.
+```
+
+Since the `lifecycle_rule` argument changed to read-only, update the configuration to use the `aws_s3_bucket_lifecycle_configuration`
+resource and remove `lifecycle_rule` and its nested arguments in the `aws_s3_bucket` resource:
+
+```terraform
+resource "aws_s3_bucket" "example" {
+  bucket = "yournamehere"
+
+  # ... other configuration ...
+}
+
 resource "aws_s3_bucket_lifecycle_configuration" "example" {
   bucket = aws_s3_bucket.example.id
 
@@ -656,7 +731,7 @@ resource "aws_s3_bucket_lifecycle_configuration" "example" {
     status = "Enabled"
 
     filter {
-      prefix = ""
+      prefix = "foobar"
     }
 
     transition {
@@ -687,8 +762,7 @@ The resources that were imported are shown above. These resources are now in
 your Terraform state and will henceforth be managed by Terraform.
 ```
 
-#### For Lifecycle Rules with a `prefix`
-
+#### For Lifecycle Rules with `prefix` and `tags`
 
 For example, given this previous configuration:
 

diff --git a/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown b/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown
@@ -22,6 +22,27 @@ For more information see the Amazon S3 User Guide on [`Lifecycle Configuration E
 
 ## Example Usage
 
+### With neither a filter nor prefix specified
+
+The Lifecycle rule applies to a subset of objects based on the key name prefix (`""`).
+
+This configuration is intended to replicate the default behavior of the `lifecycle_rule`
+parameter in the Terraform AWS Provider `aws_s3_bucket` resource prior to `v4.0`.
+
+```terraform
+resource "aws_s3_bucket_lifecycle_configuration" "example" {
+  bucket = aws_s3_bucket.bucket.id
+
+  rule {
+    id = "rule-1"
+
+    # ... other transition/expiration actions ...
+
+    status = "Enabled"
+  }
+}
+```
+
 ### Specifying an empty filter
 
 The Lifecycle rule applies to all objects in the bucket.
@@ -343,7 +364,12 @@ The following arguments are supported:
 
 ### rule
 
-~> **NOTE:** The `filter` argument, while Optional, is required if the `rule` configuration block does not contain a `prefix`. Since `prefix` is deprecated by Amazon S3 and will be removed in the next major version of the Terraform AWS Provider, we recommend users specify `filter`.
+~> **NOTE:** The `filter` argument, while Optional, is required if the `rule` configuration block does not contain a `prefix` **and** you intend to override the default behavior of setting the rule to filter objects with the empty string prefix (`""`).
+Since `prefix` is deprecated by Amazon S3 and will be removed in the next major version of the Terraform AWS Provider, we recommend users either specify `filter` or leave both `filter` and `prefix` unspecified.
+
+~> **NOTE:** A rule cannot be updated from having a filter (via either the `rule.filter` parameter or when neither `rule.filter` and `rule.prefix` are specified) to only having a prefix via the `rule.prefix` parameter.
+
+~> **NOTE** Terraform cannot distinguish a difference between configurations that use `rule.filter {}` and configurations that neither use `rule.filter` nor `rule.prefix`, so a rule cannot be updated from applying to all objects in the bucket via `rule.filter {}` to applying to a subset of objects based on the key prefix `""` and vice versa.
 
 The `rule` configuration block supports the following arguments:
 
@@ -373,7 +399,7 @@ The `expiration` configuration block supports the following arguments:
 
 ### filter
 
-~> **NOTE:** The `filter` configuration block must have exactly one of `prefix`, `tag`, `and`, `object_size_greater_than` or `object_size_less_than` specified.
+~> **NOTE:** The `filter` configuration block must either be specified as the empty configuration block (`filter {}`) or with exactly one of `prefix`, `tag`, `and`, `object_size_greater_than` or `object_size_less_than` specified.
 
 The `filter` configuration block supports the following arguments: