Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support adding custom secondary VPC CIDR blocks in AWSCluster (backport) #590

Merged
merged 1 commit into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/v1beta1/awscluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {
}

dst.Spec.NetworkSpec.VPC.EmptyRoutesDefaultVPCSecurityGroup = restored.Spec.NetworkSpec.VPC.EmptyRoutesDefaultVPCSecurityGroup
dst.Spec.NetworkSpec.VPC.SecondaryCidrBlocks = restored.Spec.NetworkSpec.VPC.SecondaryCidrBlocks

// Restore SubnetSpec.ResourceID field, if any.
for _, subnet := range restored.Spec.NetworkSpec.Subnets {
Expand Down
1 change: 1 addition & 0 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions api/v1beta2/awscluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,15 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
allErrs = append(allErrs, field.Invalid(field.NewPath("additionalControlPlaneIngressRules"), r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together"))
}
}

secondaryCidrBlocks := r.Spec.NetworkSpec.VPC.SecondaryCidrBlocks
secondaryCidrBlocksField := field.NewPath("spec", "network", "vpc", "secondaryCidrBlocks")
for _, cidrBlock := range secondaryCidrBlocks {
if r.Spec.NetworkSpec.VPC.CidrBlock != "" && r.Spec.NetworkSpec.VPC.CidrBlock == cidrBlock.IPv4CidrBlock {
allErrs = append(allErrs, field.Invalid(secondaryCidrBlocksField, secondaryCidrBlocks, fmt.Sprintf("AWSCluster.spec.network.vpc.secondaryCidrBlocks must not contain the primary AWSCluster.spec.network.vpc.cidrBlock %v", r.Spec.NetworkSpec.VPC.CidrBlock)))
}
}

return allErrs
}

Expand Down
20 changes: 16 additions & 4 deletions api/v1beta2/network_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,13 @@ type IPAMPool struct {
NetmaskLength int64 `json:"netmaskLength,omitempty"`
}

// VpcCidrBlock defines the CIDR block and settings to associate with the managed VPC. Currently, only IPv4 is supported.
type VpcCidrBlock struct {
// IPv4CidrBlock is the IPv4 CIDR block to associate with the managed VPC.
// +kubebuilder:validation:MinLength=1
IPv4CidrBlock string `json:"ipv4CidrBlock"`
}

// VPCSpec configures an AWS VPC.
type VPCSpec struct {
// ID is the vpc-id of the VPC this provider should use to create resources.
Expand All @@ -289,6 +296,12 @@ type VPCSpec struct {
// Mutually exclusive with IPAMPool.
CidrBlock string `json:"cidrBlock,omitempty"`

// SecondaryCidrBlocks are additional CIDR blocks to be associated when the provider creates a managed VPC.
// Defaults to none. Mutually exclusive with IPAMPool. This makes sense to use if, for example, you want to use
// a separate IP range for pods (e.g. Cilium ENI mode).
// +optional
SecondaryCidrBlocks []VpcCidrBlock `json:"secondaryCidrBlocks,omitempty"`

// IPAMPool defines the IPAMv4 pool to be used for VPC.
// Mutually exclusive with CidrBlock.
IPAMPool *IPAMPool `json:"ipamPool,omitempty"`
Expand Down Expand Up @@ -478,10 +491,9 @@ func (s Subnets) FilterPrivate() (res Subnets) {
return
}

// FilterPrimary returns a slice containing all subnets that do not have the
// sigs.k8s.io/cluster-api-provider-aws/association: secondary tag. These
// subnets are intended for the CNI.
func (s Subnets) FilterPrimary() (res Subnets) {
// FilterNonCni returns the subnets that are NOT intended for usage with the CNI pod network
// (i.e. do NOT have the `sigs.k8s.io/cluster-api-provider-aws/association=secondary` tag).
func (s Subnets) FilterNonCni() (res Subnets) {
for _, x := range s {
if x.Tags[NameAWSSubnetAssociation] != SecondarySubnetTagValue {
res = append(res, x)
Expand Down
20 changes: 20 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,26 @@ spec:
is set. Mutually exclusive with IPAMPool.
type: string
type: object
secondaryCidrBlocks:
description: SecondaryCidrBlocks are additional CIDR blocks
to be associated when the provider creates a managed VPC.
Defaults to none. Mutually exclusive with IPAMPool. This
makes sense to use if, for example, you want to use a separate
IP range for pods (e.g. Cilium ENI mode).
items:
description: VpcCidrBlock defines the CIDR block and settings
to associate with the managed VPC. Currently, only IPv4
is supported.
properties:
ipv4CidrBlock:
description: IPv4CidrBlock is the IPv4 CIDR block to
associate with the managed VPC.
minLength: 1
type: string
required:
- ipv4CidrBlock
type: object
type: array
tags:
additionalProperties:
type: string
Expand Down Expand Up @@ -2244,6 +2264,26 @@ spec:
is set. Mutually exclusive with IPAMPool.
type: string
type: object
secondaryCidrBlocks:
description: SecondaryCidrBlocks are additional CIDR blocks
to be associated when the provider creates a managed VPC.
Defaults to none. Mutually exclusive with IPAMPool. This
makes sense to use if, for example, you want to use a separate
IP range for pods (e.g. Cilium ENI mode).
items:
description: VpcCidrBlock defines the CIDR block and settings
to associate with the managed VPC. Currently, only IPv4
is supported.
properties:
ipv4CidrBlock:
description: IPv4CidrBlock is the IPv4 CIDR block to
associate with the managed VPC.
minLength: 1
type: string
required:
- ipv4CidrBlock
type: object
type: array
tags:
additionalProperties:
type: string
Expand Down
20 changes: 20 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,26 @@ spec:
is set. Mutually exclusive with IPAMPool.
type: string
type: object
secondaryCidrBlocks:
description: SecondaryCidrBlocks are additional CIDR blocks
to be associated when the provider creates a managed VPC.
Defaults to none. Mutually exclusive with IPAMPool. This
makes sense to use if, for example, you want to use a separate
IP range for pods (e.g. Cilium ENI mode).
items:
description: VpcCidrBlock defines the CIDR block and settings
to associate with the managed VPC. Currently, only IPv4
is supported.
properties:
ipv4CidrBlock:
description: IPv4CidrBlock is the IPv4 CIDR block to
associate with the managed VPC.
minLength: 1
type: string
required:
- ipv4CidrBlock
type: object
type: array
tags:
additionalProperties:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,27 @@ spec:
with IPAMPool.
type: string
type: object
secondaryCidrBlocks:
description: SecondaryCidrBlocks are additional CIDR
blocks to be associated when the provider creates
a managed VPC. Defaults to none. Mutually exclusive
with IPAMPool. This makes sense to use if, for example,
you want to use a separate IP range for pods (e.g.
Cilium ENI mode).
items:
description: VpcCidrBlock defines the CIDR block
and settings to associate with the managed VPC.
Currently, only IPv4 is supported.
properties:
ipv4CidrBlock:
description: IPv4CidrBlock is the IPv4 CIDR
block to associate with the managed VPC.
minLength: 1
type: string
required:
- ipv4CidrBlock
type: object
type: array
tags:
additionalProperties:
type: string
Expand Down
40 changes: 35 additions & 5 deletions controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (r *AWSManagedControlPlane) ValidateUpdate(old runtime.Object) (admission.W

if oldAWSManagedControlplane.Spec.NetworkSpec.VPC.IsIPv6Enabled() != r.Spec.NetworkSpec.VPC.IsIPv6Enabled() {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "networkSpec", "vpc", "enableIPv6"), r.Spec.NetworkSpec.VPC.IsIPv6Enabled(), "changing IP family is not allowed after it has been set"))
field.Invalid(field.NewPath("spec", "network", "vpc", "enableIPv6"), r.Spec.NetworkSpec.VPC.IsIPv6Enabled(), "changing IP family is not allowed after it has been set"))
}

if len(allErrs) == 0 {
Expand Down Expand Up @@ -392,23 +392,53 @@ func (r *AWSManagedControlPlane) validateDisableVPCCNI() field.ErrorList {
func (r *AWSManagedControlPlane) validateNetwork() field.ErrorList {
var allErrs field.ErrorList

// If only `AWSManagedControlPlane.spec.secondaryCidrBlock` is set, no additional checks are done to remain
// backward-compatible. The `VPCSpec.SecondaryCidrBlocks` field was added later - if that list is not empty, we
// require `AWSManagedControlPlane.spec.secondaryCidrBlock` to be listed in there as well. This may allow merging
// the fields later on.
podSecondaryCidrBlock := r.Spec.SecondaryCidrBlock
secondaryCidrBlocks := r.Spec.NetworkSpec.VPC.SecondaryCidrBlocks
secondaryCidrBlocksField := field.NewPath("spec", "network", "vpc", "secondaryCidrBlocks")
if podSecondaryCidrBlock != nil && len(secondaryCidrBlocks) > 0 {
found := false
for _, cidrBlock := range secondaryCidrBlocks {
if cidrBlock.IPv4CidrBlock == *podSecondaryCidrBlock {
found = true
break
}
}
if !found {
allErrs = append(allErrs, field.Invalid(secondaryCidrBlocksField, secondaryCidrBlocks, fmt.Sprintf("AWSManagedControlPlane.spec.secondaryCidrBlock %v must be listed in AWSManagedControlPlane.spec.network.vpc.secondaryCidrBlocks (required if both fields are filled)", *podSecondaryCidrBlock)))
}
}

if podSecondaryCidrBlock != nil && r.Spec.NetworkSpec.VPC.CidrBlock != "" && r.Spec.NetworkSpec.VPC.CidrBlock == *podSecondaryCidrBlock {
secondaryCidrBlockField := field.NewPath("spec", "vpc", "secondaryCidrBlock")
allErrs = append(allErrs, field.Invalid(secondaryCidrBlockField, secondaryCidrBlocks, fmt.Sprintf("AWSManagedControlPlane.spec.secondaryCidrBlock %v must not be equal to the primary AWSManagedControlPlane.spec.network.vpc.cidrBlock", *podSecondaryCidrBlock)))
}
for _, cidrBlock := range secondaryCidrBlocks {
if r.Spec.NetworkSpec.VPC.CidrBlock != "" && r.Spec.NetworkSpec.VPC.CidrBlock == cidrBlock.IPv4CidrBlock {
allErrs = append(allErrs, field.Invalid(secondaryCidrBlocksField, secondaryCidrBlocks, fmt.Sprintf("AWSManagedControlPlane.spec.network.vpc.secondaryCidrBlocks must not contain the primary AWSManagedControlPlane.spec.network.vpc.cidrBlock %v", r.Spec.NetworkSpec.VPC.CidrBlock)))
}
}

if r.Spec.NetworkSpec.VPC.IsIPv6Enabled() && r.Spec.NetworkSpec.VPC.IPv6.CidrBlock != "" && r.Spec.NetworkSpec.VPC.IPv6.PoolID == "" {
poolField := field.NewPath("spec", "networkSpec", "vpc", "ipv6", "poolId")
poolField := field.NewPath("spec", "network", "vpc", "ipv6", "poolId")
allErrs = append(allErrs, field.Invalid(poolField, r.Spec.NetworkSpec.VPC.IPv6.PoolID, "poolId cannot be empty if cidrBlock is set"))
}

if r.Spec.NetworkSpec.VPC.IsIPv6Enabled() && r.Spec.NetworkSpec.VPC.IPv6.PoolID != "" && r.Spec.NetworkSpec.VPC.IPv6.IPAMPool != nil {
poolField := field.NewPath("spec", "networkSpec", "vpc", "ipv6", "poolId")
poolField := field.NewPath("spec", "network", "vpc", "ipv6", "poolId")
allErrs = append(allErrs, field.Invalid(poolField, r.Spec.NetworkSpec.VPC.IPv6.PoolID, "poolId and ipamPool cannot be used together"))
}

if r.Spec.NetworkSpec.VPC.IsIPv6Enabled() && r.Spec.NetworkSpec.VPC.IPv6.CidrBlock != "" && r.Spec.NetworkSpec.VPC.IPv6.IPAMPool != nil {
cidrBlockField := field.NewPath("spec", "networkSpec", "vpc", "ipv6", "cidrBlock")
cidrBlockField := field.NewPath("spec", "network", "vpc", "ipv6", "cidrBlock")
allErrs = append(allErrs, field.Invalid(cidrBlockField, r.Spec.NetworkSpec.VPC.IPv6.CidrBlock, "cidrBlock and ipamPool cannot be used together"))
}

if r.Spec.NetworkSpec.VPC.IsIPv6Enabled() && r.Spec.NetworkSpec.VPC.IPv6.IPAMPool != nil && r.Spec.NetworkSpec.VPC.IPv6.IPAMPool.ID == "" && r.Spec.NetworkSpec.VPC.IPv6.IPAMPool.Name == "" {
ipamPoolField := field.NewPath("spec", "networkSpec", "vpc", "ipv6", "ipamPool")
ipamPoolField := field.NewPath("spec", "network", "vpc", "ipv6", "ipamPool")
allErrs = append(allErrs, field.Invalid(ipamPoolField, r.Spec.NetworkSpec.VPC.IPv6.IPAMPool, "ipamPool must have either id or name"))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,17 @@ func TestDefaultingWebhook(t *testing.T) {

func TestWebhookCreate(t *testing.T) {
tests := []struct { //nolint:maligned
name string
eksClusterName string
expectError bool
eksVersion string
hasAddons bool
vpcCNI VpcCni
additionalTags infrav1.Tags
secondaryCidr *string
kubeProxy KubeProxy
name string
eksClusterName string
expectError bool
expectErrorToContain string // if non-empty, the error message must contain this substring
eksVersion string
hasAddons bool
vpcCNI VpcCni
additionalTags infrav1.Tags
secondaryCidr *string
secondaryCidrBlocks []infrav1.VpcCidrBlock
kubeProxy KubeProxy
}{
{
name: "ekscluster specified",
Expand Down Expand Up @@ -253,6 +255,15 @@ func TestWebhookCreate(t *testing.T) {
vpcCNI: VpcCni{Disable: true},
secondaryCidr: aws.String("100.64.0.0/10"),
},
{
name: "secondary CIDR block not listed in NetworkSpec.VPC.SecondaryCidrBlocks",
eksClusterName: "default_cluster1",
eksVersion: "v1.19",
expectError: true,
expectErrorToContain: "100.64.0.0/16 must be listed in AWSManagedControlPlane.spec.network.vpc.secondaryCidrBlocks",
secondaryCidr: aws.String("100.64.0.0/16"),
secondaryCidrBlocks: []infrav1.VpcCidrBlock{{IPv4CidrBlock: "123.456.0.0/16"}},
},
{
name: "invalid tags not allowed",
eksClusterName: "default_cluster1",
Expand Down Expand Up @@ -327,6 +338,11 @@ func TestWebhookCreate(t *testing.T) {
KubeProxy: tc.kubeProxy,
AdditionalTags: tc.additionalTags,
VpcCni: tc.vpcCNI,
NetworkSpec: infrav1.NetworkSpec{
VPC: infrav1.VPCSpec{
SecondaryCidrBlocks: tc.secondaryCidrBlocks,
},
},
},
}
if tc.eksVersion != "" {
Expand All @@ -353,7 +369,16 @@ func TestWebhookCreate(t *testing.T) {

if tc.expectError {
g.Expect(err).ToNot(BeNil())

if tc.expectErrorToContain != "" && err != nil {
g.Expect(err.Error()).To(ContainSubstring(tc.expectErrorToContain))
}
} else {
if tc.expectErrorToContain != "" {
t.Error("Logic error: expectError=false means that expectErrorToContain must be empty")
t.FailNow()
}

g.Expect(err).To(BeNil())
}
})
Expand Down
11 changes: 11 additions & 0 deletions pkg/cloud/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,17 @@ func (s *ClusterScope) SecondaryCidrBlock() *string {
return nil
}

// SecondaryCidrBlocks returns the additional CIDR blocks to be associated with the managed VPC.
func (s *ClusterScope) SecondaryCidrBlocks() []infrav1.VpcCidrBlock {
return s.AWSCluster.Spec.NetworkSpec.VPC.SecondaryCidrBlocks
}

// AllSecondaryCidrBlocks returns all secondary CIDR blocks (combining `SecondaryCidrBlock` and `SecondaryCidrBlocks`).
func (s *ClusterScope) AllSecondaryCidrBlocks() []infrav1.VpcCidrBlock {
// Non-EKS clusters don't have anything in `SecondaryCidrBlock()`
return s.SecondaryCidrBlocks()
}

// Name returns the CAPI cluster name.
func (s *ClusterScope) Name() string {
return s.Cluster.Name
Expand Down
22 changes: 22 additions & 0 deletions pkg/cloud/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,28 @@ func (s *ManagedControlPlaneScope) SecondaryCidrBlock() *string {
return s.ControlPlane.Spec.SecondaryCidrBlock
}

// SecondaryCidrBlocks returns the additional CIDR blocks to be associated with the managed VPC.
func (s *ManagedControlPlaneScope) SecondaryCidrBlocks() []infrav1.VpcCidrBlock {
return s.ControlPlane.Spec.NetworkSpec.VPC.SecondaryCidrBlocks
}

// AllSecondaryCidrBlocks returns all secondary CIDR blocks (combining `SecondaryCidrBlock` and `SecondaryCidrBlocks`).
func (s *ManagedControlPlaneScope) AllSecondaryCidrBlocks() []infrav1.VpcCidrBlock {
secondaryCidrBlocks := s.ControlPlane.Spec.NetworkSpec.VPC.SecondaryCidrBlocks

// If only `AWSManagedControlPlane.spec.secondaryCidrBlock` is set, no additional checks are done to remain
// backward-compatible. The `VPCSpec.SecondaryCidrBlocks` field was added later - if that list is not empty, we
// require `AWSManagedControlPlane.spec.secondaryCidrBlock` to be listed in there as well (validation done in
// webhook).
if s.ControlPlane.Spec.SecondaryCidrBlock != nil && len(secondaryCidrBlocks) == 0 {
secondaryCidrBlocks = []infrav1.VpcCidrBlock{{
IPv4CidrBlock: *s.ControlPlane.Spec.SecondaryCidrBlock,
}}
}

return secondaryCidrBlocks
}

// SecurityGroupOverrides returns the security groups that are overrides in the ControlPlane spec.
func (s *ManagedControlPlaneScope) SecurityGroupOverrides() map[infrav1.SecurityGroupRole]string {
return s.ControlPlane.Spec.NetworkSpec.SecurityGroupOverrides
Expand Down
Loading
Loading