Skip to content

Commit 73f5103

Browse files
modular-magicianmodular-magician
authored
Update google_bigquery_table schema change detection to take into account presence of row access policy (#15028) (#24284)
[upstream:ca38991aa9a6cff767e5f256cb5aee2ffc9c9513] Signed-off-by: Modular Magician <[email protected]>
1 parent 86decd5 commit 73f5103

File tree

4 files changed

+235
-19
lines changed

4 files changed

+235
-19
lines changed

‎.changelog/15028.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
bigquery: updated the schema change detection for `google_bigquery_table` to take into account presence of row access policy
3+
```

‎google/services/bigquery/resource_bigquery_table.go‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,38 @@ func bigQueryTableNormalizePolicyTags(val interface{}) interface{} {
287287
return val
288288
}
289289

290+
func bigQueryTableHasRowAccessPolicy(config *transport_tpg.Config, project, datasetId, tableId string) (bool, error) {
291+
url := fmt.Sprintf("https://bigquery.googleapis.com/bigquery/v2/projects/%s/datasets/%s/tables/%s/rowAccessPolicies", project, datasetId, tableId)
292+
res, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{
293+
Config: config,
294+
Method: "GET",
295+
RawURL: url,
296+
UserAgent: config.UserAgent,
297+
})
298+
299+
if err != nil {
300+
return false, err
301+
}
302+
303+
if policies, ok := res["rowAccessPolicies"]; ok {
304+
if policiesList, ok := policies.([]interface{}); ok && len(policiesList) > 0 {
305+
log.Printf("[INFO] Table has row access policies, schema change detected dropped columns and will force the table to recreate.")
306+
return true, nil
307+
}
308+
}
309+
310+
return false, nil
311+
}
312+
313+
func bigQueryTableHasRowAccessPolicyFunc(config *transport_tpg.Config, project, datasetId, tableId string) func() (bool, error) {
314+
return func() (bool, error) {
315+
return bigQueryTableHasRowAccessPolicy(config, project, datasetId, tableId)
316+
}
317+
}
318+
290319
// Compares two existing schema implementations and decides if
291320
// it is changeable.. pairs with a force new on not changeable
292-
func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTable bool, topLevel bool) (bool, error) {
321+
func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTable bool, topLevel bool, hasRowAccessPolicyFunc func() (bool, error)) (bool, error) {
293322
switch old.(type) {
294323
case []interface{}:
295324
arrayOld := old.([]interface{})
@@ -330,7 +359,7 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTab
330359
continue
331360
}
332361

333-
isChangable, err := resourceBigQueryTableSchemaIsChangeable(mapOld[key], mapNew[key], isExternalTable, false)
362+
isChangable, err := resourceBigQueryTableSchemaIsChangeable(mapOld[key], mapNew[key], isExternalTable, false, hasRowAccessPolicyFunc)
334363
if err != nil || !isChangable {
335364
return false, err
336365
} else if isChangable && topLevel {
@@ -341,7 +370,18 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTab
341370
// in-place column dropping alongside column additions is not allowed
342371
// as of now because user intention can be ambiguous (e.g. column renaming)
343372
newColumns := len(arrayNew) - sameNameColumns
344-
return (droppedColumns == 0) || (newColumns == 0), nil
373+
isSchemaChangeable := (droppedColumns == 0) || (newColumns == 0)
374+
if isSchemaChangeable && topLevel {
375+
hasRowAccessPolicy, err := hasRowAccessPolicyFunc()
376+
if err != nil {
377+
// Default behavior when we can't get row access policies data.
378+
return isSchemaChangeable, nil
379+
}
380+
if hasRowAccessPolicy {
381+
isSchemaChangeable = false
382+
}
383+
}
384+
return isSchemaChangeable, nil
345385
case map[string]interface{}:
346386
objectOld := old.(map[string]interface{})
347387
objectNew, ok := new.(map[string]interface{})
@@ -387,7 +427,7 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTab
387427
return false, nil
388428
}
389429
case "fields":
390-
return resourceBigQueryTableSchemaIsChangeable(valOld, valNew, isExternalTable, false)
430+
return resourceBigQueryTableSchemaIsChangeable(valOld, valNew, isExternalTable, false, hasRowAccessPolicyFunc)
391431

392432
// other parameters: description, policyTags and
393433
// policyTags.names[] are changeable
@@ -404,7 +444,7 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTab
404444
}
405445
}
406446

407-
func resourceBigQueryTableSchemaCustomizeDiffFunc(d tpgresource.TerraformResourceDiff) error {
447+
func resourceBigQueryTableSchemaCustomizeDiffFunc(d tpgresource.TerraformResourceDiff, hasRowAccessPolicyFunc func() (bool, error)) error {
408448
if _, hasSchema := d.GetOk("schema"); hasSchema {
409449
oldSchema, newSchema := d.GetChange("schema")
410450
oldSchemaText := oldSchema.(string)
@@ -427,7 +467,7 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d tpgresource.TerraformResourc
427467
log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err)
428468
}
429469
_, isExternalTable := d.GetOk("external_data_configuration")
430-
isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, isExternalTable, true)
470+
isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, isExternalTable, true, hasRowAccessPolicyFunc)
431471
if err != nil {
432472
return err
433473
}
@@ -442,7 +482,15 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d tpgresource.TerraformResourc
442482
}
443483

444484
func resourceBigQueryTableSchemaCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
445-
return resourceBigQueryTableSchemaCustomizeDiffFunc(d)
485+
config := meta.(*transport_tpg.Config)
486+
project, err := tpgresource.GetProjectFromDiff(d, config)
487+
if err != nil {
488+
return err
489+
}
490+
datasetId := d.Get("dataset_id").(string)
491+
tableId := d.Get("table_id").(string)
492+
hasRowAccessPolicyFunc := bigQueryTableHasRowAccessPolicyFunc(config, project, datasetId, tableId)
493+
return resourceBigQueryTableSchemaCustomizeDiffFunc(d, hasRowAccessPolicyFunc)
446494
}
447495

448496
func validateBigQueryTableSchema(v interface{}, k string) (warnings []string, errs []error) {

‎google/services/bigquery/resource_bigquery_table_internal_test.go‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -420,13 +420,6 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin
420420
if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil {
421421
t.Fatalf("unable to unmarshal json - %v", err)
422422
}
423-
changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, testcase.isExternalTable, true)
424-
if err != nil {
425-
t.Errorf("%s failed unexpectedly: %s", testcase.name, err)
426-
}
427-
if changeable != testcase.changeable {
428-
t.Errorf("expected changeable result of %v but got %v for testcase %s", testcase.changeable, changeable, testcase.name)
429-
}
430423

431424
d := &tpgresource.ResourceDiffMock{
432425
Before: map[string]interface{}{},
@@ -441,7 +434,19 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin
441434
d.After["external_data_configuration"] = ""
442435
}
443436

444-
err = resourceBigQueryTableSchemaCustomizeDiffFunc(d)
437+
hasRowAccessPolicyFunc := func() (bool, error) {
438+
return false, nil
439+
}
440+
441+
changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, testcase.isExternalTable, true, hasRowAccessPolicyFunc)
442+
if err != nil {
443+
t.Errorf("%s failed unexpectedly: %s", testcase.name, err)
444+
}
445+
if changeable != testcase.changeable {
446+
t.Errorf("expected changeable result of %v but got %v for testcase %s", testcase.changeable, changeable, testcase.name)
447+
}
448+
449+
err = resourceBigQueryTableSchemaCustomizeDiffFunc(d, hasRowAccessPolicyFunc)
445450
if err != nil {
446451
t.Errorf("error on testcase %s - %v", testcase.name, err)
447452
}

‎google/services/bigquery/resource_bigquery_table_test.go‎

Lines changed: 164 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@ package bigquery_test
1818

1919
import (
2020
"fmt"
21-
"regexp"
22-
"strings"
23-
"testing"
24-
2521
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
2622
"github.com/hashicorp/terraform-plugin-testing/terraform"
2723
"github.com/hashicorp/terraform-provider-google/google/acctest"
2824
"github.com/hashicorp/terraform-provider-google/google/envvar"
25+
"regexp"
26+
"strings"
27+
"testing"
2928
)
3029

3130
func TestAccBigQueryTable_Basic(t *testing.T) {
@@ -1819,7 +1818,74 @@ func TestAccBigQueryTable_invalidSchemas(t *testing.T) {
18191818
})
18201819
}
18211820

1821+
func TestAccBigQueryTable_schemaColumnDropWithRowAccessPolicy(t *testing.T) {
1822+
t.Parallel()
1823+
1824+
context := map[string]interface{}{
1825+
"project_id": envvar.GetTestProjectFromEnv(),
1826+
"dataset_id": fmt.Sprintf("tf_test_dataset_%s", acctest.RandString(t, 10)),
1827+
"table_id": fmt.Sprintf("tf_test_table_%s", acctest.RandString(t, 10)),
1828+
"policy_id": fmt.Sprintf("tf_test_policy_%s", acctest.RandString(t, 10)),
1829+
}
1830+
1831+
var tableCreationTime string
1832+
1833+
acctest.VcrTest(t, resource.TestCase{
1834+
PreCheck: func() { acctest.AccTestPreCheck(t) },
1835+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
1836+
CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t),
1837+
Steps: []resource.TestStep{
1838+
{
1839+
Config: testAccBigQueryTableWithSchemaAndRowAccessPolicy(context),
1840+
Check: resource.ComposeTestCheckFunc(
1841+
// Store the creationTime of the original table
1842+
func(s *terraform.State) error {
1843+
rs, ok := s.RootModule().Resources["google_bigquery_table.test"]
1844+
if !ok {
1845+
return fmt.Errorf("Not found: google_bigquery_table.test")
1846+
}
1847+
tableCreationTime = rs.Primary.Attributes["creation_time"]
1848+
return nil
1849+
},
1850+
),
1851+
},
1852+
{
1853+
ResourceName: "google_bigquery_table.test",
1854+
ImportState: true,
1855+
ImportStateVerify: true,
1856+
ImportStateVerifyIgnore: []string{"deletion_protection", "ignore_auto_generated_schema", "generated_schema_columns"},
1857+
},
1858+
{
1859+
Config: testAccBigQueryTableWithSchemaColumnDroppedAndRowAccessPolicy(context), // Change column to trigger ForceNew
1860+
Check: resource.ComposeTestCheckFunc(
1861+
// Verify that creationTime has changed, implying that the table was recreated.
1862+
func(s *terraform.State) error {
1863+
rs, ok := s.RootModule().Resources["google_bigquery_table.test"]
1864+
if !ok {
1865+
return fmt.Errorf("Not found: google_bigquery_table.test")
1866+
}
1867+
newTimeCreated := rs.Primary.Attributes["creation_time"]
1868+
if newTimeCreated == tableCreationTime {
1869+
return fmt.Errorf("creationTime should have changed on recreation, but it's still %s", newTimeCreated)
1870+
}
1871+
return nil
1872+
},
1873+
),
1874+
ExpectNonEmptyPlan: true,
1875+
},
1876+
{
1877+
ResourceName: "google_bigquery_table.test",
1878+
ImportState: true,
1879+
ImportStateVerify: true,
1880+
ImportStateVerifyIgnore: []string{"deletion_protection", "ignore_auto_generated_schema", "generated_schema_columns"},
1881+
},
1882+
},
1883+
})
1884+
}
1885+
18221886
func TestAccBigQueryTable_schemaWithRequiredFieldAndView(t *testing.T) {
1887+
t.Parallel()
1888+
18231889
datasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
18241890
tableID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
18251891

@@ -4688,6 +4754,100 @@ resource "google_bigquery_table" "test" {
46884754
`, datasetID, tableID, schema)
46894755
}
46904756

4757+
func testAccBigQueryTableWithSchemaAndRowAccessPolicy(context map[string]interface{}) string {
4758+
return acctest.Nprintf(`
4759+
resource "google_bigquery_dataset" "test" {
4760+
dataset_id = "%{dataset_id}"
4761+
}
4762+
4763+
resource "google_bigquery_table" "test" {
4764+
deletion_protection = false
4765+
dataset_id = google_bigquery_dataset.test.dataset_id
4766+
table_id = "%{table_id}"
4767+
4768+
schema = <<EOF
4769+
[
4770+
{
4771+
"name": "user_id",
4772+
"type": "STRING",
4773+
"mode": "REQUIRED",
4774+
"description": "Unique identifier for the user"
4775+
},
4776+
{
4777+
"name": "email",
4778+
"type": "STRING",
4779+
"mode": "NULLABLE",
4780+
"description": "User's email address"
4781+
},
4782+
{
4783+
"name": "region",
4784+
"type": "STRING",
4785+
"mode": "NULLABLE",
4786+
"description": "User's assigned region"
4787+
}
4788+
]
4789+
EOF
4790+
}
4791+
4792+
resource "google_bigquery_row_access_policy" "test" {
4793+
dataset_id = google_bigquery_table.test.dataset_id
4794+
table_id = google_bigquery_table.test.table_id
4795+
policy_id = "%{policy_id}"
4796+
4797+
grantees = [
4798+
"group:[email protected]"
4799+
]
4800+
filter_predicate = "email = SESSION_USER()"
4801+
4802+
depends_on = [google_bigquery_table.test]
4803+
}
4804+
`, context)
4805+
}
4806+
4807+
func testAccBigQueryTableWithSchemaColumnDroppedAndRowAccessPolicy(context map[string]interface{}) string {
4808+
return acctest.Nprintf(`
4809+
resource "google_bigquery_dataset" "test" {
4810+
dataset_id = "%{dataset_id}"
4811+
}
4812+
4813+
resource "google_bigquery_table" "test" {
4814+
deletion_protection = false
4815+
dataset_id = google_bigquery_dataset.test.dataset_id
4816+
table_id = "%{table_id}"
4817+
4818+
schema = <<EOF
4819+
[
4820+
{
4821+
"name": "user_id",
4822+
"type": "STRING",
4823+
"mode": "REQUIRED",
4824+
"description": "Unique identifier for the user"
4825+
},
4826+
{
4827+
"name": "email",
4828+
"type": "STRING",
4829+
"mode": "NULLABLE",
4830+
"description": "User's email address"
4831+
}
4832+
]
4833+
EOF
4834+
}
4835+
4836+
resource "google_bigquery_row_access_policy" "test" {
4837+
dataset_id = google_bigquery_table.test.dataset_id
4838+
table_id = google_bigquery_table.test.table_id
4839+
policy_id = "%{policy_id}"
4840+
4841+
grantees = [
4842+
"group:[email protected]"
4843+
]
4844+
filter_predicate = "email = SESSION_USER()"
4845+
4846+
depends_on = [google_bigquery_table.test]
4847+
}
4848+
`, context)
4849+
}
4850+
46914851
func testAccBigQueryTableWithReplicationInfoAndView(datasetID, tableID string) string {
46924852
return fmt.Sprintf(`
46934853
resource "google_bigquery_dataset" "test" {

0 commit comments

Comments
 (0)