diff --git a/api/v1beta1/cluster_types.go b/api/v1beta1/cluster_types.go index 569fcfc2f43f..5d56b8e22b9e 100644 --- a/api/v1beta1/cluster_types.go +++ b/api/v1beta1/cluster_types.go @@ -468,8 +468,14 @@ func ipFamilyForCIDRStrings(cidrs []string) (ClusterIPFamily, error) { return InvalidIPFamily, fmt.Errorf("could not parse CIDR: %s", err) } if ip.To4() != nil { + if foundIPv4 { + return InvalidIPFamily, fmt.Errorf("multiple CIDRs in the IPv4 family can not be specified") + } foundIPv4 = true } else { + if foundIPv6 { + return InvalidIPFamily, fmt.Errorf("multiple CIDRs in the IPv6 family can not be specified") + } foundIPv6 = true } } diff --git a/api/v1beta1/cluster_types_test.go b/api/v1beta1/cluster_types_test.go index e1893918f444..f454402eb9f1 100644 --- a/api/v1beta1/cluster_types_test.go +++ b/api/v1beta1/cluster_types_test.go @@ -183,6 +183,26 @@ func TestClusterIPFamily(t *testing.T) { expectErr: "services: too many CIDRs specified", c: clusterWithNetwork(nil, []string{"192.168.0.0/16", "fd00:100:96::/48", "10.128.0.0/12"}), }, + { + name: "services: multiple IPv4 CIDRs", + expectErr: "services: multiple CIDRs in the IPv4 family can not be specified", + c: clusterWithNetwork(nil, []string{"192.168.0.0/16", "10.128.0.0/12"}), + }, + { + name: "pods: multiple IPv4 CIDRs", + expectErr: "pods: multiple CIDRs in the IPv4 family can not be specified", + c: clusterWithNetwork([]string{"192.168.0.0/16", "10.128.0.0/12"}, nil), + }, + { + name: "services: multiple IPv4 CIDRs", + expectErr: "services: multiple CIDRs in the IPv6 family can not be specified", + c: clusterWithNetwork(nil, []string{"fd00:100:96::/48", "fd00:100:97::/48"}), + }, + { + name: "pods: multiple IPv4 CIDRs", + expectErr: "pods: multiple CIDRs in the IPv6 family can not be specified", + c: clusterWithNetwork([]string{"fd00:100:96::/48", "fd00:100:97::/48"}, nil), + }, } for _, tt := range invalid { diff --git a/internal/test/builder/builders.go b/internal/test/builder/builders.go index 705cc092824d..acc75341db01 100644 --- a/internal/test/builder/builders.go +++ b/internal/test/builder/builders.go @@ -39,6 +39,7 @@ type ClusterBuilder struct { topology *clusterv1.Topology infrastructureCluster *unstructured.Unstructured controlPlane *unstructured.Unstructured + network *clusterv1.ClusterNetwork } // Cluster returns a ClusterBuilder with the given name and namespace. @@ -49,6 +50,12 @@ func Cluster(namespace, name string) *ClusterBuilder { } } +// WithClusterNetwork sets the ClusterNetwork for the ClusterBuilder. +func (c *ClusterBuilder) WithClusterNetwork(clusterNetwork *clusterv1.ClusterNetwork) *ClusterBuilder { + c.network = clusterNetwork + return c +} + // WithLabels sets the labels for the ClusterBuilder. func (c *ClusterBuilder) WithLabels(labels map[string]string) *ClusterBuilder { c.labels = labels @@ -93,7 +100,8 @@ func (c *ClusterBuilder) Build() *clusterv1.Cluster { Annotations: c.annotations, }, Spec: clusterv1.ClusterSpec{ - Topology: c.topology, + Topology: c.topology, + ClusterNetwork: c.network, }, } if c.infrastructureCluster != nil { diff --git a/internal/test/builder/zz_generated.deepcopy.go b/internal/test/builder/zz_generated.deepcopy.go index a616faa9de47..f71f8a9cb72c 100644 --- a/internal/test/builder/zz_generated.deepcopy.go +++ b/internal/test/builder/zz_generated.deepcopy.go @@ -77,6 +77,11 @@ func (in *ClusterBuilder) DeepCopyInto(out *ClusterBuilder) { in, out := &in.controlPlane, &out.controlPlane *out = (*in).DeepCopy() } + if in.network != nil { + in, out := &in.network, &out.network + *out = new(v1beta1.ClusterNetwork) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterBuilder. diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index b50f99beb632..960f4d2665ec 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/blang/semver" + "github.com/davecgh/go-spew/spew" "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -170,6 +171,18 @@ func (webhook *Cluster) validate(ctx context.Context, oldCluster, newCluster *cl ), ) } + if newCluster.Spec.ClusterNetwork != nil { + if _, err := newCluster.GetIPFamily(); err != nil { + allErrs = append( + allErrs, + field.Invalid( + specPath.Child("clusterNetwork"), + spew.Sprint(newCluster.Spec.ClusterNetwork), + err.Error(), + )) + } + } + topologyPath := specPath.Child("topology") // Validate the managed topology, if defined. @@ -433,7 +446,7 @@ func validateMachineHealthChecks(cluster *clusterv1.Cluster, clusterClass *clust } // machineDeploymentClassOfName find a MachineDeploymentClass of the given name in the provided ClusterClass. -// Returns nill if can not find one. +// Returns nil if can not find one. // TODO: Check if there is already a helper function that can do this. func machineDeploymentClassOfName(clusterClass *clusterv1.ClusterClass, name string) *clusterv1.MachineDeploymentClass { for _, mdClass := range clusterClass.Spec.Workers.MachineDeployments { diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go index de9b443ba780..f7b1d762103d 100644 --- a/internal/webhooks/cluster_test.go +++ b/internal/webhooks/cluster_test.go @@ -360,7 +360,6 @@ func TestClusterDefaultTopologyVersion(t *testing.T) { func TestClusterValidation(t *testing.T) { // NOTE: ClusterTopology feature flag is disabled by default, thus preventing to set Cluster.Topologies. - var ( tests = []struct { name string @@ -409,6 +408,81 @@ func TestClusterValidation(t *testing.T) { WithTopology(&clusterv1.Topology{}). Build(), }, + { + name: "pass with valid CIDR ranges", + expectErr: false, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"10.10.10.10/24"}}, + Pods: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"10.10.10.10/24"}}, + }). + Build(), + }, + { + name: "fails if multiple CIDR ranges of IPv4 are passed", + expectErr: true, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"10.10.10.10/24", "11.11.11.11/24"}}, + }). + Build(), + }, + { + name: "fails if multiple CIDR ranges of IPv6 are passed", + expectErr: true, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"2002::1234:abcd:ffff:c0a8:101/64", "2004::1234:abcd:ffff:c0a8:101/64"}}, + }). + Build(), + }, + + { + name: "fails if too many cidr ranges are specified in the clusterNetwork pods field", + expectErr: true, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + Pods: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"10.10.10.10/24", "11.11.11.11/24", "12.12.12.12/24"}}}). + Build(), + }, + { + name: "fails if too many cidr ranges are specified in the clusterNetwork pods field", + expectErr: true, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"10.10.10.10/24", "11.11.11.11/24", "12.12.12.12/24"}}}). + Build(), + }, + { + name: "fails if cidr ranges are not valid", + expectErr: true, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + Services: &clusterv1.NetworkRanges{ + // Invalid ranges: missing network prefix + CIDRBlocks: []string{"10.10.10.10", "11.11.11.11"}}}). + Build(), + }, + { + name: "fails if pod and service cidr families do not match", + expectErr: true, + in: builder.Cluster("fooNamespace", "cluster1"). + WithClusterNetwork(&clusterv1.ClusterNetwork{ + // Services are IPv4 + Services: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"10.10.10.10/24", "11.11.11.11/24"}}, + // Pods are IPv6 + Pods: &clusterv1.NetworkRanges{ + CIDRBlocks: []string{"2002::1234:abcd:ffff:c0a8:101/64"}}, + }). + Build(), + }, } ) for _, tt := range tests {