Skip to content

Commit 89affbe

Browse files
modular-magicianmodular-magician
authored
Add validation for IAM members. (#6897) (#13203)
Signed-off-by: Modular Magician <[email protected]> Signed-off-by: Modular Magician <[email protected]>
1 parent 5bc5b3b commit 89affbe

8 files changed

+144
-17
lines changed

‎.changelog/6897.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:enhancement
2+
iam: Added plan-time validation for IAM members
3+
```

‎google/resource_google_project_iam_binding_test.go

+32-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package google
22

33
import (
44
"fmt"
5+
"regexp"
56
"testing"
67

78
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -23,6 +24,7 @@ func TestAccProjectIamBinding_basic(t *testing.T) {
2324
org := getTestOrgFromEnv(t)
2425
pid := fmt.Sprintf("tf-test-%d", randInt(t))
2526
role := "roles/compute.instanceAdmin"
27+
member := "user:[email protected]"
2628
vcrTest(t, resource.TestCase{
2729
PreCheck: func() { testAccPreCheck(t) },
2830
Providers: testAccProviders,
@@ -36,7 +38,7 @@ func TestAccProjectIamBinding_basic(t *testing.T) {
3638
},
3739
// Apply an IAM binding
3840
{
39-
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role),
41+
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member),
4042
},
4143
projectIamBindingImportStep("google_project_iam_binding.acceptance", pid, role),
4244
},
@@ -51,6 +53,7 @@ func TestAccProjectIamBinding_multiple(t *testing.T) {
5153
pid := fmt.Sprintf("tf-test-%d", randInt(t))
5254
role := "roles/compute.instanceAdmin"
5355
role2 := "roles/viewer"
56+
member := "user:[email protected]"
5457

5558
vcrTest(t, resource.TestCase{
5659
PreCheck: func() { testAccPreCheck(t) },
@@ -65,7 +68,7 @@ func TestAccProjectIamBinding_multiple(t *testing.T) {
6568
},
6669
// Apply an IAM binding
6770
{
68-
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role),
71+
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member),
6972
},
7073
// Apply another IAM binding
7174
{
@@ -116,6 +119,7 @@ func TestAccProjectIamBinding_update(t *testing.T) {
116119
org := getTestOrgFromEnv(t)
117120
pid := fmt.Sprintf("tf-test-%d", randInt(t))
118121
role := "roles/compute.instanceAdmin"
122+
member := "user:[email protected]"
119123

120124
vcrTest(t, resource.TestCase{
121125
PreCheck: func() { testAccPreCheck(t) },
@@ -130,7 +134,7 @@ func TestAccProjectIamBinding_update(t *testing.T) {
130134
},
131135
// Apply an IAM binding
132136
{
133-
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role),
137+
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, member),
134138
},
135139
projectIamBindingImportStep("google_project_iam_binding.acceptance", pid, role),
136140

@@ -248,7 +252,29 @@ func TestAccProjectIamBinding_withCondition(t *testing.T) {
248252
})
249253
}
250254

251-
func testAccProjectAssociateBindingBasic(pid, name, org, role string) string {
255+
// Test that an IAM binding with invalid members returns an error.
256+
func TestAccProjectIamBinding_invalidMembers(t *testing.T) {
257+
t.Parallel()
258+
259+
org := getTestOrgFromEnv(t)
260+
pid := fmt.Sprintf("tf-test-%d", randInt(t))
261+
role := "roles/compute.instanceAdmin"
262+
vcrTest(t, resource.TestCase{
263+
PreCheck: func() { testAccPreCheck(t) },
264+
Providers: testAccProviders,
265+
Steps: []resource.TestStep{
266+
{
267+
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, "[email protected]"),
268+
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\\)"),
269+
},
270+
{
271+
Config: testAccProjectAssociateBindingBasic(pid, pname, org, role, "user:[email protected]"),
272+
},
273+
},
274+
})
275+
}
276+
277+
func testAccProjectAssociateBindingBasic(pid, name, org, role, member string) string {
252278
return fmt.Sprintf(`
253279
resource "google_project" "acceptance" {
254280
project_id = "%s"
@@ -258,10 +284,10 @@ resource "google_project" "acceptance" {
258284
259285
resource "google_project_iam_binding" "acceptance" {
260286
project = google_project.acceptance.project_id
261-
members = ["user:[email protected]"]
287+
members = ["%s"]
262288
role = "%s"
263289
}
264-
`, pid, name, org, role)
290+
`, pid, name, org, member, role)
265291
}
266292

267293
func testAccProjectAssociateBindingMultiple(pid, name, org, role, role2 string) string {

‎google/resource_google_project_iam_member_test.go

+23
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package google
22

33
import (
44
"fmt"
5+
"regexp"
56
"testing"
67

78
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -167,6 +168,28 @@ func TestAccProjectIamMember_withCondition(t *testing.T) {
167168
})
168169
}
169170

171+
func TestAccProjectIamMember_invalidMembers(t *testing.T) {
172+
t.Parallel()
173+
174+
org := getTestOrgFromEnv(t)
175+
pid := fmt.Sprintf("tf-test-%d", randInt(t))
176+
role := "roles/compute.instanceAdmin"
177+
178+
vcrTest(t, resource.TestCase{
179+
PreCheck: func() { testAccPreCheck(t) },
180+
Providers: testAccProviders,
181+
Steps: []resource.TestStep{
182+
{
183+
Config: testAccProjectAssociateMemberBasic(pid, pname, org, role, "[email protected]"),
184+
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\\)"),
185+
},
186+
{
187+
Config: testAccProjectAssociateMemberBasic(pid, pname, org, role, "user:[email protected]"),
188+
},
189+
},
190+
})
191+
}
192+
170193
func testAccProjectAssociateMemberBasic(pid, name, org, role, member string) string {
171194
return fmt.Sprintf(`
172195
resource "google_project" "acceptance" {

‎google/resource_google_project_iam_policy_test.go

+28-4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package google
33
import (
44
"encoding/json"
55
"fmt"
6+
"regexp"
67
"testing"
78

89
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -16,6 +17,7 @@ func TestAccProjectIamPolicy_basic(t *testing.T) {
1617

1718
org := getTestOrgFromEnv(t)
1819
pid := fmt.Sprintf("tf-test-%d", randInt(t))
20+
member := "user:[email protected]"
1921
vcrTest(t, resource.TestCase{
2022
PreCheck: func() { testAccPreCheck(t) },
2123
Providers: testAccProviders,
@@ -30,7 +32,7 @@ func TestAccProjectIamPolicy_basic(t *testing.T) {
3032
// Apply an IAM policy from a data source. The application
3133
// merges policies, so we validate the expected state.
3234
{
33-
Config: testAccProjectAssociatePolicyBasic(pid, pname, org),
35+
Config: testAccProjectAssociatePolicyBasic(pid, pname, org, member),
3436
},
3537
{
3638
ResourceName: "google_project_iam_policy.acceptance",
@@ -156,6 +158,28 @@ func TestAccProjectIamPolicy_withCondition(t *testing.T) {
156158
})
157159
}
158160

161+
// Test that an IAM policy with invalid members returns errors.
162+
func TestAccProjectIamPolicy_invalidMembers(t *testing.T) {
163+
t.Parallel()
164+
165+
org := getTestOrgFromEnv(t)
166+
pid := fmt.Sprintf("tf-test-%d", randInt(t))
167+
168+
vcrTest(t, resource.TestCase{
169+
PreCheck: func() { testAccPreCheck(t) },
170+
Providers: testAccProviders,
171+
Steps: []resource.TestStep{
172+
{
173+
Config: testAccProjectAssociatePolicyBasic(pid, pname, org, "[email protected]"),
174+
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\\)"),
175+
},
176+
{
177+
Config: testAccProjectAssociatePolicyBasic(pid, pname, org, "user:[email protected]"),
178+
},
179+
},
180+
})
181+
}
182+
159183
func getStatePrimaryResource(s *terraform.State, res, expectedID string) (*terraform.InstanceState, error) {
160184
// Get the project resource
161185
resource, ok := s.RootModule().Resources[res]
@@ -228,7 +252,7 @@ func testAccProjectExistingPolicy(t *testing.T, pid string) resource.TestCheckFu
228252
}
229253
}
230254

231-
func testAccProjectAssociatePolicyBasic(pid, name, org string) string {
255+
func testAccProjectAssociatePolicyBasic(pid, name, org, member string) string {
232256
return fmt.Sprintf(`
233257
resource "google_project" "acceptance" {
234258
project_id = "%s"
@@ -245,7 +269,7 @@ data "google_iam_policy" "admin" {
245269
binding {
246270
role = "roles/storage.objectViewer"
247271
members = [
248-
"user:[email protected]",
272+
"%s",
249273
]
250274
}
251275
binding {
@@ -256,7 +280,7 @@ data "google_iam_policy" "admin" {
256280
]
257281
}
258282
}
259-
`, pid, name, org)
283+
`, pid, name, org, member)
260284
}
261285

262286
func testAccProjectAssociatePolicyAuditConfigBasic(pid, name, org string) string {

‎google/resource_iam_binding.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@ import (
44
"errors"
55
"fmt"
66
"log"
7-
"regexp"
87
"strings"
98

109
"github.com/davecgh/go-spew/spew"
1110
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
12-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1311
"google.golang.org/api/cloudresourcemanager/v1"
1412
)
1513

@@ -25,7 +23,7 @@ var iamBindingSchema = map[string]*schema.Schema{
2523
Elem: &schema.Schema{
2624
Type: schema.TypeString,
2725
DiffSuppressFunc: caseDiffSuppress,
28-
ValidateFunc: validation.StringDoesNotMatch(regexp.MustCompile("^deleted:"), "Terraform does not support IAM bindings for deleted principals"),
26+
ValidateFunc: validateIAMMember,
2927
},
3028
Set: func(v interface{}) int {
3129
return schema.HashString(strings.ToLower(v.(string)))

‎google/resource_iam_member.go

+21-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"github.com/davecgh/go-spew/spew"
1111
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
12-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1312
"google.golang.org/api/cloudresourcemanager/v1"
1413
)
1514

@@ -21,6 +20,26 @@ func iamMemberCaseDiffSuppress(k, old, new string, d *schema.ResourceData) bool
2120
return caseDiffSuppress(k, old, new, d)
2221
}
2322

23+
func validateIAMMember(i interface{}, k string) ([]string, []error) {
24+
v, ok := i.(string)
25+
if !ok {
26+
return nil, []error{fmt.Errorf("expected type of %s to be string", k)}
27+
}
28+
29+
if matched, err := regexp.MatchString("^deleted", v); err != nil {
30+
return nil, []error{fmt.Errorf("error validating %s: %v", k, err)}
31+
} else if matched {
32+
return nil, []error{fmt.Errorf("invalid value for %s (Terraform does not support IAM members for deleted principals)", k)}
33+
}
34+
35+
if matched, err := regexp.MatchString("(.+:.+|allUsers|allAuthenticatedUsers)", v); err != nil {
36+
return nil, []error{fmt.Errorf("error validating %s: %v", k, err)}
37+
} else if !matched {
38+
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)}
39+
}
40+
return nil, nil
41+
}
42+
2443
var IamMemberBaseSchema = map[string]*schema.Schema{
2544
"role": {
2645
Type: schema.TypeString,
@@ -32,7 +51,7 @@ var IamMemberBaseSchema = map[string]*schema.Schema{
3251
Required: true,
3352
ForceNew: true,
3453
DiffSuppressFunc: iamMemberCaseDiffSuppress,
35-
ValidateFunc: validation.StringDoesNotMatch(regexp.MustCompile("^deleted:"), "Terraform does not support IAM members for deleted principals"),
54+
ValidateFunc: validateIAMMember,
3655
},
3756
"condition": {
3857
Type: schema.TypeList,

‎google/resource_iam_policy.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,15 @@ func unmarshalIamPolicy(policyData string) (*cloudresourcemanager.Policy, error)
179179
}
180180

181181
func validateIamPolicy(i interface{}, k string) (s []string, es []error) {
182-
_, err := unmarshalIamPolicy(i.(string))
183-
if err != nil {
182+
if policy, err := unmarshalIamPolicy(i.(string)); err != nil {
184183
es = append(es, err)
184+
} else {
185+
for i, binding := range policy.Bindings {
186+
for j, member := range binding.Members {
187+
_, memberErrors := validateIAMMember(member, fmt.Sprintf("bindings.%d.members.%d", i, j))
188+
es = append(es, memberErrors...)
189+
}
190+
}
185191
}
186192
return
187193
}

‎google/resource_storage_bucket_iam_test.go

+28
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ func TestAccStorageBucketIamPolicy(t *testing.T) {
3838
ImportState: true,
3939
ImportStateVerify: true,
4040
},
41+
{
42+
// Test IAM Policy with member 'allUsers'
43+
Config: testAccStorageBucketIamPolicy_allUsers(bucket),
44+
},
4145
},
4246
})
4347
}
@@ -118,3 +122,27 @@ resource "google_storage_bucket_iam_policy" "bucket-binding" {
118122
}
119123
`, bucket, account, serviceAcct)
120124
}
125+
126+
func testAccStorageBucketIamPolicy_allUsers(bucket string) string {
127+
return fmt.Sprintf(`
128+
resource "google_storage_bucket" "bucket" {
129+
name = "%s"
130+
location = "US"
131+
}
132+
133+
data "google_iam_policy" "foo-policy" {
134+
binding {
135+
role = "roles/storage.objectViewer"
136+
members = [
137+
"allUsers",
138+
"allAuthenticatedUsers",
139+
]
140+
}
141+
}
142+
143+
resource "google_storage_bucket_iam_policy" "bucket-binding" {
144+
bucket = google_storage_bucket.bucket.name
145+
policy_data = data.google_iam_policy.foo-policy.policy_data
146+
}
147+
`, bucket)
148+
}

0 commit comments

Comments
 (0)