From a8ec5f1c745828ec4bea8027c71e5b2ec24d95ed Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 28 Jan 2022 20:09:27 -0500 Subject: [PATCH 1/7] feat: deprecate 'versioning' argument --- .../service/apigateway/domain_name_test.go | 7 +- .../service/apigatewayv2/domain_name_test.go | 7 +- .../kafkaconnect/custom_plugin_test.go | 10 +- internal/service/lambda/function_test.go | 10 +- internal/service/mwaa/environment_test.go | 14 +- internal/service/s3/bucket.go | 118 +---------- .../bucket_replication_configuration_test.go | 126 +++++++++--- internal/service/s3/bucket_test.go | 183 +----------------- internal/service/s3/bucket_versioning.go | 6 + internal/service/s3/errors.go | 1 + .../service/s3/object_data_source_test.go | 32 ++- internal/service/s3/object_test.go | 86 +++++--- .../signer/signing_job_data_source_test.go | 15 +- internal/service/signer/signing_job_test.go | 15 +- internal/service/synthetics/canary_test.go | 17 +- website/docs/r/s3_bucket.html.markdown | 25 +-- ...et_replication_configuration.html.markdown | 41 +++- website/docs/r/s3_object.html.markdown | 13 +- 18 files changed, 323 insertions(+), 403 deletions(-) diff --git a/internal/service/apigateway/domain_name_test.go b/internal/service/apigateway/domain_name_test.go index a9c7ca95c87..b58efc9c739 100644 --- a/internal/service/apigateway/domain_name_test.go +++ b/internal/service/apigateway/domain_name_test.go @@ -641,9 +641,12 @@ resource "aws_s3_bucket" "test" { bucket = %[1]q force_destroy = true +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } diff --git a/internal/service/apigatewayv2/domain_name_test.go b/internal/service/apigatewayv2/domain_name_test.go index 933bd93dcb1..a5055c994ee 100644 --- a/internal/service/apigatewayv2/domain_name_test.go +++ b/internal/service/apigatewayv2/domain_name_test.go @@ -499,9 +499,12 @@ resource "aws_s3_bucket" "test" { bucket = %[1]q force_destroy = true +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } diff --git a/internal/service/kafkaconnect/custom_plugin_test.go b/internal/service/kafkaconnect/custom_plugin_test.go index 65b6fbc8cc4..b78f748004b 100644 --- a/internal/service/kafkaconnect/custom_plugin_test.go +++ b/internal/service/kafkaconnect/custom_plugin_test.go @@ -260,13 +260,19 @@ func testAccCustomPluginConfigObjectVersion(name string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_object" "test" { + # Must have versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.id key = %[1]q source = "test-fixtures/mongodb-connector.jar" diff --git a/internal/service/lambda/function_test.go b/internal/service/lambda/function_test.go index 5738c8d68e5..664172dfc23 100644 --- a/internal/service/lambda/function_test.go +++ b/internal/service/lambda/function_test.go @@ -3416,13 +3416,19 @@ resource "aws_s3_bucket" "artifacts" { bucket = "%s" acl = "private" force_destroy = true +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "artifacts" { + bucket = aws_s3_bucket.artifacts.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_object" "o" { + # Must have versioning enabled first + depends_on = [aws_s3_bucket_versioning.artifacts] + bucket = aws_s3_bucket.artifacts.bucket key = "%s" source = "%s" diff --git a/internal/service/mwaa/environment_test.go b/internal/service/mwaa/environment_test.go index fd22452de40..b8b2fcc5c1f 100644 --- a/internal/service/mwaa/environment_test.go +++ b/internal/service/mwaa/environment_test.go @@ -519,9 +519,12 @@ resource "aws_security_group" "test" { resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "private" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } @@ -533,6 +536,9 @@ resource "aws_s3_bucket_public_access_block" "test" { } resource "aws_s3_object" "dags" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.id acl = "private" key = "dags/" @@ -579,6 +585,7 @@ resource "aws_iam_role_policy" "test" { POLICY } + `, rName) } @@ -795,6 +802,9 @@ resource "aws_mwaa_environment" "test" { } resource "aws_s3_object" "plugins" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.id acl = "private" key = "plugins.zip" diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 32dc6b8005d..a59fe5ba70f 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -220,21 +220,20 @@ func ResourceBucket() *schema.Resource { }, "versioning": { - Type: schema.TypeList, - Optional: true, - Computed: true, - MaxItems: 1, + Type: schema.TypeList, + Computed: true, + Deprecated: "Use the aws_s3_bucket_versioning resource instead", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "enabled": { - Type: schema.TypeBool, - Optional: true, - Default: false, + Type: schema.TypeBool, + Computed: true, + Deprecated: "Use the aws_s3_bucket_versioning resource instead", }, "mfa_delete": { - Type: schema.TypeBool, - Optional: true, - Default: false, + Type: schema.TypeBool, + Computed: true, + Deprecated: "Use the aws_s3_bucket_versioning resource instead", }, }, }, @@ -751,23 +750,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("versioning") { - v := d.Get("versioning").([]interface{}) - - if d.IsNewResource() { - if versioning := expandVersioningWhenIsNewResource(v); versioning != nil { - err := resourceBucketInternalVersioningUpdate(conn, d.Id(), versioning) - if err != nil { - return err - } - } - } else { - if err := resourceBucketInternalVersioningUpdate(conn, d.Id(), expandVersioning(v)); err != nil { - return err - } - } - } - if d.HasChange("acl") && !d.IsNewResource() { if err := resourceBucketACLUpdate(conn, d); err != nil { return err @@ -1374,23 +1356,6 @@ func resourceBucketACLUpdate(conn *s3.S3, d *schema.ResourceData) error { return nil } -func resourceBucketInternalVersioningUpdate(conn *s3.S3, bucket string, versioningConfig *s3.VersioningConfiguration) error { - input := &s3.PutBucketVersioningInput{ - Bucket: aws.String(bucket), - VersioningConfiguration: versioningConfig, - } - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return conn.PutBucketVersioning(input) - }) - - if err != nil { - return fmt.Errorf("error putting S3 versioning for bucket (%s): %w", bucket, err) - } - - return nil -} - func resourceBucketInternalObjectLockConfigurationUpdate(conn *s3.S3, d *schema.ResourceData) error { // S3 Object Lock configuration cannot be deleted, only updated. req := &s3.PutObjectLockConfigurationInput{ @@ -2233,71 +2198,6 @@ func expandS3ObjectLockConfiguration(vConf []interface{}) *s3.ObjectLockConfigur // Versioning functions -func expandVersioning(l []interface{}) *s3.VersioningConfiguration { - if len(l) == 0 || l[0] == nil { - return nil - } - - tfMap, ok := l[0].(map[string]interface{}) - - if !ok { - return nil - } - - output := &s3.VersioningConfiguration{} - - if v, ok := tfMap["enabled"].(bool); ok { - if v { - output.Status = aws.String(s3.BucketVersioningStatusEnabled) - } else { - output.Status = aws.String(s3.BucketVersioningStatusSuspended) - } - } - - if v, ok := tfMap["mfa_delete"].(bool); ok { - if v { - output.MFADelete = aws.String(s3.MFADeleteEnabled) - } else { - output.MFADelete = aws.String(s3.MFADeleteDisabled) - } - } - - return output -} - -func expandVersioningWhenIsNewResource(l []interface{}) *s3.VersioningConfiguration { - if len(l) == 0 || l[0] == nil { - return nil - } - - tfMap, ok := l[0].(map[string]interface{}) - - if !ok { - return nil - } - - output := &s3.VersioningConfiguration{} - - // Only set and return a non-nil VersioningConfiguration with at least one of - // MFADelete or Status enabled as the PutBucketVersioning API request - // does not need to be made for new buckets that don't require versioning. - // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/4494 - - if v, ok := tfMap["enabled"].(bool); ok && v { - output.Status = aws.String(s3.BucketVersioningStatusEnabled) - } - - if v, ok := tfMap["mfa_delete"].(bool); ok && v { - output.MFADelete = aws.String(s3.MFADeleteEnabled) - } - - if output.MFADelete == nil && output.Status == nil { - return nil - } - - return output -} - func flattenVersioning(versioning *s3.GetBucketVersioningOutput) []interface{} { if versioning == nil { return []interface{}{} diff --git a/internal/service/s3/bucket_replication_configuration_test.go b/internal/service/s3/bucket_replication_configuration_test.go index acaf7a3448f..2fe4d70501d 100644 --- a/internal/service/s3/bucket_replication_configuration_test.go +++ b/internal/service/s3/bucket_replication_configuration_test.go @@ -974,24 +974,33 @@ POLICY resource "aws_s3_bucket" "destination" { provider = "awsalternate" bucket = "%[1]s-destination" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination" { + bucket = aws_s3_bucket.destination.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket" "source" { bucket = "%[1]s-source" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "source" { + bucket = aws_s3_bucket.source.id + versioning_configuration { + status = "Enabled" } -}`, rName) +} +`, rName) } func testAccBucketReplicationConfigurationBasic(rName, storageClass string) string { return testAccBucketReplicationConfigurationBase(rName) + fmt.Sprintf(` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1012,6 +1021,8 @@ func testAccBucketReplicationConfigurationRTC(rName string) string { return acctest.ConfigCompose(testAccBucketReplicationConfigurationBase(rName), ` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1046,6 +1057,8 @@ resource "aws_s3_bucket_replication_configuration" "test" { func testAccBucketReplicationConfigurationReplicaMods(rName string) string { return testAccBucketReplicationConfigurationBase(rName) + ` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1078,22 +1091,36 @@ func testAccBucketReplicationConfigurationWithMultipleDestinationsEmptyFilter(rN resource "aws_s3_bucket" "destination2" { provider = "awsalternate" bucket = "%[1]s-destination2" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination2" { + bucket = aws_s3_bucket.destination2.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket" "destination3" { provider = "awsalternate" bucket = "%[1]s-destination3" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination3" { + bucket = aws_s3_bucket.destination3.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "test" { + # Must have bucket versioning enabled first + depends_on = [ + aws_s3_bucket_versioning.source, + aws_s3_bucket_versioning.destination, + aws_s3_bucket_versioning.destination2, + aws_s3_bucket_versioning.destination3 + ] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1147,7 +1174,6 @@ resource "aws_s3_bucket_replication_configuration" "test" { storage_class = "ONEZONE_IA" } } - }`, rName)) } @@ -1158,22 +1184,30 @@ func testAccBucketReplicationConfigurationWithMultipleDestinationsNonEmptyFilter resource "aws_s3_bucket" "destination2" { provider = "awsalternate" bucket = "%[1]s-destination2" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination2" { + bucket = aws_s3_bucket.destination2.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket" "destination3" { provider = "awsalternate" bucket = "%[1]s-destination3" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination3" { + bucket = aws_s3_bucket.destination3.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1214,7 +1248,6 @@ resource "aws_s3_bucket_replication_configuration" "test" { storage_class = "STANDARD_IA" } } - }`, rName)) } @@ -1225,13 +1258,18 @@ func testAccBucketReplicationConfigurationWithMultipleDestinationsTwoDestination resource "aws_s3_bucket" "destination2" { provider = "awsalternate" bucket = "%[1]s-destination2" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination2" { + bucket = aws_s3_bucket.destination2.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1284,6 +1322,8 @@ resource "aws_kms_key" "test" { } resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1316,6 +1356,8 @@ func testAccBucketReplicationConfigurationWithAccessControlTranslation(rName str data "aws_caller_identity" "current" {} resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1342,6 +1384,8 @@ func testAccBucketReplicationConfigurationRulesDestination(rName string) string data "aws_caller_identity" "current" {} resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1370,6 +1414,8 @@ resource "aws_kms_key" "test" { } resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1403,6 +1449,8 @@ resource "aws_s3_bucket_replication_configuration" "test" { func testAccBucketReplicationConfigurationWithoutStorageClass(rName string) string { return testAccBucketReplicationConfigurationBase(rName) + ` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1421,6 +1469,8 @@ resource "aws_s3_bucket_replication_configuration" "test" { func testAccBucketReplicationConfigurationWithV2ConfigurationNoTags(rName string) string { return testAccBucketReplicationConfigurationBase(rName) + ` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1468,22 +1518,30 @@ POLICY resource "aws_s3_bucket" "destination" { bucket = %[2]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination" { + bucket = aws_s3_bucket.destination.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket" "source" { bucket = %[1]q acl = "private" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "source" { + bucket = aws_s3_bucket.source.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1533,22 +1591,30 @@ POLICY resource "aws_s3_bucket" "destination" { bucket = %[2]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination" { + bucket = aws_s3_bucket.destination.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket" "source" { bucket = %[1]q acl = "private" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "source" { + bucket = aws_s3_bucket.source.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1582,6 +1648,8 @@ func testAccBucketReplicationConfiguration_filter_tag(rName, key, value string) testAccBucketReplicationConfigurationBase(rName), fmt.Sprintf(` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1613,6 +1681,8 @@ func testAccBucketReplicationConfiguration_filter_andOperator_tags(rName, key1, testAccBucketReplicationConfigurationBase(rName), fmt.Sprintf(` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1646,6 +1716,8 @@ func testAccBucketReplicationConfiguration_filter_andOperator_prefixAndTags(rNam testAccBucketReplicationConfigurationBase(rName), fmt.Sprintf(` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1678,6 +1750,8 @@ resource "aws_s3_bucket_replication_configuration" "test" { func testAccBucketReplicationConfiguration_schemaV2DestinationMetrics_statusOnly(rName, storageClass string) string { return testAccBucketReplicationConfigurationBase(rName) + fmt.Sprintf(` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn @@ -1709,6 +1783,8 @@ func testAccBucketReplicationConfigurationWithoutPrefix(rName string) string { testAccBucketReplicationConfigurationBase(rName), ` resource "aws_s3_bucket_replication_configuration" "test" { + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.id role = aws_iam_role.test.arn diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index c47bff4d22e..70013e77775 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -392,137 +392,6 @@ func TestAccS3Bucket_Basic_shouldFailNotFound(t *testing.T) { }) } -func TestAccS3Bucket_Manage_versioning(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - resourceName := "aws_s3_bucket.bucket" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckBucketDestroy, - Steps: []resource.TestStep{ - { - Config: testAccBucketWithVersioningConfig(bucketName, true), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "true"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.mfa_delete", "false"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, - }, - { - Config: testAccBucketWithVersioningConfig(bucketName, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.mfa_delete", "false"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, - }, - }, - }) -} - -func TestAccS3Bucket_Manage_versioningDisabled(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - resourceName := "aws_s3_bucket.bucket" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckBucketDestroy, - Steps: []resource.TestStep{ - { - Config: testAccBucketWithVersioningConfig(bucketName, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.mfa_delete", "false"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, - }, - }, - }) -} - -func TestAccS3Bucket_Manage_MfaDeleteDisabled(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - resourceName := "aws_s3_bucket.bucket" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckBucketDestroy, - Steps: []resource.TestStep{ - { - Config: testAccBucketWithVersioningMfaDeleteConfig(bucketName, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.mfa_delete", "false"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, - }, - }, - }) -} - -func TestAccS3Bucket_Manage_versioningAndMfaDeleteDisabled(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - resourceName := "aws_s3_bucket.bucket" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckBucketDestroy, - Steps: []resource.TestStep{ - { - Config: testAccBucketWithVersioningVersioningAndMfaDeleteConfig(bucketName, false, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "versioning.0.mfa_delete", "false"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, - }, - }, - }) -} - func TestAccS3Bucket_Manage_objectLock(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.arbitrary" @@ -1386,48 +1255,11 @@ resource "aws_s3_bucket" "bucket" { resource "aws_s3_bucket_acl" "test" { bucket = aws_s3_bucket.bucket.id - acl = "public-read" + acl = "private" } `, bucketName) } -func testAccBucketWithVersioningConfig(bucketName string, enabled bool) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - - versioning { - enabled = %[2]t - } -} -`, bucketName, enabled) -} - -func testAccBucketWithVersioningMfaDeleteConfig(bucketName string, mfaDelete bool) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - - versioning { - mfa_delete = %[2]t - } -} -`, bucketName, mfaDelete) -} - -func testAccBucketWithVersioningVersioningAndMfaDeleteConfig(bucketName string, enabled, mfaDelete bool) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - - versioning { - enabled = %[2]t - mfa_delete = %[3]t - } -} -`, bucketName, enabled, mfaDelete) -} - func testAccObjectLockEnabledNoDefaultRetention(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "arbitrary" { @@ -1474,7 +1306,7 @@ resource "aws_s3_bucket" "bucket" { resource "aws_s3_bucket_acl" "test" { bucket = aws_s3_bucket.bucket.id - acl = "public-read" + acl = "private" } `, bucketName) } @@ -1485,10 +1317,6 @@ resource "aws_s3_bucket" "bucket" { bucket = "%s" force_destroy = true - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } @@ -1504,6 +1332,13 @@ resource "aws_s3_bucket_acl" "test" { bucket = aws_s3_bucket.bucket.id acl = "private" } + +resource "aws_s3_bucket_versioning" "bucket" { + bucket = aws_s3_bucket.bucket.id + versioning_configuration { + status = "Enabled" + } +} `, bucketName) } diff --git a/internal/service/s3/bucket_versioning.go b/internal/service/s3/bucket_versioning.go index 5f1ea2df852..2cb19368217 100644 --- a/internal/service/s3/bucket_versioning.go +++ b/internal/service/s3/bucket_versioning.go @@ -51,6 +51,7 @@ func ResourceBucketVersioning() *schema.Resource { "mfa_delete": { Type: schema.TypeString, Optional: true, + Computed: true, ValidateFunc: validation.StringInSlice(s3.MFADelete_Values(), false), }, "status": { @@ -200,6 +201,11 @@ func resourceBucketVersioningDelete(ctx context.Context, d *schema.ResourceData, return nil } + if tfawserr.ErrMessageContains(err, ErrCodeInvalidBucketState, "An Object Lock configuration is present on this bucket, so the versioning state cannot be changed") { + log.Printf("[WARN] S3 bucket versioning cannot be suspended with Object Lock Configuration present on bucket (%s), removing from state", bucket) + return nil + } + if err != nil { return diag.FromErr(fmt.Errorf("error deleting S3 bucket versioning (%s): %w", d.Id(), err)) } diff --git a/internal/service/s3/errors.go b/internal/service/s3/errors.go index 4d90cd249cd..7c7ad0db4f0 100644 --- a/internal/service/s3/errors.go +++ b/internal/service/s3/errors.go @@ -4,6 +4,7 @@ package s3 // https://docs.aws.amazon.com/sdk-for-go/api/service/s3/#pkg-constants const ( + ErrCodeInvalidBucketState = "InvalidBucketState" ErrCodeMethodNotAllowed = "MethodNotAllowed" ErrCodeNoSuchBucketPolicy = "NoSuchBucketPolicy" ErrCodeNoSuchConfiguration = "NoSuchConfiguration" diff --git a/internal/service/s3/object_data_source_test.go b/internal/service/s3/object_data_source_test.go index 821a21d273a..179f8ad1c2b 100644 --- a/internal/service/s3/object_data_source_test.go +++ b/internal/service/s3/object_data_source_test.go @@ -577,13 +577,19 @@ func testAccObjectDataSourceConfig_allParams(randInt int) string { return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket" { bucket = "tf-object-test-bucket-%[1]d" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "object_bucket" { + bucket = aws_s3_bucket.object_bucket.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.object_bucket] + bucket = aws_s3_bucket.object_bucket.bucket key = "tf-testing-obj-%[1]d-all-params" @@ -615,15 +621,18 @@ func testAccObjectDataSourceConfig_objectLockLegalHoldOff(randInt int) string { resource "aws_s3_bucket" "object_bucket" { bucket = "tf-object-test-bucket-%[1]d" - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "object_bucket" { + bucket = aws_s3_bucket.object_bucket.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { bucket = aws_s3_bucket.object_bucket.bucket key = "tf-testing-obj-%[1]d" @@ -643,15 +652,18 @@ func testAccObjectDataSourceConfig_objectLockLegalHoldOn(randInt int, retainUnti resource "aws_s3_bucket" "object_bucket" { bucket = "tf-object-test-bucket-%[1]d" - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "object_bucket" { + bucket = aws_s3_bucket.object_bucket.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { bucket = aws_s3_bucket.object_bucket.bucket key = "tf-testing-obj-%[1]d" diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 12e95f71c99..fb42e6dc542 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1677,9 +1677,12 @@ func testAccObjectConfig_updateable(rName string, bucketVersioning bool, source return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket_3" { bucket = %[1]q +} - versioning { - enabled = %[2]t +resource "aws_s3_bucket_versioning" "object_bucket_3" { + bucket = aws_s3_bucket.object_bucket_3.id + versioning_configuration { + status = "Enabled" } } @@ -1696,9 +1699,12 @@ func testAccObjectConfig_updateableViaAccessPoint(rName string, bucketVersioning return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = %[2]t +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } @@ -1752,9 +1758,12 @@ func testAccObjectConfig_acl(rName string, content, acl string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } @@ -1786,9 +1795,12 @@ func testAccObjectConfig_withTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } @@ -1810,9 +1822,12 @@ func testAccObjectConfig_withUpdatedTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } @@ -1835,9 +1850,12 @@ func testAccObjectConfig_withNoTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } @@ -1872,15 +1890,18 @@ func testAccObjectConfig_noObjectLockLegalHold(rName string, content string) str resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { bucket = aws_s3_bucket.test.bucket key = "test-key" @@ -1895,15 +1916,18 @@ func testAccObjectConfig_withObjectLockLegalHold(rName string, content, legalHol resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { bucket = aws_s3_bucket.test.bucket key = "test-key" @@ -1919,15 +1943,18 @@ func testAccObjectConfig_noObjectLockRetention(rName string, content string) str resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { bucket = aws_s3_bucket.test.bucket key = "test-key" @@ -1942,15 +1969,18 @@ func testAccObjectConfig_withObjectLockRetention(rName string, content, retainUn resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { bucket = aws_s3_bucket.test.bucket key = "test-key" diff --git a/internal/service/signer/signing_job_data_source_test.go b/internal/service/signer/signing_job_data_source_test.go index 04f6598b3cb..6d4375ec6e1 100644 --- a/internal/service/signer/signing_job_data_source_test.go +++ b/internal/service/signer/signing_job_data_source_test.go @@ -42,13 +42,15 @@ resource "aws_signer_signing_profile" "test" { } resource "aws_s3_bucket" "source" { - bucket = "%[1]s-source" + bucket = "%[1]s-source" + force_destroy = true +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "source" { + bucket = aws_s3_bucket.source.id + versioning_configuration { + status = "Enabled" } - - force_destroy = true } resource "aws_s3_bucket" "destination" { @@ -57,6 +59,9 @@ resource "aws_s3_bucket" "destination" { } resource "aws_s3_object" "source" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.bucket key = "lambdatest.zip" source = "test-fixtures/lambdatest.zip" diff --git a/internal/service/signer/signing_job_test.go b/internal/service/signer/signing_job_test.go index 426486e761b..aacafe6c575 100644 --- a/internal/service/signer/signing_job_test.go +++ b/internal/service/signer/signing_job_test.go @@ -51,13 +51,15 @@ resource "aws_signer_signing_profile" "test" { } resource "aws_s3_bucket" "source" { - bucket = "%[1]s-source" + bucket = "%[1]s-source" + force_destroy = true +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "source" { + bucket = aws_s3_bucket.source.id + versioning_configuration { + status = "Enabled" } - - force_destroy = true } resource "aws_s3_bucket" "destination" { @@ -66,6 +68,9 @@ resource "aws_s3_bucket" "destination" { } resource "aws_s3_object" "source" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.source] + bucket = aws_s3_bucket.source.bucket key = "lambdatest.zip" source = "test-fixtures/lambdatest.zip" diff --git a/internal/service/synthetics/canary_test.go b/internal/service/synthetics/canary_test.go index 22307b8f0aa..63a64041bb1 100644 --- a/internal/service/synthetics/canary_test.go +++ b/internal/service/synthetics/canary_test.go @@ -637,15 +637,18 @@ resource "aws_s3_bucket" "test" { acl = "private" force_destroy = true - versioning { - enabled = true - } - tags = { Name = %[1]q } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_iam_role" "test" { name = %[1]q @@ -803,6 +806,9 @@ resource "aws_synthetics_canary" "test" { func testAccCanaryBasicConfig(rName string) string { return acctest.ConfigCompose(testAccCanaryBaseConfig(rName), fmt.Sprintf(` resource "aws_synthetics_canary" "test" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + name = %[1]q artifact_s3_location = "s3://${aws_s3_bucket.test.bucket}/" execution_role_arn = aws_iam_role.test.arn @@ -957,6 +963,9 @@ resource "aws_synthetics_canary" "test" { } resource "aws_s3_object" "test" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = %[1]q source = "test-fixtures/lambdatest.zip" diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 5e0f47cd3e7..d3e97bc63d2 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -44,20 +44,8 @@ See the [`aws_s3_bucket_cors_configuration` resource](s3_bucket_cors_configurati ### Using versioning -```terraform -resource "aws_s3_bucket" "b" { - bucket = "my-tf-test-bucket" - - versioning { - enabled = true - } -} - -resource "aws_s3_bucket_acl" "example" { - bucket = aws_s3_bucket.b.id - acl = "private" -} -``` +The `versioning` argument is read-only as of version 4.0 of the Terraform AWS Provider. +See the [`aws_s3_bucket_versioning` resource](s3_bucket_versioning.html.markdown) for configuration details. ### Enable Logging @@ -111,14 +99,8 @@ The following arguments are supported: * `grant` - (Optional) An [ACL policy grant](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#sample-acl) (documented below). Conflicts with `acl`. * `tags` - (Optional) A map of tags to assign to the bucket. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `force_destroy` - (Optional, Default:`false`) A boolean that indicates all objects (including any [locked objects](https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock-overview.html)) should be deleted from the bucket so that the bucket can be destroyed without error. These objects are *not* recoverable. -* `versioning` - (Optional) A state of [versioning](https://docs.aws.amazon.com/AmazonS3/latest/dev/Versioning.html) (documented below) * `object_lock_configuration` - (Optional) A configuration of [S3 object locking](https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock.html) (documented below) -The `versioning` object supports the following: - -* `enabled` - (Optional) Enable versioning. Once you version-enable a bucket, it can never return to an unversioned state. You can, however, suspend versioning on that bucket. -* `mfa_delete` - (Optional) Enable MFA delete for either `Change the versioning state of your bucket` or `Permanently delete an object version`. Default is `false`. This cannot be used to toggle this setting but is available to allow managed buckets to reflect the state in AWS - The `grant` object supports the following: * `id` - (optional) Canonical user id to grant for. Used only when `type` is `CanonicalUser`. @@ -222,6 +204,9 @@ In addition to all arguments above, the following attributes are exported: * `sse_algorithm` - (required) The server-side encryption algorithm used. * `bucket_key_enabled` - (Optional) Whether an [Amazon S3 Bucket Key](https://docs.aws.amazon.com/AmazonS3/latest/dev/bucket-key.html) is used for SSE-KMS. * `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). +* `versioning` - The [versioning](https://docs.aws.amazon.com/AmazonS3/latest/dev/Versioning.html) state of the bucket. + * `enabled` - Whether versioning is enabled. + * `mfa_delete` - Whether MFA delete is enabled. * `website` - The website configuration, if configured. * `error_document` - The name of the error document for the website. * `index_document` - The name of the index document for the website. diff --git a/website/docs/r/s3_bucket_replication_configuration.html.markdown b/website/docs/r/s3_bucket_replication_configuration.html.markdown index 3f83adf71d8..54f14c7cbbd 100644 --- a/website/docs/r/s3_bucket_replication_configuration.html.markdown +++ b/website/docs/r/s3_bucket_replication_configuration.html.markdown @@ -93,9 +93,12 @@ resource "aws_iam_role_policy_attachment" "replication" { resource "aws_s3_bucket" "destination" { bucket = "tf-test-bucket-destination-12345" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "destination" { + bucket = aws_s3_bucket.destination.id + versioning_configuration { + status = "Enabled" } } @@ -103,13 +106,21 @@ resource "aws_s3_bucket" "source" { provider = aws.central bucket = "tf-test-bucket-source-12345" acl = "private" +} + +resource "aws_s3_bucket_versioning" "source" { + provider = aws.central - versioning { - enabled = true + bucket = aws_s3_bucket.source.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "replication" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.source] + role = aws_iam_role.replication.arn bucket = aws_s3_bucket.source.id @@ -133,22 +144,33 @@ resource "aws_s3_bucket_replication_configuration" "replication" { resource "aws_s3_bucket" "east" { bucket = "tf-test-bucket-east-12345" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "east" { + bucket = aws_s3_bucket.east.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket" "west" { provider = west bucket = "tf-test-bucket-west-12345" +} + +resource "aws_s3_bucket_versioning" "west" { + provider = west - versioning { - enabled = true + bucket = aws_s3_bucket.west.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_replication_configuration" "east_to_west" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.east] + role = aws_iam_role.east_replication.arn bucket = aws_s3_bucket.east.id @@ -165,6 +187,9 @@ resource "aws_s3_bucket_replication_configuration" "east_to_west" { } resource "aws_s3_bucket_replication_configuration" "west_to_east" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.west] + role = aws_iam_role.west_replication.arn bucket = aws_s3_bucket.west.id diff --git a/website/docs/r/s3_object.html.markdown b/website/docs/r/s3_object.html.markdown index fad048f3b29..26e2d837ecb 100644 --- a/website/docs/r/s3_object.html.markdown +++ b/website/docs/r/s3_object.html.markdown @@ -87,16 +87,19 @@ resource "aws_s3_bucket" "examplebucket" { bucket = "examplebuckettftest" acl = "private" - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } -resource "aws_s3_object" "example" { +resource "aws_s3_bucket_versioning" "example" { + bucket = aws_s3_bucket.examplebucket.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_object" "examplebucket_object" { key = "someobject" bucket = aws_s3_bucket.examplebucket.id source = "important.txt" From cff9affc15a3c6876fa4a8e3b402ab8b026777ca Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 28 Jan 2022 20:09:40 -0500 Subject: [PATCH 2/7] Update CHANGELOG for #22606 --- .changelog/22606.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22606.txt diff --git a/.changelog/22606.txt b/.changelog/22606.txt new file mode 100644 index 00000000000..871479c63e4 --- /dev/null +++ b/.changelog/22606.txt @@ -0,0 +1,3 @@ +```release-note:note +resource/aws_s3_bucket: The `versioning` argument has been deprecated and is now read-only. Use the `aws_s3_bucket_versioning` resource instead. +``` \ No newline at end of file From 48c9db0e73990cbe2debdc3a7b31a43750584836 Mon Sep 17 00:00:00 2001 From: angie pinilla Date: Wed, 2 Feb 2022 12:49:31 -0500 Subject: [PATCH 3/7] Update .changelog/22606.txt --- .changelog/22606.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/22606.txt b/.changelog/22606.txt index 871479c63e4..a0bd8b452f0 100644 --- a/.changelog/22606.txt +++ b/.changelog/22606.txt @@ -1,3 +1,3 @@ -```release-note:note +```release-note:breaking-change resource/aws_s3_bucket: The `versioning` argument has been deprecated and is now read-only. Use the `aws_s3_bucket_versioning` resource instead. ``` \ No newline at end of file From 3a4a4a0a0254d08afcc2ee9159736cbded6b32dd Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Sun, 6 Feb 2022 22:03:15 -0500 Subject: [PATCH 4/7] update s3 bucket object docs with versioning resource --- website/docs/r/s3_bucket_object.html.markdown | 15 +++++++++++---- website/docs/r/s3_object.html.markdown | 3 +++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/website/docs/r/s3_bucket_object.html.markdown b/website/docs/r/s3_bucket_object.html.markdown index baa723757e8..19d17247c0a 100644 --- a/website/docs/r/s3_bucket_object.html.markdown +++ b/website/docs/r/s3_bucket_object.html.markdown @@ -89,16 +89,23 @@ resource "aws_s3_bucket" "examplebucket" { bucket = "examplebuckettftest" acl = "private" - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } + +resource "aws_s3_bucket_versioning" "example" { + bucket = aws_s3_bucket.examplebucket.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_bucket_object" "example" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.example] + key = "someobject" bucket = aws_s3_bucket.examplebucket.id source = "important.txt" diff --git a/website/docs/r/s3_object.html.markdown b/website/docs/r/s3_object.html.markdown index 26e2d837ecb..8d0ab076e84 100644 --- a/website/docs/r/s3_object.html.markdown +++ b/website/docs/r/s3_object.html.markdown @@ -100,6 +100,9 @@ resource "aws_s3_bucket_versioning" "example" { } resource "aws_s3_object" "examplebucket_object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.example] + key = "someobject" bucket = aws_s3_bucket.examplebucket.id source = "important.txt" From 2285d3b0929d435e96effc6ce6f2b7b0fe2bab2e Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Sun, 6 Feb 2022 22:22:02 -0500 Subject: [PATCH 5/7] update additional s3 bucket object tests with versioning resource; update lifecycle config docs --- .../s3/bucket_object_data_source_test.go | 38 ++++-- internal/service/s3/bucket_object_test.go | 122 +++++++++++++----- ...cket_lifecycle_configuration.html.markdown | 10 +- 3 files changed, 127 insertions(+), 43 deletions(-) diff --git a/internal/service/s3/bucket_object_data_source_test.go b/internal/service/s3/bucket_object_data_source_test.go index f9a441c9a7a..c1804acc235 100644 --- a/internal/service/s3/bucket_object_data_source_test.go +++ b/internal/service/s3/bucket_object_data_source_test.go @@ -548,13 +548,19 @@ func testAccBucketObjectDataSourceConfig_allParams(randInt int) string { return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket" { bucket = "tf-object-test-bucket-%[1]d" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.object_bucket.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.object_bucket.bucket key = "tf-testing-obj-%[1]d-all-params" @@ -586,16 +592,22 @@ func testAccBucketObjectDataSourceConfig_objectLockLegalHoldOff(randInt int) str resource "aws_s3_bucket" "object_bucket" { bucket = "tf-object-test-bucket-%[1]d" - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.object_bucket.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.object_bucket.bucket key = "tf-testing-obj-%[1]d" content = "Hello World" @@ -614,16 +626,22 @@ func testAccBucketObjectDataSourceConfig_objectLockLegalHoldOn(randInt int, reta resource "aws_s3_bucket" "object_bucket" { bucket = "tf-object-test-bucket-%[1]d" - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.object_bucket.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.object_bucket.bucket key = "tf-testing-obj-%[1]d" content = "Hello World" diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index 28bff241ebc..9ba85f7604c 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -474,7 +474,7 @@ func TestAccS3BucketObject_updatesWithVersioningViaAccessPoint(t *testing.T) { CheckDestroy: testAccCheckBucketObjectDestroy, Steps: []resource.TestStep{ { - Config: testAccBucketObjectConfig_updateableViaAccessPoint(rName, true, sourceInitial), + Config: testAccBucketObjectConfig_updateableViaAccessPoint(rName, s3.BucketVersioningStatusEnabled, sourceInitial), Check: resource.ComposeTestCheckFunc( testAccCheckBucketObjectExists(resourceName, &originalObj), testAccCheckBucketObjectBody(&originalObj, "initial versioned object state"), @@ -483,7 +483,7 @@ func TestAccS3BucketObject_updatesWithVersioningViaAccessPoint(t *testing.T) { ), }, { - Config: testAccBucketObjectConfig_updateableViaAccessPoint(rName, true, sourceModified), + Config: testAccBucketObjectConfig_updateableViaAccessPoint(rName, s3.BucketVersioningStatusEnabled, sourceModified), Check: resource.ComposeTestCheckFunc( testAccCheckBucketObjectExists(resourceName, &modifiedObj), testAccCheckBucketObjectBody(&modifiedObj, "modified versioned object"), @@ -1720,13 +1720,19 @@ func testAccBucketObjectConfig_updateable(rName string, bucketVersioning bool, s return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket_3" { bucket = %[1]q +} - versioning { - enabled = %[2]t +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.object_bucket_3.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.object_bucket_3.bucket key = "updateable-key" source = %[3]q @@ -1735,13 +1741,16 @@ resource "aws_s3_bucket_object" "object" { `, rName, bucketVersioning, source) } -func testAccBucketObjectConfig_updateableViaAccessPoint(rName string, bucketVersioning bool, source string) string { +func testAccBucketObjectConfig_updateableViaAccessPoint(rName, bucketVersioning, source string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = %[2]t +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = %[2]q } } @@ -1751,6 +1760,9 @@ resource "aws_s3_access_point" "test" { } resource "aws_s3_bucket_object" "test" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_access_point.test.arn key = "updateable-key" source = %[3]q @@ -1795,13 +1807,19 @@ func testAccBucketObjectConfig_acl(rName string, content, acl string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q @@ -1829,13 +1847,19 @@ func testAccBucketObjectConfig_withTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = %[2]q content = %[3]q @@ -1853,13 +1877,19 @@ func testAccBucketObjectConfig_withUpdatedTags(rName, key, content string) strin return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = %[2]q content = %[3]q @@ -1878,13 +1908,19 @@ func testAccBucketObjectConfig_withNoTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = %[2]q content = %[3]q @@ -1915,16 +1951,22 @@ func testAccBucketObjectConfig_noObjectLockLegalHold(rName string, content strin resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q @@ -1938,16 +1980,22 @@ func testAccBucketObjectConfig_withObjectLockLegalHold(rName string, content, le resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q @@ -1962,16 +2010,22 @@ func testAccBucketObjectConfig_noObjectLockRetention(rName string, content strin resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q @@ -1985,16 +2039,22 @@ func testAccBucketObjectConfig_withObjectLockRetention(rName string, content, re resource "aws_s3_bucket" "test" { bucket = %[1]q - versioning { - enabled = true - } - object_lock_configuration { object_lock_enabled = "Enabled" } } +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q diff --git a/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown b/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown index 778507f5567..5ee77f85b68 100644 --- a/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown +++ b/website/docs/r/s3_bucket_lifecycle_configuration.html.markdown @@ -70,13 +70,19 @@ resource "aws_s3_bucket_lifecycle_configuration" "bucket-config" { resource "aws_s3_bucket" "versioning_bucket" { bucket = "my-versioning-bucket" acl = "private" +} - versioning { - enabled = true +resource "aws_s3_bucket_versioning" "versioning" { + bucket = aws_s3_bucket.versioning_bucket.id + versioning_configuration { + status = "Enabled" } } resource "aws_s3_bucket_lifecycle_configuration" "versioning-bucket-config" { + # Must have bucket versioning enabled first + depends_on = [aws_s3_bucket_versioning.versioning] + bucket = aws_s3_bucket.versioning_bucket.bucket rule { From c9974cb989b718f3d9271dd0d0e4cd2ded03974f Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Sun, 6 Feb 2022 23:29:17 -0500 Subject: [PATCH 6/7] add instructions for breaking change introduced in #22606 --- website/docs/guides/version-4-upgrade.html.md | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/website/docs/guides/version-4-upgrade.html.md b/website/docs/guides/version-4-upgrade.html.md index d45ba0b18ca..4bbe4e7169c 100644 --- a/website/docs/guides/version-4-upgrade.html.md +++ b/website/docs/guides/version-4-upgrade.html.md @@ -1318,6 +1318,64 @@ The resources that were imported are shown above. These resources are now in your Terraform state and will henceforth be managed by Terraform. ``` +### `versioning` Argument deprecation + +Switch your Terraform configuration to the `aws_s3_bucket_versioning` resource instead. + +For example, given this previous configuration: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... + versioning { + enabled = true + } +} +``` + +It 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 "versioning": its value will be decided automatically based on the result of applying this configuration. +``` + +Since the `versioning` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_versioning` +resource and remove any references to `versioning` and its nested arguments in the `aws_s3_bucket` resource: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} + +resource "aws_s3_bucket_versioning" "example" { + bucket = aws_s3_bucket.example.id + versioning_configuration { + status = "Enabled" + } +} +``` + +It is then recommended running `terraform import` on each new resource to prevent data loss, e.g. + +```shell +$ terraform import aws_s3_bucket_versioning.example example +aws_s3_bucket_versioning.example: Importing from ID "example"... +aws_s3_bucket_versioning.example: Import prepared! + Prepared aws_s3_bucket_versioning for import +aws_s3_bucket_versioning.example: Refreshing state... [id=example] + +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. +``` + ### `website`, `website_domain`, and `website_endpoint` Argument deprecation Switch your Terraform configuration to the `aws_s3_bucket_website_configuration` resource instead. From 41acf7e0bffe405a67a1675b5681fe69c9740ba8 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Sun, 6 Feb 2022 23:47:04 -0500 Subject: [PATCH 7/7] fix SSE refs in tests --- internal/service/athena/database_test.go | 17 +++++++---- internal/service/s3/bucket_object_test.go | 37 +++++++++++++++-------- internal/service/s3/object_copy_test.go | 19 +++++++----- internal/service/s3/object_test.go | 37 +++++++++++++++-------- 4 files changed, 71 insertions(+), 39 deletions(-) diff --git a/internal/service/athena/database_test.go b/internal/service/athena/database_test.go index 3d96f064e3a..3c4646d046a 100644 --- a/internal/service/athena/database_test.go +++ b/internal/service/athena/database_test.go @@ -363,18 +363,23 @@ resource "aws_kms_key" "hoge" { resource "aws_s3_bucket" "hoge" { bucket = "tf-test-athena-db-%[1]d" force_destroy = true +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "test" { + bucket = aws_s3_bucket.hoge.id - server_side_encryption_configuration { - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.hoge.arn - sse_algorithm = "aws:kms" - } + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = aws_kms_key.hoge.arn + sse_algorithm = "aws:kms" } } } resource "aws_athena_database" "hoge" { + # Must have bucket SSE enabled first + depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + name = "%[2]s" bucket = aws_s3_bucket.hoge.bucket force_destroy = %[3]t diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index 9ba85f7604c..c30508177f5 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -2131,19 +2131,24 @@ resource "aws_kms_key" "test" { resource "aws_s3_bucket" "test" { bucket = %[1]q +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "test" { + bucket = aws_s3_bucket.test.id - server_side_encryption_configuration { - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.test.arn - sse_algorithm = "aws:kms" - } - bucket_key_enabled = true + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = aws_kms_key.test.arn + sse_algorithm = "aws:kms" } + bucket_key_enabled = true } } resource "aws_s3_bucket_object" "object" { + # Must have bucket SSE enabled first + depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %q @@ -2160,17 +2165,23 @@ resource "aws_kms_key" "test" { resource "aws_s3_bucket" "test" { bucket = %[1]q - server_side_encryption_configuration { - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.test.arn - sse_algorithm = "aws:kms" - } +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "test" { + bucket = aws_s3_bucket.test.id + + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = aws_kms_key.test.arn + sse_algorithm = "aws:kms" } } } resource "aws_s3_bucket_object" "object" { + # Must have bucket SSE enabled first + depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 73f4efb790d..7cbf74b2fb8 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -179,19 +179,24 @@ resource "aws_s3_object" "source" { resource "aws_s3_bucket" "target" { bucket = "%[1]s-target" +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "test" { + bucket = aws_s3_bucket.target.id - server_side_encryption_configuration { - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.test.arn - sse_algorithm = "aws:kms" - } - bucket_key_enabled = true + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = aws_kms_key.test.arn + sse_algorithm = "aws:kms" } + bucket_key_enabled = true } } resource "aws_s3_object_copy" "test" { + # Must have bucket SSE enabled first + depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + bucket = aws_s3_bucket.target.bucket key = "test" source = "${aws_s3_bucket.source.bucket}/${aws_s3_object.source.key}" diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index fb42e6dc542..752458ff7b4 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -2058,19 +2058,24 @@ resource "aws_kms_key" "test" { resource "aws_s3_bucket" "test" { bucket = %[1]q +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "test" { + bucket = aws_s3_bucket.test.id - server_side_encryption_configuration { - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.test.arn - sse_algorithm = "aws:kms" - } - bucket_key_enabled = true + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = aws_kms_key.test.arn + sse_algorithm = "aws:kms" } + bucket_key_enabled = true } } resource "aws_s3_object" "object" { + # Must have bucket SSE enabled first + depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %q @@ -2087,17 +2092,23 @@ resource "aws_kms_key" "test" { resource "aws_s3_bucket" "test" { bucket = %[1]q - server_side_encryption_configuration { - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.test.arn - sse_algorithm = "aws:kms" - } +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "test" { + bucket = aws_s3_bucket.test.id + + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = aws_kms_key.test.arn + sse_algorithm = "aws:kms" } } } resource "aws_s3_object" "object" { + # Must have bucket SSE enabled first + depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + bucket = aws_s3_bucket.test.bucket key = "test-key" content = %[2]q