From 678cfcb7fce123ee648a6e44da460499a31ea756 Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Wed, 27 May 2020 10:43:29 -0700 Subject: [PATCH 1/7] IAM calls optimization --- controllers/providers/aws/aws.go | 44 +++++++++++++------------- controllers/provisioners/eks/create.go | 5 +-- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go index c198b94d..f442ba35 100644 --- a/controllers/providers/aws/aws.go +++ b/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,7 @@ 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) CreateScalingGroupRole(name string, managedPolicies []string) (*iam.Role, *iam.InstanceProfile, error) { var ( assumeRolePolicyDocument = `{ "Version": "2012-10-17", @@ -310,6 +310,15 @@ func (w *AwsWorker) CreateUpdateScalingGroupRole(name string, managedPolicies [] return createdRole, createdProfile, errors.Wrap(err, "failed to create role") } createdRole = out.Role + for _, policy := range managedPolicies { + _, err = w.IamClient.AttachRolePolicy(&iam.AttachRolePolicyInput{ + RoleName: aws.String(name), + PolicyArn: aws.String(policy), + }) + if err != nil { + return createdRole, createdProfile, errors.Wrap(err, "failed to attach role policies") + } + } } else { createdRole = role } @@ -323,30 +332,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 diff --git a/controllers/provisioners/eks/create.go b/controllers/provisioners/eks/create.go index 4b4dc151..688a026b 100644 --- a/controllers/provisioners/eks/create.go +++ b/controllers/provisioners/eks/create.go @@ -130,17 +130,18 @@ func (ctx *EksInstanceGroupContext) CreateManagedRole() error { ) 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.CreateUpdateScalingGroupRole(roleName, managedPolicies) + role, profile, err := ctx.AwsWorker.CreateScalingGroupRole(roleName, managedPolicies) if err != nil { return err } - 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) From 1b76d1184ef99df224bb41bffcbb2b11cd8fe875 Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Thu, 28 May 2020 09:38:34 -0700 Subject: [PATCH 2/7] iam optimization --- controllers/providers/aws/aws.go | 42 ++++++++++++++++++------ controllers/provisioners/eks/cloud.go | 18 ++++++++++ controllers/provisioners/eks/create.go | 20 ++++++----- controllers/provisioners/eks/eks_test.go | 18 ++++++++++ controllers/provisioners/eks/update.go | 35 ++++++++++++++++++++ 5 files changed, 114 insertions(+), 19 deletions(-) diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go index f442ba35..9a12eb36 100644 --- a/controllers/providers/aws/aws.go +++ b/controllers/providers/aws/aws.go @@ -286,7 +286,38 @@ func (w *AwsWorker) DeleteScalingGroupRole(name string, managedPolicies []string return nil } -func (w *AwsWorker) CreateScalingGroupRole(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) 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", @@ -310,15 +341,6 @@ func (w *AwsWorker) CreateScalingGroupRole(name string, managedPolicies []string return createdRole, createdProfile, errors.Wrap(err, "failed to create role") } createdRole = out.Role - for _, policy := range managedPolicies { - _, err = w.IamClient.AttachRolePolicy(&iam.AttachRolePolicyInput{ - RoleName: aws.String(name), - PolicyArn: aws.String(policy), - }) - if err != nil { - return createdRole, createdProfile, errors.Wrap(err, "failed to attach role policies") - } - } } else { createdRole = role } diff --git a/controllers/provisioners/eks/cloud.go b/controllers/provisioners/eks/cloud.go index 064420a8..f57c1331 100644 --- a/controllers/provisioners/eks/cloud.go +++ b/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 diff --git a/controllers/provisioners/eks/create.go b/controllers/provisioners/eks/create.go index 688a026b..6243c16f 100644 --- a/controllers/provisioners/eks/create.go +++ b/controllers/provisioners/eks/create.go @@ -122,11 +122,10 @@ 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() { @@ -134,13 +133,16 @@ func (ctx *EksInstanceGroupContext) CreateManagedRole() error { 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.CreateScalingGroupRole(roleName, managedPolicies) + err = ctx.UpdateManagedPolicies(roleName) if err != nil { - return err + return errors.Wrap(err, "failed to update managed policies") } + ctx.Log.Info("reconciled managed role", "instancegroup", instanceGroup.GetName(), "iamrole", roleName) state.SetRole(role) diff --git a/controllers/provisioners/eks/eks_test.go b/controllers/provisioners/eks/eks_test.go index f51779d9..950e7416 100644 --- a/controllers/provisioners/eks/eks_test.go +++ b/controllers/provisioners/eks/eks_test.go @@ -361,8 +361,26 @@ type MockIamClient struct { AttachRolePolicyErr error DetachRolePolicyErr error 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) { diff --git a/controllers/provisioners/eks/update.go b/controllers/provisioners/eks/update.go index 03ed2c7e..bf868947 100644 --- a/controllers/provisioners/eks/update.go +++ b/controllers/provisioners/eks/update.go @@ -319,3 +319,38 @@ 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() + needsUpdate bool + ) + + // create a controller-owned role for the instancegroup + managedPolicies := ctx.GetManagedPoliciesList(additionalPolicies) + attachedPolicies := state.GetAttachedPolicies() + + for _, policy := range attachedPolicies { + name := aws.StringValue(policy.PolicyName) + if !common.ContainsString(managedPolicies, name) { + needsUpdate = true + } + } + + if len(attachedPolicies) == 0 { + needsUpdate = true + } + + if needsUpdate { + err := ctx.AwsWorker.AttachManagedPolicies(roleName, managedPolicies) + if err != nil { + return err + } + ctx.Log.Info("updated managed policies", "instancegroup", instanceGroup.GetName(), "iamrole", roleName) + } + + return nil +} From 32f12ed3dfed0935cea8e1c1c8933f166b071e6b Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Thu, 28 May 2020 09:46:55 -0700 Subject: [PATCH 3/7] Update update.go --- controllers/provisioners/eks/update.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/provisioners/eks/update.go b/controllers/provisioners/eks/update.go index bf868947..db155c91 100644 --- a/controllers/provisioners/eks/update.go +++ b/controllers/provisioners/eks/update.go @@ -334,8 +334,8 @@ func (ctx *EksInstanceGroupContext) UpdateManagedPolicies(roleName string) error attachedPolicies := state.GetAttachedPolicies() for _, policy := range attachedPolicies { - name := aws.StringValue(policy.PolicyName) - if !common.ContainsString(managedPolicies, name) { + arn := aws.StringValue(policy.PolicyArn) + if !common.ContainsString(managedPolicies, arn) { needsUpdate = true } } From 75982c1c21a9cc701eb9e73b1ef61858644233f8 Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Thu, 28 May 2020 09:51:15 -0700 Subject: [PATCH 4/7] Update helpers.go --- controllers/provisioners/eks/helpers.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go index 683837f5..fa5d8313 100644 --- a/controllers/provisioners/eks/helpers.go +++ b/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", strings.Join(instanceIds, ",")) 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", strings.Join(instanceIds, ",")) state.SetNodesReady(false) conditions = append(conditions, v1alpha1.NewInstanceGroupCondition(v1alpha1.NodesReady, corev1.ConditionFalse)) status.SetConditions(conditions) From e17ea44f7b93fd40417068438a79ea482f029409 Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Thu, 28 May 2020 10:11:41 -0700 Subject: [PATCH 5/7] Update helpers.go --- controllers/provisioners/eks/helpers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go index fa5d8313..502034fb 100644 --- a/controllers/provisioners/eks/helpers.go +++ b/controllers/provisioners/eks/helpers.go @@ -310,7 +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", strings.Join(instanceIds, ",")) + 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) @@ -320,7 +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", strings.Join(instanceIds, ",")) + 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) From 1b15734fb7528e6c5ee2836e707e73f18ffc29d3 Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Thu, 28 May 2020 10:20:04 -0700 Subject: [PATCH 6/7] Update update.go --- controllers/provisioners/eks/update.go | 1 - 1 file changed, 1 deletion(-) diff --git a/controllers/provisioners/eks/update.go b/controllers/provisioners/eks/update.go index db155c91..cad0c97b 100644 --- a/controllers/provisioners/eks/update.go +++ b/controllers/provisioners/eks/update.go @@ -329,7 +329,6 @@ func (ctx *EksInstanceGroupContext) UpdateManagedPolicies(roleName string) error needsUpdate bool ) - // create a controller-owned role for the instancegroup managedPolicies := ctx.GetManagedPoliciesList(additionalPolicies) attachedPolicies := state.GetAttachedPolicies() From 39e64fc13b41a216fdcd2baf80dbe919df29472f Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Thu, 28 May 2020 11:23:07 -0700 Subject: [PATCH 7/7] add tests --- controllers/providers/aws/aws.go | 13 ++++++ controllers/provisioners/eks/eks_test.go | 17 ++++++++ controllers/provisioners/eks/update.go | 38 ++++++++++------ controllers/provisioners/eks/update_test.go | 48 +++++++++++++++++++++ 4 files changed, 104 insertions(+), 12 deletions(-) diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go index 9a12eb36..b49019b2 100644 --- a/controllers/providers/aws/aws.go +++ b/controllers/providers/aws/aws.go @@ -299,6 +299,19 @@ func (w *AwsWorker) AttachManagedPolicies(name string, managedPolicies []string) 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( diff --git a/controllers/provisioners/eks/eks_test.go b/controllers/provisioners/eks/eks_test.go index 950e7416..1ab90ddd 100644 --- a/controllers/provisioners/eks/eks_test.go +++ b/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,7 +372,9 @@ type MockIamClient struct { AddRoleToInstanceProfileErr error RemoveRoleFromInstanceProfileErr error AttachRolePolicyErr error + AttachRolePolicyCallCount int DetachRolePolicyErr error + DetachRolePolicyCallCount int WaitUntilInstanceProfileExistsErr error ListAttachedRolePoliciesErr error Role *iam.Role @@ -418,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 } diff --git a/controllers/provisioners/eks/update.go b/controllers/provisioners/eks/update.go index cad0c97b..3e907d7a 100644 --- a/controllers/provisioners/eks/update.go +++ b/controllers/provisioners/eks/update.go @@ -326,30 +326,44 @@ func (ctx *EksInstanceGroupContext) UpdateManagedPolicies(roleName string) error state = ctx.GetDiscoveredState() configuration = instanceGroup.GetEKSConfiguration() additionalPolicies = configuration.GetManagedPolicies() - needsUpdate bool + needsAttach = make([]string, 0) + needsDetach = make([]string, 0) ) managedPolicies := ctx.GetManagedPoliciesList(additionalPolicies) attachedPolicies := state.GetAttachedPolicies() - for _, policy := range attachedPolicies { - arn := aws.StringValue(policy.PolicyArn) - if !common.ContainsString(managedPolicies, arn) { - needsUpdate = true + 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(attachedPolicies) == 0 { - needsUpdate = true + if len(attachedArns) == 0 { + needsAttach = managedPolicies } - if needsUpdate { - err := ctx.AwsWorker.AttachManagedPolicies(roleName, managedPolicies) - if err != nil { - return err + for _, policy := range attachedArns { + if !common.ContainsString(managedPolicies, policy) { + needsDetach = append(needsDetach, policy) } - ctx.Log.Info("updated managed policies", "instancegroup", instanceGroup.GetName(), "iamrole", roleName) } + 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 } diff --git a/controllers/provisioners/eks/update_test.go b/controllers/provisioners/eks/update_test.go index aaeb2c33..606b1914 100644 --- a/controllers/provisioners/eks/update_test.go +++ b/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)) + } +}