Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 49 additions & 0 deletions api/v1alpha1/workloadnetworkconfiguration_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const (
// +kubebuilder:validation:XValidation:rule="self.type == 'vsphere-distributed' || !has(self.systemConfiguration.vsphereDistributedConfig)",message="systemConfiguration.vsphereDistributedConfig may only be set when type is vsphere-distributed"
// +kubebuilder:validation:XValidation:rule="self.type != 'vpc' || has(self.systemConfiguration.vpcConfig)",message="systemConfiguration.vpcConfig must be set when type is vpc"
// +kubebuilder:validation:XValidation:rule="self.type == 'vpc' || !has(self.systemConfiguration.vpcConfig)",message="systemConfiguration.vpcConfig may only be set when type is vpc"
// +kubebuilder:validation:XValidation:rule="self.type == 'vpc' || !has(self.defaultNamespaceConfiguration) || !has(self.defaultNamespaceConfiguration.vpcConfig)",message="defaultNamespaceConfiguration.vpcConfig may only be set when type is vpc"

// NetworkProviderEntry pairs a network provider type with its system-level
// NamespaceNetworkConfiguration template. Exactly one entry per type is allowed
Expand All @@ -50,6 +51,54 @@ type NetworkProviderEntry struct {
//
// +required
SystemConfiguration *NamespaceNetworkConfig `json:"systemConfiguration,omitempty"`

// defaultNamespaceConfiguration optionally overrides configuration values
// applied when provisioning new namespaces under this provider, going
// forward. It does not alter the configuration of namespaces already
// associated under this provider. When unset, or when a given sub-field is
// unset, the equivalent value from systemConfiguration is used instead.
//
// Values here follow different mutability rules than systemConfiguration:
// they may be replaced freely and take effect only for namespaces onboarded
// after the change.
//
// +optional
DefaultNamespaceConfiguration NetworkProviderDefaultConfig `json:"defaultNamespaceConfiguration,omitempty,omitzero"`
}

// NetworkProviderDefaultConfig holds the subset of provider configuration that
// may be independently overridden for new namespaces, distinct from and
// unconstrained by the append-only/immutable rules that apply to
// systemConfiguration.
//
// +kubebuilder:validation:MinProperties=1
type NetworkProviderDefaultConfig struct {
// vpcConfig overrides default values used when auto-creating VPCs for new
// namespaces under the vpc provider.
//
// +optional
VPCConfig *DefaultVPCConfig `json:"vpcConfig,omitempty"`
}

// DefaultVPCConfig holds VPC default values that may be changed independently
// of the vpc provider's systemConfiguration.
type DefaultVPCConfig struct {
// privateCIDRs sets the CIDR blocks used as private-subnet/private-pod-IP
// defaults for VPCs auto-created for namespaces onboarding after this is
// set. Already-created namespace VPCs are unaffected.
//
// This value fully replaces (not appends to) the previous default and is
// not subject to the append-only constraint that applies to
// systemConfiguration.vpcConfig.autoCreateConfig.privateCIDRs. When unset,
// new namespaces use systemConfiguration's privateCIDRs instead.
//
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:items:MaxLength=64
// +kubebuilder:validation:items:Pattern=`^([0-9]{1,3}\.){3}[0-9]{1,3}/[0-9]{1,2}$`
// +listType=atomic
PrivateCIDRs []string `json:"privateCIDRs,omitempty"`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing this, I suddenly thought of a question:
Whether we need to do CEL validation about the CIDRs ? Such as defaultNamespaceConfiguration.vpcConfig.privateCIDRs and systemConfiguration.privateCIDRs overlapping conflict checking.

Besides, there is one more important thing.
Previously, wcpsvc was used to verify whether CIDRs overlapped. Currently, it seems that the nsx-operater side has not added this because it requires communication with the nsx-manager. Therefore, the nnc-controller has not added this check either. My svnc side has not added this verification either. Do we need to discuss with the nsx-operator team at which layer it is more appropriate to add this validation?

}

// +kubebuilder:validation:XValidation:rule="self.providers.exists(p, p.type == self.activeSystemProvider)",message="activeSystemProvider must reference a provider type declared in providers"
Expand Down
41 changes: 41 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

137 changes: 137 additions & 0 deletions test/cel/workloadnetworkconfiguration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,140 @@ func TestWorkloadNetworkConfiguration_ConditionStatusOutsideEnum_Rejected(t *tes
t.Fatalf("expected rejection containing %q, got: %v", "Unsupported value", err)
}
}

// -----------------------------------------------------------------------
// defaultNamespaceConfiguration
// Rule: self.type == 'vpc' || !has(self.defaultNamespaceConfiguration.vpcConfig)
// NetworkProviderDefaultConfig: +kubebuilder:validation:MinProperties=1
// DefaultVPCConfig.privateCIDRs: +optional, minItems=1, maxItems=16, CIDR pattern
// -----------------------------------------------------------------------

// vpcWNC builds a minimal valid vpc-provider WNC named "default".
func vpcWNC() *netv1alpha1.WorkloadNetworkConfiguration {
return &netv1alpha1.WorkloadNetworkConfiguration{
ObjectMeta: metav1.ObjectMeta{Name: wncDefaultName},
Spec: netv1alpha1.WorkloadNetworkConfigurationSpec{
Providers: []netv1alpha1.NetworkProviderEntry{
{
Type: netv1alpha1.NetworkProviderVPC,
SystemConfiguration: &netv1alpha1.NamespaceNetworkConfig{
VPCConfig: netv1alpha1.VPCConfig{
AutoCreateConfig: netv1alpha1.AutoCreateVPCConfig{
NSXProject: testNSXProject,
VPCConnectivityProfile: testVPCConnProfile,
PrivateCIDRs: []string{testCIDR1},
},
},
},
},
},
ActiveSystemProvider: netv1alpha1.NetworkProviderVPC,
},
}
}

func TestWorkloadNetworkConfiguration_ValidDefaultVPCConfig_Admitted(t *testing.T) {
wnc := vpcWNC()
wnc.Spec.Providers[0].DefaultNamespaceConfiguration = netv1alpha1.NetworkProviderDefaultConfig{
VPCConfig: &netv1alpha1.DefaultVPCConfig{
PrivateCIDRs: []string{"10.1.0.0/24"},
},
}
if err := k8sClient.Create(testCtx, wnc); err != nil {
t.Fatalf("expected admission, got: %v", err)
}
defer func() { _ = k8sClient.Delete(testCtx, wnc) }()
}

func TestWorkloadNetworkConfiguration_DefaultVPCConfigOnNonVPCType_Rejected(t *testing.T) {
wnc := vdsWNC()
wnc.Spec.Providers[0].DefaultNamespaceConfiguration = netv1alpha1.NetworkProviderDefaultConfig{
VPCConfig: &netv1alpha1.DefaultVPCConfig{
PrivateCIDRs: []string{"10.1.0.0/24"},
},
}
err := k8sClient.Create(testCtx, wnc)
if err == nil || !strings.Contains(err.Error(), "defaultNamespaceConfiguration.vpcConfig may only be set when type is vpc") {
t.Fatalf("expected rejection for defaultNamespaceConfiguration.vpcConfig on non-vpc type, got: %v", err)
}
}

func TestWorkloadNetworkConfiguration_EmptyDefaultNamespaceConfiguration_Rejected(t *testing.T) {
obj := unstrWNC(wncDefaultName, map[string]interface{}{
"activeSystemProvider": string(netv1alpha1.NetworkProviderVPC),
"providers": []interface{}{
map[string]interface{}{
"type": string(netv1alpha1.NetworkProviderVPC),
"systemConfiguration": map[string]interface{}{
"vpcConfig": map[string]interface{}{
"autoCreateConfig": map[string]interface{}{
"nsxProject": testNSXProject,
"vpcConnectivityProfile": testVPCConnProfile,
},
},
},
"defaultNamespaceConfiguration": map[string]interface{}{},
},
},
})
if err := k8sClient.Create(testCtx, obj); !isRejected(err) {
t.Fatalf("expected rejection for empty defaultNamespaceConfiguration (MinProperties=1), got: %v", err)
}
}

// TestWorkloadNetworkConfiguration_EmptyPrivateCIDRsList_Rejected sends the
// request as Unstructured because the typed client's `omitempty` drops an
// empty (but non-nil) []string entirely on marshal, which would silently
// turn this into "field absent" instead of testing the MinItems=1 rule.
func TestWorkloadNetworkConfiguration_EmptyPrivateCIDRsList_Rejected(t *testing.T) {
obj := unstrWNC(wncDefaultName, map[string]interface{}{
"activeSystemProvider": string(netv1alpha1.NetworkProviderVPC),
"providers": []interface{}{
map[string]interface{}{
"type": string(netv1alpha1.NetworkProviderVPC),
"systemConfiguration": map[string]interface{}{
"vpcConfig": map[string]interface{}{
"autoCreateConfig": map[string]interface{}{
"nsxProject": testNSXProject,
"vpcConnectivityProfile": testVPCConnProfile,
},
},
},
"defaultNamespaceConfiguration": map[string]interface{}{
"vpcConfig": map[string]interface{}{
"privateCIDRs": []interface{}{},
},
},
},
},
})
if err := k8sClient.Create(testCtx, obj); !isRejected(err) {
t.Fatalf("expected rejection for explicit empty privateCIDRs list (MinItems=1), got: %v", err)
}
}

func TestWorkloadNetworkConfiguration_DefaultPrivateCIDRsFullReplace_Admitted(t *testing.T) {
wnc := vpcWNC()
wnc.Spec.Providers[0].DefaultNamespaceConfiguration = netv1alpha1.NetworkProviderDefaultConfig{
VPCConfig: &netv1alpha1.DefaultVPCConfig{
PrivateCIDRs: []string{"10.1.0.0/24"},
},
}
if err := k8sClient.Create(testCtx, wnc); err != nil {
t.Fatalf("create: %v", err)
}
defer func() { _ = k8sClient.Delete(testCtx, wnc) }()

latest := &netv1alpha1.WorkloadNetworkConfiguration{}
if err := k8sClient.Get(testCtx, client.ObjectKeyFromObject(wnc), latest); err != nil {
t.Fatalf("get: %v", err)
}
// Full replace with a CIDR that was never in the original list. Unlike
// systemConfiguration.vpcConfig.autoCreateConfig.privateCIDRs (append-only),
// defaultNamespaceConfiguration.vpcConfig.privateCIDRs has no append-only
// constraint, so dropping 10.1.0.0/24 entirely must be admitted.
latest.Spec.Providers[0].DefaultNamespaceConfiguration.VPCConfig.PrivateCIDRs = []string{"10.2.0.0/24"}
if err := k8sClient.Update(testCtx, latest); err != nil {
t.Fatalf("expected admission for full-replace of defaultNamespaceConfiguration privateCIDRs, got: %v", err)
}
}