From 2bfed0d2b10117bc36b741d3b9558318f59be241 Mon Sep 17 00:00:00 2001
From: Justin Santa Barbara <justin@fathomdb.com>
Date: Tue, 31 Jan 2017 02:27:01 -0500
Subject: [PATCH] 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                              | 61 +++++++++++--------
 upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go | 35 ++++++++++-
 2 files changed, 66 insertions(+), 30 deletions(-)

diff --git a/pkg/model/iam.go b/pkg/model/iam.go
index 1ea7797e92..7d05759070 100644
--- a/pkg/model/iam.go
+++ b/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)
 		}
 	}
 
diff --git a/upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go b/upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go
index 49f935ae54..a8922b97bb 100644
--- a/upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go
+++ b/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)