fixing deletion for iam members after destroy is called on bigquery_d… (

GoogleCloudPlatform#11853)

mmv1/third_party/terraform/services/bigquery/iam_bigquery_member_dataset.go

@@ -127,9 +127,11 @@ func mergeAccess(newAccess []map[string]interface{}, currAccess []interface{}) [
 	mergedAccess = append(mergedAccess, newAccess...)
 
 	for _, item := range currAccess {
-		// Type assertion to check if it's amap
 		if itemMap, ok := item.(map[string]interface{}); ok {
-			mergedAccess = append(mergedAccess, itemMap)
+			// Check if the item has a "dataset" key
+			if _, ok := itemMap["dataset"]; ok {
+				mergedAccess = append(mergedAccess, itemMap)
+			}
 		}
 	}
 	return mergedAccess

mmv1/third_party/terraform/services/bigquery/resource_bigquery_dataset_iam_member_test.go

@@ -2,11 +2,16 @@ package bigquery_test
 
 import (
 	"fmt"
+	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-provider-google/google/acctest"
 	"github.com/hashicorp/terraform-provider-google/google/envvar"
+	"github.com/hashicorp/terraform-provider-google/google/tpgresource"
+	transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
 )
 
 func TestAccBigqueryDatasetIamMember_afterDatasetCreation(t *testing.T) {
@@ -17,7 +22,7 @@ func TestAccBigqueryDatasetIamMember_afterDatasetCreation(t *testing.T) {
 	authDatasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
 	saID := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))
 
-	expected := map[string]interface{}{
+	expected_auth := map[string]interface{}{
 		"dataset": map[string]interface{}{
 			"dataset": map[string]interface{}{
 				"projectId": projectID,
@@ -27,18 +32,27 @@ func TestAccBigqueryDatasetIamMember_afterDatasetCreation(t *testing.T) {
 		},
 	}
 
+	expected_sa := map[string]interface{}{
+		"role":        "roles/viewer",
+		"userByEmail": fmt.Sprintf("%s@%s.iam.gserviceaccount.com", saID, envvar.GetTestProjectFromEnv()),
+	}
+
 	acctest.VcrTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
 		Steps: []resource.TestStep{
 			{
 				Config: testAccBigqueryDatasetIamMember_afterDatasetAccessCreation(projectID, datasetID, authDatasetID, saID),
-				Check:  testAccCheckBigQueryDatasetAccessPresent(t, "google_bigquery_dataset.dataset", expected),
+				Check:  testAccCheckBigQueryDatasetIamMemberPresent(t, "google_bigquery_dataset.dataset", expected_auth),
 			},
 			{
-				// For iam_member to be non-authoritative, we want access block to be present after destroy
+				// For iam_member to be non-authoritative, we want authorized datasets to be present after destroy,
+				// but the iam resources have to be deleted
 				Config: testAccBigqueryDatasetIamMember_destroy(datasetID),
-				Check:  testAccCheckBigQueryDatasetAccessPresent(t, "google_bigquery_dataset.dataset", expected),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckBigQueryDatasetIamMemberPresent(t, "google_bigquery_dataset.dataset", expected_auth),
+					testAccCheckBigQueryDatasetIamMemberAbsent(t, "google_bigquery_dataset.dataset", expected_sa),
+				),
 			},
 		},
 	})
@@ -61,12 +75,12 @@ func TestAccBigqueryDatasetIamMember_serviceAccount(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config: testAccBigqueryDatasetIamMember_serviceAccount(datasetID, saID),
-				Check:  testAccCheckBigQueryDatasetAccessPresent(t, "google_bigquery_dataset.dataset", expected),
+				Check:  testAccCheckBigQueryDatasetIamMemberPresent(t, "google_bigquery_dataset.dataset", expected),
 			},
 			{
 				// Destroy step instead of CheckDestroy so we can check the access is removed without deleting the dataset
 				Config: testAccBigqueryDatasetIamMember_destroy(datasetID),
-				Check:  testAccCheckBigQueryDatasetAccessAbsent(t, "google_bigquery_dataset.dataset", expected),
+				Check:  testAccCheckBigQueryDatasetIamMemberAbsent(t, "google_bigquery_dataset.dataset", expected),
 			},
 		},
 	})
@@ -89,17 +103,72 @@ func TestAccBigqueryDatasetIamMember_iamMember(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config: testAccBigqueryDatasetIamMember_iamMember(datasetID, wifIDs),
-				Check:  testAccCheckBigQueryDatasetAccessPresent(t, "google_bigquery_dataset.dataset", expected),
+				Check:  testAccCheckBigQueryDatasetIamMemberPresent(t, "google_bigquery_dataset.dataset", expected),
 			},
 			{
 				// For iam_member to be non-authoritative, we want access block to be present after destroy
 				Config: testAccBigqueryDatasetIamMember_destroy(datasetID),
-				Check:  testAccCheckBigQueryDatasetAccessPresent(t, "google_bigquery_dataset.dataset", expected),
+				Check:  testAccCheckBigQueryDatasetIamMemberAbsent(t, "google_bigquery_dataset.dataset", expected),
 			},
 		},
 	})
 }
 
+func testAccCheckBigQueryDatasetIamMemberPresent(t *testing.T, n string, expected map[string]interface{}) resource.TestCheckFunc {
+	return testAccCheckBigQueryDatasetIamMember(t, n, expected, true)
+}
+
+func testAccCheckBigQueryDatasetIamMemberAbsent(t *testing.T, n string, expected map[string]interface{}) resource.TestCheckFunc {
+	return testAccCheckBigQueryDatasetIamMember(t, n, expected, false)
+}
+
+func testAccCheckBigQueryDatasetIamMember(t *testing.T, n string, expected map[string]interface{}, expectPresent bool) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("Not found: %s", n)
+		}
+
+		config := acctest.GoogleProviderConfig(t)
+		url, err := tpgresource.ReplaceVarsForTest(config, rs, "{{BigQueryBasePath}}projects/{{project}}/datasets/{{dataset_id}}")
+		if err != nil {
+			return err
+		}
+
+		ds, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{
+			Config:    config,
+			Method:    "GET",
+			RawURL:    url,
+			UserAgent: config.UserAgent,
+		})
+		if err != nil {
+			return err
+		}
+		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)
+				}
+				return nil
+			}
+		}
+		if expectPresent {
+			return fmt.Errorf("Did not find access %+v, expected present", expected)
+		}
+		return nil
+	}
+}
+
 func testAccBigqueryDatasetIamMember_destroy(datasetID string) string {
 	return fmt.Sprintf(`
 resource "google_bigquery_dataset" "dataset" {