From a81e48290707bc6ba49ccdbaea3bdcd1ef462ad9 Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Fri, 1 Mar 2024 02:34:29 +0000 Subject: [PATCH 01/22] Added migrate-to-access-entry cmd structure --- pkg/actions/accessentry/migrator.go | 19 ++++++ pkg/ctl/utils/migrate_to_access_entry.go | 78 ++++++++++++++++++++++++ pkg/ctl/utils/utils.go | 1 + 3 files changed, 98 insertions(+) create mode 100644 pkg/actions/accessentry/migrator.go create mode 100644 pkg/ctl/utils/migrate_to_access_entry.go diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go new file mode 100644 index 0000000000..f586d7bb81 --- /dev/null +++ b/pkg/actions/accessentry/migrator.go @@ -0,0 +1,19 @@ +package accessentry + +import ( + "context" + "time" + + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" +) + +type AddonCreator interface { + Create(ctx context.Context, addon *api.Addon, waitTimeout time.Duration) error +} + +type AccessEntryMigrationOptions struct { + RemoveOIDCProviderTrustRelationship bool + TargetAuthMode string + Approve bool + Timeout time.Duration +} diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go new file mode 100644 index 0000000000..797eac1045 --- /dev/null +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -0,0 +1,78 @@ +package utils + +import ( + "context" + "fmt" + + ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + "github.com/kris-nova/logger" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/weaveworks/eksctl/pkg/actions/accessentry" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" + "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" +) + +func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { + cfg := api.NewClusterConfig() + cmd.ClusterConfig = cfg + + cmd.SetDescription("migrate-to-access-entry", "Migrates aws-auth to API authentication mode for the cluster", "") + + var options accessentry.AccessEntryMigrationOptions + cmd.FlagSetGroup.InFlagSet("Migrate to Access Entry", func(fs *pflag.FlagSet) { + fs.BoolVar(&options.RemoveOIDCProviderTrustRelationship, "remove-aws-auth", false, "Remove aws-auth from cluster") + fs.StringVar(&options.TargetAuthMode, "authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") + fs.BoolVar(&options.Approve, "approve", false, "Apply the changes") + + // fs.BoolVar(&options.SkipAgentInstallation, "skip-agent-installation", false, "Skip installing pod-identity-agent addon") + // cmdutils.AddIAMServiceAccountFilterFlags(fs, &cmd.Include, &cmd.Exclude) + }) + + cmd.FlagSetGroup.InFlagSet("General", func(fs *pflag.FlagSet) { + cmdutils.AddClusterFlag(fs, cmd.ClusterConfig.Metadata) + cmdutils.AddRegionFlag(fs, &cmd.ProviderConfig) + cmdutils.AddTimeoutFlag(fs, &options.Timeout) + }) + + cmd.CobraCommand.RunE = func(_ *cobra.Command, args []string) error { + cmd.NameArg = cmdutils.GetNameArg(args) + return doMigrateToAccessEntry(cmd, options) + } +} + +func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentry.AccessEntryMigrationOptions) error { + cfg := cmd.ClusterConfig + cmd.ClusterConfig.AccessConfig.AuthenticationMode = ekstypes.AuthenticationMode(options.TargetAuthMode) + tgAuthMode := cmd.ClusterConfig.AccessConfig.AuthenticationMode + + if cfg.Metadata.Name == "" { + return cmdutils.ErrMustBeSet(cmdutils.ClusterNameFlag(cmd)) + } + + ctx := context.Background() + ctl, err := cmd.NewProviderForExistingCluster(ctx) + if err != nil { + return err + } + + if ok, err := ctl.CanOperate(cfg); !ok { + return err + } + + if tgAuthMode != ekstypes.AuthenticationModeApi || tgAuthMode != ekstypes.AuthenticationModeApi { + return fmt.Errorf("Target authentication mode is invalid") + } + + curAuthMode := ctl.GetClusterState().AccessConfig.AuthenticationMode + + if curAuthMode != tgAuthMode { + logger.Info("Target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", tgAuthMode, curAuthMode) + + // Add UpdateAuthentication Mode Method call here + } + + // Current and Target mode is same, start migration + + return nil +} diff --git a/pkg/ctl/utils/utils.go b/pkg/ctl/utils/utils.go index b9eccaaa98..f677db4c7e 100644 --- a/pkg/ctl/utils/utils.go +++ b/pkg/ctl/utils/utils.go @@ -29,6 +29,7 @@ func Command(flagGrouping *cmdutils.FlagGrouping) *cobra.Command { cmdutils.AddResourceCmd(flagGrouping, verbCmd, describeAddonVersionsCmd) cmdutils.AddResourceCmd(flagGrouping, verbCmd, describeAddonConfigurationCmd) cmdutils.AddResourceCmd(flagGrouping, verbCmd, migrateToPodIdentityCmd) + cmdutils.AddResourceCmd(flagGrouping, verbCmd, migrateAccessEntryCmd) return verbCmd } From e7c03f10ffe257a6718039153240fa47cc22566d Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Fri, 1 Mar 2024 02:49:41 +0000 Subject: [PATCH 02/22] Fix Target Authentication mode validation --- pkg/ctl/utils/migrate_to_access_entry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index 797eac1045..826cc6b3a0 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -60,7 +60,7 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentry.AccessEntryMi return err } - if tgAuthMode != ekstypes.AuthenticationModeApi || tgAuthMode != ekstypes.AuthenticationModeApi { + if tgAuthMode != ekstypes.AuthenticationModeApi && tgAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { return fmt.Errorf("Target authentication mode is invalid") } From bcbcf5f87b2d15dc8597b688b4eabee504d431e3 Mon Sep 17 00:00:00 2001 From: Venkat Penmetsa Date: Fri, 1 Mar 2024 16:58:20 +0000 Subject: [PATCH 03/22] Added logic to get accessEntries and cmEntries from cluster --- pkg/ctl/utils/migrate_to_access_entry.go | 37 +++++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index 826cc6b3a0..cff68e4495 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/pflag" "github.com/weaveworks/eksctl/pkg/actions/accessentry" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" + "github.com/weaveworks/eksctl/pkg/authconfigmap" "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" ) @@ -61,18 +62,46 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentry.AccessEntryMi } if tgAuthMode != ekstypes.AuthenticationModeApi && tgAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { - return fmt.Errorf("Target authentication mode is invalid") + return fmt.Errorf("target authentication mode is invalid") } curAuthMode := ctl.GetClusterState().AccessConfig.AuthenticationMode if curAuthMode != tgAuthMode { - logger.Info("Target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", tgAuthMode, curAuthMode) - + logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", tgAuthMode, curAuthMode) // Add UpdateAuthentication Mode Method call here } - // Current and Target mode is same, start migration + // Get Access Entries from Cluster Provider + clusterProvider, err := cmd.NewProviderForExistingCluster(ctx) + if err != nil { + return err + } + asgetter := accessentry.NewGetter(cfg.Metadata.Name, clusterProvider.AWSProvider.EKS()) + accessEntries, err := asgetter.Get(ctx, api.ARN{}) + if err != nil { + return err + } + fmt.Printf("%+v\n", accessEntries) + + // Get CONFIGMAP Entries + clientSet, err := ctl.NewStdClientSet(cfg) + if err != nil { + return err + } + acm, err := authconfigmap.NewFromClientSet(clientSet) + if err != nil { + return err + } + cmEntries, err := acm.GetIdentities() + if err != nil { + return err + } + fmt.Printf("%+v\n", cmEntries) + + // Check if any of the cmEntries are in accessEntries, and add the remaining to NeedsUpdateList + + // Perform doCreateAccessEntry() on NeedsUpdateList return nil } From 9f796364cd60d865d6d80f9ebca6f76eab60b584 Mon Sep 17 00:00:00 2001 From: Venkat Penmetsa Date: Fri, 1 Mar 2024 23:26:51 +0000 Subject: [PATCH 04/22] Added logic to make unique list of configmap accessEntries, and stack creation logic --- pkg/ctl/utils/migrate_to_access_entry.go | 67 +++++++++++++++++------- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index cff68e4495..cb7512344e 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -8,7 +8,7 @@ import ( "github.com/kris-nova/logger" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/weaveworks/eksctl/pkg/actions/accessentry" + accessentryactions "github.com/weaveworks/eksctl/pkg/actions/accessentry" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/authconfigmap" "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" @@ -20,7 +20,7 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { cmd.SetDescription("migrate-to-access-entry", "Migrates aws-auth to API authentication mode for the cluster", "") - var options accessentry.AccessEntryMigrationOptions + var options accessentryactions.AccessEntryMigrationOptions cmd.FlagSetGroup.InFlagSet("Migrate to Access Entry", func(fs *pflag.FlagSet) { fs.BoolVar(&options.RemoveOIDCProviderTrustRelationship, "remove-aws-auth", false, "Remove aws-auth from cluster") fs.StringVar(&options.TargetAuthMode, "authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") @@ -42,7 +42,7 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { } } -func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentry.AccessEntryMigrationOptions) error { +func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.AccessEntryMigrationOptions) error { cfg := cmd.ClusterConfig cmd.ClusterConfig.AccessConfig.AuthenticationMode = ekstypes.AuthenticationMode(options.TargetAuthMode) tgAuthMode := cmd.ClusterConfig.AccessConfig.AuthenticationMode @@ -72,36 +72,67 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentry.AccessEntryMi // Add UpdateAuthentication Mode Method call here } - // Get Access Entries from Cluster Provider - clusterProvider, err := cmd.NewProviderForExistingCluster(ctx) + clientSet, err := ctl.NewStdClientSet(cfg) if err != nil { return err } - asgetter := accessentry.NewGetter(cfg.Metadata.Name, clusterProvider.AWSProvider.EKS()) - accessEntries, err := asgetter.Get(ctx, api.ARN{}) + acm, err := authconfigmap.NewFromClientSet(clientSet) if err != nil { return err } - fmt.Printf("%+v\n", accessEntries) - - // Get CONFIGMAP Entries - clientSet, err := ctl.NewStdClientSet(cfg) + cmEntries, err := acm.GetIdentities() if err != nil { return err } - acm, err := authconfigmap.NewFromClientSet(clientSet) - if err != nil { - return err + accessEntriesFromCm := []api.AccessEntry{} + for _, cme := range cmEntries { + idArn := api.MustParseARN(cme.ARN()) + accessEntriesFromCm = append(accessEntriesFromCm, api.AccessEntry{ + PrincipalARN: idArn, + // Type: "STANDARD", + KubernetesGroups: cme.Groups(), + KubernetesUsername: cme.Username(), + }) } - cmEntries, err := acm.GetIdentities() + + asgetter := accessentryactions.NewGetter(cfg.Metadata.Name, ctl.AWSProvider.EKS()) + accessEntries, err := asgetter.Get(ctx, api.ARN{}) if err != nil { return err } - fmt.Printf("%+v\n", cmEntries) - // Check if any of the cmEntries are in accessEntries, and add the remaining to NeedsUpdateList + //// Compare accessEntries and accessEntriesFromCm + // Next steps: + // 1. Instead of storing all into toDoEntries, Make a struct to contain multiple types of toDoEntries + // type ToDoEntries struct { + // NodeLinux []api.AccessEntry + // NodeWindows []api.AccessEntry + // System []api.AccessEntry + // NonSystem []api.AccessEntry + // } + // 2. Modify below code to store entries into one of above slices based on accessEntriesFromCm[item].KubernetesGroups + // 3. Before storing, add Type property to each entry i.e. STANDARD, EC2_LINUX, EC2_WINDOWS etc based on condition + toDoEntries := []api.AccessEntry{} + aeArns := make(map[string]bool) + for _, ae := range accessEntries { + aeArns[ae.PrincipalARN] = true + } + for _, cme := range accessEntriesFromCm { + if !aeArns[cme.PrincipalARN.String()] { + toDoEntries = append(toDoEntries, cme) + } else { + logger.Warning("%v already exists in Access Entry, ignoring.", cme.PrincipalARN.String()) + } + } - // Perform doCreateAccessEntry() on NeedsUpdateList + // Create Access Entries + stackManager := ctl.NewStackManager(cfg) + fmt.Printf("%+v\n%+v\n", accessEntries, accessEntriesFromCm) + accessEntryCreator := &accessentryactions.Creator{ + ClusterName: cmd.ClusterConfig.Metadata.Name, + StackCreator: stackManager, + } + accessEntryCreator.Create(ctx, toDoEntries) return nil } From 3b82eca0be77ad26545538f78d5be8e39293b8d7 Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Sun, 3 Mar 2024 01:07:06 +0000 Subject: [PATCH 05/22] Added UpdateAuthentication mode and aeEntries filter logic --- pkg/actions/accessentry/migrator.go | 263 ++++++++++++++++++++++- pkg/ctl/utils/migrate_to_access_entry.go | 75 +------ 2 files changed, 271 insertions(+), 67 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index f586d7bb81..8e8d8d6fee 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -2,14 +2,38 @@ package accessentry import ( "context" + "fmt" + "strings" "time" + "github.com/aws/aws-sdk-go-v2/aws" + 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" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" + "github.com/weaveworks/eksctl/pkg/authconfigmap" + "github.com/weaveworks/eksctl/pkg/awsapi" + "github.com/weaveworks/eksctl/pkg/eks/waiter" + "github.com/weaveworks/eksctl/pkg/iam" + "github.com/weaveworks/eksctl/pkg/kubernetes" ) -type AddonCreator interface { - Create(ctx context.Context, addon *api.Addon, waitTimeout time.Duration) error -} +// Venkat TODO +// Compare accessEntries and accessEntriesFromCm +// Next steps: +// 1. Instead of storing all into toDoEntries, Make a struct to contain multiple types of toDoEntries +// type ToDoEntries struct { +// NodeLinux []api.AccessEntry +// NodeWindows []api.AccessEntry +// System []api.AccessEntry +// NonSystem []api.AccessEntry +// } +// 2. Modify below code to store entries into one of above slices based on accessEntriesFromCm[item].KubernetesGroups +// 3. Before storing, add Type property to each entry i.e. STANDARD, EC2_LINUX, EC2_WINDOWS etc based on condition + +// Pankaj TODO +// Add loggic to remove aws-auth type AccessEntryMigrationOptions struct { RemoveOIDCProviderTrustRelationship bool @@ -17,3 +41,236 @@ type AccessEntryMigrationOptions struct { Approve bool Timeout time.Duration } + +type Migrator struct { + clusterName string + eksAPI awsapi.EKS + iamAPI awsapi.IAM + clientSet kubernetes.Interface + aeCreator Creator + curAuthMode ekstypes.AuthenticationMode + tgAuthMode ekstypes.AuthenticationMode +} + +func NewMigrator( + clusterName string, + + eksAPI awsapi.EKS, + iamAPI awsapi.IAM, + clientSet kubernetes.Interface, + aeCreator Creator, + curAuthMode ekstypes.AuthenticationMode, + tgAuthMode ekstypes.AuthenticationMode, +) *Migrator { + return &Migrator{ + clusterName: clusterName, + eksAPI: eksAPI, + iamAPI: iamAPI, + clientSet: clientSet, + aeCreator: aeCreator, + curAuthMode: curAuthMode, + tgAuthMode: tgAuthMode, + } +} + +func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntryMigrationOptions) error { + if m.curAuthMode != m.tgAuthMode { + logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", m.tgAuthMode, m.curAuthMode) + if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { + err := m.doUpdateAuhenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap) + if err != nil { + return err + } + } + m.curAuthMode = ekstypes.AuthenticationModeApiAndConfigMap + } + + cmEntries, err := m.doGetIAMIdentityMappings() + if err != nil { + return err + } + + curAccessEntries, err := m.doGetAccessEntries(ctx) + if err != nil { + return err + } + + newAccessEntries, skipAPImode := m.doFilterAccessEntries(cmEntries, curAccessEntries) + + newaelen := len(newAccessEntries) + + logger.Info("%d new access entries will be created", newaelen) + + if len(newAccessEntries) != 0 { + err = m.aeCreator.Create(ctx, newAccessEntries) + if err != nil { + return err + } + } + + if !skipAPImode { + if m.curAuthMode != m.tgAuthMode { + return m.doUpdateAuhenticationMode(ctx, m.tgAuthMode) + } + } + + return nil + +} + +func (m *Migrator) doUpdateAuhenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode) error { + output, err := m.eksAPI.UpdateClusterConfig(ctx, &awseks.UpdateClusterConfigInput{ + Name: aws.String(m.clusterName), + AccessConfig: &ekstypes.UpdateAccessConfigRequest{ + AuthenticationMode: authMode, + }, + }) + if err != nil { + return fmt.Errorf("failed to update cluster config: %v", err) + } + + updateWaiter := waiter.NewUpdateWaiter(m.eksAPI, func(options *waiter.UpdateWaiterOptions) { + options.RetryAttemptLogMessage = fmt.Sprintf("waiting for update %q in cluster %q to complete", *output.Update.Id, m.clusterName) + }) + err = updateWaiter.Wait(ctx, &awseks.DescribeUpdateInput{ + Name: &m.clusterName, + UpdateId: output.Update.Id, + }, api.DefaultWaitTimeout) + + switch e := err.(type) { + case *waiter.UpdateFailedError: + if e.Status == string(ekstypes.UpdateStatusCancelled) { + return fmt.Errorf("request to update cluster authentication mode was cancelled: %s", e.UpdateError) + } + return fmt.Errorf("failed to update cluster authentication mode: %s", e.UpdateError) + + case nil: + logger.Info("authentication mode was successfully updated to %s on cluster %s", authMode, m.clusterName) + m.curAuthMode = authMode + return nil + + default: + return err + } +} + +func (m *Migrator) doGetAccessEntries(ctx context.Context) ([]Summary, error) { + + aegetter := NewGetter(m.clusterName, m.eksAPI) + accessEntries, err := aegetter.Get(ctx, api.ARN{}) + if err != nil { + return nil, err + } + + return accessEntries, nil +} + +func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { + + acm, err := authconfigmap.NewFromClientSet(m.clientSet) + if err != nil { + return nil, err + } + + cmEntries, err := acm.GetIdentities() + if err != nil { + return nil, err + } + + return cmEntries, nil +} + +func (m *Migrator) doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([]api.AccessEntry, bool) { + + skipAPImode := false + toDoEntries := []api.AccessEntry{} + uniqueCmEntries := map[string]bool{} + + aeArns := make(map[string]bool) + + // Create ARN Map for current access entries + for _, ae := range accessEntries { + aeArns[ae.PrincipalARN] = true + } + + for _, cme := range cmEntries { + if !uniqueCmEntries[cme.ARN()] { // Check if cmEntry is not duplicate + if !aeArns[cme.ARN()] { // Check if the ARN is not in existing access entries + switch cme.Type() { + case iam.ResourceTypeRole: + if aeEntry := doBuildNodeRoleAccessEntry(cme); aeEntry != nil { + toDoEntries = append(toDoEntries, *aeEntry) + } else if aeEntry := doBuildAccessEntry(cme); aeEntry != nil { + toDoEntries = append(toDoEntries, *aeEntry) + } + case iam.ResourceTypeUser: + if aeEntry := doBuildAccessEntry(cme); aeEntry != nil { + toDoEntries = append(toDoEntries, *aeEntry) + } + case iam.ResourceTypeAccount: + logger.Warning("found account mapping %s, can not create access entry for account mapping, skipping", cme.Account()) + skipAPImode = true + } + } else { + logger.Warning("%s already exists in access entry, skipping", cme.ARN()) + } + } + } + + return toDoEntries, skipAPImode +} + +func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { + + groupsStr := strings.Join(cme.Groups(), ",") + + if strings.Contains(groupsStr, "system:nodes") && !strings.Contains(groupsStr, "eks:kube-proxy-windows") { // For Windows Nodes + return &api.AccessEntry{ + PrincipalARN: api.MustParseARN(cme.ARN()), + Type: "EC2_LINUX", + } + } + + if strings.Contains(groupsStr, "system:nodes") && strings.Contains(groupsStr, "eks:kube-proxy-windows") { // For Linux Nodes + return &api.AccessEntry{ + PrincipalARN: api.MustParseARN(cme.ARN()), + Type: "EC2_WINDOWS", + } + } + + return nil +} + +func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { + + groupsStr := strings.Join(cme.Groups(), ",") + + if strings.Contains(groupsStr, "system:masters") { // Admin Role + return &api.AccessEntry{ + PrincipalARN: api.MustParseARN(cme.ARN()), + Type: "STANDARD", + AccessPolicies: []api.AccessPolicy{ + api.AccessPolicy{ + PolicyARN: api.MustParseARN("arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy"), + AccessScope: api.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }, + }, + KubernetesUsername: cme.Username(), + } + } + + if strings.Contains(groupsStr, "system") { // Admin Role + logger.Warning("at least one group name associated with %s starts with \"system\", can not create access entry with such group name, skipping", cme.ARN()) + return nil + } + + return &api.AccessEntry{ + PrincipalARN: api.MustParseARN(cme.ARN()), + Type: "STANDARD", + KubernetesGroups: cme.Groups(), + KubernetesUsername: cme.Username(), + } + +} diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index cb7512344e..d1e55fffac 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -5,12 +5,10 @@ import ( "fmt" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" - "github.com/kris-nova/logger" "github.com/spf13/cobra" "github.com/spf13/pflag" accessentryactions "github.com/weaveworks/eksctl/pkg/actions/accessentry" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" - "github.com/weaveworks/eksctl/pkg/authconfigmap" "github.com/weaveworks/eksctl/pkg/ctl/cmdutils" ) @@ -22,12 +20,9 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { var options accessentryactions.AccessEntryMigrationOptions cmd.FlagSetGroup.InFlagSet("Migrate to Access Entry", func(fs *pflag.FlagSet) { + fs.StringVar(&options.TargetAuthMode, "target-authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") fs.BoolVar(&options.RemoveOIDCProviderTrustRelationship, "remove-aws-auth", false, "Remove aws-auth from cluster") - fs.StringVar(&options.TargetAuthMode, "authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") fs.BoolVar(&options.Approve, "approve", false, "Apply the changes") - - // fs.BoolVar(&options.SkipAgentInstallation, "skip-agent-installation", false, "Skip installing pod-identity-agent addon") - // cmdutils.AddIAMServiceAccountFilterFlags(fs, &cmd.Include, &cmd.Exclude) }) cmd.FlagSetGroup.InFlagSet("General", func(fs *pflag.FlagSet) { @@ -67,72 +62,24 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Access curAuthMode := ctl.GetClusterState().AccessConfig.AuthenticationMode - if curAuthMode != tgAuthMode { - logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", tgAuthMode, curAuthMode) - // Add UpdateAuthentication Mode Method call here - } - clientSet, err := ctl.NewStdClientSet(cfg) if err != nil { return err } - acm, err := authconfigmap.NewFromClientSet(clientSet) - if err != nil { - return err - } - cmEntries, err := acm.GetIdentities() - if err != nil { - return err - } - accessEntriesFromCm := []api.AccessEntry{} - for _, cme := range cmEntries { - idArn := api.MustParseARN(cme.ARN()) - accessEntriesFromCm = append(accessEntriesFromCm, api.AccessEntry{ - PrincipalARN: idArn, - // Type: "STANDARD", - KubernetesGroups: cme.Groups(), - KubernetesUsername: cme.Username(), - }) - } - - asgetter := accessentryactions.NewGetter(cfg.Metadata.Name, ctl.AWSProvider.EKS()) - accessEntries, err := asgetter.Get(ctx, api.ARN{}) - if err != nil { - return err - } - - //// Compare accessEntries and accessEntriesFromCm - // Next steps: - // 1. Instead of storing all into toDoEntries, Make a struct to contain multiple types of toDoEntries - // type ToDoEntries struct { - // NodeLinux []api.AccessEntry - // NodeWindows []api.AccessEntry - // System []api.AccessEntry - // NonSystem []api.AccessEntry - // } - // 2. Modify below code to store entries into one of above slices based on accessEntriesFromCm[item].KubernetesGroups - // 3. Before storing, add Type property to each entry i.e. STANDARD, EC2_LINUX, EC2_WINDOWS etc based on condition - toDoEntries := []api.AccessEntry{} - aeArns := make(map[string]bool) - for _, ae := range accessEntries { - aeArns[ae.PrincipalARN] = true - } - for _, cme := range accessEntriesFromCm { - if !aeArns[cme.PrincipalARN.String()] { - toDoEntries = append(toDoEntries, cme) - } else { - logger.Warning("%v already exists in Access Entry, ignoring.", cme.PrincipalARN.String()) - } - } - // Create Access Entries stackManager := ctl.NewStackManager(cfg) - fmt.Printf("%+v\n%+v\n", accessEntries, accessEntriesFromCm) - accessEntryCreator := &accessentryactions.Creator{ + aeCreator := accessentryactions.Creator{ ClusterName: cmd.ClusterConfig.Metadata.Name, StackCreator: stackManager, } - accessEntryCreator.Create(ctx, toDoEntries) - return nil + return accessentryactions.NewMigrator( + cfg.Metadata.Name, + ctl.AWSProvider.EKS(), + ctl.AWSProvider.IAM(), + clientSet, + aeCreator, + curAuthMode, + tgAuthMode, + ).MigrateToAccessEntry(ctx, options) } From 22ab90828b4b9aec88304d75ca9227f5fc216774 Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Tue, 5 Mar 2024 19:39:51 +0000 Subject: [PATCH 06/22] Add approve flag check --- pkg/ctl/utils/migrate_to_access_entry.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index d1e55fffac..7fbed5b294 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -22,13 +22,13 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { cmd.FlagSetGroup.InFlagSet("Migrate to Access Entry", func(fs *pflag.FlagSet) { fs.StringVar(&options.TargetAuthMode, "target-authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") fs.BoolVar(&options.RemoveOIDCProviderTrustRelationship, "remove-aws-auth", false, "Remove aws-auth from cluster") - fs.BoolVar(&options.Approve, "approve", false, "Apply the changes") }) cmd.FlagSetGroup.InFlagSet("General", func(fs *pflag.FlagSet) { cmdutils.AddClusterFlag(fs, cmd.ClusterConfig.Metadata) cmdutils.AddRegionFlag(fs, &cmd.ProviderConfig) cmdutils.AddTimeoutFlag(fs, &options.Timeout) + cmdutils.AddApproveFlag(fs, cmd) }) cmd.CobraCommand.RunE = func(_ *cobra.Command, args []string) error { @@ -46,6 +46,11 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Access return cmdutils.ErrMustBeSet(cmdutils.ClusterNameFlag(cmd)) } + if cmd.Plan { + cmdutils.LogPlanModeWarning(true) + return nil + } + ctx := context.Background() ctl, err := cmd.NewProviderForExistingCluster(ctx) if err != nil { From 8ec312dfb7d775eead9eb891dd0522984a3896ab Mon Sep 17 00:00:00 2001 From: Venkat Penmetsa Date: Fri, 22 Mar 2024 15:00:48 +0000 Subject: [PATCH 07/22] Added functionality to remove awsauth after switch to API only --- pkg/actions/accessentry/migrator.go | 53 +++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index 8e8d8d6fee..a1d307e7dc 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -17,6 +17,7 @@ import ( "github.com/weaveworks/eksctl/pkg/eks/waiter" "github.com/weaveworks/eksctl/pkg/iam" "github.com/weaveworks/eksctl/pkg/kubernetes" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Venkat TODO @@ -77,7 +78,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry if m.curAuthMode != m.tgAuthMode { logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", m.tgAuthMode, m.curAuthMode) if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { - err := m.doUpdateAuhenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap) + err := m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap) if err != nil { return err } @@ -110,7 +111,20 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry if !skipAPImode { if m.curAuthMode != m.tgAuthMode { - return m.doUpdateAuhenticationMode(ctx, m.tgAuthMode) + err = m.doUpdateAuthenticationMode(ctx, m.tgAuthMode) + if err != nil { + return err + } + } + + err = m.doDeleteIAMIdentityMapping() + if err != nil { + return err + } + + err = doDeleteAWSAuthConfigMap(m.clientSet, "kube-system", "aws-auth") + if err != nil { + return err } } @@ -118,7 +132,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry } -func (m *Migrator) doUpdateAuhenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode) error { +func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode) error { output, err := m.eksAPI.UpdateClusterConfig(ctx, &awseks.UpdateClusterConfigInput{ Name: aws.String(m.clusterName), AccessConfig: &ekstypes.UpdateAccessConfigRequest{ @@ -274,3 +288,36 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { } } + +func (m Migrator) doDeleteIAMIdentityMapping() error { + acm, err := authconfigmap.NewFromClientSet(m.clientSet) + if err != nil { + return err + } + + cmEntries, err := acm.GetIdentities() + if err != nil { + return err + } + + for _, cmEntry := range cmEntries { + arn := cmEntry.ARN() + if err := acm.RemoveIdentity(arn, true); err != nil { + return err + } + } + if err := acm.Save(); err != nil { + return err + } + + return nil +} + +func doDeleteAWSAuthConfigMap(clientset kubernetes.Interface, namespace string, name string) error { + logger.Info("Deleting %q ConfigMap as it is no longer needed in API mode", name) + err := clientset.CoreV1().ConfigMaps(namespace).Delete(context.Background(), name, metav1.DeleteOptions{}) + if err != nil { + return err + } + return nil +} From 6872eb56b493ab7f0960daea3a9b3bdc935b2542 Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Fri, 12 Apr 2024 02:51:40 +0000 Subject: [PATCH 08/22] Adds logic to fetch FullARN of path stripped IAMIdentityMappings --- pkg/actions/accessentry/migrator.go | 104 +++++++++++++++++++---- pkg/ctl/utils/migrate_to_access_entry.go | 1 - 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index a1d307e7dc..988c1679de 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -3,12 +3,14 @@ package accessentry import ( "context" "fmt" + "regexp" "strings" "time" "github.com/aws/aws-sdk-go-v2/aws" awseks "github.com/aws/aws-sdk-go-v2/service/eks" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + awsiam "github.com/aws/aws-sdk-go-v2/service/iam" "github.com/kris-nova/logger" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" @@ -37,10 +39,9 @@ import ( // Add loggic to remove aws-auth type AccessEntryMigrationOptions struct { - RemoveOIDCProviderTrustRelationship bool - TargetAuthMode string - Approve bool - Timeout time.Duration + TargetAuthMode string + Approve bool + Timeout time.Duration } type Migrator struct { @@ -55,7 +56,6 @@ type Migrator struct { func NewMigrator( clusterName string, - eksAPI awsapi.EKS, iamAPI awsapi.IAM, clientSet kubernetes.Interface, @@ -84,6 +84,8 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry } } m.curAuthMode = ekstypes.AuthenticationModeApiAndConfigMap + } else { + logger.Info("target authentication mode %v is same as current authentication mode %v, not updating the Cluster authentication mode", m.tgAuthMode, m.curAuthMode) } cmEntries, err := m.doGetIAMIdentityMappings() @@ -96,7 +98,11 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry return err } - newAccessEntries, skipAPImode := m.doFilterAccessEntries(cmEntries, curAccessEntries) + newAccessEntries, skipAPImode, err := doFilterAccessEntries(cmEntries, curAccessEntries) + + if err != nil { + return err + } newaelen := len(newAccessEntries) @@ -115,16 +121,16 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry if err != nil { return err } - } - err = m.doDeleteIAMIdentityMapping() - if err != nil { - return err - } + err = m.doDeleteIAMIdentityMapping() + if err != nil { + return err + } - err = doDeleteAWSAuthConfigMap(m.clientSet, "kube-system", "aws-auth") - if err != nil { - return err + err = doDeleteAWSAuthConfigMap(m.clientSet, "kube-system", "aws-auth") + if err != nil { + return err + } } } @@ -181,6 +187,8 @@ func (m *Migrator) doGetAccessEntries(ctx context.Context) ([]Summary, error) { func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { + nameRegex := regexp.MustCompile(`[^/]+$`) + acm, err := authconfigmap.NewFromClientSet(m.clientSet) if err != nil { return nil, err @@ -191,10 +199,55 @@ func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { return nil, err } + for idx, cme := range cmEntries { + switch cme.Type() { + case iam.ResourceTypeRole: + roleCme := iam.RoleIdentity{ + RoleARN: cme.ARN(), + KubernetesIdentity: iam.KubernetesIdentity{ + KubernetesUsername: cme.Username(), + KubernetesGroups: cme.Groups(), + }, + } + + if match := nameRegex.FindStringSubmatch(roleCme.RoleARN); match != nil { + getRoleOutput, err := m.iamAPI.GetRole(context.Background(), &awsiam.GetRoleInput{RoleName: &match[0]}) + if err != nil { + return nil, err + } + + roleCme.RoleARN = *getRoleOutput.Role.Arn + } + + cmEntries[idx] = iam.Identity(roleCme) + + case iam.ResourceTypeUser: + userCme := iam.UserIdentity{ + UserARN: cme.ARN(), + KubernetesIdentity: iam.KubernetesIdentity{ + KubernetesUsername: cme.Username(), + KubernetesGroups: cme.Groups(), + }, + } + + if match := nameRegex.FindStringSubmatch(userCme.UserARN); match != nil { + getUserOutput, err := m.iamAPI.GetUser(context.Background(), &awsiam.GetUserInput{UserName: &match[0]}) + if err != nil { + return nil, err + } + + userCme.UserARN = *getUserOutput.User.Arn + } + + cmEntries[idx] = iam.Identity(userCme) + + } + } + return cmEntries, nil } -func (m *Migrator) doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([]api.AccessEntry, bool) { +func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([]api.AccessEntry, bool, error) { skipAPImode := false toDoEntries := []api.AccessEntry{} @@ -231,7 +284,7 @@ func (m *Migrator) doFilterAccessEntries(cmEntries []iam.Identity, accessEntries } } - return toDoEntries, skipAPImode + return toDoEntries, skipAPImode, nil } func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { @@ -255,6 +308,22 @@ func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { return nil } +// func (m *Migrator) doGetRoleARN(roleCme *iam.RoleIdentity) error { +// nameRegex := regexp.MustCompile(`[^/]+$`) + +// if match := nameRegex.FindStringSubmatch(roleCme.RoleARN); match != nil { +// getRoleOutput, err := m.iamAPI.GetRole(context.Background(), &awsiam.GetRoleInput{RoleName: &match[0]}) +// if err != nil { +// return err +// } + +// roleCme.RoleARN = *getRoleOutput.Role.Arn +// return nil +// } + +// return fmt.Errorf("Could not fetch full ARN for role %s", roleCme.RoleARN) +// } + func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { groupsStr := strings.Join(cme.Groups(), ",") @@ -289,6 +358,9 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { } +func doGetFullARN() { + +} func (m Migrator) doDeleteIAMIdentityMapping() error { acm, err := authconfigmap.NewFromClientSet(m.clientSet) if err != nil { diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index 7fbed5b294..a7167b81ec 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -21,7 +21,6 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { var options accessentryactions.AccessEntryMigrationOptions cmd.FlagSetGroup.InFlagSet("Migrate to Access Entry", func(fs *pflag.FlagSet) { fs.StringVar(&options.TargetAuthMode, "target-authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") - fs.BoolVar(&options.RemoveOIDCProviderTrustRelationship, "remove-aws-auth", false, "Remove aws-auth from cluster") }) cmd.FlagSetGroup.InFlagSet("General", func(fs *pflag.FlagSet) { From 5e7203a7d659abfdab87f98ad2ba3d66b3d8426e Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Fri, 12 Apr 2024 02:58:53 +0000 Subject: [PATCH 09/22] Updates some info log text --- pkg/actions/accessentry/migrator.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index 988c1679de..a82f533005 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -76,8 +76,8 @@ func NewMigrator( func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntryMigrationOptions) error { if m.curAuthMode != m.tgAuthMode { - logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the Cluster authentication mode", m.tgAuthMode, m.curAuthMode) if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { + logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) err := m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap) if err != nil { return err @@ -85,7 +85,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry } m.curAuthMode = ekstypes.AuthenticationModeApiAndConfigMap } else { - logger.Info("target authentication mode %v is same as current authentication mode %v, not updating the Cluster authentication mode", m.tgAuthMode, m.curAuthMode) + logger.Info("target authentication mode %v is same as current authentication mode %v, not updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) } cmEntries, err := m.doGetIAMIdentityMappings() @@ -117,6 +117,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry if !skipAPImode { if m.curAuthMode != m.tgAuthMode { + logger.Info("target authentication mode %v is different than the current authentication mode %v, updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) err = m.doUpdateAuthenticationMode(ctx, m.tgAuthMode) if err != nil { return err From a6850fe2c139d69f33a2c8b9ff0d43122302b631 Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Sat, 13 Apr 2024 01:17:30 +0000 Subject: [PATCH 10/22] Adds test case and refactors code --- pkg/actions/accessentry/migrator.go | 37 +---- pkg/actions/accessentry/migrator_test.go | 139 ++++++++++++++++++ .../v1alpha5/zz_generated.defaults.go | 17 +++ pkg/ctl/utils/migrate_to_access_entry.go | 4 +- 4 files changed, 165 insertions(+), 32 deletions(-) create mode 100644 pkg/actions/accessentry/migrator_test.go create mode 100644 pkg/apis/eksctl.io/v1alpha5/zz_generated.defaults.go diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index a82f533005..d56acf27a9 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -38,7 +38,7 @@ import ( // Pankaj TODO // Add loggic to remove aws-auth -type AccessEntryMigrationOptions struct { +type MigrationOptions struct { TargetAuthMode string Approve bool Timeout time.Duration @@ -74,11 +74,11 @@ func NewMigrator( } } -func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntryMigrationOptions) error { +func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOptions) error { if m.curAuthMode != m.tgAuthMode { if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) - err := m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap) + err := m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap, options.Timeout) if err != nil { return err } @@ -118,7 +118,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry if !skipAPImode { if m.curAuthMode != m.tgAuthMode { logger.Info("target authentication mode %v is different than the current authentication mode %v, updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) - err = m.doUpdateAuthenticationMode(ctx, m.tgAuthMode) + err = m.doUpdateAuthenticationMode(ctx, m.tgAuthMode, options.Timeout) if err != nil { return err } @@ -139,7 +139,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options AccessEntry } -func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode) error { +func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode, timeout time.Duration) error { output, err := m.eksAPI.UpdateClusterConfig(ctx, &awseks.UpdateClusterConfigInput{ Name: aws.String(m.clusterName), AccessConfig: &ekstypes.UpdateAccessConfigRequest{ @@ -156,7 +156,7 @@ func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekst err = updateWaiter.Wait(ctx, &awseks.DescribeUpdateInput{ Name: &m.clusterName, UpdateId: output.Update.Id, - }, api.DefaultWaitTimeout) + }, timeout) switch e := err.(type) { case *waiter.UpdateFailedError: @@ -309,22 +309,6 @@ func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { return nil } -// func (m *Migrator) doGetRoleARN(roleCme *iam.RoleIdentity) error { -// nameRegex := regexp.MustCompile(`[^/]+$`) - -// if match := nameRegex.FindStringSubmatch(roleCme.RoleARN); match != nil { -// getRoleOutput, err := m.iamAPI.GetRole(context.Background(), &awsiam.GetRoleInput{RoleName: &match[0]}) -// if err != nil { -// return err -// } - -// roleCme.RoleARN = *getRoleOutput.Role.Arn -// return nil -// } - -// return fmt.Errorf("Could not fetch full ARN for role %s", roleCme.RoleARN) -// } - func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { groupsStr := strings.Join(cme.Groups(), ",") @@ -359,9 +343,6 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { } -func doGetFullARN() { - -} func (m Migrator) doDeleteIAMIdentityMapping() error { acm, err := authconfigmap.NewFromClientSet(m.clientSet) if err != nil { @@ -379,11 +360,7 @@ func (m Migrator) doDeleteIAMIdentityMapping() error { return err } } - if err := acm.Save(); err != nil { - return err - } - - return nil + return acm.Save() } func doDeleteAWSAuthConfigMap(clientset kubernetes.Interface, namespace string, name string) error { diff --git a/pkg/actions/accessentry/migrator_test.go b/pkg/actions/accessentry/migrator_test.go new file mode 100644 index 0000000000..b264016189 --- /dev/null +++ b/pkg/actions/accessentry/migrator_test.go @@ -0,0 +1,139 @@ +package accessentry_test + +import ( + "bytes" + "context" + "encoding/base32" + "fmt" + "os" + "strings" + + 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" + "github.com/weaveworks/eksctl/pkg/actions/accessentry" + "github.com/weaveworks/eksctl/pkg/actions/accessentry/fakes" + "github.com/weaveworks/eksctl/pkg/cfn/builder" + "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" + "k8s.io/client-go/kubernetes/fake" +) + +type migrateToAccessEntryEntry struct { + clusterName string + mockEKS func(provider *mockprovider.MockProvider) + // mockIAM func(provider *mockprovider.MockProvider) + mockK8s func(clientSet *fake.Clientset) + validateCustomLoggerOutput func(output string) + options accessentry.MigrationOptions + expectedErr string +} + +var _ = Describe("Migrate Access Entry", func() { + + var ( + migrator *accessentry.Migrator + mockProvider *mockprovider.MockProvider + fakeClientset *fake.Clientset + + clusterName = "test-cluster" + + // roleIdentity = iam.RoleIdentity{ + // RoleARN: "arn:aws:iam::111122223333:role/test-role-1", + // KubernetesIdentity: iam.KubernetesIdentity{ + // KubernetesUsername: "admin", + // KubernetesGroups: []string{"system:master"}, + // }, + // } + // userIdentity = iam.UserIdentity{ + // UserARN: "arn:aws:iam::111122223333:user/test-user-1", + // KubernetesIdentity: iam.KubernetesIdentity{ + // KubernetesUsername: "admin-user", + // KubernetesGroups: []string{"system:master"}, + // }, + // } + + tgAuthMode = ekstypes.AuthenticationModeApi + curAuthMode = ekstypes.AuthenticationModeConfigMap + // 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) + idx := strings.Index(stackName, prefix) + if idx < 0 { + return fmt.Errorf("expected stack name to have prefix %q", prefix) + } + suffix := stackName[idx+len(prefix):] + _, err := base32.StdEncoding.WithPadding(base32.NoPadding).DecodeString(suffix) + if err != nil { + return fmt.Errorf("expected stack name to have a base32-encoded suffix: %w", err) + } + return nil + } + + accessEntryCreator := &accessentry.Creator{ + ClusterName: ae.clusterName, + StackCreator: &s, + } + + mockProvider = mockprovider.NewMockProvider() + if ae.mockEKS != nil { + ae.mockEKS(mockProvider) + } + + fakeClientset = fake.NewSimpleClientset() + if ae.mockK8s != nil { + ae.mockK8s(fakeClientset) + } + + output := &bytes.Buffer{} + if ae.validateCustomLoggerOutput != nil { + defer func() { + logger.Writer = os.Stdout + }() + logger.Writer = output + } + + migrator = accessentry.NewMigrator( + ae.clusterName, + mockProvider.MockEKS(), + mockProvider.MockIAM(), + fakeClientset, + *accessEntryCreator, + curAuthMode, + tgAuthMode, + ) + + err := migrator.MigrateToAccessEntry(context.Background(), ae.options) + + if ae.expectedErr != "" { + Expect(err).To(MatchError(ContainSubstring(ae.expectedErr))) + return + } + + Expect(err).ToNot(HaveOccurred()) + + if ae.validateCustomLoggerOutput != nil { + ae.validateCustomLoggerOutput(output.String()) + } + }, Entry("[API Error] Authentication mode update fails", migrateToAccessEntryEntry{ + clusterName: clusterName, + 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")) + }, + expectedErr: "failed to update cluster config", + }), + ) +}) diff --git a/pkg/apis/eksctl.io/v1alpha5/zz_generated.defaults.go b/pkg/apis/eksctl.io/v1alpha5/zz_generated.defaults.go new file mode 100644 index 0000000000..059ad74d1d --- /dev/null +++ b/pkg/apis/eksctl.io/v1alpha5/zz_generated.defaults.go @@ -0,0 +1,17 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +// Code generated by defaulter-gen. DO NOT EDIT. + +package v1alpha5 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// RegisterDefaults adds defaulters functions to the given scheme. +// Public to allow building arbitrary schemes. +// All generated defaulters are covering - they call all nested defaulters. +func RegisterDefaults(scheme *runtime.Scheme) error { + return nil +} diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index a7167b81ec..cddb582bcd 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -18,7 +18,7 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { cmd.SetDescription("migrate-to-access-entry", "Migrates aws-auth to API authentication mode for the cluster", "") - var options accessentryactions.AccessEntryMigrationOptions + var options accessentryactions.MigrationOptions cmd.FlagSetGroup.InFlagSet("Migrate to Access Entry", func(fs *pflag.FlagSet) { fs.StringVar(&options.TargetAuthMode, "target-authentication-mode", "API_AND_CONFIG_MAP", "Target Authentication mode of migration") }) @@ -36,7 +36,7 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { } } -func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.AccessEntryMigrationOptions) error { +func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.MigrationOptions) error { cfg := cmd.ClusterConfig cmd.ClusterConfig.AccessConfig.AuthenticationMode = ekstypes.AuthenticationMode(options.TargetAuthMode) tgAuthMode := cmd.ClusterConfig.AccessConfig.AuthenticationMode From e50ac72b71c4daf481981ea168fd3df6c1e4c24b Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Mon, 15 Apr 2024 01:46:42 +0000 Subject: [PATCH 11/22] Removes comments --- pkg/actions/accessentry/migrator.go | 18 +----------------- pkg/actions/accessentry/migrator_test.go | 24 +++--------------------- 2 files changed, 4 insertions(+), 38 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index d56acf27a9..623aecf4e3 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -22,22 +22,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// Venkat TODO -// Compare accessEntries and accessEntriesFromCm -// Next steps: -// 1. Instead of storing all into toDoEntries, Make a struct to contain multiple types of toDoEntries -// type ToDoEntries struct { -// NodeLinux []api.AccessEntry -// NodeWindows []api.AccessEntry -// System []api.AccessEntry -// NonSystem []api.AccessEntry -// } -// 2. Modify below code to store entries into one of above slices based on accessEntriesFromCm[item].KubernetesGroups -// 3. Before storing, add Type property to each entry i.e. STANDARD, EC2_LINUX, EC2_WINDOWS etc based on condition - -// Pankaj TODO -// Add loggic to remove aws-auth - type MigrationOptions struct { TargetAuthMode string Approve bool @@ -318,7 +302,7 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { PrincipalARN: api.MustParseARN(cme.ARN()), Type: "STANDARD", AccessPolicies: []api.AccessPolicy{ - api.AccessPolicy{ + { PolicyARN: api.MustParseARN("arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy"), AccessScope: api.AccessScope{ Type: ekstypes.AccessScopeTypeCluster, diff --git a/pkg/actions/accessentry/migrator_test.go b/pkg/actions/accessentry/migrator_test.go index b264016189..ee81ede687 100644 --- a/pkg/actions/accessentry/migrator_test.go +++ b/pkg/actions/accessentry/migrator_test.go @@ -37,27 +37,9 @@ var _ = Describe("Migrate Access Entry", func() { migrator *accessentry.Migrator mockProvider *mockprovider.MockProvider fakeClientset *fake.Clientset - - clusterName = "test-cluster" - - // roleIdentity = iam.RoleIdentity{ - // RoleARN: "arn:aws:iam::111122223333:role/test-role-1", - // KubernetesIdentity: iam.KubernetesIdentity{ - // KubernetesUsername: "admin", - // KubernetesGroups: []string{"system:master"}, - // }, - // } - // userIdentity = iam.UserIdentity{ - // UserARN: "arn:aws:iam::111122223333:user/test-user-1", - // KubernetesIdentity: iam.KubernetesIdentity{ - // KubernetesUsername: "admin-user", - // KubernetesGroups: []string{"system:master"}, - // }, - // } - - tgAuthMode = ekstypes.AuthenticationModeApi - curAuthMode = ekstypes.AuthenticationModeConfigMap - // genericErr = fmt.Errorf("ERR") + clusterName = "test-cluster" + tgAuthMode = ekstypes.AuthenticationModeApi + curAuthMode = ekstypes.AuthenticationModeConfigMap ) DescribeTable("Migrate", func(ae migrateToAccessEntryEntry) { From 3bd5927808b03d4da25bf25e6664f6cb279fde6d Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Thu, 18 Apr 2024 00:41:31 +0000 Subject: [PATCH 12/22] Adds taskTree and address PR comments --- pkg/actions/accessentry/migrator.go | 138 +++++++++-------------- pkg/actions/accessentry/task.go | 20 ++++ pkg/ctl/utils/migrate_to_access_entry.go | 14 +-- 3 files changed, 81 insertions(+), 91 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index 623aecf4e3..2a8b1f750a 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -12,6 +12,7 @@ import ( ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" awsiam "github.com/aws/aws-sdk-go-v2/service/iam" "github.com/kris-nova/logger" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/authconfigmap" @@ -19,7 +20,7 @@ import ( "github.com/weaveworks/eksctl/pkg/eks/waiter" "github.com/weaveworks/eksctl/pkg/iam" "github.com/weaveworks/eksctl/pkg/kubernetes" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/weaveworks/eksctl/pkg/utils/tasks" ) type MigrationOptions struct { @@ -59,68 +60,71 @@ func NewMigrator( } func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOptions) error { + taskTree := tasks.TaskTree{ + Parallel: false, + PlanMode: !options.Approve, + } + if m.curAuthMode != m.tgAuthMode { - if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { - logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) - err := m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap, options.Timeout) - if err != nil { - return err - } - } - m.curAuthMode = ekstypes.AuthenticationModeApiAndConfigMap + taskTree.Append(&tasks.GenericTask{ + Description: fmt.Sprintf("update authentication mode to %v", ekstypes.AuthenticationModeApiAndConfigMap), + Doer: func() error { + if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { + logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) + return m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap, options.Timeout) + } + m.curAuthMode = ekstypes.AuthenticationModeApiAndConfigMap + return nil + }, + }) } else { logger.Info("target authentication mode %v is same as current authentication mode %v, not updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) } - cmEntries, err := m.doGetIAMIdentityMappings() + cmEntries, err := m.doGetIAMIdentityMappings(ctx) if err != nil { return err } curAccessEntries, err := m.doGetAccessEntries(ctx) - if err != nil { + if err != nil && m.curAuthMode != ekstypes.AuthenticationModeConfigMap { return err } newAccessEntries, skipAPImode, err := doFilterAccessEntries(cmEntries, curAccessEntries) - if err != nil { return err } - newaelen := len(newAccessEntries) - - logger.Info("%d new access entries will be created", newaelen) - - if len(newAccessEntries) != 0 { - err = m.aeCreator.Create(ctx, newAccessEntries) - if err != nil { - return err - } + if newaelen := len(newAccessEntries); newaelen != 0 { + logger.Info("%d new access entries will be created", newaelen) + aeTasks := m.aeCreator.CreateTasks(ctx, newAccessEntries) + aeTasks.IsSubTask = true + taskTree.Append(aeTasks) } if !skipAPImode { - if m.curAuthMode != m.tgAuthMode { - logger.Info("target authentication mode %v is different than the current authentication mode %v, updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) - err = m.doUpdateAuthenticationMode(ctx, m.tgAuthMode, options.Timeout) - if err != nil { - return err - } - - err = m.doDeleteIAMIdentityMapping() - if err != nil { - return err - } + if m.tgAuthMode == ekstypes.AuthenticationModeApi { + taskTree.Append(&tasks.GenericTask{ + Description: fmt.Sprintf("update authentication mode to %v", ekstypes.AuthenticationModeApi), + Doer: func() error { + logger.Info("target authentication mode %v is different than the current authentication mode %v, updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) + return m.doUpdateAuthenticationMode(ctx, m.tgAuthMode, options.Timeout) + }, + }) - err = doDeleteAWSAuthConfigMap(m.clientSet, "kube-system", "aws-auth") - if err != nil { - return err - } + taskTree.Append(&tasks.GenericTask{ + Description: fmt.Sprintf("delete aws-auth configMap when authentication mode is %v", ekstypes.AuthenticationModeApi), + Doer: func() error { + return doDeleteAWSAuthConfigMap(ctx, m.clientSet, authconfigmap.ObjectNamespace, authconfigmap.ObjectName) + }, + }) } + } else if m.tgAuthMode == ekstypes.AuthenticationModeApi { + logger.Warning("one or more identitymapping could not be migrated to access entry, will not update authentication mode to %v", ekstypes.AuthenticationModeApi) } - return nil - + return runAllTasks(&taskTree) } func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode, timeout time.Duration) error { @@ -148,29 +152,21 @@ func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekst return fmt.Errorf("request to update cluster authentication mode was cancelled: %s", e.UpdateError) } return fmt.Errorf("failed to update cluster authentication mode: %s", e.UpdateError) - case nil: logger.Info("authentication mode was successfully updated to %s on cluster %s", authMode, m.clusterName) m.curAuthMode = authMode return nil - default: return err } } func (m *Migrator) doGetAccessEntries(ctx context.Context) ([]Summary, error) { - - aegetter := NewGetter(m.clusterName, m.eksAPI) - accessEntries, err := aegetter.Get(ctx, api.ARN{}) - if err != nil { - return nil, err - } - - return accessEntries, nil + aeGetter := NewGetter(m.clusterName, m.eksAPI) + return aeGetter.Get(ctx, api.ARN{}) } -func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { +func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity, error) { nameRegex := regexp.MustCompile(`[^/]+$`) @@ -196,7 +192,7 @@ func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { } if match := nameRegex.FindStringSubmatch(roleCme.RoleARN); match != nil { - getRoleOutput, err := m.iamAPI.GetRole(context.Background(), &awsiam.GetRoleInput{RoleName: &match[0]}) + getRoleOutput, err := m.iamAPI.GetRole(ctx, &awsiam.GetRoleInput{RoleName: &match[0]}) if err != nil { return nil, err } @@ -216,16 +212,14 @@ func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { } if match := nameRegex.FindStringSubmatch(userCme.UserARN); match != nil { - getUserOutput, err := m.iamAPI.GetUser(context.Background(), &awsiam.GetUserInput{UserName: &match[0]}) + getUserOutput, err := m.iamAPI.GetUser(ctx, &awsiam.GetUserInput{UserName: &match[0]}) if err != nil { return nil, err } userCme.UserARN = *getUserOutput.User.Arn } - cmEntries[idx] = iam.Identity(userCme) - } } @@ -235,10 +229,9 @@ func (m *Migrator) doGetIAMIdentityMappings() ([]iam.Identity, error) { func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([]api.AccessEntry, bool, error) { skipAPImode := false - toDoEntries := []api.AccessEntry{} + var toDoEntries []api.AccessEntry uniqueCmEntries := map[string]bool{} - - aeArns := make(map[string]bool) + aeArns := map[string]bool{} // Create ARN Map for current access entries for _, ae := range accessEntries { @@ -254,10 +247,14 @@ func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([ toDoEntries = append(toDoEntries, *aeEntry) } else if aeEntry := doBuildAccessEntry(cme); aeEntry != nil { toDoEntries = append(toDoEntries, *aeEntry) + } else { + skipAPImode = true } case iam.ResourceTypeUser: if aeEntry := doBuildAccessEntry(cme); aeEntry != nil { toDoEntries = append(toDoEntries, *aeEntry) + } else { + skipAPImode = true } case iam.ResourceTypeAccount: logger.Warning("found account mapping %s, can not create access entry for account mapping, skipping", cme.Account()) @@ -327,31 +324,8 @@ func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { } -func (m Migrator) doDeleteIAMIdentityMapping() error { - acm, err := authconfigmap.NewFromClientSet(m.clientSet) - if err != nil { - return err - } - - cmEntries, err := acm.GetIdentities() - if err != nil { - return err - } - - for _, cmEntry := range cmEntries { - arn := cmEntry.ARN() - if err := acm.RemoveIdentity(arn, true); err != nil { - return err - } - } - return acm.Save() -} +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{}) -func doDeleteAWSAuthConfigMap(clientset kubernetes.Interface, namespace string, name string) error { - logger.Info("Deleting %q ConfigMap as it is no longer needed in API mode", name) - err := clientset.CoreV1().ConfigMaps(namespace).Delete(context.Background(), name, metav1.DeleteOptions{}) - if err != nil { - return err - } - return nil } diff --git a/pkg/actions/accessentry/task.go b/pkg/actions/accessentry/task.go index 29596f3993..7860c0faf9 100644 --- a/pkg/actions/accessentry/task.go +++ b/pkg/actions/accessentry/task.go @@ -14,6 +14,7 @@ import ( api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/awsapi" "github.com/weaveworks/eksctl/pkg/cfn/builder" + "github.com/weaveworks/eksctl/pkg/utils/tasks" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate @@ -127,3 +128,22 @@ func (t *deleteOwnedAccessEntryTask) Do(errorCh chan error) error { return nil } + +func runAllTasks(taskTree *tasks.TaskTree) error { + logger.Info(taskTree.Describe()) + if errs := taskTree.DoAllSync(); len(errs) > 0 { + var allErrs []string + for _, err := range errs { + allErrs = append(allErrs, err.Error()) + } + return fmt.Errorf(strings.Join(allErrs, "\n")) + } + completedAction := func() string { + if taskTree.PlanMode { + return "skipped" + } + return "completed successfully" + } + logger.Info("all tasks were %s", completedAction()) + return nil +} diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index cddb582bcd..31c00237b7 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -37,17 +37,17 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { } func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.MigrationOptions) error { + defer cmdutils.LogPlanModeWarning(options.Approve) + options.Approve = !cmd.Plan cfg := cmd.ClusterConfig - cmd.ClusterConfig.AccessConfig.AuthenticationMode = ekstypes.AuthenticationMode(options.TargetAuthMode) - tgAuthMode := cmd.ClusterConfig.AccessConfig.AuthenticationMode + tgAuthMode := ekstypes.AuthenticationMode(options.TargetAuthMode) if cfg.Metadata.Name == "" { return cmdutils.ErrMustBeSet(cmdutils.ClusterNameFlag(cmd)) } - if cmd.Plan { - cmdutils.LogPlanModeWarning(true) - return nil + if tgAuthMode != ekstypes.AuthenticationModeApi && tgAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { + return fmt.Errorf("target authentication mode is invalid, must be either %s or %s", ekstypes.AuthenticationModeApi, ekstypes.AuthenticationModeApiAndConfigMap) } ctx := context.Background() @@ -60,10 +60,6 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Migrat return err } - if tgAuthMode != ekstypes.AuthenticationModeApi && tgAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { - return fmt.Errorf("target authentication mode is invalid") - } - curAuthMode := ctl.GetClusterState().AccessConfig.AuthenticationMode clientSet, err := ctl.NewStdClientSet(cfg) From e9ca916349cd91d0635cfabf6ecf9f0feedbe83c Mon Sep 17 00:00:00 2001 From: punkwalker Date: Thu, 18 Apr 2024 21:18:32 +0000 Subject: [PATCH 13/22] Refactors code and Adds exception handling for NoSuchEntityException --- pkg/actions/accessentry/migrator.go | 112 +++++++++++++---------- pkg/actions/accessentry/migrator_test.go | 5 +- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index 2a8b1f750a..f00811aca9 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -2,8 +2,8 @@ package accessentry import ( "context" + "errors" "fmt" - "regexp" "strings" "time" @@ -11,6 +11,7 @@ import ( awseks "github.com/aws/aws-sdk-go-v2/service/eks" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" awsiam "github.com/aws/aws-sdk-go-v2/service/iam" + "github.com/aws/aws-sdk-go-v2/service/iam/types" "github.com/kris-nova/logger" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -121,7 +122,7 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOp }) } } else if m.tgAuthMode == ekstypes.AuthenticationModeApi { - logger.Warning("one or more identitymapping could not be migrated to access entry, will not update authentication mode to %v", ekstypes.AuthenticationModeApi) + logger.Warning("one or more iamidentitymapping could not be migrated to access entry, will not update authentication mode to %v", ekstypes.AuthenticationModeApi) } return runAllTasks(&taskTree) @@ -167,9 +168,6 @@ func (m *Migrator) doGetAccessEntries(ctx context.Context) ([]Summary, error) { } func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity, error) { - - nameRegex := regexp.MustCompile(`[^/]+$`) - acm, err := authconfigmap.NewFromClientSet(m.clientSet) if err != nil { return nil, err @@ -181,6 +179,10 @@ func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity } for idx, cme := range cmEntries { + lastIdx := strings.LastIndex(cme.ARN(), "/") + cmeName := cme.ARN()[lastIdx+1:] + var noSuchEntity *types.NoSuchEntityException + switch cme.Type() { case iam.ResourceTypeRole: roleCme := iam.RoleIdentity{ @@ -191,15 +193,16 @@ func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity }, } - if match := nameRegex.FindStringSubmatch(roleCme.RoleARN); match != nil { - getRoleOutput, err := m.iamAPI.GetRole(ctx, &awsiam.GetRoleInput{RoleName: &match[0]}) + if cmeName != "" { + 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, err } - roleCme.RoleARN = *getRoleOutput.Role.Arn } - cmEntries[idx] = iam.Identity(roleCme) case iam.ResourceTypeUser: @@ -211,12 +214,14 @@ func (m *Migrator) doGetIAMIdentityMappings(ctx context.Context) ([]iam.Identity }, } - if match := nameRegex.FindStringSubmatch(userCme.UserARN); match != nil { - getUserOutput, err := m.iamAPI.GetUser(ctx, &awsiam.GetUserInput{UserName: &match[0]}) + if cmeName != "" { + 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, err } - userCme.UserARN = *getUserOutput.User.Arn } cmEntries[idx] = iam.Identity(userCme) @@ -230,20 +235,26 @@ func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([ skipAPImode := false var toDoEntries []api.AccessEntry - uniqueCmEntries := map[string]bool{} - aeArns := map[string]bool{} + uniqueCmEntries := map[string]struct{}{} + aeArns := map[string]struct{}{} - // Create ARN Map for current access entries + // Create map for current access entry principal ARN for _, ae := range accessEntries { - aeArns[ae.PrincipalARN] = true + aeArns[ae.PrincipalARN] = struct{}{} } for _, cme := range cmEntries { - if !uniqueCmEntries[cme.ARN()] { // Check if cmEntry is not duplicate - if !aeArns[cme.ARN()] { // Check if the ARN is not in existing access entries + if _, ok := uniqueCmEntries[cme.ARN()]; !ok { // Check if cmEntry is not duplicate + uniqueCmEntries[cme.ARN()] = struct{}{} // Add ARN to cmEntries map + + if _, ok := aeArns[cme.ARN()]; !ok { // Check if the principal ARN is not present in existing access entries switch cme.Type() { case iam.ResourceTypeRole: - if aeEntry := doBuildNodeRoleAccessEntry(cme); aeEntry != nil { + if strings.Contains(cme.ARN(), ":role/aws-service-role/") { // Check if the principal ARN is service-linked-role + logger.Warning("found service-linked role iamidentitymapping \"%s\", can not create access entry, skipping", cme.ARN()) + skipAPImode = true + } else if cme.Username() == authconfigmap.RoleNodeGroupUsername { + aeEntry := doBuildNodeRoleAccessEntry(cme) toDoEntries = append(toDoEntries, *aeEntry) } else if aeEntry := doBuildAccessEntry(cme); aeEntry != nil { toDoEntries = append(toDoEntries, *aeEntry) @@ -257,7 +268,7 @@ func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([ skipAPImode = true } case iam.ResourceTypeAccount: - logger.Warning("found account mapping %s, can not create access entry for account mapping, skipping", cme.Account()) + logger.Warning("found account iamidentitymapping \"%s\", can not create access entry", cme.Account()) skipAPImode = true } } else { @@ -270,48 +281,53 @@ func doFilterAccessEntries(cmEntries []iam.Identity, accessEntries []Summary) ([ } func doBuildNodeRoleAccessEntry(cme iam.Identity) *api.AccessEntry { + isLinux := true - groupsStr := strings.Join(cme.Groups(), ",") - - if strings.Contains(groupsStr, "system:nodes") && !strings.Contains(groupsStr, "eks:kube-proxy-windows") { // For Windows Nodes - return &api.AccessEntry{ - PrincipalARN: api.MustParseARN(cme.ARN()), - Type: "EC2_LINUX", + for _, group := range cme.Groups() { + if group == "eks:kube-proxy-windows" { + isLinux = false } } - - if strings.Contains(groupsStr, "system:nodes") && strings.Contains(groupsStr, "eks:kube-proxy-windows") { // For Linux Nodes + // For Linux Nodes + if isLinux { return &api.AccessEntry{ PrincipalARN: api.MustParseARN(cme.ARN()), - Type: "EC2_WINDOWS", + Type: "EC2_LINUX", } } - - return nil + // For windows Nodes + return &api.AccessEntry{ + PrincipalARN: api.MustParseARN(cme.ARN()), + Type: "EC2_WINDOWS", + } } func doBuildAccessEntry(cme iam.Identity) *api.AccessEntry { - - groupsStr := strings.Join(cme.Groups(), ",") - - if strings.Contains(groupsStr, "system:masters") { // Admin Role - return &api.AccessEntry{ - PrincipalARN: api.MustParseARN(cme.ARN()), - Type: "STANDARD", - AccessPolicies: []api.AccessPolicy{ - { - PolicyARN: api.MustParseARN("arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy"), - AccessScope: api.AccessScope{ - Type: ekstypes.AccessScopeTypeCluster, + containsSys := false + + for _, group := range cme.Groups() { + if strings.HasPrefix(group, "system:") { + containsSys = true + if group == "system:masters" { // Cluster Admin Role + return &api.AccessEntry{ + PrincipalARN: api.MustParseARN(cme.ARN()), + Type: "STANDARD", + AccessPolicies: []api.AccessPolicy{ + { + PolicyARN: api.MustParseARN("arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy"), + AccessScope: api.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }, }, - }, - }, - KubernetesUsername: cme.Username(), + KubernetesUsername: cme.Username(), + } + } } } - if strings.Contains(groupsStr, "system") { // Admin Role - logger.Warning("at least one group name associated with %s starts with \"system\", can not create access entry with such group name, skipping", cme.ARN()) + 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()) return nil } diff --git a/pkg/actions/accessentry/migrator_test.go b/pkg/actions/accessentry/migrator_test.go index ee81ede687..cc3dfa0c7d 100644 --- a/pkg/actions/accessentry/migrator_test.go +++ b/pkg/actions/accessentry/migrator_test.go @@ -22,9 +22,8 @@ import ( ) type migrateToAccessEntryEntry struct { - clusterName string - mockEKS func(provider *mockprovider.MockProvider) - // mockIAM func(provider *mockprovider.MockProvider) + clusterName string + mockEKS func(provider *mockprovider.MockProvider) mockK8s func(clientSet *fake.Clientset) validateCustomLoggerOutput func(output string) options accessentry.MigrationOptions From 7b58656593174409b266dc396ca91a2c02742da9 Mon Sep 17 00:00:00 2001 From: punkwalker Date: Thu, 18 Apr 2024 21:38:12 +0000 Subject: [PATCH 14/22] Resolves go.mod and go.sum conflicts --- go.mod | 2 +- go.sum | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 9aa0835498..b324bb66f3 100644 --- a/go.mod +++ b/go.mod @@ -464,4 +464,4 @@ replace ( ) // Ensure k8s dependencies are also pinned accordingly -replace github.com/acomagu/bufpipe => github.com/acomagu/bufpipe v1.0.4 +replace github.com/acomagu/bufpipe => github.com/acomagu/bufpipe v1.0.4 \ No newline at end of file diff --git a/go.sum b/go.sum index 20cb1a6233..f3489138bf 100644 --- a/go.sum +++ b/go.sum @@ -2803,4 +2803,4 @@ sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77Vzej sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= -sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= +sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= \ No newline at end of file From 3dce3483f85b8245f29e18a24500f61e2d5f26b3 Mon Sep 17 00:00:00 2001 From: Venkat Penmetsa Date: Fri, 19 Apr 2024 16:19:28 +0000 Subject: [PATCH 15/22] Doc update for migrate-to-access-entry feature --- userdocs/src/usage/access-entries.md | 19 +++++++++++++++++++ userdocs/src/usage/minimum-iam-policies.md | 1 + 2 files changed, 20 insertions(+) diff --git a/userdocs/src/usage/access-entries.md b/userdocs/src/usage/access-entries.md index efac8fab29..98e8ae7ef2 100644 --- a/userdocs/src/usage/access-entries.md +++ b/userdocs/src/usage/access-entries.md @@ -148,6 +148,25 @@ accessEntry: eksctl delete accessentry -f config.yaml ``` +### Migrate IAM identity mappings to access entries + +The user can migrate their IAM identities from configmap to access entries by running the following: + +```shell +eksctl utils migrate-to-access-entry --cluster my-cluster --target-authentication-mode +``` + +When `--target-authentication-mode API` is used, authentication mode is switched to `API` mode (skipped if already in `API` mode), IAM identity mappings will be migrated to access entries, and `aws-auth` configmap is deleted from the cluster. + +When `--target-authentication-mode API_AND_CONFIG_MAP` is used, authentication mode is switched to `API_AND_CONFIG_MAP` mode (skipped if already in `API_AND_CONFIG_MAP` mode), IAM identity mappings will be migrated to access entries. + +???+ note + When `--target-authentication-mode API` is used, this command will not update Authentication mode to `API` mode if `aws-auth` configmap has one of the below constraints. + + * There is an Account level IAM identity mapping. + * One or more Roles/Users are mapped to the kubernetes group(s) which begin with prefix `system:` (except for `system:masters`) + * One or more IAM identity mapping(s) are for a [Service Linked Role](https://docs.aws.amazon.com/IAM/latest/UserGuide/using-service-linked-roles.html) + ## Disabling cluster creator admin permissions `eksctl` has added a new field `accessConfig.bootstrapClusterCreatorAdminPermissions: boolean` that, when set to false, disables granting cluster-admin permissions to the IAM identity creating the cluster. i.e. diff --git a/userdocs/src/usage/minimum-iam-policies.md b/userdocs/src/usage/minimum-iam-policies.md index 7b8dd35ec1..9b001ecf82 100644 --- a/userdocs/src/usage/minimum-iam-policies.md +++ b/userdocs/src/usage/minimum-iam-policies.md @@ -123,6 +123,7 @@ IamLimitedAccess "iam:DeleteInstanceProfile", "iam:GetInstanceProfile", "iam:RemoveRoleFromInstanceProfile", + "iam:GetUser", "iam:GetRole", "iam:CreateRole", "iam:DeleteRole", From 7b7b60efcac75bfa3d3ebcd86b675b6c32272f8f Mon Sep 17 00:00:00 2001 From: Venkat Penmetsa Date: Mon, 22 Apr 2024 12:53:30 +0000 Subject: [PATCH 16/22] Fixed minimum iam policies doc to add permission for iam:GetUser --- userdocs/src/usage/minimum-iam-policies.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/userdocs/src/usage/minimum-iam-policies.md b/userdocs/src/usage/minimum-iam-policies.md index 9b001ecf82..dfdc7c4c3d 100644 --- a/userdocs/src/usage/minimum-iam-policies.md +++ b/userdocs/src/usage/minimum-iam-policies.md @@ -123,7 +123,6 @@ IamLimitedAccess "iam:DeleteInstanceProfile", "iam:GetInstanceProfile", "iam:RemoveRoleFromInstanceProfile", - "iam:GetUser", "iam:GetRole", "iam:CreateRole", "iam:DeleteRole", @@ -158,10 +157,12 @@ IamLimitedAccess { "Effect": "Allow", "Action": [ - "iam:GetRole" + "iam:GetRole", + "iam:GetUser" ], "Resource": [ - "arn:aws:iam:::role/*" + "arn:aws:iam:::role/*", + "arn:aws:iam:::user/*" ] }, { From 2e14dfa613b0ab1eec9b42d5ece28623f3792d7e Mon Sep 17 00:00:00 2001 From: Venkat Penmetsa Date: Mon, 22 Apr 2024 13:22:08 +0000 Subject: [PATCH 17/22] Updated access-entries doc at migrate-to-access-entry section --- userdocs/src/usage/access-entries.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/userdocs/src/usage/access-entries.md b/userdocs/src/usage/access-entries.md index 98e8ae7ef2..c0a45f4d47 100644 --- a/userdocs/src/usage/access-entries.md +++ b/userdocs/src/usage/access-entries.md @@ -150,22 +150,22 @@ eksctl delete accessentry -f config.yaml ### Migrate IAM identity mappings to access entries -The user can migrate their IAM identities from configmap to access entries by running the following: +The user can migrate their existing IAM identities from configmap to access entries by running the following: ```shell eksctl utils migrate-to-access-entry --cluster my-cluster --target-authentication-mode ``` -When `--target-authentication-mode API` is used, authentication mode is switched to `API` mode (skipped if already in `API` mode), IAM identity mappings will be migrated to access entries, and `aws-auth` configmap is deleted from the cluster. +When `--target-authentication-mode` flag is set to `API`, authentication mode is switched to `API` mode (skipped if already in `API` mode), IAM identity mappings will be migrated to access entries, and `aws-auth` configmap is deleted from the cluster. -When `--target-authentication-mode API_AND_CONFIG_MAP` is used, authentication mode is switched to `API_AND_CONFIG_MAP` mode (skipped if already in `API_AND_CONFIG_MAP` mode), IAM identity mappings will be migrated to access entries. +When `--target-authentication-mode` flag is set to `API_AND_CONFIG_MAP`, authentication mode is switched to `API_AND_CONFIG_MAP` mode (skipped if already in `API_AND_CONFIG_MAP` mode), IAM identity mappings will be migrated to access entries. ???+ note - When `--target-authentication-mode API` is used, this command will not update Authentication mode to `API` mode if `aws-auth` configmap has one of the below constraints. + When `--target-authentication-mode` flag is set to `API`, this command will not update authentication mode to `API` mode if `aws-auth` configmap has one of the below constraints. - * There is an Account level IAM identity mapping. - * One or more Roles/Users are mapped to the kubernetes group(s) which begin with prefix `system:` (except for `system:masters`) - * One or more IAM identity mapping(s) are for a [Service Linked Role](https://docs.aws.amazon.com/IAM/latest/UserGuide/using-service-linked-roles.html) + * There is an Account level identity mapping. + * One or more Roles/Users are mapped to the kubernetes group(s) which begin with prefix `system:` (except for EKS specific groups i.e. `system:masters`, `system:bootstrappers`, `system:nodes` etc). + * One or more IAM identity mapping(s) are for a [Service Linked Role](https://docs.aws.amazon.com/IAM/latest/UserGuide/using-service-linked-roles.html). ## Disabling cluster creator admin permissions From df601bc03267c32b15c8692242a7b950ebd0563c Mon Sep 17 00:00:00 2001 From: punkwalker Date: Mon, 22 Apr 2024 17:50:49 +0000 Subject: [PATCH 18/22] Fixes failing Migrate To Access Entry Test & go.mod, go.sum --- go.mod | 2 +- go.sum | 2 +- pkg/actions/accessentry/migrator_test.go | 22 ++++++++++++++++++---- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index b324bb66f3..9aa0835498 100644 --- a/go.mod +++ b/go.mod @@ -464,4 +464,4 @@ replace ( ) // Ensure k8s dependencies are also pinned accordingly -replace github.com/acomagu/bufpipe => github.com/acomagu/bufpipe v1.0.4 \ No newline at end of file +replace github.com/acomagu/bufpipe => github.com/acomagu/bufpipe v1.0.4 diff --git a/go.sum b/go.sum index f3489138bf..20cb1a6233 100644 --- a/go.sum +++ b/go.sum @@ -2803,4 +2803,4 @@ sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77Vzej sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= -sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= \ No newline at end of file +sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= diff --git a/pkg/actions/accessentry/migrator_test.go b/pkg/actions/accessentry/migrator_test.go index cc3dfa0c7d..8342411829 100644 --- a/pkg/actions/accessentry/migrator_test.go +++ b/pkg/actions/accessentry/migrator_test.go @@ -8,6 +8,7 @@ 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" @@ -23,6 +24,7 @@ import ( type migrateToAccessEntryEntry struct { clusterName string + curAuthMode string mockEKS func(provider *mockprovider.MockProvider) mockK8s func(clientSet *fake.Clientset) validateCustomLoggerOutput func(output string) @@ -37,8 +39,6 @@ var _ = Describe("Migrate Access Entry", func() { mockProvider *mockprovider.MockProvider fakeClientset *fake.Clientset clusterName = "test-cluster" - tgAuthMode = ekstypes.AuthenticationModeApi - curAuthMode = ekstypes.AuthenticationModeConfigMap ) DescribeTable("Migrate", func(ae migrateToAccessEntryEntry) { @@ -87,8 +87,8 @@ var _ = Describe("Migrate Access Entry", func() { mockProvider.MockIAM(), fakeClientset, *accessEntryCreator, - curAuthMode, - tgAuthMode, + ekstypes.AuthenticationMode(ae.curAuthMode), + ekstypes.AuthenticationMode(ae.options.TargetAuthMode), ) err := migrator.MigrateToAccessEntry(context.Background(), ae.options) @@ -105,6 +105,10 @@ var _ = Describe("Migrate Access Entry", func() { } }, 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). @@ -113,6 +117,16 @@ var _ = Describe("Migrate Access Entry", func() { 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{}, + }, nil) }, expectedErr: "failed to update cluster config", }), From 0f55a7ae02b16297930bfa9feb4e2d1b88d18ba9 Mon Sep 17 00:00:00 2001 From: punkwalker Date: Mon, 22 Apr 2024 17:55:48 +0000 Subject: [PATCH 19/22] Amends migrate to access entry documentation --- userdocs/src/usage/access-entries.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/userdocs/src/usage/access-entries.md b/userdocs/src/usage/access-entries.md index c0a45f4d47..4c652b5474 100644 --- a/userdocs/src/usage/access-entries.md +++ b/userdocs/src/usage/access-entries.md @@ -150,7 +150,7 @@ eksctl delete accessentry -f config.yaml ### Migrate IAM identity mappings to access entries -The user can migrate their existing IAM identities from configmap to access entries by running the following: +The user can migrate their existing IAM identities from `aws-auth` configmap to access entries by running the following: ```shell eksctl utils migrate-to-access-entry --cluster my-cluster --target-authentication-mode @@ -158,7 +158,7 @@ eksctl utils migrate-to-access-entry --cluster my-cluster --target-authenticatio When `--target-authentication-mode` flag is set to `API`, authentication mode is switched to `API` mode (skipped if already in `API` mode), IAM identity mappings will be migrated to access entries, and `aws-auth` configmap is deleted from the cluster. -When `--target-authentication-mode` flag is set to `API_AND_CONFIG_MAP`, authentication mode is switched to `API_AND_CONFIG_MAP` mode (skipped if already in `API_AND_CONFIG_MAP` mode), IAM identity mappings will be migrated to access entries. +When `--target-authentication-mode` flag is set to `API_AND_CONFIG_MAP`, authentication mode is switched to `API_AND_CONFIG_MAP` mode (skipped if already in `API_AND_CONFIG_MAP` mode), IAM identity mappings will be migrated to access entries, but `aws-auth` configmap is preserved. ???+ note When `--target-authentication-mode` flag is set to `API`, this command will not update authentication mode to `API` mode if `aws-auth` configmap has one of the below constraints. From a40a3126cd223c4362cf768c32899a3c4195f3e8 Mon Sep 17 00:00:00 2001 From: Tibi <110664232+TiberiuGC@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:33:33 +0300 Subject: [PATCH 20/22] improve logs and simplify code logic --- pkg/actions/accessentry/migrator.go | 38 ++++++++++++------------ pkg/ctl/utils/migrate_to_access_entry.go | 25 +++++++--------- 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index f00811aca9..ce554d369f 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -61,25 +61,27 @@ func NewMigrator( } func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOptions) error { + if m.tgAuthMode == ekstypes.AuthenticationModeConfigMap { + return fmt.Errorf("target authentication mode is invalid, must be either %s or %s", ekstypes.AuthenticationModeApi, ekstypes.AuthenticationModeApiAndConfigMap) + } + if m.curAuthMode == ekstypes.AuthenticationModeApi { + logger.Warning("cluster authentication mode is already %s; there is no need to migrate to access entries", ekstypes.AuthenticationModeApi) + return nil + } + logger.Info("current cluster authentication mode is %s; target cluster authentication mode is %s", m.curAuthMode, m.tgAuthMode) + taskTree := tasks.TaskTree{ Parallel: false, PlanMode: !options.Approve, } - if m.curAuthMode != m.tgAuthMode { + if m.curAuthMode == ekstypes.AuthenticationModeConfigMap { taskTree.Append(&tasks.GenericTask{ - Description: fmt.Sprintf("update authentication mode to %v", ekstypes.AuthenticationModeApiAndConfigMap), + Description: fmt.Sprintf("update authentication mode from %v to %v", ekstypes.AuthenticationModeConfigMap, ekstypes.AuthenticationModeApiAndConfigMap), Doer: func() error { - if m.curAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { - logger.Info("target authentication mode %v is different than the current authentication mode %v, Updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) - return m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap, options.Timeout) - } - m.curAuthMode = ekstypes.AuthenticationModeApiAndConfigMap - return nil + return m.doUpdateAuthenticationMode(ctx, ekstypes.AuthenticationModeApiAndConfigMap, options.Timeout) }, }) - } else { - logger.Info("target authentication mode %v is same as current authentication mode %v, not updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) } cmEntries, err := m.doGetIAMIdentityMappings(ctx) @@ -97,23 +99,22 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOp return err } - if newaelen := len(newAccessEntries); newaelen != 0 { - logger.Info("%d new access entries will be created", newaelen) + if len(newAccessEntries) > 0 { aeTasks := m.aeCreator.CreateTasks(ctx, newAccessEntries) aeTasks.IsSubTask = true taskTree.Append(aeTasks) } - if !skipAPImode { - if m.tgAuthMode == ekstypes.AuthenticationModeApi { + if m.tgAuthMode == ekstypes.AuthenticationModeApi { + if skipAPImode { + logger.Warning("one or more iamidentitymapping(s) could not be migrated to access entry, will not update authentication mode to %v", ekstypes.AuthenticationModeApi) + } else { taskTree.Append(&tasks.GenericTask{ - Description: fmt.Sprintf("update authentication mode to %v", ekstypes.AuthenticationModeApi), + Description: fmt.Sprintf("update authentication mode from %v to %v", ekstypes.AuthenticationModeApiAndConfigMap, ekstypes.AuthenticationModeApi), Doer: func() error { - logger.Info("target authentication mode %v is different than the current authentication mode %v, updating the cluster authentication mode", m.tgAuthMode, m.curAuthMode) return m.doUpdateAuthenticationMode(ctx, m.tgAuthMode, options.Timeout) }, }) - taskTree.Append(&tasks.GenericTask{ Description: fmt.Sprintf("delete aws-auth configMap when authentication mode is %v", ekstypes.AuthenticationModeApi), Doer: func() error { @@ -121,14 +122,13 @@ func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOp }, }) } - } else if m.tgAuthMode == ekstypes.AuthenticationModeApi { - logger.Warning("one or more iamidentitymapping could not be migrated to access entry, will not update authentication mode to %v", ekstypes.AuthenticationModeApi) } return runAllTasks(&taskTree) } func (m *Migrator) doUpdateAuthenticationMode(ctx context.Context, authMode ekstypes.AuthenticationMode, timeout time.Duration) error { + logger.Info("updating cluster authentication mode to %v", authMode) output, err := m.eksAPI.UpdateClusterConfig(ctx, &awseks.UpdateClusterConfigInput{ Name: aws.String(m.clusterName), AccessConfig: &ekstypes.UpdateAccessConfigRequest{ diff --git a/pkg/ctl/utils/migrate_to_access_entry.go b/pkg/ctl/utils/migrate_to_access_entry.go index 31c00237b7..28248d8f00 100644 --- a/pkg/ctl/utils/migrate_to_access_entry.go +++ b/pkg/ctl/utils/migrate_to_access_entry.go @@ -2,7 +2,6 @@ package utils import ( "context" - "fmt" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" "github.com/spf13/cobra" @@ -32,24 +31,17 @@ func migrateAccessEntryCmd(cmd *cmdutils.Cmd) { cmd.CobraCommand.RunE = func(_ *cobra.Command, args []string) error { cmd.NameArg = cmdutils.GetNameArg(args) + options.Approve = !cmd.Plan return doMigrateToAccessEntry(cmd, options) } } func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.MigrationOptions) error { - defer cmdutils.LogPlanModeWarning(options.Approve) - options.Approve = !cmd.Plan cfg := cmd.ClusterConfig - tgAuthMode := ekstypes.AuthenticationMode(options.TargetAuthMode) - if cfg.Metadata.Name == "" { return cmdutils.ErrMustBeSet(cmdutils.ClusterNameFlag(cmd)) } - if tgAuthMode != ekstypes.AuthenticationModeApi && tgAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { - return fmt.Errorf("target authentication mode is invalid, must be either %s or %s", ekstypes.AuthenticationModeApi, ekstypes.AuthenticationModeApiAndConfigMap) - } - ctx := context.Background() ctl, err := cmd.NewProviderForExistingCluster(ctx) if err != nil { @@ -60,8 +52,6 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Migrat return err } - curAuthMode := ctl.GetClusterState().AccessConfig.AuthenticationMode - clientSet, err := ctl.NewStdClientSet(cfg) if err != nil { return err @@ -73,13 +63,18 @@ func doMigrateToAccessEntry(cmd *cmdutils.Cmd, options accessentryactions.Migrat StackCreator: stackManager, } - return accessentryactions.NewMigrator( + if err := accessentryactions.NewMigrator( cfg.Metadata.Name, ctl.AWSProvider.EKS(), ctl.AWSProvider.IAM(), clientSet, aeCreator, - curAuthMode, - tgAuthMode, - ).MigrateToAccessEntry(ctx, options) + ctl.GetClusterState().AccessConfig.AuthenticationMode, + ekstypes.AuthenticationMode(options.TargetAuthMode), + ).MigrateToAccessEntry(ctx, options); err != nil { + return err + } + + cmdutils.LogPlanModeWarning(cmd.Plan) + return nil } From a21a8c10b8380ff12974164a3a1361186527eed9 Mon Sep 17 00:00:00 2001 From: Tibi <110664232+TiberiuGC@users.noreply.github.com> Date: Wed, 24 Apr 2024 13:43:03 +0300 Subject: [PATCH 21/22] add unit tests --- pkg/actions/accessentry/fakes/fake_getter.go | 120 ++++++ pkg/actions/accessentry/getter.go | 6 + pkg/actions/accessentry/migrator.go | 39 +- pkg/actions/accessentry/migrator_test.go | 379 ++++++++++++++++--- pkg/ctl/utils/migrate_to_access_entry.go | 4 +- pkg/iam/mapping.go | 16 +- 6 files changed, 487 insertions(+), 77 deletions(-) create mode 100644 pkg/actions/accessentry/fakes/fake_getter.go diff --git a/pkg/actions/accessentry/fakes/fake_getter.go b/pkg/actions/accessentry/fakes/fake_getter.go new file mode 100644 index 0000000000..8456352719 --- /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 418f81a6a5..7c2f4c2638 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 ce554d369f..2be1d1993a 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 8342411829..0464506635 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 28248d8f00..31022eed78 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 526c1f1621..753e96938b 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 From f34dd6f9af96d9e07e4715e616dc204e05004105 Mon Sep 17 00:00:00 2001 From: Tibi <110664232+TiberiuGC@users.noreply.github.com> Date: Thu, 25 Apr 2024 13:34:12 +0300 Subject: [PATCH 22/22] ensure target-auth-mode has a valid value --- pkg/actions/accessentry/migrator.go | 2 +- pkg/actions/accessentry/migrator_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/actions/accessentry/migrator.go b/pkg/actions/accessentry/migrator.go index 2be1d1993a..b3a52c2ad8 100644 --- a/pkg/actions/accessentry/migrator.go +++ b/pkg/actions/accessentry/migrator.go @@ -64,7 +64,7 @@ func NewMigrator( } func (m *Migrator) MigrateToAccessEntry(ctx context.Context, options MigrationOptions) error { - if m.tgAuthMode == ekstypes.AuthenticationModeConfigMap { + if m.tgAuthMode != ekstypes.AuthenticationModeApi && m.tgAuthMode != ekstypes.AuthenticationModeApiAndConfigMap { return fmt.Errorf("target authentication mode is invalid, must be either %s or %s", ekstypes.AuthenticationModeApi, ekstypes.AuthenticationModeApiAndConfigMap) } if m.curAuthMode == ekstypes.AuthenticationModeApi { diff --git a/pkg/actions/accessentry/migrator_test.go b/pkg/actions/accessentry/migrator_test.go index 0464506635..0a8cb5c888 100644 --- a/pkg/actions/accessentry/migrator_test.go +++ b/pkg/actions/accessentry/migrator_test.go @@ -126,6 +126,7 @@ var _ = Describe("Migrate Access Entry", func() { Entry("[Validation Error] current authentication mode is API", migrateToAccessEntryEntry{ curAuthMode: ekstypes.AuthenticationModeApi, + tgAuthMode: 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))) }, @@ -133,6 +134,7 @@ var _ = Describe("Migrate Access Entry", func() { Entry("[API Error] getting access entries fails", migrateToAccessEntryEntry{ curAuthMode: ekstypes.AuthenticationModeApiAndConfigMap, + tgAuthMode: ekstypes.AuthenticationModeApi, mockAccessEntries: func(getter *fakes.FakeGetterInterface) { getter.GetReturns(nil, genericErr) },