From 1645925b73de73054fa995c9a16e6e42d3309bd9 Mon Sep 17 00:00:00 2001 From: Jake Klein Date: Mon, 1 Feb 2021 13:48:26 +0000 Subject: [PATCH] Refactor iamserviceaccounts (#3135) --- pkg/actions/{iam => irsa}/create.go | 7 +- pkg/actions/irsa/delete.go | 28 +++ .../{iam => irsa}/fakes/fake_stack_manager.go | 175 ++++++++++++++++-- pkg/actions/irsa/get.go | 43 +++++ pkg/actions/irsa/get_test.go | 171 +++++++++++++++++ pkg/actions/{iam/iam.go => irsa/irsa.go} | 34 ++-- .../irsa_suite_test.go} | 6 +- pkg/actions/{iam => irsa}/tasks.go | 6 +- pkg/actions/{iam => irsa}/update.go | 8 +- pkg/actions/{iam => irsa}/update_test.go | 18 +- pkg/addons/irsa_helper.go | 56 +++--- pkg/addons/vpc_controller.go | 2 +- pkg/cfn/manager/create_tasks.go | 11 +- pkg/cfn/manager/delete_tasks.go | 2 +- pkg/cfn/manager/iam.go | 25 +-- pkg/cfn/manager/tasks.go | 11 +- pkg/ctl/cmdutils/configfile.go | 12 +- pkg/ctl/create/iamserviceaccount.go | 4 +- pkg/ctl/delete/iamserviceaccount.go | 24 +-- pkg/ctl/get/iamserviceaccount.go | 65 ++----- pkg/ctl/update/iamserviceaccount.go | 4 +- pkg/eks/tasks.go | 11 +- 22 files changed, 517 insertions(+), 206 deletions(-) rename pkg/actions/{iam => irsa}/create.go (74%) create mode 100644 pkg/actions/irsa/delete.go rename pkg/actions/{iam => irsa}/fakes/fake_stack_manager.go (59%) create mode 100644 pkg/actions/irsa/get.go create mode 100644 pkg/actions/irsa/get_test.go rename pkg/actions/{iam/iam.go => irsa/irsa.go} (58%) rename pkg/actions/{iam/iam_suite_test.go => irsa/irsa_suite_test.go} (59%) rename pkg/actions/{iam => irsa}/tasks.go (85%) rename pkg/actions/{iam => irsa}/update.go (89%) rename pkg/actions/{iam => irsa}/update_test.go (82%) diff --git a/pkg/actions/iam/create.go b/pkg/actions/irsa/create.go similarity index 74% rename from pkg/actions/iam/create.go rename to pkg/actions/irsa/create.go index 3833324362..19ca09cbe5 100644 --- a/pkg/actions/iam/create.go +++ b/pkg/actions/irsa/create.go @@ -1,18 +1,17 @@ -package iam +package irsa import ( api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" - "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" "github.com/weaveworks/eksctl/pkg/kubernetes" ) func (a *Manager) CreateIAMServiceAccount(iamServiceAccounts []*api.ClusterIAMServiceAccount, plan bool) error { - taskTree := a.stackManager.NewTasksToCreateIAMServiceAccounts(iamServiceAccounts, a.oidcManager, kubernetes.NewCachedClientSet(a.clientSet), false) + taskTree := a.stackManager.NewTasksToCreateIAMServiceAccounts(iamServiceAccounts, a.oidcManager, kubernetes.NewCachedClientSet(a.clientSet)) taskTree.PlanMode = plan err := doTasks(taskTree) - cmdutils.LogPlanModeWarning(plan && len(iamServiceAccounts) > 0) + logPlanModeWarning(plan && len(iamServiceAccounts) > 0) return err } diff --git a/pkg/actions/irsa/delete.go b/pkg/actions/irsa/delete.go new file mode 100644 index 0000000000..1e56f42965 --- /dev/null +++ b/pkg/actions/irsa/delete.go @@ -0,0 +1,28 @@ +package irsa + +import ( + "fmt" + + "github.com/kris-nova/logger" + "github.com/weaveworks/eksctl/pkg/kubernetes" +) + +func (m *Manager) Delete(shouldDelete func(string) bool, plan, wait bool) error { + taskTree, err := m.stackManager.NewTasksToDeleteIAMServiceAccounts(shouldDelete, kubernetes.NewCachedClientSet(m.clientSet), wait) + if err != nil { + return err + } + taskTree.PlanMode = plan + + logger.Info(taskTree.Describe()) + if errs := taskTree.DoAllSync(); len(errs) > 0 { + logger.Info("%d error(s) occurred and IAM Role stacks haven't been deleted properly, you may wish to check CloudFormation console", len(errs)) + for _, err := range errs { + logger.Critical("%s\n", err.Error()) + } + return fmt.Errorf("failed to delete iamserviceaccount(s)") + } + + logPlanModeWarning(plan && taskTree.Len() > 0) + return nil +} diff --git a/pkg/actions/iam/fakes/fake_stack_manager.go b/pkg/actions/irsa/fakes/fake_stack_manager.go similarity index 59% rename from pkg/actions/iam/fakes/fake_stack_manager.go rename to pkg/actions/irsa/fakes/fake_stack_manager.go index e218bddc46..363b62200c 100644 --- a/pkg/actions/iam/fakes/fake_stack_manager.go +++ b/pkg/actions/irsa/fakes/fake_stack_manager.go @@ -5,7 +5,7 @@ import ( "sync" "github.com/aws/aws-sdk-go/service/cloudformation" - "github.com/weaveworks/eksctl/pkg/actions/iam" + "github.com/weaveworks/eksctl/pkg/actions/irsa" "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/cfn/manager" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" @@ -14,6 +14,18 @@ import ( ) type FakeStackManager struct { + GetIAMServiceAccountsStub func() ([]*v1alpha5.ClusterIAMServiceAccount, error) + getIAMServiceAccountsMutex sync.RWMutex + getIAMServiceAccountsArgsForCall []struct { + } + getIAMServiceAccountsReturns struct { + result1 []*v1alpha5.ClusterIAMServiceAccount + result2 error + } + getIAMServiceAccountsReturnsOnCall map[int]struct { + result1 []*v1alpha5.ClusterIAMServiceAccount + result2 error + } ListStacksMatchingStub func(string, ...string) ([]*cloudformation.Stack, error) listStacksMatchingMutex sync.RWMutex listStacksMatchingArgsForCall []struct { @@ -28,13 +40,12 @@ type FakeStackManager struct { result1 []*cloudformation.Stack result2 error } - NewTasksToCreateIAMServiceAccountsStub func([]*v1alpha5.ClusterIAMServiceAccount, *iamoidc.OpenIDConnectManager, kubernetes.ClientSetGetter, bool) *tasks.TaskTree + NewTasksToCreateIAMServiceAccountsStub func([]*v1alpha5.ClusterIAMServiceAccount, *iamoidc.OpenIDConnectManager, kubernetes.ClientSetGetter) *tasks.TaskTree newTasksToCreateIAMServiceAccountsMutex sync.RWMutex newTasksToCreateIAMServiceAccountsArgsForCall []struct { arg1 []*v1alpha5.ClusterIAMServiceAccount arg2 *iamoidc.OpenIDConnectManager arg3 kubernetes.ClientSetGetter - arg4 bool } newTasksToCreateIAMServiceAccountsReturns struct { result1 *tasks.TaskTree @@ -42,6 +53,21 @@ type FakeStackManager struct { newTasksToCreateIAMServiceAccountsReturnsOnCall map[int]struct { result1 *tasks.TaskTree } + NewTasksToDeleteIAMServiceAccountsStub func(func(string) bool, kubernetes.ClientSetGetter, bool) (*tasks.TaskTree, error) + newTasksToDeleteIAMServiceAccountsMutex sync.RWMutex + newTasksToDeleteIAMServiceAccountsArgsForCall []struct { + arg1 func(string) bool + arg2 kubernetes.ClientSetGetter + arg3 bool + } + newTasksToDeleteIAMServiceAccountsReturns struct { + result1 *tasks.TaskTree + result2 error + } + newTasksToDeleteIAMServiceAccountsReturnsOnCall map[int]struct { + result1 *tasks.TaskTree + result2 error + } UpdateStackStub func(string, string, string, manager.TemplateData, map[string]string) error updateStackMutex sync.RWMutex updateStackArgsForCall []struct { @@ -61,6 +87,62 @@ type FakeStackManager struct { invocationsMutex sync.RWMutex } +func (fake *FakeStackManager) GetIAMServiceAccounts() ([]*v1alpha5.ClusterIAMServiceAccount, error) { + fake.getIAMServiceAccountsMutex.Lock() + ret, specificReturn := fake.getIAMServiceAccountsReturnsOnCall[len(fake.getIAMServiceAccountsArgsForCall)] + fake.getIAMServiceAccountsArgsForCall = append(fake.getIAMServiceAccountsArgsForCall, struct { + }{}) + stub := fake.GetIAMServiceAccountsStub + fakeReturns := fake.getIAMServiceAccountsReturns + fake.recordInvocation("GetIAMServiceAccounts", []interface{}{}) + fake.getIAMServiceAccountsMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStackManager) GetIAMServiceAccountsCallCount() int { + fake.getIAMServiceAccountsMutex.RLock() + defer fake.getIAMServiceAccountsMutex.RUnlock() + return len(fake.getIAMServiceAccountsArgsForCall) +} + +func (fake *FakeStackManager) GetIAMServiceAccountsCalls(stub func() ([]*v1alpha5.ClusterIAMServiceAccount, error)) { + fake.getIAMServiceAccountsMutex.Lock() + defer fake.getIAMServiceAccountsMutex.Unlock() + fake.GetIAMServiceAccountsStub = stub +} + +func (fake *FakeStackManager) GetIAMServiceAccountsReturns(result1 []*v1alpha5.ClusterIAMServiceAccount, result2 error) { + fake.getIAMServiceAccountsMutex.Lock() + defer fake.getIAMServiceAccountsMutex.Unlock() + fake.GetIAMServiceAccountsStub = nil + fake.getIAMServiceAccountsReturns = struct { + result1 []*v1alpha5.ClusterIAMServiceAccount + result2 error + }{result1, result2} +} + +func (fake *FakeStackManager) GetIAMServiceAccountsReturnsOnCall(i int, result1 []*v1alpha5.ClusterIAMServiceAccount, result2 error) { + fake.getIAMServiceAccountsMutex.Lock() + defer fake.getIAMServiceAccountsMutex.Unlock() + fake.GetIAMServiceAccountsStub = nil + if fake.getIAMServiceAccountsReturnsOnCall == nil { + fake.getIAMServiceAccountsReturnsOnCall = make(map[int]struct { + result1 []*v1alpha5.ClusterIAMServiceAccount + result2 error + }) + } + fake.getIAMServiceAccountsReturnsOnCall[i] = struct { + result1 []*v1alpha5.ClusterIAMServiceAccount + result2 error + }{result1, result2} +} + func (fake *FakeStackManager) ListStacksMatching(arg1 string, arg2 ...string) ([]*cloudformation.Stack, error) { fake.listStacksMatchingMutex.Lock() ret, specificReturn := fake.listStacksMatchingReturnsOnCall[len(fake.listStacksMatchingArgsForCall)] @@ -126,7 +208,7 @@ func (fake *FakeStackManager) ListStacksMatchingReturnsOnCall(i int, result1 []* }{result1, result2} } -func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccounts(arg1 []*v1alpha5.ClusterIAMServiceAccount, arg2 *iamoidc.OpenIDConnectManager, arg3 kubernetes.ClientSetGetter, arg4 bool) *tasks.TaskTree { +func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccounts(arg1 []*v1alpha5.ClusterIAMServiceAccount, arg2 *iamoidc.OpenIDConnectManager, arg3 kubernetes.ClientSetGetter) *tasks.TaskTree { var arg1Copy []*v1alpha5.ClusterIAMServiceAccount if arg1 != nil { arg1Copy = make([]*v1alpha5.ClusterIAMServiceAccount, len(arg1)) @@ -138,14 +220,13 @@ func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccounts(arg1 []*v1alpha arg1 []*v1alpha5.ClusterIAMServiceAccount arg2 *iamoidc.OpenIDConnectManager arg3 kubernetes.ClientSetGetter - arg4 bool - }{arg1Copy, arg2, arg3, arg4}) + }{arg1Copy, arg2, arg3}) stub := fake.NewTasksToCreateIAMServiceAccountsStub fakeReturns := fake.newTasksToCreateIAMServiceAccountsReturns - fake.recordInvocation("NewTasksToCreateIAMServiceAccounts", []interface{}{arg1Copy, arg2, arg3, arg4}) + fake.recordInvocation("NewTasksToCreateIAMServiceAccounts", []interface{}{arg1Copy, arg2, arg3}) fake.newTasksToCreateIAMServiceAccountsMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3, arg4) + return stub(arg1, arg2, arg3) } if specificReturn { return ret.result1 @@ -159,17 +240,17 @@ func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsCallCount() int return len(fake.newTasksToCreateIAMServiceAccountsArgsForCall) } -func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsCalls(stub func([]*v1alpha5.ClusterIAMServiceAccount, *iamoidc.OpenIDConnectManager, kubernetes.ClientSetGetter, bool) *tasks.TaskTree) { +func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsCalls(stub func([]*v1alpha5.ClusterIAMServiceAccount, *iamoidc.OpenIDConnectManager, kubernetes.ClientSetGetter) *tasks.TaskTree) { fake.newTasksToCreateIAMServiceAccountsMutex.Lock() defer fake.newTasksToCreateIAMServiceAccountsMutex.Unlock() fake.NewTasksToCreateIAMServiceAccountsStub = stub } -func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsArgsForCall(i int) ([]*v1alpha5.ClusterIAMServiceAccount, *iamoidc.OpenIDConnectManager, kubernetes.ClientSetGetter, bool) { +func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsArgsForCall(i int) ([]*v1alpha5.ClusterIAMServiceAccount, *iamoidc.OpenIDConnectManager, kubernetes.ClientSetGetter) { fake.newTasksToCreateIAMServiceAccountsMutex.RLock() defer fake.newTasksToCreateIAMServiceAccountsMutex.RUnlock() argsForCall := fake.newTasksToCreateIAMServiceAccountsArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsReturns(result1 *tasks.TaskTree) { @@ -195,6 +276,72 @@ func (fake *FakeStackManager) NewTasksToCreateIAMServiceAccountsReturnsOnCall(i }{result1} } +func (fake *FakeStackManager) NewTasksToDeleteIAMServiceAccounts(arg1 func(string) bool, arg2 kubernetes.ClientSetGetter, arg3 bool) (*tasks.TaskTree, error) { + fake.newTasksToDeleteIAMServiceAccountsMutex.Lock() + ret, specificReturn := fake.newTasksToDeleteIAMServiceAccountsReturnsOnCall[len(fake.newTasksToDeleteIAMServiceAccountsArgsForCall)] + fake.newTasksToDeleteIAMServiceAccountsArgsForCall = append(fake.newTasksToDeleteIAMServiceAccountsArgsForCall, struct { + arg1 func(string) bool + arg2 kubernetes.ClientSetGetter + arg3 bool + }{arg1, arg2, arg3}) + stub := fake.NewTasksToDeleteIAMServiceAccountsStub + fakeReturns := fake.newTasksToDeleteIAMServiceAccountsReturns + fake.recordInvocation("NewTasksToDeleteIAMServiceAccounts", []interface{}{arg1, arg2, arg3}) + fake.newTasksToDeleteIAMServiceAccountsMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStackManager) NewTasksToDeleteIAMServiceAccountsCallCount() int { + fake.newTasksToDeleteIAMServiceAccountsMutex.RLock() + defer fake.newTasksToDeleteIAMServiceAccountsMutex.RUnlock() + return len(fake.newTasksToDeleteIAMServiceAccountsArgsForCall) +} + +func (fake *FakeStackManager) NewTasksToDeleteIAMServiceAccountsCalls(stub func(func(string) bool, kubernetes.ClientSetGetter, bool) (*tasks.TaskTree, error)) { + fake.newTasksToDeleteIAMServiceAccountsMutex.Lock() + defer fake.newTasksToDeleteIAMServiceAccountsMutex.Unlock() + fake.NewTasksToDeleteIAMServiceAccountsStub = stub +} + +func (fake *FakeStackManager) NewTasksToDeleteIAMServiceAccountsArgsForCall(i int) (func(string) bool, kubernetes.ClientSetGetter, bool) { + fake.newTasksToDeleteIAMServiceAccountsMutex.RLock() + defer fake.newTasksToDeleteIAMServiceAccountsMutex.RUnlock() + argsForCall := fake.newTasksToDeleteIAMServiceAccountsArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeStackManager) NewTasksToDeleteIAMServiceAccountsReturns(result1 *tasks.TaskTree, result2 error) { + fake.newTasksToDeleteIAMServiceAccountsMutex.Lock() + defer fake.newTasksToDeleteIAMServiceAccountsMutex.Unlock() + fake.NewTasksToDeleteIAMServiceAccountsStub = nil + fake.newTasksToDeleteIAMServiceAccountsReturns = struct { + result1 *tasks.TaskTree + result2 error + }{result1, result2} +} + +func (fake *FakeStackManager) NewTasksToDeleteIAMServiceAccountsReturnsOnCall(i int, result1 *tasks.TaskTree, result2 error) { + fake.newTasksToDeleteIAMServiceAccountsMutex.Lock() + defer fake.newTasksToDeleteIAMServiceAccountsMutex.Unlock() + fake.NewTasksToDeleteIAMServiceAccountsStub = nil + if fake.newTasksToDeleteIAMServiceAccountsReturnsOnCall == nil { + fake.newTasksToDeleteIAMServiceAccountsReturnsOnCall = make(map[int]struct { + result1 *tasks.TaskTree + result2 error + }) + } + fake.newTasksToDeleteIAMServiceAccountsReturnsOnCall[i] = struct { + result1 *tasks.TaskTree + result2 error + }{result1, result2} +} + func (fake *FakeStackManager) UpdateStack(arg1 string, arg2 string, arg3 string, arg4 manager.TemplateData, arg5 map[string]string) error { fake.updateStackMutex.Lock() ret, specificReturn := fake.updateStackReturnsOnCall[len(fake.updateStackArgsForCall)] @@ -263,10 +410,14 @@ func (fake *FakeStackManager) UpdateStackReturnsOnCall(i int, result1 error) { func (fake *FakeStackManager) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() + fake.getIAMServiceAccountsMutex.RLock() + defer fake.getIAMServiceAccountsMutex.RUnlock() fake.listStacksMatchingMutex.RLock() defer fake.listStacksMatchingMutex.RUnlock() fake.newTasksToCreateIAMServiceAccountsMutex.RLock() defer fake.newTasksToCreateIAMServiceAccountsMutex.RUnlock() + fake.newTasksToDeleteIAMServiceAccountsMutex.RLock() + defer fake.newTasksToDeleteIAMServiceAccountsMutex.RUnlock() fake.updateStackMutex.RLock() defer fake.updateStackMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} @@ -288,4 +439,4 @@ func (fake *FakeStackManager) recordInvocation(key string, args []interface{}) { fake.invocations[key] = append(fake.invocations[key], args) } -var _ iam.StackManager = new(FakeStackManager) +var _ irsa.StackManager = new(FakeStackManager) diff --git a/pkg/actions/irsa/get.go b/pkg/actions/irsa/get.go new file mode 100644 index 0000000000..55c0c9b1a3 --- /dev/null +++ b/pkg/actions/irsa/get.go @@ -0,0 +1,43 @@ +package irsa + +import ( + "github.com/pkg/errors" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" +) + +func (m *Manager) Get(namespace, name string) ([]*api.ClusterIAMServiceAccount, error) { + remoteServiceAccounts, err := m.stackManager.GetIAMServiceAccounts() + if err != nil { + return nil, errors.Wrap(err, "getting iamserviceaccounts") + } + + if namespace != "" { + remoteServiceAccounts = filterByNamespace(remoteServiceAccounts, namespace) + } + + if name != "" { + remoteServiceAccounts = filterByName(remoteServiceAccounts, name) + } + + return remoteServiceAccounts, nil +} + +func filterByNamespace(serviceAccounts []*api.ClusterIAMServiceAccount, namespace string) []*api.ClusterIAMServiceAccount { + var serviceAccountsMatching []*api.ClusterIAMServiceAccount + for _, sa := range serviceAccounts { + if sa.Namespace == namespace { + serviceAccountsMatching = append(serviceAccountsMatching, sa) + } + } + return serviceAccountsMatching +} + +func filterByName(serviceAccounts []*api.ClusterIAMServiceAccount, name string) []*api.ClusterIAMServiceAccount { + var serviceAccountsMatching []*api.ClusterIAMServiceAccount + for _, sa := range serviceAccounts { + if sa.Name == name { + serviceAccountsMatching = append(serviceAccountsMatching, sa) + } + } + return serviceAccountsMatching +} diff --git a/pkg/actions/irsa/get_test.go b/pkg/actions/irsa/get_test.go new file mode 100644 index 0000000000..97021d1eaa --- /dev/null +++ b/pkg/actions/irsa/get_test.go @@ -0,0 +1,171 @@ +package irsa_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/weaveworks/eksctl/pkg/actions/irsa" + "github.com/weaveworks/eksctl/pkg/actions/irsa/fakes" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" +) + +var _ = Describe("Get", func() { + + var ( + irsaManager *irsa.Manager + fakeStackManager *fakes.FakeStackManager + ) + + BeforeEach(func() { + fakeStackManager = new(fakes.FakeStackManager) + + irsaManager = irsa.New("my-cluster", fakeStackManager, nil, nil) + }) + + When("no options are specified", func() { + It("returns all service accounts", func() { + fakeStackManager.GetIAMServiceAccountsReturns([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa-2", + Namespace: "not-default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + }, nil) + + serviceAccounts, err := irsaManager.Get("", "") + Expect(err).NotTo(HaveOccurred()) + + Expect(fakeStackManager.GetIAMServiceAccountsCallCount()).To(Equal(1)) + Expect(serviceAccounts).To(Equal([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa-2", + Namespace: "not-default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + })) + }) + }) + + When("name option is specified", func() { + It("returns only the service account matching the name", func() { + fakeStackManager.GetIAMServiceAccountsReturns([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa-2", + Namespace: "not-default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + }, nil) + + serviceAccounts, err := irsaManager.Get("", "test-sa") + Expect(err).NotTo(HaveOccurred()) + + Expect(fakeStackManager.GetIAMServiceAccountsCallCount()).To(Equal(1)) + Expect(serviceAccounts).To(Equal([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + })) + }) + }) + + When("namespace option is specified", func() { + It("returns only the service account matching the name", func() { + fakeStackManager.GetIAMServiceAccountsReturns([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa-2", + Namespace: "not-default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + }, nil) + + serviceAccounts, err := irsaManager.Get("not-default", "") + Expect(err).NotTo(HaveOccurred()) + + Expect(fakeStackManager.GetIAMServiceAccountsCallCount()).To(Equal(1)) + Expect(serviceAccounts).To(Equal([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa-2", + Namespace: "not-default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + })) + }) + }) + + When("name and namespace option is specified", func() { + It("returns only the service account matching the name", func() { + fakeStackManager.GetIAMServiceAccountsReturns([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "some-other-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + }, nil) + + serviceAccounts, err := irsaManager.Get("default", "test-sa") + Expect(err).NotTo(HaveOccurred()) + + Expect(fakeStackManager.GetIAMServiceAccountsCallCount()).To(Equal(1)) + Expect(serviceAccounts).To(Equal([]*api.ClusterIAMServiceAccount{ + { + ClusterIAMMeta: api.ClusterIAMMeta{ + Name: "test-sa", + Namespace: "default", + }, + AttachPolicyARNs: []string{"arn-123"}, + }, + })) + }) + }) +}) diff --git a/pkg/actions/iam/iam.go b/pkg/actions/irsa/irsa.go similarity index 58% rename from pkg/actions/iam/iam.go rename to pkg/actions/irsa/irsa.go index ad22cb26de..2f5f5737cf 100644 --- a/pkg/actions/iam/iam.go +++ b/pkg/actions/irsa/irsa.go @@ -1,4 +1,4 @@ -package iam +package irsa import ( "fmt" @@ -6,7 +6,6 @@ import ( "github.com/kris-nova/logger" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/cfn/manager" - "github.com/weaveworks/eksctl/pkg/eks" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/utils/tasks" @@ -14,27 +13,27 @@ import ( ) type Manager struct { - clusterName string - clusterProvider *eks.ClusterProvider - oidcManager *iamoidc.OpenIDConnectManager - stackManager StackManager - clientSet kubeclient.Interface + clusterName string + oidcManager *iamoidc.OpenIDConnectManager + stackManager StackManager + clientSet kubeclient.Interface } //go:generate counterfeiter -o fakes/fake_stack_manager.go . StackManager type StackManager interface { ListStacksMatching(nameRegex string, statusFilters ...string) ([]*manager.Stack, error) UpdateStack(stackName, changeSetName, description string, templateData manager.TemplateData, parameters map[string]string) error - NewTasksToCreateIAMServiceAccounts(serviceAccounts []*api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter, replaceExistingRole bool) *tasks.TaskTree + NewTasksToCreateIAMServiceAccounts(serviceAccounts []*api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter) *tasks.TaskTree + GetIAMServiceAccounts() ([]*api.ClusterIAMServiceAccount, error) + NewTasksToDeleteIAMServiceAccounts(shouldDelete func(string) bool, clientSetGetter kubernetes.ClientSetGetter, wait bool) (*tasks.TaskTree, error) } -func New(clusterName string, clusterProvider *eks.ClusterProvider, stackManager StackManager, oidcManager *iamoidc.OpenIDConnectManager, clientSet kubeclient.Interface) *Manager { +func New(clusterName string, stackManager StackManager, oidcManager *iamoidc.OpenIDConnectManager, clientSet kubeclient.Interface) *Manager { return &Manager{ - clusterName: clusterName, - clusterProvider: clusterProvider, - oidcManager: oidcManager, - stackManager: stackManager, - clientSet: clientSet, + clusterName: clusterName, + oidcManager: oidcManager, + stackManager: stackManager, + clientSet: clientSet, } } @@ -49,3 +48,10 @@ func doTasks(taskTree *tasks.TaskTree) error { } return nil } + +// logPlanModeWarning will log a message to inform user that they are in plan-mode +func logPlanModeWarning(plan bool) { + if plan { + logger.Warning("no changes were applied, run again with '--approve' to apply the changes") + } +} diff --git a/pkg/actions/iam/iam_suite_test.go b/pkg/actions/irsa/irsa_suite_test.go similarity index 59% rename from pkg/actions/iam/iam_suite_test.go rename to pkg/actions/irsa/irsa_suite_test.go index b1a0bd6864..012ee11253 100644 --- a/pkg/actions/iam/iam_suite_test.go +++ b/pkg/actions/irsa/irsa_suite_test.go @@ -1,4 +1,4 @@ -package iam_test +package irsa_test import ( "testing" @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" ) -func TestIam(t *testing.T) { +func TestIRSA(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Iam Suite") + RunSpecs(t, "IRSA Suite") } diff --git a/pkg/actions/iam/tasks.go b/pkg/actions/irsa/tasks.go similarity index 85% rename from pkg/actions/iam/tasks.go rename to pkg/actions/irsa/tasks.go index ee771a36e4..06e2bde80f 100644 --- a/pkg/actions/iam/tasks.go +++ b/pkg/actions/irsa/tasks.go @@ -1,4 +1,4 @@ -package iam +package irsa import ( "fmt" @@ -12,9 +12,9 @@ import ( "github.com/weaveworks/eksctl/pkg/utils/tasks" ) -func NewUpdateIAMServiceAccountTask(clusterName string, sa *api.ClusterIAMServiceAccount, stackManager StackManager, iamServiceAccount *api.ClusterIAMServiceAccount, oidcManager *iamoidc.OpenIDConnectManager) (*tasks.TaskTree, error) { +func NewUpdateIAMServiceAccountTask(clusterName string, sa *api.ClusterIAMServiceAccount, stackManager StackManager, oidcManager *iamoidc.OpenIDConnectManager) (*tasks.TaskTree, error) { - rs := builder.NewIAMServiceAccountResourceSet(iamServiceAccount, oidcManager) + rs := builder.NewIAMServiceAccountResourceSet(sa, oidcManager) err := rs.AddAllResources() if err != nil { return nil, err diff --git a/pkg/actions/iam/update.go b/pkg/actions/irsa/update.go similarity index 89% rename from pkg/actions/iam/update.go rename to pkg/actions/irsa/update.go index 862350a78b..d9b49ad7f0 100644 --- a/pkg/actions/iam/update.go +++ b/pkg/actions/irsa/update.go @@ -1,10 +1,8 @@ -package iam +package irsa import ( "fmt" - "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" - "github.com/weaveworks/eksctl/pkg/cfn/manager" "github.com/kris-nova/logger" @@ -36,7 +34,7 @@ func (a *Manager) UpdateIAMServiceAccounts(iamServiceAccounts []*api.ClusterIAMS continue } - taskTree, err := NewUpdateIAMServiceAccountTask(a.clusterName, iamServiceAccount, a.stackManager, iamServiceAccount, a.oidcManager) + taskTree, err := NewUpdateIAMServiceAccountTask(a.clusterName, iamServiceAccount, a.stackManager, a.oidcManager) if err != nil { return err } @@ -47,7 +45,7 @@ func (a *Manager) UpdateIAMServiceAccounts(iamServiceAccounts []*api.ClusterIAMS logger.Info("the following IAMServiceAccounts will not be updated as they do not exist: %v", strings.Join(nonExistingSAs, ", ")) } - defer cmdutils.LogPlanModeWarning(plan && len(iamServiceAccounts) > 0) + defer logPlanModeWarning(plan && len(iamServiceAccounts) > 0) return doTasks(updateTasks) } diff --git a/pkg/actions/iam/update_test.go b/pkg/actions/irsa/update_test.go similarity index 82% rename from pkg/actions/iam/update_test.go rename to pkg/actions/irsa/update_test.go index 0ae66bac65..dd3566c33c 100644 --- a/pkg/actions/iam/update_test.go +++ b/pkg/actions/irsa/update_test.go @@ -1,4 +1,4 @@ -package iam_test +package irsa_test import ( "github.com/aws/aws-sdk-go/aws" @@ -6,22 +6,19 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/weaveworks/eksctl/pkg/cfn/manager" - "github.com/weaveworks/eksctl/pkg/eks" - "github.com/weaveworks/eksctl/pkg/actions/iam" - "github.com/weaveworks/eksctl/pkg/actions/iam/fakes" + "github.com/weaveworks/eksctl/pkg/actions/irsa" + "github.com/weaveworks/eksctl/pkg/actions/irsa/fakes" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" - "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" ) var _ = Describe("Update", func() { var ( - iamManager *iam.Manager + irsaManager *irsa.Manager oidc *iamoidc.OpenIDConnectManager fakeStackManager *fakes.FakeStackManager - mockProvider *mockprovider.MockProvider serviceAccount []*api.ClusterIAMServiceAccount ) @@ -38,12 +35,11 @@ var _ = Describe("Update", func() { var err error fakeStackManager = new(fakes.FakeStackManager) - mockProvider = mockprovider.NewMockProvider() oidc, err = iamoidc.NewOpenIDConnectManager(nil, "456123987123", "https://oidc.eks.us-west-2.amazonaws.com/id/A39A2842863C47208955D753DE205E6E", "aws") Expect(err).ToNot(HaveOccurred()) oidc.ProviderARN = "arn:aws:iam::456123987123:oidc-provider/oidc.eks.us-west-2.amazonaws.com/id/A39A2842863C47208955D753DE205E6E" - iamManager = iam.New("my-cluster", &eks.ClusterProvider{Provider: mockProvider}, fakeStackManager, oidc, nil) + irsaManager = irsa.New("my-cluster", fakeStackManager, oidc, nil) }) When("the IAMServiceAccount exists", func() { @@ -54,7 +50,7 @@ var _ = Describe("Update", func() { }, }, nil) - err := iamManager.UpdateIAMServiceAccounts(serviceAccount, false) + err := irsaManager.UpdateIAMServiceAccounts(serviceAccount, false) Expect(err).NotTo(HaveOccurred()) Expect(fakeStackManager.ListStacksMatchingCallCount()).To(Equal(1)) @@ -78,7 +74,7 @@ var _ = Describe("Update", func() { }, }, nil) - err := iamManager.UpdateIAMServiceAccounts(serviceAccount, true) + err := irsaManager.UpdateIAMServiceAccounts(serviceAccount, true) Expect(err).NotTo(HaveOccurred()) Expect(fakeStackManager.ListStacksMatchingCallCount()).To(Equal(1)) diff --git a/pkg/addons/irsa_helper.go b/pkg/addons/irsa_helper.go index 4dc68d2d71..da3e71e54e 100644 --- a/pkg/addons/irsa_helper.go +++ b/pkg/addons/irsa_helper.go @@ -2,38 +2,37 @@ package addons import ( "fmt" - "strings" + + "github.com/weaveworks/eksctl/pkg/actions/irsa" + + "github.com/weaveworks/eksctl/pkg/cfn/manager" "github.com/pkg/errors" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" - "github.com/weaveworks/eksctl/pkg/kubernetes" - "github.com/weaveworks/eksctl/pkg/utils/tasks" ) -type serviceAccountCreator interface { - NewTasksToCreateIAMServiceAccounts(serviceAccounts []*api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter, replaceExistingRole bool) *tasks.TaskTree -} - // IRSAHelper provides methods for enabling IRSA type IRSAHelper interface { IsSupported() (bool, error) - Create(serviceAccounts []*api.ClusterIAMServiceAccount) error + CreateOrUpdate(serviceAccounts *api.ClusterIAMServiceAccount) error } // irsaHelper applies the annotations required for a ServiceAccount to work with IRSA type irsaHelper struct { - oidc *iamoidc.OpenIDConnectManager - serviceAccountCreator - clientSet kubernetes.ClientSetGetter + oidc *iamoidc.OpenIDConnectManager + irsaManager *irsa.Manager + stackManager *manager.StackCollection + clusterName string } // NewIRSAHelper creates a new IRSAHelper -func NewIRSAHelper(oidc *iamoidc.OpenIDConnectManager, saCreator serviceAccountCreator, clientSet kubernetes.ClientSetGetter) IRSAHelper { +func NewIRSAHelper(oidc *iamoidc.OpenIDConnectManager, stackManager *manager.StackCollection, irsaManager *irsa.Manager, clusterName string) IRSAHelper { return &irsaHelper{ - oidc: oidc, - serviceAccountCreator: saCreator, - clientSet: clientSet, + oidc: oidc, + stackManager: stackManager, + irsaManager: irsaManager, + clusterName: clusterName, } } @@ -47,21 +46,20 @@ func (h *irsaHelper) IsSupported() (bool, error) { } // Create creates IRSA for the specified IAM service accounts -func (h *irsaHelper) Create(serviceAccounts []*api.ClusterIAMServiceAccount) error { - taskTree := h.NewTasksToCreateIAMServiceAccounts(serviceAccounts, h.oidc, h.clientSet, true) - if errs := taskTree.DoAllSync(); len(errs) > 0 { - return errors.Wrap(joinErrors(errs), "error creating IAM service account") +func (h *irsaHelper) CreateOrUpdate(sa *api.ClusterIAMServiceAccount) error { + serviceAccounts := []*api.ClusterIAMServiceAccount{sa} + stacks, err := h.stackManager.ListStacksMatching(makeIAMServiceAccountStackName(h.clusterName, sa.Namespace, sa.Name)) + if err != nil { + return errors.Wrapf(err, "error checking if iamserviceaccount %s/%s exists", sa.Namespace, sa.Name) + } + if len(stacks) == 0 { + err = h.irsaManager.CreateIAMServiceAccount(serviceAccounts, false) + } else { + err = h.irsaManager.UpdateIAMServiceAccounts(serviceAccounts, false) } - return nil + return err } -func joinErrors(errs []error) error { - if len(errs) == 1 { - return errs[0] - } - allErrs := []string{"errors:\n"} - for _, err := range errs { - allErrs = append(allErrs, fmt.Sprintf("- %v", err)) - } - return errors.New(strings.Join(allErrs, "\n")) +func makeIAMServiceAccountStackName(clusterName, namespace, name string) string { + return fmt.Sprintf("eksctl-%s-addon-iamserviceaccount-%s-%s", clusterName, namespace, name) } diff --git a/pkg/addons/vpc_controller.go b/pkg/addons/vpc_controller.go index f855fc7496..4af6c3ab57 100644 --- a/pkg/addons/vpc_controller.go +++ b/pkg/addons/vpc_controller.go @@ -244,7 +244,7 @@ func (v *VPCController) deployVPCResourceController() error { }, AttachPolicy: makePolicyDocument(), } - if err := v.irsa.Create([]*api.ClusterIAMServiceAccount{sa}); err != nil { + if err := v.irsa.CreateOrUpdate(sa); err != nil { return errors.Wrap(err, "error enabling IRSA") } } else { diff --git a/pkg/cfn/manager/create_tasks.go b/pkg/cfn/manager/create_tasks.go index 2cfb21b5f1..66cbe43192 100644 --- a/pkg/cfn/manager/create_tasks.go +++ b/pkg/cfn/manager/create_tasks.go @@ -96,7 +96,7 @@ func (c *StackCollection) NewClusterCompatTask() tasks.Task { } // NewTasksToCreateIAMServiceAccounts defines tasks required to create all of the IAM ServiceAccounts -func (c *StackCollection) NewTasksToCreateIAMServiceAccounts(serviceAccounts []*api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter, replaceExistingRole bool) *tasks.TaskTree { +func (c *StackCollection) NewTasksToCreateIAMServiceAccounts(serviceAccounts []*api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter) *tasks.TaskTree { taskTree := &tasks.TaskTree{Parallel: true} for i := range serviceAccounts { @@ -107,11 +107,10 @@ func (c *StackCollection) NewTasksToCreateIAMServiceAccounts(serviceAccounts []* } saTasks.Append(&taskWithClusterIAMServiceAccountSpec{ - info: fmt.Sprintf("create IAM role for serviceaccount %q", sa.NameString()), - stackCollection: c, - serviceAccount: sa, - oidc: oidc, - replaceExistingRole: replaceExistingRole, + info: fmt.Sprintf("create IAM role for serviceaccount %q", sa.NameString()), + stackCollection: c, + serviceAccount: sa, + oidc: oidc, }) saTasks.Append(&kubernetesTask{ diff --git a/pkg/cfn/manager/delete_tasks.go b/pkg/cfn/manager/delete_tasks.go index 65a1d42206..bdd413561f 100644 --- a/pkg/cfn/manager/delete_tasks.go +++ b/pkg/cfn/manager/delete_tasks.go @@ -159,7 +159,7 @@ func (c *StackCollection) NewTasksToDeleteIAMServiceAccounts(shouldDelete func(s Parallel: false, IsSubTask: true, } - name := c.GetIAMServiceAccountName(s) + name := GetIAMServiceAccountName(s) if !shouldDelete(name) { continue diff --git a/pkg/cfn/manager/iam.go b/pkg/cfn/manager/iam.go index 6d2456a97f..5c0ab54fc1 100644 --- a/pkg/cfn/manager/iam.go +++ b/pkg/cfn/manager/iam.go @@ -3,9 +3,6 @@ package manager import ( "fmt" - "github.com/pkg/errors" - - "github.com/aws/aws-sdk-go/aws/awserr" cfn "github.com/aws/aws-sdk-go/service/cloudformation" "github.com/kris-nova/logger" @@ -21,7 +18,7 @@ func (c *StackCollection) makeIAMServiceAccountStackName(namespace, name string) } // createIAMServiceAccountTask creates the iamserviceaccount in CloudFormation -func (c *StackCollection) createIAMServiceAccountTask(errs chan error, spec *api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, replaceExistingRole bool) error { +func (c *StackCollection) createIAMServiceAccountTask(errs chan error, spec *api.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager) error { name := c.makeIAMServiceAccountStackName(spec.Namespace, spec.Name) logger.Info("building iamserviceaccount stack %q", name) stack := builder.NewIAMServiceAccountResourceSet(spec, oidc) @@ -35,18 +32,6 @@ func (c *StackCollection) createIAMServiceAccountTask(errs chan error, spec *api spec.Tags[api.IAMServiceAccountNameTag] = spec.NameString() if err := c.CreateStack(name, stack, spec.Tags, nil, errs); err != nil { - if !replaceExistingRole { - return err - } - var awsErr awserr.Error - if errors.As(err, &awsErr) && awsErr.Code() == cfn.ErrCodeAlreadyExistsException { - logger.Debug("CFN stack for IRSA already exists, replacing it with a new stack") - if err := c.DeleteStackByNameSync(name); err != nil { - close(errs) - return errors.Wrap(err, "error deleting stack") - } - return c.createIAMServiceAccountTask(errs, spec, oidc, false) - } return err } return nil @@ -64,7 +49,7 @@ func (c *StackCollection) DescribeIAMServiceAccountStacks() ([]*Stack, error) { if *s.StackStatus == cfn.StackStatusDeleteComplete { continue } - if c.GetIAMServiceAccountName(s) != "" { + if GetIAMServiceAccountName(s) != "" { iamServiceAccountStacks = append(iamServiceAccountStacks, s) } } @@ -81,7 +66,7 @@ func (c *StackCollection) ListIAMServiceAccountStacks() ([]string, error) { names := []string{} for _, s := range stacks { - names = append(names, c.GetIAMServiceAccountName(s)) + names = append(names, GetIAMServiceAccountName(s)) } return names, nil } @@ -95,7 +80,7 @@ func (c *StackCollection) GetIAMServiceAccounts() ([]*api.ClusterIAMServiceAccou results := []*api.ClusterIAMServiceAccount{} for _, s := range stacks { - meta, err := api.ClusterIAMServiceAccountNameStringToClusterIAMMeta(c.GetIAMServiceAccountName(s)) + meta, err := api.ClusterIAMServiceAccountNameStringToClusterIAMMeta(GetIAMServiceAccountName(s)) if err != nil { return nil, err } @@ -127,7 +112,7 @@ func (c *StackCollection) GetIAMServiceAccounts() ([]*api.ClusterIAMServiceAccou } // GetIAMServiceAccountName will return iamserviceaccount name based on tags -func (*StackCollection) GetIAMServiceAccountName(s *Stack) string { +func GetIAMServiceAccountName(s *Stack) string { for _, tag := range s.Tags { if *tag.Key == api.IAMServiceAccountNameTag { return *tag.Value diff --git a/pkg/cfn/manager/tasks.go b/pkg/cfn/manager/tasks.go index f42b46f5b4..494bc3bc1c 100644 --- a/pkg/cfn/manager/tasks.go +++ b/pkg/cfn/manager/tasks.go @@ -61,16 +61,15 @@ func (t *clusterCompatTask) Do(errorCh chan error) error { } type taskWithClusterIAMServiceAccountSpec struct { - info string - stackCollection *StackCollection - serviceAccount *api.ClusterIAMServiceAccount - oidc *iamoidc.OpenIDConnectManager - replaceExistingRole bool + info string + stackCollection *StackCollection + serviceAccount *api.ClusterIAMServiceAccount + oidc *iamoidc.OpenIDConnectManager } func (t *taskWithClusterIAMServiceAccountSpec) Describe() string { return t.info } func (t *taskWithClusterIAMServiceAccountSpec) Do(errs chan error) error { - return t.stackCollection.createIAMServiceAccountTask(errs, t.serviceAccount, t.oidc, t.replaceExistingRole) + return t.stackCollection.createIAMServiceAccountTask(errs, t.serviceAccount, t.oidc) } type taskWithStackSpec struct { diff --git a/pkg/ctl/cmdutils/configfile.go b/pkg/ctl/cmdutils/configfile.go index 84332b4c41..e18f7629b3 100644 --- a/pkg/ctl/cmdutils/configfile.go +++ b/pkg/ctl/cmdutils/configfile.go @@ -608,7 +608,7 @@ func NewCreateIAMServiceAccountLoader(cmd *Cmd, saFilter *filter.IAMServiceAccou } // NewGetIAMServiceAccountLoader will load config or use flags for 'eksctl get iamserviceaccount' -func NewGetIAMServiceAccountLoader(cmd *Cmd, sa *api.ClusterIAMServiceAccount) ClusterConfigLoader { +func NewGetIAMServiceAccountLoader(cmd *Cmd) ClusterConfigLoader { l := newCommonClusterConfigLoader(cmd) l.validateWithConfigFile = func() error { @@ -619,20 +619,10 @@ func NewGetIAMServiceAccountLoader(cmd *Cmd, sa *api.ClusterIAMServiceAccount) C } l.validateWithoutConfigFile = func() error { - sa.AttachPolicyARNs = []string{""} // force to pass general validation - if l.ClusterConfig.Metadata.Name == "" { return ErrMustBeSet(ClusterNameFlag(cmd)) } - if l.NameArg != "" { - sa.Name = l.NameArg - } - - if sa.Name == "" { - l.ClusterConfig.IAM.ServiceAccounts = nil - } - l.Plan = false return nil diff --git a/pkg/ctl/create/iamserviceaccount.go b/pkg/ctl/create/iamserviceaccount.go index 9153f2e7c2..52813b56c6 100644 --- a/pkg/ctl/create/iamserviceaccount.go +++ b/pkg/ctl/create/iamserviceaccount.go @@ -3,7 +3,7 @@ package create import ( "errors" - "github.com/weaveworks/eksctl/pkg/actions/iam" + "github.com/weaveworks/eksctl/pkg/actions/irsa" "github.com/kris-nova/logger" "github.com/spf13/cobra" @@ -124,5 +124,5 @@ func doCreateIAMServiceAccount(cmd *cmdutils.Cmd, overrideExistingServiceAccount return err } - return iam.New(cfg.Metadata.Name, ctl, stackManager, oidc, clientSet).CreateIAMServiceAccount(filteredServiceAccounts, cmd.Plan) + return irsa.New(cfg.Metadata.Name, stackManager, oidc, clientSet).CreateIAMServiceAccount(filteredServiceAccounts, cmd.Plan) } diff --git a/pkg/ctl/delete/iamserviceaccount.go b/pkg/ctl/delete/iamserviceaccount.go index 0c8b3d948e..b0a2308c8b 100644 --- a/pkg/ctl/delete/iamserviceaccount.go +++ b/pkg/ctl/delete/iamserviceaccount.go @@ -7,10 +7,10 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/weaveworks/eksctl/pkg/actions/irsa" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" "github.com/weaveworks/eksctl/pkg/ctl/cmdutils/filter" - "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/printers" ) @@ -45,7 +45,7 @@ func deleteIAMServiceAccountCmdWithRunFunc(cmd *cmdutils.Cmd, runFunc func(cmd * fs.StringVar(&serviceAccount.Namespace, "namespace", "default", "namespace where to delete the iamserviceaccount") cmdutils.AddIAMServiceAccountFilterFlags(fs, &cmd.Include, &cmd.Exclude) - fs.BoolVar(&onlyMissing, "only-missing", false, "Only delete nodegroups that are not defined in the given config file") + fs.BoolVar(&onlyMissing, "only-missing", false, "Only delete iamserviceaccounts that are not defined in the given config file") cmdutils.AddApproveFlag(fs, cmd) cmdutils.AddRegionFlag(fs, &cmd.ProviderConfig) cmdutils.AddConfigFileFlag(fs, &cmd.ClusterConfigFile) @@ -116,26 +116,10 @@ func doDeleteIAMServiceAccount(cmd *cmdutils.Cmd, serviceAccount *api.ClusterIAM saSubset, _ := saFilter.MatchAll(cfg.IAM.ServiceAccounts) - tasks, err := stackManager.NewTasksToDeleteIAMServiceAccounts(saSubset.Has, kubernetes.NewCachedClientSet(clientSet), cmd.Wait) - if err != nil { - return err - } - tasks.PlanMode = cmd.Plan + irsaManager := irsa.New(cfg.Metadata.Name, stackManager, oidc, clientSet) if err := printer.LogObj(logger.Debug, "cfg.json = \\\n%s\n", cfg); err != nil { return err } - - logger.Info(tasks.Describe()) - if errs := tasks.DoAllSync(); len(errs) > 0 { - logger.Info("%d error(s) occurred and IAM Role stacks haven't been deleted properly, you may wish to check CloudFormation console", len(errs)) - for _, err := range errs { - logger.Critical("%s\n", err.Error()) - } - return fmt.Errorf("failed to delete iamserviceaccount(s)") - } - - cmdutils.LogPlanModeWarning(cmd.Plan && saSubset.Len() > 0) - - return nil + return irsaManager.Delete(saSubset.Has, cmd.Plan, cmd.Wait) } diff --git a/pkg/ctl/get/iamserviceaccount.go b/pkg/ctl/get/iamserviceaccount.go index c32372d7be..3fb49a18d2 100644 --- a/pkg/ctl/get/iamserviceaccount.go +++ b/pkg/ctl/get/iamserviceaccount.go @@ -1,16 +1,13 @@ package get import ( - "fmt" "os" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/weaveworks/eksctl/pkg/actions/irsa" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" - "github.com/weaveworks/eksctl/pkg/ctl/cmdutils/filter" - "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" "github.com/weaveworks/eksctl/pkg/printers" ) @@ -19,10 +16,8 @@ func getIAMServiceAccountCmd(cmd *cmdutils.Cmd) { cfg := api.NewClusterConfig() cmd.ClusterConfig = cfg - serviceAccount := &api.ClusterIAMServiceAccount{} - + var name, namespace string cfg.IAM.WithOIDC = api.Enabled() - cfg.IAM.ServiceAccounts = append(cfg.IAM.ServiceAccounts, serviceAccount) params := &getCmdParams{} @@ -30,14 +25,14 @@ func getIAMServiceAccountCmd(cmd *cmdutils.Cmd) { cmd.CobraCommand.RunE = func(_ *cobra.Command, args []string) error { cmd.NameArg = cmdutils.GetNameArg(args) - return doGetIAMServiceAccount(cmd, serviceAccount, params) + return doGetIAMServiceAccount(cmd, namespace, name, params) } cmd.FlagSetGroup.InFlagSet("General", func(fs *pflag.FlagSet) { fs.StringVar(&cfg.Metadata.Name, "cluster", "", "EKS cluster name") - fs.StringVar(&serviceAccount.Name, "name", "", "name of iamserviceaccount to get") - fs.StringVar(&serviceAccount.Namespace, "namespace", "default", "namespace to look for iamserviceaccount") + fs.StringVar(&namespace, "namespace", "", "namespace to look for iamserviceaccount") + fs.StringVar(&name, "name", "", "name of iamserviceaccount to get") cmdutils.AddRegionFlag(fs, &cmd.ProviderConfig) cmdutils.AddConfigFileFlag(fs, &cmd.ClusterConfigFile) @@ -49,8 +44,8 @@ func getIAMServiceAccountCmd(cmd *cmdutils.Cmd) { cmdutils.AddCommonFlagsForAWS(cmd.FlagSetGroup, &cmd.ProviderConfig, false) } -func doGetIAMServiceAccount(cmd *cmdutils.Cmd, serviceAccount *api.ClusterIAMServiceAccount, params *getCmdParams) error { - if err := cmdutils.NewGetIAMServiceAccountLoader(cmd, serviceAccount).Load(); err != nil { +func doGetIAMServiceAccount(cmd *cmdutils.Cmd, namespace, name string, params *getCmdParams) error { + if err := cmdutils.NewGetIAMServiceAccountLoader(cmd).Load(); err != nil { return err } @@ -74,44 +69,9 @@ func doGetIAMServiceAccount(cmd *cmdutils.Cmd, serviceAccount *api.ClusterIAMSer } stackManager := ctl.NewStackManager(cfg) + irsaManager := irsa.New(cfg.Metadata.Name, stackManager, nil, nil) + serviceAccounts, err := irsaManager.Get(namespace, name) - remoteServiceAccounts, err := stackManager.GetIAMServiceAccounts() - if err != nil { - return errors.Wrap(err, "getting iamserviceaccounts") - } - // we will show user the object based on given config file, - // and what we have learned about the iamserviceaccounts; - // that is not ideal, but we don't have a better option yet - cfg.IAM.ServiceAccounts = []*api.ClusterIAMServiceAccount{} - - saFilter := filter.NewIAMServiceAccountFilter() - - if cmd.ClusterConfigFile == "" { - // reset defaulted fields to avoid output being a complete lie - cfg.VPC = nil - cfg.CloudWatch = nil - // only return the iamserviceaccount that user asked for - var notFoundErr error - if serviceAccount.Name != "" { // name was given - notFoundErr = fmt.Errorf("iamserviceaccount %q not found", serviceAccount.NameString()) - saFilter.AppendIncludeNames(serviceAccount.NameString()) - } else if cmd.CobraCommand.Flag("namespace").Changed { // only namespace was given - notFoundErr = fmt.Errorf("no iamserviceaccounts found in namespace %q", serviceAccount.Namespace) - err = saFilter.AppendIncludeGlobs(remoteServiceAccounts, serviceAccount.Namespace+"/*") - if err != nil { - return fmt.Errorf("unable to append include globs in namespace %q", serviceAccount.Namespace) - } - } - saSubset, _ := saFilter.MatchAll(remoteServiceAccounts) - if saSubset.Len() == 0 { - return notFoundErr - } - } - - err = saFilter.ForEach(remoteServiceAccounts, func(_ int, remoteServiceAccount *api.ClusterIAMServiceAccount) error { - cfg.IAM.ServiceAccounts = append(cfg.IAM.ServiceAccounts, remoteServiceAccount) - return nil - }) if err != nil { return err } @@ -121,14 +81,11 @@ func doGetIAMServiceAccount(cmd *cmdutils.Cmd, serviceAccount *api.ClusterIAMSer return err } - var obj interface{} if params.output == "table" { addIAMServiceAccountSummaryTableColumns(printer.(*printers.TablePrinter)) - obj = cfg.IAM.ServiceAccounts - } else { - obj = cfg } - return printer.PrintObjWithKind("iamserviceaccounts", obj, os.Stdout) + + return printer.PrintObjWithKind("iamserviceaccounts", serviceAccounts, os.Stdout) } func addIAMServiceAccountSummaryTableColumns(printer *printers.TablePrinter) { diff --git a/pkg/ctl/update/iamserviceaccount.go b/pkg/ctl/update/iamserviceaccount.go index 5c773f00d5..0e5632ae04 100644 --- a/pkg/ctl/update/iamserviceaccount.go +++ b/pkg/ctl/update/iamserviceaccount.go @@ -3,7 +3,7 @@ package update import ( "errors" - "github.com/weaveworks/eksctl/pkg/actions/iam" + "github.com/weaveworks/eksctl/pkg/actions/irsa" "github.com/kris-nova/logger" "github.com/spf13/cobra" @@ -105,5 +105,5 @@ func doUpdateIAMServiceAccount(cmd *cmdutils.Cmd) error { return err } - return iam.New(cfg.Metadata.Name, ctl, stackManager, oidc, clientSet).UpdateIAMServiceAccounts(cfg.IAM.ServiceAccounts, cmd.Plan) + return irsa.New(cfg.Metadata.Name, stackManager, oidc, clientSet).UpdateIAMServiceAccounts(cfg.IAM.ServiceAccounts, cmd.Plan) } diff --git a/pkg/eks/tasks.go b/pkg/eks/tasks.go index c65c24a844..635297d956 100644 --- a/pkg/eks/tasks.go +++ b/pkg/eks/tasks.go @@ -5,6 +5,8 @@ import ( "fmt" "time" + "github.com/weaveworks/eksctl/pkg/actions/irsa" + "github.com/kris-nova/logger" "github.com/pkg/errors" "github.com/weaveworks/eksctl/pkg/cfn/manager" @@ -61,7 +63,13 @@ func (v *VPCControllerTask) Do(errCh chan error) error { } stackCollection := manager.NewStackCollection(v.ClusterProvider.Provider, v.ClusterConfig) - irsa := addons.NewIRSAHelper(oidc, stackCollection, kubernetes.NewCachedClientSet(rawClient.ClientSet())) + + clientSet, err := v.ClusterProvider.NewStdClientSet(v.ClusterConfig) + if err != nil { + return err + } + irsaManager := irsa.New(v.ClusterConfig.Metadata.Name, stackCollection, oidc, clientSet) + irsa := addons.NewIRSAHelper(oidc, stackCollection, irsaManager, v.ClusterConfig.Metadata.Name) // TODO PlanMode doesn't work as intended vpcController := addons.NewVPCController(rawClient, irsa, v.ClusterConfig.Status, v.ClusterProvider.Provider.Region(), v.PlanMode) @@ -299,7 +307,6 @@ func (c *ClusterProvider) appendCreateTasksForIAMServiceAccounts(cfg *api.Cluste api.IAMServiceAccountsWithImplicitServiceAccounts(cfg), oidcPlaceholder, clientSet, - false, ) newTasks.IsSubTask = true tasks.Append(newTasks)