From 2dd5d9bb7680745857a3926e112da013fcbab434 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 9 Jul 2020 16:16:33 -0400 Subject: [PATCH] resource/aws_s3_bucket: Remove automatic aws_s3_bucket_policy import Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/394 Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/9001 Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/9508 Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12805 Output from acceptance testing: ``` --- PASS: TestAccAWSS3Bucket_acceleration (70.53s) --- PASS: TestAccAWSS3Bucket_AclToGrant (64.37s) --- PASS: TestAccAWSS3Bucket_basic (37.90s) --- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (39.08s) --- PASS: TestAccAWSS3Bucket_Cors_Delete (32.28s) --- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (39.25s) --- PASS: TestAccAWSS3Bucket_Cors_Update (68.80s) --- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (67.23s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (37.19s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (44.32s) --- PASS: TestAccAWSS3Bucket_forceDestroy (37.21s) --- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (38.50s) --- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (37.77s) --- PASS: TestAccAWSS3Bucket_generatedName (38.80s) --- PASS: TestAccAWSS3Bucket_GrantToAcl (60.31s) --- PASS: TestAccAWSS3Bucket_LifecycleBasic (89.67s) --- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (67.52s) --- PASS: TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock (30.08s) --- PASS: TestAccAWSS3Bucket_Logging (56.73s) --- PASS: TestAccAWSS3Bucket_namePrefix (40.92s) --- PASS: TestAccAWSS3Bucket_objectLock (68.34s) --- PASS: TestAccAWSS3Bucket_Policy (97.07s) --- PASS: TestAccAWSS3Bucket_region (34.45s) --- PASS: TestAccAWSS3Bucket_Replication (159.22s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (94.18s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AddAccessControlTranslation (95.79s) --- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (28.62s) --- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (167.50s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (55.52s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (58.02s) --- PASS: TestAccAWSS3Bucket_RequestPayer (67.28s) --- PASS: TestAccAWSS3Bucket_shouldFailNotFound (19.65s) --- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (119.32s) --- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (171.42s) --- PASS: TestAccAWSS3Bucket_UpdateAcl (65.51s) --- PASS: TestAccAWSS3Bucket_UpdateGrant (92.38s) --- PASS: TestAccAWSS3Bucket_Versioning (95.55s) --- PASS: TestAccAWSS3Bucket_Website_Simple (95.12s) --- PASS: TestAccAWSS3Bucket_WebsiteRedirect (91.21s) --- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (65.48s) ``` --- aws/import_aws_s3_bucket.go | 40 ------------------- aws/resource_aws_s3_bucket.go | 2 +- aws/resource_aws_s3_bucket_test.go | 35 +++++++--------- website/docs/guides/version-3-upgrade.html.md | 7 ++++ website/docs/r/s3_bucket.html.markdown | 2 + 5 files changed, 24 insertions(+), 62 deletions(-) delete mode 100644 aws/import_aws_s3_bucket.go diff --git a/aws/import_aws_s3_bucket.go b/aws/import_aws_s3_bucket.go deleted file mode 100644 index 987bb180d14..00000000000 --- a/aws/import_aws_s3_bucket.go +++ /dev/null @@ -1,40 +0,0 @@ -package aws - -import ( - "fmt" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/service/s3" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" -) - -func resourceAwsS3BucketImportState( - d *schema.ResourceData, - meta interface{}) ([]*schema.ResourceData, error) { - - results := make([]*schema.ResourceData, 1) - results[0] = d - - conn := meta.(*AWSClient).s3conn - pol, err := conn.GetBucketPolicy(&s3.GetBucketPolicyInput{ - Bucket: aws.String(d.Id()), - }) - if err != nil { - if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "NoSuchBucketPolicy" { - // Bucket without policy - return results, nil - } - return nil, fmt.Errorf("Error importing AWS S3 bucket policy: %s", err) - } - - policy := resourceAwsS3BucketPolicy() - pData := policy.Data(nil) - pData.SetId(d.Id()) - pData.SetType("aws_s3_bucket_policy") - pData.Set("bucket", d.Id()) - pData.Set("policy", pol.Policy) - results = append(results, pData) - - return results, nil -} diff --git a/aws/resource_aws_s3_bucket.go b/aws/resource_aws_s3_bucket.go index 4ffe11aa591..472825a664b 100644 --- a/aws/resource_aws_s3_bucket.go +++ b/aws/resource_aws_s3_bucket.go @@ -32,7 +32,7 @@ func resourceAwsS3Bucket() *schema.Resource { Update: resourceAwsS3BucketUpdate, Delete: resourceAwsS3BucketDelete, Importer: &schema.ResourceImporter{ - State: resourceAwsS3BucketImportState, + State: schema.ImportStatePassthrough, }, Schema: map[string]*schema.Schema{ diff --git a/aws/resource_aws_s3_bucket_test.go b/aws/resource_aws_s3_bucket_test.go index 1b460f672c1..824ebe096df 100644 --- a/aws/resource_aws_s3_bucket_test.go +++ b/aws/resource_aws_s3_bucket_test.go @@ -531,24 +531,6 @@ func TestAccAWSS3Bucket_Policy(t *testing.T) { partition := testAccGetPartition() resourceName := "aws_s3_bucket.bucket" - checkFn := func(s []*terraform.InstanceState) error { - // Expect 2: bucket + policy - if len(s) != 2 { - return fmt.Errorf("expected 2 states: %#v", s) - } - bucketState, policyState := s[0], s[1] - - if bucketState.ID != bucketName { - return fmt.Errorf("expected bucket of ID %s, %s received", bucketName, bucketState.ID) - } - - if policyState.ID != bucketName { - return fmt.Errorf("expected policy of ID %s, %s received", bucketName, bucketState.ID) - } - - return nil - } - resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -562,9 +544,20 @@ func TestAccAWSS3Bucket_Policy(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateCheck: checkFn, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "acl", + "force_destroy", + "grant", + // NOTE: Prior to Terraform AWS Provider 3.0, this attribute did not import correctly either. + // The Read function does not require GetBucketPolicy, if the argument is not configured. + // Rather than introduce that breaking change as well with 3.0, instead we leave the + // current Read behavior and note this will be deprecated in a later 3.x release along + // with other inline policy attributes across the provider. + "policy", + }, }, { Config: testAccAWSS3BucketConfig_Basic(bucketName), diff --git a/website/docs/guides/version-3-upgrade.html.md b/website/docs/guides/version-3-upgrade.html.md index c0a4bf9b1c2..03f73473542 100644 --- a/website/docs/guides/version-3-upgrade.html.md +++ b/website/docs/guides/version-3-upgrade.html.md @@ -22,6 +22,7 @@ Upgrade topics: - [Data Source: aws_availability_zones](#data-source-aws_availability_zones) - [Data Source: aws_lambda_invocation](#data-source-aws_lambda_invocation) - [Resource: aws_emr_cluster](#resource-aws_emr_cluster) +- [Resource: aws_s3_bucket](#resource-aws_s3_bucket) @@ -287,3 +288,9 @@ resource "aws_lb_listener_rule" "example" { } } ``` + +## Resource: aws_s3_bucket + +### Removal of Automatic aws_s3_bucket_policy Import + +Previously when importing the `aws_s3_bucket` resource with the [`terraform import` command](/docs/commands/import.html), the Terraform AWS Provider would automatically attempt to import an associated `aws_s3_bucket_policy` resource as well. This automatic resource import has been removed. Use the [`aws_s3_bucket_policy` resource import](/docs/providers/aws/r/s3_bucket_policy.html#import) to import that resource separately. diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 39d9c28dfbc..95bb511572e 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -533,3 +533,5 @@ S3 bucket can be imported using the `bucket`, e.g. ``` $ terraform import aws_s3_bucket.bucket bucket-name ``` + +The `policy` argument is not imported and will be deprecated in a future version 3.x of the Terraform AWS Provider for removal in version 4.0. Use the [`aws_s3_bucket_policy` resource](/docs/providers/aws/r/s3_bucket_policy.html) to manage the S3 Bucket Policy instead.