Fix reusing instanceRoleARN for nodegroups authorized with access entries

This changelist changes the design of creating access entries for self-managed nodegroups that use a pre-existing instanceRoleARN by creating the access entry resource outside of the CloudFormation stack by making a separate call to the AWS API. When deleting such a nodegroup, it's the user's responsibility to also delete the corresponding access entry when no more nodegroups are associated with it. This is because eksctl cannot tell if an access entry resource is still in use by non-eksctl created self-managed nodegroups.

Self-managed nodegroups not using a pre-existing instanceRoleARN will continue to have the access entry resource in the CloudFormation stack, making delete nodegroup an atomic operation for most use cases.

Fixes eksctl-io#7502

Author: cPu1

.mockery.yaml

@@ -36,3 +36,15 @@ packages:
         config:
           dir: "{{.InterfaceDir}}/mocks"
           outpkg: mocks
+
+  github.com/weaveworks/eksctl/pkg/cfn/manager:
+    interfaces:
+      NodeGroupStackManager:
+        config:
+          dir: "{{.InterfaceDir}}/mocks"
+          outpkg: mocks
+
+      NodeGroupResourceSet:
+        config:
+          dir: "{{.InterfaceDir}}/mocks"
+          outpkg: mocks

pkg/actions/nodegroup/create.go

@@ -254,8 +254,8 @@ func (m *Manager) nodeCreationTasks(ctx context.Context, isOwnedCluster, skipEgr
 		Parallel: true,
 	}
 	disableAccessEntryCreation := !m.accessEntry.IsEnabled() || updateAuthConfigMap != nil
-	nodeGroupTasks := m.stackManager.NewUnmanagedNodeGroupTask(ctx, cfg.NodeGroups, !awsNodeUsesIRSA, skipEgressRules, disableAccessEntryCreation, vpcImporter)
-	if nodeGroupTasks.Len() > 0 {
+	if nodeGroupTasks := m.stackManager.NewUnmanagedNodeGroupTask(ctx, cfg.NodeGroups, !awsNodeUsesIRSA, skipEgressRules,
+		disableAccessEntryCreation, vpcImporter); nodeGroupTasks.Len() > 0 {
 		allNodeGroupTasks.Append(nodeGroupTasks)
 	}
 	managedTasks := m.stackManager.NewManagedNodeGroupTask(ctx, cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter)

pkg/actions/nodegroup/nodegroup.go

@@ -3,11 +3,10 @@ package nodegroup
 import (
 	"time"
 
-	"github.com/weaveworks/eksctl/pkg/accessentry"
-
 	"github.com/aws/aws-sdk-go/aws/request"
 	"k8s.io/client-go/kubernetes"
 
+	"github.com/weaveworks/eksctl/pkg/accessentry"
 	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 	"github.com/weaveworks/eksctl/pkg/cfn/builder"
 	"github.com/weaveworks/eksctl/pkg/cfn/manager"

pkg/apis/eksctl.io/v1alpha5/access_entry.go

@@ -42,6 +42,28 @@ type AccessScope struct {
 	Namespaces []string `json:"namespaces,omitempty"`
 }
 
+// AccessEntryType represents the type of access entry.
+type AccessEntryType string
+
+const (
+	// AccessEntryTypeLinux specifies the EC2 Linux access entry type.
+	AccessEntryTypeLinux AccessEntryType = "EC2_LINUX"
+	// AccessEntryTypeWindows specifies the Windows access entry type.
+	AccessEntryTypeWindows AccessEntryType = "EC2_WINDOWS"
+	// AccessEntryTypeFargateLinux specifies the Fargate Linux access entry type.
+	AccessEntryTypeFargateLinux AccessEntryType = "FARGATE_LINUX"
+	// AccessEntryTypeStandard specifies a standard access entry type.
+	AccessEntryTypeStandard AccessEntryType = "STANDARD"
+)
+
+// GetAccessEntryType returns the access entry type for the specified AMI family.
+func GetAccessEntryType(ng *NodeGroup) AccessEntryType {
+	if IsWindowsImage(ng.GetAMIFamily()) {
+		return AccessEntryTypeWindows
+	}
+	return AccessEntryTypeLinux
+}
+
 type ARN arn.ARN
 
 // ARN provides custom unmarshalling for an AWS ARN.
@@ -103,9 +125,9 @@ func validateAccessEntries(accessEntries []AccessEntry) error {
 			return fmt.Errorf("%s.principalARN must be set to a valid AWS ARN", path)
 		}
 
-		switch ae.Type {
-		case "", "STANDARD":
-		case "EC2_LINUX", "EC2_WINDOWS", "FARGATE_LINUX":
+		switch AccessEntryType(ae.Type) {
+		case "", AccessEntryTypeStandard:
+		case AccessEntryTypeLinux, AccessEntryTypeWindows, AccessEntryTypeFargateLinux:
 			if len(ae.KubernetesGroups) > 0 || ae.KubernetesUsername != "" {
 				return fmt.Errorf("cannot specify %s.kubernetesGroups nor %s.kubernetesUsername when type is set to %s", path, path, ae.Type)
 			}

pkg/cfn/builder/iam.go

@@ -124,30 +124,31 @@ func (n *NodeGroupResourceSet) WithNamedIAM() bool {
 }
 
 func (n *NodeGroupResourceSet) addResourcesForIAM(ctx context.Context) error {
-	if n.spec.IAM.InstanceProfileARN != "" {
+	nodeGroupIAM := n.options.NodeGroup.IAM
+	if nodeGroupIAM.InstanceProfileARN != "" {
 		n.rs.withIAM = false
 		n.rs.withNamedIAM = false
 
 		// if instance profile is given, as well as the role, we simply use both and export the role
 		// (which is needed in order to authorise the nodegroup)
-		n.instanceProfileARN = gfnt.NewString(n.spec.IAM.InstanceProfileARN)
-		if n.spec.IAM.InstanceRoleARN != "" {
-			n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceProfileARN, n.spec.IAM.InstanceProfileARN, true)
-			n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceRoleARN, n.spec.IAM.InstanceRoleARN, true)
+		n.instanceProfileARN = gfnt.NewString(nodeGroupIAM.InstanceProfileARN)
+		if nodeGroupIAM.InstanceRoleARN != "" {
+			n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceProfileARN, nodeGroupIAM.InstanceProfileARN, true)
+			n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceRoleARN, nodeGroupIAM.InstanceRoleARN, true)
 			return nil
 		}
 		// if instance role is not given, export profile and use the getter to call importer function
-		n.rs.defineOutput(outputs.NodeGroupInstanceProfileARN, n.spec.IAM.InstanceProfileARN, true, func(v string) error {
-			return iam.ImportInstanceRoleFromProfileARN(ctx, n.iamAPI, n.spec, v)
+		n.rs.defineOutput(outputs.NodeGroupInstanceProfileARN, nodeGroupIAM.InstanceProfileARN, true, func(v string) error {
+			return iam.ImportInstanceRoleFromProfileARN(ctx, n.iamAPI, n.options.NodeGroup, v)
 		})
 
 		return nil
 	}
 
 	n.rs.withIAM = true
 
-	if n.spec.IAM.InstanceRoleARN != "" {
-		roleARN := NormalizeARN(n.spec.IAM.InstanceRoleARN)
+	if nodeGroupIAM.InstanceRoleARN != "" {
+		roleARN := NormalizeARN(nodeGroupIAM.InstanceRoleARN)
 
 		// if role is set, but profile isn't - create profile
 		n.newResource(cfnIAMInstanceProfileName, &gfniam.InstanceProfile{
@@ -156,7 +157,7 @@ func (n *NodeGroupResourceSet) addResourcesForIAM(ctx context.Context) error {
 		})
 		n.instanceProfileARN = gfnt.MakeFnGetAttString(cfnIAMInstanceProfileName, "Arn")
 		n.rs.defineOutputFromAtt(outputs.NodeGroupInstanceProfileARN, cfnIAMInstanceProfileName, "Arn", true, func(v string) error {
-			n.spec.IAM.InstanceProfileARN = v
+			nodeGroupIAM.InstanceProfileARN = v
 			return nil
 		})
 		n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceRoleARN, roleARN, true)
@@ -165,12 +166,12 @@ func (n *NodeGroupResourceSet) addResourcesForIAM(ctx context.Context) error {
 
 	// if neither role nor profile is given - create both
 
-	if n.spec.IAM.InstanceRoleName != "" {
+	if nodeGroupIAM.InstanceRoleName != "" {
 		// setting role name requires additional capabilities
 		n.rs.withNamedIAM = true
 	}
 
-	if err := createRole(n.rs, n.clusterSpec.IAM, n.spec.IAM, false, n.forceAddCNIPolicy); err != nil {
+	if err := createRole(n.rs, n.options.ClusterConfig.IAM, nodeGroupIAM, false, n.options.ForceAddCNIPolicy); err != nil {
 		return err
 	}
 
@@ -181,11 +182,11 @@ func (n *NodeGroupResourceSet) addResourcesForIAM(ctx context.Context) error {
 	n.instanceProfileARN = gfnt.MakeFnGetAttString(cfnIAMInstanceProfileName, "Arn")
 
 	n.rs.defineOutputFromAtt(outputs.NodeGroupInstanceProfileARN, cfnIAMInstanceProfileName, "Arn", true, func(v string) error {
-		n.spec.IAM.InstanceProfileARN = v
+		nodeGroupIAM.InstanceProfileARN = v
 		return nil
 	})
 	n.rs.defineOutputFromAtt(outputs.NodeGroupInstanceRoleARN, cfnIAMInstanceRoleName, "Arn", true, func(v string) error {
-		n.spec.IAM.InstanceRoleARN = v
+		nodeGroupIAM.InstanceRoleARN = v
 		return nil
 	})
 	return nil