diff --git a/pkg/actions/accessentry/fakes/fake_getter.go b/pkg/actions/accessentry/fakes/fake_getter.go new file mode 100644 index 00000000000..8456352719f --- /dev/null +++ b/pkg/actions/accessentry/fakes/fake_getter.go @@ -0,0 +1,120 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fakes + +import ( + "context" + "sync" + + "github.com/weaveworks/eksctl/pkg/actions/accessentry" + "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" +) + +type FakeGetterInterface struct { + GetStub func(context.Context, v1alpha5.ARN) ([]accessentry.Summary, error) + getMutex sync.RWMutex + getArgsForCall []struct { + arg1 context.Context + arg2 v1alpha5.ARN + } + getReturns struct { + result1 []accessentry.Summary + result2 error + } + getReturnsOnCall map[int]struct { + result1 []accessentry.Summary + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeGetterInterface) Get(arg1 context.Context, arg2 v1alpha5.ARN) ([]accessentry.Summary, error) { + fake.getMutex.Lock() + ret, specificReturn := fake.getReturnsOnCall[len(fake.getArgsForCall)] + fake.getArgsForCall = append(fake.getArgsForCall, struct { + arg1 context.Context + arg2 v1alpha5.ARN + }{arg1, arg2}) + stub := fake.GetStub + fakeReturns := fake.getReturns + fake.recordInvocation("Get", []interface{}{arg1, arg2}) + fake.getMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeGetterInterface) GetCallCount() int { + fake.getMutex.RLock() + defer fake.getMutex.RUnlock() + return len(fake.getArgsForCall) +} + +func (fake *FakeGetterInterface) GetCalls(stub func(context.Context, v1alpha5.ARN) ([]accessentry.Summary, error)) { + fake.getMutex.Lock() + defer fake.getMutex.Unlock() + fake.GetStub = stub +} + +func (fake *FakeGetterInterface) GetArgsForCall(i int) (context.Context, v1alpha5.ARN) { + fake.getMutex.RLock() + defer fake.getMutex.RUnlock() + argsForCall := fake.getArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeGetterInterface) GetReturns(result1 []accessentry.Summary, result2 error) { + fake.getMutex.Lock() + defer fake.getMutex.Unlock() + fake.GetStub = nil + fake.getReturns = struct { + result1 []accessentry.Summary + result2 error + }{result1, result2} +} + +func (fake *FakeGetterInterface) GetReturnsOnCall(i int, result1 []accessentry.Summary, result2 error) { + fake.getMutex.Lock() + defer fake.getMutex.Unlock() + fake.GetStub = nil + if fake.getReturnsOnCall == nil { + fake.getReturnsOnCall = make(map[int]struct { + result1 []accessentry.Summary + result2 error + }) + } + fake.getReturnsOnCall[i] = struct { + result1 []accessentry.Summary + result2 error + }{result1, result2} +} + +func (fake *FakeGetterInterface) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.getMutex.RLock() + defer fake.getMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeGetterInterface) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ accessentry.GetterInterface = new(FakeGetterInterface) diff --git a/pkg/actions/accessentry/getter.go b/pkg/actions/accessentry/getter.go index 418f81a6a57..7c2f4c2638e 100644 --- a/pkg/actions/accessentry/getter.go +++ b/pkg/actions/accessentry/getter.go @@ -10,6 +10,12 @@ import ( "github.com/weaveworks/eksctl/pkg/awsapi" ) +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate +//counterfeiter:generate -o fakes/fake_getter.go . GetterInterface +type GetterInterface interface { + Get(ctx context.Context, principalARN api.ARN) ([]Summary, error) +} + type Getter struct { clusterName string eksAPI awsapi.EKS diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index ce554d369fd..2be1d1993ab 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -35,7 +35,8 @@ type Migrator struct { eksAPI awsapi.EKS iamAPI awsapi.IAM clientSet kubernetes.Interface - aeCreator Creator + aeCreator CreatorInterface + aeGetter GetterInterface curAuthMode ekstypes.AuthenticationMode tgAuthMode ekstypes.AuthenticationMode } @@ -45,7 +46,8 @@ func NewMigrator( eksAPI awsapi.EKS, iamAPI awsapi.IAM, clientSet kubernetes.Interface, - aeCreator Creator, + aeCreator CreatorInterface, + aeGetter GetterInterface, curAuthMode ekstypes.AuthenticationMode, tgAuthMode ekstypes.AuthenticationMode, ) *Migrator { @@ -55,6 +57,7 @@ func NewMigrator( iamAPI: iamAPI, clientSet: clientSet, aeCreator: aeCreator, + aeGetter: aeGetter, curAuthMode: curAuthMode, tgAuthMode: tgAuthMode, } @@ -84,21 +87,17 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOp }) } - cmEntries, err := m.doGetIAMIdentityMappings(ctx) - if err != nil { - return err - } - - curAccessEntries, err := m.doGetAccessEntries(ctx) + curAccessEntries, err := m.aeGetter.Get(ctx, api.ARN{}) if err != nil && m.curAuthMode != ekstypes.AuthenticationModeConfigMap { - return err + return fmt.Errorf("fetching existing access entries: %w", err) } - newAccessEntries, skipAPImode, err := doFilterAccessEntries(cmEntries, curAccessEntries) + cmEntries, err := m.doGetIAMIdentityMappings(ctx) if err != nil { return err } + newAccessEntries, skipAPImode := doFilterAccessEntries(cmEntries, curAccessEntries) if len(newAccessEntries) > 0 { aeTasks := m.aeCreator.CreateTasks(ctx, newAccessEntries) aeTasks.IsSubTask = true @@ -162,11 +161,6 @@ func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekst } } -func (m *Migrator) doGetAccessEntries(ctx context.Context) ([]Summary, error) { - aeGetter := NewGetter(m.clusterName, m.eksAPI) - return aeGetter.Get(ctx, api.ARN{}) -} - func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity, error) { acm, err := authconfigmap.NewFromClientSet(m.clientSet) if err != nil { @@ -197,7 +191,7 @@ func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity getRoleOutput, err := m.iamAPI.GetRole(ctx, &awsiam.GetRoleInput{RoleName: &cmeName}) if err != nil { if errors.As(err, &noSuchEntity) { - return nil, fmt.Errorf("role %s does not exists, either delete the iamidentitymapping using \"eksctl delete iamidentitymapping --cluster %s --arn %s\" or create the role in AWS", cmeName, m.clusterName, cme.ARN()) + return nil, fmt.Errorf("role %q does not exists, either delete the iamidentitymapping using \"eksctl delete iamidentitymapping --cluster %s --arn %s\" or create the role in AWS", cmeName, m.clusterName, cme.ARN()) } return nil, err } @@ -218,7 +212,7 @@ func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity getUserOutput, err := m.iamAPI.GetUser(ctx, &awsiam.GetUserInput{UserName: &cmeName}) if err != nil { if errors.As(err, &noSuchEntity) { - return nil, fmt.Errorf("user \"%s\" does not exists, either delete the iamidentitymapping using \"eksctl delete iamidentitymapping --cluster %s --arn %s\" or create the user in AWS", cmeName, m.clusterName, cme.ARN()) + return nil, fmt.Errorf("user %q does not exists, either delete the iamidentitymapping using \"eksctl delete iamidentitymapping --cluster %s --arn %s\" or create the user in AWS", cmeName, m.clusterName, cme.ARN()) } return nil, err } @@ -231,7 +225,7 @@ func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity return cmEntries, nil } -func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([]api.AccessEntry, bool, error) { +func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([]api.AccessEntry, bool) { skipAPImode := false var toDoEntries []api.AccessEntry @@ -268,7 +262,7 @@ func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([ skipAPImode = true } case iam.ResourceTypeAccount: - logger.Warning("found account iamidentitymapping \"%s\", can not create access entry", cme.Account()) + logger.Warning("found account iamidentitymapping %q, cannot create access entry, skipping", cme.Account()) skipAPImode = true } } else { @@ -277,7 +271,7 @@ func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([ } } - return toDoEntries, skipAPImode, nil + return toDoEntries, skipAPImode } func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { @@ -295,7 +289,7 @@ func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { Type: "EC2_LINUX", } } - // For windows Nodes + // For Windows Nodes return &api.AccessEntry{ PrincipalARN: api.MustParseARN(cme.ARN()), Type: "EC2_WINDOWS", @@ -327,7 +321,7 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { } if containsSys { // Check if any GroupName start with "system:"" in name - logger.Warning("at least one group name associated with %s starts with \"system:\", can not create access entry, skipping", cme.ARN()) + logger.Warning("at least one group name associated with %q starts with \"system:\", can not create access entry, skipping", cme.ARN()) return nil } @@ -343,5 +337,4 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { func doDeleteAWSAuthConfigMap(ctx context.Context, clientset kubernetes.Interface, namespace, name string) error { logger.Info("deleting %q ConfigMap as it is no longer needed in API mode", name) return clientset.CoreV1().ConfigMaps(namespace).Delete(ctx, name, metav1.DeleteOptions{}) - } diff --git a/pkg/actions/accessentry/migrator_test.go b/pkg/actions/accessentry/migrator_test.go index 83424118296..04645066357 100644 --- a/pkg/actions/accessentry/migrator_test.go +++ b/pkg/actions/accessentry/migrator_test.go @@ -8,25 +8,35 @@ import ( "os" "strings" - "github.com/aws/aws-sdk-go-v2/service/eks" - awseks "github.com/aws/aws-sdk-go-v2/service/eks" - ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" "github.com/kris-nova/logger" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/mock" + "gopkg.in/yaml.v2" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + + "github.com/aws/aws-sdk-go-v2/aws" + ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + awsiam "github.com/aws/aws-sdk-go-v2/service/iam" + iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" + "github.com/weaveworks/eksctl/pkg/actions/accessentry" "github.com/weaveworks/eksctl/pkg/actions/accessentry/fakes" + "github.com/weaveworks/eksctl/pkg/authconfigmap" "github.com/weaveworks/eksctl/pkg/cfn/builder" + "github.com/weaveworks/eksctl/pkg/iam" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" - "k8s.io/client-go/kubernetes/fake" ) type migrateToAccessEntryEntry struct { - clusterName string - curAuthMode string - mockEKS func(provider *mockprovider.MockProvider) + curAuthMode ekstypes.AuthenticationMode + tgAuthMode ekstypes.AuthenticationMode + mockIAM func(provider *mockprovider.MockProvider) mockK8s func(clientSet *fake.Clientset) + mockAccessEntries func(getter *fakes.FakeGetterInterface) validateCustomLoggerOutput func(output string) options accessentry.MigrationOptions expectedErr string @@ -38,14 +48,17 @@ var _ = Describe("Migrate Access Entry", func() { migrator *accessentry.Migrator mockProvider *mockprovider.MockProvider fakeClientset *fake.Clientset + fakeAECreator accessentry.CreatorInterface + fakeAEGetter accessentry.GetterInterface clusterName = "test-cluster" + genericErr = fmt.Errorf("ERR") ) DescribeTable("Migrate", func(ae migrateToAccessEntryEntry) { var s fakes.FakeStackCreator s.CreateStackStub = func(ctx context.Context, stackName string, r builder.ResourceSetReader, tags map[string]string, parameters map[string]string, errorCh chan error) error { defer close(errorCh) - prefix := fmt.Sprintf("eksctl-%s-accessentry-", ae.clusterName) + prefix := fmt.Sprintf("eksctl-%s-accessentry-", clusterName) idx := strings.Index(stackName, prefix) if idx < 0 { return fmt.Errorf("expected stack name to have prefix %q", prefix) @@ -58,14 +71,9 @@ var _ = Describe("Migrate Access Entry", func() { return nil } - accessEntryCreator := &accessentry.Creator{ - ClusterName: ae.clusterName, - StackCreator: &s, - } - mockProvider = mockprovider.NewMockProvider() - if ae.mockEKS != nil { - ae.mockEKS(mockProvider) + if ae.mockIAM != nil { + ae.mockIAM(mockProvider) } fakeClientset = fake.NewSimpleClientset() @@ -73,6 +81,12 @@ var _ = Describe("Migrate Access Entry", func() { ae.mockK8s(fakeClientset) } + fakeAECreator = &accessentry.Creator{ClusterName: clusterName} + fakeAEGetter = &fakes.FakeGetterInterface{} + if ae.mockAccessEntries != nil { + ae.mockAccessEntries(fakeAEGetter.(*fakes.FakeGetterInterface)) + } + output := &bytes.Buffer{} if ae.validateCustomLoggerOutput != nil { defer func() { @@ -82,13 +96,14 @@ var _ = Describe("Migrate Access Entry", func() { } migrator = accessentry.NewMigrator( - ae.clusterName, + clusterName, mockProvider.MockEKS(), mockProvider.MockIAM(), fakeClientset, - *accessEntryCreator, - ekstypes.AuthenticationMode(ae.curAuthMode), - ekstypes.AuthenticationMode(ae.options.TargetAuthMode), + fakeAECreator, + fakeAEGetter, + ae.curAuthMode, + ae.tgAuthMode, ) err := migrator.MigrateToAccessEntry(context.Background(), ae.options) @@ -103,32 +118,306 @@ var _ = Describe("Migrate Access Entry", func() { if ae.validateCustomLoggerOutput != nil { ae.validateCustomLoggerOutput(output.String()) } - }, Entry("[API Error] Authentication mode update fails", migrateToAccessEntryEntry{ - clusterName: clusterName, - options: accessentry.MigrationOptions{ - TargetAuthMode: string(ekstypes.AuthenticationModeApi), - Approve: true, - }, - mockEKS: func(provider *mockprovider.MockProvider) { - mockProvider.MockEKS(). - On("UpdateClusterConfig", mock.Anything, mock.Anything). - Run(func(args mock.Arguments) { - Expect(args).To(HaveLen(2)) - Expect(args[1]).To(BeAssignableToTypeOf(&awseks.UpdateClusterConfigInput{})) - }). - Return(nil, fmt.Errorf("failed to update cluster config")) - - provider.MockEKS(). - On("ListAccessEntries", mock.Anything, mock.Anything). - Run(func(args mock.Arguments) { - Expect(args).To(HaveLen(2)) - Expect(args[1]).To(BeAssignableToTypeOf(&eks.ListAccessEntriesInput{})) - }). - Return(&eks.ListAccessEntriesOutput{ - AccessEntries: []string{}, + }, + Entry("[Validation Error] target authentication mode is CONFIG_MAP", migrateToAccessEntryEntry{ + tgAuthMode: ekstypes.AuthenticationModeConfigMap, + expectedErr: "target authentication mode is invalid", + }), + + Entry("[Validation Error] current authentication mode is API", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApi, + validateCustomLoggerOutput: func(output string) { + Expect(output).To(ContainSubstring(fmt.Sprintf("cluster authentication mode is already %s; there is no need to migrate to access entries", ekstypes.AuthenticationModeApi))) + }, + }), + + Entry("[API Error] getting access entries fails", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns(nil, genericErr) + }, + expectedErr: "fetching existing access entries", + }), + + Entry("[API Error] getting role fails", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns([]accessentry.Summary{}, nil) + }, + mockIAM: func(provider *mockprovider.MockProvider) { + provider.MockIAM(). + On("GetRole", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + Expect(args).To(HaveLen(2)) + Expect(args[1]).To(BeAssignableToTypeOf(&awsiam.GetRoleInput{})) + }). + Return(nil, &iamtypes.NoSuchEntityException{}). + Once() + }, + mockK8s: func(clientSet *fake.Clientset) { + roles := []iam.RoleIdentity{{RoleARN: "arn:aws:iam::111122223333:role/test"}} + rolesBytes, err := yaml.Marshal(roles) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.CoreV1().ConfigMaps(authconfigmap.ObjectNamespace).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authconfigmap.ObjectName, + }, + Data: map[string]string{ + "mapRoles": string(rolesBytes), + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }, + expectedErr: fmt.Sprintf("role %q does not exists", "test"), + }), + + Entry("[API Error] getting user fails", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns([]accessentry.Summary{}, nil) + }, + mockIAM: func(provider *mockprovider.MockProvider) { + provider.MockIAM(). + On("GetUser", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + Expect(args).To(HaveLen(2)) + Expect(args[1]).To(BeAssignableToTypeOf(&awsiam.GetUserInput{})) + }). + Return(nil, &iamtypes.NoSuchEntityException{}). + Once() + }, + mockK8s: func(clientSet *fake.Clientset) { + users := []iam.UserIdentity{{UserARN: "arn:aws:iam::111122223333:user/test"}} + usersBytes, err := yaml.Marshal(users) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.CoreV1().ConfigMaps(authconfigmap.ObjectNamespace).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authconfigmap.ObjectName, + }, + Data: map[string]string{ + "mapUsers": string(usersBytes), + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }, + expectedErr: fmt.Sprintf("user %q does not exists", "test"), + }), + + Entry("[TaskTree] should not switch to API mode if service-linked role iamidentitymapping is found", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns([]accessentry.Summary{}, nil) + }, + mockIAM: func(provider *mockprovider.MockProvider) { + provider.MockIAM(). + On("GetRole", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + Expect(args).To(HaveLen(2)) + Expect(args[1]).To(BeAssignableToTypeOf(&awsiam.GetRoleInput{})) + }). + Return(&awsiam.GetRoleOutput{ + Role: &iamtypes.Role{ + Arn: aws.String("arn:aws:iam::111122223333:role/aws-service-role/test"), + }, + }, nil). + Once() + }, + mockK8s: func(clientSet *fake.Clientset) { + roles := []iam.RoleIdentity{{RoleARN: "arn:aws:iam::111122223333:role/aws-service-role/test"}} + rolesBytes, err := yaml.Marshal(roles) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.CoreV1().ConfigMaps(authconfigmap.ObjectNamespace).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authconfigmap.ObjectName, + }, + Data: map[string]string{ + "mapRoles": string(rolesBytes), + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }, + validateCustomLoggerOutput: func(output string) { + Expect(output).To(ContainSubstring("found service-linked role iamidentitymapping")) + Expect(output).NotTo(ContainSubstring("update authentication mode from API_AND_CONFIG_MAP to API")) + Expect(output).NotTo(ContainSubstring("delete aws-auth configMap when authentication mode is API")) + }, + }), + + Entry("[TaskTree] should not switch to API mode if iamidentitymapping with non-master `system:` group is found", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns([]accessentry.Summary{}, nil) + }, + mockIAM: func(provider *mockprovider.MockProvider) { + provider.MockIAM(). + On("GetRole", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + Expect(args).To(HaveLen(2)) + Expect(args[1]).To(BeAssignableToTypeOf(&awsiam.GetRoleInput{})) + }). + Return(&awsiam.GetRoleOutput{ + Role: &iamtypes.Role{ + Arn: aws.String("arn:aws:iam::111122223333:role/test"), + }, + }, nil). + Once() + }, + mockK8s: func(clientSet *fake.Clientset) { + roles := []iam.RoleIdentity{ + { + RoleARN: "arn:aws:iam::111122223333:role/test", + KubernetesIdentity: iam.KubernetesIdentity{ + KubernetesGroups: []string{"system:tests"}, + }, + }, + } + rolesBytes, err := yaml.Marshal(roles) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.CoreV1().ConfigMaps(authconfigmap.ObjectNamespace).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authconfigmap.ObjectName, + }, + Data: map[string]string{ + "mapRoles": string(rolesBytes), + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }, + validateCustomLoggerOutput: func(output string) { + Expect(output).To(ContainSubstring("at least one group name associated with %q starts with \"system:\"", "arn:aws:iam::111122223333:role/test")) + Expect(output).NotTo(ContainSubstring("update authentication mode from API_AND_CONFIG_MAP to API")) + Expect(output).NotTo(ContainSubstring("delete aws-auth configMap when authentication mode is API")) + }, + }), + + Entry("[TaskTree] should not switch to API mode if account iamidentitymapping is found", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns([]accessentry.Summary{}, nil) + }, + mockK8s: func(clientSet *fake.Clientset) { + accounts := []string{"test-account"} + accountsBytes, err := yaml.Marshal(accounts) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.CoreV1().ConfigMaps(authconfigmap.ObjectNamespace).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authconfigmap.ObjectName, + }, + Data: map[string]string{ + "mapAccounts": string(accountsBytes), + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }, + validateCustomLoggerOutput: func(output string) { + Expect(output).To(ContainSubstring("found account iamidentitymapping")) + Expect(output).NotTo(ContainSubstring("update authentication mode from API_AND_CONFIG_MAP to API")) + Expect(output).NotTo(ContainSubstring("delete aws-auth configMap when authentication mode is API")) + }, + }), + + Entry("[TaskTree] should contain all expected tasks", migrateToAccessEntryEntry{ + curAuthMode: ekstypes.AuthenticationModeConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, + mockAccessEntries: func(getter *fakes.FakeGetterInterface) { + getter.GetReturns([]accessentry.Summary{ + { + PrincipalARN: "arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-1", + }, }, nil) - }, - expectedErr: "failed to update cluster config", - }), + }, + mockIAM: func(provider *mockprovider.MockProvider) { + provider.MockIAM(). + On("GetRole", mock.Anything, &awsiam.GetRoleInput{ + RoleName: aws.String("eksctl-test-cluster-nodegroup-NodeInstanceRole-1"), + }). + Return(&awsiam.GetRoleOutput{ + Role: &iamtypes.Role{ + Arn: aws.String("arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-1"), + }, + }, nil) + provider.MockIAM(). + On("GetRole", mock.Anything, &awsiam.GetRoleInput{ + RoleName: aws.String("eksctl-test-cluster-nodegroup-NodeInstanceRole-2"), + }). + Return(&awsiam.GetRoleOutput{ + Role: &iamtypes.Role{ + Arn: aws.String("arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-2"), + }, + }, nil) + provider.MockIAM(). + On("GetUser", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + Expect(args).To(HaveLen(2)) + Expect(args[1]).To(BeAssignableToTypeOf(&awsiam.GetUserInput{})) + }). + Return(&awsiam.GetUserOutput{ + User: &iamtypes.User{ + Arn: aws.String("arn:aws:iam::111122223333:user/admin"), + }, + }, nil). + Once() + }, + mockK8s: func(clientSet *fake.Clientset) { + roles := []iam.RoleIdentity{ + { + RoleARN: "arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-1", + KubernetesIdentity: iam.KubernetesIdentity{ + KubernetesUsername: "system:node:{{EC2PrivateDNSName}}", + KubernetesGroups: []string{"system:nodes", "system:bootstrappers"}, + }, + }, + { + RoleARN: "arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-2", + KubernetesIdentity: iam.KubernetesIdentity{ + KubernetesUsername: "system:node:{{EC2PrivateDNSName}}", + KubernetesGroups: []string{"system:nodes", "system:bootstrappers"}, + }, + }, + } + users := []iam.UserIdentity{ + { + UserARN: "arn:aws:iam::111122223333:user/admin", + KubernetesIdentity: iam.KubernetesIdentity{ + KubernetesUsername: "admin", + KubernetesGroups: []string{"system:masters"}, + }, + }, + } + rolesBytes, err := yaml.Marshal(roles) + Expect(err).NotTo(HaveOccurred()) + usersBytes, err := yaml.Marshal(users) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.CoreV1().ConfigMaps(authconfigmap.ObjectNamespace).Create(context.Background(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authconfigmap.ObjectName, + }, + Data: map[string]string{ + "mapRoles": string(rolesBytes), + "mapUsers": string(usersBytes), + }, + }, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }, + validateCustomLoggerOutput: func(output string) { + Expect(output).To(ContainSubstring("create access entry for principal ARN arn:aws:iam::111122223333:user/admin")) + Expect(output).To(ContainSubstring("create access entry for principal ARN arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-2")) + Expect(output).To(ContainSubstring("update authentication mode from CONFIG_MAP to API_AND_CONFIG_MAP")) + Expect(output).To(ContainSubstring("update authentication mode from API_AND_CONFIG_MAP to API")) + // filter out existing access entries + Expect(output).NotTo(ContainSubstring("create access entry for principal ARN arn:aws:iam::111122223333:role/eksctl-test-cluster-nodegroup-NodeInstanceRole-1")) + }, + }), ) }) diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index 28248d8f006..31022eed785 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -58,10 +58,11 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Migrat } stackManager := ctl.NewStackManager(cfg) - aeCreator := accessentryactions.Creator{ + aeCreator := &accessentryactions.Creator{ ClusterName: cmd.ClusterConfig.Metadata.Name, StackCreator: stackManager, } + aeGetter := accessentryactions.NewGetter(cfg.Metadata.Name, ctl.AWSProvider.EKS()) if err := accessentryactions.NewMigrator( cfg.Metadata.Name, @@ -69,6 +70,7 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Migrat ctl.AWSProvider.IAM(), clientSet, aeCreator, + aeGetter, ctl.GetClusterState().AccessConfig.AuthenticationMode, ekstypes.AuthenticationMode(options.TargetAuthMode), ).MigrateToAccessEntry(ctx, options); err != nil { diff --git a/pkg/iam/mapping.go b/pkg/iam/mapping.go index 526c1f1621c..753e96938b4 100644 --- a/pkg/iam/mapping.go +++ b/pkg/iam/mapping.go @@ -61,26 +61,26 @@ func CompareIdentity(a, b Identity) bool { // KubernetesIdentity represents a kubernetes identity to be used in iam mappings type KubernetesIdentity struct { - KubernetesUsername string `json:"username,omitempty"` - KubernetesGroups []string `json:"groups,omitempty"` + KubernetesUsername string `json:"username,omitempty" yaml:"username,omitempty"` + KubernetesGroups []string `json:"groups,omitempty" yaml:"groups,omitempty"` } // UserIdentity represents a mapping from an IAM user to a kubernetes identity type UserIdentity struct { - UserARN string `json:"userarn,omitempty"` - KubernetesIdentity + UserARN string `json:"userarn,omitempty"` + KubernetesIdentity `yaml:",inline"` } // RoleIdentity represents a mapping from an IAM role to a kubernetes identity type RoleIdentity struct { - RoleARN string `json:"rolearn,omitempty"` - KubernetesIdentity + RoleARN string `json:"rolearn,omitempty"` + KubernetesIdentity `yaml:",inline"` } // AccountIdentity represents a mapping from an IAM role to a kubernetes identity type AccountIdentity struct { - KubernetesAccount string `json:"account,omitempty"` - KubernetesIdentity + KubernetesAccount string `json:"account,omitempty" yaml:"account,omitempty"` + KubernetesIdentity `yaml:",inline"` } // ARN returns the ARN of the iam mapping