chore: drop implicit public IP association config (aws#6213)

rschalo · May 17, 2024 · c423767 · c423767
1 parent c751b86
commit c423767
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 112 deletions.
diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go
@@ -173,30 +173,20 @@ func (p *DefaultProvider) createAMIOptions(ctx context.Context, nodeClass *v1bet
 	if len(nodeClass.Status.SecurityGroups) == 0 {
 		return nil, fmt.Errorf("no security groups are present in the status")
 	}
-	options := &amifamily.Options{
-		ClusterName:         options.FromContext(ctx).ClusterName,
-		ClusterEndpoint:     p.ClusterEndpoint,
-		ClusterCIDR:         p.ClusterCIDR.Load(),
-		InstanceProfile:     instanceProfile,
-		InstanceStorePolicy: nodeClass.Spec.InstanceStorePolicy,
-		SecurityGroups:      nodeClass.Status.SecurityGroups,
-		Tags:                tags,
-		Labels:              labels,
-		CABundle:            p.CABundle,
-		KubeDNSIP:           p.KubeDNSIP,
-		NodeClassName:       nodeClass.Name,
-	}
-	if nodeClass.Spec.AssociatePublicIPAddress != nil {
-		options.AssociatePublicIPAddress = nodeClass.Spec.AssociatePublicIPAddress
-	} else {
-		// when `AssociatePublicIPAddress` is not specified in the `EC2NodeClass` spec,
-		// If all referenced subnets do not assign public IPv4 addresses to EC2 instances therein, we explicitly set
-		// AssociatePublicIPAddress to 'false' in the Launch Template, generated based on this configuration struct.
-		// This is done to help comply with AWS account policies that require explicitly setting of that field to 'false'.
-		// https://github.com/aws/karpenter-provider-aws/issues/3815
-		options.AssociatePublicIPAddress = p.subnetProvider.AssociatePublicIPAddressValue(nodeClass)
-	}
-	return options, nil
+	return &amifamily.Options{
+		ClusterName:              options.FromContext(ctx).ClusterName,
+		ClusterEndpoint:          p.ClusterEndpoint,
+		ClusterCIDR:              p.ClusterCIDR.Load(),
+		InstanceProfile:          instanceProfile,
+		InstanceStorePolicy:      nodeClass.Spec.InstanceStorePolicy,
+		SecurityGroups:           nodeClass.Status.SecurityGroups,
+		Tags:                     tags,
+		Labels:                   labels,
+		CABundle:                 p.CABundle,
+		KubeDNSIP:                p.KubeDNSIP,
+		AssociatePublicIPAddress: nodeClass.Spec.AssociatePublicIPAddress,
+		NodeClassName:            nodeClass.Name,
+	}, nil
 }
 
 func (p *DefaultProvider) ensureLaunchTemplate(ctx context.Context, options *amifamily.LaunchTemplate) (*ec2.LaunchTemplate, error) {

diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go
@@ -1973,33 +1973,6 @@ var _ = Describe("LaunchTemplate Provider", func() {
 			})
 		})
 		Context("Public IP Association", func() {
-			It("should explicitly set 'AssociatePublicIPAddress' to false in the Launch Template", func() {
-				nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{
-					{Tags: map[string]string{"Name": "test-subnet-1"}},
-					{Tags: map[string]string{"Name": "test-subnet-3"}},
-				}
-				ExpectApplied(ctx, env.Client, nodePool, nodeClass)
-				controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
-				ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
-				pod := coretest.UnschedulablePod()
-				ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
-				ExpectScheduled(ctx, env.Client, pod)
-				input := awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Pop()
-				Expect(*input.LaunchTemplateData.NetworkInterfaces[0].AssociatePublicIpAddress).To(BeFalse())
-			})
-			It("should not explicitly set 'AssociatePublicIPAddress' when the subnets are configured to assign public IPv4 addresses", func() {
-				nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{
-					{Tags: map[string]string{"Name": "test-subnet-2"}},
-				}
-				ExpectApplied(ctx, env.Client, nodePool, nodeClass)
-				controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
-				ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
-				pod := coretest.UnschedulablePod()
-				ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
-				ExpectScheduled(ctx, env.Client, pod)
-				input := awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Pop()
-				Expect(len(input.LaunchTemplateData.NetworkInterfaces)).To(BeNumerically("==", 0))
-			})
 			DescribeTable(
 				"should set 'AssociatePublicIPAddress' based on EC2NodeClass",
 				func(setValue, expectedValue, isEFA bool) {

diff --git a/pkg/providers/subnet/subnet.go b/pkg/providers/subnet/subnet.go
@@ -37,7 +37,6 @@ import (
 type Provider interface {
 	LivenessProbe(*http.Request) error
 	List(context.Context, *v1beta1.EC2NodeClass) ([]*ec2.Subnet, error)
-	AssociatePublicIPAddressValue(*v1beta1.EC2NodeClass) *bool
 	ZonalSubnetsForLaunch(context.Context, *v1beta1.EC2NodeClass, []*cloudprovider.InstanceType, string) (map[string]*Subnet, error)
 	UpdateInflightIPs(*ec2.CreateFleetInput, *ec2.CreateFleetOutput, []*cloudprovider.InstanceType, []*Subnet, string)
 }
@@ -116,19 +115,6 @@ func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeCl
 	return lo.Values(subnets), nil
 }
 
-// associatePublicIPAddressValue validates whether we know the association value for all subnets AND
-// that all subnets don't have associatePublicIP set. If both of these are true, we set the value explicitly to false
-// For more detail see: https://github.com/aws/karpenter-provider-aws/pull/3814
-func (p *DefaultProvider) AssociatePublicIPAddressValue(nodeClass *v1beta1.EC2NodeClass) *bool {
-	for _, subnet := range nodeClass.Status.Subnets {
-		subnetAssociatePublicIP, ok := p.associatePublicIPAddressCache.Get(subnet.ID)
-		if !ok || subnetAssociatePublicIP.(bool) {
-			return nil
-		}
-	}
-	return lo.ToPtr(false)
-}
-
 // ZonalSubnetsForLaunch returns a mapping of zone to the subnet with the most available IP addresses and deducts the passed ips from the available count
 func (p *DefaultProvider) ZonalSubnetsForLaunch(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, instanceTypes []*cloudprovider.InstanceType, capacityType string) (map[string]*Subnet, error) {
 	if len(nodeClass.Status.Subnets) == 0 {

diff --git a/pkg/providers/subnet/suite_test.go b/pkg/providers/subnet/suite_test.go
@@ -219,53 +219,6 @@ var _ = Describe("SubnetProvider", func() {
 			}, subnets)
 		})
 	})
-	Context("AssociatePublicIPAddress", func() {
-		It("should be false when no subnets assign a public IPv4 address to EC2 instances on launch", func() {
-			nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{
-				{
-					ID:   "subnet-test1",
-					Tags: map[string]string{"foo": "bar"},
-				},
-			}
-			_, err := awsEnv.SubnetProvider.List(ctx, nodeClass)
-			Expect(err).To(BeNil())
-			associatePublicIP := awsEnv.SubnetProvider.AssociatePublicIPAddressValue(nodeClass)
-			Expect(lo.FromPtr(associatePublicIP)).To(BeFalse())
-		})
-		It("should be nil when at least one subnet assigns a public IPv4 address to EC2instances on launch", func() {
-			nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{
-				{
-					ID: "subnet-test2",
-				},
-			}
-			nodeClass.Status.Subnets = []v1beta1.Subnet{
-				{
-					ID:   "subnet-test2",
-					Zone: "test-zone-1b",
-				},
-			}
-			_, err := awsEnv.SubnetProvider.List(ctx, nodeClass)
-			Expect(err).To(BeNil())
-			associatePublicIP := awsEnv.SubnetProvider.AssociatePublicIPAddressValue(nodeClass)
-			Expect(associatePublicIP).To(BeNil())
-		})
-		It("should be nil when no subnet data is present in the provider cache", func() {
-			nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{
-				{
-					ID: "subnet-test2",
-				},
-			}
-			nodeClass.Status.Subnets = []v1beta1.Subnet{
-				{
-					ID:   "subnet-test2",
-					Zone: "test-zone-1b",
-				},
-			}
-			awsEnv.SubnetCache.Flush() // remove any subnet data that might be in the subnetCache
-			associatePublicIP := awsEnv.SubnetProvider.AssociatePublicIPAddressValue(nodeClass)
-			Expect(associatePublicIP).To(BeNil())
-		})
-	})
 	Context("Provider Cache", func() {
 		It("should resolve subnets from cache that are filtered by id", func() {
 			expectedSubnets := awsEnv.EC2API.DescribeSubnetsOutput.Clone().Subnets