Skip to content

Commit 59cf5cc

Browse files
tthvotthvo
committed
validations: validate vpc and subnet CIDR
Validation for specified VPC and subnet CIDRs is added for early feedback from the webhook. There are already existing checks for bastion and nodePort CIDRs.
1 parent a02faf2 commit 59cf5cc

File tree

2 files changed

+178
-4
lines changed

2 files changed

+178
-4
lines changed

‎api/v1beta2/awscluster_webhook.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,10 +301,35 @@ func (r *AWSCluster) validateSSHKeyName() field.ErrorList {
301301

302302
func (r *AWSCluster) validateNetwork() field.ErrorList {
303303
var allErrs field.ErrorList
304-
for _, subnet := range r.Spec.NetworkSpec.Subnets {
304+
305+
vpcSpec := r.Spec.NetworkSpec.VPC
306+
vpcField := field.NewPath("spec", "network", "vpc")
307+
if vpcSpec.CidrBlock != "" {
308+
if _, _, err := net.ParseCIDR(vpcSpec.CidrBlock); err != nil {
309+
allErrs = append(allErrs, field.Invalid(vpcField.Child("cidrBlock"), vpcSpec.CidrBlock, "VPC CIDR block is invalid"))
310+
}
311+
}
312+
if vpcSpec.IPv6 != nil && vpcSpec.IPv6.CidrBlock != "" {
313+
if _, _, err := net.ParseCIDR(vpcSpec.IPv6.CidrBlock); err != nil {
314+
allErrs = append(allErrs, field.Invalid(vpcField.Child("ipv6", "cidrBlock"), vpcSpec.IPv6.CidrBlock, "VPC IPv6 CIDR block is invalid"))
315+
}
316+
}
317+
318+
subnetField := field.NewPath("spec", "network", "subnets")
319+
for i, subnet := range r.Spec.NetworkSpec.Subnets {
305320
if subnet.ZoneType != nil && subnet.IsEdge() {
306321
if subnet.ParentZoneName == nil {
307-
allErrs = append(allErrs, field.Invalid(field.NewPath("subnets"), r.Spec.NetworkSpec.Subnets, "ParentZoneName must be set when ZoneType is 'local-zone'."))
322+
allErrs = append(allErrs, field.Invalid(subnetField.Index(i).Child("parentZoneName"), subnet.ParentZoneName, "ParentZoneName must be set when ZoneType is 'local-zone'."))
323+
}
324+
}
325+
if subnet.CidrBlock != "" {
326+
if _, _, err := net.ParseCIDR(subnet.CidrBlock); err != nil {
327+
allErrs = append(allErrs, field.Invalid(subnetField.Index(i).Child("cidrBlock"), subnet.CidrBlock, "subnet CIDR block is invalid"))
328+
}
329+
}
330+
if subnet.IPv6CidrBlock != "" {
331+
if _, _, err := net.ParseCIDR(subnet.IPv6CidrBlock); err != nil {
332+
allErrs = append(allErrs, field.Invalid(subnetField.Index(i).Child("ipv6CidrBlock"), subnet.IPv6CidrBlock, "subnet IPv6 CIDR block is invalid"))
308333
}
309334
}
310335
}
@@ -344,10 +369,15 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
344369

345370
secondaryCidrBlocks := r.Spec.NetworkSpec.VPC.SecondaryCidrBlocks
346371
secondaryCidrBlocksField := field.NewPath("spec", "network", "vpc", "secondaryCidrBlocks")
347-
for _, cidrBlock := range secondaryCidrBlocks {
372+
for i, cidrBlock := range secondaryCidrBlocks {
348373
if r.Spec.NetworkSpec.VPC.CidrBlock != "" && r.Spec.NetworkSpec.VPC.CidrBlock == cidrBlock.IPv4CidrBlock {
349374
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)))
350375
}
376+
if cidrBlock.IPv4CidrBlock != "" {
377+
if _, _, err := net.ParseCIDR(cidrBlock.IPv4CidrBlock); err != nil {
378+
allErrs = append(allErrs, field.Invalid(secondaryCidrBlocksField.Index(i).Child("ipv4CidrBlock"), cidrBlock.IPv4CidrBlock, "secondary VPC CIDR block is invalid"))
379+
}
380+
}
351381
}
352382

353383
return allErrs

‎api/v1beta2/awscluster_webhook_test.go

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,87 @@ func TestAWSClusterValidateCreate(t *testing.T) {
324324
wantErr: false,
325325
},
326326
{
327-
name: "accepts ipv6",
327+
name: "accepts vpc cidr",
328+
cluster: &AWSCluster{
329+
Spec: AWSClusterSpec{
330+
NetworkSpec: NetworkSpec{
331+
VPC: VPCSpec{
332+
CidrBlock: "10.0.0.0/16",
333+
},
334+
},
335+
},
336+
},
337+
wantErr: false,
338+
},
339+
{
340+
name: "rejects invalid vpc cidr",
341+
cluster: &AWSCluster{
342+
Spec: AWSClusterSpec{
343+
NetworkSpec: NetworkSpec{
344+
VPC: VPCSpec{
345+
CidrBlock: "10.0.0.0",
346+
},
347+
},
348+
},
349+
},
350+
wantErr: true,
351+
},
352+
{
353+
name: "accepts vpc secondary cidr",
354+
cluster: &AWSCluster{
355+
Spec: AWSClusterSpec{
356+
NetworkSpec: NetworkSpec{
357+
VPC: VPCSpec{
358+
CidrBlock: "10.0.0.0/16",
359+
SecondaryCidrBlocks: []VpcCidrBlock{
360+
{
361+
IPv4CidrBlock: "10.0.1.0/24",
362+
},
363+
},
364+
},
365+
},
366+
},
367+
},
368+
wantErr: false,
369+
},
370+
{
371+
name: "rejects invalid vpc secondary cidr",
372+
cluster: &AWSCluster{
373+
Spec: AWSClusterSpec{
374+
NetworkSpec: NetworkSpec{
375+
VPC: VPCSpec{
376+
CidrBlock: "10.0.0.0/16",
377+
SecondaryCidrBlocks: []VpcCidrBlock{
378+
{
379+
IPv4CidrBlock: "10.0.1.0",
380+
},
381+
},
382+
},
383+
},
384+
},
385+
},
386+
wantErr: true,
387+
},
388+
{
389+
name: "rejects vpc secondary cidr duplicate with vpc primary cidr",
390+
cluster: &AWSCluster{
391+
Spec: AWSClusterSpec{
392+
NetworkSpec: NetworkSpec{
393+
VPC: VPCSpec{
394+
CidrBlock: "10.0.0.0/16",
395+
SecondaryCidrBlocks: []VpcCidrBlock{
396+
{
397+
IPv4CidrBlock: "10.0.0.0/16",
398+
},
399+
},
400+
},
401+
},
402+
},
403+
},
404+
wantErr: true,
405+
},
406+
{
407+
name: "accepts vpc ipv6 cidr",
328408
cluster: &AWSCluster{
329409
Spec: AWSClusterSpec{
330410
NetworkSpec: NetworkSpec{
@@ -339,6 +419,22 @@ func TestAWSClusterValidateCreate(t *testing.T) {
339419
},
340420
wantErr: false,
341421
},
422+
{
423+
name: "reject invalid vpc ipv6 cidr",
424+
cluster: &AWSCluster{
425+
Spec: AWSClusterSpec{
426+
NetworkSpec: NetworkSpec{
427+
VPC: VPCSpec{
428+
IPv6: &IPv6{
429+
CidrBlock: "2001:2345:5678::",
430+
PoolID: "pool-id",
431+
},
432+
},
433+
},
434+
},
435+
},
436+
wantErr: true,
437+
},
342438
{
343439
name: "accepts ipv6 enabled subnet",
344440
cluster: &AWSCluster{
@@ -358,6 +454,38 @@ func TestAWSClusterValidateCreate(t *testing.T) {
358454
},
359455
wantErr: false,
360456
},
457+
{
458+
name: "accepts cidr block for subnets",
459+
cluster: &AWSCluster{
460+
Spec: AWSClusterSpec{
461+
NetworkSpec: NetworkSpec{
462+
Subnets: []SubnetSpec{
463+
{
464+
ID: "sub-1",
465+
CidrBlock: "10.0.10.0/24",
466+
},
467+
},
468+
},
469+
},
470+
},
471+
wantErr: false,
472+
},
473+
{
474+
name: "rejects invalid cidr block for subnets",
475+
cluster: &AWSCluster{
476+
Spec: AWSClusterSpec{
477+
NetworkSpec: NetworkSpec{
478+
Subnets: []SubnetSpec{
479+
{
480+
ID: "sub-1",
481+
CidrBlock: "10.0.10.0",
482+
},
483+
},
484+
},
485+
},
486+
},
487+
wantErr: true,
488+
},
361489
{
362490
name: "accepts ipv6 cidr block for subnets",
363491
cluster: &AWSCluster{
@@ -374,6 +502,22 @@ func TestAWSClusterValidateCreate(t *testing.T) {
374502
},
375503
wantErr: false,
376504
},
505+
{
506+
name: "rejects invalid ipv6 cidr block for subnets",
507+
cluster: &AWSCluster{
508+
Spec: AWSClusterSpec{
509+
NetworkSpec: NetworkSpec{
510+
Subnets: []SubnetSpec{
511+
{
512+
ID: "sub-1",
513+
IPv6CidrBlock: "2022:1234:5678:9101::",
514+
},
515+
},
516+
},
517+
},
518+
},
519+
wantErr: true,
520+
},
377521
{
378522
name: "rejects ingress rules with cidr block and source security group id",
379523
cluster: &AWSCluster{

0 commit comments

Comments
 (0)