Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

r/s3_bucket: make cors_rule configurable #23817

Merged
merged 2 commits into from
Mar 31, 2022
Merged

r/s3_bucket: make cors_rule configurable #23817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cd08b8
r/s3_bucket: make 'cors_rule' configurable
anGie44 Mar 23, 2022
fd098d2
Update CHANGELOG for #23817
anGie44 Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/23817.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_s3_bucket: Update `cors_rule` parameter to be configurable. Please refer to the documentation for details on drift detection and potential conflicts when configuring this parameter with the standalone `aws_s3_bucket_cors_configuration` resource.
```
106 changes: 87 additions & 19 deletions internal/service/s3/bucket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,38 +128,34 @@ func ResourceBucket() *schema.Resource {

"cors_rule": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Deprecated: "Use the aws_s3_bucket_cors_configuration resource instead",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"allowed_headers": {
Type: schema.TypeList,
Computed: true,
Deprecated: "Use the aws_s3_bucket_cors_configuration resource instead",
Elem: &schema.Schema{Type: schema.TypeString},
Type: schema.TypeList,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"allowed_methods": {
Type: schema.TypeList,
Computed: true,
Deprecated: "Use the aws_s3_bucket_cors_configuration resource instead",
Elem: &schema.Schema{Type: schema.TypeString},
Type: schema.TypeList,
Required: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"allowed_origins": {
Type: schema.TypeList,
Computed: true,
Deprecated: "Use the aws_s3_bucket_cors_configuration resource instead",
Elem: &schema.Schema{Type: schema.TypeString},
Type: schema.TypeList,
Required: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"expose_headers": {
Type: schema.TypeList,
Computed: true,
Deprecated: "Use the aws_s3_bucket_cors_configuration resource instead",
Elem: &schema.Schema{Type: schema.TypeString},
Type: schema.TypeList,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"max_age_seconds": {
Type: schema.TypeInt,
Computed: true,
Deprecated: "Use the aws_s3_bucket_cors_configuration resource instead",
Type: schema.TypeInt,
Optional: true,
},
},
},
Expand Down Expand Up @@ -776,6 +772,12 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error {
}
}

if d.HasChange("cors_rule") {
if err := resourceBucketInternalCorsUpdate(conn, d); err != nil {
return fmt.Errorf("error updating S3 Bucket (%s) CORS Rules: %w", d.Id(), err)
}
}

if d.HasChange("grant") {
if err := resourceBucketInternalGrantsUpdate(conn, d); err != nil {
return fmt.Errorf("error updating S3 Bucket (%s) Grants: %w", d.Id(), err)
Expand Down Expand Up @@ -1524,6 +1526,72 @@ func resourceBucketInternalACLUpdate(conn *s3.S3, d *schema.ResourceData) error
return err
}

func resourceBucketInternalCorsUpdate(conn *s3.S3, d *schema.ResourceData) error {
rawCors := d.Get("cors_rule").([]interface{})

if len(rawCors) == 0 {
// Delete CORS
_, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) {
return conn.DeleteBucketCors(&s3.DeleteBucketCorsInput{
Bucket: aws.String(d.Id()),
})
})

if err != nil {
return fmt.Errorf("error deleting S3 Bucket (%s) CORS: %w", d.Id(), err)
}

return nil
}
// Put CORS
rules := make([]*s3.CORSRule, 0, len(rawCors))
for _, cors := range rawCors {
// Prevent panic
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/7546
corsMap, ok := cors.(map[string]interface{})
if !ok {
continue
}
r := &s3.CORSRule{}
for k, v := range corsMap {
if k == "max_age_seconds" {
r.MaxAgeSeconds = aws.Int64(int64(v.(int)))
} else {
vMap := make([]*string, len(v.([]interface{})))
for i, vv := range v.([]interface{}) {
if str, ok := vv.(string); ok {
vMap[i] = aws.String(str)
}
}
switch k {
case "allowed_headers":
r.AllowedHeaders = vMap
case "allowed_methods":
r.AllowedMethods = vMap
case "allowed_origins":
r.AllowedOrigins = vMap
case "expose_headers":
r.ExposeHeaders = vMap
}
}
}
rules = append(rules, r)
}

input := &s3.PutBucketCorsInput{
Bucket: aws.String(d.Id()),
CORSConfiguration: &s3.CORSConfiguration{
CORSRules: rules,
},
}

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

return err
}

func resourceBucketInternalGrantsUpdate(conn *s3.S3, d *schema.ResourceData) error {
grants := d.Get("grant").(*schema.Set)

Expand Down
120 changes: 120 additions & 0 deletions internal/service/s3/bucket_cors_configuration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,89 @@ func TestAccS3BucketCorsConfiguration_MultipleRules(t *testing.T) {
})
}

func TestAccS3BucketCorsConfiguration_migrate_corsRuleNoChange(t *testing.T) {
bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
bucketResourceName := "aws_s3_bucket.test"
resourceName := "aws_s3_bucket_cors_configuration.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID),
ProviderFactories: acctest.ProviderFactories,
CheckDestroy: testAccCheckBucketDestroy,
Steps: []resource.TestStep{
{
Config: testAccBucketConfig_withCORS(bucketName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBucketExists(bucketResourceName),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.#", "1"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.allowed_headers.#", "1"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.allowed_methods.#", "2"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.allowed_origins.#", "1"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.expose_headers.#", "2"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.max_age_seconds", "3000"),
),
},
{
Config: testAccBucketCorsConfigurationConfig_Migrate_CorsRuleNoChange(bucketName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBucketCorsConfigurationExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "bucket", bucketResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "cors_rule.#", "1"),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "cors_rule.*", map[string]string{
"allowed_headers.#": "1",
"allowed_methods.#": "2",
"allowed_origins.#": "1",
"expose_headers.#": "2",
"max_age_seconds": "3000",
}),
),
},
},
})
}

func TestAccS3BucketCorsConfiguration_migrate_corsRuleWithChange(t *testing.T) {
bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
bucketResourceName := "aws_s3_bucket.test"
resourceName := "aws_s3_bucket_cors_configuration.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID),
ProviderFactories: acctest.ProviderFactories,
CheckDestroy: testAccCheckBucketDestroy,
Steps: []resource.TestStep{
{
Config: testAccBucketConfig_withCORS(bucketName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBucketExists(bucketResourceName),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.#", "1"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.allowed_headers.#", "1"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.allowed_methods.#", "2"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.allowed_origins.#", "1"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.expose_headers.#", "2"),
resource.TestCheckResourceAttr(bucketResourceName, "cors_rule.0.max_age_seconds", "3000"),
),
},
{
Config: testAccBucketCorsConfigurationConfig_Migrate_CorsRuleWithChange(bucketName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBucketCorsConfigurationExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "bucket", bucketResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "cors_rule.#", "1"),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "cors_rule.*", map[string]string{
"allowed_methods.#": "1",
"allowed_origins.#": "1",
}),
resource.TestCheckTypeSetElemAttr(resourceName, "cors_rule.*.allowed_methods.*", "PUT"),
resource.TestCheckTypeSetElemAttr(resourceName, "cors_rule.*.allowed_origins.*", "https://www.example.com"),
),
},
},
})
}

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

Expand Down Expand Up @@ -360,3 +443,40 @@ resource "aws_s3_bucket_cors_configuration" "test" {
}
`, rName)
}

func testAccBucketCorsConfigurationConfig_Migrate_CorsRuleNoChange(rName string) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
}

resource "aws_s3_bucket_cors_configuration" "test" {
bucket = aws_s3_bucket.test.id

cors_rule {
allowed_headers = ["*"]
allowed_methods = ["PUT", "POST"]
allowed_origins = ["https://www.example.com"]
expose_headers = ["x-amz-server-side-encryption", "ETag"]
max_age_seconds = 3000
}
}
`, rName)
}

func testAccBucketCorsConfigurationConfig_Migrate_CorsRuleWithChange(rName string) string {
return fmt.Sprintf(`
resource "aws_s3_bucket" "test" {
bucket = %[1]q
}

resource "aws_s3_bucket_cors_configuration" "test" {
bucket = aws_s3_bucket.test.id

cors_rule {
allowed_methods = ["PUT"]
allowed_origins = ["https://www.example.com"]
}
}
`, rName)
}
Loading