diff --git a/.changelog/24198.txt b/.changelog/24198.txt new file mode 100644 index 00000000000..e3b174b6d05 --- /dev/null +++ b/.changelog/24198.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_s3_bucket_website_configuration: Add `routing_rules` parameter to be used instead of `routing_rule` to support configurations with empty String values +``` \ No newline at end of file diff --git a/internal/service/s3/bucket_website_configuration.go b/internal/service/s3/bucket_website_configuration.go index 2578e01f277..0beb951f792 100644 --- a/internal/service/s3/bucket_website_configuration.go +++ b/internal/service/s3/bucket_website_configuration.go @@ -2,6 +2,7 @@ package s3 import ( "context" + "encoding/json" "fmt" "log" "time" @@ -11,6 +12,7 @@ import ( "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" @@ -74,6 +76,7 @@ func ResourceBucketWebsiteConfiguration() *schema.Resource { "error_document", "index_document", "routing_rule", + "routing_rules", }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -90,8 +93,10 @@ func ResourceBucketWebsiteConfiguration() *schema.Resource { }, }, "routing_rule": { - Type: schema.TypeList, - Optional: true, + Type: schema.TypeList, + Optional: true, + Computed: true, + ConflictsWith: []string{"routing_rules"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "condition": { @@ -144,6 +149,17 @@ func ResourceBucketWebsiteConfiguration() *schema.Resource { }, }, }, + "routing_rules": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ConflictsWith: []string{"routing_rule"}, + ValidateFunc: validation.StringIsJSON, + StateFunc: func(v interface{}) string { + json, _ := structure.NormalizeJsonString(v) + return json + }, + }, "website_endpoint": { Type: schema.TypeString, Computed: true, @@ -180,6 +196,14 @@ func resourceBucketWebsiteConfigurationCreate(ctx context.Context, d *schema.Res websiteConfig.RoutingRules = expandBucketWebsiteConfigurationRoutingRules(v.([]interface{})) } + if v, ok := d.GetOk("routing_rules"); ok { + var unmarshalledRules []*s3.RoutingRule + if err := json.Unmarshal([]byte(v.(string)), &unmarshalledRules); err != nil { + return diag.FromErr(fmt.Errorf("error creating S3 Bucket (%s) website configuration: %w", bucket, err)) + } + websiteConfig.RoutingRules = unmarshalledRules + } + input := &s3.PutBucketWebsiteInput{ Bucket: aws.String(bucket), WebsiteConfiguration: websiteConfig, @@ -254,6 +278,16 @@ func resourceBucketWebsiteConfigurationRead(ctx context.Context, d *schema.Resou return diag.FromErr(fmt.Errorf("error setting routing_rule: %w", err)) } + if output.RoutingRules != nil { + rr, err := normalizeRoutingRules(output.RoutingRules) + if err != nil { + return diag.FromErr(fmt.Errorf("error while marshaling routing rules: %w", err)) + } + d.Set("routing_rules", rr) + } else { + d.Set("routing_rules", nil) + } + // Add website_endpoint and website_domain as attributes websiteEndpoint, err := resourceBucketWebsiteConfigurationWebsiteEndpoint(ctx, meta.(*conns.AWSClient), bucket, expectedBucketOwner) if err != nil { @@ -290,8 +324,29 @@ func resourceBucketWebsiteConfigurationUpdate(ctx context.Context, d *schema.Res websiteConfig.RedirectAllRequestsTo = expandBucketWebsiteConfigurationRedirectAllRequestsTo(v.([]interface{})) } - if v, ok := d.GetOk("routing_rule"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { - websiteConfig.RoutingRules = expandBucketWebsiteConfigurationRoutingRules(v.([]interface{})) + if d.HasChanges("routing_rule", "routing_rules") { + if d.HasChange("routing_rule") { + websiteConfig.RoutingRules = expandBucketWebsiteConfigurationRoutingRules(d.Get("routing_rule").([]interface{})) + } else { + var unmarshalledRules []*s3.RoutingRule + if err := json.Unmarshal([]byte(d.Get("routing_rules").(string)), &unmarshalledRules); err != nil { + return diag.FromErr(fmt.Errorf("error updating S3 Bucket (%s) website configuration: %w", bucket, err)) + } + websiteConfig.RoutingRules = unmarshalledRules + } + } else { + // Still send the current RoutingRules configuration + if v, ok := d.GetOk("routing_rule"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + websiteConfig.RoutingRules = expandBucketWebsiteConfigurationRoutingRules(v.([]interface{})) + } + + if v, ok := d.GetOk("routing_rules"); ok { + var unmarshalledRules []*s3.RoutingRule + if err := json.Unmarshal([]byte(v.(string)), &unmarshalledRules); err != nil { + return diag.FromErr(fmt.Errorf("error updating S3 Bucket (%s) website configuration: %w", bucket, err)) + } + websiteConfig.RoutingRules = unmarshalledRules + } } input := &s3.PutBucketWebsiteInput{ diff --git a/internal/service/s3/bucket_website_configuration_test.go b/internal/service/s3/bucket_website_configuration_test.go index 683e9859b4d..688b8341122 100644 --- a/internal/service/s3/bucket_website_configuration_test.go +++ b/internal/service/s3/bucket_website_configuration_test.go @@ -131,7 +131,7 @@ func TestAccS3BucketWebsiteConfiguration_Redirect(t *testing.T) { }) } -func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *testing.T) { +func TestAccS3BucketWebsiteConfiguration_RoutingRule_ConditionAndRedirect(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_bucket_website_configuration.test" @@ -142,7 +142,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_OptionalRedirection(rName), + Config: testAccBucketWebsiteConfigurationConfig_RoutingRule_OptionalRedirection(rName), Check: resource.ComposeTestCheckFunc( testAccCheckBucketWebsiteConfigurationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), @@ -152,6 +152,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te "redirect.#": "1", "redirect.0.replace_key_prefix_with": "documents/", }), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), ), }, { @@ -160,7 +161,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te ImportStateVerify: true, }, { - Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectErrors(rName), + Config: testAccBucketWebsiteConfigurationConfig_RoutingRule_RedirectErrors(rName), Check: resource.ComposeTestCheckFunc( testAccCheckBucketWebsiteConfigurationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), @@ -170,6 +171,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te "redirect.#": "1", "redirect.0.replace_key_prefix_with": "report-404", }), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), ), }, { @@ -178,7 +180,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te ImportStateVerify: true, }, { - Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectToPage(rName), + Config: testAccBucketWebsiteConfigurationConfig_RoutingRule_RedirectToPage(rName), Check: resource.ComposeTestCheckFunc( testAccCheckBucketWebsiteConfigurationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), @@ -188,6 +190,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te "redirect.#": "1", "redirect.0.replace_key_with": "errorpage.html", }), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), ), }, { @@ -199,7 +202,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *te }) } -func TestAccS3BucketWebsiteConfiguration_RoutingRules_MultipleRules(t *testing.T) { +func TestAccS3BucketWebsiteConfiguration_RoutingRule_MultipleRules(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_bucket_website_configuration.test" @@ -210,7 +213,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_MultipleRules(t *testing.T CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_MultipleRules(rName), + Config: testAccBucketWebsiteConfigurationConfig_RoutingRule_MultipleRules(rName), Check: resource.ComposeTestCheckFunc( testAccCheckBucketWebsiteConfigurationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "2"), @@ -226,6 +229,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_MultipleRules(t *testing.T "redirect.#": "1", "redirect.0.replace_key_with": "errorpage.html", }), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), ), }, { @@ -243,7 +247,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_MultipleRules(t *testing.T }) } -func TestAccS3BucketWebsiteConfiguration_RoutingRules_RedirectOnly(t *testing.T) { +func TestAccS3BucketWebsiteConfiguration_RoutingRule_RedirectOnly(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_bucket_website_configuration.test" @@ -254,7 +258,7 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_RedirectOnly(t *testing.T) CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, Steps: []resource.TestStep{ { - Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectOnly(rName), + Config: testAccBucketWebsiteConfigurationConfig_RoutingRule_RedirectOnly(rName), Check: resource.ComposeTestCheckFunc( testAccCheckBucketWebsiteConfigurationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), @@ -263,6 +267,34 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_RedirectOnly(t *testing.T) "redirect.0.protocol": s3.ProtocolHttps, "redirect.0.replace_key_with": "errorpage.html", }), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirect(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_website_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_ConditionAndRedirect(rName, "documents/"), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketWebsiteConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), ), }, { @@ -274,6 +306,93 @@ func TestAccS3BucketWebsiteConfiguration_RoutingRules_RedirectOnly(t *testing.T) }) } +func TestAccS3BucketWebsiteConfiguration_RoutingRules_ConditionAndRedirectWithEmptyString(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_website_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_ConditionAndRedirect(rName, ""), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketWebsiteConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccS3BucketWebsiteConfiguration_RoutingRules_updateConditionAndRedirect(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_website_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_ConditionAndRedirect(rName, "documents/"), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketWebsiteConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), + ), + }, + { + Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_ConditionAndRedirect(rName, ""), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketWebsiteConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), + ), + }, + }, + }) +} + +func TestAccS3BucketWebsiteConfiguration_RoutingRuleToRoutingRules(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket_website_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketWebsiteConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketWebsiteConfigurationConfig_RoutingRule_OptionalRedirection(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketWebsiteConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), + ), + }, + { + Config: testAccBucketWebsiteConfigurationConfig_RoutingRules_ConditionAndRedirect(rName, "documents/"), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketWebsiteConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "routing_rule.#", "1"), + resource.TestCheckResourceAttrSet(resourceName, "routing_rules"), + ), + }, + }, + }) +} + func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithIndexDocumentNoChange(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) bucketResourceName := "aws_s3_bucket.test" @@ -336,7 +455,7 @@ func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithIndexDocumentWithCha }) } -func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithRoutingRulesNoChange(t *testing.T) { +func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithRoutingRuleNoChange(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) bucketResourceName := "aws_s3_bucket.test" resourceName := "aws_s3_bucket_website_configuration.test" @@ -366,7 +485,7 @@ func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithRoutingRulesNoChange }) } -func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithRoutingRulesWithChange(t *testing.T) { +func TestAccS3BucketWebsiteConfiguration_migrate_websiteWithRoutingRuleWithChange(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) bucketResourceName := "aws_s3_bucket.test" resourceName := "aws_s3_bucket_website_configuration.test" @@ -545,7 +664,7 @@ resource "aws_s3_bucket_website_configuration" "test" { `, rName) } -func testAccBucketWebsiteConfigurationConfig_RoutingRules_OptionalRedirection(rName string) string { +func testAccBucketWebsiteConfigurationConfig_RoutingRule_OptionalRedirection(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q @@ -580,7 +699,7 @@ resource "aws_s3_bucket_website_configuration" "test" { `, rName) } -func testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectErrors(rName string) string { +func testAccBucketWebsiteConfigurationConfig_RoutingRule_RedirectErrors(rName string) string { return acctest.ConfigCompose( acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), fmt.Sprintf(` @@ -617,7 +736,7 @@ resource "aws_s3_bucket_website_configuration" "test" { `, rName)) } -func testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectToPage(rName string) string { +func testAccBucketWebsiteConfigurationConfig_RoutingRule_RedirectToPage(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q @@ -652,7 +771,7 @@ resource "aws_s3_bucket_website_configuration" "test" { `, rName) } -func testAccBucketWebsiteConfigurationConfig_RoutingRules_RedirectOnly(rName string) string { +func testAccBucketWebsiteConfigurationConfig_RoutingRule_RedirectOnly(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q @@ -684,7 +803,7 @@ resource "aws_s3_bucket_website_configuration" "test" { `, rName) } -func testAccBucketWebsiteConfigurationConfig_RoutingRules_MultipleRules(rName string) string { +func testAccBucketWebsiteConfigurationConfig_RoutingRule_MultipleRules(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q @@ -727,6 +846,44 @@ resource "aws_s3_bucket_website_configuration" "test" { `, rName) } +func testAccBucketWebsiteConfigurationConfig_RoutingRules_ConditionAndRedirect(bucketName, replaceKeyPrefixWith string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_acl" "test" { + bucket = aws_s3_bucket.test.id + acl = "public-read" +} + +resource "aws_s3_bucket_website_configuration" "test" { + bucket = aws_s3_bucket.test.id + + index_document { + suffix = "index.html" + } + + error_document { + key = "error.html" + } + + routing_rules = <