Skip to content

Commit

Permalink
bigquery_table - add customized diff for specific properties and chan…
Browse files Browse the repository at this point in the history 
…ges on schema (#4400) (#2876)

* Add suport for seperate field entity and forceNew on specific schema table changes

Co-authored-by: Daniel Schierbeck <[email protected]>

* resolve linter issues

* go fmt

* fix readability issues and segement examples

* seperate schema from fields commits

* clean up test code and refactor for better readability

* fix tests, don't return error from json unmarshall

Co-authored-by: Daniel Schierbeck <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Daniel Schierbeck <[email protected]>
  • Loading branch information
@modular-magician @dasch
modular-magician and dasch authored Jan 15, 2021
1 parent e8afa64 commit 45997cd
Show file tree
Hide file tree
Showing 5 changed files with 286 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .changelog/4400.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
bigquery: made incompatible changes to the `google_bigquery_table.schema` field cause the resource to be recreated
```
157 changes: 144 additions & 13 deletions google-beta/resource_bigquery_table.go
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package google

import (
"context"
"encoding/json"
"errors"
"fmt"
"log"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand Down Expand Up @@ -89,20 +91,14 @@ func bigQueryTableMapKeyOverride(key string, objectA, objectB map[string]interfa
valB := objectB[key]
switch key {
case "mode":
equivelentSet := []interface{}{nil, "NULLABLE"}
eq := valueIsInArray(valA, equivelentSet) && valueIsInArray(valB, equivelentSet)
eq := bigQueryTableModeEq(valA, valB)
return eq
case "description":
equivelentSet := []interface{}{nil, ""}
eq := valueIsInArray(valA, equivelentSet) && valueIsInArray(valB, equivelentSet)
equivalentSet := []interface{}{nil, ""}
eq := valueIsInArray(valA, equivalentSet) && valueIsInArray(valB, equivalentSet)
return eq
case "type":
equivelentSet1 := []interface{}{"INTEGER", "INT64"}
equivelentSet2 := []interface{}{"FLOAT", "FLOAT64"}
eq1 := valueIsInArray(valA, equivelentSet1) && valueIsInArray(valB, equivelentSet1)
eq2 := valueIsInArray(valA, equivelentSet2) && valueIsInArray(valB, equivelentSet2)
eq := eq1 || eq2
return eq
return bigQueryTableTypeEq(valA, valB)
}

// otherwise rely on default behavior
Expand Down Expand Up @@ -132,6 +128,141 @@ func bigQueryTableSchemaDiffSuppress(_, old, new string, _ *schema.ResourceData)
return eq
}

func bigQueryTableTypeEq(old, new interface{}) bool {
equivalentSet1 := []interface{}{"INTEGER", "INT64"}
equivalentSet2 := []interface{}{"FLOAT", "FLOAT64"}
equivalentSet3 := []interface{}{"BOOLEAN", "BOOL"}
eq0 := old == new
eq1 := valueIsInArray(old, equivalentSet1) && valueIsInArray(new, equivalentSet1)
eq2 := valueIsInArray(old, equivalentSet2) && valueIsInArray(new, equivalentSet2)
eq3 := valueIsInArray(old, equivalentSet3) && valueIsInArray(new, equivalentSet3)
eq := eq0 || eq1 || eq2 || eq3
return eq
}

func bigQueryTableModeEq(old, new interface{}) bool {
equivalentSet := []interface{}{nil, "NULLABLE"}
eq0 := old == new
eq1 := valueIsInArray(old, equivalentSet) && valueIsInArray(new, equivalentSet)
eq := eq0 || eq1
return eq
}

func bigQueryTableModeIsForceNew(old, new interface{}) bool {
eq := bigQueryTableModeEq(old, new)
reqToNull := old == "REQUIRED" && new == "NULLABLE"
return !eq && !reqToNull
}

// Compares two existing schema implementations and decides if
// it is changeable.. pairs with a force new on not changeable
func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) {
switch old.(type) {
case []interface{}:
arrayOld := old.([]interface{})
arrayNew, ok := new.([]interface{})
if !ok {
// if not both arrays not changeable
return false, nil
}
if len(arrayOld) > len(arrayNew) {
// if not growing not changeable
return false, nil
}
for i := range arrayOld {
if isChangable, err :=
resourceBigQueryTableSchemaIsChangeable(arrayOld[i], arrayNew[i]); err != nil || !isChangable {
return false, err
}
}
return true, nil
case map[string]interface{}:
objectOld := old.(map[string]interface{})
objectNew, ok := new.(map[string]interface{})
if !ok {
// if both aren't objects
return false, nil
}

var unionOfKeys map[string]bool = make(map[string]bool)
for key := range objectOld {
unionOfKeys[key] = true
}
for key := range objectNew {
unionOfKeys[key] = true
}

for key := range unionOfKeys {
valOld := objectOld[key]
valNew := objectNew[key]
switch key {
case "name":
if valOld != valNew {
return false, nil
}
case "type":
if !bigQueryTableTypeEq(valOld, valNew) {
return false, nil
}
case "mode":
if bigQueryTableModeIsForceNew(valOld, valNew) {
return false, nil
}
case "fields":
return resourceBigQueryTableSchemaIsChangeable(valOld, valNew)

// other parameters: description, policyTags and
// policyTags.names[] are changeable
}
}
return true, nil
case string, float64, bool, nil:
// realistically this shouldn't hit
log.Printf("[DEBUG] comparison of generics hit... not expected")
return old == new, nil
default:
log.Printf("[DEBUG] tried to iterate through json but encountered a non native type to json deserialization... please ensure you are passing a json object from json.Unmarshall")
return false, errors.New("unable to compare values")
}
}

func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error {
if _, hasSchema := d.GetOk("schema"); hasSchema {
oldSchema, newSchema := d.GetChange("schema")
oldSchemaText := oldSchema.(string)
newSchemaText := newSchema.(string)
if oldSchemaText == "null" {
// The API can return an empty schema which gets encoded to "null" during read.
oldSchemaText = "[]"
}
var old, new interface{}
if err := json.Unmarshal([]byte(oldSchemaText), &old); err != nil {
// don't return error, its possible we are going from no schema to schema
// this case will be cover on the conparision regardless.
log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err)
}
if err := json.Unmarshal([]byte(newSchemaText), &new); err != nil {
// same as above
log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err)
}
isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new)
if err != nil {
return err
}
if !isChangeable {
if err := d.ForceNew("schema"); err != nil {
return err
}
}
return nil
}
return nil
}

func resourceBigQueryTableSchemaCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
return resourceBigQueryTableSchemaCustomizeDiffFunc(d)
}

func resourceBigQueryTable() *schema.Resource {
return &schema.Resource{
Create: resourceBigQueryTableCreate,
Expand All @@ -141,6 +272,9 @@ func resourceBigQueryTable() *schema.Resource {
Importer: &schema.ResourceImporter{
State: resourceBigQueryTableImport,
},
CustomizeDiff: customdiff.All(
resourceBigQueryTableSchemaCustomizeDiff,
),
Schema: map[string]*schema.Schema{
// TableId: [Required] The ID of the table. The ID must contain only
// letters (a-z, A-Z), numbers (0-9), or underscores (_). The maximum
Expand Down Expand Up @@ -422,7 +556,6 @@ func resourceBigQueryTable() *schema.Resource {
DiffSuppressFunc: bigQueryTableSchemaDiffSuppress,
Description: `A JSON schema for the table.`,
},

// View: [Optional] If specified, configures this table as a view.
"view": {
Type: schema.TypeList,
Expand Down Expand Up @@ -761,7 +894,6 @@ func resourceTable(d *schema.ResourceData, meta interface{}) (*bigquery.Table, e
if err != nil {
return nil, err
}

table.Schema = schema
}

Expand Down Expand Up @@ -948,7 +1080,6 @@ func resourceBigQueryTableRead(d *schema.ResourceData, meta interface{}) error {
if err != nil {
return err
}

if err := d.Set("schema", schema); err != nil {
return fmt.Errorf("Error setting schema: %s", err)
}
Expand Down
133 changes: 133 additions & 0 deletions google-beta/resource_bigquery_table_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,12 +471,135 @@ func TestUnitBigQueryDataTable_jsonEquivalency(t *testing.T) {
}
}

func TestUnitBigQueryDataTable_schemaIsChangable(t *testing.T) {
t.Parallel()
for _, testcase := range testUnitBigQueryDataTableIsChangableTestCases {
testcase.check(t)
testcaseNested := &testUnitBigQueryDataTableJSONChangeableTestCase{
testcase.name + "Nested",
fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"fields\" : %s }]", testcase.jsonOld),
fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INT64\", \"fields\" : %s }]", testcase.jsonNew),
testcase.changeable,
}
testcaseNested.check(t)
}
}

type testUnitBigQueryDataTableJSONEquivalencyTestCase struct {
jsonA string
jsonB string
equivalent bool
}

type testUnitBigQueryDataTableJSONChangeableTestCase struct {
name string
jsonOld string
jsonNew string
changeable bool
}

func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testing.T) {
var old, new interface{}
if err := json.Unmarshal([]byte(testcase.jsonOld), &old); err != nil {
t.Fatalf("unable to unmarshal json - %v", err)
}
if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil {
t.Fatalf("unable to unmarshal json - %v", err)
}
changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new)
if err != nil {
t.Errorf("%s failed unexpectedly: %s", testcase.name, err)
}
if changeable != testcase.changeable {
t.Errorf("expected changeable result of %v but got %v for testcase %s", testcase.changeable, changeable, testcase.name)
}

d := &ResourceDiffMock{
Before: map[string]interface{}{},
After: map[string]interface{}{},
}

d.Before["schema"] = testcase.jsonOld
d.After["schema"] = testcase.jsonNew

err = resourceBigQueryTableSchemaCustomizeDiffFunc(d)
if err != nil {
t.Errorf("error on testcase %s - %w", testcase.name, err)
}
if !testcase.changeable != d.IsForceNew {
t.Errorf("%s: expected d.IsForceNew to be %v, but was %v", testcase.name, !testcase.changeable, d.IsForceNew)
}
}

var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{
{
name: "defaultEquality",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
changeable: true,
},
{
name: "arraySizeIncreases",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
changeable: true,
},
{
name: "arraySizeDecreases",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
changeable: false,
},
{
name: "descriptionChanges",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
changeable: true,
},
{
name: "typeInteger",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INT64\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
changeable: true,
},
{
name: "typeFloat",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"FLOAT\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"FLOAT64\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
changeable: true,
},
{
name: "typeBool",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOL\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
changeable: true,
},
{
name: "typeChangeIncompatible",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"DATETIME\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
changeable: false,
},
{
name: "typeModeReqToNull",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
changeable: true,
},
{
name: "typeModeIncompatible",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REPEATED\", \"description\" : \"some new value\" }]",
changeable: false,
},
{
name: "typeModeOmission",
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]",
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"description\" : \"some new value\" }]",
changeable: false,
},
}

var testUnitBigQueryDataTableJSONEquivalencyTestCases = []testUnitBigQueryDataTableJSONEquivalencyTestCase{
{
"[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]",
Expand Down Expand Up @@ -528,6 +651,16 @@ var testUnitBigQueryDataTableJSONEquivalencyTestCases = []testUnitBigQueryDataTa
"[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]",
false,
},
{
"[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]",
"[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]",
true,
},
{
"[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]",
"[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]",
false,
},
{
"[1,2,3]",
"[1,2,3]",
Expand Down
5 changes: 5 additions & 0 deletions google-beta/test_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ func (d *ResourceDiffMock) Get(key string) interface{} {
return d.After[key]
}

func (d *ResourceDiffMock) GetOk(key string) (interface{}, bool) {
v, ok := d.After[key]
return v, ok
}

func (d *ResourceDiffMock) Clear(key string) error {
if d.Cleared == nil {
d.Cleared = map[string]struct{}{}
Expand Down
1 change: 1 addition & 0 deletions google-beta/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type TerraformResourceDiff interface {
HasChange(string) bool
GetChange(string) (interface{}, interface{})
Get(string) interface{}
GetOk(string) (interface{}, bool)
Clear(string) error
ForceNew(string) error
}
Expand Down

0 comments on commit 45997cd

Please sign in to comment.