AWS API Optimization - IAM

controllers/providers/aws/aws.go

@@ -50,7 +50,7 @@ type AwsWorker struct {
 }
 
 var (
-	DefaultInstanceProfilePropagationDelay = time.Second * 20
+	DefaultInstanceProfilePropagationDelay = time.Second * 25
 	DefaultWaiterDuration                  = time.Second * 5
 	DefaultWaiterRetries                   = 12
 
@@ -286,7 +286,51 @@ func (w *AwsWorker) DeleteScalingGroupRole(name string, managedPolicies []string
 	return nil
 }
 
-func (w *AwsWorker) CreateUpdateScalingGroupRole(name string, managedPolicies []string) (*iam.Role, *iam.InstanceProfile, error) {
+func (w *AwsWorker) AttachManagedPolicies(name string, managedPolicies []string) error {
+	for _, policy := range managedPolicies {
+		_, err := w.IamClient.AttachRolePolicy(&iam.AttachRolePolicyInput{
+			RoleName:  aws.String(name),
+			PolicyArn: aws.String(policy),
+		})
+		if err != nil {
+			return errors.Wrap(err, "failed to attach role policies")
+		}
+	}
+	return nil
+}
+
+func (w *AwsWorker) DetachManagedPolicies(name string, managedPolicies []string) error {
+	for _, policy := range managedPolicies {
+		_, err := w.IamClient.DetachRolePolicy(&iam.DetachRolePolicyInput{
+			RoleName:  aws.String(name),
+			PolicyArn: aws.String(policy),
+		})
+		if err != nil {
+			return errors.Wrap(err, "failed to detach role policies")
+		}
+	}
+	return nil
+}
+
+func (w *AwsWorker) ListRolePolicies(name string) ([]*iam.AttachedPolicy, error) {
+	policies := []*iam.AttachedPolicy{}
+	err := w.IamClient.ListAttachedRolePoliciesPages(
+		&iam.ListAttachedRolePoliciesInput{
+			RoleName: aws.String(name),
+		},
+		func(page *iam.ListAttachedRolePoliciesOutput, lastPage bool) bool {
+			for _, p := range page.AttachedPolicies {
+				policies = append(policies, p)
+			}
+			return page.Marker != nil
+		})
+	if err != nil {
+		return policies, err
+	}
+	return policies, nil
+}
+
+func (w *AwsWorker) CreateScalingGroupRole(name string) (*iam.Role, *iam.InstanceProfile, error) {
 	var (
 		assumeRolePolicyDocument = `{
 			"Version": "2012-10-17",
@@ -323,30 +367,21 @@ func (w *AwsWorker) CreateUpdateScalingGroupRole(name string, managedPolicies []
 		}
 		createdProfile = out.InstanceProfile
 		time.Sleep(DefaultInstanceProfilePropagationDelay)
-	} else {
-		createdProfile = instanceProfile
-	}
 
-	_, err := w.IamClient.AddRoleToInstanceProfile(&iam.AddRoleToInstanceProfileInput{
-		InstanceProfileName: aws.String(name),
-		RoleName:            aws.String(name),
-	})
-	if err != nil {
-		if aerr, ok := err.(awserr.Error); ok {
-			if aerr.Code() != iam.ErrCodeLimitExceededException {
-				return createdRole, createdProfile, errors.Wrap(err, "failed to attach instance-profile")
-			}
-		}
-	}
-
-	for _, policy := range managedPolicies {
-		_, err = w.IamClient.AttachRolePolicy(&iam.AttachRolePolicyInput{
-			RoleName:  aws.String(name),
-			PolicyArn: aws.String(policy),
+		_, err = w.IamClient.AddRoleToInstanceProfile(&iam.AddRoleToInstanceProfileInput{
+			InstanceProfileName: aws.String(name),
+			RoleName:            aws.String(name),
 		})
 		if err != nil {
-			return createdRole, createdProfile, errors.Wrap(err, "failed to attach policies")
+			if aerr, ok := err.(awserr.Error); ok {
+				if aerr.Code() != iam.ErrCodeLimitExceededException {
+					return createdRole, createdProfile, errors.Wrap(err, "failed to attach instance-profile")
+				}
+			}
 		}
+
+	} else {
+		createdProfile = instanceProfile
 	}
 
 	return createdRole, createdProfile, nil

controllers/provisioners/eks/cloud.go

@@ -40,6 +40,7 @@ type DiscoveredState struct {
 	LaunchConfiguration           *autoscaling.LaunchConfiguration
 	ActiveLaunchConfigurationName string
 	IAMRole                       *iam.Role
+	AttachedPolicies              []*iam.AttachedPolicy
 	InstanceProfile               *iam.InstanceProfile
 	Publisher                     kubeprovider.EventPublisher
 }
@@ -80,6 +81,14 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 	if val, ok := ctx.AwsWorker.RoleExist(roleName); ok {
 		state.SetRole(val)
 		status.SetNodesArn(aws.StringValue(val.Arn))
+
+		if !configuration.HasExistingRole() {
+			policies, err := ctx.AwsWorker.ListRolePolicies(roleName)
+			if err != nil {
+				return errors.Wrap(err, "failed to list attached role policies")
+			}
+			state.SetAttachedPolicies(policies)
+		}
 	}
 
 	if val, ok := ctx.AwsWorker.InstanceProfileExist(instanceProfileName); ok {
@@ -184,6 +193,15 @@ func (d *DiscoveredState) SetOwnedScalingGroups(groups []*autoscaling.Group) {
 func (d *DiscoveredState) GetOwnedScalingGroups() []*autoscaling.Group {
 	return d.OwnedScalingGroups
 }
+func (d *DiscoveredState) SetAttachedPolicies(policies []*iam.AttachedPolicy) {
+	d.AttachedPolicies = policies
+}
+func (d *DiscoveredState) GetAttachedPolicies() []*iam.AttachedPolicy {
+	if d.AttachedPolicies == nil {
+		d.AttachedPolicies = []*iam.AttachedPolicy{}
+	}
+	return d.AttachedPolicies
+}
 func (d *DiscoveredState) SetLaunchConfiguration(lc *autoscaling.LaunchConfiguration) {
 	if lc != nil {
 		d.LaunchConfiguration = lc

controllers/provisioners/eks/create.go

@@ -122,25 +122,28 @@ func (ctx *EksInstanceGroupContext) CreateLaunchConfiguration(name string) error
 
 func (ctx *EksInstanceGroupContext) CreateManagedRole() error {
 	var (
-		instanceGroup      = ctx.GetInstanceGroup()
-		state              = ctx.GetDiscoveredState()
-		configuration      = instanceGroup.GetEKSConfiguration()
-		additionalPolicies = configuration.GetManagedPolicies()
-		roleName           = ctx.ResourcePrefix
+		instanceGroup = ctx.GetInstanceGroup()
+		state         = ctx.GetDiscoveredState()
+		configuration = instanceGroup.GetEKSConfiguration()
+		roleName      = ctx.ResourcePrefix
 	)
 
 	if configuration.HasExistingRole() {
+		// avoid updating if using an existing role
 		return nil
 	}
 
-	// create a controller-owned role for the instancegroup
-	managedPolicies := ctx.GetManagedPoliciesList(additionalPolicies)
+	role, profile, err := ctx.AwsWorker.CreateScalingGroupRole(roleName)
+	if err != nil {
+		return errors.Wrap(err, "failed to create scaling group role")
+	}
 
-	role, profile, err := ctx.AwsWorker.CreateUpdateScalingGroupRole(roleName, managedPolicies)
+	err = ctx.UpdateManagedPolicies(roleName)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "failed to update managed policies")
 	}
-	ctx.Log.Info("created managed role", "instancegroup", instanceGroup.GetName(), "iamrole", roleName)
+
+	ctx.Log.Info("reconciled managed role", "instancegroup", instanceGroup.GetName(), "iamrole", roleName)
 
 	state.SetRole(role)
 	state.SetInstanceProfile(profile)

controllers/provisioners/eks/eks_test.go

@@ -154,6 +154,19 @@ func MockCustomResourceDefinition() *unstructured.Unstructured {
 	}
 }
 
+func MockAttachedPolicies(policies ...string) []*iam.AttachedPolicy {
+	mock := []*iam.AttachedPolicy{}
+	for _, p := range policies {
+		arn := fmt.Sprintf("%v/%v", awsprovider.IAMPolicyPrefix, p)
+		policy := &iam.AttachedPolicy{
+			PolicyName: aws.String(p),
+			PolicyArn:  aws.String(arn),
+		}
+		mock = append(mock, policy)
+	}
+	return mock
+}
+
 func MockTagDescription(key, value string) *autoscaling.TagDescription {
 	return &autoscaling.TagDescription{
 		Key:   aws.String(key),
@@ -359,10 +372,30 @@ type MockIamClient struct {
 	AddRoleToInstanceProfileErr       error
 	RemoveRoleFromInstanceProfileErr  error
 	AttachRolePolicyErr               error
+	AttachRolePolicyCallCount         int
 	DetachRolePolicyErr               error
+	DetachRolePolicyCallCount         int
 	WaitUntilInstanceProfileExistsErr error
+	ListAttachedRolePoliciesErr       error
 	Role                              *iam.Role
 	InstanceProfile                   *iam.InstanceProfile
+	AttachedPolicies                  []*iam.AttachedPolicy
+}
+
+func (i *MockIamClient) ListAttachedRolePolicies(input *iam.ListAttachedRolePoliciesInput) (*iam.ListAttachedRolePoliciesOutput, error) {
+	if i.AttachedPolicies != nil {
+		return &iam.ListAttachedRolePoliciesOutput{AttachedPolicies: i.AttachedPolicies}, i.ListAttachedRolePoliciesErr
+	}
+	return &iam.ListAttachedRolePoliciesOutput{}, i.ListAttachedRolePoliciesErr
+}
+
+func (i *MockIamClient) ListAttachedRolePoliciesPages(input *iam.ListAttachedRolePoliciesInput, callback func(*iam.ListAttachedRolePoliciesOutput, bool) bool) error {
+	page, err := i.ListAttachedRolePolicies(input)
+	if err != nil {
+		return err
+	}
+	callback(page, false)
+	return nil
 }
 
 func (i *MockIamClient) CreateRole(input *iam.CreateRoleInput) (*iam.CreateRoleOutput, error) {
@@ -400,10 +433,12 @@ func (i *MockIamClient) RemoveRoleFromInstanceProfile(input *iam.RemoveRoleFromI
 }
 
 func (i *MockIamClient) AttachRolePolicy(input *iam.AttachRolePolicyInput) (*iam.AttachRolePolicyOutput, error) {
+	i.AttachRolePolicyCallCount++
 	return &iam.AttachRolePolicyOutput{}, i.AttachRolePolicyErr
 }
 
 func (i *MockIamClient) DetachRolePolicy(input *iam.DetachRolePolicyInput) (*iam.DetachRolePolicyOutput, error) {
+	i.DetachRolePolicyCallCount++
 	return &iam.DetachRolePolicyOutput{}, i.DetachRolePolicyErr
 }
 

controllers/provisioners/eks/helpers.go

@@ -310,6 +310,7 @@ func (ctx *EksInstanceGroupContext) UpdateNodeReadyCondition() bool {
 		if !state.IsNodesReady() {
 			state.Publisher.Publish(kubeprovider.NodesReadyEvent, "instancegroup", instanceGroup.GetName(), "instances", instances)
 		}
+		ctx.Log.Info("desired nodes are ready", "instancegroup", instanceGroup.GetName(), "instances", instances)
 		state.SetNodesReady(true)
 		conditions = append(conditions, v1alpha1.NewInstanceGroupCondition(v1alpha1.NodesReady, corev1.ConditionTrue))
 		status.SetConditions(conditions)
@@ -319,6 +320,7 @@ func (ctx *EksInstanceGroupContext) UpdateNodeReadyCondition() bool {
 	if state.IsNodesReady() {
 		state.Publisher.Publish(kubeprovider.NodesNotReadyEvent, "instancegroup", instanceGroup.GetName(), "instances", instances)
 	}
+	ctx.Log.Info("desired nodes are not ready", "instancegroup", instanceGroup.GetName(), "instances", instances)
 	state.SetNodesReady(false)
 	conditions = append(conditions, v1alpha1.NewInstanceGroupCondition(v1alpha1.NodesReady, corev1.ConditionFalse))
 	status.SetConditions(conditions)

controllers/provisioners/eks/update.go

@@ -319,3 +319,51 @@ func (ctx *EksInstanceGroupContext) LaunchConfigurationDrifted() bool {
 
 	return drift
 }
+
+func (ctx *EksInstanceGroupContext) UpdateManagedPolicies(roleName string) error {
+	var (
+		instanceGroup      = ctx.GetInstanceGroup()
+		state              = ctx.GetDiscoveredState()
+		configuration      = instanceGroup.GetEKSConfiguration()
+		additionalPolicies = configuration.GetManagedPolicies()
+		needsAttach        = make([]string, 0)
+		needsDetach        = make([]string, 0)
+	)
+
+	managedPolicies := ctx.GetManagedPoliciesList(additionalPolicies)
+	attachedPolicies := state.GetAttachedPolicies()
+
+	attachedArns := make([]string, 0)
+	for _, p := range attachedPolicies {
+		attachedArns = append(attachedArns, aws.StringValue(p.PolicyArn))
+	}
+
+	for _, policy := range managedPolicies {
+		if !common.ContainsString(attachedArns, policy) {
+			needsAttach = append(needsAttach, policy)
+		}
+	}
+
+	if len(attachedArns) == 0 {
+		needsAttach = managedPolicies
+	}
+
+	for _, policy := range attachedArns {
+		if !common.ContainsString(managedPolicies, policy) {
+			needsDetach = append(needsDetach, policy)
+		}
+	}
+
+	err := ctx.AwsWorker.AttachManagedPolicies(roleName, needsAttach)
+	if err != nil {
+		return err
+	}
+
+	err = ctx.AwsWorker.DetachManagedPolicies(roleName, needsDetach)
+	if err != nil {
+		return err
+	}
+
+	ctx.Log.Info("updated managed policies", "instancegroup", instanceGroup.GetName(), "iamrole", roleName)
+	return nil
+}

controllers/provisioners/eks/update_test.go

@@ -420,3 +420,51 @@ func TestScalingGroupUpdatePredicate(t *testing.T) {
 		g.Expect(got).To(gomega.Equal(tc.expected))
 	}
 }
+
+func TestUpdateManagedPolicies(t *testing.T) {
+	var (
+		g             = gomega.NewGomegaWithT(t)
+		k             = MockKubernetesClientSet()
+		ig            = MockInstanceGroup()
+		configuration = ig.GetEKSConfiguration()
+		asgMock       = NewAutoScalingMocker()
+		iamMock       = NewIamMocker()
+	)
+
+	w := MockAwsWorker(asgMock, iamMock)
+	ctx := MockContext(ig, k, w)
+
+	tests := []struct {
+		attachedPolicies   []*iam.AttachedPolicy
+		additionalPolicies []string
+		expectedAttached   int
+		expectedDetached   int
+	}{
+		// default policies attached, no changes needed
+		{attachedPolicies: MockAttachedPolicies(DefaultManagedPolicies...), additionalPolicies: []string{}, expectedAttached: 0, expectedDetached: 0},
+		// default policies not attached
+		{attachedPolicies: MockAttachedPolicies(), additionalPolicies: []string{}, expectedAttached: 3, expectedDetached: 0},
+		// additional policies need to be attached
+		{attachedPolicies: MockAttachedPolicies(DefaultManagedPolicies...), additionalPolicies: []string{"policy-1", "policy-2"}, expectedAttached: 2, expectedDetached: 0},
+		// additional policies need to be detached
+		{attachedPolicies: MockAttachedPolicies("AmazonEKSWorkerNodePolicy", "AmazonEKS_CNI_Policy", "AmazonEC2ContainerRegistryReadOnly", "policy-1"), additionalPolicies: []string{}, expectedAttached: 0, expectedDetached: 1},
+		// additional policies need to be attached & detached
+		{attachedPolicies: MockAttachedPolicies("AmazonEKSWorkerNodePolicy", "AmazonEKS_CNI_Policy", "AmazonEC2ContainerRegistryReadOnly", "policy-1"), additionalPolicies: []string{"policy-2"}, expectedAttached: 1, expectedDetached: 1},
+	}
+
+	for _, tc := range tests {
+		iamMock.AttachRolePolicyCallCount = 0
+		iamMock.DetachRolePolicyCallCount = 0
+		ctx.SetDiscoveredState(&DiscoveredState{
+			Publisher: kubeprovider.EventPublisher{
+				Client: k.Kubernetes,
+			},
+			AttachedPolicies: tc.attachedPolicies,
+		})
+		configuration.SetManagedPolicies(tc.additionalPolicies)
+		err := ctx.UpdateManagedPolicies("some-role")
+		g.Expect(err).NotTo(gomega.HaveOccurred())
+		g.Expect(iamMock.AttachRolePolicyCallCount).To(gomega.Equal(tc.expectedAttached))
+		g.Expect(iamMock.DetachRolePolicyCallCount).To(gomega.Equal(tc.expectedDetached))
+	}
+}