Skip to content

Commit 45e5c83

Browse files
joekrjoekr
authored
fix: validator for webhooks (#461)
Updated validator to handle the ingress and egress rules required fields for: - UdpOptions - IcmpOptions - TcpOptions
1 parent 1868286 commit 45e5c83

File tree

3 files changed

+415
-32
lines changed

3 files changed

+415
-32
lines changed

‎api/v1beta2/ocicluster_webhook_test.go‎

Lines changed: 206 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,11 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
8686
badClusterName := "bad.cluster"
8787

8888
tests := []struct {
89-
name string
90-
c *OCICluster
91-
errorMgsShouldContain string
92-
expectErr bool
89+
name string
90+
c *OCICluster
91+
errorMgsShouldContain string
92+
multipleErrorMgsShouldContain []string
93+
expectErr bool
9394
}{
9495
{
9596
name: "shouldn't allow spaces in region",
@@ -354,7 +355,7 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
354355
},
355356
},
356357
},
357-
errorMgsShouldContain: "invalid egressRules CIDR format",
358+
errorMgsShouldContain: "invalid egressRules: CIDR format",
358359
expectErr: true,
359360
},
360361
{
@@ -387,6 +388,39 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
387388
errorMgsShouldContain: "invalid egressRules: Destination may not be empty",
388389
expectErr: true,
389390
},
391+
{
392+
name: "shouldn't allow empty NSG egress icmpOptions nil",
393+
c: &OCICluster{
394+
ObjectMeta: metav1.ObjectMeta{
395+
Name: goodClusterName,
396+
},
397+
Spec: OCIClusterSpec{
398+
CompartmentId: "ocid",
399+
OCIResourceIdentifier: "uuid",
400+
NetworkSpec: NetworkSpec{
401+
Vcn: VCN{
402+
NetworkSecurityGroup: NetworkSecurityGroup{
403+
List: []*NSG{{
404+
Role: Custom,
405+
EgressRules: []EgressSecurityRuleForNSG{{
406+
EgressSecurityRule: EgressSecurityRule{
407+
Destination: common.String("0.0.0.0/0"),
408+
DestinationType: EgressSecurityRuleDestinationTypeCidrBlock,
409+
Protocol: common.String("all"),
410+
IcmpOptions: &IcmpOptions{
411+
Type: nil,
412+
},
413+
},
414+
}},
415+
}},
416+
},
417+
},
418+
},
419+
},
420+
},
421+
errorMgsShouldContain: "invalid egressRules: IcmpOptions Type may not be empty",
422+
expectErr: true,
423+
},
390424
{
391425
name: "shouldn't allow empty NSG egress protocol",
392426
c: &OCICluster{
@@ -417,6 +451,102 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
417451
errorMgsShouldContain: "invalid egressRules: Protocol may not be empty",
418452
expectErr: true,
419453
},
454+
{
455+
name: "shouldn't allow empty NSG egress udpSource PortRange Max",
456+
c: &OCICluster{
457+
ObjectMeta: metav1.ObjectMeta{
458+
Name: goodClusterName,
459+
},
460+
Spec: OCIClusterSpec{
461+
CompartmentId: "ocid",
462+
OCIResourceIdentifier: "uuid",
463+
NetworkSpec: NetworkSpec{
464+
Vcn: VCN{
465+
NetworkSecurityGroup: NetworkSecurityGroup{
466+
List: []*NSG{{
467+
Role: Custom,
468+
EgressRules: []EgressSecurityRuleForNSG{{
469+
EgressSecurityRule: EgressSecurityRule{
470+
Destination: common.String("10.0.0.0/15"),
471+
DestinationType: EgressSecurityRuleDestinationTypeCidrBlock,
472+
Protocol: common.String("all"),
473+
UdpOptions: &UdpOptions{
474+
DestinationPortRange: &PortRange{
475+
Max: nil,
476+
Min: common.Int(80),
477+
},
478+
},
479+
},
480+
}},
481+
}},
482+
},
483+
},
484+
},
485+
},
486+
},
487+
errorMgsShouldContain: "invalid egressRules: UdpOptions DestinationPortRange Max may not be empty",
488+
expectErr: true,
489+
},
490+
{
491+
name: "shouldn't allow empty NSG egress rule required values for port ranges",
492+
c: &OCICluster{
493+
ObjectMeta: metav1.ObjectMeta{
494+
Name: goodClusterName,
495+
},
496+
Spec: OCIClusterSpec{
497+
CompartmentId: "ocid",
498+
OCIResourceIdentifier: "uuid",
499+
NetworkSpec: NetworkSpec{
500+
Vcn: VCN{
501+
NetworkSecurityGroup: NetworkSecurityGroup{
502+
List: []*NSG{{
503+
Role: Custom,
504+
EgressRules: []EgressSecurityRuleForNSG{{
505+
EgressSecurityRule: EgressSecurityRule{
506+
Destination: common.String("10.0.0.0/15"),
507+
DestinationType: EgressSecurityRuleDestinationTypeCidrBlock,
508+
Protocol: common.String("all"),
509+
UdpOptions: &UdpOptions{
510+
DestinationPortRange: &PortRange{
511+
Max: nil, // required
512+
Min: nil, // required
513+
},
514+
SourcePortRange: &PortRange{
515+
Max: nil, // required
516+
Min: nil, // required
517+
},
518+
},
519+
TcpOptions: &TcpOptions{
520+
DestinationPortRange: &PortRange{
521+
Max: nil, // required
522+
Min: nil, // required
523+
},
524+
SourcePortRange: &PortRange{
525+
Max: nil, // required
526+
Min: nil, // required
527+
},
528+
},
529+
},
530+
}},
531+
}},
532+
},
533+
},
534+
},
535+
},
536+
},
537+
errorMgsShouldContain: "",
538+
multipleErrorMgsShouldContain: []string{
539+
"invalid egressRules: UdpOptions DestinationPortRange Max may not be empty",
540+
"invalid egressRules: UdpOptions DestinationPortRange Min may not be empty",
541+
"invalid egressRules: UdpOptions SourcePortRange Max may not be empty",
542+
"invalid egressRules: UdpOptions SourcePortRange Min may not be empty",
543+
"invalid egressRules: TcpOptions DestinationPortRange Max may not be empty",
544+
"invalid egressRules: TcpOptions DestinationPortRange Min may not be empty",
545+
"invalid egressRules: TcpOptions SourcePortRange Max may not be empty",
546+
"invalid egressRules: TcpOptions SourcePortRange Min may not be empty",
547+
},
548+
expectErr: true,
549+
},
420550
{
421551
name: "shouldn't allow bad NSG ingress cidr",
422552
c: &OCICluster{
@@ -440,7 +570,7 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
440570
},
441571
},
442572
},
443-
errorMgsShouldContain: "invalid ingressRule CIDR format",
573+
errorMgsShouldContain: "invalid ingressRules: CIDR format",
444574
expectErr: true,
445575
},
446576
{
@@ -503,6 +633,66 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
503633
errorMgsShouldContain: "invalid ingressRules: Source may not be empty",
504634
expectErr: true,
505635
},
636+
{
637+
name: "shouldn't allow empty NSG ingress rule required values for port ranges",
638+
c: &OCICluster{
639+
ObjectMeta: metav1.ObjectMeta{
640+
Name: goodClusterName,
641+
},
642+
Spec: OCIClusterSpec{
643+
CompartmentId: "ocid",
644+
OCIResourceIdentifier: "uuid",
645+
NetworkSpec: NetworkSpec{
646+
Vcn: VCN{
647+
NetworkSecurityGroup: NetworkSecurityGroup{
648+
List: []*NSG{{
649+
Role: Custom,
650+
IngressRules: []IngressSecurityRuleForNSG{{
651+
IngressSecurityRule: IngressSecurityRule{
652+
Source: common.String("10.0.0.0/15"),
653+
SourceType: IngressSecurityRuleSourceTypeCidrBlock,
654+
Protocol: common.String("all"),
655+
UdpOptions: &UdpOptions{
656+
DestinationPortRange: &PortRange{
657+
Max: nil, // required
658+
Min: nil, // required
659+
},
660+
SourcePortRange: &PortRange{
661+
Max: nil, // required
662+
Min: nil, // required
663+
},
664+
},
665+
TcpOptions: &TcpOptions{
666+
DestinationPortRange: &PortRange{
667+
Max: nil, // required
668+
Min: nil, // required
669+
},
670+
SourcePortRange: &PortRange{
671+
Max: nil, // required
672+
Min: nil, // required
673+
},
674+
},
675+
},
676+
}},
677+
}},
678+
},
679+
},
680+
},
681+
},
682+
},
683+
errorMgsShouldContain: "",
684+
multipleErrorMgsShouldContain: []string{
685+
"invalid ingressRules: UdpOptions DestinationPortRange Max may not be empty",
686+
"invalid ingressRules: UdpOptions DestinationPortRange Min may not be empty",
687+
"invalid ingressRules: UdpOptions SourcePortRange Max may not be empty",
688+
"invalid ingressRules: UdpOptions SourcePortRange Min may not be empty",
689+
"invalid ingressRules: TcpOptions DestinationPortRange Max may not be empty",
690+
"invalid ingressRules: TcpOptions DestinationPortRange Min may not be empty",
691+
"invalid ingressRules: TcpOptions SourcePortRange Max may not be empty",
692+
"invalid ingressRules: TcpOptions SourcePortRange Min may not be empty",
693+
},
694+
expectErr: true,
695+
},
506696
{
507697
name: "shouldn't allow bad NSG role",
508698
c: &OCICluster{
@@ -609,7 +799,16 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
609799
if test.expectErr {
610800
_, err := (&OCIClusterWebhook{}).ValidateCreate(context.Background(), test.c)
611801
g.Expect(err).NotTo(gomega.Succeed())
612-
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
802+
803+
// handle the tests that produce multiple error messages
804+
// to help keep the number of tests down
805+
if len(test.multipleErrorMgsShouldContain) > 0 {
806+
for _, errMgs := range test.multipleErrorMgsShouldContain {
807+
g.Expect(strings.Contains(err.Error(), errMgs)).To(gomega.BeTrue())
808+
}
809+
} else {
810+
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
811+
}
613812
} else {
614813
_, err := (&OCIClusterWebhook{}).ValidateCreate(context.Background(), test.c)
615814
g.Expect(err).To(gomega.Succeed())

‎api/v1beta2/ocimanagedcluster_webhook_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) {
338338
},
339339
},
340340
},
341-
errorMgsShouldContain: "invalid egressRules CIDR format",
341+
errorMgsShouldContain: "invalid egressRules: CIDR format",
342342
expectErr: true,
343343
},
344344
{
@@ -364,7 +364,7 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) {
364364
},
365365
},
366366
},
367-
errorMgsShouldContain: "invalid ingressRule CIDR format",
367+
errorMgsShouldContain: "invalid ingressRules: CIDR format",
368368
expectErr: true,
369369
},
370370
{

0 commit comments

Comments
 (0)