From eeb744577f58b2c8a5a218a1be6b0b8ccea025ae Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Mon, 12 Jun 2017 14:49:34 +0200 Subject: [PATCH 01/12] [WIP] Add provider/google/google_storage_bucket lifecycle interface --- google/resource_storage_bucket.go | 202 ++++++++++++++++++++++++ google/resource_storage_bucket_test.go | 205 +++++++++++++++++++++++++ 2 files changed, 407 insertions(+) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index f58a5785c29..1a7fdc00bb7 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -1,12 +1,14 @@ package google import ( + "bytes" "errors" "fmt" "log" "strings" "time" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" @@ -77,6 +79,64 @@ func resourceStorageBucket() *schema.Resource { ForceNew: true, }, + "lifecycle_rule": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "action": { + Type: schema.TypeSet, + Required: true, + MaxItems: 1, + Set: resourceGCSBucketLifecycleRuleActionHash, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "type": { + Type: schema.TypeString, + Required: true, + }, + "storage_class": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + "condition": { + Type: schema.TypeSet, + Required: true, + MaxItems: 1, + Set: resourceGCSBucketLifecycleRuleConditionHash, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "age": { + Type: schema.TypeInt, + Optional: true, + }, + "created_before": { + Type: schema.TypeString, + Optional: true, + }, + "is_live": { + Type: schema.TypeBool, + Optional: true, + }, + "matches_storage_class": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "number_of_newer_versions": { + Type: schema.TypeInt, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + "website": &schema.Schema{ Type: schema.TypeList, Optional: true, @@ -150,6 +210,10 @@ func resourceStorageBucketCreate(d *schema.ResourceData, meta interface{}) error sb.StorageClass = v.(string) } + if err := resourceGCSBucketLifecycleCreateOrUpdate(d, sb, false); err != nil { + return err + } + if v, ok := d.GetOk("website"); ok { websites := v.([]interface{}) @@ -208,6 +272,12 @@ func resourceStorageBucketUpdate(d *schema.ResourceData, meta interface{}) error sb := &storage.Bucket{} + if d.HasChange("lifecycle_rule") { + if err := resourceGCSBucketLifecycleCreateOrUpdate(d, sb, true); err != nil { + return err + } + } + if d.HasChange("website") { if v, ok := d.GetOk("website"); ok { websites := v.([]interface{}) @@ -382,3 +452,135 @@ func flattenCors(corsRules []*storage.BucketCors) []map[string]interface{} { } return corsRulesSchema } + +func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storage.Bucket, update bool) error { + if v, ok := d.GetOk("lifecycle_rule"); ok { + lifecycle_rules := v.([]interface{}) + + if len(lifecycle_rules) > 100 { + return fmt.Errorf("At most 100 lifecycle_rule blocks are allowed") + } + + sb.Lifecycle = &storage.BucketLifecycle{} + sb.Lifecycle.Rule = make([]*storage.BucketLifecycleRule, 0, len(lifecycle_rules)) + + for _, lifecycle_rule := range lifecycle_rules { + lifecycle_rule := lifecycle_rule.(map[string]interface{}) + + target_lifecycle_rule := &storage.BucketLifecycleRule{} + + if v, ok := lifecycle_rule["action"]; ok { + action := v.(*schema.Set).List()[0].(map[string]interface{}) + + target_lifecycle_rule.Action = &storage.BucketLifecycleRuleAction{} + + if v, ok := action["type"]; ok { + target_lifecycle_rule.Action.Type = v.(string) + } + + if v, ok := action["storage_class"]; ok { + target_lifecycle_rule.Action.StorageClass = v.(string) + } + } + + if v, ok := lifecycle_rule["condition"]; ok { + condition := v.(*schema.Set).List()[0].(map[string]interface{}) + condition_elements := 0 + + target_lifecycle_rule.Condition = &storage.BucketLifecycleRuleCondition{} + + if v, ok := condition["age"]; ok { + condition_elements++ + target_lifecycle_rule.Condition.Age = int64(v.(int)) + } + + if v, ok := condition["created_before"]; ok { + condition_elements++ + target_lifecycle_rule.Condition.CreatedBefore = v.(string) + } + + if v, ok := condition["is_live"]; ok { + condition_elements++ + target_lifecycle_rule.Condition.IsLive = v.(bool) + } + + if v, ok := condition["matches_storage_class"]; ok { + matches_storage_classes := v.([]interface{}) + + target_matches_storage_classes := make([]string, 0, len(matches_storage_classes)) + + for _, v := range matches_storage_classes { + target_matches_storage_classes = append(target_matches_storage_classes, v.(string)) + condition_elements++ + } + + target_lifecycle_rule.Condition.MatchesStorageClass = target_matches_storage_classes + } + + if v, ok := condition["number_of_newer_versions"]; ok { + condition_elements++ + target_lifecycle_rule.Condition.NumNewerVersions = int64(v.(int)) + } + + if condition_elements < 1 { + return fmt.Errorf("At least one condition element is required") + } + + sb.Lifecycle.Rule = append(sb.Lifecycle.Rule, target_lifecycle_rule) + } + } + } + + return nil +} + +func resourceGCSBucketLifecycleRuleActionHash(v interface{}) int { + if v == nil { + return 0 + } + + var buf bytes.Buffer + m := v.(map[string]interface{}) + + buf.WriteString(fmt.Sprintf("%s-", m["type"].(string))) + + if v, ok := m["storage_class"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + + return hashcode.String(buf.String()) +} + +func resourceGCSBucketLifecycleRuleConditionHash(v interface{}) int { + if v == nil { + return 0 + } + + var buf bytes.Buffer + m := v.(map[string]interface{}) + + if v, ok := m["age"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + + if v, ok := m["created_before"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + + if v, ok := m["is_live"]; ok { + buf.WriteString(fmt.Sprintf("%t-", v.(bool))) + } + + if v, ok := m["matches_storage_class"]; ok { + matches_storage_classes := v.([]interface{}) + for _, matches_storage_class := range matches_storage_classes { + buf.WriteString(fmt.Sprintf("%s-", matches_storage_class)) + } + } + + if v, ok := m["number_of_newer_versions"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + + return hashcode.String(buf.String()) +} diff --git a/google/resource_storage_bucket_test.go b/google/resource_storage_bucket_test.go index d1055a00001..5f57d03ea9e 100644 --- a/google/resource_storage_bucket_test.go +++ b/google/resource_storage_bucket_test.go @@ -82,6 +82,52 @@ func TestAccStorageBucket_customAttributes(t *testing.T) { }) } +func TestAccStorageBucket_lifecycleRules(t *testing.T) { + var bucket storage.Bucket + bucketName := fmt.Sprintf("tf-test-acc-bucket-%d", acctest.RandInt()) + + hash_step0_lc0_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "SetStorageClass", "storage_class": "NEARLINE"}) + hash_step0_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 2, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + + hash_step0_lc1_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "Delete", "storage_class": ""}) + hash_step0_lc1_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccStorageBucketDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccStorageBucket_lifecycleRules(bucketName), + Check: resource.ComposeTestCheckFunc( + testAccCheckStorageBucketExists( + "google_storage_bucket.bucket", bucketName, &bucket), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#", "2"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.0.action.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.action.%d.type", hash_step0_lc0_action), "SetStorageClass"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.action.%d.storage_class", hash_step0_lc0_action), "NEARLINE"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.0.condition.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.condition.%d.age", hash_step0_lc0_condition), "2"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.1.action.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.action.%d.type", hash_step0_lc1_action), "Delete"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.1.condition.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.condition.%d.age", hash_step0_lc1_condition), "10"), + ), + }, + }, + }) +} + func TestAccStorageBucket_storageClass(t *testing.T) { var bucket storage.Bucket bucketName := fmt.Sprintf("tf-test-acc-bucket-%d", acctest.RandInt()) @@ -128,6 +174,15 @@ func TestAccStorageBucket_update(t *testing.T) { var bucket storage.Bucket bucketName := fmt.Sprintf("tf-test-acl-bucket-%d", acctest.RandInt()) + hash_step2_lc0_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "Delete", "storage_class": ""}) + hash_step2_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + + hash_step3_lc0_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "SetStorageClass", "storage_class": "NEARLINE"}) + hash_step3_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 2, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + + hash_step3_lc1_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "Delete", "storage_class": ""}) + hash_step3_lc1_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "number_of_newer_versions": 2}) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -142,6 +197,81 @@ func TestAccStorageBucket_update(t *testing.T) { "google_storage_bucket.bucket", "location", "US"), resource.TestCheckResourceAttr( "google_storage_bucket.bucket", "force_destroy", "false"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#", "0"), + ), + }, + resource.TestStep{ + Config: testAccStorageBucket_customAttributes(bucketName), + Check: resource.ComposeTestCheckFunc( + testAccCheckStorageBucketExists( + "google_storage_bucket.bucket", bucketName, &bucket), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "predefined_acl", "publicReadWrite"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "location", "EU"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "force_destroy", "true"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#", "0"), + ), + }, + resource.TestStep{ + Config: testAccStorageBucket_customAttributes_withLifecycle1(bucketName), + Check: resource.ComposeTestCheckFunc( + testAccCheckStorageBucketExists( + "google_storage_bucket.bucket", bucketName, &bucket), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "predefined_acl", "publicReadWrite"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "location", "EU"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "force_destroy", "true"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.0.action.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.action.%d.type", hash_step2_lc0_action), "Delete"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.0.condition.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.condition.%d.age", hash_step2_lc0_condition), "10"), + ), + }, + resource.TestStep{ + Config: testAccStorageBucket_customAttributes_withLifecycle2(bucketName), + Check: resource.ComposeTestCheckFunc( + testAccCheckStorageBucketExists( + "google_storage_bucket.bucket", bucketName, &bucket), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "predefined_acl", "publicReadWrite"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "location", "EU"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "force_destroy", "true"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#", "2"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.0.action.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.action.%d.type", hash_step3_lc0_action), "SetStorageClass"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.action.%d.storage_class", hash_step3_lc0_action), "NEARLINE"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.0.condition.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.0.condition.%d.age", hash_step3_lc0_condition), "2"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.1.action.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.action.%d.type", hash_step3_lc1_action), "Delete"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.1.condition.#", "1"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.condition.%d.age", hash_step3_lc1_condition), "10"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.condition.%d.number_of_newer_versions", hash_step3_lc1_condition), "2"), ), }, resource.TestStep{ @@ -155,6 +285,8 @@ func TestAccStorageBucket_update(t *testing.T) { "google_storage_bucket.bucket", "location", "EU"), resource.TestCheckResourceAttr( "google_storage_bucket.bucket", "force_destroy", "true"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#", "0"), ), }, }, @@ -375,6 +507,54 @@ resource "google_storage_bucket" "bucket" { `, bucketName) } +func testAccStorageBucket_customAttributes_withLifecycle1(bucketName string) string { + return fmt.Sprintf(` +resource "google_storage_bucket" "bucket" { + name = "%s" + predefined_acl = "publicReadWrite" + location = "EU" + force_destroy = "true" + lifecycle_rule { + action { + type = "Delete" + } + condition { + age = 10 + } + } +} +`, bucketName) +} + +func testAccStorageBucket_customAttributes_withLifecycle2(bucketName string) string { + return fmt.Sprintf(` +resource "google_storage_bucket" "bucket" { + name = "%s" + predefined_acl = "publicReadWrite" + location = "EU" + force_destroy = "true" + lifecycle_rule { + action { + type = "SetStorageClass" + storage_class = "NEARLINE" + } + condition { + age = 2 + } + } + lifecycle_rule { + action { + type = "Delete" + } + condition { + age = 10 + number_of_newer_versions = 2 + } + } +} +`, bucketName) +} + func testAccStorageBucket_storageClass(bucketName, storageClass, location string) string { var locationBlock string if location != "" { @@ -409,3 +589,28 @@ resource "google_storage_bucket" "bucket" { } `, bucketName) } + +func testAccStorageBucket_lifecycleRules(bucketName string) string { + return fmt.Sprintf(` +resource "google_storage_bucket" "bucket" { + name = "%s" + lifecycle_rule { + action { + type = "SetStorageClass" + storage_class = "NEARLINE" + } + condition { + age = 2 + } + } + lifecycle_rule { + action { + type = "Delete" + } + condition { + age = 10 + } + } +} +`, bucketName) +} From 61eccaeac6bb256cf6a3f4fc8cf3688f322653cd Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 13 Jun 2017 12:32:02 +0200 Subject: [PATCH 02/12] Add documentation provider/google/google_storage_bucket lifecycle interface --- website/docs/r/storage_bucket.html.markdown | 26 +++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/website/docs/r/storage_bucket.html.markdown b/website/docs/r/storage_bucket.html.markdown index fb87f240489..6428cc837b7 100644 --- a/website/docs/r/storage_bucket.html.markdown +++ b/website/docs/r/storage_bucket.html.markdown @@ -56,10 +56,36 @@ to `google_storage_bucket_acl.predefined_acl`. * `storage_class` - (Optional) The [Storage Class](https://cloud.google.com/storage/docs/storage-classes) of the new bucket. Supported values include: `MULTI_REGIONAL`, `REGIONAL`, `NEARLINE`, `COLDLINE`. +* `lifecycle_rule` - (Optional) The bucket's [Lifecycle Rules](https://cloud.google.com/storage/docs/lifecycle#configuration) configuration. Multiple blocks of this type are permitted. Structure is documented below. + * `website` - (Optional) Configuration if the bucket acts as a website. Structure is documented below. * `cors` - (Optional) The bucket's [Cross-Origin Resource Sharing (CORS)](https://www.w3.org/TR/cors/) configuration. Multiple blocks of this type are permitted. Structure is documented below. +The `lifecycle_rule` block supports: + +* `action` - The Lifecycle Rule's action configuration. A single block of this type is supported. Structure is documented below. + +* `condition` - The Lifecycle Rule's condition configuration. A single block of this type is supported. Structure is documented below. + +The `action` block supports: + +* `type` - The type of the action of this Lifecycle Rule. Supported values include: `Delete` and `SetStorageClass`. + +* `storage_class` - (Required if action type is `SetStorageClass`) The target [Storage Class](https://cloud.google.com/storage/docs/storage-classes) of objects affected by this Lifecycle Rule. Supported values include: `MULTI_REGIONAL`, `REGIONAL`, `NEARLINE`, `COLDLINE`. + +The `condition` block supports: + +* `age` - (Optional) Minimum age of an object in days to satisfy this condition. + +* `created_before` - (Optional) Creation date of an object in RFC 3339 (e.g. `2017-06-13`) to satisfy this condition. + +* `is_live` - (Optional) Relevant only for versioned objects. If `true`, this condition matches live objects, archived objects otherwise. + +* `matches_storage_class` - (Optional) [Storage Class](https://cloud.google.com/storage/docs/storage-classes) of objects to satisfy this condition. Supported values include: `MULTI_REGIONAL`, `REGIONAL`, `NEARLINE`, `COLDLINE`, `STANDARD`, `DURABLE_REDUCED_AVAILABILITY`. + +* `number_of_newer_versions` - (Optional) Relevant only for versioned objects. The number of newer versions of an object to satisfy this condition. + The `website` block supports: * `main_page_suffix` - (Optional) Behaves as the bucket's directory index where From 51701d7d9d0f45d188eae78b4eafc6d12fc9f686 Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 20 Jun 2017 19:47:58 +0200 Subject: [PATCH 03/12] Rename number_of_newer_versions as num_newer_versions --- google/resource_storage_bucket.go | 6 +++--- google/resource_storage_bucket_test.go | 14 +++++++------- website/docs/r/storage_bucket.html.markdown | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index 1a7fdc00bb7..dabaaf6039d 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -126,7 +126,7 @@ func resourceStorageBucket() *schema.Resource { Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - "number_of_newer_versions": { + "num_newer_versions": { Type: schema.TypeInt, Optional: true, }, @@ -517,7 +517,7 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag target_lifecycle_rule.Condition.MatchesStorageClass = target_matches_storage_classes } - if v, ok := condition["number_of_newer_versions"]; ok { + if v, ok := condition["num_newer_versions"]; ok { condition_elements++ target_lifecycle_rule.Condition.NumNewerVersions = int64(v.(int)) } @@ -578,7 +578,7 @@ func resourceGCSBucketLifecycleRuleConditionHash(v interface{}) int { } } - if v, ok := m["number_of_newer_versions"]; ok { + if v, ok := m["num_newer_versions"]; ok { buf.WriteString(fmt.Sprintf("%d-", v.(int))) } diff --git a/google/resource_storage_bucket_test.go b/google/resource_storage_bucket_test.go index 5f57d03ea9e..1cb174e8c11 100644 --- a/google/resource_storage_bucket_test.go +++ b/google/resource_storage_bucket_test.go @@ -87,10 +87,10 @@ func TestAccStorageBucket_lifecycleRules(t *testing.T) { bucketName := fmt.Sprintf("tf-test-acc-bucket-%d", acctest.RandInt()) hash_step0_lc0_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "SetStorageClass", "storage_class": "NEARLINE"}) - hash_step0_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 2, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + hash_step0_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 2, "created_before": "", "is_live": false, "num_newer_versions": 0}) hash_step0_lc1_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "Delete", "storage_class": ""}) - hash_step0_lc1_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + hash_step0_lc1_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "num_newer_versions": 0}) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -175,13 +175,13 @@ func TestAccStorageBucket_update(t *testing.T) { bucketName := fmt.Sprintf("tf-test-acl-bucket-%d", acctest.RandInt()) hash_step2_lc0_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "Delete", "storage_class": ""}) - hash_step2_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + hash_step2_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "num_newer_versions": 0}) hash_step3_lc0_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "SetStorageClass", "storage_class": "NEARLINE"}) - hash_step3_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 2, "created_before": "", "is_live": false, "number_of_newer_versions": 0}) + hash_step3_lc0_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 2, "created_before": "", "is_live": false, "num_newer_versions": 0}) hash_step3_lc1_action := resourceGCSBucketLifecycleRuleActionHash(map[string]interface{}{"type": "Delete", "storage_class": ""}) - hash_step3_lc1_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "number_of_newer_versions": 2}) + hash_step3_lc1_condition := resourceGCSBucketLifecycleRuleConditionHash(map[string]interface{}{"age": 10, "created_before": "", "is_live": false, "num_newer_versions": 2}) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -271,7 +271,7 @@ func TestAccStorageBucket_update(t *testing.T) { resource.TestCheckResourceAttr( "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.condition.%d.age", hash_step3_lc1_condition), "10"), resource.TestCheckResourceAttr( - "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.condition.%d.number_of_newer_versions", hash_step3_lc1_condition), "2"), + "google_storage_bucket.bucket", fmt.Sprintf("lifecycle_rule.1.condition.%d.num_newer_versions", hash_step3_lc1_condition), "2"), ), }, resource.TestStep{ @@ -548,7 +548,7 @@ resource "google_storage_bucket" "bucket" { } condition { age = 10 - number_of_newer_versions = 2 + num_newer_versions = 2 } } } diff --git a/website/docs/r/storage_bucket.html.markdown b/website/docs/r/storage_bucket.html.markdown index 6428cc837b7..ee6a2874ab4 100644 --- a/website/docs/r/storage_bucket.html.markdown +++ b/website/docs/r/storage_bucket.html.markdown @@ -84,7 +84,7 @@ The `condition` block supports: * `matches_storage_class` - (Optional) [Storage Class](https://cloud.google.com/storage/docs/storage-classes) of objects to satisfy this condition. Supported values include: `MULTI_REGIONAL`, `REGIONAL`, `NEARLINE`, `COLDLINE`, `STANDARD`, `DURABLE_REDUCED_AVAILABILITY`. -* `number_of_newer_versions` - (Optional) Relevant only for versioned objects. The number of newer versions of an object to satisfy this condition. +* `num_newer_versions` - (Optional) Relevant only for versioned objects. The number of newer versions of an object to satisfy this condition. The `website` block supports: From 90316253157b46aad9b917fcc1b68d46ce375a2d Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 20 Jun 2017 19:50:52 +0200 Subject: [PATCH 04/12] Remove useless `update` param --- google/resource_storage_bucket.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index dabaaf6039d..7469676542e 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -210,7 +210,7 @@ func resourceStorageBucketCreate(d *schema.ResourceData, meta interface{}) error sb.StorageClass = v.(string) } - if err := resourceGCSBucketLifecycleCreateOrUpdate(d, sb, false); err != nil { + if err := resourceGCSBucketLifecycleCreateOrUpdate(d, sb); err != nil { return err } @@ -273,7 +273,7 @@ func resourceStorageBucketUpdate(d *schema.ResourceData, meta interface{}) error sb := &storage.Bucket{} if d.HasChange("lifecycle_rule") { - if err := resourceGCSBucketLifecycleCreateOrUpdate(d, sb, true); err != nil { + if err := resourceGCSBucketLifecycleCreateOrUpdate(d, sb); err != nil { return err } } @@ -453,7 +453,7 @@ func flattenCors(corsRules []*storage.BucketCors) []map[string]interface{} { return corsRulesSchema } -func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storage.Bucket, update bool) error { +func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storage.Bucket) error { if v, ok := d.GetOk("lifecycle_rule"); ok { lifecycle_rules := v.([]interface{}) From 081fa5a1b9236f63aabd926576964c1949a53ece Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 20 Jun 2017 19:55:47 +0200 Subject: [PATCH 05/12] Print number of lifecycle rules passed if there are too many (> 100) --- google/resource_storage_bucket.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index 7469676542e..c3ca512a2a2 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -457,12 +457,13 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag if v, ok := d.GetOk("lifecycle_rule"); ok { lifecycle_rules := v.([]interface{}) - if len(lifecycle_rules) > 100 { - return fmt.Errorf("At most 100 lifecycle_rule blocks are allowed") + len_lifecycle_rules := len(lifecycle_rules) + if len_lifecycle_rules > 100 { + return fmt.Errorf("At most 100 lifecycle_rule blocks are allowed, %d given", len_lifecycle_rules) } sb.Lifecycle = &storage.BucketLifecycle{} - sb.Lifecycle.Rule = make([]*storage.BucketLifecycleRule, 0, len(lifecycle_rules)) + sb.Lifecycle.Rule = make([]*storage.BucketLifecycleRule, 0, len_lifecycle_rules) for _, lifecycle_rule := range lifecycle_rules { lifecycle_rule := lifecycle_rule.(map[string]interface{}) From 2c7d10811c384cba934d0bc123b0d50a7c663b53 Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 20 Jun 2017 20:07:06 +0200 Subject: [PATCH 06/12] Simplify minimum condition elements assertion --- google/resource_storage_bucket.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index c3ca512a2a2..2a8f62d4bd5 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -124,6 +124,7 @@ func resourceStorageBucket() *schema.Resource { "matches_storage_class": { Type: schema.TypeList, Optional: true, + MinItems: 1, Elem: &schema.Schema{Type: schema.TypeString}, }, "num_newer_versions": { @@ -486,22 +487,22 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag if v, ok := lifecycle_rule["condition"]; ok { condition := v.(*schema.Set).List()[0].(map[string]interface{}) - condition_elements := 0 + + if len(condition) < 1 { + return fmt.Errorf("At least one condition element is required") + } target_lifecycle_rule.Condition = &storage.BucketLifecycleRuleCondition{} if v, ok := condition["age"]; ok { - condition_elements++ target_lifecycle_rule.Condition.Age = int64(v.(int)) } if v, ok := condition["created_before"]; ok { - condition_elements++ target_lifecycle_rule.Condition.CreatedBefore = v.(string) } if v, ok := condition["is_live"]; ok { - condition_elements++ target_lifecycle_rule.Condition.IsLive = v.(bool) } @@ -512,21 +513,15 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag for _, v := range matches_storage_classes { target_matches_storage_classes = append(target_matches_storage_classes, v.(string)) - condition_elements++ } target_lifecycle_rule.Condition.MatchesStorageClass = target_matches_storage_classes } if v, ok := condition["num_newer_versions"]; ok { - condition_elements++ target_lifecycle_rule.Condition.NumNewerVersions = int64(v.(int)) } - if condition_elements < 1 { - return fmt.Errorf("At least one condition element is required") - } - sb.Lifecycle.Rule = append(sb.Lifecycle.Rule, target_lifecycle_rule) } } From e4079f199425bd04cea749813fbee7461f8cf42f Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 20 Jun 2017 20:12:30 +0200 Subject: [PATCH 07/12] Clarify documentation regarding the required action and condition blocks, and that at least one condition element has to be defined --- website/docs/r/storage_bucket.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/r/storage_bucket.html.markdown b/website/docs/r/storage_bucket.html.markdown index ee6a2874ab4..c4692a385c8 100644 --- a/website/docs/r/storage_bucket.html.markdown +++ b/website/docs/r/storage_bucket.html.markdown @@ -64,9 +64,9 @@ to `google_storage_bucket_acl.predefined_acl`. The `lifecycle_rule` block supports: -* `action` - The Lifecycle Rule's action configuration. A single block of this type is supported. Structure is documented below. +* `action` - (Required) The Lifecycle Rule's action configuration. A single block of this type is supported. Structure is documented below. -* `condition` - The Lifecycle Rule's condition configuration. A single block of this type is supported. Structure is documented below. +* `condition` - (Required) The Lifecycle Rule's condition configuration. A single block of this type is supported. Structure is documented below. The `action` block supports: @@ -74,7 +74,7 @@ The `action` block supports: * `storage_class` - (Required if action type is `SetStorageClass`) The target [Storage Class](https://cloud.google.com/storage/docs/storage-classes) of objects affected by this Lifecycle Rule. Supported values include: `MULTI_REGIONAL`, `REGIONAL`, `NEARLINE`, `COLDLINE`. -The `condition` block supports: +The `condition` block supports the following elements, and requires at least one to be defined: * `age` - (Optional) Minimum age of an object in days to satisfy this condition. From 31021f7dca33db84562e2c51e50655d816710bd0 Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 20 Jun 2017 20:20:51 +0200 Subject: [PATCH 08/12] Fix acceptance tests --- google/resource_storage_bucket_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/google/resource_storage_bucket_test.go b/google/resource_storage_bucket_test.go index 1cb174e8c11..a0df751b65d 100644 --- a/google/resource_storage_bucket_test.go +++ b/google/resource_storage_bucket_test.go @@ -197,8 +197,8 @@ func TestAccStorageBucket_update(t *testing.T) { "google_storage_bucket.bucket", "location", "US"), resource.TestCheckResourceAttr( "google_storage_bucket.bucket", "force_destroy", "false"), - resource.TestCheckResourceAttr( - "google_storage_bucket.bucket", "lifecycle_rule.#", "0"), + resource.TestCheckNoResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#"), ), }, resource.TestStep{ @@ -212,8 +212,8 @@ func TestAccStorageBucket_update(t *testing.T) { "google_storage_bucket.bucket", "location", "EU"), resource.TestCheckResourceAttr( "google_storage_bucket.bucket", "force_destroy", "true"), - resource.TestCheckResourceAttr( - "google_storage_bucket.bucket", "lifecycle_rule.#", "0"), + resource.TestCheckNoResourceAttr( + "google_storage_bucket.bucket", "lifecycle_rule.#"), ), }, resource.TestStep{ From 7c74709a4c82b1367d1f1a53aef1cf3442664115 Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 4 Jul 2017 13:12:59 +0200 Subject: [PATCH 09/12] Avoid shadowing lifecycle_rule variable, rename unconverted one to raw_lifecycle_rule --- google/resource_storage_bucket.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index 2a8f62d4bd5..226dfcf8a6a 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -466,8 +466,8 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag sb.Lifecycle = &storage.BucketLifecycle{} sb.Lifecycle.Rule = make([]*storage.BucketLifecycleRule, 0, len_lifecycle_rules) - for _, lifecycle_rule := range lifecycle_rules { - lifecycle_rule := lifecycle_rule.(map[string]interface{}) + for _, raw_lifecycle_rule := range lifecycle_rules { + lifecycle_rule := raw_lifecycle_rule.(map[string]interface{}) target_lifecycle_rule := &storage.BucketLifecycleRule{} From 2b775a6398455d603c8db1cbb8957d0e979ad648 Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 4 Jul 2017 13:21:56 +0200 Subject: [PATCH 10/12] Implement limit on lifecycle_rules using MaxItems in schema --- google/resource_storage_bucket.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index 226dfcf8a6a..5e0ec39669f 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -82,6 +82,7 @@ func resourceStorageBucket() *schema.Resource { "lifecycle_rule": { Type: schema.TypeList, Optional: true, + MaxItems: 100, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "action": { @@ -458,13 +459,8 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag if v, ok := d.GetOk("lifecycle_rule"); ok { lifecycle_rules := v.([]interface{}) - len_lifecycle_rules := len(lifecycle_rules) - if len_lifecycle_rules > 100 { - return fmt.Errorf("At most 100 lifecycle_rule blocks are allowed, %d given", len_lifecycle_rules) - } - sb.Lifecycle = &storage.BucketLifecycle{} - sb.Lifecycle.Rule = make([]*storage.BucketLifecycleRule, 0, len_lifecycle_rules) + sb.Lifecycle.Rule = make([]*storage.BucketLifecycleRule, 0, len(lifecycle_rules)) for _, raw_lifecycle_rule := range lifecycle_rules { lifecycle_rule := raw_lifecycle_rule.(map[string]interface{}) From 8332b4aeb4a508a2e72797fb7314c9e5fc5c7cd1 Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Tue, 4 Jul 2017 13:56:57 +0200 Subject: [PATCH 11/12] Action and condition blocks of a lifecycle_rule block are both required, use MinItems in schema to enforce --- google/resource_storage_bucket.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index 5e0ec39669f..732bfab32cd 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -88,6 +88,7 @@ func resourceStorageBucket() *schema.Resource { "action": { Type: schema.TypeSet, Required: true, + MinItems: 1, MaxItems: 1, Set: resourceGCSBucketLifecycleRuleActionHash, Elem: &schema.Resource{ @@ -106,6 +107,7 @@ func resourceStorageBucket() *schema.Resource { "condition": { Type: schema.TypeSet, Required: true, + MinItems: 1, MaxItems: 1, Set: resourceGCSBucketLifecycleRuleConditionHash, Elem: &schema.Resource{ @@ -484,10 +486,6 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag if v, ok := lifecycle_rule["condition"]; ok { condition := v.(*schema.Set).List()[0].(map[string]interface{}) - if len(condition) < 1 { - return fmt.Errorf("At least one condition element is required") - } - target_lifecycle_rule.Condition = &storage.BucketLifecycleRuleCondition{} if v, ok := condition["age"]; ok { From 1bb145bb401033d637aa75ec4a0343073f91356a Mon Sep 17 00:00:00 2001 From: Patrick Decat Date: Mon, 17 Jul 2017 11:46:17 +0200 Subject: [PATCH 12/12] Add checks on actions and conditions lists lengths --- google/resource_storage_bucket.go | 68 +++++++++++++++++-------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/google/resource_storage_bucket.go b/google/resource_storage_bucket.go index 732bfab32cd..c355a608dff 100644 --- a/google/resource_storage_bucket.go +++ b/google/resource_storage_bucket.go @@ -470,54 +470,62 @@ func resourceGCSBucketLifecycleCreateOrUpdate(d *schema.ResourceData, sb *storag target_lifecycle_rule := &storage.BucketLifecycleRule{} if v, ok := lifecycle_rule["action"]; ok { - action := v.(*schema.Set).List()[0].(map[string]interface{}) + if actions := v.(*schema.Set).List(); len(actions) == 1 { + action := actions[0].(map[string]interface{}) - target_lifecycle_rule.Action = &storage.BucketLifecycleRuleAction{} + target_lifecycle_rule.Action = &storage.BucketLifecycleRuleAction{} - if v, ok := action["type"]; ok { - target_lifecycle_rule.Action.Type = v.(string) - } + if v, ok := action["type"]; ok { + target_lifecycle_rule.Action.Type = v.(string) + } - if v, ok := action["storage_class"]; ok { - target_lifecycle_rule.Action.StorageClass = v.(string) + if v, ok := action["storage_class"]; ok { + target_lifecycle_rule.Action.StorageClass = v.(string) + } + } else { + return fmt.Errorf("Exactly one action is required") } } if v, ok := lifecycle_rule["condition"]; ok { - condition := v.(*schema.Set).List()[0].(map[string]interface{}) + if conditions := v.(*schema.Set).List(); len(conditions) == 1 { + condition := conditions[0].(map[string]interface{}) - target_lifecycle_rule.Condition = &storage.BucketLifecycleRuleCondition{} + target_lifecycle_rule.Condition = &storage.BucketLifecycleRuleCondition{} - if v, ok := condition["age"]; ok { - target_lifecycle_rule.Condition.Age = int64(v.(int)) - } + if v, ok := condition["age"]; ok { + target_lifecycle_rule.Condition.Age = int64(v.(int)) + } - if v, ok := condition["created_before"]; ok { - target_lifecycle_rule.Condition.CreatedBefore = v.(string) - } + if v, ok := condition["created_before"]; ok { + target_lifecycle_rule.Condition.CreatedBefore = v.(string) + } - if v, ok := condition["is_live"]; ok { - target_lifecycle_rule.Condition.IsLive = v.(bool) - } + if v, ok := condition["is_live"]; ok { + target_lifecycle_rule.Condition.IsLive = v.(bool) + } - if v, ok := condition["matches_storage_class"]; ok { - matches_storage_classes := v.([]interface{}) + if v, ok := condition["matches_storage_class"]; ok { + matches_storage_classes := v.([]interface{}) - target_matches_storage_classes := make([]string, 0, len(matches_storage_classes)) + target_matches_storage_classes := make([]string, 0, len(matches_storage_classes)) - for _, v := range matches_storage_classes { - target_matches_storage_classes = append(target_matches_storage_classes, v.(string)) - } + for _, v := range matches_storage_classes { + target_matches_storage_classes = append(target_matches_storage_classes, v.(string)) + } - target_lifecycle_rule.Condition.MatchesStorageClass = target_matches_storage_classes - } + target_lifecycle_rule.Condition.MatchesStorageClass = target_matches_storage_classes + } - if v, ok := condition["num_newer_versions"]; ok { - target_lifecycle_rule.Condition.NumNewerVersions = int64(v.(int)) + if v, ok := condition["num_newer_versions"]; ok { + target_lifecycle_rule.Condition.NumNewerVersions = int64(v.(int)) + } + } else { + return fmt.Errorf("Exactly one condition is required") } - - sb.Lifecycle.Rule = append(sb.Lifecycle.Rule, target_lifecycle_rule) } + + sb.Lifecycle.Rule = append(sb.Lifecycle.Rule, target_lifecycle_rule) } }