From 245ba08eb4c678ffa1f8a52432baf83936e0da0e Mon Sep 17 00:00:00 2001 From: Yanjun Zhou Date: Fri, 10 Jan 2025 18:26:44 +0800 Subject: [PATCH] Update dhcp validation to prevent switch from DHCPServer/DHCPRelay to DHCPDeactivated (#996) Signed-off-by: Yanjun Zhou --- .../crd/vpc/crd.nsx.vmware.com_subnets.yaml | 29 ++++++++++++------- .../vpc/crd.nsx.vmware.com_subnetsets.yaml | 29 ++++++++++++------- pkg/apis/vpc/v1alpha1/subnet_types.go | 19 +++++++----- pkg/apis/vpc/v1alpha1/subnetset_types.go | 17 ++++++----- test/e2e/nsx_subnet_test.go | 15 ---------- 5 files changed, 59 insertions(+), 50 deletions(-) diff --git a/build/yaml/crd/vpc/crd.nsx.vmware.com_subnets.yaml b/build/yaml/crd/vpc/crd.nsx.vmware.com_subnets.yaml index ec2777b51..d8a54ec17 100644 --- a/build/yaml/crd/vpc/crd.nsx.vmware.com_subnets.yaml +++ b/build/yaml/crd/vpc/crd.nsx.vmware.com_subnets.yaml @@ -94,20 +94,26 @@ spec: - DHCPDeactivated type: string x-kubernetes-validations: - - message: subnetDHCPConfig cannot switch from DHCPDeactivated - to other modes - rule: oldSelf!='DHCPDeactivated' || oldSelf==self + - message: subnetDHCPConfig mode can only switch between DHCPServer + and DHCPRelay + rule: oldSelf!='DHCPDeactivated' && self!='DHCPDeactivated' + || oldSelf==self type: object x-kubernetes-validations: - - message: subnetDHCPConfig cannot switch from DHCPDeactivated to - other modes - rule: has(oldSelf.mode) || !has(self.mode) || self.mode=='DHCPDeactivated' + - message: subnetDHCPConfig mode can only switch between DHCPServer + and DHCPRelay + rule: has(oldSelf.mode)==has(self.mode) || (has(oldSelf.mode) && + !has(self.mode) && oldSelf.mode=='DHCPDeactivated') || (!has(oldSelf.mode) + && has(self.mode) && self.mode=='DHCPDeactivated') type: object x-kubernetes-validations: - - message: subnetDHCPConfig cannot switch from DHCPDeactivated to other - modes - rule: has(oldSelf.subnetDHCPConfig) || !has(self.subnetDHCPConfig) || - !has(self.subnetDHCPConfig.mode) || self.subnetDHCPConfig.mode=='DHCPDeactivated' + - message: subnetDHCPConfig mode can only switch between DHCPServer and + DHCPRelay + rule: has(oldSelf.subnetDHCPConfig)==has(self.subnetDHCPConfig) || (has(oldSelf.subnetDHCPConfig) + && !has(self.subnetDHCPConfig) && (!has(oldSelf.subnetDHCPConfig.mode) + || oldSelf.subnetDHCPConfig.mode=='DHCPDeactivated')) || (!has(oldSelf.subnetDHCPConfig) + && has(self.subnetDHCPConfig) && (!has(self.subnetDHCPConfig.mode) + || self.subnetDHCPConfig.mode=='DHCPDeactivated')) - message: ipv4SubnetSize is required once set rule: '!has(oldSelf.ipv4SubnetSize) || has(self.ipv4SubnetSize)' - message: accessMode is required once set @@ -162,6 +168,9 @@ spec: type: array type: object type: object + x-kubernetes-validations: + - message: spec is required once set + rule: '!has(oldSelf.spec) || has(self.spec)' served: true storage: true subresources: diff --git a/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetsets.yaml b/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetsets.yaml index a357d0faf..3a69fb34c 100644 --- a/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetsets.yaml +++ b/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetsets.yaml @@ -84,20 +84,26 @@ spec: - DHCPDeactivated type: string x-kubernetes-validations: - - message: subnetDHCPConfig cannot switch from DHCPDeactivated - to other modes - rule: oldSelf!='DHCPDeactivated' || oldSelf==self + - message: subnetDHCPConfig mode can only switch between DHCPServer + and DHCPRelay + rule: oldSelf!='DHCPDeactivated' && self!='DHCPDeactivated' + || oldSelf==self type: object x-kubernetes-validations: - - message: subnetDHCPConfig cannot switch from DHCPDeactivated to - other modes - rule: has(oldSelf.mode) || !has(self.mode) || self.mode=='DHCPDeactivated' + - message: subnetDHCPConfig mode can only switch between DHCPServer + and DHCPRelay + rule: has(oldSelf.mode)==has(self.mode) || (has(oldSelf.mode) && + !has(self.mode) && oldSelf.mode=='DHCPDeactivated') || (!has(oldSelf.mode) + && has(self.mode) && self.mode=='DHCPDeactivated') type: object x-kubernetes-validations: - - message: subnetDHCPConfig cannot switch from DHCPDeactivated to other - modes - rule: has(oldSelf.subnetDHCPConfig) || !has(self.subnetDHCPConfig) || - !has(self.subnetDHCPConfig.mode) || self.subnetDHCPConfig.mode=='DHCPDeactivated' + - message: subnetDHCPConfig mode can only switch between DHCPServer and + DHCPRelay + rule: has(oldSelf.subnetDHCPConfig)==has(self.subnetDHCPConfig) || (has(oldSelf.subnetDHCPConfig) + && !has(self.subnetDHCPConfig) && (!has(oldSelf.subnetDHCPConfig.mode) + || oldSelf.subnetDHCPConfig.mode=='DHCPDeactivated')) || (!has(oldSelf.subnetDHCPConfig) + && has(self.subnetDHCPConfig) && (!has(self.subnetDHCPConfig.mode) + || self.subnetDHCPConfig.mode=='DHCPDeactivated')) - message: accessMode is required once set rule: '!has(oldSelf.accessMode) || has(self.accessMode)' - message: ipv4SubnetSize is required once set @@ -157,6 +163,9 @@ spec: type: array type: object type: object + x-kubernetes-validations: + - message: spec is required once set + rule: '!has(oldSelf.spec) || has(self.spec)' served: true storage: true subresources: diff --git a/pkg/apis/vpc/v1alpha1/subnet_types.go b/pkg/apis/vpc/v1alpha1/subnet_types.go index 743e9e2fd..5a0217170 100644 --- a/pkg/apis/vpc/v1alpha1/subnet_types.go +++ b/pkg/apis/vpc/v1alpha1/subnet_types.go @@ -20,7 +20,7 @@ const ( ) // SubnetSpec defines the desired state of Subnet. -// +kubebuilder:validation:XValidation:rule="has(oldSelf.subnetDHCPConfig) || !has(self.subnetDHCPConfig) || !has(self.subnetDHCPConfig.mode) || self.subnetDHCPConfig.mode=='DHCPDeactivated'", message="subnetDHCPConfig cannot switch from DHCPDeactivated to other modes" +// +kubebuilder:validation:XValidation:rule="has(oldSelf.subnetDHCPConfig)==has(self.subnetDHCPConfig) || (has(oldSelf.subnetDHCPConfig) && !has(self.subnetDHCPConfig) && (!has(oldSelf.subnetDHCPConfig.mode) || oldSelf.subnetDHCPConfig.mode=='DHCPDeactivated')) || (!has(oldSelf.subnetDHCPConfig) && has(self.subnetDHCPConfig) && (!has(self.subnetDHCPConfig.mode) || self.subnetDHCPConfig.mode=='DHCPDeactivated'))", message="subnetDHCPConfig mode can only switch between DHCPServer and DHCPRelay" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.ipv4SubnetSize) || has(self.ipv4SubnetSize)", message="ipv4SubnetSize is required once set" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.accessMode) || has(self.accessMode)", message="accessMode is required once set" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.ipAddresses) || has(self.ipAddresses)", message="ipAddresses is required once set" @@ -40,15 +40,17 @@ type SubnetSpec struct { // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" IPAddresses []string `json:"ipAddresses,omitempty"` - // DHCP mode of a Subnet cannot switch from DHCPDeactivated to DHCPServer or DHCPRelay. + // DHCP mode of a Subnet can only switch between DHCPServer or DHCPRelay. // If subnetDHCPConfig is not set, the DHCP mode is DHCPDeactivated by default. // In order to enforce this rule, three XValidation rules are defined. // The rule on SubnetSpec prevents the condition that subnetDHCPConfig is not set in - // old SubnetSpec while the new SubnetSpec specifies a Mode other than DHCPDeactivated. + // old or current SubnetSpec while the current or old SubnetSpec specifies a Mode + // other than DHCPDeactivated. // The rule on SubnetDHCPConfig prevents the condition that Mode is not set in old - // SubnetDHCPConfig while the new one specifies a Mode other than DHCPDeactivated. - // The rule on SubnetDHCPConfig.Mode prevents the Mode changing from DHCPDeactivated - // to DHCPServer or DHCPRelay. + // or current SubnetDHCPConfig while the current or old one specifies a Mode other + // than DHCPDeactivated. + // The rule on SubnetDHCPConfig.Mode prevents the Mode changing between DHCPDeactivated + // and DHCPServer or DHCPRelay. // DHCP configuration for Subnet. SubnetDHCPConfig SubnetDHCPConfig `json:"subnetDHCPConfig,omitempty"` @@ -74,6 +76,7 @@ type SubnetStatus struct { // +kubebuilder:printcolumn:name="AccessMode",type=string,JSONPath=`.spec.accessMode`,description="Access mode of Subnet" // +kubebuilder:printcolumn:name="IPv4SubnetSize",type=string,JSONPath=`.spec.ipv4SubnetSize`,description="Size of Subnet" // +kubebuilder:printcolumn:name="NetworkAddresses",type=string,JSONPath=`.status.networkAddresses[*]`,description="CIDRs for the Subnet" +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec) || has(self.spec)", message="spec is required once set" type Subnet struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -92,12 +95,12 @@ type SubnetList struct { } // SubnetDHCPConfig is DHCP configuration for Subnet. -// +kubebuilder:validation:XValidation:rule="has(oldSelf.mode) || !has(self.mode) || self.mode=='DHCPDeactivated'", message="subnetDHCPConfig cannot switch from DHCPDeactivated to other modes" +// +kubebuilder:validation:XValidation:rule="has(oldSelf.mode)==has(self.mode) || (has(oldSelf.mode) && !has(self.mode) && oldSelf.mode=='DHCPDeactivated') || (!has(oldSelf.mode) && has(self.mode) && self.mode=='DHCPDeactivated')", message="subnetDHCPConfig mode can only switch between DHCPServer and DHCPRelay" type SubnetDHCPConfig struct { // DHCP Mode. DHCPDeactivated will be used if it is not defined. // It cannot switch from DHCPDeactivated to DHCPServer or DHCPRelay. // +kubebuilder:validation:Enum=DHCPServer;DHCPRelay;DHCPDeactivated - // +kubebuilder:validation:XValidation:rule="oldSelf!='DHCPDeactivated' || oldSelf==self", message="subnetDHCPConfig cannot switch from DHCPDeactivated to other modes" + // +kubebuilder:validation:XValidation:rule="oldSelf!='DHCPDeactivated' && self!='DHCPDeactivated' || oldSelf==self", message="subnetDHCPConfig mode can only switch between DHCPServer and DHCPRelay" Mode DHCPConfigMode `json:"mode,omitempty"` } diff --git a/pkg/apis/vpc/v1alpha1/subnetset_types.go b/pkg/apis/vpc/v1alpha1/subnetset_types.go index f64160151..9c489ca54 100644 --- a/pkg/apis/vpc/v1alpha1/subnetset_types.go +++ b/pkg/apis/vpc/v1alpha1/subnetset_types.go @@ -8,7 +8,7 @@ import ( ) // SubnetSetSpec defines the desired state of SubnetSet. -// +kubebuilder:validation:XValidation:rule="has(oldSelf.subnetDHCPConfig) || !has(self.subnetDHCPConfig) || !has(self.subnetDHCPConfig.mode) || self.subnetDHCPConfig.mode=='DHCPDeactivated'", message="subnetDHCPConfig cannot switch from DHCPDeactivated to other modes" +// +kubebuilder:validation:XValidation:rule="has(oldSelf.subnetDHCPConfig)==has(self.subnetDHCPConfig) || (has(oldSelf.subnetDHCPConfig) && !has(self.subnetDHCPConfig) && (!has(oldSelf.subnetDHCPConfig.mode) || oldSelf.subnetDHCPConfig.mode=='DHCPDeactivated')) || (!has(oldSelf.subnetDHCPConfig) && has(self.subnetDHCPConfig) && (!has(self.subnetDHCPConfig.mode) || self.subnetDHCPConfig.mode=='DHCPDeactivated'))", message="subnetDHCPConfig mode can only switch between DHCPServer and DHCPRelay" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.accessMode) || has(self.accessMode)", message="accessMode is required once set" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.ipv4SubnetSize) || has(self.ipv4SubnetSize)", message="ipv4SubnetSize is required once set" type SubnetSetSpec struct { @@ -21,15 +21,17 @@ type SubnetSetSpec struct { // +kubebuilder:validation:Enum=Private;Public;PrivateTGW // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" AccessMode AccessMode `json:"accessMode,omitempty"` - // DHCP mode of a SubnetSet cannot switch from DHCPDeactivated to DHCPServer or DHCPRelay. + // DHCP mode of a Subnet can only switch between DHCPServer or DHCPRelay. // If subnetDHCPConfig is not set, the DHCP mode is DHCPDeactivated by default. // In order to enforce this rule, three XValidation rules are defined. - // The rule on SubnetSetSpec prevents the condition that subnetDHCPConfig is not set in - // old SubnetSetSpec while the new SubnetSetSpec specifies a Mode other than DHCPDeactivated. + // The rule on SubnetSpec prevents the condition that subnetDHCPConfig is not set in + // old or current SubnetSpec while the current or old SubnetSpec specifies a Mode + // other than DHCPDeactivated. // The rule on SubnetDHCPConfig prevents the condition that Mode is not set in old - // SubnetDHCPConfig while the new one specifies a Mode other than DHCPDeactivated. - // The rule on SubnetDHCPConfig.Mode prevents the Mode changing from DHCPDeactivated - // to DHCPServer or DHCPRelay. + // or current SubnetDHCPConfig while the current or old one specifies a Mode other + // than DHCPDeactivated. + // The rule on SubnetDHCPConfig.Mode prevents the Mode changing between DHCPDeactivated + // and DHCPServer or DHCPRelay. // Subnet DHCP configuration. SubnetDHCPConfig SubnetDHCPConfig `json:"subnetDHCPConfig,omitempty"` @@ -60,6 +62,7 @@ type SubnetSetStatus struct { // +kubebuilder:printcolumn:name="AccessMode",type=string,JSONPath=`.spec.accessMode`,description="Access mode of Subnet" // +kubebuilder:printcolumn:name="IPv4SubnetSize",type=string,JSONPath=`.spec.ipv4SubnetSize`,description="Size of Subnet" // +kubebuilder:printcolumn:name="NetworkAddresses",type=string,JSONPath=`.status.subnets[*].networkAddresses[*]`,description="CIDRs for the SubnetSet" +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec) || has(self.spec)", message="spec is required once set" type SubnetSet struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/test/e2e/nsx_subnet_test.go b/test/e2e/nsx_subnet_test.go index c47c6f956..be684e87e 100644 --- a/test/e2e/nsx_subnet_test.go +++ b/test/e2e/nsx_subnet_test.go @@ -341,21 +341,6 @@ func SubnetCIDR(t *testing.T) { nsxSubnets = testData.fetchSubnetBySubnetUID(t, newSubnetCRUID) require.Equal(t, 1, len(nsxSubnets)) - // Change the DHCP mode from DHCPServer to DHCPDeactived - allocatedSubnet.Spec.SubnetDHCPConfig.Mode = v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeDeactivated) - _, err = testData.crdClientset.CrdV1alpha1().Subnets(subnetTestNamespace).Update(context.TODO(), allocatedSubnet, v1.UpdateOptions{}) - require.NoError(t, err) - allocatedSubnet = assureSubnet(t, subnetTestNamespace, subnet.Name, v1alpha1.DHCPConfigModeDeactivated) - nsxSubnets = testData.fetchSubnetBySubnetUID(t, newSubnetCRUID) - require.Equal(t, 1, len(nsxSubnets)) - require.Equal(t, "DHCP_DEACTIVATED", *nsxSubnets[0].SubnetDhcpConfig.Mode) - require.Equal(t, true, *nsxSubnets[0].AdvancedConfig.StaticIpAllocation.Enabled) - - // Change the DHCP mode from DHCPDeactived to DHCPServer - allocatedSubnet.Spec.SubnetDHCPConfig.Mode = v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeServer) - _, err = testData.crdClientset.CrdV1alpha1().Subnets(subnetTestNamespace).Update(context.TODO(), allocatedSubnet, v1.UpdateOptions{}) - require.Contains(t, err.Error(), "subnetDHCPConfig cannot switch from DHCPDeactivated to other modes") - // Delete the Subnet err = testData.crdClientset.CrdV1alpha1().Subnets(subnetTestNamespace).Delete(context.TODO(), subnet.Name, v1.DeleteOptions{}) require.NoError(t, err)