Skip to content

Commit bef7d72

Browse files
killianmuldoonkillianmuldoon
committed
Add validation for number of CIDR ranges specified in Clusters
Signed-off-by: killianmuldoon <[email protected]>
1 parent bda7e5c commit bef7d72

File tree

6 files changed

+156
-5
lines changed

6 files changed

+156
-5
lines changed

‎api/v1beta1/cluster_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,8 @@ func (c *Cluster) SetConditions(conditions Conditions) {
422422
}
423423

424424
// GetIPFamily returns a ClusterIPFamily from the configuration provided.
425+
// Note: IPFamily is not a concept in Kubernetes. It was originally introduced in CAPI for CAPD.
426+
// IPFamily may be dropped in a future release. More details at https://github.com/kubernetes-sigs/cluster-api/issues/7521
425427
func (c *Cluster) GetIPFamily() (ClusterIPFamily, error) {
426428
var podCIDRs, serviceCIDRs []string
427429
if c.Spec.ClusterNetwork != nil {

‎internal/controllers/topology/cluster/patches/variables/variables.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ type ClusterNetworkBuiltins struct {
7777
// Pods is the network ranges from which Pod networks are allocated.
7878
Pods []string `json:"pods,omitempty"`
7979
// IPFamily is the IPFamily the Cluster is operating in. One of Invalid, IPv4, IPv6, DualStack.
80+
// Note: IPFamily is not a concept in Kubernetes. It was originally introduced in CAPI for CAPD.
81+
// IPFamily may be dropped in a future release. More details at https://github.com/kubernetes-sigs/cluster-api/issues/7521
8082
IPFamily string `json:"ipFamily,omitempty"`
8183
}
8284

@@ -197,10 +199,7 @@ func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster) ([]
197199
},
198200
}
199201
if cluster.Spec.ClusterNetwork != nil {
200-
clusterNetworkIPFamily, err := cluster.GetIPFamily()
201-
if err != nil {
202-
return nil, err
203-
}
202+
clusterNetworkIPFamily, _ := cluster.GetIPFamily()
204203
builtin.Cluster.Network = &ClusterNetworkBuiltins{
205204
IPFamily: ipFamilyToString(clusterNetworkIPFamily),
206205
}

‎internal/test/builder/builders.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ type ClusterBuilder struct {
3939
topology *clusterv1.Topology
4040
infrastructureCluster *unstructured.Unstructured
4141
controlPlane *unstructured.Unstructured
42+
network *clusterv1.ClusterNetwork
4243
}
4344

4445
// Cluster returns a ClusterBuilder with the given name and namespace.
@@ -49,6 +50,12 @@ func Cluster(namespace, name string) *ClusterBuilder {
4950
}
5051
}
5152

53+
// WithClusterNetwork sets the ClusterNetwork for the ClusterBuilder.
54+
func (c *ClusterBuilder) WithClusterNetwork(clusterNetwork *clusterv1.ClusterNetwork) *ClusterBuilder {
55+
c.network = clusterNetwork
56+
return c
57+
}
58+
5259
// WithLabels sets the labels for the ClusterBuilder.
5360
func (c *ClusterBuilder) WithLabels(labels map[string]string) *ClusterBuilder {
5461
c.labels = labels
@@ -93,7 +100,8 @@ func (c *ClusterBuilder) Build() *clusterv1.Cluster {
93100
Annotations: c.annotations,
94101
},
95102
Spec: clusterv1.ClusterSpec{
96-
Topology: c.topology,
103+
Topology: c.topology,
104+
ClusterNetwork: c.network,
97105
},
98106
}
99107
if c.infrastructureCluster != nil {

‎internal/test/builder/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/webhooks/cluster.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package webhooks
1919
import (
2020
"context"
2121
"fmt"
22+
"net"
2223
"strings"
2324

2425
"github.com/blang/semver"
@@ -170,6 +171,19 @@ func (webhook *Cluster) validate(ctx context.Context, oldCluster, newCluster *cl
170171
),
171172
)
172173
}
174+
if newCluster.Spec.ClusterNetwork != nil {
175+
// Ensure that the CIDR blocks defined under ClusterNetwork are valid.
176+
if newCluster.Spec.ClusterNetwork.Pods != nil {
177+
allErrs = append(allErrs, validateCIDRBlocks(specPath.Child("clusterNetwork", "pods", "cidrBlocks"),
178+
newCluster.Spec.ClusterNetwork.Pods.CIDRBlocks)...)
179+
}
180+
181+
if newCluster.Spec.ClusterNetwork.Services != nil {
182+
allErrs = append(allErrs, validateCIDRBlocks(specPath.Child("clusterNetwork", "services", "cidrBlocks"),
183+
newCluster.Spec.ClusterNetwork.Services.CIDRBlocks)...)
184+
}
185+
}
186+
173187
topologyPath := specPath.Child("topology")
174188

175189
// Validate the managed topology, if defined.
@@ -443,3 +457,17 @@ func machineDeploymentClassOfName(clusterClass *clusterv1.ClusterClass, name str
443457
}
444458
return nil
445459
}
460+
461+
// validateCIDRBlocks ensures the passed CIDR is valid.
462+
func validateCIDRBlocks(fldPath *field.Path, cidrs []string) field.ErrorList {
463+
var allErrs field.ErrorList
464+
for i, cidr := range cidrs {
465+
if _, _, err := net.ParseCIDR(cidr); err != nil {
466+
allErrs = append(allErrs, field.Invalid(
467+
fldPath.Index(i),
468+
cidr,
469+
err.Error()))
470+
}
471+
}
472+
return allErrs
473+
}

‎internal/webhooks/cluster_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,115 @@ func TestClusterValidation(t *testing.T) {
409409
WithTopology(&clusterv1.Topology{}).
410410
Build(),
411411
},
412+
{
413+
name: "pass with undefined CIDR ranges",
414+
expectErr: false,
415+
in: builder.Cluster("fooNamespace", "cluster1").
416+
WithClusterNetwork(&clusterv1.ClusterNetwork{
417+
Services: &clusterv1.NetworkRanges{
418+
CIDRBlocks: []string{}},
419+
Pods: &clusterv1.NetworkRanges{
420+
CIDRBlocks: []string{}},
421+
}).
422+
Build(),
423+
},
424+
{
425+
name: "pass with nil CIDR ranges",
426+
expectErr: false,
427+
in: builder.Cluster("fooNamespace", "cluster1").
428+
WithClusterNetwork(&clusterv1.ClusterNetwork{
429+
Services: &clusterv1.NetworkRanges{
430+
CIDRBlocks: nil},
431+
Pods: &clusterv1.NetworkRanges{
432+
CIDRBlocks: nil},
433+
}).
434+
Build(),
435+
},
436+
{
437+
name: "pass with valid IPv4 CIDR ranges",
438+
expectErr: false,
439+
in: builder.Cluster("fooNamespace", "cluster1").
440+
WithClusterNetwork(&clusterv1.ClusterNetwork{
441+
Services: &clusterv1.NetworkRanges{
442+
CIDRBlocks: []string{"10.10.10.10/24"}},
443+
Pods: &clusterv1.NetworkRanges{
444+
CIDRBlocks: []string{"10.10.10.10/24"}},
445+
}).
446+
Build(),
447+
},
448+
{
449+
name: "pass with valid IPv6 CIDR ranges",
450+
expectErr: false,
451+
in: builder.Cluster("fooNamespace", "cluster1").
452+
WithClusterNetwork(&clusterv1.ClusterNetwork{
453+
Services: &clusterv1.NetworkRanges{
454+
CIDRBlocks: []string{"2004::1234:abcd:ffff:c0a8:101/64"}},
455+
Pods: &clusterv1.NetworkRanges{
456+
CIDRBlocks: []string{"2004::1234:abcd:ffff:c0a8:101/64"}},
457+
}).
458+
Build(),
459+
},
460+
{
461+
name: "pass with valid dualstack CIDR ranges",
462+
expectErr: false,
463+
in: builder.Cluster("fooNamespace", "cluster1").
464+
WithClusterNetwork(&clusterv1.ClusterNetwork{
465+
Services: &clusterv1.NetworkRanges{
466+
CIDRBlocks: []string{"2004::1234:abcd:ffff:c0a8:101/64", "10.10.10.10/24"}},
467+
Pods: &clusterv1.NetworkRanges{
468+
CIDRBlocks: []string{"2004::1234:abcd:ffff:c0a8:101/64", "10.10.10.10/24"}},
469+
}).
470+
Build(),
471+
},
472+
{
473+
name: "pass if multiple CIDR ranges of IPv4 are passed",
474+
expectErr: false,
475+
in: builder.Cluster("fooNamespace", "cluster1").
476+
WithClusterNetwork(&clusterv1.ClusterNetwork{
477+
Services: &clusterv1.NetworkRanges{
478+
CIDRBlocks: []string{"10.10.10.10/24", "11.11.11.11/24"}},
479+
}).
480+
Build(),
481+
},
482+
{
483+
name: "pass if multiple CIDR ranges of IPv6 are passed",
484+
expectErr: false,
485+
in: builder.Cluster("fooNamespace", "cluster1").
486+
WithClusterNetwork(&clusterv1.ClusterNetwork{
487+
Services: &clusterv1.NetworkRanges{
488+
CIDRBlocks: []string{"2002::1234:abcd:ffff:c0a8:101/64", "2004::1234:abcd:ffff:c0a8:101/64"}},
489+
}).
490+
Build(),
491+
},
492+
{
493+
name: "pass if too many cidr ranges are specified in the clusterNetwork pods field",
494+
expectErr: false,
495+
in: builder.Cluster("fooNamespace", "cluster1").
496+
WithClusterNetwork(&clusterv1.ClusterNetwork{
497+
Pods: &clusterv1.NetworkRanges{
498+
CIDRBlocks: []string{"10.10.10.10/24", "11.11.11.11/24", "12.12.12.12/24"}}}).
499+
Build(),
500+
},
501+
{
502+
name: "fails if service cidr ranges are not valid",
503+
expectErr: true,
504+
in: builder.Cluster("fooNamespace", "cluster1").
505+
WithClusterNetwork(&clusterv1.ClusterNetwork{
506+
Services: &clusterv1.NetworkRanges{
507+
// Invalid ranges: missing network suffix
508+
CIDRBlocks: []string{"10.10.10.10", "11.11.11.11"}}}).
509+
Build(),
510+
},
511+
{
512+
name: "fails if pod cidr ranges are not valid",
513+
expectErr: true,
514+
in: builder.Cluster("fooNamespace", "cluster1").
515+
WithClusterNetwork(&clusterv1.ClusterNetwork{
516+
Pods: &clusterv1.NetworkRanges{
517+
// Invalid ranges: missing network suffix
518+
CIDRBlocks: []string{"10.10.10.10", "11.11.11.11"}}}).
519+
Build(),
520+
},
412521
}
413522
)
414523
for _, tt := range tests {

0 commit comments

Comments
 (0)