From 9336898af4f2e46484cfbb6ba97a200718ce4e56 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 28 Jan 2022 19:24:51 -0500 Subject: [PATCH 01/12] feat: deprecate 'website', 'website_endpoint', and 'website_domain' arguments --- internal/service/cloudformation/stack_test.go | 9 + internal/service/route53/record_test.go | 3 + internal/service/s3/bucket.go | 281 +++++----------- .../service/s3/bucket_data_source_test.go | 11 +- internal/service/s3/bucket_test.go | 312 ------------------ internal/service/s3/errors.go | 1 + website/docs/r/s3_bucket.html.markdown | 39 +-- 7 files changed, 112 insertions(+), 544 deletions(-) diff --git a/internal/service/cloudformation/stack_test.go b/internal/service/cloudformation/stack_test.go index fab244b7513..e93dbc8f1bd 100644 --- a/internal/service/cloudformation/stack_test.go +++ b/internal/service/cloudformation/stack_test.go @@ -299,6 +299,9 @@ func TestAccCloudFormationStack_withParams(t *testing.T) { // Regression for https://github.com/hashicorp/terraform/issues/4534 func TestAccCloudFormationStack_WithURL_withParams(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") + var stack cloudformation.Stack rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudformation_stack.test" @@ -332,6 +335,9 @@ func TestAccCloudFormationStack_WithURL_withParams(t *testing.T) { } func TestAccCloudFormationStack_WithURLWithParams_withYAML(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") + var stack cloudformation.Stack rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudformation_stack.test" @@ -360,6 +366,9 @@ func TestAccCloudFormationStack_WithURLWithParams_withYAML(t *testing.T) { // Test for https://github.com/hashicorp/terraform/issues/5653 func TestAccCloudFormationStack_WithURLWithParams_noUpdate(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") + var stack cloudformation.Stack rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudformation_stack.test" diff --git a/internal/service/route53/record_test.go b/internal/service/route53/record_test.go index 5fa9ccd1bc9..62f486f7a46 100644 --- a/internal/service/route53/record_test.go +++ b/internal/service/route53/record_test.go @@ -571,6 +571,9 @@ func TestAccRoute53Record_Alias_elb(t *testing.T) { } func TestAccRoute53Record_Alias_s3(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") + var record1 route53.ResourceRecordSet rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_route53_record.alias" diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 1703b68ddd6..7308497210b 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -162,39 +162,33 @@ func ResourceBucket() *schema.Resource { }, "website": { - Type: schema.TypeList, - Optional: true, - MaxItems: 1, + Type: schema.TypeList, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "index_document": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", }, "error_document": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", }, "redirect_all_requests_to": { - Type: schema.TypeString, - ConflictsWith: []string{ - "website.0.index_document", - "website.0.error_document", - "website.0.routing_rules", - }, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", }, "routing_rules": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringIsJSON, - StateFunc: func(v interface{}) string { - json, _ := structure.NormalizeJsonString(v) - return json - }, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", }, }, }, @@ -211,14 +205,14 @@ func ResourceBucket() *schema.Resource { Computed: true, }, "website_endpoint": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", }, "website_domain": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", }, "versioning": { @@ -770,12 +764,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("website") { - if err := resourceBucketWebsiteUpdate(conn, d); err != nil { - return err - } - } - if d.HasChange("versioning") { v := d.Get("versioning").([]interface{}) @@ -999,64 +987,18 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) - if err != nil && !tfawserr.ErrMessageContains(err, "NotImplemented", "") && !tfawserr.ErrMessageContains(err, "NoSuchWebsiteConfiguration", "") { - return fmt.Errorf("error getting S3 Bucket website configuration: %s", err) + if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeNotImplemented, ErrCodeNoSuchWebsiteConfiguration) { + return fmt.Errorf("error getting S3 Bucket website configuration: %w", err) } - websites := make([]map[string]interface{}, 0, 1) if ws, ok := wsResponse.(*s3.GetBucketWebsiteOutput); ok { - w := make(map[string]interface{}) - - if v := ws.IndexDocument; v != nil { - w["index_document"] = aws.StringValue(v.Suffix) - } - - if v := ws.ErrorDocument; v != nil { - w["error_document"] = aws.StringValue(v.Key) - } - - if v := ws.RedirectAllRequestsTo; v != nil { - if v.Protocol == nil { - w["redirect_all_requests_to"] = aws.StringValue(v.HostName) - } else { - var host string - var path string - var query string - parsedHostName, err := url.Parse(aws.StringValue(v.HostName)) - if err == nil { - host = parsedHostName.Host - path = parsedHostName.Path - query = parsedHostName.RawQuery - } else { - host = aws.StringValue(v.HostName) - path = "" - } - - w["redirect_all_requests_to"] = (&url.URL{ - Host: host, - Path: path, - Scheme: aws.StringValue(v.Protocol), - RawQuery: query, - }).String() - } + website, err := flattenBucketWebsite(ws) + if err != nil { + return err } - - if v := ws.RoutingRules; v != nil { - rr, err := normalizeRoutingRules(v) - if err != nil { - return fmt.Errorf("Error while marshaling routing rules: %s", err) - } - w["routing_rules"] = rr + if err := d.Set("website", website); err != nil { + return fmt.Errorf("error setting website: %w", err) } - - // We have special handling for the website configuration, - // so only add the configuration if there is any - if len(w) > 0 { - websites = append(websites, w) - } - } - if err := d.Set("website", websites); err != nil { - return fmt.Errorf("error setting website: %s", err) } // Read the versioning configuration @@ -1641,115 +1583,6 @@ func resourceBucketCorsUpdate(conn *s3.S3, d *schema.ResourceData) error { return nil } -func resourceBucketWebsiteUpdate(conn *s3.S3, d *schema.ResourceData) error { - ws := d.Get("website").([]interface{}) - - if len(ws) == 0 { - return resourceBucketWebsiteDelete(conn, d) - } - - var w map[string]interface{} - if ws[0] != nil { - w = ws[0].(map[string]interface{}) - } else { - w = make(map[string]interface{}) - } - return resourceBucketWebsitePut(conn, d, w) -} - -func resourceBucketWebsitePut(conn *s3.S3, d *schema.ResourceData, website map[string]interface{}) error { - bucket := d.Get("bucket").(string) - - var indexDocument, errorDocument, redirectAllRequestsTo, routingRules string - if v, ok := website["index_document"]; ok { - indexDocument = v.(string) - } - if v, ok := website["error_document"]; ok { - errorDocument = v.(string) - } - if v, ok := website["redirect_all_requests_to"]; ok { - redirectAllRequestsTo = v.(string) - } - if v, ok := website["routing_rules"]; ok { - routingRules = v.(string) - } - - if indexDocument == "" && redirectAllRequestsTo == "" { - return fmt.Errorf("Must specify either index_document or redirect_all_requests_to.") - } - - websiteConfiguration := &s3.WebsiteConfiguration{} - - if indexDocument != "" { - websiteConfiguration.IndexDocument = &s3.IndexDocument{Suffix: aws.String(indexDocument)} - } - - if errorDocument != "" { - websiteConfiguration.ErrorDocument = &s3.ErrorDocument{Key: aws.String(errorDocument)} - } - - if redirectAllRequestsTo != "" { - redirect, err := url.Parse(redirectAllRequestsTo) - if err == nil && redirect.Scheme != "" { - var redirectHostBuf bytes.Buffer - redirectHostBuf.WriteString(redirect.Host) - if redirect.Path != "" { - redirectHostBuf.WriteString(redirect.Path) - } - if redirect.RawQuery != "" { - redirectHostBuf.WriteString("?") - redirectHostBuf.WriteString(redirect.RawQuery) - } - websiteConfiguration.RedirectAllRequestsTo = &s3.RedirectAllRequestsTo{HostName: aws.String(redirectHostBuf.String()), Protocol: aws.String(redirect.Scheme)} - } else { - websiteConfiguration.RedirectAllRequestsTo = &s3.RedirectAllRequestsTo{HostName: aws.String(redirectAllRequestsTo)} - } - } - - if routingRules != "" { - var unmarshaledRules []*s3.RoutingRule - if err := json.Unmarshal([]byte(routingRules), &unmarshaledRules); err != nil { - return err - } - websiteConfiguration.RoutingRules = unmarshaledRules - } - - putInput := &s3.PutBucketWebsiteInput{ - Bucket: aws.String(bucket), - WebsiteConfiguration: websiteConfiguration, - } - - log.Printf("[DEBUG] S3 put bucket website: %#v", putInput) - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return conn.PutBucketWebsite(putInput) - }) - if err != nil { - return fmt.Errorf("Error putting S3 website: %s", err) - } - - return nil -} - -func resourceBucketWebsiteDelete(conn *s3.S3, d *schema.ResourceData) error { - bucket := d.Get("bucket").(string) - deleteInput := &s3.DeleteBucketWebsiteInput{Bucket: aws.String(bucket)} - - log.Printf("[DEBUG] S3 delete bucket website: %#v", deleteInput) - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return conn.DeleteBucketWebsite(deleteInput) - }) - if err != nil { - return fmt.Errorf("Error deleting S3 website: %s", err) - } - - d.Set("website_endpoint", "") - d.Set("website_domain", "") - - 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 @@ -2391,6 +2224,64 @@ func flattenServerSideEncryptionConfiguration(c *s3.ServerSideEncryptionConfigur return encryptionConfiguration } +func flattenBucketWebsite(ws *s3.GetBucketWebsiteOutput) ([]interface{}, error) { + if ws == nil { + return []interface{}{}, nil + } + + m := make(map[string]interface{}) + + if v := ws.IndexDocument; v != nil { + m["index_document"] = aws.StringValue(v.Suffix) + } + + if v := ws.ErrorDocument; v != nil { + m["error_document"] = aws.StringValue(v.Key) + } + + if v := ws.RedirectAllRequestsTo; v != nil { + if v.Protocol == nil { + m["redirect_all_requests_to"] = aws.StringValue(v.HostName) + } else { + var host string + var path string + var query string + parsedHostName, err := url.Parse(aws.StringValue(v.HostName)) + if err == nil { + host = parsedHostName.Host + path = parsedHostName.Path + query = parsedHostName.RawQuery + } else { + host = aws.StringValue(v.HostName) + path = "" + } + + m["redirect_all_requests_to"] = (&url.URL{ + Host: host, + Path: path, + Scheme: aws.StringValue(v.Protocol), + RawQuery: query, + }).String() + } + } + + if v := ws.RoutingRules; v != nil { + rr, err := normalizeRoutingRules(v) + if err != nil { + return nil, fmt.Errorf("error while marshaling routing rules: %w", err) + } + m["routing_rules"] = rr + } + + // We have special handling for the website configuration, + // so only return the configuration if there is any + if len(m) == 0 { + return []interface{}{}, nil + } + + return []interface{}{m}, nil +} + func flattenBucketReplicationConfiguration(r *s3.ReplicationConfiguration) []map[string]interface{} { replication_configuration := make([]map[string]interface{}, 0, 1) diff --git a/internal/service/s3/bucket_data_source_test.go b/internal/service/s3/bucket_data_source_test.go index 3a45fb13649..7cb982f4e87 100644 --- a/internal/service/s3/bucket_data_source_test.go +++ b/internal/service/s3/bucket_data_source_test.go @@ -38,8 +38,10 @@ func TestAccS3BucketDataSource_basic(t *testing.T) { } func TestAccS3BucketDataSource_website(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") + bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - region := acctest.Region() resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -50,8 +52,9 @@ func TestAccS3BucketDataSource_website(t *testing.T) { Config: testAccBucketWebsiteDataSourceConfig(bucketName), Check: resource.ComposeTestCheckFunc( testAccCheckBucketExists("data.aws_s3_bucket.bucket"), - testAccCheckBucketWebsite("data.aws_s3_bucket.bucket", "index.html", "error.html", "", ""), - testAccCheckS3BucketWebsiteEndpoint("data.aws_s3_bucket.bucket", "website_endpoint", bucketName, region), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "bucket", "aws_s3_bucket.bucket", "id"), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "website_domain", "aws_s3_bucket.bucket", "website_domain"), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "website_endpoint", "aws_s3_bucket.bucket", "website_endpoint"), ), }, }, @@ -75,13 +78,11 @@ func testAccBucketWebsiteDataSourceConfig(bucketName string) string { resource "aws_s3_bucket" "bucket" { bucket = %[1]q acl = "public-read" - website { index_document = "index.html" error_document = "error.html" } } - data "aws_s3_bucket" "bucket" { bucket = aws_s3_bucket.bucket.id } diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 2add7d5766f..c0b96bf01d9 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -649,148 +649,6 @@ func TestAccS3Bucket_Security_grantToACL(t *testing.T) { }) } -func TestAccS3Bucket_Web_simple(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - region := acctest.Region() - resourceName := "aws_s3_bucket.bucket" - - 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: testAccBucketWebsiteConfig(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "index.html", "", "", ""), - testAccCheckS3BucketWebsiteEndpoint(resourceName, "website_endpoint", bucketName, region), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "grant"}, - }, - { - Config: testAccBucketWebsiteWithErrorConfig(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "index.html", "error.html", "", ""), - testAccCheckS3BucketWebsiteEndpoint(resourceName, "website_endpoint", bucketName, region), - ), - }, - { - Config: testAccBucketConfig_Basic(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "", "", "", ""), - resource.TestCheckResourceAttr(resourceName, "website_endpoint", ""), - ), - }, - }, - }) -} - -func TestAccS3Bucket_Web_redirect(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - region := acctest.Region() - resourceName := "aws_s3_bucket.bucket" - - 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: testAccBucketWebsiteWithRedirectConfig(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "", "", "", "hashicorp.com?my=query"), - testAccCheckS3BucketWebsiteEndpoint(resourceName, "website_endpoint", bucketName, region), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "grant"}, - }, - { - Config: testAccBucketWebsiteWithHTTPSRedirectConfig(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "", "", "https", "hashicorp.com?my=query"), - testAccCheckS3BucketWebsiteEndpoint(resourceName, "website_endpoint", bucketName, region), - ), - }, - { - Config: testAccBucketConfig_Basic(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "", "", "", ""), - resource.TestCheckResourceAttr(resourceName, "website_endpoint", ""), - ), - }, - }, - }) -} - -func TestAccS3Bucket_Web_routingRules(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - region := acctest.Region() - resourceName := "aws_s3_bucket.bucket" - - 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: testAccBucketWebsiteWithRoutingRulesConfig(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite( - resourceName, "index.html", "error.html", "", ""), - testAccCheckBucketWebsiteRoutingRules( - resourceName, - []*s3.RoutingRule{ - { - Condition: &s3.Condition{ - KeyPrefixEquals: aws.String("docs/"), - }, - Redirect: &s3.Redirect{ - ReplaceKeyPrefixWith: aws.String("documents/"), - }, - }, - }, - ), - testAccCheckS3BucketWebsiteEndpoint(resourceName, "website_endpoint", bucketName, region), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "grant"}, - }, - { - Config: testAccBucketConfig_Basic(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketWebsite(resourceName, "", "", "", ""), - testAccCheckBucketWebsiteRoutingRules(resourceName, nil), - resource.TestCheckResourceAttr(resourceName, "website_endpoint", ""), - ), - }, - }, - }) -} - func TestAccS3Bucket_Security_enableDefaultEncryptionWhenTypical(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.arbitrary" @@ -3158,86 +3016,6 @@ func testAccCheckBucketPolicy(n string, policy string) resource.TestCheckFunc { } } -func testAccCheckBucketWebsite(n string, indexDoc string, errorDoc string, redirectProtocol string, redirectTo string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs := s.RootModule().Resources[n] - conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn - - out, err := conn.GetBucketWebsite(&s3.GetBucketWebsiteInput{ - Bucket: aws.String(rs.Primary.ID), - }) - - if err != nil { - if indexDoc == "" { - // If we want to assert that the website is not there, than - // this error is expected - return nil - } else { - return fmt.Errorf("S3BucketWebsite error: %v", err) - } - } - - if v := out.IndexDocument; v == nil { - if indexDoc != "" { - return fmt.Errorf("bad index doc, found nil, expected: %s", indexDoc) - } - } else { - if *v.Suffix != indexDoc { - return fmt.Errorf("bad index doc, expected: %s, got %#v", indexDoc, out.IndexDocument) - } - } - - if v := out.ErrorDocument; v == nil { - if errorDoc != "" { - return fmt.Errorf("bad error doc, found nil, expected: %s", errorDoc) - } - } else { - if *v.Key != errorDoc { - return fmt.Errorf("bad error doc, expected: %s, got %#v", errorDoc, out.ErrorDocument) - } - } - - if v := out.RedirectAllRequestsTo; v == nil { - if redirectTo != "" { - return fmt.Errorf("bad redirect to, found nil, expected: %s", redirectTo) - } - } else { - if *v.HostName != redirectTo { - return fmt.Errorf("bad redirect to, expected: %s, got %#v", redirectTo, out.RedirectAllRequestsTo) - } - if redirectProtocol != "" && v.Protocol != nil && *v.Protocol != redirectProtocol { - return fmt.Errorf("bad redirect protocol to, expected: %s, got %#v", redirectProtocol, out.RedirectAllRequestsTo) - } - } - - return nil - } -} - -func testAccCheckBucketWebsiteRoutingRules(n string, routingRules []*s3.RoutingRule) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs := s.RootModule().Resources[n] - conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn - - out, err := conn.GetBucketWebsite(&s3.GetBucketWebsiteInput{ - Bucket: aws.String(rs.Primary.ID), - }) - - if err != nil { - if routingRules == nil { - return nil - } - return fmt.Errorf("GetBucketWebsite error: %v", err) - } - - if !reflect.DeepEqual(out.RoutingRules, routingRules) { - return fmt.Errorf("bad routing rule, expected: %v, got %v", routingRules, out.RoutingRules) - } - - return nil - } -} - func testAccCheckBucketCors(n string, corsRules []*s3.CORSRule) resource.TestCheckFunc { return func(s *terraform.State) error { rs := s.RootModule().Resources[n] @@ -3405,15 +3183,6 @@ func testAccBucketRegionalDomainName(bucket, region string) string { return regionalEndpoint } -func testAccCheckS3BucketWebsiteEndpoint(resourceName string, attributeName string, bucketName string, region string) resource.TestCheckFunc { - return func(s *terraform.State) error { - website := tfs3.WebsiteEndpoint(acctest.Provider.Meta().(*conns.AWSClient), bucketName, region) - expectedValue := website.Endpoint - - return resource.TestCheckResourceAttr(resourceName, attributeName, expectedValue)(s) - } -} - func testAccCheckBucketUpdateTags(n string, oldTags, newTags map[string]string) resource.TestCheckFunc { return func(s *terraform.State) error { rs := s.RootModule().Resources[n] @@ -3580,87 +3349,6 @@ resource "aws_s3_bucket" "bucket6" { `, randInt) } -func testAccBucketWebsiteConfig(bucketName string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - acl = "public-read" - - website { - index_document = "index.html" - } -} -`, bucketName) -} - -func testAccBucketWebsiteWithErrorConfig(bucketName string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - acl = "public-read" - - website { - index_document = "index.html" - error_document = "error.html" - } -} -`, bucketName) -} - -func testAccBucketWebsiteWithRedirectConfig(bucketName string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - acl = "public-read" - - website { - redirect_all_requests_to = "hashicorp.com?my=query" - } -} -`, bucketName) -} - -func testAccBucketWebsiteWithHTTPSRedirectConfig(bucketName string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - acl = "public-read" - - website { - redirect_all_requests_to = "https://hashicorp.com?my=query" - } -} -`, bucketName) -} - -func testAccBucketWebsiteWithRoutingRulesConfig(bucketName string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - acl = "public-read" - - website { - index_document = "index.html" - error_document = "error.html" - - routing_rules = < **NOTE:** You cannot use `acceleration_status` in `cn-north-1` or `us-gov-west-1` -The `website` object supports the following: - -* `index_document` - (Required, unless using `redirect_all_requests_to`) Amazon S3 returns this index document when requests are made to the root domain or any of the subfolders. -* `error_document` - (Optional) An absolute path to the document to return in case of a 4XX error. -* `redirect_all_requests_to` - (Optional) A hostname to redirect all website requests for this bucket to. Hostname can optionally be prefixed with a protocol (`http://` or `https://`) to use when redirecting requests. The default is the protocol that is used in the original request. -* `routing_rules` - (Optional) A json array containing [routing rules](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-websiteconfiguration-routingrules.html) -describing redirect behavior and when redirects are applied. - The `CORS` object supports the following: * `allowed_headers` (Optional) Specifies which headers are allowed. @@ -561,6 +531,11 @@ In addition to all arguments above, the following attributes are exported: * `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. * `region` - The AWS region this bucket resides in. * `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). +* `website` - The website configuration, if configured. + * `error_document` - The name of the error document for the website. + * `index_document` - The name of the index document for the website. + * `redirect_all_requests_to` - The redirect behavior for every request to this bucket's website endpoint. + * `routing_rules` - (Optional) The rules that define when a redirect is applied and the redirect behavior. * `website_endpoint` - The website endpoint, if the bucket is configured with a website. If not, this will be an empty string. * `website_domain` - The domain of the website endpoint, if the bucket is configured with a website. If not, this will be an empty string. This is used to create Route 53 alias records. From 762c1f1424206fe7b3d095d383b36a8faa73faef Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 28 Jan 2022 19:24:59 -0500 Subject: [PATCH 02/12] Update CHANGELOG for #22614 --- .changelog/22614.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22614.txt diff --git a/.changelog/22614.txt b/.changelog/22614.txt new file mode 100644 index 00000000000..ed649ac3305 --- /dev/null +++ b/.changelog/22614.txt @@ -0,0 +1,3 @@ +```release-note:note +resource/aws_s3_bucket: The `website`, `website_domain`, and `website_endpoints` arguments have been deprecated and are now read-only. Use the `aws_s3_bucket_website_configuration` resource instead. +``` \ No newline at end of file From 5d33a9e90dd98c166ddff8fb6f64c374e917228d Mon Sep 17 00:00:00 2001 From: angie pinilla Date: Wed, 2 Feb 2022 12:47:26 -0500 Subject: [PATCH 03/12] Update .changelog/22614.txt --- .changelog/22614.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.changelog/22614.txt b/.changelog/22614.txt index ed649ac3305..012de847f5b 100644 --- a/.changelog/22614.txt +++ b/.changelog/22614.txt @@ -1,3 +1,2 @@ -```release-note:note -resource/aws_s3_bucket: The `website`, `website_domain`, and `website_endpoints` arguments have been deprecated and are now read-only. Use the `aws_s3_bucket_website_configuration` resource instead. -``` \ No newline at end of file +```release-note:breaking-change +resource/aws_s3_bucket: The `website`, `website_domain`, and `website_endpoint` arguments have been deprecated and are now read-only. Use the `aws_s3_bucket_website_configuration` resource instead. \ No newline at end of file From c26a9c6384940b740a9a88f36f084990bb49c2de Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Wed, 2 Feb 2022 23:23:48 -0500 Subject: [PATCH 04/12] cloudformation/stack: migrate tests to use aws_s3_bucket_website_configuration resource --- internal/service/cloudformation/stack_test.go | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/internal/service/cloudformation/stack_test.go b/internal/service/cloudformation/stack_test.go index e93dbc8f1bd..b74b2e0d06a 100644 --- a/internal/service/cloudformation/stack_test.go +++ b/internal/service/cloudformation/stack_test.go @@ -299,9 +299,6 @@ func TestAccCloudFormationStack_withParams(t *testing.T) { // Regression for https://github.com/hashicorp/terraform/issues/4534 func TestAccCloudFormationStack_WithURL_withParams(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") - var stack cloudformation.Stack rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudformation_stack.test" @@ -335,9 +332,6 @@ func TestAccCloudFormationStack_WithURL_withParams(t *testing.T) { } func TestAccCloudFormationStack_WithURLWithParams_withYAML(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") - var stack cloudformation.Stack rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudformation_stack.test" @@ -366,9 +360,6 @@ func TestAccCloudFormationStack_WithURLWithParams_withYAML(t *testing.T) { // Test for https://github.com/hashicorp/terraform/issues/5653 func TestAccCloudFormationStack_WithURLWithParams_noUpdate(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") - var stack cloudformation.Stack rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudformation_stack.test" @@ -860,11 +851,15 @@ resource "aws_s3_bucket" "b" { ] } POLICY +} - - website { - index_document = "index.html" - error_document = "error.html" +resource "aws_s3_website_configuration" "test" { + bucket = aws_s3_bucket.b.id + index_document { + suffix = "index.html" + } + error_document { + key = "error.html" } } @@ -914,11 +909,15 @@ resource "aws_s3_bucket" "b" { ] } POLICY +} - - website { - index_document = "index.html" - error_document = "error.html" +resource "aws_s3_bucket_website_configuration" "test" { + bucket = aws_s3_bucket.b.id + index_document { + suffix = "index.html" + } + error_document { + key = "error.html" } } From ee0c351e08efef687cb3ce4c685aa8436b6b2aa7 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Wed, 2 Feb 2022 23:34:45 -0500 Subject: [PATCH 05/12] r/s3_bucket_website_configuration: add website_domain and website_endpoint to support route53_record resources --- .../s3/bucket_website_configuration.go | 61 +++++++++++-------- .../s3/bucket_website_configuration_test.go | 2 + website/docs/r/s3_bucket.html.markdown | 2 +- ...bucket_website_configuration.html.markdown | 2 + 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/internal/service/s3/bucket_website_configuration.go b/internal/service/s3/bucket_website_configuration.go index e1fd88042b7..9e0d6362801 100644 --- a/internal/service/s3/bucket_website_configuration.go +++ b/internal/service/s3/bucket_website_configuration.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" @@ -143,6 +142,14 @@ func ResourceBucketWebsiteConfiguration() *schema.Resource { }, }, }, + "website_endpoint": { + Type: schema.TypeString, + Computed: true, + }, + "website_domain": { + Type: schema.TypeString, + Computed: true, + }, }, } } @@ -188,7 +195,7 @@ func resourceBucketWebsiteConfigurationCreate(ctx context.Context, d *schema.Res return diag.FromErr(fmt.Errorf("error creating S3 bucket (%s) website configuration: %w", bucket, err)) } - d.SetId(resourceBucketWebsiteConfigurationCreateResourceID(bucket, expectedBucketOwner)) + d.SetId(CreateResourceID(bucket, expectedBucketOwner)) return resourceBucketWebsiteConfigurationRead(ctx, d, meta) } @@ -196,7 +203,7 @@ func resourceBucketWebsiteConfigurationCreate(ctx context.Context, d *schema.Res func resourceBucketWebsiteConfigurationRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - bucket, expectedBucketOwner, err := resourceBucketWebsiteConfigurationParseResourceID(d.Id()) + bucket, expectedBucketOwner, err := ParseResourceID(d.Id()) if err != nil { return diag.FromErr(err) } @@ -245,13 +252,24 @@ func resourceBucketWebsiteConfigurationRead(ctx context.Context, d *schema.Resou return diag.FromErr(fmt.Errorf("error setting routing_rule: %w", err)) } + // Add website_endpoint and website_domain as attributes + websiteEndpoint, err := resourceBucketWebsiteConfigurationWebsiteEndpoint(ctx, meta.(*conns.AWSClient), bucket, expectedBucketOwner) + if err != nil { + return diag.FromErr(err) + } + + if websiteEndpoint != nil { + d.Set("website_endpoint", websiteEndpoint.Endpoint) + d.Set("website_domain", websiteEndpoint.Domain) + } + return nil } func resourceBucketWebsiteConfigurationUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - bucket, expectedBucketOwner, err := resourceBucketWebsiteConfigurationParseResourceID(d.Id()) + bucket, expectedBucketOwner, err := ParseResourceID(d.Id()) if err != nil { return diag.FromErr(err) } @@ -295,7 +313,7 @@ func resourceBucketWebsiteConfigurationUpdate(ctx context.Context, d *schema.Res func resourceBucketWebsiteConfigurationDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - bucket, expectedBucketOwner, err := resourceBucketWebsiteConfigurationParseResourceID(d.Id()) + bucket, expectedBucketOwner, err := ParseResourceID(d.Id()) if err != nil { return diag.FromErr(err) } @@ -321,33 +339,28 @@ func resourceBucketWebsiteConfigurationDelete(ctx context.Context, d *schema.Res return nil } -func resourceBucketWebsiteConfigurationCreateResourceID(bucket, expectedBucketOwner string) string { - if bucket == "" { - return expectedBucketOwner - } +func resourceBucketWebsiteConfigurationWebsiteEndpoint(ctx context.Context, client *conns.AWSClient, bucket, expectedBucketOwner string) (*S3Website, error) { + conn := client.S3Conn - if expectedBucketOwner == "" { - return bucket + input := &s3.GetBucketLocationInput{ + Bucket: aws.String(bucket), } - parts := []string{bucket, expectedBucketOwner} - id := strings.Join(parts, ",") - - return id -} - -func resourceBucketWebsiteConfigurationParseResourceID(id string) (string, string, error) { - parts := strings.Split(id, ",") + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) + } - if len(parts) == 1 && parts[0] != "" { - return parts[0], "", nil + output, err := conn.GetBucketLocationWithContext(ctx, input) + if err != nil { + return nil, fmt.Errorf("error getting S3 Bucket (%s) Location: %w", bucket, err) } - if len(parts) == 2 && parts[0] != "" && parts[1] != "" { - return parts[0], parts[1], nil + var region string + if output.LocationConstraint != nil { + region = aws.StringValue(output.LocationConstraint) } - return "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected BUCKET or BUCKET,EXPECTED_BUCKET_OWNER", id) + return WebsiteEndpoint(client, bucket, region), nil } func expandS3BucketWebsiteConfigurationErrorDocument(l []interface{}) *s3.ErrorDocument { diff --git a/internal/service/s3/bucket_website_configuration_test.go b/internal/service/s3/bucket_website_configuration_test.go index 45639c2ae04..58de6619e20 100644 --- a/internal/service/s3/bucket_website_configuration_test.go +++ b/internal/service/s3/bucket_website_configuration_test.go @@ -32,6 +32,8 @@ func TestAccS3BucketWebsiteConfiguration_basic(t *testing.T) { resource.TestCheckResourceAttrPair(resourceName, "bucket", "aws_s3_bucket.test", "id"), resource.TestCheckResourceAttr(resourceName, "index_document.#", "1"), resource.TestCheckResourceAttr(resourceName, "index_document.0.suffix", "index.html"), + resource.TestCheckResourceAttrSet(resourceName, "website_domain"), + resource.TestCheckResourceAttrSet(resourceName, "website_endpoint"), ), }, { diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index d648d00128e..b9aa8e1fc5e 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -30,7 +30,7 @@ resource "aws_s3_bucket" "b" { ### Static Website Hosting -The `website` argument is read-only as of version 4.0. +The `website` argument is read-only as of version 4.0 of the Terraform AWS Provider. See the [`aws_s3_bucket_website_configuration` resource](s3_bucket_website_configuration.html.markdown) for configuration details. ### Using CORS diff --git a/website/docs/r/s3_bucket_website_configuration.html.markdown b/website/docs/r/s3_bucket_website_configuration.html.markdown index de6bf20d022..15f7646d93b 100644 --- a/website/docs/r/s3_bucket_website_configuration.html.markdown +++ b/website/docs/r/s3_bucket_website_configuration.html.markdown @@ -96,6 +96,8 @@ The `redirect` configuration block supports the following arguments: 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. +* `website_domain` - The domain of the website endpoint. This is used to create Route 53 alias records. +* `website_endpoint` - The website endpoint. ## Import From 37c59de87b70761b2ea140f858eddd08c739d739 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Wed, 2 Feb 2022 23:36:30 -0500 Subject: [PATCH 06/12] route53/record: migrate test to use aws_s3_bucket_website_configuration resource --- internal/service/route53/record_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/service/route53/record_test.go b/internal/service/route53/record_test.go index 62f486f7a46..59778293b1f 100644 --- a/internal/service/route53/record_test.go +++ b/internal/service/route53/record_test.go @@ -571,9 +571,6 @@ func TestAccRoute53Record_Alias_elb(t *testing.T) { } func TestAccRoute53Record_Alias_s3(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") - var record1 route53.ResourceRecordSet rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_route53_record.alias" @@ -1770,9 +1767,12 @@ resource "aws_route53_zone" "main" { resource "aws_s3_bucket" "website" { bucket = %[1]q acl = "public-read" +} - website { - index_document = "index.html" +resource "aws_s3_bucket_website_configuration" "test" { + bucket = aws_s3_bucket.website.id + index_document { + suffix = "index.html" } } @@ -1783,7 +1783,7 @@ resource "aws_route53_record" "alias" { alias { zone_id = aws_s3_bucket.website.hosted_zone_id - name = aws_s3_bucket.website.website_domain + name = aws_s3_bucket_website_configuration.test.website_domain evaluate_target_health = true } } From 2a29a75906b9fde8962a0bd71059b125c7ceef96 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Wed, 2 Feb 2022 23:44:02 -0500 Subject: [PATCH 07/12] s3/bucket_data_source: migrate test to use aws_s3_bucket_website_configuration resource --- .../service/s3/bucket_data_source_test.go | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/internal/service/s3/bucket_data_source_test.go b/internal/service/s3/bucket_data_source_test.go index 7cb982f4e87..d9bbd30e5c4 100644 --- a/internal/service/s3/bucket_data_source_test.go +++ b/internal/service/s3/bucket_data_source_test.go @@ -38,9 +38,6 @@ func TestAccS3BucketDataSource_basic(t *testing.T) { } func TestAccS3BucketDataSource_website(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_website_configuration resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'website' is read-only, migrate configuration to aws_s3_bucket_website_configuration") - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resource.ParallelTest(t, resource.TestCase{ @@ -53,8 +50,8 @@ func TestAccS3BucketDataSource_website(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckBucketExists("data.aws_s3_bucket.bucket"), resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "bucket", "aws_s3_bucket.bucket", "id"), - resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "website_domain", "aws_s3_bucket.bucket", "website_domain"), - resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "website_endpoint", "aws_s3_bucket.bucket", "website_endpoint"), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "website_domain", "aws_s3_bucket_website_configuration.test", "website_domain"), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket.bucket", "website_endpoint", "aws_s3_bucket_website_configuration.test", "website_endpoint"), ), }, }, @@ -78,13 +75,21 @@ func testAccBucketWebsiteDataSourceConfig(bucketName string) string { resource "aws_s3_bucket" "bucket" { bucket = %[1]q acl = "public-read" - website { - index_document = "index.html" - error_document = "error.html" +} + +resource "aws_s3_bucket_website_configuration" "test" { + bucket = aws_s3_bucket.bucket.id + index_document { + suffix = "index.html" + } + error_document { + key = "error.html" } } + data "aws_s3_bucket" "bucket" { - bucket = aws_s3_bucket.bucket.id + # Must have bucket website configured first + bucket = aws_s3_bucket_website_configuration.test.id } `, bucketName) } From 7284a5cb6357c95af14e16f8a5b71d2f784a5865 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Thu, 3 Feb 2022 00:48:10 -0500 Subject: [PATCH 08/12] add instructions for breaking change introduced in #22614 --- website/docs/guides/version-4-upgrade.html.md | 128 +++++++++++++++++- 1 file changed, 127 insertions(+), 1 deletion(-) diff --git a/website/docs/guides/version-4-upgrade.html.md b/website/docs/guides/version-4-upgrade.html.md index c887afe0ae0..3fdda628886 100644 --- a/website/docs/guides/version-4-upgrade.html.md +++ b/website/docs/guides/version-4-upgrade.html.md @@ -439,7 +439,133 @@ output "elasticache_global_replication_group_version_result" { ## Resource: aws_s3_bucket -!> **WARNING:** This topic is placeholder documentation. +To help distribute the management of S3 bucket settings via independent resources, various arguments and attributes in the `aws_s3_bucket` resource +have become read-only. Configurations dependent on these arguments should be updated to use the corresponding `aws_s3_bucket_*` resource. +Once updated, new `aws_s3_bucket_*` resources should be imported into Terraform state. + +### `website`, `website_domain`, and `website_endpoint` Argument deprecation + +Switch your Terraform configuration to the `aws_s3_bucket_website_configuration` resource instead. + +For example, given this previous configuration: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... + website { + index_document = "index.html" + error_document = "error.html" + } +} +``` + +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 "website": its value will be decided automatically based on the result of applying this configuration. +``` + +Since the `website` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_website_configuration` +resource and remove any references to `website` and its nested arguments in the `aws_s3_bucket` resource: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} + +resource "aws_s3_bucket_website_configuration" "example" { + bucket = aws_s3_bucket.example.id + + index_document { + suffix = "index.html" + } + + error_document { + key = "error.html" + } +} +``` + +It is then recommended running `terraform import` on each new resource to prevent data loss, e.g. + +```shell +$ terraform import aws_s3_bucket_website_configuration.example example +aws_s3_bucket_website_configuration.example: Importing from ID "example"... +aws_s3_bucket_website_configuration.example: Import prepared! + Prepared aws_s3_bucket_website_configuration for import +aws_s3_bucket_website_configuration.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. +``` + +For configurations that previously used the `website_domain` attribute to create Route 53 alias records e.g. + +```terraform +resource "aws_route53_zone" "main" { + name = "domain.test" +} + +resource "aws_s3_bucket" "website" { + # ... other configuration ... + website { + index_document = "index.html" + error_document = "error.html" + } +} + +resource "aws_route53_record" "alias" { + zone_id = aws_route53_zone.main.zone_id + name = "www" + type = "A" + + alias { + zone_id = aws_s3_bucket.website.hosted_zone_id + name = aws_s3_bucket.website.website_domain + evaluate_target_health = true + } +} +``` + +An updated configuration: + +```terraform +resource "aws_route53_zone" "main" { + name = "domain.test" +} + +resource "aws_s3_bucket" "website" { + # ... other configuration ... +} + +resource "aws_s3_bucket_website_configuration" "example" { + bucket = aws_s3_bucket.website.id + + index_document { + suffix = "index.html" + } +} + +resource "aws_route53_record" "alias" { + zone_id = aws_route53_zone.main.zone_id + name = "www" + type = "A" + + alias { + zone_id = aws_s3_bucket.website.hosted_zone_id + name = aws_s3_bucket_website_configuration.test.website_domain + evaluate_target_health = true + } +} +``` ## Resource: aws_spot_instance_request From f8a16eaeade9a91303a7718764949b87a81627d6 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Thu, 3 Feb 2022 01:03:50 -0500 Subject: [PATCH 09/12] r/s3_bucket_website_configuration: safe to remove lifecycle ignore_changes --- .../s3/bucket_website_configuration_test.go | 48 ------------------- 1 file changed, 48 deletions(-) diff --git a/internal/service/s3/bucket_website_configuration_test.go b/internal/service/s3/bucket_website_configuration_test.go index 58de6619e20..2ed316bdc25 100644 --- a/internal/service/s3/bucket_website_configuration_test.go +++ b/internal/service/s3/bucket_website_configuration_test.go @@ -340,12 +340,6 @@ func testAccBucketWebsiteConfigurationBasicConfig(rName string) string { resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -362,12 +356,6 @@ func testAccBucketWebsiteConfigurationUpdateConfig(rName string) string { resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -389,12 +377,6 @@ func testAccBucketWebsiteConfigurationConfig_Redirect(rName string) string { resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -411,12 +393,6 @@ func testAccBucketWebsiteConfigurationConfig_RoutingRules_OptionalRedirection(rN resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -449,12 +425,6 @@ func testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectErrors(rName s resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -485,12 +455,6 @@ func testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectToPage(rName s resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -521,12 +485,6 @@ func testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectOnly(rName str resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { @@ -555,12 +513,6 @@ func testAccBucketWebsiteConfigurationConfig_RoutingRules_MultipleRules(rName st resource "aws_s3_bucket" "test" { bucket = %[1]q acl = "public-read" - - lifecycle { - ignore_changes = [ - website - ] - } } resource "aws_s3_bucket_website_configuration" "test" { From 04233b379c6b8f75851a47b9306aa4804fb8fc11 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Thu, 3 Feb 2022 01:18:27 -0500 Subject: [PATCH 10/12] r/s3_bucket_website_configuration: update destroy/exists checks with ID parse method --- .../s3/bucket_website_configuration_test.go | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/internal/service/s3/bucket_website_configuration_test.go b/internal/service/s3/bucket_website_configuration_test.go index 2ed316bdc25..4b759889bc2 100644 --- a/internal/service/s3/bucket_website_configuration_test.go +++ b/internal/service/s3/bucket_website_configuration_test.go @@ -282,8 +282,17 @@ func testAccCheckBucketWebsiteConfigurationDestroy(s *terraform.State) error { continue } + bucket, expectedBucketOwner, err := tfs3.ParseResourceID(rs.Primary.ID) + if err != nil { + return err + } + input := &s3.GetBucketWebsiteInput{ - Bucket: aws.String(rs.Primary.ID), + Bucket: aws.String(bucket), + } + + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) } output, err := conn.GetBucketWebsite(input) @@ -317,8 +326,17 @@ func testAccCheckBucketWebsiteConfigurationExists(resourceName string) resource. conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn + bucket, expectedBucketOwner, err := tfs3.ParseResourceID(rs.Primary.ID) + if err != nil { + return err + } + input := &s3.GetBucketWebsiteInput{ - Bucket: aws.String(rs.Primary.ID), + Bucket: aws.String(bucket), + } + + if expectedBucketOwner != "" { + input.ExpectedBucketOwner = aws.String(expectedBucketOwner) } output, err := conn.GetBucketWebsite(input) From 5be33bd95fd0741b90489ee916b141095e65e538 Mon Sep 17 00:00:00 2001 From: angie pinilla Date: Thu, 3 Feb 2022 02:30:50 -0500 Subject: [PATCH 11/12] Update website/docs/guides/version-4-upgrade.html.md --- website/docs/guides/version-4-upgrade.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/guides/version-4-upgrade.html.md b/website/docs/guides/version-4-upgrade.html.md index 3fdda628886..f9337d26b19 100644 --- a/website/docs/guides/version-4-upgrade.html.md +++ b/website/docs/guides/version-4-upgrade.html.md @@ -561,7 +561,7 @@ resource "aws_route53_record" "alias" { alias { zone_id = aws_s3_bucket.website.hosted_zone_id - name = aws_s3_bucket_website_configuration.test.website_domain + name = aws_s3_bucket_website_configuration.example.website_domain evaluate_target_health = true } } From 4a7f8e57c91c8155db932939768f68d74fb24afa Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Thu, 3 Feb 2022 17:40:31 -0500 Subject: [PATCH 12/12] gofmt yet again.. --- internal/service/s3/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/s3/errors.go b/internal/service/s3/errors.go index 454e05a78af..ca656827361 100644 --- a/internal/service/s3/errors.go +++ b/internal/service/s3/errors.go @@ -9,7 +9,7 @@ const ( ErrCodeNoSuchLifecycleConfiguration = "NoSuchLifecycleConfiguration" ErrCodeNoSuchPublicAccessBlockConfiguration = "NoSuchPublicAccessBlockConfiguration" ErrCodeNoSuchWebsiteConfiguration = "NoSuchWebsiteConfiguration" - ErrCodeNotImplemented = "NotImplemented" + ErrCodeNotImplemented = "NotImplemented" ErrCodeObjectLockConfigurationNotFound = "ObjectLockConfigurationNotFoundError" ErrCodeOperationAborted = "OperationAborted" ErrCodeServerSideEncryptionConfigurationNotFound = "ServerSideEncryptionConfigurationNotFoundError"