diff --git a/.changelog/5132.txt b/.changelog/5132.txt new file mode 100644 index 000000000000..897a24e95aa2 --- /dev/null +++ b/.changelog/5132.txt @@ -0,0 +1,3 @@ +```release-note:new-resource +aws_s3_bucket_versioning +``` \ No newline at end of file diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 753a401dcd90..68ad1de237cd 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -1575,7 +1575,7 @@ func Provider() *schema.Provider { "aws_s3_bucket_policy": s3.ResourceBucketPolicy(), "aws_s3_bucket_public_access_block": s3.ResourceBucketPublicAccessBlock(), "aws_s3_bucket_replication_configuration": s3.ResourceBucketReplicationConfiguration(), - "aws_s3_bucket_versioning_configuration": s3.ResourceBucketVersioning(), + "aws_s3_bucket_versioning": s3.ResourceBucketVersioning(), "aws_s3_object_copy": s3.ResourceObjectCopy(), "aws_s3_access_point": s3control.ResourceAccessPoint(), diff --git a/internal/service/s3/bucket_versioning.go b/internal/service/s3/bucket_versioning.go index 3f67409c84a2..cfd797c33373 100644 --- a/internal/service/s3/bucket_versioning.go +++ b/internal/service/s3/bucket_versioning.go @@ -1,167 +1,249 @@ package s3 import ( + "context" "fmt" - "time" + "log" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// the S3 API can be very inconsistent with the status of Versioning on a bucket -// so we require at least this many confirmations that the value is actually set -// before we move on -const s3BucketVersioningConfirmationsRequired = 10 - func ResourceBucketVersioning() *schema.Resource { return &schema.Resource{ - Create: resourceBucketVersioningCreate, - Read: resourceBucketVersioningRead, - Update: resourceBucketVersioningUpdate, - Delete: resourceBucketVersioningDelete, + CreateContext: resourceBucketVersioningCreate, + ReadContext: resourceBucketVersioningRead, + UpdateContext: resourceBucketVersioningUpdate, + DeleteContext: resourceBucketVersioningDelete, + Importer: &schema.ResourceImporter{ + StateContext: schema.ImportStatePassthroughContext, + }, + Schema: map[string]*schema.Schema{ "bucket": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringLenBetween(1, 63), + }, + "expected_bucket_owner": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: verify.ValidAccountID, + }, + "mfa": { Type: schema.TypeString, - Required: true, - ForceNew: true, + Optional: true, }, - "enabled": { - Type: schema.TypeBool, + "versioning_configuration": { + Type: schema.TypeList, Required: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "mfa_delete": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice(s3.MFADelete_Values(), false), + }, + "status": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(s3.BucketVersioningStatus_Values(), false), + }, + }, + }, }, }, } } -func BucketVersioningStatusToBool(status *string) bool { - if status == nil { - return false - } +func resourceBucketVersioningCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).S3Conn + + bucket := d.Get("bucket").(string) + expectedBucketOwner := d.Get("expected_bucket_owner").(string) - if *status == s3.BucketVersioningStatusEnabled { - return true + input := &s3.PutBucketVersioningInput{ + Bucket: aws.String(bucket), + VersioningConfiguration: expandBucketVersioningConfiguration(d.Get("versioning_configuration").([]interface{})), } - return false -} + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) + } -// updates s3 bucket versioning and waits for the change to be confirmed -func putBucketVersioning(s3conn *s3.S3, bucket string, enabled bool) error { - vc := &s3.VersioningConfiguration{} - if enabled { - vc.SetStatus(s3.BucketVersioningStatusEnabled) - } else { - vc.SetStatus(s3.BucketVersioningStatusSuspended) + if v, ok := d.GetOk("mfa"); ok { + input.MFA = aws.String(v.(string)) } - _, err := s3conn.PutBucketVersioning(&s3.PutBucketVersioningInput{ - Bucket: aws.String(bucket), - VersioningConfiguration: vc, + _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { + return conn.PutBucketVersioningWithContext(ctx, input) }) + if err != nil { - return err + return diag.FromErr(fmt.Errorf("error creating S3 bucket versioning for %s: %w", bucket, err)) } - // wait up to 30 seconds for the change to be reflected - updateConfirmations := 0 - for i := 0; i < 120; i++ { - bucketVersioningStatus, err := s3conn.GetBucketVersioning(&s3.GetBucketVersioningInput{ - Bucket: aws.String(bucket), - }) - if err != nil { - return fmt.Errorf("Error getting versioning config for %s: %s", bucket, err) - } + d.SetId(CreateResourceID(bucket, expectedBucketOwner)) - if bucketVersioningStatus != nil && BucketVersioningStatusToBool(bucketVersioningStatus.Status) == enabled { - updateConfirmations++ - if updateConfirmations >= s3BucketVersioningConfirmationsRequired { - return nil - } + return resourceBucketVersioningRead(ctx, d, meta) +} + +func resourceBucketVersioningRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).S3Conn + + bucket, expectedBucketOwner, err := ParseResourceID(d.Id()) + if err != nil { + return diag.FromErr(err) + } + + input := &s3.GetBucketVersioningInput{ + Bucket: aws.String(bucket), + } + + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) + } + + output, err := conn.GetBucketVersioningWithContext(ctx, input) + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket Versioning (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return diag.FromErr(fmt.Errorf("error getting S3 bucket versioning (%s): %w", d.Id(), err)) + } + + if output == nil { + if d.IsNewResource() { + return diag.FromErr(fmt.Errorf("error getting S3 bucket versioning (%s): empty output", d.Id())) } + log.Printf("[WARN] S3 Bucket Versioning (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } - time.Sleep(250 * time.Millisecond) + d.Set("bucket", bucket) + d.Set("expected_bucket_owner", expectedBucketOwner) + if err := d.Set("versioning_configuration", flattenBucketVersioningConfiguration(output)); err != nil { + return diag.FromErr(fmt.Errorf("error setting versioning_configuration: %w", err)) } - return fmt.Errorf("Timed out waiting for confirmation") + return nil } -func resourceBucketVersioningCreate(d *schema.ResourceData, meta interface{}) error { +func resourceBucketVersioningUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - bucket := d.Get("bucket").(string) - versioningEnabled := d.Get("enabled").(bool) - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return nil, putBucketVersioning(conn, bucket, versioningEnabled) - }) + bucket, expectedBucketOwner, err := ParseResourceID(d.Id()) if err != nil { - return fmt.Errorf("Error putting bucket versioning for %s: %s", bucket, err) + return diag.FromErr(err) } - d.SetId(resource.UniqueId()) + input := &s3.PutBucketVersioningInput{ + Bucket: aws.String(bucket), + VersioningConfiguration: expandBucketVersioningConfiguration(d.Get("versioning_configuration").([]interface{})), + } - return nil + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) + } + + if v, ok := d.GetOk("mfa"); ok { + input.MFA = aws.String(v.(string)) + } + + _, err = conn.PutBucketVersioningWithContext(ctx, input) + + if err != nil { + return diag.FromErr(fmt.Errorf("error updating S3 bucket versioning (%s): %w", d.Id(), err)) + } + + return resourceBucketVersioningRead(ctx, d, meta) } -func resourceBucketVersioningRead(d *schema.ResourceData, meta interface{}) error { +func resourceBucketVersioningDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - bucket := d.Get("bucket").(string) + bucket, expectedBucketOwner, err := ParseResourceID(d.Id()) + if err != nil { + return diag.FromErr(err) + } - vo, err := conn.GetBucketVersioning(&s3.GetBucketVersioningInput{ + input := &s3.PutBucketVersioningInput{ Bucket: aws.String(bucket), - }) - if err != nil { - return fmt.Errorf("Error getting bucket versioning for %s: %s", bucket, err) + VersioningConfiguration: &s3.VersioningConfiguration{ + // Status must be provided thus to "remove" this resource, + // we suspend versioning + Status: aws.String(s3.BucketVersioningStatusSuspended), + }, } - var versioningEnabled bool - if vo == nil { - versioningEnabled = false - } else { - versioningEnabled = BucketVersioningStatusToBool(vo.Status) + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) } - if err := d.Set("enabled", versioningEnabled); err != nil { - return fmt.Errorf("[WARN] Error setting versioning status from S3 (%s / %t), error: %s", bucket, versioningEnabled, err) + _, err = conn.PutBucketVersioningWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + return nil + } + + if err != nil { + return diag.FromErr(fmt.Errorf("error deleting S3 bucket versioning (%s): %w", d.Id(), err)) } return nil } -func resourceBucketVersioningUpdate(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*conns.AWSClient).S3Conn - bucket := d.Get("bucket").(string) +func expandBucketVersioningConfiguration(l []interface{}) *s3.VersioningConfiguration { + if len(l) == 0 || l[0] == nil { + return nil + } - if d.HasChange("enabled") { - _, versioningEnabled := d.GetChange("enabled") + tfMap, ok := l[0].(map[string]interface{}) + if !ok { + return nil + } - err := putBucketVersioning(conn, bucket, versioningEnabled.(bool)) - if err != nil { - return fmt.Errorf("Error setting versioning = %t on %s: %s", versioningEnabled, bucket, err) - } + result := &s3.VersioningConfiguration{} + + if v, ok := tfMap["mfa_delete"].(string); ok && v != "" { + result.MFADelete = aws.String(v) } - return nil + if v, ok := tfMap["status"].(string); ok && v != "" { + result.Status = aws.String(v) + } + + return result } -func resourceBucketVersioningDelete(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*conns.AWSClient).S3Conn - bucket := d.Get("bucket").(string) +func flattenBucketVersioningConfiguration(config *s3.GetBucketVersioningOutput) []interface{} { + if config == nil { + return []interface{}{} + } - // disable versioning if it was enabled - if d.Get("enabled").(bool) { - err := putBucketVersioning(conn, bucket, false) - if err != nil { - return fmt.Errorf("Error deleting aws_s3_bucket_versioning: could not disable versioning on bucket %s: %s", bucket, err) - } + m := make(map[string]interface{}) + + if config.MFADelete != nil { + m["mfa_delete"] = aws.StringValue(config.MFADelete) } - d.SetId("") + if config.Status != nil { + m["status"] = aws.StringValue(config.Status) + } - return nil + return []interface{}{m} } diff --git a/internal/service/s3/bucket_versioning_test.go b/internal/service/s3/bucket_versioning_test.go index 92c469ef3561..5867a19b2a45 100644 --- a/internal/service/s3/bucket_versioning_test.go +++ b/internal/service/s3/bucket_versioning_test.go @@ -2,129 +2,228 @@ package s3_test import ( "fmt" - tfs3 "github.com/hashicorp/terraform-provider-aws/internal/service/s3" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfs3 "github.com/hashicorp/terraform-provider-aws/internal/service/s3" ) -func TestAccAWSS3BucketVersioning_basic(t *testing.T) { - rString := sdkacctest.RandString(8) - bucketName := fmt.Sprintf("tf-acc-s3bv-basic-%s", rString) +func TestAccBucketVersioning_basic(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_versioning.test" - resource.Test(t, resource.TestCase{ + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), ProviderFactories: acctest.ProviderFactories, - CheckDestroy: testAccAWSS3BucketVersioningDestroy, + CheckDestroy: testAccCheckBucketVersioningDestroy, Steps: []resource.TestStep{ - // step 0 - initialize a bucket with versioning and make sure it applies { - Config: testAccAWSS3BucketVersioningConfig(bucketName, true), + Config: testAccBucketVersioningBasicConfig(rName, s3.BucketVersioningStatusEnabled), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("aws_s3_bucket_versioning.test", "bucket", bucketName), - testAccAWSS3BucketVersioningCheckStatus(bucketName, true), + testAccCheckBucketVersioningExists(resourceName), + resource.TestCheckResourceAttrPair(resourceName, "bucket", "aws_s3_bucket.test", "id"), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.0.status", s3.BucketVersioningStatusEnabled), ), }, - // step 1 - disable versioning test { - Config: testAccAWSS3BucketVersioningConfig(bucketName, false), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccBucketVersioning_disappears(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_versioning.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketVersioningDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketVersioningBasicConfig(rName, s3.BucketVersioningStatusEnabled), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("aws_s3_bucket_versioning.test", "bucket", bucketName), - testAccAWSS3BucketVersioningCheckStatus(bucketName, false), + testAccCheckBucketVersioningExists(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfs3.ResourceBucketVersioning(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func TestAccBucketVersioning_update(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_versioning.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketVersioningDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketVersioningBasicConfig(rName, s3.BucketVersioningStatusEnabled), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketVersioningExists(resourceName), ), }, - // step 2 - re-enable { - Config: testAccAWSS3BucketVersioningConfig(bucketName, true), + Config: testAccBucketVersioningBasicConfig(rName, s3.BucketVersioningStatusSuspended), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("aws_s3_bucket_versioning.test", "bucket", bucketName), - testAccAWSS3BucketVersioningCheckStatus(bucketName, true), + testAccCheckBucketVersioningExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.0.status", s3.BucketVersioningStatusSuspended), ), }, - // step 3 - test deleting { - Config: testAccAWSS3BucketVersioningConfig_bucket(bucketName), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccBucketVersioningBasicConfig(rName, s3.BucketVersioningStatusEnabled), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("aws_s3_bucket.test", "bucket", bucketName), - testAccAWSS3BucketVersioningCheckStatus(bucketName, false), + testAccCheckBucketVersioningExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.0.status", s3.BucketVersioningStatusEnabled), ), }, }, }) } -func TestS3VersioningStatusToBool(t *testing.T) { - var enabled *string = nil - if tfs3.BucketVersioningStatusToBool(enabled) != false { - t.Errorf("Expected s3VersioningStatusToBool to return false for nil input") - } +// TestAccBucketVersioning_MFADelete can only test for a "Disabled" +// mfa_delete configuration as the "mfa" argument is required if it's enabled +func TestAccBucketVersioning_MFADelete(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_versioning.test" - enabled = new(string) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketVersioningDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketVersioningConfig_MFADelete(rName, s3.MFADeleteDisabled), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketVersioningExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.0.mfa_delete", s3.MFADeleteDisabled), + resource.TestCheckResourceAttr(resourceName, "versioning_configuration.0.status", s3.BucketVersioningStatusEnabled), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} - *enabled = s3.BucketVersioningStatusEnabled - if tfs3.BucketVersioningStatusToBool(enabled) != true { - t.Errorf("Expected s3VersioningStatusToBool to return true for s3.BucketVersioningStatusEnabled") - } +func testAccCheckBucketVersioningDestroy(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_s3_bucket_versioning" { + continue + } + + input := &s3.GetBucketVersioningInput{ + Bucket: aws.String(rs.Primary.ID), + } + + output, err := conn.GetBucketVersioning(input) + + if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + continue + } + + if err != nil { + return fmt.Errorf("error getting S3 bucket versioning (%s): %w", rs.Primary.ID, err) + } - *enabled = s3.BucketVersioningStatusSuspended - if tfs3.BucketVersioningStatusToBool(enabled) != false { - t.Errorf("Expected s3VersioningStatusToBool to return false for s3.BucketVersioningStatusSuspended") + if output != nil && aws.StringValue(output.Status) != s3.BucketVersioningStatusSuspended { + return fmt.Errorf("S3 bucket versioning (%s) still exists", rs.Primary.ID) + } } -} -func testAccAWSS3BucketVersioningDestroy(s *terraform.State) error { - // nothing to do - we get destroyed along with the aws_s3_bucket return nil } -func testAccAWSS3BucketVersioningCheckStatus(bucketName string, versioningEnabled bool) resource.TestCheckFunc { +func testAccCheckBucketVersioningExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("Resource (%s) ID not set", resourceName) + } + conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn - bucketVersioningStatus, err := conn.GetBucketVersioning(&s3.GetBucketVersioningInput{ - Bucket: aws.String(bucketName), - }) - if err != nil { - return fmt.Errorf("Error getting versioning config for %s: %s", bucketName, err) + input := &s3.GetBucketVersioningInput{ + Bucket: aws.String(rs.Primary.ID), } - var bucketVersioningEnabled bool - if bucketVersioningStatus == nil { - bucketVersioningEnabled = false - } else { - bucketVersioningEnabled = tfs3.BucketVersioningStatusToBool(bucketVersioningStatus.Status) + output, err := conn.GetBucketVersioning(input) + + if err != nil { + return fmt.Errorf("error getting S3 bucket versioning (%s): %w", rs.Primary.ID, err) } - if bucketVersioningEnabled != versioningEnabled { - return fmt.Errorf("Expected versioning for %s = %t but got %t", bucketName, versioningEnabled, bucketVersioningEnabled) + if output == nil { + return fmt.Errorf("S3 Bucket versioning (%s) not found", rs.Primary.ID) } return nil } } -func testAccAWSS3BucketVersioningConfig_bucket(bucketName string) string { +func testAccBucketVersioningBasicConfig(rName, status string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { - bucket = "%s" + bucket = %[1]q acl = "private" } -`, bucketName) + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = %[2]q + } +} +`, rName, status) } -func testAccAWSS3BucketVersioningConfig(bucketName string, versioningEnabled bool) string { +func testAccBucketVersioningConfig_MFADelete(rName, mfaDelete string) string { return fmt.Sprintf(` -%s +resource "aws_s3_bucket" "test" { + bucket = %[1]q + acl = "private" +} resource "aws_s3_bucket_versioning" "test" { - bucket = "${aws_s3_bucket.test.id}" - enabled = %t + bucket = aws_s3_bucket.test.id + versioning_configuration { + mfa_delete = %[2]q + status = "Enabled" + } } -`, testAccAWSS3BucketVersioningConfig_bucket(bucketName), versioningEnabled) +`, rName, mfaDelete) } diff --git a/internal/service/s3/id.go b/internal/service/s3/id.go new file mode 100644 index 000000000000..23e67789579b --- /dev/null +++ b/internal/service/s3/id.go @@ -0,0 +1,41 @@ +package s3 + +import ( + "fmt" + "strings" +) + +const resourceIDSeparator = "," + +// CreateResourceID is a generic method for creating an ID string for a bucket-related resource e.g. aws_s3_bucket_versioning. +// The method expects a bucket name and an optional accountID. +func CreateResourceID(bucket, expectedBucketOwner string) string { + if expectedBucketOwner == "" { + return bucket + } + + parts := []string{bucket, expectedBucketOwner} + id := strings.Join(parts, resourceIDSeparator) + + return id +} + +// ParseResourceID is a generic method for parsing an ID string +// for a bucket name and accountID if provided. +func ParseResourceID(id string) (bucket, expectedBucketOwner string, err error) { + parts := strings.Split(id, resourceIDSeparator) + + if len(parts) == 1 && parts[0] != "" { + bucket = parts[0] + return + } + + if len(parts) == 2 && parts[0] != "" && parts[1] != "" { + bucket = parts[0] + expectedBucketOwner = parts[1] + return + } + + err = fmt.Errorf("unexpected format for ID (%s), expected BUCKET or BUCKET%sEXPECTED_BUCKET_OWNER", id, resourceIDSeparator) + return +} diff --git a/internal/service/s3/id_test.go b/internal/service/s3/id_test.go new file mode 100644 index 000000000000..295d58164eac --- /dev/null +++ b/internal/service/s3/id_test.go @@ -0,0 +1,62 @@ +package s3_test + +import ( + "testing" + + tfs3 "github.com/hashicorp/terraform-provider-aws/internal/service/s3" +) + +func TestParseResourceID(t *testing.T) { + testCases := []struct { + TestName string + InputID string + ExpectError bool + ExpectedBucket string + ExpectedBucketOwner string + }{ + { + TestName: "empty ID", + InputID: "", + ExpectError: true, + }, + { + TestName: "incorrect format", + InputID: "test,example,123456789012", + ExpectError: true, + }, + { + TestName: "valid ID with bucket", + InputID: tfs3.CreateResourceID("example", ""), + ExpectedBucket: "example", + ExpectedBucketOwner: "", + }, + { + TestName: "valid ID with bucket and bucket owner", + InputID: tfs3.CreateResourceID("example", "123456789012"), + ExpectedBucket: "example", + ExpectedBucketOwner: "123456789012", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.TestName, func(t *testing.T) { + gotBucket, gotExpectedBucketOwner, err := tfs3.ParseResourceID(testCase.InputID) + + if err == nil && testCase.ExpectError { + t.Fatalf("expected error") + } + + if err != nil && !testCase.ExpectError { + t.Fatalf("unexpected error") + } + + if gotBucket != testCase.ExpectedBucket { + t.Errorf("got bucket %s, expected %s", gotBucket, testCase.ExpectedBucket) + } + + if gotExpectedBucketOwner != testCase.ExpectedBucketOwner { + t.Errorf("got ExpectedBucketOwner %s, expected %s", gotExpectedBucketOwner, testCase.ExpectedBucketOwner) + } + }) + } +} diff --git a/website/docs/r/s3_bucket_versioning.html.markdown b/website/docs/r/s3_bucket_versioning.html.markdown index d8c6344908be..494e68e1dc98 100644 --- a/website/docs/r/s3_bucket_versioning.html.markdown +++ b/website/docs/r/s3_bucket_versioning.html.markdown @@ -3,26 +3,30 @@ layout: "aws" page_title: "AWS - aws_s3_bucket_versioning" sidebar_current: "docs-aws-s3-bucket-versioning" description: |- -Provides a resource for controlling S3 bucket versioning + Provides an S3 bucket versioning resource. --- -# aws_s3_bucket_versioning +# Resource: aws_s3_bucket_versioning -Provides a resource for controlling versioning on an [S3 bucket][1]. Note that this -may conflict with the `versioning` block of an [aws_s3_bucket][1] if the settings are -not the same. +Provides a resource for controlling versioning on an S3 bucket. +Deleting this resource will suspend versioning on the associated S3 bucket. +For more information, see [How S3 versioning works](https://docs.aws.amazon.com/AmazonS3/latest/userguide/manage-versioning-examples.html). + +~> **NOTE:** If you are enabling versioning on the bucket for the first time, AWS recommends that you wait for 15 minutes after enabling versioning before issuing write operations (PUT or DELETE) on objects in the bucket. ## Example Usage -```hcl -resource "aws_s3_bucket" "b" { +```terraform +resource "aws_s3_bucket" "example" { bucket = "example-bucket" acl = "private" } resource "aws_s3_bucket_versioning" "versioning_example" { - bucket = "${aws_s3_bucket.b.id}" - enabled = true + bucket = aws_s3_bucket.example.id + versioning_configuration { + status = "Enabled" + } } ``` @@ -30,11 +34,34 @@ resource "aws_s3_bucket_versioning" "versioning_example" { The following arguments are supported: -* `bucket` - (Required) The name of the S3 bucket -* `enabled` - (Required) Whether to enable versioning on the bucket +* `bucket` - (Required, Forces new resource) The name of the S3 bucket. +* `versioning_configuration` - (Required) Configuration block for the versioning parameters [detailed below](#versioning_configuration). +* `expected_bucket_owner` - (Optional, Forces new resource) The account ID of the expected bucket owner. +* `mfa` - (Optional, Required if `versioning_configuration` `mfa_delete` is enabled) The concatenation of the authentication device's serial number, a space, and the value that is displayed on your authentication device. + +### versioning_configuration + +The `versioning_configuration` configuration block supports the following arguments: + +* `status` - (Required) The versioning state of the bucket. Valid values: `Enabled` or `Suspended`. +* `mfa_delete` - (Optional) Specifies whether MFA delete is enabled in the bucket versioning configuration. Valid values: `Enabled` or `Disabled`. + +## Attributes Reference + +In addition to all arguments above, the following attributes are exported: + +* `id` - The `bucket` or `bucket` and `expected_bucket_owner` separated by a comma (`,`) if the latter is provided. -## Deletion +## Import -Deleting this resource will disable versioning on the bucket. +S3 bucket versioning can be imported using the `bucket`, e.g. -[1]: /docs/providers/aws/r/s3_bucket.html +``` +$ terraform import aws_s3_bucket_versioning.example bucket-name +``` + +In addition, S3 bucket versioning can be imported using the `bucket` and `expected_bucket_owner` separated by a comma (`,`), e.g. + +``` +$ terraform import aws_s3_bucket_versioning.example bucket-name,123456789012 +```