Fix Adding Unexpected Conditions (#9547) (#16683)

* Fix Adding Unexpected Conditions

* Adds no_age field

* Modify no_age description and variables name.

Adds relevant comments.

* Fixes Build Failure

* Fixes indent issues and removes unnecessary condition.

* Modified condition to include case when no_age is not present in the file.
[upstream:cc5606ed9072cb2cea144c1769d601d71bdc8ed1]

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/9547.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+storage: fixed unexpected `lifecycle_rule` conditions being added for `google_storage_bucket`
+```

google/services/storage/resource_storage_bucket.go

@@ -218,6 +218,11 @@ func ResourceStorageBucket() *schema.Resource {
 										Optional:    true,
 										Description: `Creation date of an object in RFC 3339 (e.g. 2017-06-13) to satisfy this condition.`,
 									},
+									"no_age": {
+										Type:        schema.TypeBool,
+										Optional:    true,
+										Description: `While set true, age value will be omitted.Required to set true when age is unset in the config file.`,
+									},
 									"with_state": {
 										Type:         schema.TypeString,
 										Computed:     true,
@@ -1207,7 +1212,7 @@ func flattenBucketAutoclass(bucketAutoclass *storage.BucketAutoclass) []map[stri
 	return autoclassList
 }
 
-func flattenBucketLifecycle(lifecycle *storage.BucketLifecycle) []map[string]interface{} {
+func flattenBucketLifecycle(d *schema.ResourceData, lifecycle *storage.BucketLifecycle) []map[string]interface{} {
 	if lifecycle == nil || lifecycle.Rule == nil {
 		return []map[string]interface{}{}
 	}
@@ -1217,7 +1222,7 @@ func flattenBucketLifecycle(lifecycle *storage.BucketLifecycle) []map[string]int
 	for _, rule := range lifecycle.Rule {
 		rules = append(rules, map[string]interface{}{
 			"action":    schema.NewSet(resourceGCSBucketLifecycleRuleActionHash, []interface{}{flattenBucketLifecycleRuleAction(rule.Action)}),
-			"condition": schema.NewSet(resourceGCSBucketLifecycleRuleConditionHash, []interface{}{flattenBucketLifecycleRuleCondition(rule.Condition)}),
+			"condition": schema.NewSet(resourceGCSBucketLifecycleRuleConditionHash, []interface{}{flattenBucketLifecycleRuleCondition(d, rule.Condition)}),
 		})
 	}
 
@@ -1231,7 +1236,7 @@ func flattenBucketLifecycleRuleAction(action *storage.BucketLifecycleRuleAction)
 	}
 }
 
-func flattenBucketLifecycleRuleCondition(condition *storage.BucketLifecycleRuleCondition) map[string]interface{} {
+func flattenBucketLifecycleRuleCondition(d *schema.ResourceData, condition *storage.BucketLifecycleRuleCondition) map[string]interface{} {
 	ruleCondition := map[string]interface{}{
 		"created_before":             condition.CreatedBefore,
 		"matches_storage_class":      tpgresource.ConvertStringArrToInterface(condition.MatchesStorageClass),
@@ -1255,6 +1260,12 @@ func flattenBucketLifecycleRuleCondition(condition *storage.BucketLifecycleRuleC
 			ruleCondition["with_state"] = "ARCHIVED"
 		}
 	}
+	// setting no_age value from state config since it is terraform only variable and not getting value from backend.
+	if v, ok := d.GetOk("lifecycle_rule.0.condition"); ok {
+		state_condition := v.(*schema.Set).List()[0].(map[string]interface{})
+		ruleCondition["no_age"] = state_condition["no_age"].(bool)
+	}
+
 	return ruleCondition
 }
 
@@ -1402,11 +1413,14 @@ func expandStorageBucketLifecycleRuleCondition(v interface{}) (*storage.BucketLi
 
 	condition := conditions[0].(map[string]interface{})
 	transformed := &storage.BucketLifecycleRuleCondition{}
-
-	if v, ok := condition["age"]; ok {
-		age := int64(v.(int))
-		transformed.Age = &age
-		transformed.ForceSendFields = append(transformed.ForceSendFields, "Age")
+	// Setting high precedence of no_age over age when both used together.
+	// Only sets age value when no_age is not present or no_age is present and has false value
+	if v, ok := condition["no_age"]; !ok || !(v.(bool)) {
+		if v, ok := condition["age"]; ok {
+			age := int64(v.(int))
+			transformed.Age = &age
+			transformed.ForceSendFields = append(transformed.ForceSendFields, "Age")
+		}
 	}
 
 	if v, ok := condition["created_before"]; ok {
@@ -1507,8 +1521,12 @@ func resourceGCSBucketLifecycleRuleConditionHash(v interface{}) int {
 	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["no_age"]; ok && v.(bool) {
+		buf.WriteString(fmt.Sprintf("%t-", v.(bool)))
+	} else {
+		if v, ok := m["age"]; ok {
+			buf.WriteString(fmt.Sprintf("%d-", v.(int)))
+		}
 	}
 
 	if v, ok := m["days_since_custom_time"]; ok {
@@ -1651,7 +1669,9 @@ func setStorageBucket(d *schema.ResourceData, config *transport_tpg.Config, res
 	if err := d.Set("autoclass", flattenBucketAutoclass(res.Autoclass)); err != nil {
 		return fmt.Errorf("Error setting autoclass: %s", err)
 	}
-	if err := d.Set("lifecycle_rule", flattenBucketLifecycle(res.Lifecycle)); err != nil {
+	// lifecycle_rule contains terraform only variable no_age.
+	// Passing config("d") to flattener function to set no_age separately.
+	if err := d.Set("lifecycle_rule", flattenBucketLifecycle(d, res.Lifecycle)); err != nil {
 		return fmt.Errorf("Error setting lifecycle_rule: %s", err)
 	}
 	if err := tpgresource.SetLabels(res.Labels, d, "labels"); err != nil {

google/services/storage/resource_storage_bucket_test.go

@@ -382,6 +382,74 @@ func TestAccStorageBucket_lifecycleRuleStateAny(t *testing.T) {
 	})
 }
 
+func TestAccStorageBucket_lifecycleRulesNoAge(t *testing.T) {
+	t.Parallel()
+	var bucket storage.Bucket
+	bucketName := acctest.TestBucketName(t)
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccStorageBucketDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccStorageBucket_customAttributes_withLifecycle1(bucketName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStorageBucketExists(
+						t, "google_storage_bucket.bucket", bucketName, &bucket),
+				),
+			},
+			{
+				ResourceName:            "google_storage_bucket.bucket",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"force_destroy"},
+			},
+			{
+				Config: testAccStorageBucket_customAttributes_withLifecycleNoAge(bucketName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStorageBucketExists(
+						t, "google_storage_bucket.bucket", bucketName, &bucket),
+					testAccCheckStorageBucketLifecycleConditionNoAge(nil, &bucket),
+				),
+			},
+			{
+				ResourceName:            "google_storage_bucket.bucket",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"force_destroy", "lifecycle_rule.0.condition.0.no_age"},
+			},
+			{
+				Config: testAccStorageBucket_customAttributes_withLifecycleNoAgeAndAge(bucketName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStorageBucketExists(
+						t, "google_storage_bucket.bucket", bucketName, &bucket),
+					testAccCheckStorageBucketLifecycleConditionNoAge(nil, &bucket),
+				),
+			},
+			{
+				ResourceName:            "google_storage_bucket.bucket",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"force_destroy", "lifecycle_rule.0.condition.0.no_age"},
+			},
+			{
+				Config: testAccStorageBucket_customAttributes_withLifecycle1(bucketName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStorageBucketExists(
+						t, "google_storage_bucket.bucket", bucketName, &bucket),
+				),
+			},
+			{
+				ResourceName:            "google_storage_bucket.bucket",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"force_destroy"},
+			},
+		},
+	})
+}
+
 func TestAccStorageBucket_storageClass(t *testing.T) {
 	t.Parallel()
 
@@ -1316,6 +1384,25 @@ func testAccCheckStorageBucketLifecycleConditionState(expected *bool, b *storage
 	}
 }
 
+func testAccCheckStorageBucketLifecycleConditionNoAge(expected *int64, b *storage.Bucket) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		actual := b.Lifecycle.Rule[0].Condition.Age
+		if expected == nil && b.Lifecycle.Rule[0].Condition.Age == nil {
+			return nil
+		}
+		if expected == nil {
+			return fmt.Errorf("expected condition Age to be unset, instead got %d", *actual)
+		}
+		if actual == nil {
+			return fmt.Errorf("expected condition Age to be %d, instead got nil (unset)", *expected)
+		}
+		if *expected != *actual {
+			return fmt.Errorf("expected condition Age to be %d, instead got %d", *expected, *actual)
+		}
+		return nil
+	}
+}
+
 func testAccStorageBucketDestroyProducer(t *testing.T) func(s *terraform.State) error {
 	return func(s *terraform.State) error {
 		config := acctest.GoogleProviderConfig(t)
@@ -1479,6 +1566,45 @@ resource "google_storage_bucket" "bucket" {
 `, bucketName)
 }
 
+func testAccStorageBucket_customAttributes_withLifecycleNoAge(bucketName string) string {
+	return fmt.Sprintf(`
+resource "google_storage_bucket" "bucket" {
+  name          = "%s"
+  location      = "EU"
+  force_destroy = "true"
+  lifecycle_rule {
+    action {
+      type = "Delete"
+    }
+    condition {
+      num_newer_versions = 2
+      no_age = true
+    }
+  }
+}
+`, bucketName)
+}
+
+func testAccStorageBucket_customAttributes_withLifecycleNoAgeAndAge(bucketName string) string {
+	return fmt.Sprintf(`
+resource "google_storage_bucket" "bucket" {
+  name          = "%s"
+  location      = "EU"
+  force_destroy = "true"
+  lifecycle_rule {
+    action {
+      type = "Delete"
+    }
+    condition {
+      num_newer_versions = 2
+      age = 10
+      no_age = true
+    }
+  }
+}
+`, bucketName)
+}
+
 func testAccStorageBucket_storageClass(bucketName, storageClass, location string) string {
 	return fmt.Sprintf(`
 resource "google_storage_bucket" "bucket" {

website/docs/r/storage_bucket.html.markdown

@@ -147,6 +147,8 @@ The following arguments are supported:
 
 * `age` - (Optional) Minimum age of an object in days to satisfy this condition.
 
+* `no_age` - (Optional) While set `true`, `age` value will be omitted. **Note** Required to set `true` when `age` is unset in the config file.  
+
 * `created_before` - (Optional) A date in the RFC 3339 format YYYY-MM-DD. This condition is satisfied when an object is created before midnight of the specified date in UTC.
 
 * `with_state` - (Optional) Match to live and/or archived objects. Unversioned buckets have only live objects. Supported values include: `"LIVE"`, `"ARCHIVED"`, `"ANY"`.