From e1362a70aca0fc00d6ec6f959c2657c0488b3303 Mon Sep 17 00:00:00 2001 From: AJ Bowen Date: Wed, 6 Oct 2021 13:11:58 +0200 Subject: [PATCH 1/7] WIP --- website/docs/r/s3_bucket.html.markdown | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 39962ecb897..3a6b3581ca7 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -277,12 +277,22 @@ resource "aws_s3_bucket" "source" { rules { id = "foobar" - prefix = "foo" status = "Enabled" + filter { + tags = {} + } destination { bucket = aws_s3_bucket.destination.arn storage_class = "STANDARD" + replication_time { + status = "Enabled" + minutes = 15 + } + } + metrics { + status = "Enabled" + minutes = 15 } } } @@ -452,6 +462,18 @@ The `destination` object supports the following: `sse_kms_encrypted_objects` source selection criteria. * `access_control_translation` - (Optional) Specifies the overrides to use for object owners on replication. Must be used in conjunction with `account_id` owner override configuration. * `account_id` - (Optional) The Account ID to use for overriding the object owner on replication. Must be used in conjunction with `access_control_translation` override configuration. +* `replication_time` - (Optional) Enables S3 Replication Time Control (S3 RTC) (documented below). +* `metrics` - (Optional) Enables replication metrics (required for S3 RTC) (documented below). + +The `replication_time` object supports the following: + +* `status` - (Optional) The status of RTC. Either `Enabled` or `Disabled`. +* `minutes` - (Optional) Threshold within which objects are to be replicated. The only valid value is `15`. + +The `metrics` object supports the following: + +* `status` - (Optional) The status of replication metrics. Either `Enabled` or `Disabled`. +* `minutes` - (Optional) Threshold within which objects are to be replicated. The only valid value is `15`. The `source_selection_criteria` object supports the following: From 74403802fd3ff60a07558613c8925690cdb34c06 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 25 Oct 2021 17:14:34 -0400 Subject: [PATCH 2/7] r/aws-s3_bucket: Incorporate RTC modifications after Service Package restructure. --- internal/service/s3/bucket.go | 108 +++++++- internal/service/s3/bucket_test.go | 409 +++++++++++++++++++++++++++++ 2 files changed, 514 insertions(+), 3 deletions(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 44226b18e70..609050648f5 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -456,6 +456,49 @@ func ResourceBucket() *schema.Resource { }, }, }, + "replication_time": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "minutes": { + Type: schema.TypeInt, + Optional: true, + Default: 15, + ValidateFunc: validation.IntBetween(15, 15), + }, + "status": { + Type: schema.TypeString, + Optional: true, + Default: s3.ReplicationTimeStatusEnabled, + ValidateFunc: validation.StringInSlice(s3.ReplicationTimeStatus_Values(), false), + }, + }, + }, + }, + "metrics": { + Type: schema.TypeList, + Optional: true, + MinItems: 1, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "minutes": { + Type: schema.TypeInt, + Optional: true, + Default: 15, + ValidateFunc: validation.IntBetween(10, 15), + }, + "status": { + Type: schema.TypeString, + Optional: true, + Default: s3.MetricsStatusEnabled, + ValidateFunc: validation.StringInSlice(s3.MetricsStatus_Values(), false), + }, + }, + }, + }, }, }, }, @@ -2043,6 +2086,7 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc rcRule.ID = aws.String(rrid.(string)) } + rtcEnabled := false // Note: RTC requires schemaV2 ruleDestination := &s3.Destination{} if dest, ok := rr["destination"].([]interface{}); ok && len(dest) > 0 { if dest[0] != nil { @@ -2069,6 +2113,40 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc ruleAclTranslation.Owner = aws.String(aclTranslationValues["owner"].(string)) ruleDestination.AccessControlTranslation = ruleAclTranslation } + + // replication metrics (required for RTC) + metricsEnabled := false + if metrics, ok := bd["metrics"].([]interface{}); ok && len(metrics) > 0 { + metricsConfig := &s3.Metrics{} + metricsValues := metrics[0].(map[string]interface{}) + metricsConfig.EventThreshold = &s3.ReplicationTimeValue{} + + metricsConfig.Status = aws.String(metricsValues["status"].(string)) + metricsConfig.EventThreshold.Minutes = aws.Int64(int64(metricsValues["minutes"].(int))) + ruleDestination.Metrics = metricsConfig + + if *metricsConfig.Status == s3.MetricsStatusEnabled { + metricsEnabled = true + } + } + + // replication time control (RTC) + if rtc, ok := bd["replication_time"].([]interface{}); ok && len(rtc) > 0 { + rtcValues := rtc[0].(map[string]interface{}) + rtcConfig := &s3.ReplicationTime{} + rtcConfig.Status = aws.String(rtcValues["status"].(string)) + rtcConfig.Time = &s3.ReplicationTimeValue{} + rtcConfig.Time.Minutes = aws.Int64(int64(rtcValues["minutes"].(int))) + + if *rtcConfig.Status == s3.ReplicationTimeStatusEnabled { + rtcEnabled = true + if !metricsEnabled { + return fmt.Errorf("Metrics must be enabled to allow S3 RTC. Enable by adding a block like: metrics {}") + } + } + + ruleDestination.ReplicationTime = rtcConfig + } } } rcRule.Destination = ruleDestination @@ -2093,8 +2171,13 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc } } - if f, ok := rr["filter"].([]interface{}); ok && len(f) > 0 && f[0] != nil { - // XML schema V2. + f, ok := rr["filter"].([]interface{}) + if ok && (len(f) == 0 || f[0] == nil) { + // an empty filter means apply the rule to all objects in the bucket + rcRule.Priority = aws.Int64(int64(rr["priority"].(int))) + rcRule.Filter = &s3.ReplicationRuleFilter{} + } else if ok && len(f) > 0 && f[0] != nil { + // XML schema V2 (required for S3 RTC). rcRule.Priority = aws.Int64(int64(rr["priority"].(int))) rcRule.Filter = &s3.ReplicationRuleFilter{} filter := f[0].(map[string]interface{}) @@ -2117,8 +2200,10 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc Status: aws.String(s3.DeleteMarkerReplicationStatusDisabled), } } + } else if rtcEnabled { + return fmt.Errorf("A filter block must be present to enable RTC due to differences between V1 and V2 of the replication configuration schema.") } else { - // XML schema V1. + // XML schema V1. (Note: Incompatible with RTC) rcRule.Prefix = aws.String(rr["prefix"].(string)) } @@ -2355,6 +2440,23 @@ func flattenBucketReplicationConfiguration(r *s3.ReplicationConfiguration) []map if v.Destination.StorageClass != nil { rd["storage_class"] = aws.StringValue(v.Destination.StorageClass) } + if v.Destination.ReplicationTime != nil { + rtc := map[string]interface{}{ + "minutes": aws.Int64Value(v.Destination.ReplicationTime.Time.Minutes), + "status": aws.StringValue(v.Destination.ReplicationTime.Status), + } + rd["replication_time"] = []interface{}{rtc} + if v.Destination.Metrics == nil { + log.Printf("[WARN] Metrics must be enabled to enable RTC") + } + } + if v.Destination.Metrics != nil { + metrics := map[string]interface{}{ + "minutes": aws.Int64Value(v.Destination.Metrics.EventThreshold.Minutes), + "status": aws.StringValue(v.Destination.Metrics.Status), + } + rd["metrics"] = []interface{}{metrics} + } if v.Destination.EncryptionConfiguration != nil { if v.Destination.EncryptionConfiguration.ReplicaKmsKeyID != nil { rd["replica_kms_key_id"] = aws.StringValue(v.Destination.EncryptionConfiguration.ReplicaKmsKeyID) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index a9ec1bb6c1b..d0e25e20811 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -2217,6 +2217,228 @@ func TestAccS3Bucket_Replication_schemaV2SameRegion(t *testing.T) { }) } +func TestAccBucket_Replication_RTC_expectValidationError(t *testing.T) { + rInt := sdkacctest.RandInt() + + // record the initialized providers so that we can use them to check for the instances in each region + 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(testAccCheckBucketDestroyWithProvider, &providers), + Steps: []resource.TestStep{ + { + Config: testAccBucketReplicationWithReplicationConfigurationWithRTCNoFilterConfig(rInt), + ExpectError: regexp.MustCompile(`A filter block must be present to enable RTC due to differences between V1 and V2 of the replication configuration schema.`), + }, + { + Config: testAccBucketReplicationWithReplicationConfigurationWithRTCInvalidNoMetricsConfig(rInt), + ExpectError: regexp.MustCompile(`Metrics must be enabled to allow S3 RTC. Enable by adding a block like: metrics {}`), + }, + }, + }) +} + +func TestAccBucket_Replication_RTC_valid(t *testing.T) { + rInt := sdkacctest.RandInt() + alternateRegion := acctest.AlternateRegion() + region := acctest.Region() + partition := acctest.Partition() + iamRoleResourceName := "aws_iam_role.role" + resourceName := "aws_s3_bucket.bucket" + + // record the initialized providers so that we can use them to check for the instances in each region + 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(testAccCheckBucketDestroyWithProvider, &providers), + Steps: []resource.TestStep{ + { + Config: testAccBucketReplicationWithReplicationConfigurationWithRTCConfig(rInt, 15), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExistsWithProvider(resourceName, acctest.RegionProviderFunc(region, &providers)), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.#", "1"), + testAccCheckBucketExistsWithProvider("aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), + testAccCheckBucketReplicationRules( + resourceName, + []*s3.ReplicationRule{ + { + ID: aws.String("rtc"), + DeleteMarkerReplication: &s3.DeleteMarkerReplication{ + Status: aws.String(s3.DeleteMarkerReplicationStatusDisabled), + }, + Destination: &s3.Destination{ + Bucket: aws.String(fmt.Sprintf("arn:%s:s3:::tf-test-bucket-destination-%d", partition, rInt)), + StorageClass: aws.String(s3.StorageClassStandard), + Metrics: &s3.Metrics{ + EventThreshold: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.MetricsStatusEnabled), + }, + ReplicationTime: &s3.ReplicationTime{ + Time: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.ReplicationTimeStatusEnabled), + }, + }, + Filter: &s3.ReplicationRuleFilter{ + Prefix: aws.String(""), + }, + Status: aws.String(s3.ReplicationRuleStatusEnabled), + Priority: aws.Int64(0), + }, + }, + ), + ), + }, + { + Config: testAccBucketReplicationWithReplicationConfigurationWithRTCNoMinutesConfig(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExistsWithProvider(resourceName, acctest.RegionProviderFunc(region, &providers)), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.#", "1"), + testAccCheckBucketExistsWithProvider("aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), + testAccCheckBucketReplicationRules( + resourceName, + []*s3.ReplicationRule{ + { + ID: aws.String("rtc-no-minutes"), + DeleteMarkerReplication: &s3.DeleteMarkerReplication{ + Status: aws.String(s3.DeleteMarkerReplicationStatusDisabled), + }, + Destination: &s3.Destination{ + Bucket: aws.String(fmt.Sprintf("arn:%s:s3:::tf-test-bucket-destination-%d", partition, rInt)), + StorageClass: aws.String(s3.StorageClassStandard), + Metrics: &s3.Metrics{ + EventThreshold: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.MetricsStatusEnabled), + }, + ReplicationTime: &s3.ReplicationTime{ + Time: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.ReplicationTimeStatusEnabled), + }, + }, + Filter: &s3.ReplicationRuleFilter{ + Prefix: aws.String(""), + }, + Status: aws.String(s3.ReplicationRuleStatusEnabled), + Priority: aws.Int64(0), + }, + }, + ), + ), + }, + { + Config: testAccBucketReplicationWithReplicationConfigurationWithRTCNoStatusConfig(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExistsWithProvider(resourceName, acctest.RegionProviderFunc(region, &providers)), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.#", "1"), + testAccCheckBucketExistsWithProvider("aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), + testAccCheckBucketReplicationRules( + resourceName, + []*s3.ReplicationRule{ + { + ID: aws.String("rtc-no-status"), + DeleteMarkerReplication: &s3.DeleteMarkerReplication{ + Status: aws.String(s3.DeleteMarkerReplicationStatusDisabled), + }, + Destination: &s3.Destination{ + Bucket: aws.String(fmt.Sprintf("arn:%s:s3:::tf-test-bucket-destination-%d", partition, rInt)), + StorageClass: aws.String(s3.StorageClassStandard), + Metrics: &s3.Metrics{ + EventThreshold: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.MetricsStatusEnabled), + }, + ReplicationTime: &s3.ReplicationTime{ + Time: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.ReplicationTimeStatusEnabled), + }, + }, + Filter: &s3.ReplicationRuleFilter{ + Prefix: aws.String("foo"), + }, + Priority: aws.Int64(0), + Status: aws.String(s3.ReplicationRuleStatusEnabled), + }, + }, + ), + ), + }, + { + Config: testAccBucketReplicationWithReplicationConfigurationWithRTCNoConfigConfig(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExistsWithProvider(resourceName, acctest.RegionProviderFunc(region, &providers)), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.#", "1"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.0.destination.#", "1"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.0.destination.0.replication_time.#", "1"), + resource.TestCheckResourceAttr(resourceName, "replication_configuration.0.rules.0.destination.0.metrics.#", "1"), + testAccCheckBucketExistsWithProvider("aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), + testAccCheckBucketReplicationRules( + resourceName, + []*s3.ReplicationRule{ + { + ID: aws.String("rtc-no-config"), + DeleteMarkerReplication: &s3.DeleteMarkerReplication{ + Status: aws.String(s3.DeleteMarkerReplicationStatusDisabled), + }, + Destination: &s3.Destination{ + Bucket: aws.String(fmt.Sprintf("arn:%s:s3:::tf-test-bucket-destination-%d", partition, rInt)), + StorageClass: aws.String(s3.StorageClassStandard), + Metrics: &s3.Metrics{ + EventThreshold: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.MetricsStatusEnabled), + }, + ReplicationTime: &s3.ReplicationTime{ + Time: &s3.ReplicationTimeValue{ + Minutes: aws.Int64(15), + }, + Status: aws.String(s3.ReplicationTimeStatusEnabled), + }, + }, + Filter: &s3.ReplicationRuleFilter{ + Prefix: aws.String("foo"), + }, + Priority: aws.Int64(0), + Status: aws.String(s3.ReplicationRuleStatusEnabled), + }, + }, + ), + ), + }, + }, + }) +} + func TestAccS3Bucket_Manage_objectLock(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.arbitrary" @@ -3963,6 +4185,193 @@ resource "aws_s3_bucket" "bucket" { `, randInt, storageClass) } +func testAccBucketReplicationWithReplicationConfigurationWithRTCConfig(randInt int, minutes int) string { + return acctest.ConfigCompose( + testAccBucketReplicationBasicConfig(randInt), + fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "tf-test-bucket-rtc-%[1]d" + acl = "private" + versioning { + enabled = true + } + replication_configuration { + role = aws_iam_role.role.arn + rules { + id = "rtc" + status = "Enabled" + filter { + tags = {} + } + destination { + bucket = aws_s3_bucket.destination.arn + storage_class = "STANDARD" + metrics { + status = "Enabled" + minutes = %[2]d + } + replication_time { + status = "Enabled" + minutes = %[2]d + } + } + } + } +} +`, randInt, minutes)) +} + +func testAccBucketReplicationWithReplicationConfigurationWithRTCNoMinutesConfig(randInt int) string { + return acctest.ConfigCompose( + testAccBucketReplicationBasicConfig(randInt), + fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "tf-test-bucket-rtc-no-minutes-%[1]d" + acl = "private" + versioning { + enabled = true + } + replication_configuration { + role = aws_iam_role.role.arn + rules { + id = "rtc-no-minutes" + status = "Enabled" + filter { + tags = {} + } + destination { + bucket = aws_s3_bucket.destination.arn + storage_class = "STANDARD" + metrics {} + replication_time { + status = "Enabled" + } + } + } + } +} +`, randInt)) +} + +func testAccBucketReplicationWithReplicationConfigurationWithRTCNoStatusConfig(randInt int) string { + return acctest.ConfigCompose( + testAccBucketReplicationBasicConfig(randInt), + fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "tf-test-bucket-rtc-no-minutes-%[1]d" + acl = "private" + versioning { + enabled = true + } + replication_configuration { + role = aws_iam_role.role.arn + rules { + id = "rtc-no-status" + status = "Enabled" + filter { + prefix = "foo" + } + destination { + bucket = aws_s3_bucket.destination.arn + storage_class = "STANDARD" + metrics {} + replication_time { + minutes = 15 + } + } + } + } +} +`, randInt)) +} + +func testAccBucketReplicationWithReplicationConfigurationWithRTCNoConfigConfig(randInt int) string { + return acctest.ConfigCompose( + testAccBucketReplicationBasicConfig(randInt), + fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "tf-test-bucket-rtc-no-config-%[1]d" + acl = "private" + versioning { + enabled = true + } + replication_configuration { + role = aws_iam_role.role.arn + rules { + id = "rtc-no-config" + status = "Enabled" + filter { + prefix = "foo" + } + destination { + bucket = aws_s3_bucket.destination.arn + storage_class = "STANDARD" + metrics {} + replication_time {} + } + } + } +} +`, randInt)) +} + +func testAccBucketReplicationWithReplicationConfigurationWithRTCInvalidNoMetricsConfig(randInt int) string { + return acctest.ConfigCompose( + testAccBucketReplicationBasicConfig(randInt), + fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "tf-test-bucket-rtc-no-metrics-%[1]d" + acl = "private" + versioning { + enabled = true + } + replication_configuration { + role = aws_iam_role.role.arn + rules { + id = "rtc-no-metrics" + status = "Enabled" + filter { + tags = {} + } + destination { + bucket = aws_s3_bucket.destination.arn + storage_class = "STANDARD" + replication_time {} + } + } + } +} +`, randInt)) +} + +func testAccBucketReplicationWithReplicationConfigurationWithRTCNoFilterConfig(randInt int) string { + return acctest.ConfigCompose( + testAccBucketReplicationBasicConfig(randInt), + fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "tf-test-bucket-rtc-no-metrics-%[1]d" + acl = "private" + versioning { + enabled = true + } + replication_configuration { + role = aws_iam_role.role.arn + rules { + id = "rtc-no-metrics" + prefix = "foo" + status = "Enabled" + destination { + bucket = aws_s3_bucket.destination.arn + storage_class = "STANDARD" + replication_time {} + metrics {} + } + } + } +} +`, randInt)) +} + func testAccBucketReplicationWithMultipleDestinationsEmptyFilterConfig(randInt int) string { return acctest.ConfigCompose( testAccBucketReplicationBasicConfig(randInt), From 3316cdf12339a267dd24f5ce3bceacab4f18f08c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 25 Oct 2021 17:18:05 -0400 Subject: [PATCH 3/7] Add CHANGELOG entry. --- .changelog/21176.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/21176.txt diff --git a/.changelog/21176.txt b/.changelog/21176.txt new file mode 100644 index 00000000000..a8e3aa84589 --- /dev/null +++ b/.changelog/21176.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_s3_bucket: Add `metrics` and `replication_time` arguments to `replication_configuration.rules` configuration block to support Amazon S3 Replication Time Control +``` \ No newline at end of file From 9c6c7c3c8243fe9554b2aa9bcd34b8f6b5ac6d05 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 26 Oct 2021 08:08:58 -0400 Subject: [PATCH 4/7] Fix terrafmt errors. --- website/docs/r/s3_bucket.html.markdown | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 3a6b3581ca7..bdce5f7f38c 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -285,14 +285,16 @@ resource "aws_s3_bucket" "source" { destination { bucket = aws_s3_bucket.destination.arn storage_class = "STANDARD" + replication_time { - status = "Enabled" + status = "Enabled" + minutes = 15 + } + + metrics { + status = "Enabled" minutes = 15 } - } - metrics { - status = "Enabled" - minutes = 15 } } } From 01e754803b9236f1eed16f217447bae66d27db7c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 26 Oct 2021 08:28:01 -0400 Subject: [PATCH 5/7] Nudge GitHub actions. From a09c8985a05507d26305594d067e8f4922374dcc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 26 Oct 2021 08:30:35 -0400 Subject: [PATCH 6/7] Fix providerlint error 'AT012: file contains multiple acceptance test name prefixes: [TestAccS3Bucket TestAccBucket]'. --- internal/service/s3/bucket_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index d0e25e20811..895d570fa41 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -2217,7 +2217,7 @@ func TestAccS3Bucket_Replication_schemaV2SameRegion(t *testing.T) { }) } -func TestAccBucket_Replication_RTC_expectValidationError(t *testing.T) { +func TestAccS3Bucket_Replication_RTC_expectValidationError(t *testing.T) { rInt := sdkacctest.RandInt() // record the initialized providers so that we can use them to check for the instances in each region @@ -2244,7 +2244,7 @@ func TestAccBucket_Replication_RTC_expectValidationError(t *testing.T) { }) } -func TestAccBucket_Replication_RTC_valid(t *testing.T) { +func TestAccS3Bucket_Replication_RTC_valid(t *testing.T) { rInt := sdkacctest.RandInt() alternateRegion := acctest.AlternateRegion() region := acctest.Region() From 7f4ea96f8d8c9a744c488dcd3b104e9281fcd9ca Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 26 Oct 2021 12:31:30 -0400 Subject: [PATCH 7/7] r/aws_s3_bucket: Add RTC attributes to hash. Acceptance test output: % make testacc TESTARGS='-run=TestAccS3Bucket_Replication_RTC_valid' PKG_NAME=internal/service/s3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20 -run=TestAccS3Bucket_Replication_RTC_valid -timeout 180m === RUN TestAccS3Bucket_Replication_RTC_valid === PAUSE TestAccS3Bucket_Replication_RTC_valid === CONT TestAccS3Bucket_Replication_RTC_valid --- PASS: TestAccS3Bucket_Replication_RTC_valid (125.64s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 128.582s --- internal/service/s3/bucket.go | 76 +++++++++++++++----------- internal/service/s3/bucket_test.go | 86 +----------------------------- 2 files changed, 46 insertions(+), 116 deletions(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 609050648f5..f143f556cb3 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -480,7 +480,6 @@ func ResourceBucket() *schema.Resource { "metrics": { Type: schema.TypeList, Optional: true, - MinItems: 1, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -2086,7 +2085,6 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc rcRule.ID = aws.String(rrid.(string)) } - rtcEnabled := false // Note: RTC requires schemaV2 ruleDestination := &s3.Destination{} if dest, ok := rr["destination"].([]interface{}); ok && len(dest) > 0 { if dest[0] != nil { @@ -2115,19 +2113,13 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc } // replication metrics (required for RTC) - metricsEnabled := false if metrics, ok := bd["metrics"].([]interface{}); ok && len(metrics) > 0 { metricsConfig := &s3.Metrics{} metricsValues := metrics[0].(map[string]interface{}) metricsConfig.EventThreshold = &s3.ReplicationTimeValue{} - metricsConfig.Status = aws.String(metricsValues["status"].(string)) metricsConfig.EventThreshold.Minutes = aws.Int64(int64(metricsValues["minutes"].(int))) ruleDestination.Metrics = metricsConfig - - if *metricsConfig.Status == s3.MetricsStatusEnabled { - metricsEnabled = true - } } // replication time control (RTC) @@ -2137,14 +2129,6 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc rtcConfig.Status = aws.String(rtcValues["status"].(string)) rtcConfig.Time = &s3.ReplicationTimeValue{} rtcConfig.Time.Minutes = aws.Int64(int64(rtcValues["minutes"].(int))) - - if *rtcConfig.Status == s3.ReplicationTimeStatusEnabled { - rtcEnabled = true - if !metricsEnabled { - return fmt.Errorf("Metrics must be enabled to allow S3 RTC. Enable by adding a block like: metrics {}") - } - } - ruleDestination.ReplicationTime = rtcConfig } } @@ -2171,13 +2155,8 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc } } - f, ok := rr["filter"].([]interface{}) - if ok && (len(f) == 0 || f[0] == nil) { - // an empty filter means apply the rule to all objects in the bucket - rcRule.Priority = aws.Int64(int64(rr["priority"].(int))) - rcRule.Filter = &s3.ReplicationRuleFilter{} - } else if ok && len(f) > 0 && f[0] != nil { - // XML schema V2 (required for S3 RTC). + if f, ok := rr["filter"].([]interface{}); ok && len(f) > 0 && f[0] != nil { + // XML schema V2. rcRule.Priority = aws.Int64(int64(rr["priority"].(int))) rcRule.Filter = &s3.ReplicationRuleFilter{} filter := f[0].(map[string]interface{}) @@ -2200,10 +2179,8 @@ func resourceBucketReplicationConfigurationUpdate(conn *s3.S3, d *schema.Resourc Status: aws.String(s3.DeleteMarkerReplicationStatusDisabled), } } - } else if rtcEnabled { - return fmt.Errorf("A filter block must be present to enable RTC due to differences between V1 and V2 of the replication configuration schema.") } else { - // XML schema V1. (Note: Incompatible with RTC) + // XML schema V1. rcRule.Prefix = aws.String(rr["prefix"].(string)) } @@ -2442,17 +2419,14 @@ func flattenBucketReplicationConfiguration(r *s3.ReplicationConfiguration) []map } if v.Destination.ReplicationTime != nil { rtc := map[string]interface{}{ - "minutes": aws.Int64Value(v.Destination.ReplicationTime.Time.Minutes), + "minutes": int(aws.Int64Value(v.Destination.ReplicationTime.Time.Minutes)), "status": aws.StringValue(v.Destination.ReplicationTime.Status), } rd["replication_time"] = []interface{}{rtc} - if v.Destination.Metrics == nil { - log.Printf("[WARN] Metrics must be enabled to enable RTC") - } } if v.Destination.Metrics != nil { metrics := map[string]interface{}{ - "minutes": aws.Int64Value(v.Destination.Metrics.EventThreshold.Minutes), + "minutes": int(aws.Int64Value(v.Destination.Metrics.EventThreshold.Minutes)), "status": aws.StringValue(v.Destination.Metrics.Status), } rd["metrics"] = []interface{}{metrics} @@ -2735,6 +2709,12 @@ func destinationHash(v interface{}) int { if v, ok := m["access_control_translation"].([]interface{}); ok && len(v) > 0 && v[0] != nil { buf.WriteString(fmt.Sprintf("%d-", accessControlTranslationHash(v[0]))) } + if v, ok := m["replication_time"].([]interface{}); ok && len(v) > 0 && v[0] != nil { + buf.WriteString(fmt.Sprintf("%d-", replicationTimeHash(v[0]))) + } + if v, ok := m["metrics"].([]interface{}); ok && len(v) > 0 && v[0] != nil { + buf.WriteString(fmt.Sprintf("%d-", metricsHash(v[0]))) + } return create.StringHashcode(buf.String()) } @@ -2752,6 +2732,40 @@ func accessControlTranslationHash(v interface{}) int { return create.StringHashcode(buf.String()) } +func metricsHash(v interface{}) int { + var buf bytes.Buffer + m, ok := v.(map[string]interface{}) + + if !ok { + return 0 + } + + if v, ok := m["minutes"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + if v, ok := m["status"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + return create.StringHashcode(buf.String()) +} + +func replicationTimeHash(v interface{}) int { + var buf bytes.Buffer + m, ok := v.(map[string]interface{}) + + if !ok { + return 0 + } + + if v, ok := m["minutes"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + if v, ok := m["status"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + return create.StringHashcode(buf.String()) +} + func sourceSelectionCriteriaHash(v interface{}) int { var buf bytes.Buffer m, ok := v.(map[string]interface{}) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 895d570fa41..18a2ddf5793 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -2217,33 +2217,6 @@ func TestAccS3Bucket_Replication_schemaV2SameRegion(t *testing.T) { }) } -func TestAccS3Bucket_Replication_RTC_expectValidationError(t *testing.T) { - rInt := sdkacctest.RandInt() - - // record the initialized providers so that we can use them to check for the instances in each region - 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(testAccCheckBucketDestroyWithProvider, &providers), - Steps: []resource.TestStep{ - { - Config: testAccBucketReplicationWithReplicationConfigurationWithRTCNoFilterConfig(rInt), - ExpectError: regexp.MustCompile(`A filter block must be present to enable RTC due to differences between V1 and V2 of the replication configuration schema.`), - }, - { - Config: testAccBucketReplicationWithReplicationConfigurationWithRTCInvalidNoMetricsConfig(rInt), - ExpectError: regexp.MustCompile(`Metrics must be enabled to allow S3 RTC. Enable by adding a block like: metrics {}`), - }, - }, - }) -} - func TestAccS3Bucket_Replication_RTC_valid(t *testing.T) { rInt := sdkacctest.RandInt() alternateRegion := acctest.AlternateRegion() @@ -4276,7 +4249,7 @@ resource "aws_s3_bucket" "bucket" { storage_class = "STANDARD" metrics {} replication_time { - minutes = 15 + minutes = 15 } } } @@ -4315,63 +4288,6 @@ resource "aws_s3_bucket" "bucket" { `, randInt)) } -func testAccBucketReplicationWithReplicationConfigurationWithRTCInvalidNoMetricsConfig(randInt int) string { - return acctest.ConfigCompose( - testAccBucketReplicationBasicConfig(randInt), - fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = "tf-test-bucket-rtc-no-metrics-%[1]d" - acl = "private" - versioning { - enabled = true - } - replication_configuration { - role = aws_iam_role.role.arn - rules { - id = "rtc-no-metrics" - status = "Enabled" - filter { - tags = {} - } - destination { - bucket = aws_s3_bucket.destination.arn - storage_class = "STANDARD" - replication_time {} - } - } - } -} -`, randInt)) -} - -func testAccBucketReplicationWithReplicationConfigurationWithRTCNoFilterConfig(randInt int) string { - return acctest.ConfigCompose( - testAccBucketReplicationBasicConfig(randInt), - fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = "tf-test-bucket-rtc-no-metrics-%[1]d" - acl = "private" - versioning { - enabled = true - } - replication_configuration { - role = aws_iam_role.role.arn - rules { - id = "rtc-no-metrics" - prefix = "foo" - status = "Enabled" - destination { - bucket = aws_s3_bucket.destination.arn - storage_class = "STANDARD" - replication_time {} - metrics {} - } - } - } -} -`, randInt)) -} - func testAccBucketReplicationWithMultipleDestinationsEmptyFilterConfig(randInt int) string { return acctest.ConfigCompose( testAccBucketReplicationBasicConfig(randInt),