From a273c9bd8bbdd0f423fa32a35de78c00dde38410 Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Tue, 15 Oct 2024 14:28:49 +0000 Subject: [PATCH] Fix the regression of handling deleted IAM members in google_bigquery_dataset_iam* (#11898) [upstream:0b8a8585660d86b3ee6494caa91dfa737f3ff507] Signed-off-by: Modular Magician --- .changelog/11898.txt | 3 + .../services/bigquery/iam_bigquery_dataset.go | 4 +- .../bigquery/iam_bigquery_member_dataset.go | 4 +- ...source_bigquery_dataset_iam_member_test.go | 103 ++++++++++++++++-- 4 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 .changelog/11898.txt diff --git a/.changelog/11898.txt b/.changelog/11898.txt new file mode 100644 index 00000000000..0b60e4dfd48 --- /dev/null +++ b/.changelog/11898.txt @@ -0,0 +1,3 @@ +```release-note:bug +bigquery: fixed a regression that caused `google_bigquery_dataset_iam_*` resources to attempt to set deleted IAM members, thereby triggering an API error +``` \ No newline at end of file diff --git a/google/services/bigquery/iam_bigquery_dataset.go b/google/services/bigquery/iam_bigquery_dataset.go index fc6fa3c7cde..f7037e4735b 100644 --- a/google/services/bigquery/iam_bigquery_dataset.go +++ b/google/services/bigquery/iam_bigquery_dataset.go @@ -191,7 +191,7 @@ func policyToAccess(policy *cloudresourcemanager.Policy) ([]map[string]interface } for _, member := range binding.Members { // Do not append any deleted members - if strings.HasPrefix(member, "deleted:") { + if strings.HasPrefix(member, "iamMember:deleted:") { continue } access := map[string]interface{}{ @@ -213,7 +213,7 @@ func policyToAccess(policy *cloudresourcemanager.Policy) ([]map[string]interface // Dataset access uses different member types to identify groups, domains, etc. // these types are used as keys in the access JSON payload func iamMemberToAccess(member string) (string, string, error) { - if strings.HasPrefix(member, "deleted:") { + if strings.HasPrefix(member, "iamMember:deleted:") { return "", "", fmt.Errorf("BigQuery Dataset IAM member is deleted: %s", member) } pieces := strings.SplitN(member, ":", 2) diff --git a/google/services/bigquery/iam_bigquery_member_dataset.go b/google/services/bigquery/iam_bigquery_member_dataset.go index cbb91c98dda..f97a0bb5016 100644 --- a/google/services/bigquery/iam_bigquery_member_dataset.go +++ b/google/services/bigquery/iam_bigquery_member_dataset.go @@ -233,7 +233,7 @@ func policyToAccessForIamMember(policy *cloudresourcemanager.Policy) ([]map[stri } for _, member := range binding.Members { // Do not append any deleted members - if strings.HasPrefix(member, "deleted:") { + if strings.HasPrefix(member, "iamMember:deleted:") { continue } access := map[string]interface{}{ @@ -255,7 +255,7 @@ func policyToAccessForIamMember(policy *cloudresourcemanager.Policy) ([]map[stri // Dataset access uses different member types to identify groups, domains, etc. // these types are used as keys in the access JSON payload func iamMemberToAccessForIamMember(member string) (string, string, error) { - if strings.HasPrefix(member, "deleted:") { + if strings.HasPrefix(member, "iamMember:deleted:") { return "", "", fmt.Errorf("BigQuery Dataset IAM member is deleted: %s", member) } pieces := strings.SplitN(member, ":", 2) diff --git a/google/services/bigquery/resource_bigquery_dataset_iam_member_test.go b/google/services/bigquery/resource_bigquery_dataset_iam_member_test.go index 1c895a2a02b..98395a35e39 100644 --- a/google/services/bigquery/resource_bigquery_dataset_iam_member_test.go +++ b/google/services/bigquery/resource_bigquery_dataset_iam_member_test.go @@ -5,7 +5,7 @@ package bigquery_test import ( "fmt" "reflect" - "strings" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -116,6 +116,48 @@ func TestAccBigqueryDatasetIamMember_iamMember(t *testing.T) { }) } +func TestAccBigqueryDatasetIamMember_withDeletedServiceAccount(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10)) + serviceAccountID := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + serviceAccountEmail := fmt.Sprintf("%s@%s.iam.gserviceaccount.com", serviceAccountID, envvar.GetTestProjectFromEnv()) + + expected := map[string]interface{}{ + "role": "roles/viewer", + "userByEmail": serviceAccountEmail, + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccBigqueryDatasetIamMember_withServiceAccount(datasetID, serviceAccountID), + Check: testAccCheckBigQueryDatasetIamMemberPresent(t, "google_bigquery_dataset.dataset", expected), + }, + { + // No change from Step 1 except for hard-coding the service account email to remove the explicit resource reference in preparation for the service accoutn deletion. + Config: testAccBigqueryDatasetIamMember_withServiceAccountEmail(datasetID, serviceAccountID, serviceAccountEmail), + Check: testAccCheckBigQueryDatasetIamMemberPresent(t, "google_bigquery_dataset.dataset", expected), + }, + { + // Delete the service account but keep the rest the same. + // The service account will show up with a "deleted:serviceAccount:" prefix and a "?uid=" suffix when getting dataset access from now on. + // The plan should show an attempt to recreate the IAM member resource because the dataset access no longer contains the original account email so there is a diff. + Config: testAccBigqueryDatasetIamMember_withServiceAccountDeleted(datasetID, serviceAccountEmail), + ExpectNonEmptyPlan: true, + }, + { + // Apply the same plan as the previous step. + // It should fail because the specified account has now been deleted, so the account email is invalid. + Config: testAccBigqueryDatasetIamMember_withServiceAccountDeleted(datasetID, serviceAccountEmail), + ExpectError: regexp.MustCompile(fmt.Sprintf(".+Service account %s does not exist.+", serviceAccountEmail)), + }, + }, + }) +} + func testAccCheckBigQueryDatasetIamMemberPresent(t *testing.T, n string, expected map[string]interface{}) resource.TestCheckFunc { return testAccCheckBigQueryDatasetIamMember(t, n, expected, true) } @@ -148,15 +190,6 @@ func testAccCheckBigQueryDatasetIamMember(t *testing.T, n string, expected map[s } access := ds["access"].([]interface{}) for _, a := range access { - if aMap, ok := a.(map[string]interface{}); ok { - if iamMember, ok := aMap["iamMember"].(string); ok { - // The iam account may have been deleted but the binding may be present for the dataset. - // This case is supposed to always throw an error. - if strings.HasPrefix(iamMember, "deleted:") { - return fmt.Errorf("Found deleted service account: %s", iamMember) - } - } - } if reflect.DeepEqual(a, expected) { if !expectPresent { return fmt.Errorf("Found access %+v, expected not present", expected) @@ -260,3 +293,53 @@ resource "google_iam_workload_identity_pool_provider" "wif_provider" { } `, datasetID, wifIDs, wifIDs) } + +func testAccBigqueryDatasetIamMember_withServiceAccount(datasetID, serviceAccountID string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} + +resource "google_service_account" "sa" { + account_id = "%s" +} + +resource "google_bigquery_dataset_iam_member" "access" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/viewer" + member = "serviceAccount:${google_service_account.sa.email}" +} +`, datasetID, serviceAccountID) +} + +func testAccBigqueryDatasetIamMember_withServiceAccountEmail(datasetID, serviceAccountID, serviceAccountEmail string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} + +resource "google_service_account" "sa" { + account_id = "%s" +} + +resource "google_bigquery_dataset_iam_member" "access" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/viewer" + member = "serviceAccount:%s" +} +`, datasetID, serviceAccountID, serviceAccountEmail) +} + +func testAccBigqueryDatasetIamMember_withServiceAccountDeleted(datasetID, serviceAccountEmail string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset" "dataset" { + dataset_id = "%s" +} + +resource "google_bigquery_dataset_iam_member" "access" { + dataset_id = google_bigquery_dataset.dataset.dataset_id + role = "roles/viewer" + member = "serviceAccount:%s" +} +`, datasetID, serviceAccountEmail) +}