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

bigquery_table - add customized diff for specific properties and changes on schema #8232

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
3 changes: 3 additions & 0 deletions .changelog/4400.txt
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/resource_bigquery_table.go
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/resource_bigquery_table_test.go
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/test_utils.go
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/utils.go
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