Create EFA specific security group for self-managed node groups always (#8554)

Author: NicholasBlaskey

pkg/cfn/builder/managed_launch_template.go

@@ -84,7 +84,7 @@ func (m *ManagedNodeGroupResourceSet) makeLaunchTemplateData(ctx context.Context
 			Description:    "worker nodes in group " + m.nodeGroup.Name,
 		}
 
-		efaSG, err := efa.ProcessSecurityGroup(config, m.addEFASecurityGroup)
+		efaSG, err := efa.ProcessSecurityGroup(config, m.addEFASecurityGroup, true)
 		if err != nil {
 			return nil, err
 		} else if efaSG != nil {

pkg/cfn/builder/nodegroup.go

@@ -231,7 +231,7 @@ func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() error {
 			Description:    desc,
 		}
 
-		efaSG, err := efa.ProcessSecurityGroup(config, n.rs.addEFASecurityGroup)
+		efaSG, err := efa.ProcessSecurityGroup(config, n.rs.addEFASecurityGroup, false)
 		if err != nil {
 			return err
 		} else if efaSG != nil {

pkg/cfn/builder/nodegroup_test.go

@@ -684,27 +684,15 @@ var _ = Describe("Unmanaged NodeGroup Template Builder", func() {
 					)
 				})
 
-				Context("Kubernetes 1.33+ with built-in EFA support", func() {
+				Context("Kubernetes 1.33+ has no built-in EFA support since self managed", func() {
 					BeforeEach(func() {
 						cfg.Metadata.Version = "1.33"
 					})
 
-					It("does not create custom EFA security groups", func() {
-						Expect(ngTemplate.Resources).NotTo(HaveKey("EFASG"))
-						Expect(ngTemplate.Resources).NotTo(HaveKey("EFAEgressSelf"))
-						Expect(ngTemplate.Resources).NotTo(HaveKey("EFAIngressSelf"))
-					})
-				})
-
-				Context("Kubernetes 1.34 with built-in EFA support", func() {
-					BeforeEach(func() {
-						cfg.Metadata.Version = "1.34"
-					})
-
-					It("does not create custom EFA security groups", func() {
-						Expect(ngTemplate.Resources).NotTo(HaveKey("EFASG"))
-						Expect(ngTemplate.Resources).NotTo(HaveKey("EFAEgressSelf"))
-						Expect(ngTemplate.Resources).NotTo(HaveKey("EFAIngressSelf"))
+					It("creates custom EFA security groups", func() {
+						Expect(ngTemplate.Resources).To(HaveKey("EFASG"))
+						Expect(ngTemplate.Resources).To(HaveKey("EFAEgressSelf"))
+						Expect(ngTemplate.Resources).To(HaveKey("EFAIngressSelf"))
 					})
 				})
 

pkg/utils/efa/efa.go

@@ -10,13 +10,13 @@ import (
 )
 
 // IsBuiltInSupported returns true if the Kubernetes version supports built-in EFA in the default security group
-func IsBuiltInSupported(kubernetesVersion string) (bool, error) {
-	supported, err := version.IsMinVersion(api.EFABuiltInSupportVersion, kubernetesVersion)
+func IsBuiltInSupported(kubernetesVersion string, isManagedNodeGroups bool) (bool, error) {
+	minVersion, err := version.IsMinVersion(api.EFABuiltInSupportVersion, kubernetesVersion)
 	if err != nil {
 		return false, fmt.Errorf("failed to determine EFA built-in support for Kubernetes version %q (minimum required: %s): %w",
 			kubernetesVersion, api.EFABuiltInSupportVersion, err)
 	}
-	return supported, nil
+	return minVersion && isManagedNodeGroups, nil
 }
 
 // SecurityGroupConfig holds the configuration needed for EFA security group creation
@@ -30,16 +30,16 @@ type SecurityGroupConfig struct {
 
 // ProcessSecurityGroup handles the common EFA security group logic
 // Returns the security group (nil if built-in EFA is used) and any error
-func ProcessSecurityGroup(config SecurityGroupConfig, addEFASecurityGroupFunc func(*gfnt.Value, string, string) *gfnt.Value) (*gfnt.Value, error) {
-	supported, err := IsBuiltInSupported(config.ClusterVersion)
+func ProcessSecurityGroup(config SecurityGroupConfig, addEFASecurityGroupFunc func(*gfnt.Value, string, string) *gfnt.Value, isManagedNodeGroups bool) (*gfnt.Value, error) {
+	supported, err := IsBuiltInSupported(config.ClusterVersion, isManagedNodeGroups)
 	if err != nil {
 		logger.Warning("failed to parse Kubernetes version %s for EFA configuration: %v; falling back to custom EFA security group creation", config.ClusterVersion, err)
 		// Fall back to creating custom EFA security group when version parsing fails
 		supported = false
 	}
 
 	if !supported {
-		logger.Info("creating custom EFA security group for nodegroup %s with Kubernetes %s (EFA built-in support requires version 1.33+)", config.NodeGroupName, config.ClusterVersion)
+		logger.Info("creating custom EFA security group for nodegroup %s with Kubernetes %s (EFA built-in support requires version 1.33+ and managed-node groups)", config.NodeGroupName, config.ClusterVersion)
 		efaSG := addEFASecurityGroupFunc(config.VPCID, config.ClusterName, config.Description)
 		if efaSG == nil {
 			return nil, fmt.Errorf("failed to create EFA security group for nodegroup %s with Kubernetes %s: invalid VPC ID or cluster configuration", config.NodeGroupName, config.ClusterVersion)
@@ -48,6 +48,6 @@ func ProcessSecurityGroup(config SecurityGroupConfig, addEFASecurityGroupFunc fu
 		return efaSG, nil
 	}
 
-	logger.Info("using built-in EFA support in default security group for nodegroup %s with Kubernetes %s (no custom EFA security group needed)", config.NodeGroupName, config.ClusterVersion)
+	logger.Info("using built-in EFA support in default security group for managed nodegroup %s with Kubernetes %s (no custom EFA security group needed)", config.NodeGroupName, config.ClusterVersion)
 	return nil, nil
 }

pkg/utils/efa/efa_test.go

@@ -12,7 +12,7 @@ var _ = Describe("EFA Utils", func() {
 	Describe("IsBuiltInSupported", func() {
 		DescribeTable("EFA built-in support version detection",
 			func(kubernetesVersion string, expected bool, expectError bool) {
-				result, err := efa.IsBuiltInSupported(kubernetesVersion)
+				result, err := efa.IsBuiltInSupported(kubernetesVersion, true)
 				if expectError {
 					Expect(err).To(HaveOccurred())
 				} else {
@@ -42,22 +42,21 @@ var _ = Describe("EFA Utils", func() {
 
 		Context("Test error handling", func() {
 			It("should provide detailed error context for invalid versions", func() {
-				_, err := efa.IsBuiltInSupported("invalid.version")
+				_, err := efa.IsBuiltInSupported("invalid.version", true)
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring("failed to determine EFA built-in support"))
 				Expect(err.Error()).To(ContainSubstring("invalid.version"))
 				Expect(err.Error()).To(ContainSubstring("minimum required: 1.33"))
 			})
 
-			It("should provide detailed error context for empty versions", func() {
-				_, err := efa.IsBuiltInSupported("")
-				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("failed to determine EFA built-in support"))
-				Expect(err.Error()).To(ContainSubstring("minimum required: 1.33"))
+			It("should return false if managed node groups is false", func() {
+				result, err := efa.IsBuiltInSupported("1.34", false)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result).To(BeFalse())
 			})
 
 			It("should provide detailed error context for malformed versions", func() {
-				_, err := efa.IsBuiltInSupported("1.33.invalid.extra")
+				_, err := efa.IsBuiltInSupported("1.33.invalid.extra", true)
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring("failed to determine EFA built-in support"))
 				Expect(err.Error()).To(ContainSubstring("1.33.invalid.extra"))