Skip to content

Commit

Permalink
Bigtable: Update local gc_rules on read (#6526)
Browse files Browse the repository at this point in the history 
* Update local gc_rules on read

* Early return + add comment

* Check list length

* Handle empty GC policy
  • Loading branch information
@kevinsi4508
kevinsi4508 authored Sep 15, 2022
1 parent ea8148c commit 87e92f8
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 17 deletions.
93 changes: 90 additions & 3 deletions mmv1/third_party/terraform/resources/resource_bigtable_gc_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,32 @@ func resourceBigtableGCPolicyRead(d *schema.ResourceData, meta interface{}) erro
}

for _, fi := range ti.FamilyInfos {
if fi.Name == columnFamily {
d.SetId(fi.GCPolicy)
break
if fi.Name != columnFamily {
continue
}

d.SetId(fi.GCPolicy)

// No GC Policy.
if fi.FullGCPolicy.String() == "" {
return nil
}

// Only set `gc_rules`` when the legacy fields are not set. We are not planning to support legacy fields.
maxAge := d.Get("max_age")
maxVersion := d.Get("max_version")
if d.Get("mode") == "" && len(maxAge.([]interface{})) == 0 && len(maxVersion.([]interface{})) == 0 {
gcRuleString, err := gcPolicyToGCRuleString(fi.FullGCPolicy, true)
if err != nil {
return err
}
gcRuleJsonString, err := json.Marshal(gcRuleString)
if err != nil {
return fmt.Errorf("Error marshaling GC policy to json: %s", err)
}
d.Set("gc_rules", string(gcRuleJsonString))
}
break
}

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

// Recursively convert Bigtable GC policy to JSON format in a map.
func gcPolicyToGCRuleString(gc bigtable.GCPolicy, topLevel bool) (map[string]interface{}, error) {
result := make(map[string]interface{})
switch bigtable.GetPolicyType(gc) {
case bigtable.PolicyMaxAge:
age := gc.(bigtable.MaxAgeGCPolicy).GetDurationString()
if topLevel {
rule := make(map[string]interface{})
rule["max_age"] = age
rules := []interface{}{}
rules = append(rules, rule)
result["rules"] = rules
} else {
result["max_age"] = age
}
break
case bigtable.PolicyMaxVersion:
// bigtable.MaxVersionsGCPolicy is an int.
// Not sure why max_version is a float64.
// TODO: Maybe change max_version to an int.
version := float64(int(gc.(bigtable.MaxVersionsGCPolicy)))
if topLevel {
rule := make(map[string]interface{})
rule["max_version"] = version
rules := []interface{}{}
rules = append(rules, rule)
result["rules"] = rules
} else {
result["max_version"] = version
}
break
case bigtable.PolicyUnion:
result["mode"] = "union"
rules := []interface{}{}
for _, c := range gc.(bigtable.UnionGCPolicy).Children {
gcRuleString, err := gcPolicyToGCRuleString(c, false)
if err != nil {
return nil, err
}
rules = append(rules, gcRuleString)
}
result["rules"] = rules
break
case bigtable.PolicyIntersection:
result["mode"] = "intersection"
rules := []interface{}{}
for _, c := range gc.(bigtable.IntersectionGCPolicy).Children {
gcRuleString, err := gcPolicyToGCRuleString(c, false)
if err != nil {
return nil, err
}
rules = append(rules, gcRuleString)
}
result["rules"] = rules
default:
break
}

if err := validateNestedPolicy(result, topLevel); err != nil {
return nil, err
}

return result, nil
}

func resourceBigtableGCPolicyDestroy(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
userAgent, err := generateUserAgentString(d, config.userAgent)
Expand Down
134 changes: 120 additions & 14 deletions mmv1/third_party/terraform/tests/resource_bigtable_gc_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"testing"
"time"

"cloud.google.com/go/bigtable"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
Expand All @@ -29,7 +30,7 @@ func TestAccBigtableGCPolicy_basic(t *testing.T) {
Config: testAccBigtableGCPolicy(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policy"),
t, "google_bigtable_gc_policy.policy", false),
),
},
},
Expand All @@ -54,7 +55,7 @@ func TestAccBigtableGCPolicy_swapOffDeprecated(t *testing.T) {
Config: testAccBigtableGCPolicy_days(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policy"),
t, "google_bigtable_gc_policy.policy", false),
// Verify can write some data.
testAccBigtableCanWriteData(
t, "google_bigtable_gc_policy.policy", 10),
Expand All @@ -64,7 +65,7 @@ func TestAccBigtableGCPolicy_swapOffDeprecated(t *testing.T) {
Config: testAccBigtableGCPolicy(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policy"),
t, "google_bigtable_gc_policy.policy", false),
// Verify no data loss after the GC policy update.
testAccBigtableCanReadData(
t, "google_bigtable_gc_policy.policy", 10),
Expand Down Expand Up @@ -92,7 +93,7 @@ func TestAccBigtableGCPolicy_union(t *testing.T) {
Config: testAccBigtableGCPolicyUnion(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policy"),
t, "google_bigtable_gc_policy.policy", false),
),
},
},
Expand All @@ -118,11 +119,11 @@ func TestAccBigtableGCPolicy_multiplePolicies(t *testing.T) {
Config: testAccBigtableGCPolicy_multiplePolicies(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policyA"),
t, "google_bigtable_gc_policy.policyA", false),
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policyB"),
t, "google_bigtable_gc_policy.policyB", false),
testAccBigtableGCPolicyExists(
t, "google_bigtable_gc_policy.policyC"),
t, "google_bigtable_gc_policy.policyC", false),
),
},
},
Expand All @@ -148,7 +149,7 @@ func TestAccBigtableGCPolicy_gcRulesPolicy(t *testing.T) {
{
Config: testAccBigtableGCPolicy_gcRulesCreate(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy"),
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy", true),
resource.TestCheckResourceAttr("google_bigtable_gc_policy.policy", "gc_rules", gcRulesOriginal),
),
},
Expand All @@ -157,7 +158,7 @@ func TestAccBigtableGCPolicy_gcRulesPolicy(t *testing.T) {
{
Config: testAccBigtableGCPolicy_gcRulesUpdate(instanceName, tableName, familyName),
Check: resource.ComposeTestCheckFunc(
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy"),
testAccBigtableGCPolicyExists(t, "google_bigtable_gc_policy.policy", true),
resource.TestCheckResourceAttr("google_bigtable_gc_policy.policy", "gc_rules", gcRulesUpdate),
),
},
Expand Down Expand Up @@ -293,7 +294,7 @@ func TestUnitBigtableGCPolicy_getGCPolicyFromJSON(t *testing.T) {
t.Fatalf("unexpected error: %v", err)
} else {
if got != nil && got.String() != tc.want {
t.Errorf("error getting policy from JSON, got: %v, want: %v", tc.want, got)
t.Errorf("error getting policy from JSON, got: %v, want: %v", got, tc.want)
}
}
})
Expand Down Expand Up @@ -346,6 +347,96 @@ var testUnitBigtableGCPolicyCustomizeDiffTestcases = []testUnitBigtableGCPolicyC
},
}

type testUnitGcPolicyToGCRuleString struct {
name string
policy bigtable.GCPolicy
topLevel bool
want string
errorExpected bool
}

var testUnitGcPolicyToGCRuleStringTestCases = []testUnitGcPolicyToGCRuleString{
{
name: "NoGcPolicy",
policy: bigtable.NoGcPolicy(),
topLevel: true,
want: `{"rules":[{"max_version":1}]}`,
errorExpected: true,
},
{
name: "MaxVersionPolicy",
policy: bigtable.MaxVersionsPolicy(1),
topLevel: true,
want: `{"rules":[{"max_version":1}]}`,
errorExpected: false,
},
{
name: "MaxAgePolicy",
policy: bigtable.MaxAgePolicy(time.Hour),
topLevel: true,
want: `{"rules":[{"max_age":"1h"}]}`,
errorExpected: false,
},
{
name: "UnionPolicy",
policy: bigtable.UnionPolicy(bigtable.MaxVersionsPolicy(1), bigtable.MaxAgePolicy(time.Hour)),
topLevel: true,
want: `{"mode":"union","rules":[{"max_version":1},{"max_age":"1h"}]}`,
errorExpected: false,
},
{
name: "IntersectionPolicy",
policy: bigtable.IntersectionPolicy(bigtable.MaxVersionsPolicy(1), bigtable.MaxAgePolicy(time.Hour)),
topLevel: true,
want: `{"mode":"intersection","rules":[{"max_version":1},{"max_age":"1h"}]}`,
errorExpected: false,
},
{
name: "NestedPolicy",
policy: bigtable.UnionPolicy(bigtable.IntersectionPolicy(bigtable.MaxVersionsPolicy(1), bigtable.MaxAgePolicy(3*time.Hour)), bigtable.MaxAgePolicy(time.Hour)),
topLevel: true,
want: `{"mode":"union","rules":[{"mode":"intersection","rules":[{"max_version":1},{"max_age":"3h"}]},{"max_age":"1h"}]}`,
errorExpected: false,
},
{
name: "MaxVersionPolicyNotTopeLevel",
policy: bigtable.MaxVersionsPolicy(1),
topLevel: false,
want: `{"max_version":1}`,
errorExpected: false,
},
{
name: "MaxAgePolicyNotTopeLevel",
policy: bigtable.MaxAgePolicy(time.Hour),
topLevel: false,
want: `{"max_age":"1h"}`,
errorExpected: false,
},
}

func TestUnitBigtableGCPolicy_gcPolicyToGCRuleString(t *testing.T) {
for _, tc := range testUnitGcPolicyToGCRuleStringTestCases {
t.Run(tc.name, func(t *testing.T) {
got, err := gcPolicyToGCRuleString(tc.policy, tc.topLevel)
if tc.errorExpected && err == nil {
t.Fatal("expect error, got nil")
} else if !tc.errorExpected && err != nil {
t.Fatalf("unexpected error: %v", err)
} else {
if got != nil {
gcRuleJsonString, err := json.Marshal(got)
if err != nil {
t.Fatalf("Error marshaling GC policy to json: %s", err)
}
if string(gcRuleJsonString) != tc.want {
t.Errorf("Unexpected GC policy, got: %v, want: %v", string(gcRuleJsonString), tc.want)
}
}
}
})
}
}

func testAccCheckBigtableGCPolicyDestroyProducer(t *testing.T) func(s *terraform.State) error {
return func(s *terraform.State) error {
var ctx = context.Background()
Expand Down Expand Up @@ -382,7 +473,7 @@ func testAccCheckBigtableGCPolicyDestroyProducer(t *testing.T) func(s *terraform
}
}

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

for _, i := range table.FamilyInfos {
if i.Name == rs.Primary.Attributes["column_family"] && i.GCPolicy == rs.Primary.ID {
return nil
for _, familyInfo := range table.FamilyInfos {
if familyInfo.Name == rs.Primary.Attributes["column_family"] && familyInfo.GCPolicy == rs.Primary.ID {
// Ensure the remote GC policy matches the local copy if `compareGcRules` is set to true.
if !compareGcRules {
return nil
}
gcRuleString, err := gcPolicyToGCRuleString(familyInfo.FullGCPolicy /*isTopLevel=*/, true)
if err != nil {
return fmt.Errorf("Error converting GC policy to JSON string: %s", err)
}
gcRuleJsonString, err := json.Marshal(gcRuleString)
if err != nil {
return fmt.Errorf("Error marshaling GC Policy to JSON: %s", err)
}
if string(gcRuleJsonString) == rs.Primary.Attributes["gc_rules"] {
return nil
}
return fmt.Errorf("Found differences in the local and the remote GC policies: %s vs %s", rs.Primary.Attributes["gc_rules"], string(gcRuleJsonString))
}
}

Expand Down

0 comments on commit 87e92f8

Please sign in to comment.