Merge pull request #21901 from hashicorp/b-s3-bucket-replication-conf…

…ig-event-threshold r/s3_bucket_replication_configuration: make `event_threshold` optional in `destination` `metrics` config block
hashicorp · Dec 1, 2021 · 9addd5c · 9addd5c
2 parents 71f50c3 + 429b2a3
commit 9addd5c
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 11 deletions.
diff --git a/.changelog/21901.txt b/.changelog/21901.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_s3_bucket_replication_configuration: Mark `event_threshold` in `destination` `metrics` configuration block as `Optional`
+```
diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go
@@ -2426,8 +2426,10 @@ func flattenBucketReplicationConfiguration(r *s3.ReplicationConfiguration) []map
 			}
 			if v.Destination.Metrics != nil {
 				metrics := map[string]interface{}{
-					"minutes": int(aws.Int64Value(v.Destination.Metrics.EventThreshold.Minutes)),
-					"status":  aws.StringValue(v.Destination.Metrics.Status),
+					"status": aws.StringValue(v.Destination.Metrics.Status),
+				}
+				if v.Destination.Metrics.EventThreshold != nil {
+					metrics["minutes"] = int(aws.Int64Value(v.Destination.Metrics.EventThreshold.Minutes))
 				}
 				rd["metrics"] = []interface{}{metrics}
 			}

diff --git a/internal/service/s3/bucket_replication_configuration.go b/internal/service/s3/bucket_replication_configuration.go
@@ -111,7 +111,7 @@ func ResourceBucketReplicationConfiguration() *schema.Resource {
 											Schema: map[string]*schema.Schema{
 												"event_threshold": {
 													Type:     schema.TypeList,
-													Required: true,
+													Optional: true,
 													MaxItems: 1,
 													Elem: &schema.Resource{
 														Schema: map[string]*schema.Schema{

diff --git a/internal/service/s3/bucket_replication_configuration_test.go b/internal/service/s3/bucket_replication_configuration_test.go
@@ -458,13 +458,13 @@ func TestAccS3BucketReplicationConfiguration_replicationTimeControl(t *testing.T
 						"delete_marker_replication.0.status": s3.DeleteMarkerReplicationStatusEnabled,
 						"destination.#":                      "1",
 						"destination.0.replication_time.#":   "1",
-						"destination.0.replication_time.0.status":         s3.ReplicationTimeStatusEnabled,
-						"destination.0.replication_time.0.time.#":         "1",
-						"destination.0.replication_time.0.time.0.minutes": "15",
-						"destination.0.metrics.#":                         "1",
-						//"destination.0.metrics.0.status":                    s3.MetricsStatusEnabled,
-						//"destination.0.metrics.0.event_threshold.#":         "1",
-						//"destination.0.metrics.0.event_threshold.0.minutes": "15",
+						"destination.0.replication_time.0.status":           s3.ReplicationTimeStatusEnabled,
+						"destination.0.replication_time.0.time.#":           "1",
+						"destination.0.replication_time.0.time.0.minutes":   "15",
+						"destination.0.metrics.#":                           "1",
+						"destination.0.metrics.0.status":                    s3.MetricsStatusEnabled,
+						"destination.0.metrics.0.event_threshold.#":         "1",
+						"destination.0.metrics.0.event_threshold.0.minutes": "15",
 					}),
 					resource.TestCheckTypeSetElemAttrPair(resourceName, "rule.*.destination.0.bucket", dstBucketResourceName, "arn"),
 				),
@@ -659,6 +659,43 @@ func TestAccS3BucketReplicationConfiguration_schemaV2SameRegion(t *testing.T) {
 	})
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/21895
+func TestAccS3BucketReplicationConfiguration_schemaV2DestinationMetrics(t *testing.T) {
+	resourceName := "aws_s3_bucket_replication_configuration.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	var providers []*schema.Provider
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck: func() {
+			acctest.PreCheck(t)
+			acctest.PreCheckMultipleRegion(t, 2)
+		},
+		ErrorCheck:        acctest.ErrorCheck(t, s3.EndpointsID),
+		ProviderFactories: acctest.FactoriesAlternate(&providers),
+		CheckDestroy:      acctest.CheckWithProviders(testAccCheckReplicationConfigDestroy, &providers),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccBucketReplicationConfiguration_schemaV2DestinationMetrics_statusOnly(rName, s3.StorageClassStandard),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckBucketReplicationConfigurationExists(resourceName),
+					resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{
+						"destination.#":                             "1",
+						"destination.0.metrics.#":                   "1",
+						"destination.0.metrics.0.status":            s3.MetricsStatusEnabled,
+						"destination.0.metrics.0.event_threshold.#": "0",
+					}),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccS3BucketReplicationConfiguration_existingObjectReplication(t *testing.T) {
 	t.Skipf("skipping test: AWS Technical Support request required to allow ExistingObjectReplication")
 
@@ -1647,3 +1684,32 @@ resource "aws_s3_bucket_replication_configuration" "test" {
   }
 }`, key1, value1, key2, value2))
 }
+
+func testAccBucketReplicationConfiguration_schemaV2DestinationMetrics_statusOnly(rName, storageClass string) string {
+	return testAccBucketReplicationConfigurationBase(rName) + fmt.Sprintf(`
+resource "aws_s3_bucket_replication_configuration" "test" {
+  bucket = aws_s3_bucket.source.id
+  role   = aws_iam_role.test.arn
+
+  rule {
+    id = "foobar"
+    filter {
+      prefix = "foo"
+    }
+    status = "Enabled"
+
+    delete_marker_replication {
+      status = "Enabled"
+    }
+
+    destination {
+      bucket        = aws_s3_bucket.destination.arn
+      storage_class = %[1]q
+
+      metrics {
+        status = "Enabled"
+      }
+    }
+  }
+}`, storageClass)
+}
diff --git a/website/docs/r/s3_bucket_replication_configuration.html.markdown b/website/docs/r/s3_bucket_replication_configuration.html.markdown
@@ -313,7 +313,7 @@ metrics {
 
 The `metrics` configuration block supports the following arguments:
 
-* `event_threshold` - (Required) A configuration block that specifies the time threshold for emitting the `s3:Replication:OperationMissedThreshold` event [documented below](#event_threshold).
+* `event_threshold` - (Optional) A configuration block that specifies the time threshold for emitting the `s3:Replication:OperationMissedThreshold` event [documented below](#event_threshold).
 * `status` - (Required) The status of the Destination Metrics. Either `"Enabled"` or `"Disabled"`.
 
 ### event_threshold