Feature #4784: SubnetSchema support

krasoffski · krasoffski · commit 9534ec4be53b · 2024-07-18T12:52:28.000+02:00
diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go
@@ -104,6 +104,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.CarrierGatewayID = restored.Spec.NetworkSpec.VPC.CarrierGatewayID
+	dst.Spec.NetworkSpec.VPC.SubnetSchema = restored.Spec.NetworkSpec.VPC.SubnetSchema
 
 	if restored.Spec.NetworkSpec.VPC.ElasticIPPool != nil {
 		if dst.Spec.NetworkSpec.VPC.ElasticIPPool == nil {
diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go
diff --git a/api/v1beta2/awscluster_webhook_test.go b/api/v1beta2/awscluster_webhook_test.go
@@ -1062,6 +1062,7 @@ func TestAWSClusterDefaultCNIIngressRules(t *testing.T) {
 	defaultVPCSpec := VPCSpec{
 		AvailabilityZoneUsageLimit: &AZUsageLimit,
 		AvailabilityZoneSelection:  &AZSelectionSchemeOrdered,
+		SubnetSchema:               &SubnetSchemaPreferPrivate,
 	}
 	g := NewWithT(t)
 	tests := []struct {
diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go
@@ -461,6 +461,16 @@ type VPCSpec struct {
 	// the API Server.
 	// +optional
 	ElasticIPPool *ElasticIPPool `json:"elasticIpPool,omitempty"`
+
+	// SubnetSchema specifies how CidrBlock should be divided on subnets in the VPC depending on the number of AZs.
+	// PreferPrivate - one private subnet for each AZ plus one other subnet that will be further sub-divided for the public subnets.
+	// PreferPublic - have the reverse logic of PreferPrivate, one public subnet for each AZ plus one other subnet
+	// that will be further sub-divided for the private subnets.
+	// Defaults to PreferPrivate
+	// +optional
+	// +kubebuilder:default=PreferPrivate
+	// +kubebuilder:validation:Enum=PreferPrivate;PreferPublic
+	SubnetSchema *SubnetSchemaType `json:"subnetSchema,omitempty"`
 }
 
 // String returns a string representation of the VPC.
diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1beta2
 
 import (
+	"strings"
+
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -439,3 +441,19 @@ type PrivateDNSName struct {
 	// +kubebuilder:validation:Enum:=ip-name;resource-name
 	HostnameType *string `json:"hostnameType,omitempty"`
 }
+
+// SubnetSchemaType specifies how given network should be divided on subnets
+// in the VPC depending on the number of AZs.
+type SubnetSchemaType string
+
+// Name returns subnet schema type name without prefix.
+func (s *SubnetSchemaType) Name() string {
+	return strings.ToLower(strings.TrimPrefix(string(*s), "Prefer"))
+}
+
+var (
+	// SubnetSchemaPreferPrivate allocates more subnets in the VPC to private subnets.
+	SubnetSchemaPreferPrivate = SubnetSchemaType("PreferPrivate")
+	// SubnetSchemaPreferPublic allocates more subnets in the VPC to public subnets.
+	SubnetSchemaPreferPublic = SubnetSchemaType("PreferPublic")
+)
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
@@ -759,6 +759,18 @@ spec:
                         - ip-name
                         - resource-name
                         type: string
+                      subnetSchema:
+                        default: PreferPrivate
+                        description: |-
+                          SubnetSchema specifies how CidrBlock should be divided on subnets in the VPC depending on the number of AZs.
+                          PreferPrivate - one private subnet for each AZ plus one other subnet that will be further sub-divided for the public subnets.
+                          PreferPublic - have the reverse logic of PreferPrivate, one public subnet for each AZ plus one other subnet
+                          that will be further sub-divided for the private subnets.
+                          Defaults to PreferPrivate
+                        enum:
+                        - PreferPrivate
+                        - PreferPublic
+                        type: string
                       tags:
                         additionalProperties:
                           type: string
@@ -2750,6 +2762,18 @@ spec:
                         - ip-name
                         - resource-name
                         type: string
+                      subnetSchema:
+                        default: PreferPrivate
+                        description: |-
+                          SubnetSchema specifies how CidrBlock should be divided on subnets in the VPC depending on the number of AZs.
+                          PreferPrivate - one private subnet for each AZ plus one other subnet that will be further sub-divided for the public subnets.
+                          PreferPublic - have the reverse logic of PreferPrivate, one public subnet for each AZ plus one other subnet
+                          that will be further sub-divided for the private subnets.
+                          Defaults to PreferPrivate
+                        enum:
+                        - PreferPrivate
+                        - PreferPublic
+                        type: string
                       tags:
                         additionalProperties:
                           type: string
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1699,6 +1699,18 @@ spec:
                         - ip-name
                         - resource-name
                         type: string
+                      subnetSchema:
+                        default: PreferPrivate
+                        description: |-
+                          SubnetSchema specifies how CidrBlock should be divided on subnets in the VPC depending on the number of AZs.
+                          PreferPrivate - one private subnet for each AZ plus one other subnet that will be further sub-divided for the public subnets.
+                          PreferPublic - have the reverse logic of PreferPrivate, one public subnet for each AZ plus one other subnet
+                          that will be further sub-divided for the private subnets.
+                          Defaults to PreferPrivate
+                        enum:
+                        - PreferPrivate
+                        - PreferPublic
+                        type: string
                       tags:
                         additionalProperties:
                           type: string
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml
@@ -1298,6 +1298,18 @@ spec:
                                 - ip-name
                                 - resource-name
                                 type: string
+                              subnetSchema:
+                                default: PreferPrivate
+                                description: |-
+                                  SubnetSchema specifies how CidrBlock should be divided on subnets in the VPC depending on the number of AZs.
+                                  PreferPrivate - one private subnet for each AZ plus one other subnet that will be further sub-divided for the public subnets.
+                                  PreferPublic - have the reverse logic of PreferPrivate, one public subnet for each AZ plus one other subnet
+                                  that will be further sub-divided for the private subnets.
+                                  Defaults to PreferPrivate
+                                enum:
+                                - PreferPrivate
+                                - PreferPublic
+                                type: string
                               tags:
                                 additionalProperties:
                                   type: string
diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go
@@ -45,6 +45,7 @@ func TestDefaultingWebhook(t *testing.T) {
 	defaultVPCSpec := infrav1.VPCSpec{
 		AvailabilityZoneUsageLimit: &AZUsageLimit,
 		AvailabilityZoneSelection:  &infrav1.AZSelectionSchemeOrdered,
+		SubnetSchema:               &infrav1.SubnetSchemaPreferPrivate,
 	}
 	defaultIdentityRef := &infrav1.AWSIdentityReference{
 		Kind: infrav1.ControllerIdentityKind,
diff --git a/pkg/cloud/services/network/subnets.go b/pkg/cloud/services/network/subnets.go
@@ -275,26 +275,38 @@ func (s *Service) getDefaultSubnets() (infrav1.Subnets, error) {
 		s.scope.Debug("zones selected", "region", s.scope.Region(), "zones", zones)
 	}
 
-	// 1 private subnet for each AZ plus 1 other subnet that will be further sub-divided for the public subnets
+	// 1 private subnet for each AZ plus 1 other subnet that will be further sub-divided for the public subnets or vice versa if
+	// the subnet schema is set to prefer public subnets.
 	// All subnets will have an ipv4 address for now as well. We aren't supporting ipv6-only yet.
 	numSubnets := len(zones) + 1
 	var (
-		subnetCIDRs            []*net.IPNet
-		publicSubnetCIDRs      []*net.IPNet
-		ipv6SubnetCIDRs        []*net.IPNet
-		publicIPv6SubnetCIDRs  []*net.IPNet
-		privateIPv6SubnetCIDRs []*net.IPNet
+		subnetCIDRs              []*net.IPNet
+		preferredSubnetCIDRs     []*net.IPNet
+		residualSubnetCIDRs      []*net.IPNet
+		ipv6SubnetCIDRs          []*net.IPNet
+		preferredIPv6SubnetCIDRs []*net.IPNet
+		residualIPv6SubnetCIDRs  []*net.IPNet
 	)
+	subnetScheme := infrav1.SubnetSchemaPreferPrivate
+	if s.scope.VPC().SubnetSchema != nil {
+		subnetScheme = *s.scope.VPC().SubnetSchema
+	}
+
+	residualSubnetsName := infrav1.SubnetSchemaPreferPublic.Name()
+	if subnetScheme == infrav1.SubnetSchemaPreferPublic {
+		residualSubnetsName = infrav1.SubnetSchemaPreferPrivate.Name()
+	}
+
 	subnetCIDRs, err = cidr.SplitIntoSubnetsIPv4(s.scope.VPC().CidrBlock, numSubnets)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed splitting VPC CIDR %q into subnets", s.scope.VPC().CidrBlock)
 	}
 
-	publicSubnetCIDRs, err = cidr.SplitIntoSubnetsIPv4(subnetCIDRs[0].String(), len(zones))
+	residualSubnetCIDRs, err = cidr.SplitIntoSubnetsIPv4(subnetCIDRs[0].String(), len(zones))
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed splitting CIDR %q into public subnets", subnetCIDRs[0].String())
+		return nil, errors.Wrapf(err, "failed splitting CIDR %q into %s subnets", subnetCIDRs[0].String(), residualSubnetsName)
 	}
-	privateSubnetCIDRs := append(subnetCIDRs[:0], subnetCIDRs[1:]...)
+	preferredSubnetCIDRs = append(subnetCIDRs[:0], subnetCIDRs[1:]...)
 
 	if s.scope.VPC().IsIPv6Enabled() {
 		ipv6SubnetCIDRs, err = cidr.SplitIntoSubnetsIPv6(s.scope.VPC().IPv6.CidrBlock, numSubnets)
@@ -303,12 +315,23 @@ func (s *Service) getDefaultSubnets() (infrav1.Subnets, error) {
 		}
 
 		// We need to take the last, so it doesn't conflict with the rest. The subnetID is increment each time by 1.
-		publicIPv6SubnetCIDRs, err = cidr.SplitIntoSubnetsIPv6(ipv6SubnetCIDRs[len(ipv6SubnetCIDRs)-1].String(), len(zones))
+		ipv6SubnetCIDRsStr := ipv6SubnetCIDRs[len(ipv6SubnetCIDRs)-1].String()
+		residualIPv6SubnetCIDRs, err = cidr.SplitIntoSubnetsIPv6(ipv6SubnetCIDRsStr, len(zones))
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed splitting IPv6 CIDR %q into public subnets", ipv6SubnetCIDRs[len(ipv6SubnetCIDRs)-1].String())
+			return nil, errors.Wrapf(err, "failed splitting IPv6 CIDR %q into %s subnets", ipv6SubnetCIDRsStr, residualSubnetsName)
 		}
 		// TODO: this might need to be the last instead of the first..
-		privateIPv6SubnetCIDRs = append(ipv6SubnetCIDRs[:0], ipv6SubnetCIDRs[1:]...)
+		preferredIPv6SubnetCIDRs = append(ipv6SubnetCIDRs[:0], ipv6SubnetCIDRs[1:]...)
+	}
+
+	// By default, the preferred subnets are the private subnets and the residual subnets are the public subnets.
+	privateSubnetCIDRs, publicSubnetCIDRs := preferredSubnetCIDRs, residualSubnetCIDRs
+	privateIPv6SubnetCIDRs, publicIPv6SubnetCIDRs := preferredIPv6SubnetCIDRs, residualIPv6SubnetCIDRs
+
+	// If the subnet schema is set to prefer public, we need to swap the private and public subnets.
+	if subnetScheme == infrav1.SubnetSchemaPreferPublic {
+		privateSubnetCIDRs, publicSubnetCIDRs = residualSubnetCIDRs, preferredSubnetCIDRs
+		privateIPv6SubnetCIDRs, publicIPv6SubnetCIDRs = residualIPv6SubnetCIDRs, preferredIPv6SubnetCIDRs
 	}
 
 	subnets := infrav1.Subnets{}
diff --git a/pkg/cloud/services/network/subnets_test.go b/pkg/cloud/services/network/subnets_test.go

Original file line number	Diff line number	Diff line change
`@@ -1062,6 +1062,7 @@ func TestAWSClusterDefaultCNIIngressRules(t *testing.T) {`
`1062`	`1062`	`defaultVPCSpec := VPCSpec{`
`1063`	`1063`	`AvailabilityZoneUsageLimit: &AZUsageLimit,`
`1064`	`1064`	`AvailabilityZoneSelection: &AZSelectionSchemeOrdered,`
	`1065`	`+ SubnetSchema: &SubnetSchemaPreferPrivate,`
`1065`	`1066`	`}`
`1066`	`1067`	`g := NewWithT(t)`
`1067`	`1068`	`tests := []struct {`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ func TestDefaultingWebhook(t *testing.T) {`
`45`	`45`	`defaultVPCSpec := infrav1.VPCSpec{`
`46`	`46`	`AvailabilityZoneUsageLimit: &AZUsageLimit,`
`47`	`47`	`AvailabilityZoneSelection: &infrav1.AZSelectionSchemeOrdered,`
	`48`	`+ SubnetSchema: &infrav1.SubnetSchemaPreferPrivate,`
`48`	`49`	`}`
`49`	`50`	`defaultIdentityRef := &infrav1.AWSIdentityReference{`
`50`	`51`	`Kind: infrav1.ControllerIdentityKind,`