Introduce optional DefaultNamespaceConfiguration to WorkloadNetworkConfiguration - #61
Conversation
Adds an optional defaultNamespaceConfiguration field to NetworkProviderEntry, letting the vpc provider's namespace-onboarding defaults (currently just privateCIDRs) be changed independently of the append-only/immutable systemConfiguration.vpcConfig, per the design spec (docs/superpowers/specs/2026-06-30-default-namespace-configuration-design.md). NetworkProviderDefaultConfig is a value type with omitzero (not a pointer) to satisfy KAL's optionalfields check, now that the controller-tools bump makes required-field generation independent of that choice. This required guarding the new CEL rule's nested has() check at both levels (self.defaultNamespaceConfiguration and .vpcConfig) rather than just the leaf, since has() errors instead of returning false when the intermediate object is entirely absent from the wire representation (as it will be for any WNC that doesn't set defaultNamespaceConfiguration at all). CEL test coverage for the new field lives in test/cel/workloadnetworkconfiguration_test.go, appended to the ported suite: valid override, rejection on non-vpc types, MinProperties=1 and MinItems=1 rejection (via unstructured, since typed-client omitempty/omitzero would otherwise drop the very empty values under test), and full-replace admission (no append-only constraint, unlike the system config).
|
It is very likely I don't fully grasp the main idea of this change yet. IIUC, the idea here is to
If true, I wonder if we should generalize the API to have versioning capability or "mutable vs immutable NNC fields"? |
The system configuration directly influences the The idea here is we are describing what is primarily a structured ConfigMap - with a primary System configuration, and overridable defaults for new namespaces. IOW - the system configuration is the baseline configuration. New namespaces can have differing, overridden values that correlate to settings in the system configuration. There is a side benefit to this model which is also to reflect how Simplified Supervisor is encoded, which does not actually have a system workload network configuration (we consider it management only) - although this change does not yet bring in that support. |
| // +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"` |
There was a problem hiding this comment.
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?
yangna0420
left a comment
yangna0420
left a comment
There was a problem hiding this comment.
LGTM, but seems This branch has conflicts that must be resolved.
3 participants
This change introduces an addition to the
WorkloadNetworkConfigurationCR to add optionalDefaultNamespaceConfigurationin the case of a VPC environment.This is to retain backwards-compatibility in the Supervisor where
defaultPrivateCIDRsmay be overwritten for new namespaces, disparate from existing configurations.The logic is as such:
SystemConfigurationdirectly influences thesystemNamespaceNetworkConfigurationthat gets constructed. We cannot support mutation on this field simply, as downstream components do not support this.SystemConfigurationis also used to influence the defaults for new namespaces, read by namespace operator in the Supervisor. This change is providing an overlay on top of the system configuration defaults for very specific fields when applying to new namespaces.As such, a WorkloadNetworkConfiguration with provider
vpcmay have:/orgs/default/projects/foo/orgs/default/projects/foo/vpc-connectivity-profiles/bar[172.24.0.0/16]This will construct a
systemNamespaceNetworkConfigurationwith these same settings, and new namespaces would similarly adopt these settings. By applying this overlay, we can allow new namespaces to adopt a modified "Default Private CIDRs" without influencing thesystemNNC, which is non-modifiable.So, with the
defaultNamespaceConfiguration.defaultPrivateCIDRsset to something like[172.25.0.0/16], now a new namespace will use:/orgs/default/projects/foo/orgs/default/projects/foo/vpc-connectivity-profiles/bar[172.25.0.0/16]<-- Note that it's now172.25.0.0/16instead of172.24.0.0/16