diff --git a/.changelog/22537.txt b/.changelog/22537.txt new file mode 100644 index 00000000000..a246fd4f951 --- /dev/null +++ b/.changelog/22537.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +resource/aws_s3_bucket: The `acl` and `grant` arguments have been deprecated and are now read-only. Use the `aws_s3_bucket_acl` resource instead. +``` \ No newline at end of file diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 13e95c0dad8..ce489102cd9 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -75,46 +75,38 @@ func ResourceBucket() *schema.Resource { }, "acl": { - Type: schema.TypeString, - Default: "private", - Optional: true, - ConflictsWith: []string{"grant"}, - ValidateFunc: validation.StringInSlice(BucketCannedACL_Values(), false), + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "grant": { - Type: schema.TypeSet, - Optional: true, - Set: grantHash, - ConflictsWith: []string{"acl"}, + Type: schema.TypeSet, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "id": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "type": { - Type: schema.TypeString, - Required: true, - // TypeAmazonCustomerByEmail is not currently supported - ValidateFunc: validation.StringInSlice([]string{ - s3.TypeCanonicalUser, - s3.TypeGroup, - }, false), + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "uri": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "permissions": { - Type: schema.TypeSet, - Required: true, - Set: schema.HashString, - Elem: &schema.Schema{ - Type: schema.TypeString, - ValidateFunc: validation.StringInSlice(s3.Permission_Values(), false), - }, + Type: schema.TypeSet, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", + Elem: &schema.Schema{Type: schema.TypeString}, }, }, }, @@ -168,31 +160,31 @@ func ResourceBucket() *schema.Resource { "website": { Type: schema.TypeList, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "index_document": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "error_document": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "redirect_all_requests_to": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "routing_rules": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, }, }, @@ -211,12 +203,12 @@ func ResourceBucket() *schema.Resource { "website_endpoint": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "website_domain": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "versioning": { @@ -681,12 +673,6 @@ func resourceBucketCreate(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(bucket), } - if acl, ok := d.GetOk("acl"); ok { - acl := acl.(string) - req.ACL = aws.String(acl) - log.Printf("[DEBUG] S3 bucket %s has canned ACL %s", bucket, acl) - } - awsRegion := meta.(*conns.AWSClient).Region log.Printf("[DEBUG] S3 bucket create: %s, using region: %s", bucket, awsRegion) @@ -750,18 +736,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("acl") && !d.IsNewResource() { - if err := resourceBucketACLUpdate(conn, d); err != nil { - return err - } - } - - if d.HasChange("grant") { - if err := resourceBucketGrantsUpdate(conn, d); err != nil { - return err - } - } - if d.HasChange("object_lock_configuration") { if err := resourceBucketInternalObjectLockConfigurationUpdate(conn, d); err != nil { return err @@ -832,6 +806,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The call to HeadBucket above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketPolicy, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeNoSuchBucketPolicy) { return fmt.Errorf("error getting S3 bucket (%s) policy: %w", d.Id(), err) } @@ -842,24 +825,43 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { d.Set("policy", nil) } - //Read the Grant ACL. Reset if `acl` (canned ACL) is set. - if acl, ok := d.GetOk("acl"); ok && acl.(string) != "private" { + // Read the Grant ACL if configured outside this resource; + // In the event grants are not configured on the bucket, the API returns an empty array + + // Reset `grant` if `acl` (canned ACL) is set. + if acl, ok := d.GetOk("acl"); ok && acl.(string) != s3.BucketCannedACLPrivate { if err := d.Set("grant", nil); err != nil { - return fmt.Errorf("error resetting grant %s", err) + return fmt.Errorf("error resetting grant %w", err) } } else { - apResponse, err := verify.RetryOnAWSCode("NoSuchBucket", func() (interface{}, error) { + // Set the ACL to its default i.e. "private" (to mimic pre-v4.0 schema) + d.Set("acl", s3.BucketCannedACLPrivate) + + apResponse, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { return conn.GetBucketAcl(&s3.GetBucketAclInput{ Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketAcl, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { - return fmt.Errorf("error getting S3 Bucket (%s) ACL: %s", d.Id(), err) + return fmt.Errorf("error getting S3 Bucket (%s) ACL: %w", d.Id(), err) } - log.Printf("[DEBUG] S3 bucket: %s, read ACL grants policy: %+v", d.Id(), apResponse) - grants := flattenGrants(apResponse.(*s3.GetBucketAclOutput)) - if err := d.Set("grant", schema.NewSet(grantHash, grants)); err != nil { - return fmt.Errorf("error setting grant %s", err) + + if aclOutput, ok := apResponse.(*s3.GetBucketAclOutput); ok { + if err := d.Set("grant", flattenGrants(aclOutput)); err != nil { + return fmt.Errorf("error setting grant %s", err) + } + } else { + d.Set("grant", nil) } } @@ -869,6 +871,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketCors, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeNoSuchCORSConfiguration) { return fmt.Errorf("error getting S3 Bucket CORS configuration: %s", err) } @@ -887,6 +899,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketWebsite, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeNotImplemented, ErrCodeNoSuchWebsiteConfiguration) { return fmt.Errorf("error getting S3 Bucket website configuration: %w", err) } @@ -911,6 +933,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketVersioning, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error getting S3 Bucket versioning (%s): %w", d.Id(), err) } @@ -929,6 +960,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketAccelerateConfiguration, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + // Amazon S3 Transfer Acceleration might not be supported in the region if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeMethodNotAllowed, ErrCodeUnsupportedArgument) { return fmt.Errorf("error getting S3 Bucket acceleration configuration: %w", err) @@ -946,6 +986,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketRequestPayment, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error getting S3 Bucket request payment: %s", err) } @@ -961,6 +1010,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLogging, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error getting S3 Bucket logging: %s", err) } @@ -980,6 +1038,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLifecycleConfiguration, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeNoSuchLifecycleConfiguration) { return fmt.Errorf("error getting S3 Bucket (%s) Lifecycle Configuration: %w", d.Id(), err) } @@ -1000,6 +1068,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketReplication, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeReplicationConfigurationNotFound) { return fmt.Errorf("error getting S3 Bucket replication: %w", err) } @@ -1020,6 +1097,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketEncryption, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil && !tfawserr.ErrMessageContains(err, ErrCodeServerSideEncryptionConfigurationNotFound, "encryption configuration was not found") { return fmt.Errorf("error getting S3 Bucket encryption: %w", err) } @@ -1039,6 +1126,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetObjectLockConfiguration, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + // Object lock not supported in all partitions (extra guard, also guards in read func) if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeMethodNotAllowed, ErrCodeObjectLockConfigurationNotFound) { if meta.(*conns.AWSClient).Partition == endpoints.AwsPartitionID || meta.(*conns.AWSClient).Partition == endpoints.AwsUsGovPartitionID { @@ -1074,6 +1170,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { r.Config.Credentials = conn.Config.Credentials }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as s3manager.GetBucketRegionWithClient, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error getting S3 Bucket location: %s", err) } @@ -1100,6 +1206,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { // Add website_endpoint as an attribute websiteEndpoint, err := websiteEndpoint(meta.(*conns.AWSClient), d) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLocation, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return err } @@ -1117,6 +1233,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { return BucketListTags(conn, d.Id()) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketTagging, the error should be caught for non-new buckets as follows. + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { return fmt.Errorf("error listing tags for S3 Bucket (%s): %s", d.Id(), err) } @@ -1195,74 +1320,6 @@ func resourceBucketDelete(d *schema.ResourceData, meta interface{}) error { return nil } -func resourceBucketGrantsUpdate(conn *s3.S3, d *schema.ResourceData) error { - bucket := d.Get("bucket").(string) - rawGrants := d.Get("grant").(*schema.Set).List() - - if len(rawGrants) == 0 { - log.Printf("[DEBUG] S3 bucket: %s, Grants fallback to canned ACL", bucket) - if err := resourceBucketACLUpdate(conn, d); err != nil { - return fmt.Errorf("Error fallback to canned ACL, %s", err) - } - } else { - apResponse, err := verify.RetryOnAWSCode("NoSuchBucket", func() (interface{}, error) { - return conn.GetBucketAcl(&s3.GetBucketAclInput{ - Bucket: aws.String(d.Id()), - }) - }) - - if err != nil { - return fmt.Errorf("error getting S3 Bucket (%s) ACL: %s", d.Id(), err) - } - - ap := apResponse.(*s3.GetBucketAclOutput) - log.Printf("[DEBUG] S3 bucket: %s, read ACL grants policy: %+v", d.Id(), ap) - - grants := make([]*s3.Grant, 0, len(rawGrants)) - for _, rawGrant := range rawGrants { - log.Printf("[DEBUG] S3 bucket: %s, put grant: %#v", bucket, rawGrant) - grantMap := rawGrant.(map[string]interface{}) - for _, rawPermission := range grantMap["permissions"].(*schema.Set).List() { - ge := &s3.Grantee{} - if i, ok := grantMap["id"].(string); ok && i != "" { - ge.SetID(i) - } - if t, ok := grantMap["type"].(string); ok && t != "" { - ge.SetType(t) - } - if u, ok := grantMap["uri"].(string); ok && u != "" { - ge.SetURI(u) - } - - g := &s3.Grant{ - Grantee: ge, - Permission: aws.String(rawPermission.(string)), - } - grants = append(grants, g) - } - } - - grantsInput := &s3.PutBucketAclInput{ - Bucket: aws.String(bucket), - AccessControlPolicy: &s3.AccessControlPolicy{ - Grants: grants, - Owner: ap.Owner, - }, - } - - log.Printf("[DEBUG] S3 bucket: %s, put Grants: %#v", bucket, grantsInput) - - _, err = verify.RetryOnAWSCode("NoSuchBucket", func() (interface{}, error) { - return conn.PutBucketAcl(grantsInput) - }) - - if err != nil { - return fmt.Errorf("Error putting S3 Grants: %s", err) - } - } - return nil -} - func websiteEndpoint(client *conns.AWSClient, d *schema.ResourceData) (*S3Website, error) { // If the bucket doesn't have a website configuration, return an empty // endpoint @@ -1344,26 +1401,6 @@ func isOldRegion(region string) bool { return false } -func resourceBucketACLUpdate(conn *s3.S3, d *schema.ResourceData) error { - acl := d.Get("acl").(string) - bucket := d.Get("bucket").(string) - - i := &s3.PutBucketAclInput{ - Bucket: aws.String(bucket), - ACL: aws.String(acl), - } - log.Printf("[DEBUG] S3 put bucket ACL: %#v", i) - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return conn.PutBucketAcl(i) - }) - if err != nil { - return fmt.Errorf("Error putting S3 ACL: %s", err) - } - - return nil -} - func resourceBucketInternalObjectLockConfigurationUpdate(conn *s3.S3, d *schema.ResourceData) error { // S3 Object Lock configuration cannot be deleted, only updated. req := &s3.PutObjectLockConfigurationInput{ @@ -2212,6 +2249,9 @@ func flattenS3ObjectLockConfiguration(conf *s3.ObjectLockConfiguration) []interf } func flattenGrants(ap *s3.GetBucketAclOutput) []interface{} { + if len(ap.Grants) == 0 { + return []interface{}{} + } //if ACL grants contains bucket owner FULL_CONTROL only - it is default "private" acl if len(ap.Grants) == 1 && aws.StringValue(ap.Grants[0].Grantee.ID) == aws.StringValue(ap.Owner.ID) && aws.StringValue(ap.Grants[0].Permission) == s3.PermissionFullControl { diff --git a/internal/service/s3/bucket_acl_test.go b/internal/service/s3/bucket_acl_test.go index 7a706180815..24c8de07562 100644 --- a/internal/service/s3/bucket_acl_test.go +++ b/internal/service/s3/bucket_acl_test.go @@ -435,12 +435,6 @@ func testAccBucketAclBasicConfig(rName, acl string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -456,12 +450,6 @@ data "aws_canonical_user_id" "current" {} resource "aws_s3_bucket" "test" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -499,12 +487,6 @@ data "aws_partition" "current" {} resource "aws_s3_bucket" "test" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 81d18d1414a..85e8d550eb1 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -70,7 +70,7 @@ func TestAccS3Bucket_Basic_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -98,7 +98,7 @@ func TestAccS3Bucket_Basic_emptyString(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -128,7 +128,7 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, { Config: testAccBucketConfig_withUpdatedTags(bucketName), @@ -211,7 +211,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, { Config: testAccBucketConfig_withTags(bucketName), @@ -310,7 +310,7 @@ func TestAccS3Bucket_Tags_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -336,7 +336,7 @@ func TestAccS3Bucket_Basic_namePrefix(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "bucket_prefix"}, + ImportStateVerifyIgnore: []string{"force_destroy", "bucket_prefix"}, }, }, }) @@ -361,7 +361,7 @@ func TestAccS3Bucket_Basic_generatedName(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "bucket_prefix"}, + ImportStateVerifyIgnore: []string{"force_destroy", "bucket_prefix"}, }, }, }) @@ -415,7 +415,7 @@ func TestAccS3Bucket_Manage_objectLock(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -443,7 +443,7 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -1060,12 +1060,6 @@ func testAccBucketConfig_withNoTags(bucketName string) string { resource "aws_s3_bucket" "bucket" { bucket = %[1]q force_destroy = false - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1086,12 +1080,6 @@ resource "aws_s3_bucket" "bucket" { Key2 = "BBB" Key3 = "CCC" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1113,12 +1101,6 @@ resource "aws_s3_bucket" "bucket" { Key4 = "DDD" Key5 = "EEE" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1138,12 +1120,6 @@ resource "aws_s3_bucket" "bucket1" { Name = "tf-test-bucket-1-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test1" { @@ -1159,12 +1135,6 @@ resource "aws_s3_bucket" "bucket2" { Name = "tf-test-bucket-2-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test2" { @@ -1180,12 +1150,6 @@ resource "aws_s3_bucket" "bucket3" { Name = "tf-test-bucket-3-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test3" { @@ -1201,12 +1165,6 @@ resource "aws_s3_bucket" "bucket4" { Name = "tf-test-bucket-4-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test4" { @@ -1222,12 +1180,6 @@ resource "aws_s3_bucket" "bucket5" { Name = "tf-test-bucket-5-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test5" { @@ -1243,12 +1195,6 @@ resource "aws_s3_bucket" "bucket6" { Name = "tf-test-bucket-6-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test6" { @@ -1262,12 +1208,6 @@ func testAccBucketDestroyedConfig(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "bucket" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1292,8 +1232,7 @@ resource "aws_s3_bucket" "test" { func testAccBucketConfig_objectLockEnabledWithVersioning(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { - bucket = "%s" - acl = "private" + bucket = %[1]q force_destroy = true object_lock_configuration { @@ -1301,6 +1240,11 @@ resource "aws_s3_bucket" "test" { } } +resource "aws_s3_bucket_acl" "test" { + bucket = aws_s3_bucket.test.id + acl = "private" +} + resource "aws_s3_bucket_versioning" "test" { bucket = aws_s3_bucket.test.id versioning_configuration { @@ -1315,12 +1259,6 @@ func testAccBucketConfig_forceDestroy(bucketName string) string { resource "aws_s3_bucket" "bucket" { bucket = "%s" force_destroy = true - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1339,12 +1277,6 @@ resource "aws_s3_bucket" "bucket" { object_lock_configuration { object_lock_enabled = "Enabled" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { diff --git a/website/docs/guides/version-4-upgrade.html.md b/website/docs/guides/version-4-upgrade.html.md index a4c0fe02744..e7bdba9cb89 100644 --- a/website/docs/guides/version-4-upgrade.html.md +++ b/website/docs/guides/version-4-upgrade.html.md @@ -743,6 +743,60 @@ The resources that were imported are shown above. These resources are now in your Terraform state and will henceforth be managed by Terraform. ``` +### `acl` Argument deprecation + +Switch your Terraform configuration to the `aws_s3_bucket_acl` resource instead. + +For example, given this previous configuration: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... + acl = "private" +} +``` + +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 "acl": its value will be decided automatically based on the result of applying this configuration. +``` + +Since the `acl` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_acl` +resource and remove any reference to `acl` in the `aws_s3_bucket` resource: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} + +resource "aws_s3_bucket_acl" "example" { + bucket = aws_s3_bucket.example.id + acl = "private" +} +``` + +It is then recommended running `terraform import` on each new resource to prevent data loss, e.g. + +```shell +$ terraform import aws_s3_bucket_acl.example example,private +aws_s3_bucket_acl.example: Importing from ID "example,private"... +aws_s3_bucket_acl.example: Import prepared! + Prepared aws_s3_bucket_acl for import +aws_s3_bucket_acl.example: Refreshing state... [id=example,private] + +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. +``` + ### `cors_rule` Argument deprecation Switch your Terraform configuration to the `aws_s3_bucket_cors_configuration` resource instead. @@ -810,6 +864,98 @@ The resources that were imported are shown above. These resources are now in your Terraform state and will henceforth be managed by Terraform. ``` +### `grant` Argument deprecation + +Switch your Terraform configuration to the `aws_s3_bucket_acl` resource instead. + +For example, given this previous configuration: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... + grant { + id = data.aws_canonical_user_id.current_user.id + type = "CanonicalUser" + permissions = ["FULL_CONTROL"] + } + grant { + type = "Group" + permissions = ["READ_ACP", "WRITE"] + uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" + } +} +``` + +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 "grant": its value will be decided automatically based on the result of applying this configuration. +``` + +Since the `grant` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_acl` +resource and remove any reference to `grant` in the `aws_s3_bucket` resource: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} + +resource "aws_s3_bucket_acl" "example" { + bucket = aws_s3_bucket.example.id + + access_control_policy { + grant { + grantee { + id = data.aws_canonical_user_id.current_user.id + type = "CanonicalUser" + } + permission = "FULL_CONTROL" + } + + grant { + grantee { + type = "Group" + uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" + } + permission = "READ_ACP" + } + + grant { + grantee { + type = "Group" + uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" + } + permission = "WRITE" + } + + owner { + id = data.aws_canonical_user_id.current_user.id + } + } +} +``` + +It is then recommended running `terraform import` on each new resource to prevent data loss, e.g. + +```shell +$ terraform import aws_s3_bucket_acl.example example +aws_s3_bucket_acl.example: Importing from ID "example"... +aws_s3_bucket_acl.example: Import prepared! + Prepared aws_s3_bucket_acl for import +aws_s3_bucket_acl.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. +``` + ### `lifecycle_rule` Argument deprecation Switch your Terraform configuration to the `aws_s3_bucket_lifecycle_configuration` resource instead. diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 1806a8c672b..edd8fea4d64 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -69,25 +69,8 @@ See the [`aws_s3_bucket_server_side_encryption_configuration` resource](s3_bucke ### Using ACL policy grants -```terraform -data "aws_canonical_user_id" "current_user" {} - -resource "aws_s3_bucket" "bucket" { - bucket = "mybucket" - - grant { - id = data.aws_canonical_user_id.current_user.id - type = "CanonicalUser" - permissions = ["FULL_CONTROL"] - } - - grant { - type = "Group" - permissions = ["READ_ACP", "WRITE"] - uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" - } -} -``` +The `acl` and `grant` arguments are read-only as of version 4.0 of the Terraform AWS Provider. +See the [`aws_s3_bucket_acl` resource](s3_bucket_acl.html.markdown) for configuration details. ## Argument Reference @@ -95,19 +78,10 @@ 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). -* `acl` - (Optional) 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`. -* `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. * `object_lock_configuration` - (Optional) A configuration of [S3 object locking](https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock.html) (documented below) -The `grant` object supports the following: - -* `id` - (optional) Canonical user id to grant for. Used only when `type` is `CanonicalUser`. -* `type` - (required) - Type of grantee to apply for. Valid values are `CanonicalUser` and `Group`. `AmazonCustomerByEmail` is not supported. -* `permissions` - (required) List of permissions to apply for grantee. Valid values are `READ`, `WRITE`, `READ_ACP`, `WRITE_ACP`, `FULL_CONTROL`. -* `uri` - (optional) Uri address to grant for. Used only when `type` is `Group`. - The `object_lock_configuration` object supports the following: * `object_lock_enabled` - (Required) Indicates whether this bucket has an Object Lock configuration enabled. Valid value is `Enabled`. @@ -122,6 +96,7 @@ 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`. +* `acl` - The [canned ACL](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl) applied to the bucket. * `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. @@ -131,6 +106,11 @@ In addition to all arguments above, the following attributes are exported: * `allowed_origins` - Set of origins customers are able to access the bucket from. * `expose_headers` - Set of headers in the response that customers are able to access from their applications. * `max_age_seconds` The time in seconds that browser can cache the response for a preflight request. +* `grant` - The set of [ACL policy grants](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#sample-acl). + * `id` - Canonical user id of the grantee. + * `type` - Type of grantee. + * `permissions` - List of permissions given to the grantee. + * `uri` - URI of the grantee group. * `hosted_zone_id` - The [Route 53 Hosted Zone ID](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_website_region_endpoints) for this bucket's region. * `lifecycle_rule` - A configuration of [object lifecycle management](http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html). * `id` - Unique identifier for the rule.