Skip to content

Commit 601cbc4

Browse files
gurusai-voletigurusai-voleti
authored
fix: (storage) fixed nil issue in expandCors in google_storage_bucket resource (#14482)
1 parent 27bdf16 commit 601cbc4

File tree

3 files changed

+193
-12
lines changed

3 files changed

+193
-12
lines changed

‎mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.tmpl‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,20 +1330,38 @@ func expandCors(configured []interface{}) []*storage.BucketCors {
13301330
}
13311331
corsRules := make([]*storage.BucketCors, 0, len(configured))
13321332
for _, raw := range configured {
1333+
if raw == nil {
1334+
corsRule := storage.BucketCors{}
1335+
corsRules = append(corsRules, &corsRule)
1336+
continue
1337+
}
13331338
data := raw.(map[string]interface{})
1334-
corsRule := storage.BucketCors{
1335-
Origin: tpgresource.ConvertStringArr(data["origin"].([]interface{})),
1336-
Method: tpgresource.ConvertStringArr(data["method"].([]interface{})),
1337-
ResponseHeader: tpgresource.ConvertStringArr(data["response_header"].([]interface{})),
1338-
MaxAgeSeconds: int64(data["max_age_seconds"].(int)),
1339+
corsRule := storage.BucketCors{}
1340+
if v, ok := data["method"].([]interface{}); ok {
1341+
corsRule.Method = tpgresource.ConvertStringArr(v)
1342+
}
1343+
1344+
if v, ok := data["origin"].([]interface{}); ok {
1345+
corsRule.Origin = tpgresource.ConvertStringArr(v)
13391346
}
13401347

1348+
if v, ok := data["response_header"].([]interface{}); ok {
1349+
corsRule.ResponseHeader = tpgresource.ConvertStringArr(v)
1350+
}
1351+
1352+
if v, ok := data["max_age_seconds"].(int); ok {
1353+
corsRule.MaxAgeSeconds = int64(v)
1354+
}
13411355
corsRules = append(corsRules, &corsRule)
13421356
}
13431357
return corsRules
13441358
}
13451359

1346-
func flattenCors(corsRules []*storage.BucketCors) []map[string]interface{} {
1360+
func flattenCors(d *schema.ResourceData, corsRules []*storage.BucketCors) []map[string]interface{} {
1361+
cors, _ := d.GetOk("cors")
1362+
if corsRules == nil && len(cors.([]interface{})) == 0 {
1363+
return []map[string]interface{}{}
1364+
}
13471365
corsRulesSchema := make([]map[string]interface{}, 0, len(corsRules))
13481366
for _, corsRule := range corsRules {
13491367
data := map[string]interface{}{
@@ -1352,12 +1370,43 @@ func flattenCors(corsRules []*storage.BucketCors) []map[string]interface{} {
13521370
"response_header": corsRule.ResponseHeader,
13531371
"max_age_seconds": corsRule.MaxAgeSeconds,
13541372
}
1355-
13561373
corsRulesSchema = append(corsRulesSchema, data)
13571374
}
1375+
1376+
if len(corsRules) < len(cors.([]interface{})) {
1377+
emptyCors := map[string]interface{}{
1378+
"origin": []string{},
1379+
"method": []string{},
1380+
"response_header": []string{},
1381+
"max_age_seconds": 0,
1382+
}
1383+
for k, v := range cors.([]interface{}) {
1384+
if isCorsRuleEmpty(v) {
1385+
corsRulesSchema = append(corsRulesSchema[:k], append([]map[string]interface{}{emptyCors}, corsRulesSchema[k:]...)...)
1386+
}
1387+
}
1388+
}
13581389
return corsRulesSchema
13591390
}
13601391

1392+
func isCorsRuleEmpty(rule interface{}) bool {
1393+
if rule == nil {
1394+
return true
1395+
}
1396+
corsRule, ok := rule.(map[string]interface{})
1397+
if !ok {
1398+
return true
1399+
}
1400+
method := corsRule["method"] == nil || len(corsRule["method"].([]interface{})) == 0
1401+
origin := corsRule["origin"] == nil || len(corsRule["origin"].([]interface{})) == 0
1402+
responseHeader := corsRule["response_header"] == nil || len(corsRule["response_header"].([]interface{})) == 0
1403+
maxAgeSeconds := corsRule["max_age_seconds"] == 0
1404+
if method && origin && responseHeader && maxAgeSeconds {
1405+
return true
1406+
}
1407+
return false
1408+
}
1409+
13611410
func expandBucketEncryption(configured interface{}) *storage.BucketEncryption {
13621411
encs := configured.([]interface{})
13631412
if len(encs) == 0 || encs[0] == nil {
@@ -2218,7 +2267,7 @@ func setStorageBucket(d *schema.ResourceData, config *transport_tpg.Config, res
22182267
if err := d.Set("location", res.Location); err != nil {
22192268
return fmt.Errorf("Error setting location: %s", err)
22202269
}
2221-
if err := d.Set("cors", flattenCors(res.Cors)); err != nil {
2270+
if err := d.Set("cors", flattenCors(d, res.Cors)); err != nil {
22222271
return fmt.Errorf("Error setting cors: %s", err)
22232272
}
22242273
if err := d.Set("default_event_based_hold", res.DefaultEventBasedHold); err != nil {

‎mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go‎

Lines changed: 130 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"log"
77
"regexp"
8+
"strconv"
89
"testing"
910
"time"
1011

@@ -1149,6 +1150,62 @@ func TestAccStorageBucket_cors(t *testing.T) {
11491150
})
11501151
}
11511152

1153+
func TestAccStorageBucket_emptyCors(t *testing.T) {
1154+
t.Parallel()
1155+
1156+
bucketName := fmt.Sprintf("tf-test-acl-bucket-%d", acctest.RandInt(t))
1157+
1158+
acctest.VcrTest(t, resource.TestCase{
1159+
PreCheck: func() { acctest.AccTestPreCheck(t) },
1160+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
1161+
CheckDestroy: testAccStorageBucketDestroyProducer(t),
1162+
Steps: []resource.TestStep{
1163+
{
1164+
Config: testGoogleStorageBucketsCors(bucketName),
1165+
},
1166+
{
1167+
ResourceName: "google_storage_bucket.bucket",
1168+
ImportState: true,
1169+
ImportStateVerify: true,
1170+
ImportStateVerifyIgnore: []string{"force_destroy"},
1171+
},
1172+
{
1173+
Config: testGoogleStorageBucketsEmptyCors(bucketName),
1174+
Check: resource.ComposeTestCheckFunc(
1175+
testAccCheckBucketCorsCount(t, 3),
1176+
),
1177+
},
1178+
{
1179+
ResourceName: "google_storage_bucket.bucket",
1180+
ImportState: true,
1181+
ImportStateVerify: true,
1182+
ImportStateVerifyIgnore: []string{"force_destroy", "cors"},
1183+
},
1184+
{
1185+
Config: testGoogleStorageBucketPartiallyEmptyCors(bucketName),
1186+
Check: resource.ComposeTestCheckFunc(
1187+
testAccCheckBucketCorsCount(t, 4),
1188+
),
1189+
},
1190+
{
1191+
ResourceName: "google_storage_bucket.bucket",
1192+
ImportState: true,
1193+
ImportStateVerify: true,
1194+
ImportStateVerifyIgnore: []string{"force_destroy", "cors"},
1195+
},
1196+
{
1197+
Config: testGoogleStorageBucketsRemoveCorsCompletely(bucketName),
1198+
},
1199+
{
1200+
ResourceName: "google_storage_bucket.bucket",
1201+
ImportState: true,
1202+
ImportStateVerify: true,
1203+
ImportStateVerifyIgnore: []string{"force_destroy"},
1204+
},
1205+
},
1206+
})
1207+
}
1208+
11521209
func TestAccStorageBucket_defaultEventBasedHold(t *testing.T) {
11531210
t.Parallel()
11541211

@@ -2184,15 +2241,84 @@ resource "google_storage_bucket" "bucket" {
21842241
}
21852242
21862243
cors {
2187-
origin = ["ghi", "jkl"]
2188-
method = ["z9z"]
2189-
response_header = ["000"]
2190-
max_age_seconds = 5
2244+
origin = ["ghi", "jkl"]
2245+
method = ["z9z"]
2246+
response_header = ["000"]
2247+
max_age_seconds = 0
2248+
}
2249+
}
2250+
`, bucketName)
2251+
}
2252+
2253+
func testGoogleStorageBucketsEmptyCors(bucketName string) string {
2254+
return fmt.Sprintf(`
2255+
resource "google_storage_bucket" "bucket" {
2256+
name = "%s"
2257+
location = "US"
2258+
force_destroy = true
2259+
cors {
2260+
origin = []
2261+
method = []
2262+
response_header = []
2263+
max_age_seconds = 0
21912264
}
2265+
cors {}
2266+
cors {
2267+
origin = []
2268+
method = []
2269+
}
2270+
}
2271+
`, bucketName)
2272+
}
2273+
2274+
func testGoogleStorageBucketsRemoveCorsCompletely(bucketName string) string {
2275+
return fmt.Sprintf(`
2276+
resource "google_storage_bucket" "bucket" {
2277+
name = "%s"
2278+
location = "US"
2279+
force_destroy = true
21922280
}
21932281
`, bucketName)
21942282
}
21952283

2284+
func testGoogleStorageBucketPartiallyEmptyCors(bucketName string) string {
2285+
return fmt.Sprintf(`
2286+
resource "google_storage_bucket" "bucket" {
2287+
name = "%s"
2288+
location = "US"
2289+
force_destroy = true
2290+
cors {
2291+
origin = ["*"]
2292+
method = ["GET"]
2293+
}
2294+
cors{}
2295+
cors {
2296+
origin = ["https://sample.com"]
2297+
method = ["GET"]
2298+
}
2299+
cors{}
2300+
}
2301+
`, bucketName)
2302+
}
2303+
2304+
func testAccCheckBucketCorsCount(t *testing.T, corsInConfig int) resource.TestCheckFunc {
2305+
return func(s *terraform.State) error {
2306+
rs, ok := s.RootModule().Resources["google_storage_bucket.bucket"]
2307+
if !ok {
2308+
return fmt.Errorf("Bucket not found: %s", "google_storage_bucket.bucket")
2309+
}
2310+
corsInState, err := strconv.Atoi(rs.Primary.Attributes["cors.#"])
2311+
if err != nil {
2312+
return fmt.Errorf("Error conersion string to int %s", err)
2313+
}
2314+
2315+
if corsInConfig != corsInState {
2316+
return fmt.Errorf("Length of Cors in terraform state file and config should be equal")
2317+
}
2318+
return nil
2319+
}
2320+
}
2321+
21962322
func testAccStorageBucket_defaultEventBasedHold(bucketName string) string {
21972323
return fmt.Sprintf(`
21982324
resource "google_storage_bucket" "bucket" {

‎mmv1/third_party/terraform/website/docs/r/storage_bucket.html.markdown‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ resource "google_storage_bucket" "static-site" {
3838
response_header = ["*"]
3939
max_age_seconds = 3600
4040
}
41+
cors {
42+
origin = ["http://image-store.com"]
43+
method = ["GET", "HEAD", "PUT", "POST", "DELETE"]
44+
response_header = ["*"]
45+
max_age_seconds = 0
46+
}
4147
}
4248
```
4349

0 commit comments

Comments
 (0)