Skip to content

Commit

Permalink
Support adding custom secondary VPC CIDR blocks in AWSCluster
Browse files Browse the repository at this point in the history
  • Loading branch information
AndiDog committed Apr 22, 2024
1 parent ad71a54 commit 9b917e8
Show file tree
Hide file tree
Showing 22 changed files with 510 additions and 93 deletions.
1 change: 1 addition & 0 deletions api/v1beta1/awscluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {

dst.Spec.NetworkSpec.VPC.EmptyRoutesDefaultVPCSecurityGroup = restored.Spec.NetworkSpec.VPC.EmptyRoutesDefaultVPCSecurityGroup
dst.Spec.NetworkSpec.VPC.PrivateDNSHostnameTypeOnLaunch = restored.Spec.NetworkSpec.VPC.PrivateDNSHostnameTypeOnLaunch
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.

8 changes: 8 additions & 0 deletions api/v1beta2/awscluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,14 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
}
}

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
24 changes: 24 additions & 0 deletions api/v1beta2/network_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,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 @@ -308,6 +315,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 @@ -510,6 +523,17 @@ func (s Subnets) FilterPrivate() (res Subnets) {
return
}

// 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)
}
}
return
}

// FilterPublic returns a slice containing all subnets marked as public.
func (s Subnets) FilterPublic() (res Subnets) {
for _, x := range s {
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 @@ -660,6 +660,26 @@ spec:
- ip-name
- resource-name
type: string
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 @@ -2512,6 +2532,26 @@ spec:
- ip-name
- resource-name
type: string
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 @@ -1494,6 +1494,26 @@ spec:
- ip-name
- resource-name
type: string
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 @@ -1119,6 +1119,27 @@ spec:
- ip-name
- resource-name
type: string
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 @@ -164,7 +164,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 @@ -406,23 +406,53 @@ func (r *AWSManagedControlPlane) validatePrivateDNSHostnameTypeOnLaunch() field.
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 @@ -153,6 +153,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
Loading

0 comments on commit 9b917e8

Please sign in to comment.