renames + comments

third_party/terraform/resources/resource_iam_binding.go.erb

@@ -89,10 +89,10 @@ func resourceIamBindingCreateUpdate(newUpdaterFunc newResourceIamUpdaterFunc, en
 
 		binding := getResourceIamBinding(d)
 		modifyF := func(ep *cloudresourcemanager.Policy) error {
-			cleaned := removeAllBindingsWithRoleAndCondition(ep.Bindings, binding.Role, binding.Condition)
+			cleaned := filterBindingsWithRoleAndCondition(ep.Bindings, binding.Role, binding.Condition)
 			ep.Bindings = append(cleaned, binding)
 <% unless version == 'ga' -%>
-			ep.Version = 3
+			ep.Version = iamPolicyVersion
 <% end -%>
 			return nil
 		}
@@ -201,7 +201,11 @@ func iamBindingImport(newUpdaterFunc newResourceIamUpdaterFunc, resourceIdParser
 		d.SetId(d.Id() + "/" + role)
 
 <% unless version == 'ga' -%>
-		// Read the upstream policy so we can set the full condition.
+		// Since condition titles can have any character in them, we can't separate them from any other
+		// field the user might set in import (like the condition description and expression). So, we
+		// have the user just specify the title and then read the upstream policy to set the full
+		// condition. We can't rely on the read fn to do this for us because it looks for a match of the
+		// full condition.
 		updater, err := newUpdaterFunc(d, config)
 		if err != nil {
 			return nil, err
@@ -253,7 +257,7 @@ func resourceIamBindingDelete(newUpdaterFunc newResourceIamUpdaterFunc, enableBa
 
 		binding := getResourceIamBinding(d)
 		modifyF := func(p *cloudresourcemanager.Policy) error {
-			p.Bindings = removeAllBindingsWithRoleAndCondition(p.Bindings, binding.Role, binding.Condition)
+			p.Bindings = filterBindingsWithRoleAndCondition(p.Bindings, binding.Role, binding.Condition)
 			return nil
 		}
 

third_party/terraform/resources/resource_iam_member.go.erb

@@ -181,7 +181,7 @@ func resourceIamMemberCreate(newUpdaterFunc newResourceIamUpdaterFunc, enableBat
 			// Merge the bindings together
 			ep.Bindings = mergeBindings(append(ep.Bindings, memberBind))
 <% unless version == 'ga' -%>
-			ep.Version = 3
+			ep.Version = iamPolicyVersion
 <% end -%>
 			return nil
 		}

third_party/terraform/resources/resource_iam_policy.go.erb

@@ -131,7 +131,7 @@ func setIamPolicyData(d *schema.ResourceData, updater ResourceIamUpdater) error
 		return fmt.Errorf("'policy_data' is not valid for %s: %s", updater.DescribeResource(), err)
 	}
 <% unless version == 'ga' -%>
-	policy.Version = 3
+	policy.Version = iamPolicyVersion
 <% end -%>
 
 	err = updater.SetResourceIamPolicy(policy)

third_party/terraform/tests/resource_google_service_account_iam_test.go.erb

@@ -227,7 +227,7 @@ func testAccCheckGoogleServiceAccountIam(account string, numBindings int) resour
 <% if version == 'ga' -%>
 		p, err := config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(serviceAccountCanonicalId(account)).Do()
 <% else -%>
-		p, err := config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(serviceAccountCanonicalId(account)).OptionsRequestedPolicyVersion(3).Do()
+		p, err := config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(serviceAccountCanonicalId(account)).OptionsRequestedPolicyVersion(iamPolicyVersion).Do()
 <% end -%>
 		if err != nil {
 			return err

third_party/terraform/utils/iam.go.erb

@@ -16,6 +16,7 @@ import (
 )
 
 const maxBackoffSeconds = 30
+const iamPolicyVersion = 3
 
 // These types are implemented per GCP resource type and specify how to do per-resource IAM operations.
 // They are used in the generic Terraform IAM resource definitions
@@ -186,7 +187,7 @@ type iamBindingKey struct {
 }
 
 // Removes a single role+condition binding from a list of Bindings
-func removeAllBindingsWithRoleAndCondition(b []*cloudresourcemanager.Binding, role string, condition *cloudresourcemanager.Expr) []*cloudresourcemanager.Binding {
+func filterBindingsWithRoleAndCondition(b []*cloudresourcemanager.Binding, role string, condition *cloudresourcemanager.Expr) []*cloudresourcemanager.Binding {
 	bMap := createIamBindingsMap(b)
 	key := iamBindingKey{role, conditionKeyFromCondition(condition)}
 	delete(bMap, key)

third_party/terraform/utils/iam_service_account.go.erb

@@ -40,7 +40,7 @@ func (u *ServiceAccountIamUpdater) GetResourceIamPolicy() (*cloudresourcemanager
 <% if version == 'ga' -%>
 	p, err := u.Config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(u.serviceAccountId).Do()
 <% else -%>
-	p, err := u.Config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(u.serviceAccountId).OptionsRequestedPolicyVersion(3).Do()
+	p, err := u.Config.clientIAM.Projects.ServiceAccounts.GetIamPolicy(u.serviceAccountId).OptionsRequestedPolicyVersion(iamPolicyVersion).Do()
 <% end -%>
 
 	if err != nil {

third_party/terraform/utils/iam_test.go.erb

@@ -259,7 +259,7 @@ func TestIamMergeBindings(t *testing.T) {
 	}
 }
 
-func TestIamRemoveAllBindingsWithRoleAndCondition(t *testing.T) {
+func TestIamFilterBindingsWithRoleAndCondition(t *testing.T) {
 	testCases := []struct {
 		input          []*cloudresourcemanager.Binding
 		role           string
@@ -365,7 +365,7 @@ func TestIamRemoveAllBindingsWithRoleAndCondition(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		got := removeAllBindingsWithRoleAndCondition(tc.input, tc.role, &cloudresourcemanager.Expr{Title: tc.conditionTitle})
+		got := filterBindingsWithRoleAndCondition(tc.input, tc.role, &cloudresourcemanager.Expr{Title: tc.conditionTitle})
 		if !compareBindings(got, tc.expect) {
 			t.Errorf("Got unexpected value for removeAllBindingsWithRole(%s, %s).\nActual: %s\nExpected: %s",
 				debugPrintBindings(tc.input), tc.role, debugPrintBindings(got), debugPrintBindings(tc.expect))