From 2a5c38d8a6d12459716e6ac3cf585e12c07c0877 Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Mon, 4 May 2020 17:56:07 -0700 Subject: [PATCH 1/5] account for iam vs primative bigquery roles --- products/bigquery/terraform.yaml | 5 ++++ .../constants/bigquery_dataset_access.go | 12 ++++++++++ .../custom_expand/bigquery_access_role.go.erb | 24 +++++++++++++++++++ 3 files changed, 41 insertions(+) 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/terraform.yaml b/products/bigquery/terraform.yaml index c15d9db8baca..2962f121cb71 100644 --- a/products/bigquery/terraform.yaml +++ b/products/bigquery/terraform.yaml @@ -91,6 +91,11 @@ overrides: !ruby/object:Overrides::ResourceOverrides properties: datasetId: !ruby/object:Overrides::Terraform::PropertyOverride ignore_read: true + role: !ruby/object:Overrides::Terraform::PropertyOverride + diff_suppress_func: 'resourceBigQueryDatasetAccessRoleDiffSuppress' + 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..2d96ba0ebd54 --- /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": "EDITOR", + "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 +} From ae443ffb0fdc87e5882269179df0f9256e060c12 Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Mon, 4 May 2020 18:37:23 -0700 Subject: [PATCH 2/5] add test, fix description --- products/bigquery/api.yaml | 2 +- .../constants/bigquery_dataset_access.go | 6 +-- .../resource_bigquery_dataset_access_test.go | 45 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) 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/templates/terraform/constants/bigquery_dataset_access.go b/templates/terraform/constants/bigquery_dataset_access.go index 2d96ba0ebd54..e5444beedb3d 100644 --- a/templates/terraform/constants/bigquery_dataset_access.go +++ b/templates/terraform/constants/bigquery_dataset_access.go @@ -1,7 +1,7 @@ var bigqueryAccessRoleToPrimitiveMap = map[string]string { - "roles/bigQuery.dataOwner": "OWNER", - "roles/bigQuery.dataEditor": "EDITOR", - "roles/bigQuery.dataViewer": "VIEWER", + "roles/bigquery.dataOwner": "OWNER", + "roles/bigquery.dataEditor": "EDITOR", + "roles/bigquery.dataViewer": "VIEWER", } func resourceBigQueryDatasetAccessRoleDiffSuppress(k, old, new string, d *schema.ResourceData) bool { 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..67ca6d471aa6 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": "EDITOR", + "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) +} From 99c4744aa4cfea72c9c310614c58aa6c71fbd556 Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Tue, 5 May 2020 09:37:19 -0700 Subject: [PATCH 3/5] s/EDITOR/WRITER --- templates/terraform/constants/bigquery_dataset_access.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/terraform/constants/bigquery_dataset_access.go b/templates/terraform/constants/bigquery_dataset_access.go index e5444beedb3d..6ef05340deb1 100644 --- a/templates/terraform/constants/bigquery_dataset_access.go +++ b/templates/terraform/constants/bigquery_dataset_access.go @@ -1,6 +1,6 @@ var bigqueryAccessRoleToPrimitiveMap = map[string]string { "roles/bigquery.dataOwner": "OWNER", - "roles/bigquery.dataEditor": "EDITOR", + "roles/bigquery.dataEditor": "WRITER", "roles/bigquery.dataViewer": "VIEWER", } From 99aa4106faf543614c777a981acca6c163d60ffc Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Tue, 5 May 2020 09:41:55 -0700 Subject: [PATCH 4/5] fix test --- .../terraform/tests/resource_bigquery_dataset_access_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 67ca6d471aa6..3cb1b0f3acd7 100644 --- a/third_party/terraform/tests/resource_bigquery_dataset_access_test.go +++ b/third_party/terraform/tests/resource_bigquery_dataset_access_test.go @@ -112,7 +112,7 @@ func TestAccBigQueryDatasetAccess_predefinedRole(t *testing.T) { datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) expected1 := map[string]interface{}{ - "role": "EDITOR", + "role": "WRITER", "domain": "google.com", } From af1f03729b3af7fb878b90c36905f68c0557e21c Mon Sep 17 00:00:00 2001 From: "emilyye@google.com" Date: Tue, 5 May 2020 19:05:41 -0700 Subject: [PATCH 5/5] comments --- products/bigquery/terraform.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/products/bigquery/terraform.yaml b/products/bigquery/terraform.yaml index 2962f121cb71..b8ad93868a9b 100644 --- a/products/bigquery/terraform.yaml +++ b/products/bigquery/terraform.yaml @@ -92,7 +92,15 @@ overrides: !ruby/object:Overrides::ResourceOverrides 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