Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/s3_bucket: make acceleration status configurable #23816

Merged
merged 2 commits into from
Mar 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/23816.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_s3_bucket: Update `acceleration_status` 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_accelerate_configuration` resource.
```
31 changes: 27 additions & 4 deletions internal/service/s3/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,11 @@ func ResourceBucket() *schema.Resource {
},

"acceleration_status": {
Type: schema.TypeString,
Computed: true,
Deprecated: "Use the aws_s3_bucket_accelerate_configuration resource instead",
Type: schema.TypeString,
Optional: true,
Computed: true,
Deprecated: "Use the aws_s3_bucket_accelerate_configuration resource instead",
ValidateFunc: validation.StringInSlice(s3.BucketAccelerateStatus_Values(), false),
},

"request_payer": {
Expand Down Expand Up @@ -762,6 +764,12 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error {
}
}

if d.HasChange("acceleration_status") {
if err := resourceBucketInternalAccelerationUpdate(conn, d); err != nil {
return fmt.Errorf("error updating S3 Bucket (%s) Acceleration Status: %w", d.Id(), err)
}
}

if d.HasChange("acl") && !d.IsNewResource() {
if err := resourceBucketInternalACLUpdate(conn, d); err != nil {
return fmt.Errorf("error updating S3 Bucket (%s) ACL: %w", d.Id(), err)
Expand Down Expand Up @@ -1000,7 +1008,7 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error {

// Amazon S3 Transfer Acceleration might not be supported in the region
if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeMethodNotAllowed, ErrCodeUnsupportedArgument, ErrCodeNotImplemented) {
return fmt.Errorf("error getting S3 Bucket acceleration configuration: %w", err)
return fmt.Errorf("error getting S3 Bucket (%s) accelerate configuration: %w", d.Id(), err)
}

if accelerate, ok := accelerateResponse.(*s3.GetBucketAccelerateConfigurationOutput); ok {
Expand Down Expand Up @@ -1482,6 +1490,21 @@ func normalizeRegion(region string) string {

////////////////////////////////////////// Argument-Specific Update Functions //////////////////////////////////////////

func resourceBucketInternalAccelerationUpdate(conn *s3.S3, d *schema.ResourceData) error {
input := &s3.PutBucketAccelerateConfigurationInput{
Bucket: aws.String(d.Id()),
AccelerateConfiguration: &s3.AccelerateConfiguration{
Status: aws.String(d.Get("acceleration_status").(string)),
},
}

_, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) {
return conn.PutBucketAccelerateConfiguration(input)
})

return err
}

func resourceBucketInternalACLUpdate(conn *s3.S3, d *schema.ResourceData) error {
acl := d.Get("acl").(string)
if acl == "" {
Expand Down
60 changes: 60 additions & 0 deletions internal/service/s3/bucket_accelerate_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,66 @@ func TestAccS3BucketAccelerateConfiguration_disappears(t *testing.T) {
})
}

func TestAccS3BucketAccelerateConfiguration_migrate_noChange(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket_accelerate_configuration.test"
bucketResourceName := "aws_s3_bucket.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID),
ProviderFactories: acctest.ProviderFactories,
CheckDestroy: testAccCheckBucketAccelerateConfigurationDestroy,
Steps: []resource.TestStep{
{
Config: testAccBucketConfig_withAcceleration(rName, s3.BucketAccelerateStatusEnabled),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckBucketExists(bucketResourceName),
resource.TestCheckResourceAttr(bucketResourceName, "acceleration_status", s3.BucketAccelerateStatusEnabled),
),
},
{
Config: testAccBucketAccelerateConfigurationBasicConfig(rName, s3.BucketAccelerateStatusEnabled),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckBucketAccelerateConfigurationExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "bucket", bucketResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "status", s3.BucketAccelerateStatusEnabled),
),
},
},
})
}

func TestAccS3BucketAccelerateConfiguration_migrate_withChange(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_s3_bucket_accelerate_configuration.test"
bucketResourceName := "aws_s3_bucket.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID),
ProviderFactories: acctest.ProviderFactories,
CheckDestroy: testAccCheckBucketAccelerateConfigurationDestroy,
Steps: []resource.TestStep{
{
Config: testAccBucketConfig_withAcceleration(rName, s3.BucketAccelerateStatusEnabled),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckBucketExists(bucketResourceName),
resource.TestCheckResourceAttr(bucketResourceName, "acceleration_status", s3.BucketAccelerateStatusEnabled),
),
},
{
Config: testAccBucketAccelerateConfigurationBasicConfig(rName, s3.BucketAccelerateStatusSuspended),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckBucketAccelerateConfigurationExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "bucket", bucketResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "status", s3.BucketAccelerateStatusSuspended),
),
},
},
})
}

func testAccCheckBucketAccelerateConfigurationDestroy(s *terraform.State) error {
conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn

Expand Down
47 changes: 47 additions & 0 deletions internal/service/s3/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/service/cloudformation"
"github.com/aws/aws-sdk-go/service/cloudfront"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
Expand Down Expand Up @@ -225,6 +226,43 @@ func TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled(t *testing.T) {
})
}

func TestAccS3Bucket_Basic_acceleration(t *testing.T) {
bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket")
resourceName := "aws_s3_bucket.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
acctest.PreCheck(t)
acctest.PreCheckPartitionHasService(cloudfront.EndpointsID, t)
},
ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckBucketDestroy,
Steps: []resource.TestStep{
{
Config: testAccBucketConfig_withAcceleration(bucketName, s3.BucketAccelerateStatusEnabled),
Check: resource.ComposeTestCheckFunc(
testAccCheckBucketExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "acceleration_status", s3.BucketAccelerateStatusEnabled),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"force_destroy"},
},
{
Config: testAccBucketConfig_withAcceleration(bucketName, s3.BucketAccelerateStatusSuspended),
Check: resource.ComposeTestCheckFunc(
testAccCheckBucketExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "acceleration_status", s3.BucketAccelerateStatusSuspended),
),
},
},
})
}

// Test TestAccS3Bucket_disappears is designed to fail with a "plan
// not empty" error in Terraform, to check against regressions.
// See https://github.com/hashicorp/terraform/pull/2925
Expand Down Expand Up @@ -1275,6 +1313,15 @@ resource "aws_s3_bucket" "bucket" {
`, bucketName)
}

func testAccBucketConfig_withAcceleration(bucketName, acceleration string) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
acceleration_status = %[2]q
}
`, bucketName, acceleration)
}

func testAccBucketConfig_withACL(bucketName, acl string) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "bucket" {
Expand Down
7 changes: 6 additions & 1 deletion website/docs/r/s3_bucket.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ Provides a S3 bucket resource.

-> This functionality is for managing S3 in an AWS Partition. To manage [S3 on Outposts](https://docs.aws.amazon.com/AmazonS3/latest/dev/S3onOutposts.html), see the [`aws_s3control_bucket`](/docs/providers/aws/r/s3control_bucket.html) resource.

~> **NOTE on S3 Bucket Accelerate Configuration:** S3 Bucket Accelerate can be configured in either the standalone resource [`aws_s3_bucket_accelerate_configuration`](s3_bucket_accelerate_configuration.html)
or with the deprecated parameter `acceleration_status` in the resource `aws_s3_bucket`.
Configuring with both will cause inconsistencies and may overwrite configuration.

~> **NOTE on S3 Bucket canned ACL Configuration:** S3 Bucket canned ACL can be configured in either the standalone resource [`aws_s3_bucket_acl`](s3_bucket_acl.html.markdown)
or with the deprecated parameter `acl` in the resource `aws_s3_bucket`.
Configuring with both will cause inconsistencies and may overwrite configuration.
Expand Down Expand Up @@ -113,6 +117,8 @@ The following arguments are supported:

* `bucket` - (Optional, Forces new resource) The name of the bucket. If omitted, Terraform will assign a random, unique name. Must be lowercase and less than or equal to 63 characters in length. A full list of bucket naming rules [may be found here](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html).
* `bucket_prefix` - (Optional, Forces new resource) Creates a unique bucket name beginning with the specified prefix. Conflicts with `bucket`. Must be lowercase and less than or equal to 37 characters in length. A full list of bucket naming rules [may be found here](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html).
* `acceleration_status` - (Optional, **Deprecated**) Sets the accelerate configuration of an existing bucket. Can be `Enabled` or `Suspended`. Cannot be used in `cn-north-1` or `us-gov-west-1`. Terraform will only perform drift detection if a configuration value is provided.
Use the resource [`aws_s3_bucket_accelerate_configuration`](s3_bucket_accelerate_configuration.html) instead.
* `acl` - (Optional, **Deprecated**) The [canned ACL](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl) to apply. Valid values are `private`, `public-read`, `public-read-write`, `aws-exec-read`, `authenticated-read`, and `log-delivery-write`. Defaults to `private`. Conflicts with `grant`. Terraform will only perform drift detection if a configuration value is provided. Use the resource [`aws_s3_bucket_acl`](s3_bucket_acl.html.markdown) instead.
* `grant` - (Optional, **Deprecated**) An [ACL policy grant](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#sample-acl). See [Grant](#grant) below for details. Conflicts with `acl`. Terraform will only perform drift detection if a configuration value is provided. Use the resource [`aws_s3_bucket_acl`](s3_bucket_acl.html.markdown) instead.
* `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.
Expand Down Expand Up @@ -147,7 +153,6 @@ The `object_lock_configuration` configuration block supports the following argum
In addition to all arguments above, the following attributes are exported:

* `id` - The name of the bucket.
* `acceleration_status` - (Optional) The accelerate configuration status of the bucket. Not available in `cn-north-1` or `us-gov-west-1`.
* `arn` - The ARN of the bucket. Will be of format `arn:aws:s3:::bucketname`.
* `bucket_domain_name` - The bucket domain name. Will be of format `bucketname.s3.amazonaws.com`.
* `bucket_regional_domain_name` - The bucket region-specific domain name. The bucket domain name including the region name, please refer [here](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region) for format. Note: The AWS CloudFront allows specifying S3 region-specific endpoint when creating S3 origin, it will prevent [redirect issues](https://forums.aws.amazon.com/thread.jspa?threadID=216814) from CloudFront to S3 Origin URL.
Expand Down