diff --git a/third_party/terraform/resources/resource_iam_binding.go.erb b/third_party/terraform/resources/resource_iam_binding.go.erb index a23534f5d524..8e7fe7782157 100644 --- a/third_party/terraform/resources/resource_iam_binding.go.erb +++ b/third_party/terraform/resources/resource_iam_binding.go.erb @@ -33,19 +33,23 @@ var iamBindingSchema = map[string]*schema.Schema{ Type: schema.TypeList, Optional: true, MaxItems: 1, + ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "expression": { Type: schema.TypeString, Required: true, + ForceNew: true, }, "title": { Type: schema.TypeString, Required: true, + ForceNew: true, }, "description": { Type: schema.TypeString, Optional: true, + ForceNew: true, }, }, }, @@ -64,39 +68,18 @@ func ResourceIamBinding(parentSpecificSchema map[string]*schema.Schema, newUpdat // Resource that batches requests to the same IAM policy across multiple IAM fine-grained resources func ResourceIamBindingWithBatching(parentSpecificSchema map[string]*schema.Schema, newUpdaterFunc newResourceIamUpdaterFunc, resourceIdParser resourceIdParserFunc, enableBatching bool) *schema.Resource { return &schema.Resource{ - Create: resourceIamBindingCreate(newUpdaterFunc, enableBatching), + Create: resourceIamBindingCreateUpdate(newUpdaterFunc, enableBatching), Read: resourceIamBindingRead(newUpdaterFunc), - Update: resourceIamBindingUpdate(newUpdaterFunc, enableBatching), + Update: resourceIamBindingCreateUpdate(newUpdaterFunc, enableBatching), Delete: resourceIamBindingDelete(newUpdaterFunc, enableBatching), Schema: mergeSchemas(iamBindingSchema, parentSpecificSchema), Importer: &schema.ResourceImporter{ - State: iamBindingImport(resourceIdParser), - }, - SchemaVersion: 1, - StateUpgraders: []schema.StateUpgrader{ - { - Type: resourceIAMBindingResourceV0(parentSpecificSchema).CoreConfigSchema().ImpliedType(), - Upgrade: func(rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { - // IAM conditions require parsing information from the ID, which means the ID needs - // a separator that won't occur naturally in the project or role. - // Previous format: projects/{project}/{resourceType}/{resourceName}/roles/{role} - // New format: projects/{project}/{resourceType}/{resourceName} roles/{role} - oldId := rawState["id"].(string) - role := rawState["role"].(string) - rawState["id"] = strings.TrimSuffix(oldId, "/"+role) + " " + role - return rawState, nil - }, - Version: 0, - }, + State: iamBindingImport(newUpdaterFunc, resourceIdParser), }, } } -func resourceIAMBindingResourceV0(parentSpecificSchema map[string]*schema.Schema) *schema.Resource { - return &schema.Resource{Schema: mergeSchemas(iamBindingSchema, parentSpecificSchema)} -} - -func resourceIamBindingCreate(newUpdaterFunc newResourceIamUpdaterFunc, enableBatching bool) func(*schema.ResourceData, interface{}) error { +func resourceIamBindingCreateUpdate(newUpdaterFunc newResourceIamUpdaterFunc, enableBatching bool) func(*schema.ResourceData, interface{}) error { return func(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) updater, err := newUpdaterFunc(d, config) @@ -123,11 +106,8 @@ func resourceIamBindingCreate(newUpdaterFunc newResourceIamUpdaterFunc, enableBa if err != nil { return err } - conditionTitle := "" - if binding.Condition != nil { - conditionTitle = binding.Condition.Title - } - d.SetId(updater.GetResourceId() + " " + binding.Role + " " + conditionTitle) + + d.SetId(updater.GetResourceId() + "/" + binding.Role) return resourceIamBindingRead(newUpdaterFunc)(d, meta) } } @@ -140,37 +120,26 @@ func resourceIamBindingRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Rea return err } - var role, conditionTitle string - s := strings.SplitN(d.Id(), " ", 3) - if len(s) == 2 { - role = s[1] - } else if len(s) == 3 { - role, conditionTitle = s[1], s[2] - } else { - return fmt.Errorf("Unexpected resource ID %s", d.Id()) - } + eBinding := getResourceIamBinding(d) + eCondition := conditionKeyFromCondition(eBinding.Condition) p, err := iamPolicyReadWithRetry(updater) if err != nil { - return handleNotFoundError(err, d, fmt.Sprintf("Resource %q with IAM Binding (Role %q)", updater.DescribeResource(), role)) + return handleNotFoundError(err, d, fmt.Sprintf("Resource %q with IAM Binding (Role %q)", updater.DescribeResource(), eBinding.Role)) } - log.Printf("[DEBUG]: Retrieved policy for %s: %+v", updater.DescribeResource(), p) + log.Printf("[DEBUG] Retrieved policy for %s: %+v", updater.DescribeResource(), p) + log.Printf("[DEBUG] Looking for binding with role %q and condition %+v", eBinding.Role, eCondition) - log.Printf("Looking for binding with role %s and condition %s", role, conditionTitle) var binding *cloudresourcemanager.Binding for _, b := range p.Bindings { - if b.Role == role && ((b.Condition == nil && conditionTitle == "") || (b.Condition != nil && b.Condition.Title == conditionTitle)) { - c := "" - if b.Condition != nil { - c = b.Condition.Title - } - log.Printf("Found binding with role %s and condition %s", b.Role, c) + if b.Role == eBinding.Role && conditionKeyFromCondition(b.Condition) == eCondition { binding = b break } } + if binding == nil { - log.Printf("[DEBUG]: Binding for role %q and condition %q not found in policy for %s, assuming it has no members.", role, conditionTitle, updater.DescribeResource()) - d.Set("role", role) + log.Printf("[DEBUG] Binding for role %q and condition %+v not found in policy for %s, assuming it has no members.", eBinding.Role, eCondition, updater.DescribeResource()) + d.Set("role", eBinding.Role) d.Set("members", nil) return nil } else { @@ -185,61 +154,34 @@ func resourceIamBindingRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Rea } } -func resourceIamBindingUpdate(newUpdaterFunc newResourceIamUpdaterFunc, enableBatching bool) func(*schema.ResourceData, interface{}) error { - return func(d *schema.ResourceData, meta interface{}) error { - config := meta.(*Config) - updater, err := newUpdaterFunc(d, config) - if err != nil { - return err - } - - oBinding, nBinding := getResourceIamBindingChange(d) - modifyF := func(ep *cloudresourcemanager.Policy) error { - cleaned := removeAllBindingsWithRoleAndCondition(ep.Bindings, oBinding.Role, oBinding.Condition) - cleaned = removeAllBindingsWithRoleAndCondition(cleaned, nBinding.Role, nBinding.Condition) - ep.Bindings = append(cleaned, nBinding) -<% unless version == 'ga' -%> - ep.Version = 3 -<% end -%> - return nil - } - - if enableBatching { - err = BatchRequestModifyIamPolicy(updater, modifyF, config, fmt.Sprintf( - "Set IAM Binding for role %q on %q", nBinding.Role, updater.DescribeResource())) - } else { - err = iamPolicyReadModifyWrite(updater, modifyF) - } - if err != nil { - return err - } - - conditionTitle := "" - if nBinding.Condition != nil { - conditionTitle = nBinding.Condition.Title - } - d.SetId(updater.GetResourceId() + " " + nBinding.Role + " " + conditionTitle) - return resourceIamBindingRead(newUpdaterFunc)(d, meta) - } -} - -func iamBindingImport(resourceIdParser resourceIdParserFunc) schema.StateFunc { +func iamBindingImport(newUpdaterFunc newResourceIamUpdaterFunc, resourceIdParser resourceIdParserFunc) schema.StateFunc { return func(d *schema.ResourceData, m interface{}) ([]*schema.ResourceData, error) { if resourceIdParser == nil { return nil, errors.New("Import not supported for this IAM resource.") } config := m.(*Config) s := strings.Fields(d.Id()) - var id, role, conditionTitle string + var id, role string +<% if version == 'ga' -%> + if len(s) != 2 { + d.SetId("") + return nil, fmt.Errorf("Wrong number of parts to Binding id %s; expected 'resource_name role'.", s) + } + id, role = s[0], s[1] +<% else -%> + if len(s) < 2 { + d.SetId("") + return nil, fmt.Errorf("Wrong number of parts to Binding id %s; expected 'resource_name role [condition_title]'.", s) + } + + var conditionTitle string if len(s) == 2 { id, role = s[0], s[1] - } else if len(s) >= 3 { + } else { // condition titles can have any characters in them, so re-join the split string id, role, conditionTitle = s[0], s[1], strings.Join(s[2:], " ") - } else { - d.SetId("") - return nil, fmt.Errorf("Wrong number of parts to Binding id %s; expected 'resource_name role [condition_title]'.", s) } +<% end -%> // Set the ID only to the first part so all IAM types can share the same resourceIdParserFunc. d.SetId(id) @@ -251,8 +193,30 @@ func iamBindingImport(resourceIdParser resourceIdParserFunc) schema.StateFunc { // Set the ID again so that the ID matches the ID it would have if it had been created via TF. // Use the current ID in case it changed in the resourceIdParserFunc. - // Use " " as the delimiter because projects can have ':'s and roles can have '/'s. - d.SetId(d.Id() + " " + role + " " + conditionTitle) + d.SetId(d.Id() + "/" + role) + +<% unless version == 'ga' -%> + // Read the upstream policy so we can set the full condition. + updater, err := newUpdaterFunc(d, config) + if err != nil { + return nil, err + } + p, err := iamPolicyReadWithRetry(updater) + if err != nil { + return nil, err + } + var binding *cloudresourcemanager.Binding + for _, b := range p.Bindings { + if (b.Role == role && conditionKeyFromCondition(b.Condition).Title == conditionTitle) { + if binding != nil { + return nil, fmt.Errorf("Cannot import IAM member with condition title %q, it matches multiple conditions", conditionTitle) + } + binding = b + } + } + d.Set("condition", flattenIamCondition(binding.Condition)) +<% end -%> + // It is possible to return multiple bindings, since we can learn about all the bindings // for this resource here. Unfortunately, `terraform import` has some messy behavior here - // there's no way to know at this point which resource is being imported, so it's not possible @@ -306,30 +270,6 @@ func getResourceIamBinding(d *schema.ResourceData) *cloudresourcemanager.Binding } } -func getResourceIamBindingChange(d *schema.ResourceData) (*cloudresourcemanager.Binding, *cloudresourcemanager.Binding) { - oMembers, nMembers := d.GetChange("members") - oRole, nRole := d.GetChange("role") -<% unless version == 'ga' -%> - oCondition, nCondition := d.GetChange("condition") -<% end -%> - - oBinding := &cloudresourcemanager.Binding{ - Members: convertStringArr(oMembers.(*schema.Set).List()), - Role: oRole.(string), -<% unless version == 'ga' -%> - Condition: expandIamCondition(oCondition), -<% end -%> - } - nBinding := &cloudresourcemanager.Binding{ - Members: convertStringArr(nMembers.(*schema.Set).List()), - Role: nRole.(string), -<% unless version == 'ga' -%> - Condition: expandIamCondition(nCondition), -<% end -%> - } - return oBinding, nBinding -} - <% unless version == 'ga' -%> func expandIamCondition(v interface{}) *cloudresourcemanager.Expr { l := v.([]interface{}) diff --git a/third_party/terraform/resources/resource_iam_member.go.erb b/third_party/terraform/resources/resource_iam_member.go.erb index dbf5b0d907df..f9068bd32f01 100644 --- a/third_party/terraform/resources/resource_iam_member.go.erb +++ b/third_party/terraform/resources/resource_iam_member.go.erb @@ -56,28 +56,29 @@ var IamMemberBaseSchema = map[string]*schema.Schema{ }, } -func iamMemberImport(resourceIdParser resourceIdParserFunc) schema.StateFunc { +func iamMemberImport(newUpdaterFunc newResourceIamUpdaterFunc, resourceIdParser resourceIdParserFunc) schema.StateFunc { return func(d *schema.ResourceData, m interface{}) ([]*schema.ResourceData, error) { if resourceIdParser == nil { return nil, errors.New("Import not supported for this IAM resource.") } config := m.(*Config) s := strings.Fields(d.Id()) + var id, role, member string <% if version == 'ga' -%> - if len(s) == 3 { + if len(s) != 3 { + d.SetId("") + return nil, fmt.Errorf("Wrong number of parts to Member id %s; expected 'resource_name role member'.", s) + } + id, role, member = s[0], s[1], s[2] <% else -%> if len(s) < 3 { -<% end -%> d.SetId("") return nil, fmt.Errorf("Wrong number of parts to Member id %s; expected 'resource_name role member [condition_title]'.", s) } - var id, role, member string -<% unless version == 'ga' -%> - var conditionTitle string + + var conditionTitle string if len(s) == 3 { -<% end -%> id, role, member = s[0], s[1], s[2] -<% unless version == 'ga' -%> } else { // condition titles can have any characters in them, so re-join the split string id, role, member, conditionTitle = s[0], s[1], s[2], strings.Join(s[3:], " ") @@ -88,9 +89,7 @@ func iamMemberImport(resourceIdParser resourceIdParserFunc) schema.StateFunc { d.SetId(id) d.Set("role", role) d.Set("member", strings.ToLower(member)) -<% unless version == 'ga' -%> - d.Set("condition", flattenIamCondition(&cloudresourcemanager.Expr{Title: conditionTitle})) -<% end -%> + err := resourceIdParser(d, config) if err != nil { return nil, err @@ -99,6 +98,39 @@ func iamMemberImport(resourceIdParser resourceIdParserFunc) schema.StateFunc { // Set the ID again so that the ID matches the ID it would have if it had been created via TF. // Use the current ID in case it changed in the resourceIdParserFunc. d.SetId(d.Id() + "/" + role + "/" + strings.ToLower(member)) + +<% unless version == 'ga' -%> + // Read the upstream policy so we can set the full condition. + updater, err := newUpdaterFunc(d, config) + if err != nil { + return nil, err + } + p, err := iamPolicyReadWithRetry(updater) + if err != nil { + return nil, err + } + var binding *cloudresourcemanager.Binding + for _, b := range p.Bindings { + if (b.Role == role && conditionKeyFromCondition(b.Condition).Title == conditionTitle) { + containsMember := false + for _, m := range b.Members { + if strings.ToLower(m) == strings.ToLower(member) { + containsMember = true + } + } + if !containsMember { + continue + } + + if binding != nil { + return nil, fmt.Errorf("Cannot import IAM member with condition title %q, it matches multiple conditions", conditionTitle) + } + binding = b + } + } + d.Set("condition", flattenIamCondition(binding.Condition)) +<% end -%> + return []*schema.ResourceData{d}, nil } } @@ -114,7 +146,7 @@ func ResourceIamMemberWithBatching(parentSpecificSchema map[string]*schema.Schem Delete: resourceIamMemberDelete(newUpdaterFunc, enableBatching), Schema: mergeSchemas(IamMemberBaseSchema, parentSpecificSchema), Importer: &schema.ResourceImporter{ - State: iamMemberImport(resourceIdParser), + State: iamMemberImport(newUpdaterFunc, resourceIdParser), }, } } @@ -169,36 +201,42 @@ func resourceIamMemberRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read } eMember := getResourceIamMember(d) + eCondition := conditionKeyFromCondition(eMember.Condition) p, err := iamPolicyReadWithRetry(updater) if err != nil { return handleNotFoundError(err, d, fmt.Sprintf("Resource %q with IAM Member: Role %q Member %q", updater.DescribeResource(), eMember.Role, eMember.Members[0])) } log.Printf("[DEBUG]: Retrieved policy for %s: %+v\n", updater.DescribeResource(), p) + log.Printf("[DEBUG]: Looking for binding with role %q and condition %+v", eMember.Role, eCondition) var binding *cloudresourcemanager.Binding for _, b := range p.Bindings { - if b.Role != eMember.Role || (b.Condition != nil && eMember.Condition != nil && b.Condition.Title != eMember.Condition.Title) { - continue + if (b.Role == eMember.Role && conditionKeyFromCondition(b.Condition) == eCondition) { + binding = b + break } - binding = b - break } + if binding == nil { - log.Printf("[DEBUG]: Binding for role %q does not exist in policy of %s, removing member %q from state.", eMember.Role, updater.DescribeResource(), eMember.Members[0]) + log.Printf("[DEBUG]: Binding for role %+v with condition %q does not exist in policy of %s, removing member %q from state.", eMember.Role, eCondition, updater.DescribeResource(), eMember.Members[0]) d.SetId("") return nil } + + log.Printf("[DEBUG]: Looking for member %q in found binding", eMember.Members[0]) var member string for _, m := range binding.Members { if strings.ToLower(m) == strings.ToLower(eMember.Members[0]) { member = m } } + if member == "" { - log.Printf("[DEBUG]: Member %q for binding for role %q does not exist in policy of %s, removing from state.", eMember.Members[0], eMember.Role, updater.DescribeResource()) + log.Printf("[DEBUG]: Member %q for binding for role %q with condition %+v does not exist in policy of %s, removing from state.", eMember.Members[0], eMember.Role, eCondition, updater.DescribeResource()) d.SetId("") return nil } + d.Set("etag", p.Etag) d.Set("member", member) d.Set("role", binding.Role) diff --git a/third_party/terraform/tests/resource_google_service_account_iam_test.go.erb b/third_party/terraform/tests/resource_google_service_account_iam_test.go.erb index 9c8ac2df3311..6c7107aea2c7 100644 --- a/third_party/terraform/tests/resource_google_service_account_iam_test.go.erb +++ b/third_party/terraform/tests/resource_google_service_account_iam_test.go.erb @@ -27,6 +27,7 @@ func TestAccServiceAccountIamBinding(t *testing.T) { ResourceName: "google_service_account_iam_binding.foo", ImportState: true, ImportStateVerify: true, + ImportStateId: fmt.Sprintf("%s %s", serviceAccountCanonicalId(account), "roles/iam.serviceAccountUser"), }, }, }) @@ -52,6 +53,7 @@ func TestAccServiceAccountIamBinding_withCondition(t *testing.T) { ResourceName: "google_service_account_iam_binding.foo", ImportState: true, ImportStateVerify: true, + ImportStateId: fmt.Sprintf("%s %s %s", serviceAccountCanonicalId(account), "roles/iam.serviceAccountUser", conditionTitle), }, }, }) @@ -76,77 +78,13 @@ func TestAccServiceAccountIamBinding_withAndWithoutCondition(t *testing.T) { ResourceName: "google_service_account_iam_binding.foo", ImportState: true, ImportStateVerify: true, + ImportStateId: fmt.Sprintf("%s %s", serviceAccountCanonicalId(account), "roles/iam.serviceAccountUser"), }, { ResourceName: "google_service_account_iam_binding.foo2", ImportState: true, ImportStateVerify: true, - }, - }, - }) -} - -func TestAccServiceAccountIamBinding_update(t *testing.T) { - t.Parallel() - - account := acctest.RandomWithPrefix("tf-test") - conditionExpr := `request.time < timestamp(\"2020-01-01T00:00:00Z\")` - conditionExpr2 := `request.time < timestamp(\"2021-01-01T00:00:00Z\")` - conditionTitle := "expires_after_2019_12_31" - conditionTitle2 := "expires_after_2020_12_31" - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: testAccServiceAccountIamBinding_basic(account), - Check: testAccCheckGoogleServiceAccountIam(account, 1), - }, - { - ResourceName: "google_service_account_iam_binding.foo", - ImportState: true, - ImportStateVerify: true, - }, - // Update: add condition - { - Config: testAccServiceAccountIamBinding_withCondition(account, "user:admin@hashicorptest.com", conditionTitle, conditionExpr), - Check: testAccCheckGoogleServiceAccountIam(account, 1), - }, - { - ResourceName: "google_service_account_iam_binding.foo", - ImportState: true, - ImportStateVerify: true, - }, - // Update condition member list - { - Config: testAccServiceAccountIamBinding_withCondition(account, "user:paddy@hashicorp.com", conditionTitle, conditionExpr), - Check: testAccCheckGoogleServiceAccountIam(account, 1), - }, - { - ResourceName: "google_service_account_iam_binding.foo", - ImportState: true, - ImportStateVerify: true, - }, - // Update condition expression - { - Config: testAccServiceAccountIamBinding_withCondition(account, "user:paddy@hashicorp.com", conditionTitle, conditionExpr2), - Check: testAccCheckGoogleServiceAccountIam(account, 1), - }, - { - ResourceName: "google_service_account_iam_binding.foo", - ImportState: true, - ImportStateVerify: true, - }, - // Update condition title - { - Config: testAccServiceAccountIamBinding_withCondition(account, "user:paddy@hashicorp.com", conditionTitle2, conditionExpr2), - Check: testAccCheckGoogleServiceAccountIam(account, 1), - }, - { - ResourceName: "google_service_account_iam_binding.foo", - ImportState: true, - ImportStateVerify: true, + ImportStateId: fmt.Sprintf("%s %s %s", serviceAccountCanonicalId(account), "roles/iam.serviceAccountUser", conditionTitle), }, }, }) @@ -183,18 +121,19 @@ func TestAccServiceAccountIamMember_withCondition(t *testing.T) { account := acctest.RandomWithPrefix("tf-test") identity := fmt.Sprintf("serviceAccount:%s", serviceAccountCanonicalEmail(account)) + conditionTitle := "expires_after_2019_12_31" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccServiceAccountIamMember_withCondition(account), + Config: testAccServiceAccountIamMember_withCondition(account, conditionTitle), Check: testAccCheckGoogleServiceAccountIam(account, 1), }, { ResourceName: "google_service_account_iam_member.foo", - ImportStateId: fmt.Sprintf("%s %s %s", serviceAccountCanonicalId(account), "roles/iam.serviceAccountUser", identity), + ImportStateId: fmt.Sprintf("%s %s %s %s", serviceAccountCanonicalId(account), "roles/iam.serviceAccountUser", identity, conditionTitle), ImportState: true, ImportStateVerify: true, }, @@ -358,7 +297,7 @@ resource "google_service_account_iam_member" "foo" { } <% unless version == 'ga' -%> -func testAccServiceAccountIamMember_withCondition(account string) string { +func testAccServiceAccountIamMember_withCondition(account, conditionTitle string) string { return fmt.Sprintf(` resource "google_service_account" "test_account" { account_id = "%s" @@ -370,12 +309,12 @@ resource "google_service_account_iam_member" "foo" { role = "roles/iam.serviceAccountUser" member = "serviceAccount:${google_service_account.test_account.email}" condition { - title = "expires_after_2019_12_31" + title = "%s" description = "Expiring at midnight of 2019-12-31" expression = "request.time < timestamp(\"2020-01-01T00:00:00Z\")" } } -`, account) +`, account, conditionTitle) } <% end -%> diff --git a/third_party/terraform/utils/iam.go.erb b/third_party/terraform/utils/iam.go.erb index 36b8a59fd866..470478fd7947 100644 --- a/third_party/terraform/utils/iam.go.erb +++ b/third_party/terraform/utils/iam.go.erb @@ -177,7 +177,7 @@ type iamBindingKey struct { Condition conditionKey } -// Flattens Bindings so each role+condition has a single Binding with combined members +// Flattens a list of Bindings so each role+condition has a single Binding with combined members func removeAllBindingsWithRoleAndCondition(b []*cloudresourcemanager.Binding, role string, condition *cloudresourcemanager.Expr) []*cloudresourcemanager.Binding { bMap := createIamBindingsMap(b) key := iamBindingKey{role, conditionKeyFromCondition(condition)}