Skip to content

Commit

Permalink
feat: deprecate 'versioning' argument
Browse files Browse the repository at this point in the history
  • Loading branch information
@anGie44
anGie44 committed Jan 15, 2022
1 parent 0c9bcee commit d236ad7
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 298 deletions.
118 changes: 9 additions & 109 deletions internal/service/s3/bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,21 +222,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",
},
},
},
Expand Down Expand Up @@ -776,23 +775,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 := resourceBucketVersioningUpdate(conn, d.Id(), versioning)
if err != nil {
return err
}
}
} else {
if err := resourceBucketVersioningUpdate(conn, d.Id(), expandVersioning(v)); err != nil {
return err
}
}
}

if d.HasChange("acl") && !d.IsNewResource() {
if err := resourceBucketACLUpdate(conn, d); err != nil {
return err
Expand Down Expand Up @@ -1851,23 +1833,6 @@ func resourceBucketACLUpdate(conn *s3.S3, d *schema.ResourceData) error {
return nil
}

func resourceBucketVersioningUpdate(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 resourceBucketLoggingUpdate(conn *s3.S3, d *schema.ResourceData) error {
logging := d.Get("logging").(*schema.Set).List()
bucket := d.Get("bucket").(string)
Expand Down Expand Up @@ -2863,71 +2828,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{}{}
Expand Down
172 changes: 2 additions & 170 deletions internal/service/s3/bucket_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,137 +931,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_Security_corsUpdate(t *testing.T) {
bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket")
resourceName := "aws_s3_bucket.bucket"
Expand Down Expand Up @@ -1819,7 +1688,7 @@ func TestAccS3Bucket_Replication_ruleDestinationAccessControlTranslation(t *test
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"force_destroy", "acl", "versioning"},
ImportStateVerifyIgnore: []string{"force_destroy", "acl"},
},
{
Config: testAccBucketReplicationWithSseKMSEncryptedObjectsAndAccessControlTranslationConfig(rInt),
Expand Down Expand Up @@ -1909,7 +1778,7 @@ func TestAccS3Bucket_Replication_ruleDestinationAddAccessControlTranslation(t *t
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"force_destroy", "acl", "versioning"},
ImportStateVerifyIgnore: []string{"force_destroy", "acl"},
},
{
Config: testAccBucketReplicationWithAccessControlTranslationConfig(rInt),
Expand Down Expand Up @@ -3806,43 +3675,6 @@ resource "aws_s3_bucket" "bucket" {
`, 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 testAccBucketWithCORSConfig(bucketName string) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "bucket" {
Expand Down
Loading

0 comments on commit d236ad7

Please sign in to comment.