Remove additional IAM policies that have been removed

This uses an explicit deletion approach, where we set the policy to
empty, and use that to signal that the policy should be deleted.  This
is acceptable because IAM policies can't be empty anyway.

We probably should use a tag-based "garbage-collection" approach, but
IAM objects can't be tagged, so we're pretty much always going to be
doing something name based.

Fix #1642

pkg/model/iam.go

@@ -112,35 +112,42 @@ func (b *IAMModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		}
 
 		// Generate additional policies if needed, and attach to existing role
-		if b.Cluster.Spec.AdditionalPolicies != nil {
-			roleAsString := reflect.ValueOf(role).String()
-			additionalPolicies := *(b.Cluster.Spec.AdditionalPolicies)
+		{
+			additionalPolicy := ""
+			if b.Cluster.Spec.AdditionalPolicies != nil {
+				roleAsString := reflect.ValueOf(role).String()
+				additionalPolicies := *(b.Cluster.Spec.AdditionalPolicies)
 
-			if additionalPolicy, ok := additionalPolicies[strings.ToLower(roleAsString)]; ok {
-				additionalPolicyName := "additional." + name
-
-				{
-					p := &iam.IAMPolicy{
-						Version: iam.IAMPolicyDefaultVersion,
-					}
-
-					statements := make([]*iam.IAMStatement, 0)
-					json.Unmarshal([]byte(additionalPolicy), &statements)
-					p.Statement = append(p.Statement, statements...)
-
-					policy, err := p.AsJSON()
-					if err != nil {
-						return fmt.Errorf("error building IAM policy: %v", err)
-					}
-
-					t := &awstasks.IAMRolePolicy{
-						Name:           s(additionalPolicyName),
-						Role:           iamRole,
-						PolicyDocument: fi.WrapResource(fi.NewStringResource(policy)),
-					}
-					c.AddTask(t)
-				}
+				additionalPolicy = additionalPolicies[strings.ToLower(roleAsString)]
 			}
+
+			additionalPolicyName := "additional." + name
+
+			t := &awstasks.IAMRolePolicy{
+				Name: s(additionalPolicyName),
+				Role: iamRole,
+			}
+
+			if additionalPolicy != "" {
+				p := &iam.IAMPolicy{
+					Version: iam.IAMPolicyDefaultVersion,
+				}
+
+				statements := make([]*iam.IAMStatement, 0)
+				json.Unmarshal([]byte(additionalPolicy), &statements)
+				p.Statement = append(p.Statement, statements...)
+
+				policy, err := p.AsJSON()
+				if err != nil {
+					return fmt.Errorf("error building IAM policy: %v", err)
+				}
+
+				t.PolicyDocument = fi.WrapResource(fi.NewStringResource(policy))
+			} else {
+				t.PolicyDocument = fi.WrapResource(fi.NewStringResource(""))
+			}
+
+			c.AddTask(t)
 		}
 	}
 

upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go

@@ -32,9 +32,12 @@ import (
 
 //go:generate fitask -type=IAMRolePolicy
 type IAMRolePolicy struct {
-	ID             *string
-	Name           *string
-	Role           *IAMRole
+	ID   *string
+	Name *string
+	Role *IAMRole
+
+	// The PolicyDocument to create as an inline policy.
+	// If the PolicyDocument is empty, the policy will be removed.
 	PolicyDocument *fi.ResourceHolder
 }
 
@@ -97,6 +100,21 @@ func (_ *IAMRolePolicy) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *IAMRoleP
 		return fmt.Errorf("error rendering PolicyDocument: %v", err)
 	}
 
+	if policy == "" {
+		// A deletion
+
+		request := &iam.DeleteRolePolicyInput{}
+		request.RoleName = e.Role.Name
+		request.PolicyName = e.Name
+
+		glog.V(2).Infof("Deleting role policy %s/%s", aws.StringValue(e.Role.Name), aws.StringValue(e.Name))
+		_, err = t.Cloud.IAM().DeleteRolePolicy(request)
+		if err != nil {
+			return fmt.Errorf("error deleting IAMRolePolicy: %v", err)
+		}
+		return nil
+	}
+
 	doPut := false
 
 	if a == nil {
@@ -148,6 +166,17 @@ type terraformIAMRolePolicy struct {
 }
 
 func (_ *IAMRolePolicy) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *IAMRolePolicy) error {
+	{
+		policyString, err := e.PolicyDocument.AsString()
+		if err != nil {
+			return fmt.Errorf("error rendering PolicyDocument: %v", err)
+		}
+		if policyString == "" {
+			// A deletion; we simply don't render; terraform will observe the removal
+			return nil
+		}
+	}
+
 	policy, err := t.AddFile("aws_iam_role_policy", *e.Name, "policy", e.PolicyDocument)
 	if err != nil {
 		return fmt.Errorf("error rendering PolicyDocument: %v", err)