From c591599428e00fadb7635843dbb5e90fbd36fae1 Mon Sep 17 00:00:00 2001 From: megan07 Date: Tue, 18 Aug 2020 09:21:02 -0500 Subject: [PATCH] fix bug with data access iam member (#3741) * fix bug with data access iam member * update to still allow iam_member if prefix is not in map * update to add special groups without prefixes to map * updates post create rather than using customize diff * clean up map * undo unnecessary changes to nested_query * add comment --- products/bigquery/terraform.yaml | 14 ++- .../constants/bigquery_dataset_access.go | 12 -- .../constants/bigquery_dataset_access.go.erb | 114 ++++++++++++++++++ .../bigquery_dataset_access.go.erb | 19 +++ .../bigquery_dataset_access.go.erb | 34 ++++++ .../resource_bigquery_dataset_access_test.go | 104 ++++++++++++++++ 6 files changed, 284 insertions(+), 13 deletions(-) delete mode 100644 templates/terraform/constants/bigquery_dataset_access.go create mode 100644 templates/terraform/constants/bigquery_dataset_access.go.erb create mode 100644 templates/terraform/extra_schema_entry/bigquery_dataset_access.go.erb create mode 100644 templates/terraform/post_create/bigquery_dataset_access.go.erb diff --git a/products/bigquery/terraform.yaml b/products/bigquery/terraform.yaml index cf0285efa9d5..473178dfbc40 100644 --- a/products/bigquery/terraform.yaml +++ b/products/bigquery/terraform.yaml @@ -102,8 +102,20 @@ overrides: !ruby/object:Overrides::ResourceOverrides # converting IAM roles set in state to their primitive equivalents # before comparison. custom_expand: "templates/terraform/custom_expand/bigquery_access_role.go.erb" + iamMember: !ruby/object:Overrides::Terraform::PropertyOverride + diff_suppress_func: resourceBigQueryDatasetAccessIamMemberDiffSuppress + userByEmail: !ruby/object:Overrides::Terraform::PropertyOverride + diff_suppress_func: resourceBigQueryDatasetAccessIamMemberDiffSuppress + groupByEmail: !ruby/object:Overrides::Terraform::PropertyOverride + diff_suppress_func: resourceBigQueryDatasetAccessIamMemberDiffSuppress + specialGroup: !ruby/object:Overrides::Terraform::PropertyOverride + diff_suppress_func: resourceBigQueryDatasetAccessIamMemberDiffSuppress + domain: !ruby/object:Overrides::Terraform::PropertyOverride + diff_suppress_func: resourceBigQueryDatasetAccessIamMemberDiffSuppress custom_code: !ruby/object:Provider::Terraform::CustomCode - constants: templates/terraform/constants/bigquery_dataset_access.go + constants: templates/terraform/constants/bigquery_dataset_access.go.erb + post_create: templates/terraform/post_create/bigquery_dataset_access.go.erb + extra_schema_entry: templates/terraform/extra_schema_entry/bigquery_dataset_access.go.erb 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 deleted file mode 100644 index fcf90d4c1d60..000000000000 --- a/templates/terraform/constants/bigquery_dataset_access.go +++ /dev/null @@ -1,12 +0,0 @@ -var bigqueryAccessRoleToPrimitiveMap = map[string]string { - "roles/bigquery.dataOwner": "OWNER", - "roles/bigquery.dataEditor": "WRITER", - "roles/bigquery.dataViewer": "READER", -} - -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/constants/bigquery_dataset_access.go.erb b/templates/terraform/constants/bigquery_dataset_access.go.erb new file mode 100644 index 000000000000..49065ca17ab1 --- /dev/null +++ b/templates/terraform/constants/bigquery_dataset_access.go.erb @@ -0,0 +1,114 @@ +var bigqueryAccessRoleToPrimitiveMap = map[string]string { + "roles/bigquery.dataOwner": "OWNER", + "roles/bigquery.dataEditor": "WRITER", + "roles/bigquery.dataViewer": "READER", +} + +func resourceBigQueryDatasetAccessRoleDiffSuppress(k, old, new string, d *schema.ResourceData) bool { + if primitiveRole, ok := bigqueryAccessRoleToPrimitiveMap[new]; ok { + return primitiveRole == old + } + return false +} + +// we want to diff suppress any iam_members that are configured as `iam_member`, but stored in state as a different member type +func resourceBigQueryDatasetAccessIamMemberDiffSuppress(k, old, new string, d *schema.ResourceData) bool { + if primitiveRole, ok := bigqueryAccessRoleToPrimitiveMap[new]; ok { + return primitiveRole == old + } + + if d.Get("api_updated_member") == true { + expectedIamMember := d.Get("iam_member").(string) + parts := strings.SplitAfter(expectedIamMember, ":") + + strippedIamMember := parts[0] + if len(parts) > 1 { + strippedIamMember = parts[1] + } + + if memberInState := d.Get("user_by_email").(string); memberInState != "" { + return memberInState == strippedIamMember + } + + if memberInState := d.Get("group_by_email").(string); memberInState != "" { + return memberInState == strippedIamMember + } + + if memberInState := d.Get("domain").(string); memberInState != "" { + return memberInState == strippedIamMember + } + + if memberInState := d.Get("special_group").(string); memberInState != "" { + return memberInState == strippedIamMember + } + } + + return false +} + +// this function will go through a response's access list and see if the iam_member has been reassigned to a different member_type +// if it has, it will return the member type, and the member +func resourceBigQueryDatasetAccessReassignIamMemberInNestedObjectList(d *schema.ResourceData, meta interface{}, items []interface{}) (member_type string, member interface{}, err error) { + expectedRole, err := expandNestedBigQueryDatasetAccessRole(d.Get("role"), d, meta.(*Config)) + if err != nil { + return "", nil, err + } + expectedFlattenedRole := flattenNestedBigQueryDatasetAccessRole(expectedRole, d, meta.(*Config)) + + expectedIamMember, err := expandNestedBigQueryDatasetAccessIamMember(d.Get("iam_member"), d, meta.(*Config)) + if err != nil { + return "", nil, err + } + expectedFlattenedIamMember := flattenNestedBigQueryDatasetAccessIamMember(expectedIamMember, d, meta.(*Config)) + + parts := strings.SplitAfter(expectedFlattenedIamMember.(string), ":") + + expectedStrippedIamMember := parts[0] + if len(parts) > 1 { + expectedStrippedIamMember = parts[1] + } + + // Search list for this resource. + for _, itemRaw := range items { + if itemRaw == nil { + continue + } + item := itemRaw.(map[string]interface{}) + + itemRole := flattenNestedBigQueryDatasetAccessRole(item["role"], d, meta.(*Config)) + // isEmptyValue check so that if one is nil and the other is "", that's considered a match + if !(isEmptyValue(reflect.ValueOf(itemRole)) && isEmptyValue(reflect.ValueOf(expectedFlattenedRole))) && !reflect.DeepEqual(itemRole, expectedFlattenedRole) { + log.Printf("[DEBUG] Skipping item with role= %#v, looking for %#v)", itemRole, expectedFlattenedRole) + continue + } + + itemUserByEmail := flattenNestedBigQueryDatasetAccessUserByEmail(item["userByEmail"], d, meta.(*Config)) + if reflect.DeepEqual(itemUserByEmail, expectedStrippedIamMember) { + log.Printf("[DEBUG] Iam Member changed to userByEmail= %#v)", itemUserByEmail) + return "user_by_email", itemUserByEmail, nil + } + itemGroupByEmail := flattenNestedBigQueryDatasetAccessGroupByEmail(item["groupByEmail"], d, meta.(*Config)) + if reflect.DeepEqual(itemGroupByEmail, expectedStrippedIamMember) { + log.Printf("[DEBUG] Iam Member changed to groupByEmail= %#v)", itemGroupByEmail) + return "group_by_email", itemGroupByEmail, nil + } + itemDomain := flattenNestedBigQueryDatasetAccessDomain(item["domain"], d, meta.(*Config)) + if reflect.DeepEqual(itemDomain, expectedStrippedIamMember) { + log.Printf("[DEBUG] Iam Member changed to domain= %#v)", itemDomain) + return "domain", itemDomain, nil + } + itemSpecialGroup := flattenNestedBigQueryDatasetAccessSpecialGroup(item["specialGroup"], d, meta.(*Config)) + if reflect.DeepEqual(itemSpecialGroup, expectedStrippedIamMember) { + log.Printf("[DEBUG] Iam Member changed to specialGroup= %#v)", itemSpecialGroup) + return "special_group", itemSpecialGroup, nil + } + itemIamMember := flattenNestedBigQueryDatasetAccessIamMember(item["iamMember"], d, meta.(*Config)) + if reflect.DeepEqual(itemIamMember, expectedFlattenedIamMember) { + log.Printf("[DEBUG] Iam Member stayed as iamMember= %#v)", itemIamMember) + return "", nil, nil + } + continue + } + log.Printf("[DEBUG] Did not find item for resource %q)", d.Id()) + return "", nil, nil +} \ No newline at end of file diff --git a/templates/terraform/extra_schema_entry/bigquery_dataset_access.go.erb b/templates/terraform/extra_schema_entry/bigquery_dataset_access.go.erb new file mode 100644 index 000000000000..43c02ebc2482 --- /dev/null +++ b/templates/terraform/extra_schema_entry/bigquery_dataset_access.go.erb @@ -0,0 +1,19 @@ +<%# 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. +-%> +"api_updated_member": { + Type: schema.TypeBool, + Computed: true, + Description: "If true, represents that that the iam_member in the config was translated to a different member type by the API, and is stored in state as a different member type", +}, \ No newline at end of file diff --git a/templates/terraform/post_create/bigquery_dataset_access.go.erb b/templates/terraform/post_create/bigquery_dataset_access.go.erb new file mode 100644 index 000000000000..ed93a3f2648e --- /dev/null +++ b/templates/terraform/post_create/bigquery_dataset_access.go.erb @@ -0,0 +1,34 @@ +<%# 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. +-%> + +// by default, we are not updating the member +d.Set("api_updated_member", false) + +// iam_member is a generalized attribute, if the API can map it to a different member type on the backend, it will return +// the correct member_type in the response. If it cannot be mapped to a different member type, it will stay in iam_member. +if iamMemberProp != "" { + member_type, member, err := resourceBigQueryDatasetAccessReassignIamMemberInNestedObjectList(d, meta, res["access"].([]interface{})) + if err != nil { + fmt.Println(err) + } + + // if the member type changed, we set that member_type in state (it's already in the response) and we clear iam_member + // and we set "api_updated_member" to true to acknowledge that we are making this change + if member_type != "" { + d.Set(member_type, member.(string)) + d.Set("iam_member", "") + d.Set("api_updated_member", true) + } +} \ No newline at end of file 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 5e1aea640d30..237036de7b74 100644 --- a/third_party/terraform/tests/resource_bigquery_dataset_access_test.go +++ b/third_party/terraform/tests/resource_bigquery_dataset_access_test.go @@ -151,6 +151,58 @@ func TestAccBigQueryDatasetAccess_predefinedRole(t *testing.T) { }) } +func TestAccBigQueryDatasetAccess_iamMember(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + sinkName := fmt.Sprintf("tf_test_%s", randString(t, 10)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccBigQueryDatasetAccess_iamMember(datasetID, sinkName), + }, + }, + }) +} + +func TestAccBigQueryDatasetAccess_allUsers(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccBigQueryDatasetAccess_allUsers(datasetID), + }, + { + Config: testAccBigQueryDatasetAccess_allAuthenticatedUsers(datasetID), + }, + }, + }) +} + +func TestAccBigQueryDatasetAccess_allAuthenticatedUsers(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", randString(t, 10)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccBigQueryDatasetAccess_allAuthenticatedUsers(datasetID), + }, + }, + }) +} + func testAccCheckBigQueryDatasetAccessPresent(t *testing.T, n string, expected map[string]interface{}) resource.TestCheckFunc { return testAccCheckBigQueryDatasetAccess(t, n, expected, true) } @@ -283,3 +335,55 @@ resource "google_bigquery_dataset" "dataset" { } `, role, datasetID) } + +func testAccBigQueryDatasetAccess_iamMember(datasetID, sinkName string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset_access" "dns_query_sink" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/bigquery.dataEditor" + iam_member = google_logging_project_sink.logging_sink.writer_identity +} + +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} + +resource "google_logging_project_sink" "logging_sink" { + name = "%s_logging_project_sink" + + destination = "bigquery.googleapis.com/${google_bigquery_dataset.dataset.id}" + + filter = "resource.type=\"dns_query\"" + + unique_writer_identity = true +} +`, datasetID, sinkName) +} + +func testAccBigQueryDatasetAccess_allUsers(datasetID string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset_access" "dns_query_sink" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/bigquery.dataEditor" + iam_member = "allUsers" +} + +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} +`, datasetID) +} + +func testAccBigQueryDatasetAccess_allAuthenticatedUsers(datasetID string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset_access" "dns_query_sink" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/bigquery.dataEditor" + iam_member = "allAuthenticatedUsers" +} + +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} +`, datasetID) +}