Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cherry-pick]Update dhcp validation to prevent switch from DHCPServer/DHCPRelay to DHCPDeactivated (#996) #998

Merged
merged 1 commit into from
Jan 10, 2025
Merged

[cherry-pick]Update dhcp validation to prevent switch from DHCPServer/DHCPRelay to DHCPDeactivated (#996) #998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions build/yaml/crd/vpc/crd.nsx.vmware.com_subnets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
29 changes: 19 additions & 10 deletions build/yaml/crd/vpc/crd.nsx.vmware.com_subnetsets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
19 changes: 11 additions & 8 deletions pkg/apis/vpc/v1alpha1/subnet_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"`
Expand All @@ -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"`
Expand All @@ -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"`
}

Expand Down
17 changes: 10 additions & 7 deletions pkg/apis/vpc/v1alpha1/subnetset_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"`
Expand Down Expand Up @@ -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"`
Expand Down
15 changes: 0 additions & 15 deletions test/e2e/nsx_subnet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading