Skip to content

Check more property for pre-created subnet #1428

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

Open
TaoZou1 wants to merge 1 commit into
base: main
Choose a base branch
from
+345 −6
Open

Check more property for pre-created subnet #1428

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
38 changes: 32 additions & 6 deletions pkg/controllers/subnetset/subnetset_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (v *SubnetSetValidator) Handle(ctx context.Context, req admission.Request)
if isDefaultSubnetSet(subnetSet) && req.UserInfo.Username != NSXOperatorSA {
return admission.Denied("default SubnetSet only can be created by nsx-operator")
}
deny, err := v.validateSubnetNames(ctx, subnetSet.Namespace, subnetSet.Spec.SubnetNames, subnetSet.Name)
deny, err := v.validateSubnets(ctx, subnetSet.Namespace, subnetSet.Spec.SubnetNames, subnetSet.Name)
if err != nil {
if deny {
return admission.Denied(err.Error())
Expand All @@ -145,7 +145,7 @@ func (v *SubnetSetValidator) Handle(ctx context.Context, req admission.Request)
log.Debug("Default SubnetSet label change detected", "oldLabels", oldSubnetSet.ObjectMeta.Labels, "newLabels", subnetSet.ObjectMeta.Labels, "username", req.UserInfo.Username)
return admission.Denied(fmt.Sprintf("SubnetSet label %s can only be updated by NSX Operator", common.LabelDefaultNetwork))
}
deny, err := v.validateSubnetNames(ctx, subnetSet.Namespace, subnetSet.Spec.SubnetNames, subnetSet.Name)
deny, err := v.validateSubnets(ctx, subnetSet.Namespace, subnetSet.Spec.SubnetNames, subnetSet.Name)
if err != nil {
if deny {
return admission.Denied(err.Error())
Expand Down Expand Up @@ -322,10 +322,24 @@ func (v *SubnetSetValidator) getVPCPath(ns string) (string, error) {
// If any rule is broken, return true with error message.
// If there is error when getting resources, return false with error.

func (v *SubnetSetValidator) validateSubnetNames(ctx context.Context, ns string, subnetNames *[]string, subnetSet string) (bool, error) {
// getEffectiveStaticIPAllocation computes the effective StaticIPAllocation value for a subnet.
// This replicates the logic from subnet controller to avoid race conditions during validation.
func getEffectiveStaticIPAllocation(subnet *v1alpha1.Subnet) bool {
// If explicitly set, use the set value
if subnet.Spec.AdvancedConfig.StaticIPAllocation.Enabled != nil {
return *subnet.Spec.AdvancedConfig.StaticIPAllocation.Enabled
}
// If not set, compute default value based on DHCP config (same logic as subnet controller)
return !util.CRSubnetDHCPEnabled(subnet)
}

func (v *SubnetSetValidator) validateSubnets(ctx context.Context, ns string, subnetNames *[]string, subnetSet string) (bool, error) {
var namespaceVpc string
var existingVPC string
firstAccessMode := ""
firstDHCPMode := ""
var firstEffectiveStaticIPAllocation bool
isFirstSubnet := true
Comment thread
TaoZou1 marked this conversation as resolved.
if subnetNames == nil {
return true, nil
}
Expand All @@ -338,10 +352,22 @@ func (v *SubnetSetValidator) validateSubnetNames(ctx context.Context, ns string,
if crdSubnet.Spec.SubnetDHCPConfig.Mode == v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeRelay) {
return true, fmt.Errorf("DHCPRelay Subnet %s/%s is not supported in SubnetSet", crdSubnet.Namespace, crdSubnet.Name)
}
if firstAccessMode == "" {
if isFirstSubnet {
firstAccessMode = string(crdSubnet.Spec.AccessMode)
} else if firstAccessMode != string(crdSubnet.Spec.AccessMode) {
return true, fmt.Errorf("Subnets in SubnetSet %s/%s must have the same AccessMode, found different AccessModes: [%s, %s]", ns, subnetSet, firstAccessMode, crdSubnet.Spec.AccessMode)
firstDHCPMode = string(crdSubnet.Spec.SubnetDHCPConfig.Mode)
firstEffectiveStaticIPAllocation = getEffectiveStaticIPAllocation(crdSubnet)
isFirstSubnet = false
} else {
if firstAccessMode != string(crdSubnet.Spec.AccessMode) {
return true, fmt.Errorf("Subnets in SubnetSet %s/%s must have the same AccessMode, found different AccessModes: [%s, %s]", ns, subnetSet, firstAccessMode, crdSubnet.Spec.AccessMode)
}
if firstDHCPMode != string(crdSubnet.Spec.SubnetDHCPConfig.Mode) {
return true, fmt.Errorf("Subnets in SubnetSet %s/%s must have the same DHCPConfigMode, found different DHCPConfigModes: [%s, %s]", ns, subnetSet, firstDHCPMode, crdSubnet.Spec.SubnetDHCPConfig.Mode)
}
currentEffectiveStaticIPAllocation := getEffectiveStaticIPAllocation(crdSubnet)
if firstEffectiveStaticIPAllocation != currentEffectiveStaticIPAllocation {
return true, fmt.Errorf("Subnets in SubnetSet %s/%s must have the same StaticIPAllocation, found different StaticIPAllocations: [%t, %t]", ns, subnetSet, firstEffectiveStaticIPAllocation, currentEffectiveStaticIPAllocation)
}
}
var subnetVPC string
associatedResource, exists := crdSubnet.Annotations[common.AnnotationAssociatedResource]
Expand Down
Loading
Loading