Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add customize diff for deprecated attribute on gc_policy #4556

Merged
merged 5 commits into from
Mar 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mmv1/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
end

if override_dir
Google::LOGGER.info "Using override directory '#{override_dir}'"
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
Dir["#{override_dir}/products/**/api.yaml"].each do |file_path|
product = File.dirname(Pathname.new(file_path).relative_path_from(override_dir))
all_product_files.push(product) unless all_product_files.include? product
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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.`,
Expand All @@ -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(),
Expand Down
143 changes: 143 additions & 0 deletions mmv1/third_party/terraform/tests/resource_bigtable_gc_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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" {
Expand Down
6 changes: 3 additions & 3 deletions mmv1/third_party/terraform/utils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this wasn't used anywhere... Could this be map[string]bool instead?

IsForceNew bool
}

Expand All @@ -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
}

Expand Down