Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#16607] Fix: support iamMember (i.e. for WIF ids) in big query datasets #9948

Merged
merged 4 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -214,27 +214,26 @@ func iamMemberToAccess(member string) (string, string, error) {
if strings.HasPrefix(member, "deleted:") {
return "", "", fmt.Errorf("BigQuery Dataset IAM member is deleted: %s", member)
}

pieces := strings.SplitN(member, ":", 2)
if len(pieces) > 1 {
switch pieces[0] {
case "group":
return "groupByEmail", pieces[1], nil
case "domain":
return "domain", pieces[1], nil
case "iamMember":
return "iamMember", pieces[1], nil
LucaPrete marked this conversation as resolved.
Show resolved Hide resolved
case "user":
return "userByEmail", pieces[1], nil
case "serviceAccount":
return "userByEmail", pieces[1], nil
default:
return "", "", fmt.Errorf("Failed to parse BigQuery Dataset IAM member type: %s", member)
}
}
if member == "projectOwners" || member == "projectReaders" || member == "projectWriters" || member == "allAuthenticatedUsers" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opportunistic refactoring/fix opportunity:
1- projectWriters, projectReaders, projectOwners are not documented as possible output https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/bigquery_dataset_iam#google_bigquery_dataset_iam_member

2- allUsers is documented but doesn't look to be a case handled by the API (?). https://cloud.google.com/bigquery/docs/reference/rest/v2/datasets#Dataset

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to ignore this comment if not relevant / I lack of context but I just ping you in case you missed this comment @LucaPrete .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, sorry..I missed this. We can certainly work on this in a separate PR.

// These are special BigQuery Dataset permissions
return "specialGroup", member, nil
}
return "iamMember", member, nil
return "", "", fmt.Errorf("Failed to parse BigQuery Dataset IAM member type: %s", member)
}

func accessToIamMember(access map[string]interface{}) (string, error) {
Expand All @@ -249,7 +248,7 @@ func accessToIamMember(access map[string]interface{}) (string, error) {
return member.(string), nil
}
if member, ok := access["iamMember"]; ok {
return member.(string), nil
return fmt.Sprintf("iamMember:%s", member.(string)), nil
}
if _, ok := access["view"]; ok {
// view does not map to an IAM member, use access instead
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/hashicorp/terraform-provider-google/google/envvar"
)

func TestAccBigqueryDatasetIamMember_basic(t *testing.T) {
func TestAccBigqueryDatasetIamMember_serviceAccount(t *testing.T) {
t.Parallel()

datasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
Expand All @@ -25,27 +25,55 @@ func TestAccBigqueryDatasetIamMember_basic(t *testing.T) {
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
Steps: []resource.TestStep{
{
Config: testAccBigqueryDatasetIamMember_basic(datasetID, saID),
Config: testAccBigqueryDatasetIamMember_serviceAccount(datasetID, saID),
Check: testAccCheckBigQueryDatasetAccessPresent(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, "dataset"),
Config: testAccBigqueryDatasetIamMember_destroy(datasetID),
Check: testAccCheckBigQueryDatasetAccessAbsent(t, "google_bigquery_dataset.dataset", expected),
},
},
})
}

func testAccBigqueryDatasetIamMember_destroy(datasetID, rs string) string {
func TestAccBigqueryDatasetIamMember_iamMember(t *testing.T) {
t.Parallel()

datasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
wifIDs := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))

expected := map[string]interface{}{
"role": "roles/viewer",
"iamMember": fmt.Sprintf("principal://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/subject/test", envvar.GetTestProjectNumberFromEnv(), wifIDs),
}

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
Steps: []resource.TestStep{
{
Config: testAccBigqueryDatasetIamMember_iamMember(datasetID, wifIDs),
Check: testAccCheckBigQueryDatasetAccessPresent(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),
},
},
})
}

func testAccBigqueryDatasetIamMember_destroy(datasetID string) string {
return fmt.Sprintf(`
resource "google_bigquery_dataset" "%s" {
resource "google_bigquery_dataset" "dataset" {
dataset_id = "%s"
}
`, rs, datasetID)
`, datasetID)
}

func testAccBigqueryDatasetIamMember_basic(datasetID, saID string) string {
func testAccBigqueryDatasetIamMember_serviceAccount(datasetID, saID string) string {
return fmt.Sprintf(`
resource "google_bigquery_dataset_iam_member" "access" {
dataset_id = google_bigquery_dataset.dataset.dataset_id
Expand All @@ -62,3 +90,32 @@ resource "google_service_account" "bqviewer" {
}
`, datasetID, saID)
}

func testAccBigqueryDatasetIamMember_iamMember(datasetID, wifIDs string) string {
return fmt.Sprintf(`
resource "google_bigquery_dataset_iam_member" "access" {
dataset_id = google_bigquery_dataset.dataset.dataset_id
role = "roles/viewer"
member = "iamMember:principal://iam.googleapis.com/${google_iam_workload_identity_pool.wif_pool.name}/subject/test"
LucaPrete marked this conversation as resolved.
Show resolved Hide resolved
}

resource "google_bigquery_dataset" "dataset" {
dataset_id = "%s"
}

resource "google_iam_workload_identity_pool" "wif_pool" {
workload_identity_pool_id = "%s"
}

resource "google_iam_workload_identity_pool_provider" "wif_provider" {
workload_identity_pool_id = google_iam_workload_identity_pool.wif_pool.workload_identity_pool_id
workload_identity_pool_provider_id = "%s"
attribute_mapping = {
"google.subject" = "assertion.sub"
}
oidc {
issuer_uri = "https://issuer-uri.com"
}
}
`, datasetID, wifIDs, wifIDs)
}
5 changes: 5 additions & 0 deletions mmv1/third_party/terraform/tpgiamresource/iam.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ func normalizeIamMemberCasing(member string) string {
if len(pieces) > 2 && !iamMemberIsCaseSensitive(strings.TrimPrefix(member, "deleted:")) {
pieces[2] = strings.ToLower(pieces[2])
}
} else if strings.HasPrefix(member, "iamMember:") {
pieces = strings.SplitN(member, ":", 3)
if len(pieces) > 2 && !iamMemberIsCaseSensitive(strings.TrimPrefix(member, "iamMember:")) {
pieces[2] = strings.ToLower(pieces[2])
}
} else if !iamMemberIsCaseSensitive(member) {
pieces = strings.SplitN(member, ":", 2)
if len(pieces) > 1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,13 @@ The following arguments are supported:

* `member/members` - (Required) Identities that will be granted the privilege in `role`.
Each entry can have one of the following values:
* **allUsers**: A special identifier that represents anyone who is on the internet; with or without a Google account.
* **allAuthenticatedUsers**: A special identifier that represents anyone who is authenticated with a Google account or a service account.
* **user:{emailid}**: An email address that represents a specific Google account. For example, [email protected] or [email protected].
* **serviceAccount:{emailid}**: An email address that represents a service account. For example, [email protected].
* **group:{emailid}**: An email address that represents a Google group. For example, [email protected].
* **allUsers**: A special identifier that represents anyone who is on the internet; with or without a Google account.
* **domain:{domain}**: A G Suite domain (primary, instead of alias) name that represents all the users of that domain. For example, google.com or example.com.
* **group:{emailid}**: An email address that represents a Google group. For example, [email protected].
* **iamMember:{principal}**: Some other type of member that appears in the IAM Policy but isn't a user, group, domain, or special group. This is used for example for workload/workforce federated identities (principal, principalSet).
* **serviceAccount:{emailid}**: An email address that represents a service account. For example, [email protected].
* **user:{emailid}**: An email address that represents a specific Google account. For example, [email protected] or [email protected].

* `role` - (Required) The role that should be applied. Only one
`google_bigquery_dataset_iam_binding` can be used per role. Note that custom roles must be of the format
Expand Down
Loading