Skip to content

Commit

Permalink
chore: Update associatePublicIPAddress to not be set in the launch …
Browse files Browse the repository at this point in the history
…templets (#6159)
  • Loading branch information
engedaam authored May 14, 2024
1 parent 9751fd0 commit 47cc587
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 20 deletions.
4 changes: 2 additions & 2 deletions pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ const (
// InstanceProfileTTL is the time before we refresh checking instance profile existence at IAM
InstanceProfileTTL = 15 * time.Minute
// AvailableIPAddressTTL is time to drop AvailableIPAddress data if it is not updated within the TTL
AvailableIPAddressTTL = 2 * time.Minute
AvailableIPAddressTTL = 5 * time.Minute
// AvailableIPAddressTTL is time to drop AssociatePublicIPAddressTTL data if it is not updated within the TTL
AssociatePublicIPAddressTTL = 2 * time.Minute
AssociatePublicIPAddressTTL = 5 * time.Minute
)

const (
Expand Down
6 changes: 2 additions & 4 deletions pkg/providers/launchtemplate/launchtemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,13 @@ func (p *DefaultProvider) createAMIOptions(ctx context.Context, nodeClass *v1bet
}
if nodeClass.Spec.AssociatePublicIPAddress != nil {
options.AssociatePublicIPAddress = nodeClass.Spec.AssociatePublicIPAddress
} else if ok, err := p.subnetProvider.CheckAnyPublicIPAssociations(ctx, nodeClass); err != nil {
return nil, err
} else if !ok {
} 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 = aws.Bool(false)
options.AssociatePublicIPAddress = p.subnetProvider.AssociatePublicIPAddressValue(nodeClass)
}
return options, nil
}
Expand Down
15 changes: 9 additions & 6 deletions pkg/providers/subnet/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
type Provider interface {
LivenessProbe(*http.Request) error
List(context.Context, *v1beta1.EC2NodeClass) ([]*ec2.Subnet, error)
CheckAnyPublicIPAssociations(context.Context, *v1beta1.EC2NodeClass) (bool, 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)
}
Expand Down Expand Up @@ -116,14 +116,17 @@ func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeCl
return lo.Values(subnets), nil
}

// CheckAnyPublicIPAssociations returns a bool indicating whether all referenced subnets assign public IPv4 addresses to EC2 instances created therein
func (p *DefaultProvider) CheckAnyPublicIPAssociations(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (bool, error) {
// 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 {
if subnetAssociatePublicIP, ok := p.associatePublicIPAddressCache.Get(subnet.ID); ok && subnetAssociatePublicIP.(bool) {
return true, nil
subnetAssociatePublicIP, ok := p.associatePublicIPAddressCache.Get(subnet.ID)
if !ok || subnetAssociatePublicIP.(bool) {
return nil
}
}
return false, 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
Expand Down
32 changes: 24 additions & 8 deletions pkg/providers/subnet/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,19 +219,20 @@ var _ = Describe("SubnetProvider", func() {
}, subnets)
})
})
Context("CheckAnyPublicIPAssociations", func() {
It("should note that no subnets assign a public IPv4 address to EC2 instances on launch", func() {
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"},
},
}
onlyPrivate, err := awsEnv.SubnetProvider.CheckAnyPublicIPAssociations(ctx, nodeClass)
_, err := awsEnv.SubnetProvider.List(ctx, nodeClass)
Expect(err).To(BeNil())
Expect(onlyPrivate).To(BeFalse())
associatePublicIP := awsEnv.SubnetProvider.AssociatePublicIPAddressValue(nodeClass)
Expect(lo.FromPtr(associatePublicIP)).To(BeFalse())
})
It("should note that at least one subnet assigns a public IPv4 address to EC2instances on launch", func() {
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",
Expand All @@ -245,9 +246,24 @@ var _ = Describe("SubnetProvider", func() {
}
_, err := awsEnv.SubnetProvider.List(ctx, nodeClass)
Expect(err).To(BeNil())
onlyPrivate, err := awsEnv.SubnetProvider.CheckAnyPublicIPAssociations(ctx, nodeClass)
Expect(err).To(BeNil())
Expect(onlyPrivate).To(BeTrue())
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() {
Expand Down

0 comments on commit 47cc587

Please sign in to comment.