diff --git a/.changelog/6897.txt b/.changelog/6897.txt new file mode 100644 index 00000000000..aee46830114 --- /dev/null +++ b/.changelog/6897.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +iam: Added plan-time validation for IAM members +``` diff --git a/google/resource_google_project_iam_binding_test.go b/google/resource_google_project_iam_binding_test.go index 7c1812a5b9b..da328aef0b8 100644 --- a/google/resource_google_project_iam_binding_test.go +++ b/google/resource_google_project_iam_binding_test.go @@ -2,6 +2,7 @@ package google import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -23,6 +24,7 @@ func TestAccProjectIamBinding_basic(t *testing.T) { org := getTestOrgFromEnv(t) pid := fmt.Sprintf("tf-test-%d", randInt(t)) role := "roles/compute.instanceAdmin" + member := "user:admin@hashicorptest.com" vcrTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -36,7 +38,7 @@ func TestAccProjectIamBinding_basic(t *testing.T) { }, // Apply an IAM binding { - Config: testAccProjectAssociateBindingBasic(pid, pname, org, role), + Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member), }, projectIamBindingImportStep("google_project_iam_binding.acceptance", pid, role), }, @@ -51,6 +53,7 @@ func TestAccProjectIamBinding_multiple(t *testing.T) { pid := fmt.Sprintf("tf-test-%d", randInt(t)) role := "roles/compute.instanceAdmin" role2 := "roles/viewer" + member := "user:admin@hashicorptest.com" vcrTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -65,7 +68,7 @@ func TestAccProjectIamBinding_multiple(t *testing.T) { }, // Apply an IAM binding { - Config: testAccProjectAssociateBindingBasic(pid, pname, org, role), + Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member), }, // Apply another IAM binding { @@ -116,6 +119,7 @@ func TestAccProjectIamBinding_update(t *testing.T) { org := getTestOrgFromEnv(t) pid := fmt.Sprintf("tf-test-%d", randInt(t)) role := "roles/compute.instanceAdmin" + member := "user:admin@hashicorptest.com" vcrTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -130,7 +134,7 @@ func TestAccProjectIamBinding_update(t *testing.T) { }, // Apply an IAM binding { - Config: testAccProjectAssociateBindingBasic(pid, pname, org, role), + Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member), }, projectIamBindingImportStep("google_project_iam_binding.acceptance", pid, role), @@ -248,7 +252,29 @@ func TestAccProjectIamBinding_withCondition(t *testing.T) { }) } -func testAccProjectAssociateBindingBasic(pid, name, org, role string) string { +// Test that an IAM binding with invalid members returns an error. +func TestAccProjectIamBinding_invalidMembers(t *testing.T) { + t.Parallel() + + org := getTestOrgFromEnv(t) + pid := fmt.Sprintf("tf-test-%d", randInt(t)) + role := "roles/compute.instanceAdmin" + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, "admin@hashicorptest.com"), + ExpectError: regexp.MustCompile("invalid value for members\\.0 \\(IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding\\)"), + }, + { + Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, "user:admin@hashicorptest.com"), + }, + }, + }) +} + +func testAccProjectAssociateBindingBasic(pid, name, org, role, member string) string { return fmt.Sprintf(` resource "google_project" "acceptance" { project_id = "%s" @@ -258,10 +284,10 @@ resource "google_project" "acceptance" { resource "google_project_iam_binding" "acceptance" { project = google_project.acceptance.project_id - members = ["user:admin@hashicorptest.com"] + members = ["%s"] role = "%s" } -`, pid, name, org, role) +`, pid, name, org, member, role) } func testAccProjectAssociateBindingMultiple(pid, name, org, role, role2 string) string { diff --git a/google/resource_google_project_iam_member_test.go b/google/resource_google_project_iam_member_test.go index be05f5f6196..0adf3cd99f6 100644 --- a/google/resource_google_project_iam_member_test.go +++ b/google/resource_google_project_iam_member_test.go @@ -2,6 +2,7 @@ package google import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -167,6 +168,28 @@ func TestAccProjectIamMember_withCondition(t *testing.T) { }) } +func TestAccProjectIamMember_invalidMembers(t *testing.T) { + t.Parallel() + + org := getTestOrgFromEnv(t) + pid := fmt.Sprintf("tf-test-%d", randInt(t)) + role := "roles/compute.instanceAdmin" + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccProjectAssociateMemberBasic(pid, pname, org, role, "admin@hashicorptest.com"), + ExpectError: regexp.MustCompile("invalid value for member \\(IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding\\)"), + }, + { + Config: testAccProjectAssociateMemberBasic(pid, pname, org, role, "user:admin@hashicorptest.com"), + }, + }, + }) +} + func testAccProjectAssociateMemberBasic(pid, name, org, role, member string) string { return fmt.Sprintf(` resource "google_project" "acceptance" { diff --git a/google/resource_google_project_iam_policy_test.go b/google/resource_google_project_iam_policy_test.go index 8fe54e78111..5da3d3dcf2d 100644 --- a/google/resource_google_project_iam_policy_test.go +++ b/google/resource_google_project_iam_policy_test.go @@ -3,6 +3,7 @@ package google import ( "encoding/json" "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -16,6 +17,7 @@ func TestAccProjectIamPolicy_basic(t *testing.T) { org := getTestOrgFromEnv(t) pid := fmt.Sprintf("tf-test-%d", randInt(t)) + member := "user:evanbrown@google.com" vcrTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -30,7 +32,7 @@ func TestAccProjectIamPolicy_basic(t *testing.T) { // Apply an IAM policy from a data source. The application // merges policies, so we validate the expected state. { - Config: testAccProjectAssociatePolicyBasic(pid, pname, org), + Config: testAccProjectAssociatePolicyBasic(pid, pname, org, member), }, { ResourceName: "google_project_iam_policy.acceptance", @@ -156,6 +158,28 @@ func TestAccProjectIamPolicy_withCondition(t *testing.T) { }) } +// Test that an IAM policy with invalid members returns errors. +func TestAccProjectIamPolicy_invalidMembers(t *testing.T) { + t.Parallel() + + org := getTestOrgFromEnv(t) + pid := fmt.Sprintf("tf-test-%d", randInt(t)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccProjectAssociatePolicyBasic(pid, pname, org, "admin@hashicorptest.com"), + ExpectError: regexp.MustCompile("invalid value for bindings\\.1\\.members\\.0 \\(IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding\\)"), + }, + { + Config: testAccProjectAssociatePolicyBasic(pid, pname, org, "user:admin@hashicorptest.com"), + }, + }, + }) +} + func getStatePrimaryResource(s *terraform.State, res, expectedID string) (*terraform.InstanceState, error) { // Get the project resource resource, ok := s.RootModule().Resources[res] @@ -228,7 +252,7 @@ func testAccProjectExistingPolicy(t *testing.T, pid string) resource.TestCheckFu } } -func testAccProjectAssociatePolicyBasic(pid, name, org string) string { +func testAccProjectAssociatePolicyBasic(pid, name, org, member string) string { return fmt.Sprintf(` resource "google_project" "acceptance" { project_id = "%s" @@ -245,7 +269,7 @@ data "google_iam_policy" "admin" { binding { role = "roles/storage.objectViewer" members = [ - "user:evanbrown@google.com", + "%s", ] } binding { @@ -256,7 +280,7 @@ data "google_iam_policy" "admin" { ] } } -`, pid, name, org) +`, pid, name, org, member) } func testAccProjectAssociatePolicyAuditConfigBasic(pid, name, org string) string { diff --git a/google/resource_iam_binding.go b/google/resource_iam_binding.go index 1c05e2adef8..0e1bb41116d 100644 --- a/google/resource_iam_binding.go +++ b/google/resource_iam_binding.go @@ -4,12 +4,10 @@ import ( "errors" "fmt" "log" - "regexp" "strings" "github.com/davecgh/go-spew/spew" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "google.golang.org/api/cloudresourcemanager/v1" ) @@ -25,7 +23,7 @@ var iamBindingSchema = map[string]*schema.Schema{ Elem: &schema.Schema{ Type: schema.TypeString, DiffSuppressFunc: caseDiffSuppress, - ValidateFunc: validation.StringDoesNotMatch(regexp.MustCompile("^deleted:"), "Terraform does not support IAM bindings for deleted principals"), + ValidateFunc: validateIAMMember, }, Set: func(v interface{}) int { return schema.HashString(strings.ToLower(v.(string))) diff --git a/google/resource_iam_member.go b/google/resource_iam_member.go index 1c9ba8c9091..8d060ac391a 100644 --- a/google/resource_iam_member.go +++ b/google/resource_iam_member.go @@ -9,7 +9,6 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "google.golang.org/api/cloudresourcemanager/v1" ) @@ -21,6 +20,26 @@ func iamMemberCaseDiffSuppress(k, old, new string, d *schema.ResourceData) bool return caseDiffSuppress(k, old, new, d) } +func validateIAMMember(i interface{}, k string) ([]string, []error) { + v, ok := i.(string) + if !ok { + return nil, []error{fmt.Errorf("expected type of %s to be string", k)} + } + + if matched, err := regexp.MatchString("^deleted", v); err != nil { + return nil, []error{fmt.Errorf("error validating %s: %v", k, err)} + } else if matched { + return nil, []error{fmt.Errorf("invalid value for %s (Terraform does not support IAM members for deleted principals)", k)} + } + + if matched, err := regexp.MatchString("(.+:.+|allUsers|allAuthenticatedUsers)", v); err != nil { + return nil, []error{fmt.Errorf("error validating %s: %v", k, err)} + } else if !matched { + return nil, []error{fmt.Errorf("invalid value for %s (IAM members must have one of the values outlined here: https://cloud.google.com/billing/docs/reference/rest/v1/Policy#Binding)", k)} + } + return nil, nil +} + var IamMemberBaseSchema = map[string]*schema.Schema{ "role": { Type: schema.TypeString, @@ -32,7 +51,7 @@ var IamMemberBaseSchema = map[string]*schema.Schema{ Required: true, ForceNew: true, DiffSuppressFunc: iamMemberCaseDiffSuppress, - ValidateFunc: validation.StringDoesNotMatch(regexp.MustCompile("^deleted:"), "Terraform does not support IAM members for deleted principals"), + ValidateFunc: validateIAMMember, }, "condition": { Type: schema.TypeList, diff --git a/google/resource_iam_policy.go b/google/resource_iam_policy.go index d5f7164a8b4..161f3c951cb 100644 --- a/google/resource_iam_policy.go +++ b/google/resource_iam_policy.go @@ -179,9 +179,15 @@ func unmarshalIamPolicy(policyData string) (*cloudresourcemanager.Policy, error) } func validateIamPolicy(i interface{}, k string) (s []string, es []error) { - _, err := unmarshalIamPolicy(i.(string)) - if err != nil { + if policy, err := unmarshalIamPolicy(i.(string)); err != nil { es = append(es, err) + } else { + for i, binding := range policy.Bindings { + for j, member := range binding.Members { + _, memberErrors := validateIAMMember(member, fmt.Sprintf("bindings.%d.members.%d", i, j)) + es = append(es, memberErrors...) + } + } } return } diff --git a/google/resource_storage_bucket_iam_test.go b/google/resource_storage_bucket_iam_test.go index 36dc38043d6..7e75d1cb239 100644 --- a/google/resource_storage_bucket_iam_test.go +++ b/google/resource_storage_bucket_iam_test.go @@ -38,6 +38,10 @@ func TestAccStorageBucketIamPolicy(t *testing.T) { ImportState: true, ImportStateVerify: true, }, + { + // Test IAM Policy with member 'allUsers' + Config: testAccStorageBucketIamPolicy_allUsers(bucket), + }, }, }) } @@ -118,3 +122,27 @@ resource "google_storage_bucket_iam_policy" "bucket-binding" { } `, bucket, account, serviceAcct) } + +func testAccStorageBucketIamPolicy_allUsers(bucket string) string { + return fmt.Sprintf(` +resource "google_storage_bucket" "bucket" { + name = "%s" + location = "US" +} + +data "google_iam_policy" "foo-policy" { + binding { + role = "roles/storage.objectViewer" + members = [ + "allUsers", + "allAuthenticatedUsers", + ] + } +} + +resource "google_storage_bucket_iam_policy" "bucket-binding" { + bucket = google_storage_bucket.bucket.name + policy_data = data.google_iam_policy.foo-policy.policy_data +} +`, bucket) +}