From 240a65b5240cfd8272fcb52d5ea347ca26da6296 Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Fri, 4 Aug 2023 08:48:50 +0530 Subject: [PATCH 1/5] Remove validation from Subnet/NSG roles --- api/v1beta2/ocicluster_webhook.go | 2 +- api/v1beta2/ocicluster_webhook_test.go | 43 ------------------ api/v1beta2/ocimanagedcluster_webhook.go | 2 +- api/v1beta2/ocimanagedcluster_webhook_test.go | 45 ------------------- api/v1beta2/validator.go | 17 +++---- 5 files changed, 7 insertions(+), 102 deletions(-) diff --git a/api/v1beta2/ocicluster_webhook.go b/api/v1beta2/ocicluster_webhook.go index a276898c..124e0038 100644 --- a/api/v1beta2/ocicluster_webhook.go +++ b/api/v1beta2/ocicluster_webhook.go @@ -120,7 +120,7 @@ func (c *OCICluster) validate(old *OCICluster) field.ErrorList { oldNetworkSpec = old.Spec.NetworkSpec } - allErrs = append(allErrs, ValidateNetworkSpec(OCIClusterSubnetRoles, c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) + allErrs = append(allErrs, ValidateNetworkSpec(c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) allErrs = append(allErrs, ValidateClusterName(c.Name)...) if len(c.Spec.CompartmentId) <= 0 { diff --git a/api/v1beta2/ocicluster_webhook_test.go b/api/v1beta2/ocicluster_webhook_test.go index db335471..94a1f224 100644 --- a/api/v1beta2/ocicluster_webhook_test.go +++ b/api/v1beta2/ocicluster_webhook_test.go @@ -72,11 +72,6 @@ func TestOCICluster_ValidateCreate(t *testing.T) { Role: ControlPlaneEndpointRole, }, } - badSubnetRole := []*Subnet{ - &Subnet{ - Role: "not-control-plane", - }, - } goodClusterName := "test-cluster" badClusterName := "bad.cluster" @@ -226,44 +221,6 @@ func TestOCICluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "networkSpec.subnets: Duplicate value", expectErr: true, }, - { - name: "shouldn't allow subnet role outside of pre-defined roles", - c: &OCICluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: goodClusterName, - }, - Spec: OCIClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - CIDR: "10.0.0.0/16", - Subnets: badSubnetRole, - }, - }, - }, - }, - errorMgsShouldContain: "subnet role invalid", - expectErr: true, - }, - { - name: "shouldn't allow invalid role", - c: &OCICluster{ - ObjectMeta: metav1.ObjectMeta{}, - Spec: OCIClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - CIDR: "10.0.0.0/16", - Subnets: []*Subnet{ - &Subnet{ - Role: PodRole, - }, - }, - }, - }, - }, - }, - errorMgsShouldContain: "subnet role invalid", - expectErr: true, - }, { name: "shouldn't allow bad cluster names", c: &OCICluster{ diff --git a/api/v1beta2/ocimanagedcluster_webhook.go b/api/v1beta2/ocimanagedcluster_webhook.go index c16ffe20..00d544fe 100644 --- a/api/v1beta2/ocimanagedcluster_webhook.go +++ b/api/v1beta2/ocimanagedcluster_webhook.go @@ -176,7 +176,7 @@ func (c *OCIManagedCluster) validate(old *OCIManagedCluster) field.ErrorList { oldNetworkSpec = old.Spec.NetworkSpec } - allErrs = append(allErrs, ValidateNetworkSpec(OCIManagedClusterSubnetRoles, c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) + allErrs = append(allErrs, ValidateNetworkSpec(c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) allErrs = append(allErrs, ValidateClusterName(c.Name)...) if len(c.Spec.CompartmentId) <= 0 { diff --git a/api/v1beta2/ocimanagedcluster_webhook_test.go b/api/v1beta2/ocimanagedcluster_webhook_test.go index 1c25c9ad..65cc48d9 100644 --- a/api/v1beta2/ocimanagedcluster_webhook_test.go +++ b/api/v1beta2/ocimanagedcluster_webhook_test.go @@ -72,11 +72,6 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) { Role: ControlPlaneEndpointRole, }, } - badSubnetRole := []*Subnet{ - &Subnet{ - Role: "not-control-plane", - }, - } goodClusterName := "test-cluster" badClusterName := "bad.cluster" @@ -227,46 +222,6 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "networkSpec.subnets: Duplicate value", expectErr: true, }, - { - name: "shouldn't allow subnet role outside of pre-defined roles", - c: &OCIManagedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: goodClusterName, - }, - Spec: OCIManagedClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - CIDR: "10.0.0.0/16", - Subnets: badSubnetRole, - }, - }, - }, - }, - errorMgsShouldContain: "subnet role invalid", - expectErr: true, - }, - { - name: "shouldn't allow invalid role", - c: &OCIManagedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: goodClusterName, - }, - Spec: OCIManagedClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - CIDR: "10.0.0.0/16", - Subnets: []*Subnet{ - &Subnet{ - Role: ControlPlaneRole, - }, - }, - }, - }, - }, - }, - errorMgsShouldContain: "subnet role invalid", - expectErr: true, - }, { name: "should allow empty subnet name", c: &OCIManagedCluster{ diff --git a/api/v1beta2/validator.go b/api/v1beta2/validator.go index 86c990da..c9a62c13 100644 --- a/api/v1beta2/validator.go +++ b/api/v1beta2/validator.go @@ -69,7 +69,7 @@ func ValidRegion(stringRegion string) bool { } // ValidateNetworkSpec validates the NetworkSpec -func ValidateNetworkSpec(validRoles []Role, networkSpec NetworkSpec, old NetworkSpec, fldPath *field.Path) field.ErrorList { +func ValidateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if len(networkSpec.Vcn.CIDR) > 0 { @@ -77,11 +77,11 @@ func ValidateNetworkSpec(validRoles []Role, networkSpec NetworkSpec, old Network } if networkSpec.Vcn.Subnets != nil { - allErrs = append(allErrs, validateSubnets(validRoles, networkSpec.Vcn.Subnets, networkSpec.Vcn, fldPath.Child("subnets"))...) + allErrs = append(allErrs, validateSubnets(networkSpec.Vcn.Subnets, networkSpec.Vcn, fldPath.Child("subnets"))...) } if networkSpec.Vcn.NetworkSecurityGroup.List != nil { - allErrs = append(allErrs, validateNSGs(validRoles, networkSpec.Vcn.NetworkSecurityGroup.List, fldPath.Child("networkSecurityGroups"))...) + allErrs = append(allErrs, validateNSGs(networkSpec.Vcn.NetworkSecurityGroup.List, fldPath.Child("networkSecurityGroups"))...) } if len(allErrs) == 0 { @@ -146,13 +146,10 @@ func validateSubnetCIDR(subnetCidr string, vcnCidr string, fldPath *field.Path) } // validateNSGs validates a list of Subnets. -func validateNSGs(validRoles []Role, networkSecurityGroups []*NSG, fldPath *field.Path) field.ErrorList { +func validateNSGs(networkSecurityGroups []*NSG, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList for i, nsg := range networkSecurityGroups { - if err := validateRole(validRoles, nsg.Role, fldPath.Index(i).Child("role"), "networkSecurityGroup role invalid"); err != nil { - allErrs = append(allErrs, err) - } allErrs = append(allErrs, validateEgressSecurityRuleForNSG(nsg.EgressRules, fldPath.Index(i).Child("egressRules"))...) allErrs = append(allErrs, validateIngressSecurityRuleForNSG(nsg.IngressRules, fldPath.Index(i).Child("ingressRules"))...) } @@ -195,7 +192,7 @@ func validateIngressSecurityRuleForNSG(egressRules []IngressSecurityRuleForNSG, } // validateSubnets validates a list of Subnets. -func validateSubnets(validRoles []Role, subnets []*Subnet, vcn VCN, fldPath *field.Path) field.ErrorList { +func validateSubnets(subnets []*Subnet, vcn VCN, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList subnetNames := make(map[string]bool, len(subnets)) @@ -210,10 +207,6 @@ func validateSubnets(validRoles []Role, subnets []*Subnet, vcn VCN, fldPath *fie subnetNames[subnet.Name] = true } - if err := validateRole(validRoles, subnet.Role, fldPath.Index(i).Child("role"), "subnet role invalid"); err != nil { - allErrs = append(allErrs, err) - } - allErrs = append(allErrs, validateSubnetCIDR(subnet.CIDR, vcn.CIDR, fldPath.Index(i).Child("cidr"))...) } From a4bee44754b416d17baec3f0dd78a66a67ad3c90 Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Fri, 4 Aug 2023 08:58:42 +0530 Subject: [PATCH 2/5] Add support for compute cluster id --- api/v1beta2/ocicluster_webhook_test.go | 38 ----------------- api/v1beta2/ocimanagedcluster_webhook_test.go | 41 ------------------- 2 files changed, 79 deletions(-) diff --git a/api/v1beta2/ocicluster_webhook_test.go b/api/v1beta2/ocicluster_webhook_test.go index 94a1f224..cd889854 100644 --- a/api/v1beta2/ocicluster_webhook_test.go +++ b/api/v1beta2/ocicluster_webhook_test.go @@ -312,44 +312,6 @@ func TestOCICluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "invalid ingressRule CIDR format", expectErr: true, }, - { - name: "shouldn't allow bad NSG role", - c: &OCICluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: goodClusterName, - }, - Spec: OCIClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - NetworkSecurityGroup: NetworkSecurityGroup{ - List: []*NSG{{ - Role: "bad-role", - }}, - }, - }, - }, - }, - }, - errorMgsShouldContain: "networkSecurityGroup role invalid", - expectErr: true, - }, - { - name: "shouldn't allow invalid NSG role", - c: &OCICluster{ - ObjectMeta: metav1.ObjectMeta{}, - Spec: OCIClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - NetworkSecurityGroup: NetworkSecurityGroup{List: []*NSG{{ - Role: PodRole, - }}}, - }, - }, - }, - }, - errorMgsShouldContain: "networkSecurityGroup role invalid", - expectErr: true, - }, { name: "should allow blank region", c: &OCICluster{ diff --git a/api/v1beta2/ocimanagedcluster_webhook_test.go b/api/v1beta2/ocimanagedcluster_webhook_test.go index 65cc48d9..0076f4e1 100644 --- a/api/v1beta2/ocimanagedcluster_webhook_test.go +++ b/api/v1beta2/ocimanagedcluster_webhook_test.go @@ -314,47 +314,6 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "networkSecurityGroup role invalid", expectErr: true, }, - { - name: "shouldn't allow invalid NSG role", - c: &OCIManagedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: goodClusterName, - }, - Spec: OCIManagedClusterSpec{ - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - NetworkSecurityGroup: NetworkSecurityGroup{ - List: []*NSG{{ - Role: ControlPlaneRole, - }}, - }, - }, - }, - }, - }, - errorMgsShouldContain: "networkSecurityGroup role invalid", - expectErr: true, - }, - { - name: "should allow blank region", - c: &OCIManagedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster-name", - }, - Spec: OCIManagedClusterSpec{ - Region: "", - CompartmentId: "ocid", - OCIResourceIdentifier: "uuid", - NetworkSpec: NetworkSpec{ - Vcn: VCN{ - CIDR: "10.0.0.0/16", - Subnets: goodSubnets, - }, - }, - }, - }, - expectErr: false, - }, { name: "shouldn't allow loadbalancer", c: &OCIManagedCluster{ From de01ae3b7530e72c0cf713f5a6ceb354b9fabcfc Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Fri, 4 Aug 2023 09:13:01 +0530 Subject: [PATCH 3/5] Add support for compute cluster id --- api/v1beta2/ocicluster_webhook.go | 2 +- api/v1beta2/ocicluster_webhook_test.go | 122 ++++++++++++++++ api/v1beta2/ocimanagedcluster_webhook.go | 2 +- api/v1beta2/ocimanagedcluster_webhook_test.go | 132 ++++++++++++++++++ api/v1beta2/types.go | 5 +- api/v1beta2/validator.go | 17 ++- 6 files changed, 271 insertions(+), 9 deletions(-) diff --git a/api/v1beta2/ocicluster_webhook.go b/api/v1beta2/ocicluster_webhook.go index 124e0038..a276898c 100644 --- a/api/v1beta2/ocicluster_webhook.go +++ b/api/v1beta2/ocicluster_webhook.go @@ -120,7 +120,7 @@ func (c *OCICluster) validate(old *OCICluster) field.ErrorList { oldNetworkSpec = old.Spec.NetworkSpec } - allErrs = append(allErrs, ValidateNetworkSpec(c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) + allErrs = append(allErrs, ValidateNetworkSpec(OCIClusterSubnetRoles, c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) allErrs = append(allErrs, ValidateClusterName(c.Name)...) if len(c.Spec.CompartmentId) <= 0 { diff --git a/api/v1beta2/ocicluster_webhook_test.go b/api/v1beta2/ocicluster_webhook_test.go index cd889854..cc7e57ef 100644 --- a/api/v1beta2/ocicluster_webhook_test.go +++ b/api/v1beta2/ocicluster_webhook_test.go @@ -72,6 +72,11 @@ func TestOCICluster_ValidateCreate(t *testing.T) { Role: ControlPlaneEndpointRole, }, } + badSubnetRole := []*Subnet{ + &Subnet{ + Role: "not-control-plane", + }, + } goodClusterName := "test-cluster" badClusterName := "bad.cluster" @@ -221,6 +226,67 @@ func TestOCICluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "networkSpec.subnets: Duplicate value", expectErr: true, }, + { + name: "shouldn't allow subnet role outside of pre-defined roles", + c: &OCICluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: badSubnetRole, + }, + }, + }, + }, + errorMgsShouldContain: "subnet role invalid", + expectErr: true, + }, + { + name: "allow subnet custom role", + c: &OCICluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIClusterSpec{ + CompartmentId: "ocid", + OCIResourceIdentifier: "uuid", + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: []*Subnet{ + &Subnet{ + Role: Custom, + }, + }, + }, + }, + }, + }, + expectErr: false, + }, + { + name: "shouldn't allow invalid role", + c: &OCICluster{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: OCIClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: []*Subnet{ + &Subnet{ + Role: PodRole, + }, + }, + }, + }, + }, + }, + errorMgsShouldContain: "subnet role invalid", + expectErr: true, + }, { name: "shouldn't allow bad cluster names", c: &OCICluster{ @@ -312,6 +378,62 @@ func TestOCICluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "invalid ingressRule CIDR format", expectErr: true, }, + { + name: "shouldn't allow bad NSG role", + c: &OCICluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + NetworkSecurityGroup: NetworkSecurityGroup{ + List: []*NSG{{ + Role: "bad-role", + }}, + }, + }, + }, + }, + }, + errorMgsShouldContain: "networkSecurityGroup role invalid", + expectErr: true, + }, + { + name: "shouldn't allow invalid NSG role", + c: &OCICluster{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: OCIClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + NetworkSecurityGroup: NetworkSecurityGroup{List: []*NSG{{ + Role: PodRole, + }}}, + }, + }, + }, + }, + errorMgsShouldContain: "networkSecurityGroup role invalid", + expectErr: true, + }, + { + name: "allow nsg custom role", + c: &OCICluster{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: OCIClusterSpec{ + CompartmentId: "ocid", + OCIResourceIdentifier: "uuid", + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + NetworkSecurityGroup: NetworkSecurityGroup{List: []*NSG{{ + Role: Custom, + }}}, + }, + }, + }, + }, + expectErr: false, + }, { name: "should allow blank region", c: &OCICluster{ diff --git a/api/v1beta2/ocimanagedcluster_webhook.go b/api/v1beta2/ocimanagedcluster_webhook.go index 00d544fe..c16ffe20 100644 --- a/api/v1beta2/ocimanagedcluster_webhook.go +++ b/api/v1beta2/ocimanagedcluster_webhook.go @@ -176,7 +176,7 @@ func (c *OCIManagedCluster) validate(old *OCIManagedCluster) field.ErrorList { oldNetworkSpec = old.Spec.NetworkSpec } - allErrs = append(allErrs, ValidateNetworkSpec(c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) + allErrs = append(allErrs, ValidateNetworkSpec(OCIManagedClusterSubnetRoles, c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...) allErrs = append(allErrs, ValidateClusterName(c.Name)...) if len(c.Spec.CompartmentId) <= 0 { diff --git a/api/v1beta2/ocimanagedcluster_webhook_test.go b/api/v1beta2/ocimanagedcluster_webhook_test.go index 0076f4e1..365ad83d 100644 --- a/api/v1beta2/ocimanagedcluster_webhook_test.go +++ b/api/v1beta2/ocimanagedcluster_webhook_test.go @@ -72,6 +72,11 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) { Role: ControlPlaneEndpointRole, }, } + badSubnetRole := []*Subnet{ + &Subnet{ + Role: "not-control-plane", + }, + } goodClusterName := "test-cluster" badClusterName := "bad.cluster" @@ -222,6 +227,70 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "networkSpec.subnets: Duplicate value", expectErr: true, }, + { + name: "shouldn't allow subnet role outside of pre-defined roles", + c: &OCIManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIManagedClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: badSubnetRole, + }, + }, + }, + }, + errorMgsShouldContain: "subnet role invalid", + expectErr: true, + }, + { + name: "shouldn't allow invalid role", + c: &OCIManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIManagedClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: []*Subnet{ + &Subnet{ + Role: ControlPlaneRole, + }, + }, + }, + }, + }, + }, + errorMgsShouldContain: "subnet role invalid", + expectErr: true, + }, + { + name: "should allow custom subnet role", + c: &OCIManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIManagedClusterSpec{ + Region: "", + CompartmentId: "ocid", + OCIResourceIdentifier: "uuid", + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: []*Subnet{ + &Subnet{ + Role: Custom, + }, + }, + }, + }, + }, + }, + expectErr: false, + }, { name: "should allow empty subnet name", c: &OCIManagedCluster{ @@ -314,6 +383,69 @@ func TestOCIManagedCluster_ValidateCreate(t *testing.T) { errorMgsShouldContain: "networkSecurityGroup role invalid", expectErr: true, }, + { + name: "shouldn't allow invalid NSG role", + c: &OCIManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIManagedClusterSpec{ + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + NetworkSecurityGroup: NetworkSecurityGroup{ + List: []*NSG{{ + Role: ControlPlaneRole, + }}, + }, + }, + }, + }, + }, + errorMgsShouldContain: "networkSecurityGroup role invalid", + expectErr: true, + }, + { + name: "should allow custom NSG role", + c: &OCIManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, + Spec: OCIManagedClusterSpec{ + CompartmentId: "ocid", + OCIResourceIdentifier: "uuid", + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + NetworkSecurityGroup: NetworkSecurityGroup{ + List: []*NSG{{ + Role: Custom, + }}, + }, + }, + }, + }, + }, + expectErr: false, + }, + { + name: "should allow blank region", + c: &OCIManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-name", + }, + Spec: OCIManagedClusterSpec{ + Region: "", + CompartmentId: "ocid", + OCIResourceIdentifier: "uuid", + NetworkSpec: NetworkSpec{ + Vcn: VCN{ + CIDR: "10.0.0.0/16", + Subnets: goodSubnets, + }, + }, + }, + }, + expectErr: false, + }, { name: "shouldn't allow loadbalancer", c: &OCIManagedCluster{ diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go index 38bcfae4..f7be929b 100644 --- a/api/v1beta2/types.go +++ b/api/v1beta2/types.go @@ -24,13 +24,14 @@ const ( PodRole = "pod" Private = "private" Public = "public" + Custom = "custom" ) // OCIClusterSubnetRoles a slice of all the subnet roles for self managed cluster -var OCIClusterSubnetRoles = []Role{ControlPlaneRole, ControlPlaneEndpointRole, WorkerRole, ServiceLoadBalancerRole} +var OCIClusterSubnetRoles = []Role{ControlPlaneRole, ControlPlaneEndpointRole, WorkerRole, ServiceLoadBalancerRole, Custom} // OCIManagedClusterSubnetRoles a slice of all the subnet roles for managed cluster -var OCIManagedClusterSubnetRoles = []Role{PodRole, ControlPlaneEndpointRole, WorkerRole, ServiceLoadBalancerRole} +var OCIManagedClusterSubnetRoles = []Role{PodRole, ControlPlaneEndpointRole, WorkerRole, ServiceLoadBalancerRole, Custom} // NetworkDetails defines the configuration options for the network type NetworkDetails struct { diff --git a/api/v1beta2/validator.go b/api/v1beta2/validator.go index c9a62c13..86c990da 100644 --- a/api/v1beta2/validator.go +++ b/api/v1beta2/validator.go @@ -69,7 +69,7 @@ func ValidRegion(stringRegion string) bool { } // ValidateNetworkSpec validates the NetworkSpec -func ValidateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *field.Path) field.ErrorList { +func ValidateNetworkSpec(validRoles []Role, networkSpec NetworkSpec, old NetworkSpec, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if len(networkSpec.Vcn.CIDR) > 0 { @@ -77,11 +77,11 @@ func ValidateNetworkSpec(networkSpec NetworkSpec, old NetworkSpec, fldPath *fiel } if networkSpec.Vcn.Subnets != nil { - allErrs = append(allErrs, validateSubnets(networkSpec.Vcn.Subnets, networkSpec.Vcn, fldPath.Child("subnets"))...) + allErrs = append(allErrs, validateSubnets(validRoles, networkSpec.Vcn.Subnets, networkSpec.Vcn, fldPath.Child("subnets"))...) } if networkSpec.Vcn.NetworkSecurityGroup.List != nil { - allErrs = append(allErrs, validateNSGs(networkSpec.Vcn.NetworkSecurityGroup.List, fldPath.Child("networkSecurityGroups"))...) + allErrs = append(allErrs, validateNSGs(validRoles, networkSpec.Vcn.NetworkSecurityGroup.List, fldPath.Child("networkSecurityGroups"))...) } if len(allErrs) == 0 { @@ -146,10 +146,13 @@ func validateSubnetCIDR(subnetCidr string, vcnCidr string, fldPath *field.Path) } // validateNSGs validates a list of Subnets. -func validateNSGs(networkSecurityGroups []*NSG, fldPath *field.Path) field.ErrorList { +func validateNSGs(validRoles []Role, networkSecurityGroups []*NSG, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList for i, nsg := range networkSecurityGroups { + if err := validateRole(validRoles, nsg.Role, fldPath.Index(i).Child("role"), "networkSecurityGroup role invalid"); err != nil { + allErrs = append(allErrs, err) + } allErrs = append(allErrs, validateEgressSecurityRuleForNSG(nsg.EgressRules, fldPath.Index(i).Child("egressRules"))...) allErrs = append(allErrs, validateIngressSecurityRuleForNSG(nsg.IngressRules, fldPath.Index(i).Child("ingressRules"))...) } @@ -192,7 +195,7 @@ func validateIngressSecurityRuleForNSG(egressRules []IngressSecurityRuleForNSG, } // validateSubnets validates a list of Subnets. -func validateSubnets(subnets []*Subnet, vcn VCN, fldPath *field.Path) field.ErrorList { +func validateSubnets(validRoles []Role, subnets []*Subnet, vcn VCN, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList subnetNames := make(map[string]bool, len(subnets)) @@ -207,6 +210,10 @@ func validateSubnets(subnets []*Subnet, vcn VCN, fldPath *field.Path) field.Erro subnetNames[subnet.Name] = true } + if err := validateRole(validRoles, subnet.Role, fldPath.Index(i).Child("role"), "subnet role invalid"); err != nil { + allErrs = append(allErrs, err) + } + allErrs = append(allErrs, validateSubnetCIDR(subnet.CIDR, vcn.CIDR, fldPath.Index(i).Child("cidr"))...) } From 2cc0cd25d9e38ea91bccb32d40fb6d21e2faa0d6 Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Fri, 4 Aug 2023 09:18:12 +0530 Subject: [PATCH 4/5] Add support for custom roles in NSG/Subnet --- docs/src/networking/custom-networking.md | 39 ++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/src/networking/custom-networking.md b/docs/src/networking/custom-networking.md index 47967b20..c8708ec6 100644 --- a/docs/src/networking/custom-networking.md +++ b/docs/src/networking/custom-networking.md @@ -300,6 +300,45 @@ spec: loadBalancerType: "lb" ``` +## Example spec to use custom role + +CAPOCI can be used to create Subnet/NSG in the VCN for custom workloads such as private load balancers, +dedicated subnet for DB connection etc. The roles for such custom subnest must be defined as `custom`. +The following spec shows an example for this scenario. + +```yaml +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta2 +kind: OCICluster +metadata: + name: "${CLUSTER_NAME}" +spec: + compartmentId: "${OCI_COMPARTMENT_ID}" + networkSpec: + vcn: + name: ${CLUSTER_NAME} + subnets: + - name: db + role: custom + type: public + cidr: "172.16.5.0/28" + networkSecurityGroup: + list: + - name: db + role: custom + egressRules: + - egressRule: + isStateless: false + destination: "172.16.5.0/28" + protocol: "6" + destinationType: "CIDR_BLOCK" + description: "All traffic to control plane nodes" + tcpOptions: + destinationPortRange: + max: 6443 + min: 6443 +``` + [sl-vs-nsg]: https://docs.oracle.com/en-us/iaas/Content/Network/Concepts/securityrules.htm#comparison [externally-managed-cluster-infrastructure]: ../gs/externally-managed-cluster-infrastructure.md#example-spec-for-externally-managed-vcn-infrastructure [oci-nlb]: https://docs.oracle.com/en-us/iaas/Content/NetworkLoadBalancer/introducton.htm#Overview From 730a78aea4bc8331a17fc6e0827b5982a178046f Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Fri, 4 Aug 2023 09:26:32 +0530 Subject: [PATCH 5/5] Add support for custom roles in NSG/Subnet --- api/v1beta2/ocicluster_webhook_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/v1beta2/ocicluster_webhook_test.go b/api/v1beta2/ocicluster_webhook_test.go index cc7e57ef..933ea58e 100644 --- a/api/v1beta2/ocicluster_webhook_test.go +++ b/api/v1beta2/ocicluster_webhook_test.go @@ -419,7 +419,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) { { name: "allow nsg custom role", c: &OCICluster{ - ObjectMeta: metav1.ObjectMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: goodClusterName, + }, Spec: OCIClusterSpec{ CompartmentId: "ocid", OCIResourceIdentifier: "uuid",