Skip to content

Commit

Permalink
Merge pull request #23820 from hashicorp/s3-bucket-versioning
Browse files Browse the repository at this point in the history 
r/s3_bucket: make `versioning` configurable
  • Loading branch information
@anGie44
anGie44 committed Mar 31, 2022
2 parents 6d18853 + dbeae56 commit e9f9d58
Show file tree
Hide file tree
Showing 5 changed files with 490 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .changelog/23820.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_s3_bucket: Update `versioning` parameter to be configurable. Please refer to the documentation for details on drift detection and potential conflicts when configuring this parameter with the standalone `aws_s3_bucket_versioning` resource.
```
109 changes: 103 additions & 6 deletions internal/service/s3/bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,19 +216,21 @@ func ResourceBucket() *schema.Resource {

"versioning": {
Type: schema.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Deprecated: "Use the aws_s3_bucket_versioning resource instead",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Computed: true,
Deprecated: "Use the aws_s3_bucket_versioning resource instead",
Type: schema.TypeBool,
Optional: true,
Default: false,
},
"mfa_delete": {
Type: schema.TypeBool,
Computed: true,
Deprecated: "Use the aws_s3_bucket_versioning resource instead",
Type: schema.TypeBool,
Optional: true,
Default: false,
},
},
},
Expand Down Expand Up @@ -797,6 +799,23 @@ 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 fmt.Errorf("error updating (new) S3 Bucket (%s) Versioning: %w", d.Id(), err)
}
}
} else {
if err := resourceBucketInternalVersioningUpdate(conn, d.Id(), expandVersioning(v)); err != nil {
return fmt.Errorf("error updating S3 Bucket (%s) Versioning: %w", d.Id(), err)
}
}
}

return resourceBucketRead(d, meta)
}

Expand Down Expand Up @@ -1809,6 +1828,19 @@ func resourceBucketInternalObjectLockConfigurationUpdate(conn *s3.S3, d *schema.
return err
}

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)
})

return err
}

///////////////////////////////////////////// Expand and Flatten functions /////////////////////////////////////////////

// Cors Rule functions
Expand Down Expand Up @@ -2376,6 +2408,71 @@ func flattenServerSideEncryptionConfigurationRules(rules []*s3.ServerSideEncrypt

// 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
170 changes: 169 additions & 1 deletion internal/service/s3/bucket_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,139 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning_deprecatedEnabled(t *testin
})
}

func TestAccS3Bucket_Manage_versioning(t *testing.T) {
bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket.test"

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: testAccBucketConfig_withVersioning(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: testAccBucketConfig_withVersioning(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(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket.test"

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: testAccBucketConfig_withVersioning(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(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket.test"

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: testAccBucketConfig_withVersioningMfaDelete(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(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket.test"

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: testAccBucketConfig_withVersioningDisabledAndMfaDelete(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_Security_updateACL(t *testing.T) {
bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket")
bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket.test"

resource.ParallelTest(t, resource.TestCase{
Expand Down Expand Up @@ -2013,6 +2144,43 @@ resource "aws_s3_bucket" "test" {
`, bucketName)
}

func testAccBucketConfig_withVersioning(bucketName string, enabled bool) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
versioning {
enabled = %[2]t
}
}
`, bucketName, enabled)
}

func testAccBucketConfig_withVersioningMfaDelete(bucketName string, mfaDelete bool) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
versioning {
mfa_delete = %[2]t
}
}
`, bucketName, mfaDelete)
}

func testAccBucketConfig_withVersioningDisabledAndMfaDelete(bucketName string, mfaDelete bool) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
versioning {
enabled = false
mfa_delete = %[2]t
}
}
`, bucketName, mfaDelete)
}

func testAccBucketConfig_withNoTags(bucketName string) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
Expand Down
Loading

0 comments on commit e9f9d58

Please sign in to comment.