Skip to content

Commit

Permalink
fix logic for updating aws-auth configmap
Browse files Browse the repository at this point in the history
  • Loading branch information
@TiberiuGC
TiberiuGC committed Apr 3, 2024
1 parent 4d0d67e commit d64a798
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 60 deletions.
25 changes: 20 additions & 5 deletions integration/tests/accessentries/accessentries_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ var (
namespaceRoleARN string
err error

apiEnabledCluster = "accessentries-api-enabled-2"
apiDisabledCluster = "accessentries-api-disabled-2"
apiEnabledCluster = "accessentries-api-enabled"
apiDisabledCluster = "accessentries-api-disabled"
)

func init() {
Expand Down Expand Up @@ -123,24 +123,39 @@ var _ = Describe("(Integration) [AccessEntries Test]", func() {
cfg = makeClusterConfig(apiDisabledCluster)
})

It("should create a cluster with authenticationMode set to CONFIG_MAP", func() {
It("should create a cluster with authenticationMode set to CONFIG_MAP and allow self-managed nodes to join via aws-auth", func() {
cfg.AccessConfig.AuthenticationMode = ekstypes.AuthenticationModeConfigMap

cfg.NodeGroups = append(cfg.NodeGroups, &api.NodeGroup{
NodeGroupBase: &api.NodeGroupBase{
Name: "aws-auth-ng",
ScalingConfig: &api.ScalingConfig{
DesiredCapacity: aws.Int(1),
},
},
})
data, err := json.Marshal(cfg)
Expect(err).NotTo(HaveOccurred())

Expect(params.EksctlCreateCmd.
WithArgs(
"cluster",
"--config-file", "-",
"--without-nodegroup",
"--verbose", "4",
).
WithoutArg("--region", params.Region).
WithStdin(bytes.NewReader(data))).To(RunSuccessfully())

Expect(ctl.RefreshClusterStatus(context.Background(), cfg)).NotTo(HaveOccurred())
Expect(ctl.IsAccessEntryEnabled()).To(BeFalse())

Expect(params.EksctlGetCmd.WithArgs(
"nodegroup",
"--cluster", apiDisabledCluster,
"--name", "aws-auth-ng",
"-o", "yaml",
)).To(runner.RunSuccessfullyWithOutputStringLines(
ContainElement(ContainSubstring("Status: CREATE_COMPLETE")),
))
})

It("should fail early when trying to create access entries", func() {
Expand Down
18 changes: 14 additions & 4 deletions pkg/actions/nodegroup/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/ec2"
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"

"github.com/kris-nova/logger"
"github.com/pkg/errors"
Expand Down Expand Up @@ -253,7 +254,7 @@ func (m *Manager) nodeCreationTasks(ctx context.Context, isOwnedCluster, skipEgr
allNodeGroupTasks := &tasks.TaskTree{
Parallel: true,
}
disableAccessEntryCreation := !m.accessEntry.IsEnabled() || updateAuthConfigMap != nil
disableAccessEntryCreation := !m.accessEntry.IsEnabled()
nodeGroupTasks := m.stackManager.NewUnmanagedNodeGroupTask(ctx, cfg.NodeGroups, !awsNodeUsesIRSA, skipEgressRules, disableAccessEntryCreation, vpcImporter)
if nodeGroupTasks.Len() > 0 {
allNodeGroupTasks.Append(nodeGroupTasks)
Expand Down Expand Up @@ -285,19 +286,28 @@ func (m *Manager) postNodeCreationTasks(ctx context.Context, clientSet kubernete
timeoutCtx, cancel := context.WithTimeout(ctx, m.ctl.AWSProvider.WaitTimeout())
defer cancel()

if (!m.accessEntry.IsEnabled() && !api.IsDisabled(options.UpdateAuthConfigMap)) || api.IsEnabled(options.UpdateAuthConfigMap) {
// authorize self-managed nodes to join the cluster via aws-auth configmap
// if EKS access entries are disabled OR
if (!m.accessEntry.IsEnabled() && !api.IsDisabled(options.UpdateAuthConfigMap)) ||
// if explicitly requested by the user
api.IsEnabled(options.UpdateAuthConfigMap) {
if err := eks.UpdateAuthConfigMap(m.cfg.NodeGroups, clientSet); err != nil {
return err
}
}
if !api.IsDisabled(options.UpdateAuthConfigMap) {

// only wait for self-managed nodes to join if either authorization method is being used
if m.accessEntry.IsEnabled() || !api.IsDisabled(options.UpdateAuthConfigMap) {
for _, ng := range m.cfg.NodeGroups {
if err := eks.WaitForNodes(timeoutCtx, clientSet, ng); err != nil {
return err
}
}
} else {
logger.Warning(fmt.Sprintf("cluster autenticationMode is %s; setting --update-auth-configmap to false will prevent self-managed nodes to join the cluster until authorized", ekstypes.AuthenticationModeConfigMap))
}
logger.Success("created %d nodegroup(s) in cluster %q", len(m.cfg.NodeGroups), m.cfg.Metadata.Name)

for _, ng := range m.cfg.ManagedNodeGroups {
if err := eks.WaitForNodes(timeoutCtx, clientSet, ng); err != nil {
if m.cfg.PrivateCluster.Enabled {
Expand All @@ -308,8 +318,8 @@ func (m *Manager) postNodeCreationTasks(ctx context.Context, clientSet kubernete
}
}
}

logger.Success("created %d managed nodegroup(s) in cluster %q", len(m.cfg.ManagedNodeGroups), m.cfg.Metadata.Name)

return nil
}

Expand Down
112 changes: 62 additions & 50 deletions pkg/actions/nodegroup/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/types"
"github.com/pkg/errors"
"github.com/stretchr/testify/mock"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -494,7 +495,7 @@ var _ = DescribeTable("Create", func(t ngEntry) {
},
}),

Entry("fails to create nodegroup when authenticationMode is API and updateAuthConfigMap is false", ngEntry{
Entry("[Nodegroup authorization error] when authenticationMode is API and updateAuthConfigMap is false", ngEntry{
opts: nodegroup.CreateOpts{
UpdateAuthConfigMap: api.Disabled(),
},
Expand All @@ -506,15 +507,13 @@ var _ = DescribeTable("Create", func(t ngEntry) {
},
refreshCluster: true,
expectedCalls: func(e expectedCalls) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(0))
Expect(e.kubeProvider.ServerVersionCallCount()).To(Equal(0))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(0))
expectedSetupCalls(e, 0)
},

expectedErr: errors.New("--update-auth-configmap is not supported when authenticationMode is set to API"),
}),

Entry("fails to create nodegroup when authenticationMode is API and updateAuthConfigMap is true", ngEntry{
Entry("[Nodegroup authorization error] when authenticationMode is API and updateAuthConfigMap is true", ngEntry{
opts: nodegroup.CreateOpts{
UpdateAuthConfigMap: api.Enabled(),
},
Expand All @@ -526,32 +525,28 @@ var _ = DescribeTable("Create", func(t ngEntry) {
},
refreshCluster: true,
expectedCalls: func(e expectedCalls) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(0))
Expect(e.kubeProvider.ServerVersionCallCount()).To(Equal(0))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(0))
expectedSetupCalls(e, 0)
},

expectedErr: errors.New("--update-auth-configmap is not supported when authenticationMode is set to API"),
}),

Entry("creates nodegroup using access entries when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is not supplied", ngEntry{
Entry("[Nodegroup authorization via aws-auth ConfigMap] when authenticationMode is CONFIG_MAP and updateAuthConfigMap is not supplied", ngEntry{
mockCalls: func(m mockCalls) {
mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{
AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap,
AuthenticationMode: ekstypes.AuthenticationModeConfigMap,
})
defaultProviderMocks(m.mockProvider, defaultOutput)
},
refreshCluster: true,
expectedCalls: func(e expectedCalls) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(1))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(1))
Expect(e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskCallCount()).To(Equal(1))
_, _, _, _, disableAccessEntryCreation, _ := e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskArgsForCall(0)
Expect(disableAccessEntryCreation).To(BeFalse())
Expect(getIAMIdentities(e.clientset)).To(HaveLen(0))
expectedSetupCalls(e, 1)
expectAccessEntriesCreationDisabled(e, true)
expectConfigMapIAMIdentities(e, 1)
},
}),

Entry("creates nodegroup using aws-auth ConfigMap when authenticationMode is CONFIG_MAP and updateAuthConfigMap is true", ngEntry{
Entry("[Nodegroup authorization via aws-auth ConfigMap] when authenticationMode is CONFIG_MAP and updateAuthConfigMap is true", ngEntry{
mockCalls: func(m mockCalls) {
mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{
AuthenticationMode: ekstypes.AuthenticationModeConfigMap,
Expand All @@ -562,24 +557,28 @@ var _ = DescribeTable("Create", func(t ngEntry) {
UpdateAuthConfigMap: api.Enabled(),
},
refreshCluster: true,
expectedCalls: expectedCallsForAWSAuth,
expectedCalls: func(e expectedCalls) {
expectedSetupCalls(e, 1)
expectAccessEntriesCreationDisabled(e, true)
expectConfigMapIAMIdentities(e, 1)
},
}),

Entry("creates nodegroup using aws-auth ConfigMap when authenticationMode is CONFIG_MAP and updateAuthConfigMap is not supplied", ngEntry{
Entry("[Nodegroup authorization via access entries] when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is not supplied", ngEntry{
mockCalls: func(m mockCalls) {
mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{
AuthenticationMode: ekstypes.AuthenticationModeConfigMap,
AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap,
})
defaultProviderMocks(m.mockProvider, defaultOutput)
},
opts: nodegroup.CreateOpts{
UpdateAuthConfigMap: api.Enabled(),
expectedCalls: func(e expectedCalls) {
expectedSetupCalls(e, 1)
expectAccessEntriesCreationDisabled(e, false)
expectConfigMapIAMIdentities(e, 0)
},
refreshCluster: true,
expectedCalls: expectedCallsForAWSAuth,
}),

Entry("creates nodegroup but does not use either aws-auth ConfigMap or access entries when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is false", ngEntry{
Entry("[Nodegroup authorization via access entries] when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is false", ngEntry{
mockCalls: func(m mockCalls) {
mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{
AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap,
Expand All @@ -591,48 +590,46 @@ var _ = DescribeTable("Create", func(t ngEntry) {
UpdateAuthConfigMap: api.Disabled(),
},
expectedCalls: func(e expectedCalls) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(1))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(1))
Expect(e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskCallCount()).To(Equal(1))
_, _, _, _, disableAccessEntryCreation, _ := e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskArgsForCall(0)
Expect(disableAccessEntryCreation).To(BeTrue())
Expect(getIAMIdentities(e.clientset)).To(HaveLen(0))
expectedSetupCalls(e, 1)
expectAccessEntriesCreationDisabled(e, false)
expectConfigMapIAMIdentities(e, 0)
},
}),

Entry("creates nodegroup but does not use either aws-auth ConfigMap or access entries when authenticationMode is CONFIG_MAP and updateAuthConfigMap is false", ngEntry{
Entry("[Nodegroup authorization via both methods] when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is true", ngEntry{
mockCalls: func(m mockCalls) {
mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{
AuthenticationMode: ekstypes.AuthenticationModeConfigMap,
AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap,
})
defaultProviderMocks(m.mockProvider, defaultOutput)
},
refreshCluster: true,
opts: nodegroup.CreateOpts{
UpdateAuthConfigMap: api.Disabled(),
UpdateAuthConfigMap: api.Enabled(),
},
expectedCalls: func(e expectedCalls) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(1))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(1))
Expect(e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskCallCount()).To(Equal(1))
_, _, _, _, disableAccessEntryCreation, _ := e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskArgsForCall(0)
Expect(disableAccessEntryCreation).To(BeTrue())
Expect(getIAMIdentities(e.clientset)).To(HaveLen(0))
expectedSetupCalls(e, 1)
expectAccessEntriesCreationDisabled(e, false)
expectConfigMapIAMIdentities(e, 1)
},
}),

Entry("authorizes nodegroups using aws-auth ConfigMap when authenticationMode is API_AND_CONFIG_MAP and updateAuthConfigMap is true", ngEntry{
Entry("[Nodegroup authorization via neither method] when authenticationMode is CONFIG_MAP and updateAuthConfigMap is false", ngEntry{
mockCalls: func(m mockCalls) {
mockProviderWithConfig(m.mockProvider, defaultOutput, nil, nil, &ekstypes.AccessConfigResponse{
AuthenticationMode: ekstypes.AuthenticationModeApiAndConfigMap,
AuthenticationMode: ekstypes.AuthenticationModeConfigMap,
})
defaultProviderMocks(m.mockProvider, defaultOutput)
},
refreshCluster: true,
opts: nodegroup.CreateOpts{
UpdateAuthConfigMap: api.Enabled(),
UpdateAuthConfigMap: api.Disabled(),
},
expectedCalls: func(e expectedCalls) {
expectedSetupCalls(e, 1)
expectAccessEntriesCreationDisabled(e, true)
expectConfigMapIAMIdentities(e, 0)
},
expectedCalls: expectedCallsForAWSAuth,
}),

Entry("[happy path] creates nodegroup with no options", ngEntry{
Expand Down Expand Up @@ -744,14 +741,29 @@ func getIAMIdentities(clientset kubernetes.Interface) []iam.Identity {
return identities
}

func expectedCallsForAWSAuth(e expectedCalls) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(1))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(1))
Expect(e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskCallCount()).To(Equal(1))
func expectedSetupCalls(e expectedCalls, callCount int) {
Expect(e.kubeProvider.NewRawClientCallCount()).To(Equal(callCount))
Expect(e.nodeGroupFilter.SetOnlyLocalCallCount()).To(Equal(callCount))
Expect(e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskCallCount()).To(Equal(callCount))
}

func expectAccessEntriesCreationDisabled(e expectedCalls, shouldDisable bool) {
var match types.GomegaMatcher
if shouldDisable {
match = BeTrue()
} else {
match = BeFalse()
}
_, _, _, _, disableAccessEntryCreation, _ := e.nodeGroupTaskCreator.NewUnmanagedNodeGroupTaskArgsForCall(0)
Expect(disableAccessEntryCreation).To(BeTrue())
Expect(disableAccessEntryCreation).To(match)
}

func expectConfigMapIAMIdentities(e expectedCalls, iamIdentitiesCount int) {
identities := getIAMIdentities(e.clientset)
Expect(identities).To(HaveLen(1))
Expect(identities).To(HaveLen(iamIdentitiesCount))
if iamIdentitiesCount == 0 {
return
}
for _, id := range identities {
roleIdentity, ok := id.(iam.RoleIdentity)
Expect(ok).To(BeTrue())
Expand Down
13 changes: 12 additions & 1 deletion pkg/ctl/create/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sync"

"github.com/aws/aws-sdk-go-v2/aws"
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"

"github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector"
"github.com/kris-nova/logger"
Expand Down Expand Up @@ -426,18 +427,28 @@ func doCreateCluster(cmd *cmdutils.Cmd, ngFilter *filter.NodeGroupFilter, params
} else {
ngCtx, cancel := context.WithTimeout(ctx, cmd.ProviderConfig.WaitTimeout)
defer cancel()

// authorize self-managed nodes to join the cluster via aws-auth configmap
// only if EKS access entries are disabled
if cfg.AccessConfig.AuthenticationMode == ekstypes.AuthenticationModeConfigMap {
if err := eks.UpdateAuthConfigMap(cfg.NodeGroups, clientSet); err != nil {
return err
}
}

for _, ng := range cfg.NodeGroups {
// wait for nodes to join
if err := eks.WaitForNodes(ngCtx, clientSet, ng); err != nil {
return err
}
}
logger.Success("created %d nodegroup(s) in cluster %q", len(cfg.NodeGroups), cfg.Metadata.Name)

for _, ng := range cfg.ManagedNodeGroups {
if err := eks.WaitForNodes(ngCtx, clientSet, ng); err != nil {
return err
}
}
logger.Success("created %d managed nodegroup(s) in cluster %q", len(cfg.ManagedNodeGroups), cfg.Metadata.Name)
}
}
if postNodegroupAddons != nil && postNodegroupAddons.Len() > 0 {
Expand Down

0 comments on commit d64a798

Please sign in to comment.