From 1372e70052a76bfb064d102cec8898cb42e06801 Mon Sep 17 00:00:00 2001 From: Scott Suarez Date: Thu, 28 Jan 2021 01:15:42 -0500 Subject: [PATCH] sort array of schema for bigquery_table (#4440) * sort array for bigquery_table * address bad merge * add prevent_destroy testcase * renamed functions to be specific to bigquery * remove testcases that are no longer applicable * spelling lint fix --- .../resources/resource_bigquery_table.go | 33 ++++ .../tests/resource_bigquery_table_test.go | 172 +++++++++++++++--- 2 files changed, 175 insertions(+), 30 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_bigquery_table.go b/mmv1/third_party/terraform/resources/resource_bigquery_table.go index 3500aaad74f8..3b5a5b0a285e 100644 --- a/mmv1/third_party/terraform/resources/resource_bigquery_table.go +++ b/mmv1/third_party/terraform/resources/resource_bigquery_table.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log" + "sort" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -14,6 +15,22 @@ import ( "google.golang.org/api/bigquery/v2" ) +func bigQueryTableSortArrayByName(array []interface{}) { + sort.Slice(array, func(i, k int) bool { + return array[i].(map[string]interface{})["name"].(string) < array[k].(map[string]interface{})["name"].(string) + }) +} + +func bigQueryTablecheckNameExists(jsonList []interface{}) error { + for _, m := range jsonList { + if _, ok := m.(map[string]interface{})["name"]; !ok { + return fmt.Errorf("No name in schema %+v", m) + } + } + + return nil +} + // Compares two json's while optionally taking in a compareMapKeyVal function. // This function will override any comparison of a given map[string]interface{} // on a specific key value allowing for a separate equality in specific scenarios @@ -27,6 +44,14 @@ func jsonCompareWithMapKeyOverride(a, b interface{}, compareMapKeyVal func(key s } else if len(arrayA) != len(arrayB) { return false, nil } + if err := bigQueryTablecheckNameExists(arrayA); err != nil { + return false, err + } + bigQueryTableSortArrayByName(arrayA) + if err := bigQueryTablecheckNameExists(arrayB); err != nil { + return false, err + } + bigQueryTableSortArrayByName(arrayB) for i := range arrayA { eq, err := jsonCompareWithMapKeyOverride(arrayA[i], arrayB[i], compareMapKeyVal) if err != nil { @@ -169,6 +194,14 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) // if not growing not changeable return false, nil } + if err := bigQueryTablecheckNameExists(arrayOld); err != nil { + return false, err + } + bigQueryTableSortArrayByName(arrayOld) + if err := bigQueryTablecheckNameExists(arrayNew); err != nil { + return false, err + } + bigQueryTableSortArrayByName(arrayNew) for i := range arrayOld { if isChangable, err := resourceBigQueryTableSchemaIsChangeable(arrayOld[i], arrayNew[i]); err != nil || !isChangable { diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index dd875fe14658..044df3bbf51e 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -438,7 +438,7 @@ func TestAccBigQueryDataTable_jsonEquivalency(t *testing.T) { ImportStateVerifyIgnore: []string{"etag", "last_modified_time"}, }, { - Config: testAccBigQueryTable_jsonEqUpdate(datasetID, tableID), + Config: testAccBigQueryTable_jsonEqModeRemoved(datasetID, tableID), }, { ResourceName: "google_bigquery_table.test", @@ -450,6 +450,32 @@ func TestAccBigQueryDataTable_jsonEquivalency(t *testing.T) { }) } +func TestAccBigQueryDataTable_canReorderParameters(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{ + { + // we don't run any checks because the resource will error out if + // it attempts to destroy/tear down. + Config: testAccBigQueryTable_jsonPreventDestroy(datasetID, tableID), + }, + { + Config: testAccBigQueryTable_jsonPreventDestroyOrderChanged(datasetID, tableID), + }, + { + Config: testAccBigQueryTable_jsonEq(datasetID, tableID), + }, + }, + }) +} + func TestUnitBigQueryDataTable_jsonEquivalency(t *testing.T) { t.Parallel() @@ -598,77 +624,91 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"description\" : \"some new value\" }]", changeable: false, }, + { + name: "orderOfArrayChangesAndDescriptionChanges", + jsonOld: "[{\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"value2\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"value2\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"newVal\" }, {\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + changeable: true, + }, + { + name: "orderOfArrayChangesAndNameChanges", + jsonOld: "[{\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"value2\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"value3\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"newVal\" }, {\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + changeable: false, + }, } var testUnitBigQueryDataTableJSONEquivalencyTestCases = []testUnitBigQueryDataTableJSONEquivalencyTestCase{ { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", true, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - "[{\"someKey\": \"someValue\", \"finalKey\" : {} }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + "[{\"name\": \"someValue\", \"finalKey\" : {} }]", false, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", true, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"somethingRandom\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"somethingRandom\" }]", false, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", true, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"somethingRandom\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"somethingRandom\" }]", false, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INT64\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INT64\" }]", true, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"somethingRandom\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"somethingRandom\" }]", false, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT64\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT64\" }]", true, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", false, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]", true, }, { - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", - "[{\"someKey\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", + "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", false, }, { - "[1,2,3]", - "[1,2,3]", + // order changes, name same + "[{\"name\": \"value1\", \"1\" : \"1\"},{\"name\": \"value2\", \"2\" : \"2\" }]", + "[{\"name\": \"value2\", \"2\" : \"2\" },{\"name\": \"value1\", \"1\" : \"1\" }]", true, }, { - "[1,2,3]", - "[1,2,\"banana\"]", + // order changes, value different + "[{\"name\": \"value1\", \"1\" : \"1\"},{\"name\": \"value2\", \"2\" : \"2\" }]", + "[{\"name\": \"value2\", \"2\" : \"random\" },{\"name\": \"value1\", \"1\" : \"1\" }]", false, }, } @@ -1338,7 +1378,7 @@ resource "google_bigquery_table" "test" { `, datasetID, tableID) } -func testAccBigQueryTable_jsonEqUpdate(datasetID, tableID string) string { +func testAccBigQueryTable_jsonEqModeRemoved(datasetID, tableID string) string { return fmt.Sprintf(` resource "google_bigquery_dataset" "test" { dataset_id = "%s" @@ -1370,6 +1410,78 @@ resource "google_bigquery_table" "test" { `, datasetID, tableID) } +func testAccBigQueryTable_jsonPreventDestroy(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 + lifecycle { + prevent_destroy = true + } + + friendly_name = "bigquerytest" + labels = { + "terrafrom_managed" = "true" + } + + schema = jsonencode( + [ + { + 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" + }, + { + description = "Timestamp of dataset creation" + name = "creation_time" + type = "TIMESTAMP" + }, + ]) +} +`, datasetID, tableID) +} + +func testAccBigQueryTable_jsonPreventDestroyOrderChanged(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 + lifecycle { + prevent_destroy = true + } + + friendly_name = "bigquerytest" + labels = { + "terrafrom_managed" = "true" + } + + schema = jsonencode( + [ + { + description = "Timestamp of dataset creation" + name = "creation_time" + type = "TIMESTAMP" + }, + { + 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" + }, + ]) +} +`, 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