Skip to content

Commit 3b0ae51

Browse files
modular-magicianmodular-magician
authored
Bigtable: Update local gc_rules on read (#6526) (#12568)
* Update local gc_rules on read * Early return + add comment * Check list length * Handle empty GC policy Signed-off-by: Modular Magician <[email protected]> Signed-off-by: Modular Magician <[email protected]>
1 parent 0d68b7e commit 3b0ae51

File tree

3 files changed

+213
-17
lines changed

3 files changed

+213
-17
lines changed

‎.changelog/6526.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
bigtable: added drift detection on `gc_rules` for `google_bigtable_gc_policy`
3+
```

‎google/resource_bigtable_gc_policy.go

+90-3
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,32 @@ func resourceBigtableGCPolicyRead(d *schema.ResourceData, meta interface{}) erro
255255
}
256256

257257
for _, fi := range ti.FamilyInfos {
258-
if fi.Name == columnFamily {
259-
d.SetId(fi.GCPolicy)
260-
break
258+
if fi.Name != columnFamily {
259+
continue
261260
}
261+
262+
d.SetId(fi.GCPolicy)
263+
264+
// No GC Policy.
265+
if fi.FullGCPolicy.String() == "" {
266+
return nil
267+
}
268+
269+
// Only set `gc_rules`` when the legacy fields are not set. We are not planning to support legacy fields.
270+
maxAge := d.Get("max_age")
271+
maxVersion := d.Get("max_version")
272+
if d.Get("mode") == "" && len(maxAge.([]interface{})) == 0 && len(maxVersion.([]interface{})) == 0 {
273+
gcRuleString, err := gcPolicyToGCRuleString(fi.FullGCPolicy, true)
274+
if err != nil {
275+
return err
276+
}
277+
gcRuleJsonString, err := json.Marshal(gcRuleString)
278+
if err != nil {
279+
return fmt.Errorf("Error marshaling GC policy to json: %s", err)
280+
}
281+
d.Set("gc_rules", string(gcRuleJsonString))
282+
}
283+
break
262284
}
263285

264286
if err := d.Set("project", project); err != nil {
@@ -268,6 +290,71 @@ func resourceBigtableGCPolicyRead(d *schema.ResourceData, meta interface{}) erro
268290
return nil
269291
}
270292

293+
// Recursively convert Bigtable GC policy to JSON format in a map.
294+
func gcPolicyToGCRuleString(gc bigtable.GCPolicy, topLevel bool) (map[string]interface{}, error) {
295+
result := make(map[string]interface{})
296+
switch bigtable.GetPolicyType(gc) {
297+
case bigtable.PolicyMaxAge:
298+
age := gc.(bigtable.MaxAgeGCPolicy).GetDurationString()
299+
if topLevel {
300+
rule := make(map[string]interface{})
301+
rule["max_age"] = age
302+
rules := []interface{}{}
303+
rules = append(rules, rule)
304+
result["rules"] = rules
305+
} else {
306+
result["max_age"] = age
307+
}
308+
break
309+
case bigtable.PolicyMaxVersion:
310+
// bigtable.MaxVersionsGCPolicy is an int.
311+
// Not sure why max_version is a float64.
312+
// TODO: Maybe change max_version to an int.
313+
version := float64(int(gc.(bigtable.MaxVersionsGCPolicy)))
314+
if topLevel {
315+
rule := make(map[string]interface{})
316+
rule["max_version"] = version
317+
rules := []interface{}{}
318+
rules = append(rules, rule)
319+
result["rules"] = rules
320+
} else {
321+
result["max_version"] = version
322+
}
323+
break
324+
case bigtable.PolicyUnion:
325+
result["mode"] = "union"
326+
rules := []interface{}{}
327+
for _, c := range gc.(bigtable.UnionGCPolicy).Children {
328+
gcRuleString, err := gcPolicyToGCRuleString(c, false)
329+
if err != nil {
330+
return nil, err
331+
}
332+
rules = append(rules, gcRuleString)
333+
}
334+
result["rules"] = rules
335+
break
336+
case bigtable.PolicyIntersection:
337+
result["mode"] = "intersection"
338+
rules := []interface{}{}
339+
for _, c := range gc.(bigtable.IntersectionGCPolicy).Children {
340+
gcRuleString, err := gcPolicyToGCRuleString(c, false)
341+
if err != nil {
342+
return nil, err
343+
}
344+
rules = append(rules, gcRuleString)
345+
}
346+
result["rules"] = rules
347+
default:
348+
break
349+
}
350+
351+
if err := validateNestedPolicy(result, topLevel); err != nil {
352+
return nil, err
353+
}
354+
355+
return result, nil
356+
}
357+
271358
func resourceBigtableGCPolicyDestroy(d *schema.ResourceData, meta interface{}) error {
272359
config := meta.(*Config)
273360
userAgent, err := generateUserAgentString(d, config.userAgent)

‎google/resource_bigtable_gc_policy_test.go

+120-14
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"testing"
8+
"time"
89

910
"cloud.google.com/go/bigtable"
1011
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -29,7 +30,7 @@ func TestAccBigtableGCPolicy_basic(t *testing.T) {
2930
Config: testAccBigtableGCPolicy(instanceName, tableName, familyName),
3031
Check: resource.ComposeTestCheckFunc(
3132
testAccBigtableGCPolicyExists(
32-
t, "google_bigtable_gc_policy.policy"),
33+
t, "google_bigtable_gc_policy.policy", false),
3334
),
3435
},
3536
},
@@ -54,7 +55,7 @@ func TestAccBigtableGCPolicy_swapOffDeprecated(t *testing.T) {
5455
Config: testAccBigtableGCPolicy_days(instanceName, tableName, familyName),
5556
Check: resource.ComposeTestCheckFunc(
5657
testAccBigtableGCPolicyExists(
57-
t, "google_bigtable_gc_policy.policy"),
58+
t, "google_bigtable_gc_policy.policy", false),
5859
// Verify can write some data.
5960
testAccBigtableCanWriteData(
6061
t, "google_bigtable_gc_policy.policy", 10),
@@ -64,7 +65,7 @@ func TestAccBigtableGCPolicy_swapOffDeprecated(t *testing.T) {
6465
Config: testAccBigtableGCPolicy(instanceName, tableName, familyName),
6566
Check: resource.ComposeTestCheckFunc(
6667
testAccBigtableGCPolicyExists(
67-
t, "google_bigtable_gc_policy.policy"),
68+
t, "google_bigtable_gc_policy.policy", false),
6869
// Verify no data loss after the GC policy update.
6970
testAccBigtableCanReadData(
7071
t, "google_bigtable_gc_policy.policy", 10),
@@ -92,7 +93,7 @@ func TestAccBigtableGCPolicy_union(t *testing.T) {
9293
Config: testAccBigtableGCPolicyUnion(instanceName, tableName, familyName),
9394
Check: resource.ComposeTestCheckFunc(
9495
testAccBigtableGCPolicyExists(
95-
t, "google_bigtable_gc_policy.policy"),
96+
t, "google_bigtable_gc_policy.policy", false),
9697
),
9798
},
9899
},
@@ -118,11 +119,11 @@ func TestAccBigtableGCPolicy_multiplePolicies(t *testing.T) {
118119
Config: testAccBigtableGCPolicy_multiplePolicies(instanceName, tableName, familyName),
119120
Check: resource.ComposeTestCheckFunc(
120121
testAccBigtableGCPolicyExists(
121-
t, "google_bigtable_gc_policy.policyA"),
122+
t, "google_bigtable_gc_policy.policyA", false),
122123
testAccBigtableGCPolicyExists(
123-
t, "google_bigtable_gc_policy.policyB"),
124+
t, "google_bigtable_gc_policy.policyB", false),
124125
testAccBigtableGCPolicyExists(
125-
t, "google_bigtable_gc_policy.policyC"),
126+
t, "google_bigtable_gc_policy.policyC", false),
126127
),
127128
},
128129
},
@@ -148,7 +149,7 @@ func TestAccBigtableGCPolicy_gcRulesPolicy(t *testing.T) {
148149
{
149150
Config: testAccBigtableGCPolicy_gcRulesCreate(instanceName, tableName, familyName),
150151
Check: resource.ComposeTestCheckFunc(
151-
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy"),
152+
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy", true),
152153
resource.TestCheckResourceAttr("google_bigtable_gc_policy.policy", "gc_rules", gcRulesOriginal),
153154
),
154155
},
@@ -157,7 +158,7 @@ func TestAccBigtableGCPolicy_gcRulesPolicy(t *testing.T) {
157158
{
158159
Config: testAccBigtableGCPolicy_gcRulesUpdate(instanceName, tableName, familyName),
159160
Check: resource.ComposeTestCheckFunc(
160-
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy"),
161+
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy", true),
161162
resource.TestCheckResourceAttr("google_bigtable_gc_policy.policy", "gc_rules", gcRulesUpdate),
162163
),
163164
},
@@ -293,7 +294,7 @@ func TestUnitBigtableGCPolicy_getGCPolicyFromJSON(t *testing.T) {
293294
t.Fatalf("unexpected error: %v", err)
294295
} else {
295296
if got != nil && got.String() != tc.want {
296-
t.Errorf("error getting policy from JSON, got: %v, want: %v", tc.want, got)
297+
t.Errorf("error getting policy from JSON, got: %v, want: %v", got, tc.want)
297298
}
298299
}
299300
})
@@ -346,6 +347,96 @@ var testUnitBigtableGCPolicyCustomizeDiffTestcases = []testUnitBigtableGCPolicyC
346347
},
347348
}
348349

350+
type testUnitGcPolicyToGCRuleString struct {
351+
name string
352+
policy bigtable.GCPolicy
353+
topLevel bool
354+
want string
355+
errorExpected bool
356+
}
357+
358+
var testUnitGcPolicyToGCRuleStringTestCases = []testUnitGcPolicyToGCRuleString{
359+
{
360+
name: "NoGcPolicy",
361+
policy: bigtable.NoGcPolicy(),
362+
topLevel: true,
363+
want: `{"rules":[{"max_version":1}]}`,
364+
errorExpected: true,
365+
},
366+
{
367+
name: "MaxVersionPolicy",
368+
policy: bigtable.MaxVersionsPolicy(1),
369+
topLevel: true,
370+
want: `{"rules":[{"max_version":1}]}`,
371+
errorExpected: false,
372+
},
373+
{
374+
name: "MaxAgePolicy",
375+
policy: bigtable.MaxAgePolicy(time.Hour),
376+
topLevel: true,
377+
want: `{"rules":[{"max_age":"1h"}]}`,
378+
errorExpected: false,
379+
},
380+
{
381+
name: "UnionPolicy",
382+
policy: bigtable.UnionPolicy(bigtable.MaxVersionsPolicy(1), bigtable.MaxAgePolicy(time.Hour)),
383+
topLevel: true,
384+
want: `{"mode":"union","rules":[{"max_version":1},{"max_age":"1h"}]}`,
385+
errorExpected: false,
386+
},
387+
{
388+
name: "IntersectionPolicy",
389+
policy: bigtable.IntersectionPolicy(bigtable.MaxVersionsPolicy(1), bigtable.MaxAgePolicy(time.Hour)),
390+
topLevel: true,
391+
want: `{"mode":"intersection","rules":[{"max_version":1},{"max_age":"1h"}]}`,
392+
errorExpected: false,
393+
},
394+
{
395+
name: "NestedPolicy",
396+
policy: bigtable.UnionPolicy(bigtable.IntersectionPolicy(bigtable.MaxVersionsPolicy(1), bigtable.MaxAgePolicy(3*time.Hour)), bigtable.MaxAgePolicy(time.Hour)),
397+
topLevel: true,
398+
want: `{"mode":"union","rules":[{"mode":"intersection","rules":[{"max_version":1},{"max_age":"3h"}]},{"max_age":"1h"}]}`,
399+
errorExpected: false,
400+
},
401+
{
402+
name: "MaxVersionPolicyNotTopeLevel",
403+
policy: bigtable.MaxVersionsPolicy(1),
404+
topLevel: false,
405+
want: `{"max_version":1}`,
406+
errorExpected: false,
407+
},
408+
{
409+
name: "MaxAgePolicyNotTopeLevel",
410+
policy: bigtable.MaxAgePolicy(time.Hour),
411+
topLevel: false,
412+
want: `{"max_age":"1h"}`,
413+
errorExpected: false,
414+
},
415+
}
416+
417+
func TestUnitBigtableGCPolicy_gcPolicyToGCRuleString(t *testing.T) {
418+
for _, tc := range testUnitGcPolicyToGCRuleStringTestCases {
419+
t.Run(tc.name, func(t *testing.T) {
420+
got, err := gcPolicyToGCRuleString(tc.policy, tc.topLevel)
421+
if tc.errorExpected && err == nil {
422+
t.Fatal("expect error, got nil")
423+
} else if !tc.errorExpected && err != nil {
424+
t.Fatalf("unexpected error: %v", err)
425+
} else {
426+
if got != nil {
427+
gcRuleJsonString, err := json.Marshal(got)
428+
if err != nil {
429+
t.Fatalf("Error marshaling GC policy to json: %s", err)
430+
}
431+
if string(gcRuleJsonString) != tc.want {
432+
t.Errorf("Unexpected GC policy, got: %v, want: %v", string(gcRuleJsonString), tc.want)
433+
}
434+
}
435+
}
436+
})
437+
}
438+
}
439+
349440
func testAccCheckBigtableGCPolicyDestroyProducer(t *testing.T) func(s *terraform.State) error {
350441
return func(s *terraform.State) error {
351442
var ctx = context.Background()
@@ -382,7 +473,7 @@ func testAccCheckBigtableGCPolicyDestroyProducer(t *testing.T) func(s *terraform
382473
}
383474
}
384475

385-
func testAccBigtableGCPolicyExists(t *testing.T, n string) resource.TestCheckFunc {
476+
func testAccBigtableGCPolicyExists(t *testing.T, n string, compareGcRules bool) resource.TestCheckFunc {
386477
var ctx = context.Background()
387478
return func(s *terraform.State) error {
388479
rs, ok := s.RootModule().Resources[n]
@@ -406,9 +497,24 @@ func testAccBigtableGCPolicyExists(t *testing.T, n string) resource.TestCheckFun
406497
return fmt.Errorf("Error retrieving table. Could not find %s in %s.", rs.Primary.Attributes["table"], rs.Primary.Attributes["instance_name"])
407498
}
408499

409-
for _, i := range table.FamilyInfos {
410-
if i.Name == rs.Primary.Attributes["column_family"] && i.GCPolicy == rs.Primary.ID {
411-
return nil
500+
for _, familyInfo := range table.FamilyInfos {
501+
if familyInfo.Name == rs.Primary.Attributes["column_family"] && familyInfo.GCPolicy == rs.Primary.ID {
502+
// Ensure the remote GC policy matches the local copy if `compareGcRules` is set to true.
503+
if !compareGcRules {
504+
return nil
505+
}
506+
gcRuleString, err := gcPolicyToGCRuleString(familyInfo.FullGCPolicy /*isTopLevel=*/, true)
507+
if err != nil {
508+
return fmt.Errorf("Error converting GC policy to JSON string: %s", err)
509+
}
510+
gcRuleJsonString, err := json.Marshal(gcRuleString)
511+
if err != nil {
512+
return fmt.Errorf("Error marshaling GC Policy to JSON: %s", err)
513+
}
514+
if string(gcRuleJsonString) == rs.Primary.Attributes["gc_rules"] {
515+
return nil
516+
}
517+
return fmt.Errorf("Found differences in the local and the remote GC policies: %s vs %s", rs.Primary.Attributes["gc_rules"], string(gcRuleJsonString))
412518
}
413519
}
414520

0 commit comments

Comments
 (0)