From f20e9abba310850167e9ea3e29e91959d6cc7459 Mon Sep 17 00:00:00 2001 From: Scott Suarez Date: Fri, 8 Jan 2021 13:04:57 -0800 Subject: [PATCH 1/7] Add suport for seperate field entity and forceNew on specific schema table changes Co-authored-by: Daniel Schierbeck --- .../resources/resource_bigquery_table.go | 263 +++++++++++- .../tests/resource_bigquery_table_test.go | 375 ++++++++++++++++++ third_party/terraform/utils/test_utils.go | 5 + third_party/terraform/utils/utils.go.erb | 1 + .../docs/r/bigquery_table.html.markdown | 47 ++- 5 files changed, 672 insertions(+), 19 deletions(-) diff --git a/third_party/terraform/resources/resource_bigquery_table.go b/third_party/terraform/resources/resource_bigquery_table.go index 73ab4afc978f..61c94c077211 100644 --- a/third_party/terraform/resources/resource_bigquery_table.go +++ b/third_party/terraform/resources/resource_bigquery_table.go @@ -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" @@ -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) 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 @@ -132,6 +128,154 @@ func bigQueryTableSchemaDiffSuppress(_, old, new string, _ *schema.ResourceData) return eq } +func bigQueryTableTypeEq(old, new interface{}) bool { + equivelentSet1 := []interface{}{"INTEGER", "INT64"} + equivelentSet2 := []interface{}{"FLOAT", "FLOAT64"} + equivelentSet3 := []interface{}{"BOOLEAN", "BOOL"} + eq0 := old == new + eq1 := valueIsInArray(old, equivelentSet1) && valueIsInArray(new, equivelentSet1) + eq2 := valueIsInArray(old, equivelentSet2) && valueIsInArray(new, equivelentSet2) + eq3 := valueIsInArray(old, equivelentSet3) && valueIsInArray(new, equivelentSet3) + eq := eq0 || eq1 || eq2 || eq3 + return eq +} + +func bigQueryTableModeEq(old, new interface{}) bool { + equivelentSet := []interface{}{nil, "NULLABLE"} + eq0 := old == new + eq1 := valueIsInArray(old, equivelentSet) && valueIsInArray(new, equivelentSet) + eq := eq0 || eq1 + return eq +} + +func bigQueryTableTypeDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { + return bigQueryTableTypeEq(old, new) +} + +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 changable.. pairs with a force new on not changable +func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) { + switch old.(type) { + case []interface{}: + arrayOld := old.([]interface{}) + arrayNew, ok := new.([]interface{}) + if !ok { + // if not both arrays not changable + return false, nil + } else if len(arrayOld) > len(arrayNew) { + // if not growing not changable + return false, nil + } + for i := range arrayOld { + isChangable, err := resourceBigQueryTableSchemaIsChangable(arrayOld[i], arrayNew[i]) + if 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 resourceBigQueryTableSchemaIsChangable(valOld, valNew) + + // other parameters: description, policyTags and + // policyTags.names[] are changable + } + } + 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 { + if _, hasfield := d.GetOk("field.#"); hasfield { + d.ForceNew("schema") + d.ForceNew("field.#") + } else { + oldSchema, newSchema := d.GetChange("schema") + oldSchemaText := oldSchema.(string) + newSchemaText := newSchema.(string) + if oldSchemaText == "null" { + oldSchemaText = "[]" + } + var old, new interface{} + if err := json.Unmarshal([]byte(oldSchemaText), &old); err != nil { + log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) + } + if err := json.Unmarshal([]byte(newSchemaText), &new); err != nil { + log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) + } + isChangable, err := resourceBigQueryTableSchemaIsChangable(old, new) + if err != nil { + return err + } else if !isChangable { + d.ForceNew("schema") + } + } + } else { + oldCount, newCount := d.GetChange("field.#") + if oldCount.(int) > newCount.(int) { + // if array is shrinking not changable + d.ForceNew("field.#") + } + for i := 0; i < oldCount.(int); i++ { + modeKey := fmt.Sprintf("field.%d.mode", i) + oldMode, newMode := d.GetChange(modeKey) + if bigQueryTableModeIsForceNew(oldMode, newMode) { + d.ForceNew(modeKey) + } + } + } + return nil +} + +func resourceBigQueryTableSchemaCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta interface{}) error { + return resourceBigQueryTableSchemaCustomizeDiffFunc(d) +} + func resourceBigQueryTable() *schema.Resource { return &schema.Resource{ Create: resourceBigQueryTableCreate, @@ -141,6 +285,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 @@ -421,8 +568,40 @@ func resourceBigQueryTable() *schema.Resource { }, DiffSuppressFunc: bigQueryTableSchemaDiffSuppress, Description: `A JSON schema for the table.`, + ConflictsWith: []string{"field"}, + }, + // Field: A field in the table's schema. + "field": { + Type: schema.TypeList, + Optional: true, + ConflictsWith: []string{"schema"}, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "type": { + Type: schema.TypeString, + Required: true, + DiffSuppressFunc: bigQueryTableTypeDiffSuppress, + ForceNew: true, + }, + "description": { + Type: schema.TypeString, + Computed: true, + Optional: true, + }, + "mode": { + Type: schema.TypeString, + Optional: true, + Default: "NULLABLE", + ValidateFunc: validation.StringInSlice([]string{"NULLABLE", "REQUIRED", "REPEATED"}, false), + }, + }, + }, }, - // View: [Optional] If specified, configures this table as a view. "view": { Type: schema.TypeList, @@ -761,7 +940,12 @@ func resourceTable(d *schema.ResourceData, meta interface{}) (*bigquery.Table, e if err != nil { return nil, err } - + table.Schema = schema + } else if v, ok := d.GetOk("field"); ok { + schema, err := expandFields(v) + if err != nil { + return nil, err + } table.Schema = schema } @@ -944,13 +1128,20 @@ func resourceBigQueryTableRead(d *schema.ResourceData, meta interface{}) error { } if res.Schema != nil { - schema, err := flattenSchema(res.Schema) - if err != nil { - return err - } - - if err := d.Set("schema", schema); err != nil { - return fmt.Errorf("Error setting schema: %s", err) + _, hasSchema := d.GetOk("schema") + _, hasFields := d.GetOk("field.#") + if hasSchema || !hasFields { + schema, err := flattenSchema(res.Schema) + if err != nil { + return err + } + if err := d.Set("schema", schema); err != nil { + return fmt.Errorf("Error setting schema: %s", err) + } + } else { + if err := d.Set("field", flattenTableFields(res.Schema.Fields)); err != nil { + return fmt.Errorf("Error setting fields: %s", err) + } } } @@ -1286,6 +1477,31 @@ func expandTimePartitioning(configured interface{}) *bigquery.TimePartitioning { return tp } +func expandFields(raw interface{}) (*bigquery.TableSchema, error) { + fields, ok := raw.([]interface{}) + if !ok { + log.Printf("[DEBUG] - unable to cast schema fields to array") + return nil, errors.New("unable to cast schema fields to array") + } + tableSchema := &bigquery.TableSchema{} + tableSchema.Fields = make([]*bigquery.TableFieldSchema, len(fields)) + for i, v := range fields { + field, ok := v.(map[string]interface{}) + if !ok { + log.Printf("[DEBUG] - unable to cast schema field to map") + return nil, errors.New("unable to cast schema field to map") + } + log.Printf("[DEBUG] - raw - %s, cast - %s", raw, fields) + tableSchema.Fields[i] = &bigquery.TableFieldSchema{ + Name: field["name"].(string), + Type: field["type"].(string), + Description: field["description"].(string), + Mode: field["mode"].(string), + } + } + return tableSchema, nil +} + func expandRangePartitioning(configured interface{}) (*bigquery.RangePartitioning, error) { if configured == nil { return nil, nil @@ -1400,6 +1616,19 @@ func flattenMaterializedView(mvd *bigquery.MaterializedViewDefinition) []map[str return []map[string]interface{}{result} } +func flattenTableFields(fields []*bigquery.TableFieldSchema) []map[string]interface{} { + result := make([]map[string]interface{}, 0, len(fields)) + for _, field := range fields { + fieldMap := make(map[string]interface{}) + fieldMap["name"] = field.Name + fieldMap["type"] = field.Type + fieldMap["description"] = field.Description + fieldMap["mode"] = field.Mode + result = append(result, fieldMap) + } + return result +} + func resourceBigQueryTableImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { config := meta.(*Config) if err := parseImportId([]string{ diff --git a/third_party/terraform/tests/resource_bigquery_table_test.go b/third_party/terraform/tests/resource_bigquery_table_test.go index ae0312f6e0b8..cfb4a47ccf7d 100644 --- a/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/third_party/terraform/tests/resource_bigquery_table_test.go @@ -450,6 +450,62 @@ func TestAccBigQueryDataTable_jsonEquivalency(t *testing.T) { }) } +func TestAccBigQueryDataTable_schemaToFieldsAndBack(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + tableID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccBigQueryTable_jsonEq(datasetID, tableID), + }, + { + ResourceName: "google_bigquery_table.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"etag", "last_modified_time"}, + }, + { + Config: testAccBigQueryTable_fields(datasetID, tableID), + Check: testAccCheckBigQueryTableFields(t), + }, + { + Config: testAccBigQueryTable_jsonEq(datasetID, tableID), + }, + { + ResourceName: "google_bigquery_table.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"etag", "last_modified_time"}, + }, + }, + }) +} + +func TestAccBigQueryDataTable_fields(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + tableID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccBigQueryTable_fields(datasetID, tableID), + Check: testAccCheckBigQueryTableFields(t), + }, + }, + }) +} + func TestUnitBigQueryDataTable_jsonEquivalency(t *testing.T) { t.Parallel() @@ -471,12 +527,271 @@ 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.changable, + } + testcaseNested.check(t) + } +} + +func TestUnitBigQueryDataTable_customizedDiffSchema(t *testing.T) { + t.Parallel() + + tcs := testUnitBigQueryDataCustomDiffFieldChangeTestcases + + for _, changableTestcase := range testUnitBigQueryDataTableIsChangableTestCases { + extraTestcase := testUnitBigQueryDataCustomDiffFieldChangeTestcase{ + name: changableTestcase.name, + schemaBefore: changableTestcase.jsonOld, + schemaAfter: changableTestcase.jsonNew, + shouldForceNew: !changableTestcase.changable, + } + tcs = append(tcs, extraTestcase) + } + + for _, testcase := range tcs { + testcase.check(t) + } +} + type testUnitBigQueryDataTableJSONEquivalencyTestCase struct { jsonA string jsonB string equivalent bool } +type testUnitBigQueryDataTableJSONChangeableTestCase struct { + name string + jsonOld string + jsonNew string + changable bool +} + +type testUnitBigQueryDataCustomDiffField struct { + mode string + description string +} + +type testUnitBigQueryDataCustomDiffFieldChangeTestcase struct { + name string + fieldBefore []testUnitBigQueryDataCustomDiffField + fieldAfter []testUnitBigQueryDataCustomDiffField + schemaBefore string + schemaAfter string + shouldForceNew bool +} + +func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testing.T) { + var old, new interface{} + if err := json.Unmarshal([]byte(testcase.jsonOld), &old); err != nil { + panic(fmt.Sprintf("unable to unmarshal json - %v", err)) + } + if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil { + panic(fmt.Sprintf("unable to unmarshal json - %v", err)) + } + changable, err := resourceBigQueryTableSchemaIsChangable(old, new) + if err != nil { + t.Errorf("ahhhh an error I did not expect this! especially not on testscase %s - %s", testcase.name, err) + } + if changable != testcase.changable { + t.Errorf("expected changable result of %v but got %v for testcase %s", testcase.changable, changable, testcase.name) + } +} + +func (testcase *testUnitBigQueryDataCustomDiffFieldChangeTestcase) check(t *testing.T) { + d := &ResourceDiffMock{ + Before: map[string]interface{}{}, + After: map[string]interface{}{}, + } + if testcase.fieldBefore != nil { + d.Before["field.#"] = len(testcase.fieldBefore) + for i, f := range testcase.fieldBefore { + d.Before[fmt.Sprintf("field.%d.mode", i)] = f.mode + d.Before[fmt.Sprintf("field.%d.description", i)] = f.description + } + } + if testcase.fieldAfter != nil { + d.After["field.#"] = len(testcase.fieldAfter) + for i, f := range testcase.fieldAfter { + d.After[fmt.Sprintf("field.%d.mode", i)] = f.mode + d.After[fmt.Sprintf("field.%d.description", i)] = f.description + } + } + if testcase.schemaBefore != "" { + d.Before["schema"] = testcase.schemaBefore + } + if testcase.schemaAfter != "" { + d.After["schema"] = testcase.schemaAfter + } + + err := resourceBigQueryTableSchemaCustomizeDiffFunc(d) + if err != nil { + t.Errorf("error on testcase %s - %w", testcase.name, err) + } + if testcase.shouldForceNew != d.IsForceNew { + t.Errorf("%s: expected d.IsForceNew to be %v, but was %v", testcase.name, testcase.shouldForceNew, d.IsForceNew) + } +} + +var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{ + { + "defaultEquality", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + true, + }, + { + "arraySizeIncreases", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + true, + }, + { + "arraySizeDecreases", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + false, + }, + { + "descriptionChanges", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + true, + }, + { + "typeInteger", + "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"INT64\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + true, + }, + { + "typeFloat", + "[{\"name\": \"someValue\", \"type\" : \"FLOAT\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"FLOAT64\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + true, + }, + { + "typeBool", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"BOOL\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + true, + }, + { + "typeRandom", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"DATETIME\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + false, + }, + { + "typeModeReqToNull", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + true, + }, + { + "typeModeRandom", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REPEATED\", \"description\" : \"some new value\" }]", + false, + }, + { + "typeModeOmission", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"description\" : \"some new value\" }]", + false, + }, +} + +var testUnitBigQueryDataCustomDiffFieldChangeTestcases = []testUnitBigQueryDataCustomDiffFieldChangeTestcase{ + { + name: "control", + fieldBefore: []testUnitBigQueryDataCustomDiffField{{ + mode: "Nullable", + description: "someValue", + }}, + fieldAfter: []testUnitBigQueryDataCustomDiffField{{ + mode: "Nullable", + description: "someValue", + }}, + shouldForceNew: false, + }, + { + name: "requiredToNullable", + fieldBefore: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }}, + fieldAfter: []testUnitBigQueryDataCustomDiffField{{ + mode: "NULLABLE", + description: "someValue", + }}, + shouldForceNew: false, + }, + { + name: "descriptionChanged", + fieldBefore: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }}, + fieldAfter: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "some other value", + }}, + shouldForceNew: false, + }, + { + name: "nullToRequired", + fieldBefore: []testUnitBigQueryDataCustomDiffField{{ + mode: "NULLABLE", + description: "someValue", + }}, + fieldAfter: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }}, + shouldForceNew: true, + }, + { + name: "arraySizeIncreases", + fieldBefore: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }}, + fieldAfter: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }, + { + mode: "REQUIRED", + description: "some other value", + }}, + shouldForceNew: false, + }, + { + name: "arraySizeDecreases", + fieldBefore: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }, + { + mode: "REQUIRED", + description: "someValue", + }}, + fieldAfter: []testUnitBigQueryDataCustomDiffField{{ + mode: "REQUIRED", + description: "someValue", + }}, + shouldForceNew: true, + }, +} + var testUnitBigQueryDataTableJSONEquivalencyTestCases = []testUnitBigQueryDataTableJSONEquivalencyTestCase{ { "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", @@ -528,6 +843,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]", @@ -586,6 +911,25 @@ func testAccCheckBigQueryTableDestroyProducer(t *testing.T) func(s *terraform.St } } +func testAccCheckBigQueryTableFields(t *testing.T) func(s *terraform.State) error { + return func(s *terraform.State) error { + for _, rs := range s.RootModule().Resources { + if rs.Type != "google_bigquery_table" { + continue + } + config := googleProviderConfig(t) + bqt, err := config.NewBigQueryClient(config.userAgent).Tables.Get(config.Project, rs.Primary.Attributes["dataset_id"], rs.Primary.Attributes["table_id"]).Do() + if err != nil { + return err + } + if bqt.Schema.Fields[1].Description != "testegg" { + t.Errorf("schema field not set to testegg for fields scenario") + } + } + return nil + } +} + func testAccBigQueryTableTimePartitioning(datasetID, tableID, partitioningType string) string { return fmt.Sprintf(` resource "google_bigquery_dataset" "test" { @@ -1237,6 +1581,37 @@ resource "google_bigquery_table" "test" { `, datasetID, tableID) } +func testAccBigQueryTable_fields(datasetID, tableID string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset" "test" { + dataset_id = "%s" +} + +resource "google_bigquery_table" "test" { + table_id = "%s" + dataset_id = google_bigquery_dataset.test.dataset_id + + friendly_name = "bigquerytest" + labels = { + "terrafrom_managed" = "true" + } + + field { + description = "Time snapshot was taken, in Epoch milliseconds. Same across all rows and all tables in the snapshot, and uniquely defines a particular snapshot." + name = "snapshot_timestamp" + mode = "NULLABLE" + type = "INTEGER" + } + + field { + description = "testegg" + name = "creation_time" + type = "TIMESTAMP" + } +} +`, datasetID, tableID) +} + var TEST_CSV = `lifelock,LifeLock,,web,Tempe,AZ,1-May-07,6850000,USD,b lifelock,LifeLock,,web,Tempe,AZ,1-Oct-06,6000000,USD,a lifelock,LifeLock,,web,Tempe,AZ,1-Jan-08,25000000,USD,c diff --git a/third_party/terraform/utils/test_utils.go b/third_party/terraform/utils/test_utils.go index 02789a82c297..b46eab955a71 100644 --- a/third_party/terraform/utils/test_utils.go +++ b/third_party/terraform/utils/test_utils.go @@ -39,6 +39,11 @@ func (d *ResourceDataMock) GetOk(key string) (interface{}, bool) { } } +func (d *ResourceDiffMock) GetOk(key string) (interface{}, bool) { + v, ok := d.After[key] + return v, ok +} + func (d *ResourceDataMock) GetOkExists(key string) (interface{}, bool) { for k, v := range d.FieldsInSchema { if key == k { diff --git a/third_party/terraform/utils/utils.go.erb b/third_party/terraform/utils/utils.go.erb index 8c193ecb239b..9f11f39e7134 100644 --- a/third_party/terraform/utils/utils.go.erb +++ b/third_party/terraform/utils/utils.go.erb @@ -35,6 +35,7 @@ type TerraformResourceDiff interface { HasChange(string) bool GetChange(string) (interface{}, interface{}) Get(string) interface{} + GetOk(string) (interface{}, bool) Clear(string) error ForceNew(string) error } diff --git a/third_party/terraform/website/docs/r/bigquery_table.html.markdown b/third_party/terraform/website/docs/r/bigquery_table.html.markdown index 8ea824628d7a..7d0b21485c17 100644 --- a/third_party/terraform/website/docs/r/bigquery_table.html.markdown +++ b/third_party/terraform/website/docs/r/bigquery_table.html.markdown @@ -41,6 +41,33 @@ resource "google_bigquery_table" "default" { env = "default" } + field { + name = "permalink" + type = "STRING" + mode = "NULLABLE" + description = "The Permalink" + } + + field { + name = "state" + type = "STRING" + mode = "NULLABLE" + description = "State where the head office is located" + } +} + +resource "google_bigquery_table" "default_schema_implementation" { + dataset_id = google_bigquery_dataset.default.dataset_id + table_id = "bar" + + time_partitioning { + type = "DAY" + } + + labels = { + env = "default" + } + schema = < Date: Fri, 8 Jan 2021 13:23:59 -0800 Subject: [PATCH 2/7] resolve linter issues --- .../resources/resource_bigquery_table.go | 30 ++++++++++++------- .../tests/resource_bigquery_table_test.go | 20 ++++++------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/third_party/terraform/resources/resource_bigquery_table.go b/third_party/terraform/resources/resource_bigquery_table.go index 61c94c077211..eba3c704ca84 100644 --- a/third_party/terraform/resources/resource_bigquery_table.go +++ b/third_party/terraform/resources/resource_bigquery_table.go @@ -159,17 +159,17 @@ func bigQueryTableModeIsForceNew(old, new interface{}) bool { } // Compares two existing schema implementations and decides if -// it is changable.. pairs with a force new on not changable +// it is changeable.. pairs with a force new on not changeable func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) { switch old.(type) { case []interface{}: arrayOld := old.([]interface{}) arrayNew, ok := new.([]interface{}) if !ok { - // if not both arrays not changable + // if not both arrays not changeable return false, nil } else if len(arrayOld) > len(arrayNew) { - // if not growing not changable + // if not growing not changeable return false, nil } for i := range arrayOld { @@ -215,7 +215,7 @@ func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) return resourceBigQueryTableSchemaIsChangable(valOld, valNew) // other parameters: description, policyTags and - // policyTags.names[] are changable + // policyTags.names[] are changeable } } return true, nil @@ -232,8 +232,12 @@ func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error { if _, hasSchema := d.GetOk("schema"); hasSchema { if _, hasfield := d.GetOk("field.#"); hasfield { - d.ForceNew("schema") - d.ForceNew("field.#") + if err := d.ForceNew("schema"); err != nil { + return err + } + if err := d.ForceNew("field.#"); err != nil { + return err + } } else { oldSchema, newSchema := d.GetChange("schema") oldSchemaText := oldSchema.(string) @@ -252,20 +256,26 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error if err != nil { return err } else if !isChangable { - d.ForceNew("schema") + if err := d.ForceNew("schema"); err != nil { + return err + } } } } else { oldCount, newCount := d.GetChange("field.#") if oldCount.(int) > newCount.(int) { - // if array is shrinking not changable - d.ForceNew("field.#") + // if array is shrinking not changeable + if err := d.ForceNew("field.#"); err != nil { + return err + } } for i := 0; i < oldCount.(int); i++ { modeKey := fmt.Sprintf("field.%d.mode", i) oldMode, newMode := d.GetChange(modeKey) if bigQueryTableModeIsForceNew(oldMode, newMode) { - d.ForceNew(modeKey) + if err := d.ForceNew(modeKey); err != nil { + return err + } } } } diff --git a/third_party/terraform/tests/resource_bigquery_table_test.go b/third_party/terraform/tests/resource_bigquery_table_test.go index cfb4a47ccf7d..4157fa9d9822 100644 --- a/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/third_party/terraform/tests/resource_bigquery_table_test.go @@ -535,7 +535,7 @@ func TestUnitBigQueryDataTable_schemaIsChangable(t *testing.T) { testcase.name + "Nested", fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"fields\" : %s }]", testcase.jsonOld), fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INT64\", \"fields\" : %s }]", testcase.jsonNew), - testcase.changable, + testcase.changeable, } testcaseNested.check(t) } @@ -546,12 +546,12 @@ func TestUnitBigQueryDataTable_customizedDiffSchema(t *testing.T) { tcs := testUnitBigQueryDataCustomDiffFieldChangeTestcases - for _, changableTestcase := range testUnitBigQueryDataTableIsChangableTestCases { + for _, changeableTestcase := range testUnitBigQueryDataTableIsChangableTestCases { extraTestcase := testUnitBigQueryDataCustomDiffFieldChangeTestcase{ - name: changableTestcase.name, - schemaBefore: changableTestcase.jsonOld, - schemaAfter: changableTestcase.jsonNew, - shouldForceNew: !changableTestcase.changable, + name: changeableTestcase.name, + schemaBefore: changeableTestcase.jsonOld, + schemaAfter: changeableTestcase.jsonNew, + shouldForceNew: !changeableTestcase.changeable, } tcs = append(tcs, extraTestcase) } @@ -571,7 +571,7 @@ type testUnitBigQueryDataTableJSONChangeableTestCase struct { name string jsonOld string jsonNew string - changable bool + changeable bool } type testUnitBigQueryDataCustomDiffField struct { @@ -596,12 +596,12 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil { panic(fmt.Sprintf("unable to unmarshal json - %v", err)) } - changable, err := resourceBigQueryTableSchemaIsChangable(old, new) + changeable, err := resourceBigQueryTableSchemaIsChangable(old, new) if err != nil { t.Errorf("ahhhh an error I did not expect this! especially not on testscase %s - %s", testcase.name, err) } - if changable != testcase.changable { - t.Errorf("expected changable result of %v but got %v for testcase %s", testcase.changable, changable, testcase.name) + if changeable != testcase.changeable { + t.Errorf("expected changeable result of %v but got %v for testcase %s", testcase.changeable, changeable, testcase.name) } } From d5b8f02b3202123e61726c315a16e5b7ee540499 Mon Sep 17 00:00:00 2001 From: Scott Suarez Date: Fri, 8 Jan 2021 14:07:39 -0800 Subject: [PATCH 3/7] go fmt --- third_party/terraform/tests/resource_bigquery_table_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/third_party/terraform/tests/resource_bigquery_table_test.go b/third_party/terraform/tests/resource_bigquery_table_test.go index 4157fa9d9822..1bfb4f25ebe4 100644 --- a/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/third_party/terraform/tests/resource_bigquery_table_test.go @@ -568,9 +568,9 @@ type testUnitBigQueryDataTableJSONEquivalencyTestCase struct { } type testUnitBigQueryDataTableJSONChangeableTestCase struct { - name string - jsonOld string - jsonNew string + name string + jsonOld string + jsonNew string changeable bool } From 00c9896b076fbc2306d3da3aa7a45f1ef8384485 Mon Sep 17 00:00:00 2001 From: Scott Suarez Date: Tue, 12 Jan 2021 16:13:21 -0800 Subject: [PATCH 4/7] fix readability issues and segement examples --- .../resources/resource_bigquery_table.go | 71 ++++++++++--------- .../tests/resource_bigquery_table_test.go | 2 +- .../docs/r/bigquery_table.html.markdown | 64 ++++++++++++----- 3 files changed, 85 insertions(+), 52 deletions(-) diff --git a/third_party/terraform/resources/resource_bigquery_table.go b/third_party/terraform/resources/resource_bigquery_table.go index eba3c704ca84..921bfd175fbf 100644 --- a/third_party/terraform/resources/resource_bigquery_table.go +++ b/third_party/terraform/resources/resource_bigquery_table.go @@ -94,8 +94,8 @@ func bigQueryTableMapKeyOverride(key string, objectA, objectB map[string]interfa 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": return bigQueryTableTypeEq(valA, valB) @@ -129,21 +129,21 @@ func bigQueryTableSchemaDiffSuppress(_, old, new string, _ *schema.ResourceData) } func bigQueryTableTypeEq(old, new interface{}) bool { - equivelentSet1 := []interface{}{"INTEGER", "INT64"} - equivelentSet2 := []interface{}{"FLOAT", "FLOAT64"} - equivelentSet3 := []interface{}{"BOOLEAN", "BOOL"} + equivalentSet1 := []interface{}{"INTEGER", "INT64"} + equivalentSet2 := []interface{}{"FLOAT", "FLOAT64"} + equivalentSet3 := []interface{}{"BOOLEAN", "BOOL"} eq0 := old == new - eq1 := valueIsInArray(old, equivelentSet1) && valueIsInArray(new, equivelentSet1) - eq2 := valueIsInArray(old, equivelentSet2) && valueIsInArray(new, equivelentSet2) - eq3 := valueIsInArray(old, equivelentSet3) && valueIsInArray(new, equivelentSet3) + 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 { - equivelentSet := []interface{}{nil, "NULLABLE"} + equivalentSet := []interface{}{nil, "NULLABLE"} eq0 := old == new - eq1 := valueIsInArray(old, equivelentSet) && valueIsInArray(new, equivelentSet) + eq1 := valueIsInArray(old, equivalentSet) && valueIsInArray(new, equivalentSet) eq := eq0 || eq1 return eq } @@ -160,7 +160,7 @@ func bigQueryTableModeIsForceNew(old, new interface{}) bool { // Compares two existing schema implementations and decides if // it is changeable.. pairs with a force new on not changeable -func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) { +func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) { switch old.(type) { case []interface{}: arrayOld := old.([]interface{}) @@ -173,7 +173,7 @@ func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) return false, nil } for i := range arrayOld { - isChangable, err := resourceBigQueryTableSchemaIsChangable(arrayOld[i], arrayNew[i]) + isChangable, err := resourceBigQueryTableSchemaIsChangeable(arrayOld[i], arrayNew[i]) if err != nil || !isChangable { return false, err } @@ -212,7 +212,7 @@ func resourceBigQueryTableSchemaIsChangable(old, new interface{}) (bool, error) return false, nil } case "fields": - return resourceBigQueryTableSchemaIsChangable(valOld, valNew) + return resourceBigQueryTableSchemaIsChangeable(valOld, valNew) // other parameters: description, policyTags and // policyTags.names[] are changeable @@ -238,30 +238,33 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error if err := d.ForceNew("field.#"); err != nil { return err } - } else { - oldSchema, newSchema := d.GetChange("schema") - oldSchemaText := oldSchema.(string) - newSchemaText := newSchema.(string) - if oldSchemaText == "null" { - oldSchemaText = "[]" - } - var old, new interface{} - if err := json.Unmarshal([]byte(oldSchemaText), &old); err != nil { - log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) - } - if err := json.Unmarshal([]byte(newSchemaText), &new); err != nil { - log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) - } - isChangable, err := resourceBigQueryTableSchemaIsChangable(old, new) - if err != nil { + return nil + } + 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 { + log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) + } + if err := json.Unmarshal([]byte(newSchemaText), &new); err != nil { + log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) + } + isChangable, err := resourceBigQueryTableSchemaIsChangeable(old, new) + if err != nil { + return err + } + if !isChangable { + if err := d.ForceNew("schema"); err != nil { return err - } else if !isChangable { - if err := d.ForceNew("schema"); err != nil { - return err - } } } - } else { + return nil + } else if _, hasfield := d.GetOk("field.#"); hasfield { oldCount, newCount := d.GetChange("field.#") if oldCount.(int) > newCount.(int) { // if array is shrinking not changeable diff --git a/third_party/terraform/tests/resource_bigquery_table_test.go b/third_party/terraform/tests/resource_bigquery_table_test.go index 1bfb4f25ebe4..57a28a57a8a9 100644 --- a/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/third_party/terraform/tests/resource_bigquery_table_test.go @@ -596,7 +596,7 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil { panic(fmt.Sprintf("unable to unmarshal json - %v", err)) } - changeable, err := resourceBigQueryTableSchemaIsChangable(old, new) + changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new) if err != nil { t.Errorf("ahhhh an error I did not expect this! especially not on testscase %s - %s", testcase.name, err) } diff --git a/third_party/terraform/website/docs/r/bigquery_table.html.markdown b/third_party/terraform/website/docs/r/bigquery_table.html.markdown index 7d0b21485c17..53ff3ee9dd83 100644 --- a/third_party/terraform/website/docs/r/bigquery_table.html.markdown +++ b/third_party/terraform/website/docs/r/bigquery_table.html.markdown @@ -14,7 +14,7 @@ Creates a table resource in a dataset for Google BigQuery. For more information [API](https://cloud.google.com/bigquery/docs/reference/rest/v2/tables). -## Example Usage +## Example Usage - field implementation ```hcl resource "google_bigquery_dataset" "default" { @@ -55,6 +55,22 @@ resource "google_bigquery_table" "default" { description = "State where the head office is located" } } +``` + +## Example Usage - schema implementation + +```hcl +resource "google_bigquery_dataset" "default" { + dataset_id = "foo" + friendly_name = "test" + description = "This is a test description" + location = "EU" + default_table_expiration_ms = 3600000 + + labels = { + env = "default" + } +} resource "google_bigquery_table" "default_schema_implementation" { dataset_id = google_bigquery_dataset.default.dataset_id @@ -68,23 +84,37 @@ resource "google_bigquery_table" "default_schema_implementation" { env = "default" } - schema = < Date: Thu, 14 Jan 2021 07:56:27 -0800 Subject: [PATCH 5/7] seperate schema from fields commits --- .../resources/resource_bigquery_table.go | 127 +-------- .../tests/resource_bigquery_table_test.go | 254 +----------------- .../docs/r/bigquery_table.html.markdown | 109 ++------ 3 files changed, 30 insertions(+), 460 deletions(-) diff --git a/third_party/terraform/resources/resource_bigquery_table.go b/third_party/terraform/resources/resource_bigquery_table.go index 921bfd175fbf..a61180f1ebed 100644 --- a/third_party/terraform/resources/resource_bigquery_table.go +++ b/third_party/terraform/resources/resource_bigquery_table.go @@ -148,10 +148,6 @@ func bigQueryTableModeEq(old, new interface{}) bool { return eq } -func bigQueryTableTypeDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { - return bigQueryTableTypeEq(old, new) -} - func bigQueryTableModeIsForceNew(old, new interface{}) bool { eq := bigQueryTableModeEq(old, new) reqToNull := old == "REQUIRED" && new == "NULLABLE" @@ -231,15 +227,6 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error { if _, hasSchema := d.GetOk("schema"); hasSchema { - if _, hasfield := d.GetOk("field.#"); hasfield { - if err := d.ForceNew("schema"); err != nil { - return err - } - if err := d.ForceNew("field.#"); err != nil { - return err - } - return nil - } oldSchema, newSchema := d.GetChange("schema") oldSchemaText := oldSchema.(string) newSchemaText := newSchema.(string) @@ -264,23 +251,6 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error } } return nil - } else if _, hasfield := d.GetOk("field.#"); hasfield { - oldCount, newCount := d.GetChange("field.#") - if oldCount.(int) > newCount.(int) { - // if array is shrinking not changeable - if err := d.ForceNew("field.#"); err != nil { - return err - } - } - for i := 0; i < oldCount.(int); i++ { - modeKey := fmt.Sprintf("field.%d.mode", i) - oldMode, newMode := d.GetChange(modeKey) - if bigQueryTableModeIsForceNew(oldMode, newMode) { - if err := d.ForceNew(modeKey); err != nil { - return err - } - } - } } return nil } @@ -581,39 +551,6 @@ func resourceBigQueryTable() *schema.Resource { }, DiffSuppressFunc: bigQueryTableSchemaDiffSuppress, Description: `A JSON schema for the table.`, - ConflictsWith: []string{"field"}, - }, - // Field: A field in the table's schema. - "field": { - Type: schema.TypeList, - Optional: true, - ConflictsWith: []string{"schema"}, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - "type": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: bigQueryTableTypeDiffSuppress, - ForceNew: true, - }, - "description": { - Type: schema.TypeString, - Computed: true, - Optional: true, - }, - "mode": { - Type: schema.TypeString, - Optional: true, - Default: "NULLABLE", - ValidateFunc: validation.StringInSlice([]string{"NULLABLE", "REQUIRED", "REPEATED"}, false), - }, - }, - }, }, // View: [Optional] If specified, configures this table as a view. "view": { @@ -954,12 +891,6 @@ func resourceTable(d *schema.ResourceData, meta interface{}) (*bigquery.Table, e return nil, err } table.Schema = schema - } else if v, ok := d.GetOk("field"); ok { - schema, err := expandFields(v) - if err != nil { - return nil, err - } - table.Schema = schema } if v, ok := d.GetOk("time_partitioning"); ok { @@ -1141,20 +1072,12 @@ func resourceBigQueryTableRead(d *schema.ResourceData, meta interface{}) error { } if res.Schema != nil { - _, hasSchema := d.GetOk("schema") - _, hasFields := d.GetOk("field.#") - if hasSchema || !hasFields { - schema, err := flattenSchema(res.Schema) - if err != nil { - return err - } - if err := d.Set("schema", schema); err != nil { - return fmt.Errorf("Error setting schema: %s", err) - } - } else { - if err := d.Set("field", flattenTableFields(res.Schema.Fields)); err != nil { - return fmt.Errorf("Error setting fields: %s", err) - } + schema, err := flattenSchema(res.Schema) + if err != nil { + return err + } + if err := d.Set("schema", schema); err != nil { + return fmt.Errorf("Error setting schema: %s", err) } } @@ -1490,31 +1413,6 @@ func expandTimePartitioning(configured interface{}) *bigquery.TimePartitioning { return tp } -func expandFields(raw interface{}) (*bigquery.TableSchema, error) { - fields, ok := raw.([]interface{}) - if !ok { - log.Printf("[DEBUG] - unable to cast schema fields to array") - return nil, errors.New("unable to cast schema fields to array") - } - tableSchema := &bigquery.TableSchema{} - tableSchema.Fields = make([]*bigquery.TableFieldSchema, len(fields)) - for i, v := range fields { - field, ok := v.(map[string]interface{}) - if !ok { - log.Printf("[DEBUG] - unable to cast schema field to map") - return nil, errors.New("unable to cast schema field to map") - } - log.Printf("[DEBUG] - raw - %s, cast - %s", raw, fields) - tableSchema.Fields[i] = &bigquery.TableFieldSchema{ - Name: field["name"].(string), - Type: field["type"].(string), - Description: field["description"].(string), - Mode: field["mode"].(string), - } - } - return tableSchema, nil -} - func expandRangePartitioning(configured interface{}) (*bigquery.RangePartitioning, error) { if configured == nil { return nil, nil @@ -1629,19 +1527,6 @@ func flattenMaterializedView(mvd *bigquery.MaterializedViewDefinition) []map[str return []map[string]interface{}{result} } -func flattenTableFields(fields []*bigquery.TableFieldSchema) []map[string]interface{} { - result := make([]map[string]interface{}, 0, len(fields)) - for _, field := range fields { - fieldMap := make(map[string]interface{}) - fieldMap["name"] = field.Name - fieldMap["type"] = field.Type - fieldMap["description"] = field.Description - fieldMap["mode"] = field.Mode - result = append(result, fieldMap) - } - return result -} - func resourceBigQueryTableImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { config := meta.(*Config) if err := parseImportId([]string{ diff --git a/third_party/terraform/tests/resource_bigquery_table_test.go b/third_party/terraform/tests/resource_bigquery_table_test.go index 57a28a57a8a9..2fd05402d452 100644 --- a/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/third_party/terraform/tests/resource_bigquery_table_test.go @@ -450,62 +450,6 @@ func TestAccBigQueryDataTable_jsonEquivalency(t *testing.T) { }) } -func TestAccBigQueryDataTable_schemaToFieldsAndBack(t *testing.T) { - t.Parallel() - - datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) - tableID := fmt.Sprintf("tf_test_%s", randString(t, 10)) - - vcrTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t), - Steps: []resource.TestStep{ - { - Config: testAccBigQueryTable_jsonEq(datasetID, tableID), - }, - { - ResourceName: "google_bigquery_table.test", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"etag", "last_modified_time"}, - }, - { - Config: testAccBigQueryTable_fields(datasetID, tableID), - Check: testAccCheckBigQueryTableFields(t), - }, - { - Config: testAccBigQueryTable_jsonEq(datasetID, tableID), - }, - { - ResourceName: "google_bigquery_table.test", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"etag", "last_modified_time"}, - }, - }, - }) -} - -func TestAccBigQueryDataTable_fields(t *testing.T) { - t.Parallel() - - datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) - tableID := fmt.Sprintf("tf_test_%s", randString(t, 10)) - - vcrTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t), - Steps: []resource.TestStep{ - { - Config: testAccBigQueryTable_fields(datasetID, tableID), - Check: testAccCheckBigQueryTableFields(t), - }, - }, - }) -} - func TestUnitBigQueryDataTable_jsonEquivalency(t *testing.T) { t.Parallel() @@ -541,26 +485,6 @@ func TestUnitBigQueryDataTable_schemaIsChangable(t *testing.T) { } } -func TestUnitBigQueryDataTable_customizedDiffSchema(t *testing.T) { - t.Parallel() - - tcs := testUnitBigQueryDataCustomDiffFieldChangeTestcases - - for _, changeableTestcase := range testUnitBigQueryDataTableIsChangableTestCases { - extraTestcase := testUnitBigQueryDataCustomDiffFieldChangeTestcase{ - name: changeableTestcase.name, - schemaBefore: changeableTestcase.jsonOld, - schemaAfter: changeableTestcase.jsonNew, - shouldForceNew: !changeableTestcase.changeable, - } - tcs = append(tcs, extraTestcase) - } - - for _, testcase := range tcs { - testcase.check(t) - } -} - type testUnitBigQueryDataTableJSONEquivalencyTestCase struct { jsonA string jsonB string @@ -574,20 +498,6 @@ type testUnitBigQueryDataTableJSONChangeableTestCase struct { changeable bool } -type testUnitBigQueryDataCustomDiffField struct { - mode string - description string -} - -type testUnitBigQueryDataCustomDiffFieldChangeTestcase struct { - name string - fieldBefore []testUnitBigQueryDataCustomDiffField - fieldAfter []testUnitBigQueryDataCustomDiffField - schemaBefore string - schemaAfter string - shouldForceNew bool -} - func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testing.T) { var old, new interface{} if err := json.Unmarshal([]byte(testcase.jsonOld), &old); err != nil { @@ -603,40 +513,21 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin if changeable != testcase.changeable { t.Errorf("expected changeable result of %v but got %v for testcase %s", testcase.changeable, changeable, testcase.name) } -} -func (testcase *testUnitBigQueryDataCustomDiffFieldChangeTestcase) check(t *testing.T) { d := &ResourceDiffMock{ Before: map[string]interface{}{}, After: map[string]interface{}{}, } - if testcase.fieldBefore != nil { - d.Before["field.#"] = len(testcase.fieldBefore) - for i, f := range testcase.fieldBefore { - d.Before[fmt.Sprintf("field.%d.mode", i)] = f.mode - d.Before[fmt.Sprintf("field.%d.description", i)] = f.description - } - } - if testcase.fieldAfter != nil { - d.After["field.#"] = len(testcase.fieldAfter) - for i, f := range testcase.fieldAfter { - d.After[fmt.Sprintf("field.%d.mode", i)] = f.mode - d.After[fmt.Sprintf("field.%d.description", i)] = f.description - } - } - if testcase.schemaBefore != "" { - d.Before["schema"] = testcase.schemaBefore - } - if testcase.schemaAfter != "" { - d.After["schema"] = testcase.schemaAfter - } - err := resourceBigQueryTableSchemaCustomizeDiffFunc(d) + 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.shouldForceNew != d.IsForceNew { - t.Errorf("%s: expected d.IsForceNew to be %v, but was %v", testcase.name, testcase.shouldForceNew, d.IsForceNew) + if !testcase.changeable != d.IsForceNew { + t.Errorf("%s: expected d.IsForceNew to be %v, but was %v", testcase.name, !testcase.changeable, d.IsForceNew) } } @@ -709,89 +600,6 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ }, } -var testUnitBigQueryDataCustomDiffFieldChangeTestcases = []testUnitBigQueryDataCustomDiffFieldChangeTestcase{ - { - name: "control", - fieldBefore: []testUnitBigQueryDataCustomDiffField{{ - mode: "Nullable", - description: "someValue", - }}, - fieldAfter: []testUnitBigQueryDataCustomDiffField{{ - mode: "Nullable", - description: "someValue", - }}, - shouldForceNew: false, - }, - { - name: "requiredToNullable", - fieldBefore: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }}, - fieldAfter: []testUnitBigQueryDataCustomDiffField{{ - mode: "NULLABLE", - description: "someValue", - }}, - shouldForceNew: false, - }, - { - name: "descriptionChanged", - fieldBefore: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }}, - fieldAfter: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "some other value", - }}, - shouldForceNew: false, - }, - { - name: "nullToRequired", - fieldBefore: []testUnitBigQueryDataCustomDiffField{{ - mode: "NULLABLE", - description: "someValue", - }}, - fieldAfter: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }}, - shouldForceNew: true, - }, - { - name: "arraySizeIncreases", - fieldBefore: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }}, - fieldAfter: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }, - { - mode: "REQUIRED", - description: "some other value", - }}, - shouldForceNew: false, - }, - { - name: "arraySizeDecreases", - fieldBefore: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }, - { - mode: "REQUIRED", - description: "someValue", - }}, - fieldAfter: []testUnitBigQueryDataCustomDiffField{{ - mode: "REQUIRED", - description: "someValue", - }}, - shouldForceNew: true, - }, -} - var testUnitBigQueryDataTableJSONEquivalencyTestCases = []testUnitBigQueryDataTableJSONEquivalencyTestCase{ { "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", @@ -911,25 +719,6 @@ func testAccCheckBigQueryTableDestroyProducer(t *testing.T) func(s *terraform.St } } -func testAccCheckBigQueryTableFields(t *testing.T) func(s *terraform.State) error { - return func(s *terraform.State) error { - for _, rs := range s.RootModule().Resources { - if rs.Type != "google_bigquery_table" { - continue - } - config := googleProviderConfig(t) - bqt, err := config.NewBigQueryClient(config.userAgent).Tables.Get(config.Project, rs.Primary.Attributes["dataset_id"], rs.Primary.Attributes["table_id"]).Do() - if err != nil { - return err - } - if bqt.Schema.Fields[1].Description != "testegg" { - t.Errorf("schema field not set to testegg for fields scenario") - } - } - return nil - } -} - func testAccBigQueryTableTimePartitioning(datasetID, tableID, partitioningType string) string { return fmt.Sprintf(` resource "google_bigquery_dataset" "test" { @@ -1581,37 +1370,6 @@ resource "google_bigquery_table" "test" { `, datasetID, tableID) } -func testAccBigQueryTable_fields(datasetID, tableID string) string { - return fmt.Sprintf(` -resource "google_bigquery_dataset" "test" { - dataset_id = "%s" -} - -resource "google_bigquery_table" "test" { - table_id = "%s" - dataset_id = google_bigquery_dataset.test.dataset_id - - friendly_name = "bigquerytest" - labels = { - "terrafrom_managed" = "true" - } - - field { - description = "Time snapshot was taken, in Epoch milliseconds. Same across all rows and all tables in the snapshot, and uniquely defines a particular snapshot." - name = "snapshot_timestamp" - mode = "NULLABLE" - type = "INTEGER" - } - - field { - description = "testegg" - name = "creation_time" - type = "TIMESTAMP" - } -} -`, datasetID, tableID) -} - var TEST_CSV = `lifelock,LifeLock,,web,Tempe,AZ,1-May-07,6850000,USD,b lifelock,LifeLock,,web,Tempe,AZ,1-Oct-06,6000000,USD,a lifelock,LifeLock,,web,Tempe,AZ,1-Jan-08,25000000,USD,c diff --git a/third_party/terraform/website/docs/r/bigquery_table.html.markdown b/third_party/terraform/website/docs/r/bigquery_table.html.markdown index 53ff3ee9dd83..8ea824628d7a 100644 --- a/third_party/terraform/website/docs/r/bigquery_table.html.markdown +++ b/third_party/terraform/website/docs/r/bigquery_table.html.markdown @@ -14,7 +14,7 @@ Creates a table resource in a dataset for Google BigQuery. For more information [API](https://cloud.google.com/bigquery/docs/reference/rest/v2/tables). -## Example Usage - field implementation +## Example Usage ```hcl resource "google_bigquery_dataset" "default" { @@ -41,80 +41,23 @@ resource "google_bigquery_table" "default" { env = "default" } - field { - name = "permalink" - type = "STRING" - mode = "NULLABLE" - description = "The Permalink" + schema = < Date: Thu, 14 Jan 2021 13:26:53 -0800 Subject: [PATCH 6/7] clean up test code and refactor for better readability --- .../resources/resource_bigquery_table.go | 13 ++- .../tests/resource_bigquery_table_test.go | 94 +++++++++---------- third_party/terraform/utils/test_utils.go | 10 +- 3 files changed, 60 insertions(+), 57 deletions(-) diff --git a/third_party/terraform/resources/resource_bigquery_table.go b/third_party/terraform/resources/resource_bigquery_table.go index a61180f1ebed..d3adb0fa654c 100644 --- a/third_party/terraform/resources/resource_bigquery_table.go +++ b/third_party/terraform/resources/resource_bigquery_table.go @@ -164,13 +164,14 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) if !ok { // if not both arrays not changeable return false, nil - } else if len(arrayOld) > len(arrayNew) { + } + if len(arrayOld) > len(arrayNew) { // if not growing not changeable return false, nil } for i := range arrayOld { - isChangable, err := resourceBigQueryTableSchemaIsChangeable(arrayOld[i], arrayNew[i]) - if err != nil || !isChangable { + if isChangable, err := + resourceBigQueryTableSchemaIsChangeable(arrayOld[i], arrayNew[i]); err != nil || !isChangable { return false, err } } @@ -237,15 +238,17 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error var old, new interface{} if err := json.Unmarshal([]byte(oldSchemaText), &old); err != nil { log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) + return err } if err := json.Unmarshal([]byte(newSchemaText), &new); err != nil { log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) + return err } - isChangable, err := resourceBigQueryTableSchemaIsChangeable(old, new) + isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new) if err != nil { return err } - if !isChangable { + if !isChangeable { if err := d.ForceNew("schema"); err != nil { return err } diff --git a/third_party/terraform/tests/resource_bigquery_table_test.go b/third_party/terraform/tests/resource_bigquery_table_test.go index 2fd05402d452..dd875fe14658 100644 --- a/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/third_party/terraform/tests/resource_bigquery_table_test.go @@ -501,14 +501,14 @@ type testUnitBigQueryDataTableJSONChangeableTestCase struct { func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testing.T) { var old, new interface{} if err := json.Unmarshal([]byte(testcase.jsonOld), &old); err != nil { - panic(fmt.Sprintf("unable to unmarshal json - %v", err)) + t.Fatalf("unable to unmarshal json - %v", err) } if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil { - panic(fmt.Sprintf("unable to unmarshal json - %v", err)) + t.Fatalf("unable to unmarshal json - %v", err) } changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new) if err != nil { - t.Errorf("ahhhh an error I did not expect this! especially not on testscase %s - %s", testcase.name, err) + 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) @@ -533,70 +533,70 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{ { - "defaultEquality", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - true, + name: "defaultEquality", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + changeable: true, }, { - "arraySizeIncreases", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - 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, }, { - "arraySizeDecreases", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - false, + 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, }, { - "descriptionChanges", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", - true, + name: "descriptionChanges", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + changeable: true, }, { - "typeInteger", - "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"INT64\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", - 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, }, { - "typeFloat", - "[{\"name\": \"someValue\", \"type\" : \"FLOAT\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"FLOAT64\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", - 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, }, { - "typeBool", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"BOOL\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", - 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, }, { - "typeRandom", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"DATETIME\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", - false, + name: "typeChangeIncompatible", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"DATETIME\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + changeable: false, }, { - "typeModeReqToNull", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", - true, + name: "typeModeReqToNull", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + changeable: true, }, { - "typeModeRandom", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REPEATED\", \"description\" : \"some new value\" }]", - false, + name: "typeModeIncompatible", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REPEATED\", \"description\" : \"some new value\" }]", + changeable: false, }, { - "typeModeOmission", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", - "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"description\" : \"some new value\" }]", - false, + name: "typeModeOmission", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"description\" : \"some new value\" }]", + changeable: false, }, } diff --git a/third_party/terraform/utils/test_utils.go b/third_party/terraform/utils/test_utils.go index b46eab955a71..56c423f05a41 100644 --- a/third_party/terraform/utils/test_utils.go +++ b/third_party/terraform/utils/test_utils.go @@ -39,11 +39,6 @@ func (d *ResourceDataMock) GetOk(key string) (interface{}, bool) { } } -func (d *ResourceDiffMock) GetOk(key string) (interface{}, bool) { - v, ok := d.After[key] - return v, ok -} - func (d *ResourceDataMock) GetOkExists(key string) (interface{}, bool) { for k, v := range d.FieldsInSchema { if key == k { @@ -91,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{}{} From b92a165614d87d3bc8e4afb757a788d272651cd0 Mon Sep 17 00:00:00 2001 From: Scott Suarez Date: Fri, 15 Jan 2021 09:21:54 -0800 Subject: [PATCH 7/7] fix tests, don't return error from json unmarshall --- third_party/terraform/resources/resource_bigquery_table.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/third_party/terraform/resources/resource_bigquery_table.go b/third_party/terraform/resources/resource_bigquery_table.go index d3adb0fa654c..baf643fed2fa 100644 --- a/third_party/terraform/resources/resource_bigquery_table.go +++ b/third_party/terraform/resources/resource_bigquery_table.go @@ -237,12 +237,13 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d TerraformResourceDiff) error } 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) - return err } if err := json.Unmarshal([]byte(newSchemaText), &new); err != nil { + // same as above log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) - return err } isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new) if err != nil {