Add customize diff for deprecated attribute on gc_policy (#4556) (#3037)

Signed-off-by: Modular Magician <magic-modules@google.com>

.changelog/4556.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+bigtable: fixed bug where gc_policy would attempt to recreate the resource when switching from deprecated attribute but maintaining the same value underlying value
+```

google-beta/resource_bigtable_gc_policy.go

@@ -16,11 +16,50 @@ const (
 	GCPolicyModeUnion        = "UNION"
 )
 
+func resourceBigtableGCPolicyCustomizeDiffFunc(diff TerraformResourceDiff) error {
+	count := diff.Get("max_age.#").(int)
+	if count < 1 {
+		return nil
+	}
+
+	oldDays, newDays := diff.GetChange("max_age.0.days")
+	oldDuration, newDuration := diff.GetChange("max_age.0.duration")
+	log.Printf("days: %v %v", oldDays, newDays)
+	log.Printf("duration: %v %v", oldDuration, newDuration)
+
+	if oldDuration == "" && newDuration != "" {
+		// flatten the old days and the new duration to duration... if they are
+		// equal then do nothing.
+		do, err := time.ParseDuration(newDuration.(string))
+		if err != nil {
+			return err
+		}
+		dn := time.Hour * 24 * time.Duration(oldDays.(int))
+		if do == dn {
+			err := diff.Clear("max_age.0.days")
+			if err != nil {
+				return err
+			}
+			err = diff.Clear("max_age.0.duration")
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
+func resourceBigtableGCPolicyCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
+	return resourceBigtableGCPolicyCustomizeDiffFunc(d)
+}
+
 func resourceBigtableGCPolicy() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceBigtableGCPolicyCreate,
-		Read:   resourceBigtableGCPolicyRead,
-		Delete: resourceBigtableGCPolicyDestroy,
+		Create:        resourceBigtableGCPolicyCreate,
+		Read:          resourceBigtableGCPolicyRead,
+		Delete:        resourceBigtableGCPolicyDestroy,
+		CustomizeDiff: resourceBigtableGCPolicyCustomizeDiff,
 
 		Schema: map[string]*schema.Schema{
 			"instance_name": {
@@ -64,6 +103,7 @@ func resourceBigtableGCPolicy() *schema.Resource {
 						"days": {
 							Type:         schema.TypeInt,
 							Optional:     true,
+							Computed:     true,
 							ForceNew:     true,
 							Deprecated:   "Deprecated in favor of duration",
 							Description:  `Number of days before applying GC policy.`,
@@ -72,6 +112,7 @@ func resourceBigtableGCPolicy() *schema.Resource {
 						"duration": {
 							Type:         schema.TypeString,
 							Optional:     true,
+							Computed:     true,
 							ForceNew:     true,
 							Description:  `Duration before applying GC policy`,
 							ValidateFunc: validateDuration(),

google-beta/resource_bigtable_gc_policy_test.go

@@ -34,6 +34,38 @@ func TestAccBigtableGCPolicy_basic(t *testing.T) {
 	})
 }
 
+func TestAccBigtableGCPolicy_swapOffDeprecated(t *testing.T) {
+	// bigtable instance does not use the shared HTTP client, this test creates an instance
+	skipIfVcr(t)
+	t.Parallel()
+
+	instanceName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+	tableName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+	familyName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckBigtableGCPolicyDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccBigtableGCPolicy_days(instanceName, tableName, familyName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccBigtableGCPolicyExists(
+						t, "google_bigtable_gc_policy.policy"),
+				),
+			},
+			{
+				Config: testAccBigtableGCPolicy(instanceName, tableName, familyName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccBigtableGCPolicyExists(
+						t, "google_bigtable_gc_policy.policy"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccBigtableGCPolicy_union(t *testing.T) {
 	// bigtable instance does not use the shared HTTP client, this test creates an instance
 	skipIfVcr(t)
@@ -59,6 +91,82 @@ func TestAccBigtableGCPolicy_union(t *testing.T) {
 	})
 }
 
+func TestUnitBigtableGCPolicy_customizeDiff(t *testing.T) {
+	for _, tc := range testUnitBigtableGCPolicyCustomizeDiffTestcases {
+		tc.check(t)
+	}
+}
+
+func (testcase *testUnitBigtableGCPolicyCustomizeDiffTestcase) check(t *testing.T) {
+	d := &ResourceDiffMock{
+		Before: map[string]interface{}{},
+		After:  map[string]interface{}{},
+	}
+
+	d.Before["max_age.0.days"] = testcase.oldDays
+	d.Before["max_age.0.duration"] = testcase.oldDuration
+
+	d.After["max_age.#"] = testcase.arraySize
+	d.After["max_age.0.days"] = testcase.newDays
+	d.After["max_age.0.duration"] = testcase.newDuration
+
+	err := resourceBigtableGCPolicyCustomizeDiffFunc(d)
+	if err != nil {
+		t.Errorf("error on testcase %s - %w", testcase.testName, err)
+	}
+
+	var cleared bool = d.Cleared != nil && d.Cleared["max_age.0.duration"] == true && d.Cleared["max_age.0.days"] == true
+	if cleared != testcase.cleared {
+		t.Errorf("%s: expected diff clear to be %v, but was %v", testcase.testName, testcase.cleared, cleared)
+	}
+}
+
+type testUnitBigtableGCPolicyCustomizeDiffTestcase struct {
+	testName    string
+	arraySize   int
+	oldDays     int
+	newDays     int
+	oldDuration string
+	newDuration string
+	cleared     bool
+}
+
+var testUnitBigtableGCPolicyCustomizeDiffTestcases = []testUnitBigtableGCPolicyCustomizeDiffTestcase{
+	{
+		testName:  "ArraySize0",
+		arraySize: 0,
+		cleared:   false,
+	},
+	{
+		testName:  "DaysChange",
+		arraySize: 1,
+		oldDays:   3,
+		newDays:   2,
+		cleared:   false,
+	},
+	{
+		testName:    "DurationChanges",
+		arraySize:   1,
+		oldDuration: "3h",
+		newDuration: "4h",
+		cleared:     false,
+	},
+	{
+		testName:    "DaysToDurationEq",
+		arraySize:   1,
+		oldDays:     3,
+		newDuration: "72h",
+		cleared:     true,
+	},
+	{
+		testName:    "DaysToDurationNotEq",
+		arraySize:   1,
+		oldDays:     3,
+		newDuration: "70h",
+		cleared:     false,
+	},
+}
+
 func testAccCheckBigtableGCPolicyDestroyProducer(t *testing.T) func(s *terraform.State) error {
 	return func(s *terraform.State) error {
 		var ctx = context.Background()
@@ -129,6 +237,41 @@ func testAccBigtableGCPolicyExists(t *testing.T, n string) resource.TestCheckFun
 	}
 }
 
+func testAccBigtableGCPolicy_days(instanceName, tableName, family string) string {
+	return fmt.Sprintf(`
+resource "google_bigtable_instance" "instance" {
+  name = "%s"
+
+  cluster {
+    cluster_id = "%s"
+    zone       = "us-central1-b"
+  }
+
+  instance_type = "DEVELOPMENT"
+  deletion_protection = false
+}
+
+resource "google_bigtable_table" "table" {
+  name          = "%s"
+  instance_name = google_bigtable_instance.instance.id
+
+  column_family {
+    family = "%s"
+  }
+}
+
+resource "google_bigtable_gc_policy" "policy" {
+  instance_name = google_bigtable_instance.instance.id
+  table         = google_bigtable_table.table.name
+  column_family = "%s"
+
+  max_age {
+    days = 3
+  }
+}
+`, instanceName, instanceName, tableName, family, family)
+}
+
 func testAccBigtableGCPolicy(instanceName, tableName, family string) string {
 	return fmt.Sprintf(`
 resource "google_bigtable_instance" "instance" {

google-beta/test_utils.go

@@ -74,7 +74,7 @@ func (d *ResourceDataMock) Timeout(key string) time.Duration {
 type ResourceDiffMock struct {
 	Before     map[string]interface{}
 	After      map[string]interface{}
-	Cleared    map[string]struct{}
+	Cleared    map[string]interface{}
 	IsForceNew bool
 }
 
@@ -98,9 +98,9 @@ func (d *ResourceDiffMock) GetOk(key string) (interface{}, bool) {
 
 func (d *ResourceDiffMock) Clear(key string) error {
 	if d.Cleared == nil {
-		d.Cleared = map[string]struct{}{}
+		d.Cleared = map[string]interface{}{}
 	}
-	d.Cleared[key] = struct{}{}
+	d.Cleared[key] = true
 	return nil
 }