From 9f3bc2698ebb4fd1d9942a8606bd6ecd80125911 Mon Sep 17 00:00:00 2001 From: emily Date: Tue, 5 May 2020 19:53:55 -0700 Subject: [PATCH] Fix bigquery dataset access iam roles with primative equivalent (#3471) * account for iam vs primative bigquery roles * add test, fix description * s/EDITOR/WRITER * fix test * comments --- products/bigquery/api.yaml | 2 +- products/bigquery/terraform.yaml | 13 ++++++ .../constants/bigquery_dataset_access.go | 12 +++++ .../custom_expand/bigquery_access_role.go.erb | 24 ++++++++++ .../resource_bigquery_dataset_access_test.go | 45 +++++++++++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 templates/terraform/constants/bigquery_dataset_access.go create mode 100644 templates/terraform/custom_expand/bigquery_access_role.go.erb diff --git a/products/bigquery/api.yaml b/products/bigquery/api.yaml index 6a8598672db1..7464d9a0101e 100644 --- a/products/bigquery/api.yaml +++ b/products/bigquery/api.yaml @@ -61,7 +61,7 @@ objects: member of the access object. Primitive, Predefined and custom roles are supported. Predefined roles that have equivalent primitive roles are swapped by the API to their Primitive - counterparts, and will show a diff post-create. See + counterparts. See [official docs](https://cloud.google.com/bigquery/docs/access-control). - !ruby/object:Api::Type::String name: 'specialGroup' diff --git a/products/bigquery/terraform.yaml b/products/bigquery/terraform.yaml index c15d9db8baca..b8ad93868a9b 100644 --- a/products/bigquery/terraform.yaml +++ b/products/bigquery/terraform.yaml @@ -91,6 +91,19 @@ overrides: !ruby/object:Overrides::ResourceOverrides properties: datasetId: !ruby/object:Overrides::Terraform::PropertyOverride ignore_read: true + role: !ruby/object:Overrides::Terraform::PropertyOverride + # Bigquery allows for two different formats for specific roles + # (IAM vs "primitive" format), but will return the primative role in API + # responses. We identify this fine-grained resource from a list + # of DatasetAccess objects by comparing role, and we must use the same + # format when comparing. + diff_suppress_func: 'resourceBigQueryDatasetAccessRoleDiffSuppress' + # This custom expand makes sure we are correctly + # converting IAM roles set in state to their primitive equivalents + # before comparison. + custom_expand: "templates/terraform/custom_expand/bigquery_access_role.go.erb" + custom_code: !ruby/object:Provider::Terraform::CustomCode + constants: templates/terraform/constants/bigquery_dataset_access.go Job: !ruby/object:Overrides::Terraform::ResourceOverride import_format: ["projects/{{project}}/jobs/{{job_id}}"] skip_delete: true diff --git a/templates/terraform/constants/bigquery_dataset_access.go b/templates/terraform/constants/bigquery_dataset_access.go new file mode 100644 index 000000000000..6ef05340deb1 --- /dev/null +++ b/templates/terraform/constants/bigquery_dataset_access.go @@ -0,0 +1,12 @@ +var bigqueryAccessRoleToPrimitiveMap = map[string]string { + "roles/bigquery.dataOwner": "OWNER", + "roles/bigquery.dataEditor": "WRITER", + "roles/bigquery.dataViewer": "VIEWER", +} + +func resourceBigQueryDatasetAccessRoleDiffSuppress(k, old, new string, d *schema.ResourceData) bool { + if primitiveRole, ok := bigqueryAccessRoleToPrimitiveMap[new]; ok { + return primitiveRole == old + } + return false +} diff --git a/templates/terraform/custom_expand/bigquery_access_role.go.erb b/templates/terraform/custom_expand/bigquery_access_role.go.erb new file mode 100644 index 000000000000..5a5aae5115c9 --- /dev/null +++ b/templates/terraform/custom_expand/bigquery_access_role.go.erb @@ -0,0 +1,24 @@ +<%- # the license inside this block applies to this file + # Copyright 2020 Google Inc. + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-%> +func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { + if v == nil { + return nil, nil + } + + if primitiveRole, ok := bigqueryAccessRoleToPrimitiveMap[v.(string)]; ok { + return primitiveRole, nil + } + return v, nil +} diff --git a/third_party/terraform/tests/resource_bigquery_dataset_access_test.go b/third_party/terraform/tests/resource_bigquery_dataset_access_test.go index 0cbd8657ce66..3cb1b0f3acd7 100644 --- a/third_party/terraform/tests/resource_bigquery_dataset_access_test.go +++ b/third_party/terraform/tests/resource_bigquery_dataset_access_test.go @@ -106,6 +106,37 @@ func TestAccBigQueryDatasetAccess_multiple(t *testing.T) { }) } +func TestAccBigQueryDatasetAccess_predefinedRole(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + + expected1 := map[string]interface{}{ + "role": "WRITER", + "domain": "google.com", + } + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccBigQueryDatasetAccess_predefinedRole(datasetID), + Check: resource.ComposeTestCheckFunc( + testAccCheckBigQueryDatasetAccessPresent(t, "google_bigquery_dataset.dataset", expected1), + ), + }, + { + // Destroy step instead of CheckDestroy so we can check the access is removed without deleting the dataset + Config: testAccBigQueryDatasetAccess_destroy(datasetID, "dataset"), + Check: resource.ComposeTestCheckFunc( + testAccCheckBigQueryDatasetAccessAbsent(t, "google_bigquery_dataset.dataset", expected1), + ), + }, + }, + }) +} + func testAccCheckBigQueryDatasetAccessPresent(t *testing.T, n string, expected map[string]interface{}) resource.TestCheckFunc { return testAccCheckBigQueryDatasetAccess(t, n, expected, true) } @@ -224,3 +255,17 @@ resource "google_bigquery_dataset" "dataset" { } `, datasetID) } + +func testAccBigQueryDatasetAccess_predefinedRole(datasetID string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset_access" "access" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/bigquery.dataEditor" + domain = "google.com" +} + +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} +`, datasetID) +}