From 0c83fc7c813a3163e3ae1fdb83c8935696c2d5f1 Mon Sep 17 00:00:00 2001 From: Bryan Venteicher Date: Mon, 15 Jun 2026 11:38:26 -0500 Subject: [PATCH] Update VM validation webhook to support multiple network providers When a namespace has been migrated, the prior (legacy) provider must still be supported as to not break existing VMs or VKS clusters. --- ...etoperator.vmware.com_networksettings.yaml | 20 ++ go.mod | 2 +- go.sum | 4 +- .../kube/networksettings/networksettings.go | 56 ++++- .../networksettings/networksettings_test.go | 213 +++++++++++++++++- test/e2e/go.mod | 2 +- test/e2e/go.sum | 4 +- .../validation/virtualmachine_validator.go | 102 ++++++--- .../virtualmachine_validator_unit_test.go | 163 ++++++++++++++ 9 files changed, 524 insertions(+), 42 deletions(-) diff --git a/config/crd/external-crds/netoperator.vmware.com_networksettings.yaml b/config/crd/external-crds/netoperator.vmware.com_networksettings.yaml index f20266ac05..8f79cd4ea6 100644 --- a/config/crd/external-crds/netoperator.vmware.com_networksettings.yaml +++ b/config/crd/external-crds/netoperator.vmware.com_networksettings.yaml @@ -37,6 +37,23 @@ spec: In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string + legacyProvider: + description: |- + legacyProvider is the network provider to which this namespace was previously affined. + When set, the namespace has transitioned from legacyProvider to the current provider. + APIs and resources associated with legacyProvider remain functional within this namespace + but are no longer the governing provider; new network resources will be created under + the current provider's APIs. + + This field is absent when the namespace has never undergone a provider transition. + enum: + - vsphere-distributed + - nsx-tier1 + - vpc + type: string + x-kubernetes-validations: + - message: legacyProvider must be vsphere-distributed or nsx-tier1 + rule: self in ['vsphere-distributed', 'nsx-tier1'] metadata: type: object provider: @@ -53,5 +70,8 @@ spec: required: - provider type: object + x-kubernetes-validations: + - message: legacyProvider must differ from provider + rule: '!has(self.legacyProvider) || self.legacyProvider != self.provider' served: true storage: true diff --git a/go.mod b/go.mod index 065b749564..927d087fd1 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/onsi/gomega v1.40.0 github.com/prometheus/client_golang v1.23.2 github.com/vmware-tanzu/image-registry-operator-api v0.0.0-20250813160346-0f6259af5cbb - github.com/vmware-tanzu/net-operator-api v0.0.0-20260521184348-f9c023dead14 + github.com/vmware-tanzu/net-operator-api v0.0.0-20260611174009-a2d7e608727d github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20260423081355-beab2417344a github.com/vmware/govmomi v0.55.0-alpha.0.0.20260518191903-48ab34adb211 golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 diff --git a/go.sum b/go.sum index efc136116b..ef5a4993b5 100644 --- a/go.sum +++ b/go.sum @@ -183,8 +183,8 @@ github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= github.com/vmware-tanzu/image-registry-operator-api v0.0.0-20250813160346-0f6259af5cbb h1:2dVpNgnwahauhEhRTrqEvN97URr79JHIel1dZ8WjpAk= github.com/vmware-tanzu/image-registry-operator-api v0.0.0-20250813160346-0f6259af5cbb/go.mod h1:sh4NJb1tCbzNRJ+ajRuu3thDovFN10Hic2wYmyklG/M= -github.com/vmware-tanzu/net-operator-api v0.0.0-20260521184348-f9c023dead14 h1:QdHz/WzDkNCzeN3sFyG9bdlNrn/j7osp3qgrwBMv+TU= -github.com/vmware-tanzu/net-operator-api v0.0.0-20260521184348-f9c023dead14/go.mod h1:w6QJGm3crIA16ZIz1FVQXD2NVeJhOgGXxW05RbVTSTo= +github.com/vmware-tanzu/net-operator-api v0.0.0-20260611174009-a2d7e608727d h1:JrTKl9lT9G62iZbPN/rqaisx5pR3TO+lif0GYiWU8G8= +github.com/vmware-tanzu/net-operator-api v0.0.0-20260611174009-a2d7e608727d/go.mod h1:w6QJGm3crIA16ZIz1FVQXD2NVeJhOgGXxW05RbVTSTo= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20260423081355-beab2417344a h1:yqGxhqSJ78veQjdOHINJLE9IWDcreMTzwDsOAdwrUWM= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20260423081355-beab2417344a/go.mod h1:Q4JzNkNMvjo7pXtlB5/R3oME4Nhah7fAObWgghVmtxk= github.com/vmware/govmomi v0.55.0-alpha.0.0.20260518191903-48ab34adb211 h1:n8hoHi/26x5GaTKTS04PqC7bNrCh7Wa7Eh44RKTM214= diff --git a/pkg/util/kube/networksettings/networksettings.go b/pkg/util/kube/networksettings/networksettings.go index 55833f3e3f..9d2f798aa7 100644 --- a/pkg/util/kube/networksettings/networksettings.go +++ b/pkg/util/kube/networksettings/networksettings.go @@ -43,16 +43,66 @@ func GetProviderType( return pkgcfg.FromContext(ctx).NetworkProviderType, nil } + ns, err := getNetworkSettings(ctx, reader, namespace) + if err != nil { + return "", err + } + + return providerToType(ns.Provider) +} + +// GetSupportedProviderTypes returns the supported NetworkProviderType for the given +// namespace. A namespace that underwent network provider migration, the prior (legacy) +// network provider is still supported. +func GetSupportedProviderTypes( + ctx context.Context, + reader ctrlclient.Reader, + namespace string) ([]pkgcfg.NetworkProviderType, error) { + + t := make([]pkgcfg.NetworkProviderType, 0, 2) + + if !pkgcfg.FromContext(ctx).Features.PerNamespaceNetworkProvider { + return append(t, pkgcfg.FromContext(ctx).NetworkProviderType), nil + } + + ns, err := getNetworkSettings(ctx, reader, namespace) + if err != nil { + return nil, err + } + + defaultProvider, err := providerToType(ns.Provider) + if err != nil { + return nil, err + } + + t = append(t, defaultProvider) + + if ns.LegacyProvider != "" { + legacyProvider, err := providerToType(ns.LegacyProvider) + if err != nil { + return nil, err + } + t = append(t, legacyProvider) + } + + return t, nil +} + +func getNetworkSettings( + ctx context.Context, + reader ctrlclient.Reader, + namespace string) (*netopv1alpha1.NetworkSettings, error) { + var ns netopv1alpha1.NetworkSettings err := reader.Get(ctx, ctrlclient.ObjectKey{Name: defaultNetworkSettingsName, Namespace: namespace}, &ns) if err != nil { if apierrors.IsNotFound(err) { - return "", ErrNetworkSettingsNotFound + return nil, ErrNetworkSettingsNotFound } - return "", err + return nil, err } - return providerToType(ns.Provider) + return &ns, nil } // providerToType maps a netopv1alpha1.NetworkProvider value to the diff --git a/pkg/util/kube/networksettings/networksettings_test.go b/pkg/util/kube/networksettings/networksettings_test.go index 751c5589a9..f6d5f68393 100644 --- a/pkg/util/kube/networksettings/networksettings_test.go +++ b/pkg/util/kube/networksettings/networksettings_test.go @@ -70,6 +70,7 @@ var _ = Describe("GetProviderType", func() { It("returns a not-found error", func() { Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(netsetutil.ErrNetworkSettingsNotFound)) + Expect(result).To(BeEmpty()) }) }) @@ -89,9 +90,10 @@ var _ = Describe("GetProviderType", func() { } }) - It("propagates the error", func() { + It("returns error", func() { Expect(err).To(HaveOccurred()) - Expect(apierrors.IsNotFound(err)).To(BeFalse()) + Expect(apierrors.IsServiceUnavailable(err)).To(BeTrue()) + Expect(result).To(BeEmpty()) }) }) @@ -164,3 +166,210 @@ var _ = Describe("GetProviderType", func() { }) }) }) + +var _ = Describe("GetSupportedProviderTypes", func() { + const namespace = "test-ns" + + var ( + ctx context.Context + reader ctrlclient.Reader + withObjects []ctrlclient.Object + withFuncs interceptor.Funcs + result []pkgcfg.NetworkProviderType + err error + ) + + BeforeEach(func() { + ctx = pkgcfg.NewContext() + withFuncs = interceptor.Funcs{} + withObjects = nil + }) + + JustBeforeEach(func() { + reader = builder.NewFakeClientWithInterceptors(withFuncs, withObjects...) + result, err = netsetutil.GetSupportedProviderTypes(ctx, reader, namespace) + }) + + When("PerNamespaceNetworkProvider capability is disabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.PerNamespaceNetworkProvider = false + config.NetworkProviderType = pkgcfg.NetworkProviderTypeVDS + }) + }) + + It("returns a single-element slice with the global network provider config value", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(ConsistOf(pkgcfg.NetworkProviderTypeVDS)) + }) + }) + + When("PerNamespaceNetworkProvider capability is enabled", func() { + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.PerNamespaceNetworkProvider = true + }) + }) + + When("NetworkSettings/default does not exist", func() { + It("returns a not-found error", func() { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(netsetutil.ErrNetworkSettingsNotFound)) + Expect(result).To(BeNil()) + }) + }) + + When("the client returns an unexpected error", func() { + BeforeEach(func() { + withFuncs.Get = func( + ctx context.Context, + client ctrlclient.WithWatch, + key ctrlclient.ObjectKey, + obj ctrlclient.Object, + opts ...ctrlclient.GetOption) error { + + if _, ok := obj.(*netopv1alpha1.NetworkSettings); ok { + return apierrors.NewServiceUnavailable("fake error") + } + return client.Get(ctx, key, obj, opts...) + } + }) + + It("propagates the error", func() { + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).To(BeFalse()) + Expect(result).To(BeNil()) + }) + }) + + When("NetworkSettings/default has provider vsphere-distributed and no legacy provider", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: netopv1alpha1.NetworkProviderVSphereDistributed, + }) + }) + + It("returns a single-element slice with NetworkProviderTypeVDS", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(ConsistOf(pkgcfg.NetworkProviderTypeVDS)) + }) + }) + + When("NetworkSettings/default has provider nsx-tier1 and no legacy provider", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: netopv1alpha1.NetworkProviderNSXTier1, + }) + }) + + It("returns a single-element slice with NetworkProviderTypeNSXT", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(ConsistOf(pkgcfg.NetworkProviderTypeNSXT)) + }) + }) + + When("NetworkSettings/default has provider vpc and no legacy provider", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: netopv1alpha1.NetworkProviderVPC, + }) + }) + + It("returns a single-element slice with NetworkProviderTypeVPC", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(ConsistOf(pkgcfg.NetworkProviderTypeVPC)) + }) + }) + + When("NetworkSettings/default has provider vpc and legacy provider vsphere-distributed", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: netopv1alpha1.NetworkProviderVPC, + LegacyProvider: netopv1alpha1.NetworkProviderVSphereDistributed, + }) + }) + + It("returns NetworkProviderTypeVPC and NetworkProviderTypeVDS", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(ConsistOf( + pkgcfg.NetworkProviderTypeVPC, + pkgcfg.NetworkProviderTypeVDS, + )) + }) + }) + + When("NetworkSettings/default has provider vpc and legacy provider nsx-tier1", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: netopv1alpha1.NetworkProviderVPC, + LegacyProvider: netopv1alpha1.NetworkProviderNSXTier1, + }) + }) + + It("returns NetworkProviderTypeVPC and NetworkProviderTypeNSXT", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(ConsistOf( + pkgcfg.NetworkProviderTypeVPC, + pkgcfg.NetworkProviderTypeNSXT, + )) + }) + }) + + When("NetworkSettings/default has an unknown provider value", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: "unknown-provider", + }) + }) + + It("returns an error", func() { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("unknown network provider"))) + Expect(result).To(BeNil()) + }) + }) + + When("NetworkSettings/default has a valid provider and an unknown legacy provider value", func() { + BeforeEach(func() { + withObjects = append(withObjects, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Provider: netopv1alpha1.NetworkProviderVPC, + LegacyProvider: "unknown-legacy-provider", + }) + }) + + It("returns an error", func() { + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("unknown network provider"))) + Expect(result).To(BeNil()) + }) + }) + }) +}) diff --git a/test/e2e/go.mod b/test/e2e/go.mod index 2b76f0795c..5b15d8afd0 100644 --- a/test/e2e/go.mod +++ b/test/e2e/go.mod @@ -56,7 +56,7 @@ require ( github.com/onsi/ginkgo/v2 v2.28.1 github.com/onsi/gomega v1.40.0 github.com/sirupsen/logrus v1.9.4 - github.com/vmware-tanzu/net-operator-api v0.0.0-20260521184348-f9c023dead14 + github.com/vmware-tanzu/net-operator-api v0.0.0-20260611174009-a2d7e608727d github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20260423081355-beab2417344a github.com/vmware-tanzu/vm-operator/api v0.0.0-00010101000000-000000000000 github.com/vmware-tanzu/vm-operator/external/image-registry-operator v0.0.0-00010101000000-000000000000 diff --git a/test/e2e/go.sum b/test/e2e/go.sum index a99fd53043..6e83c5174d 100644 --- a/test/e2e/go.sum +++ b/test/e2e/go.sum @@ -216,8 +216,8 @@ github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= -github.com/vmware-tanzu/net-operator-api v0.0.0-20260521184348-f9c023dead14 h1:QdHz/WzDkNCzeN3sFyG9bdlNrn/j7osp3qgrwBMv+TU= -github.com/vmware-tanzu/net-operator-api v0.0.0-20260521184348-f9c023dead14/go.mod h1:w6QJGm3crIA16ZIz1FVQXD2NVeJhOgGXxW05RbVTSTo= +github.com/vmware-tanzu/net-operator-api v0.0.0-20260611174009-a2d7e608727d h1:JrTKl9lT9G62iZbPN/rqaisx5pR3TO+lif0GYiWU8G8= +github.com/vmware-tanzu/net-operator-api v0.0.0-20260611174009-a2d7e608727d/go.mod h1:w6QJGm3crIA16ZIz1FVQXD2NVeJhOgGXxW05RbVTSTo= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20260423081355-beab2417344a h1:yqGxhqSJ78veQjdOHINJLE9IWDcreMTzwDsOAdwrUWM= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20260423081355-beab2417344a/go.mod h1:Q4JzNkNMvjo7pXtlB5/R3oME4Nhah7fAObWgghVmtxk= github.com/vmware/govmomi v0.55.0-alpha.0.0.20260518191903-48ab34adb211 h1:n8hoHi/26x5GaTKTS04PqC7bNrCh7Wa7Eh44RKTM214= diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator.go b/webhooks/virtualmachine/validation/virtualmachine_validator.go index 45876c044e..06a1ff2bca 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/validation/virtualmachine_validator.go @@ -882,7 +882,7 @@ func (v validator) validateNetwork( } if len(networkSpec.Interfaces) > 0 { - providerType, err := v.getNetworkProviderType(ctx, vm.Namespace) + providerTypes, err := v.getSupportedNetworkProviderTypes(ctx, vm.Namespace) if err != nil { return nil, err } @@ -890,8 +890,9 @@ func (v validator) validateNetwork( p := networkPath.Child("interfaces") for i, interfaceSpec := range networkSpec.Interfaces { - allErrs = append(allErrs, v.validateNetworkInterfaceSpec(ctx, p.Index(i), interfaceSpec, vm.Name, providerType)...) - allErrs = append(allErrs, v.validateNetworkInterfaceSpecWithBootstrap(ctx, p.Index(i), interfaceSpec, vm)...) + p := p.Index(i) + allErrs = append(allErrs, v.validateNetworkInterfaceSpec(ctx, p, interfaceSpec, vm.Name, providerTypes)...) + allErrs = append(allErrs, v.validateNetworkInterfaceSpecWithBootstrap(ctx, p, interfaceSpec, vm)...) } } @@ -1044,18 +1045,16 @@ func (v validator) validateNetworkVLANs( return allErrs } -// getNetworkProviderType returns the network provider type for the given -// namespace. When PerNamespaceNetworkProvider is disabled it reads the global -// config; when enabled it fetches NetworkSettings/default from that namespace. -func (v validator) getNetworkProviderType( +// getSupportedNetworkProviderTypes returns all network provider types for the +// given namespace. When PerNamespaceNetworkProvider is disabled it returns the +// global config value as a single-element slice; when enabled it fetches +// NetworkSettings/default from that namespace, returning the primary provider +// and, if set, the legacy provider as well. +func (v validator) getSupportedNetworkProviderTypes( ctx *pkgctx.WebhookRequestContext, - namespace string) (pkgcfg.NetworkProviderType, error) { + namespace string) ([]pkgcfg.NetworkProviderType, error) { - providerType, err := netsetutil.GetProviderType(ctx, v.client, namespace) - if err != nil { - return "", err - } - return providerType, nil + return netsetutil.GetSupportedProviderTypes(ctx, v.client, namespace) } // Note the code for VDS is basically done, but only support this for VPC right @@ -1091,35 +1090,76 @@ var networkProviderValidations = map[pkgcfg.NetworkProviderType]networkProviderV func (v validator) validateNetworkInterfaceNetworkRef( _ *pkgctx.WebhookRequestContext, - providerType pkgcfg.NetworkProviderType, + providerTypes []pkgcfg.NetworkProviderType, interfacePath *field.Path, networkGV schema.GroupVersion, networkKind string) field.ErrorList { var allErrs field.ErrorList - supported, ok := networkProviderValidations[providerType] - if !ok { - // No supported for this provider type (e.g., Named network provider). + // Collect validations for all supported provider types. + var validations []networkProviderValidation + for _, pt := range providerTypes { + if v, ok := networkProviderValidations[pt]; ok { + validations = append(validations, v) + } + } + + if len(validations) == 0 { + // No validation for any provider type (e.g., Named network provider). return allErrs } - if networkGV.Group != "" && networkGV.Group != supported.group { + // validationsForKind is the subset of validations used to check the kind. + // When a group is specified and matches one or more providers, only that + // group's kinds are valid. Without a group, all supported kinds apply. + validationsForKind := validations + + if networkGV.Group != "" { // We don't care about the version for anything but show the user provided // APIVersion since that is the field the group comes from. For supported, // just show the version we're importing which is OK'ish since none of the // providers have moved past v1a1. - allErrs = append(allErrs, field.NotSupported( - interfacePath.Child("network", "apiVersion"), - networkGV.String(), - []string{supported.apiVersion})) + var matchingValidations []networkProviderValidation + for _, val := range validations { + if networkGV.Group == val.group { + matchingValidations = append(matchingValidations, val) + } + } + if len(matchingValidations) == 0 { + supportedAPIVersions := make([]string, 0, len(validations)) + for _, val := range validations { + supportedAPIVersions = append(supportedAPIVersions, val.apiVersion) + } + allErrs = append(allErrs, field.NotSupported( + interfacePath.Child("network", "apiVersion"), + networkGV.String(), + supportedAPIVersions)) + // validationsForKind intentionally stays as all validations here so + // we avoid reporting a spurious kind error when the group is wrong. + } else { + // Group matched: constrain the kind check to this group's provider(s). + validationsForKind = matchingValidations + } } - if networkKind != "" && !slices.Contains(supported.kinds, networkKind) { - allErrs = append(allErrs, field.NotSupported( - interfacePath.Child("network", "kind"), - networkKind, - supported.kinds)) + if networkKind != "" { + var supportedKinds []string + kindMatchesAny := false + for _, val := range validationsForKind { + for _, k := range val.kinds { + supportedKinds = append(supportedKinds, k) + if networkKind == k { + kindMatchesAny = true + } + } + } + if !kindMatchesAny { + allErrs = append(allErrs, field.NotSupported( + interfacePath.Child("network", "kind"), + networkKind, + supportedKinds)) + } } return allErrs @@ -1131,7 +1171,7 @@ func (v validator) validateNetworkInterfaceSpec( interfacePath *field.Path, interfaceSpec vmopv1.VirtualMachineNetworkInterfaceSpec, vmName string, - providerType pkgcfg.NetworkProviderType) field.ErrorList { + providerTypes []pkgcfg.NetworkProviderType) field.ErrorList { var ( allErrs field.ErrorList @@ -1158,7 +1198,7 @@ func (v validator) validateNetworkInterfaceSpec( } allErrs = append(allErrs, - v.validateNetworkInterfaceNetworkRef(ctx, providerType, interfacePath, networkGV, networkKind)...) + v.validateNetworkInterfaceNetworkRef(ctx, providerTypes, interfacePath, networkGV, networkKind)...) // The networkInterface CR name ("vmName-networkName-interfaceName" or "vmName-interfaceName") needs to be a DNS1123 Label if networkName != "" { @@ -2127,13 +2167,13 @@ func (v validator) validateReadinessProbe( if probe.TCPSocket != nil { tcpSocketPath := readinessProbePath.Child("tcpSocket") - providerType, err := v.getNetworkProviderType(ctx, vm.Namespace) + providerTypes, err := v.getSupportedNetworkProviderTypes(ctx, vm.Namespace) if err != nil { return nil, err } // TCP readiness probe is not allowed under VPC Networking - if providerType == pkgcfg.NetworkProviderTypeVPC { + if slices.Contains(providerTypes, pkgcfg.NetworkProviderTypeVPC) { allErrs = append(allErrs, field.Forbidden(tcpSocketPath, tcpReadinessProbeNotAllowedVPC)) } else if probe.TCPSocket.Port.IntValue() != allowedRestrictedNetworkTCPProbePort { // Validate port if environment is a restricted network environment between SV CP VMs and Workload VMs e.g. VMC. diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go b/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go index 649bce2aa4..cbb2bc25ae 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go +++ b/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go @@ -3407,6 +3407,169 @@ func unitTestsValidateCreate() { Entry("vpc capability", pkgcfg.NetworkProviderTypeVPC, true), ) + DescribeTableSubtree("network create - validate network provider API group and kind with legacy provider", + func( + primaryProviderType pkgcfg.NetworkProviderType, + legacyProviderType pkgcfg.NetworkProviderType, + ) { + var ( + primaryCapProvider netopv1alpha1.NetworkProvider + legacyCapProvider netopv1alpha1.NetworkProvider + + primaryAPIVersion string + primaryKind string + + legacyAPIVersion string + legacyKind string + + unrelatedAPIVersion string + unrelatedKind string + ) + + //nolint:goconst + BeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.PerNamespaceNetworkProvider = true + }) + + switch primaryProviderType { + case pkgcfg.NetworkProviderTypeVPC: + primaryCapProvider = netopv1alpha1.NetworkProviderVPC + primaryAPIVersion = "crd.nsx.vmware.com/v1alpha1" + primaryKind = "SubnetSet" + default: + Fail(fmt.Sprintf("unsupported primaryProviderType: %v", primaryProviderType)) + } + + switch legacyProviderType { + case pkgcfg.NetworkProviderTypeVDS: + legacyCapProvider = netopv1alpha1.NetworkProviderVSphereDistributed + legacyAPIVersion = "netoperator.vmware.com/v1alpha1" + legacyKind = "Network" + unrelatedAPIVersion = "vmware.com/v1alpha1" + unrelatedKind = "VirtualNetwork" + case pkgcfg.NetworkProviderTypeNSXT: + legacyCapProvider = netopv1alpha1.NetworkProviderNSXTier1 + legacyAPIVersion = "vmware.com/v1alpha1" + legacyKind = "VirtualNetwork" + unrelatedAPIVersion = "netoperator.vmware.com/v1alpha1" + unrelatedKind = "Network" + } + + Expect(ctx.Client.Create(ctx, &netopv1alpha1.NetworkSettings{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: ctx.vm.Namespace, + }, + Provider: primaryCapProvider, + LegacyProvider: legacyCapProvider, + })).To(Succeed()) + }) + + It("allows an interface using the primary provider's APIVersion and Kind", func() { + ctx.vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{ + Interfaces: []vmopv1.VirtualMachineNetworkInterfaceSpec{ + { + Name: "eth0", + Network: &common.PartialObjectRef{ + TypeMeta: metav1.TypeMeta{ + APIVersion: primaryAPIVersion, + Kind: primaryKind, + }, + Name: "my-network", + }, + }, + }, + } + var err error + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vm) + Expect(err).ToNot(HaveOccurred()) + Expect(ctx.ValidateCreate(&ctx.WebhookRequestContext).Allowed).To(BeTrue()) + }) + + It("allows an interface using the legacy provider's APIVersion and Kind", func() { + ctx.vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{ + Interfaces: []vmopv1.VirtualMachineNetworkInterfaceSpec{ + { + Name: "eth0", + Network: &common.PartialObjectRef{ + TypeMeta: metav1.TypeMeta{ + APIVersion: legacyAPIVersion, + Kind: legacyKind, + }, + Name: "my-network", + }, + }, + }, + } + var err error + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vm) + Expect(err).ToNot(HaveOccurred()) + Expect(ctx.ValidateCreate(&ctx.WebhookRequestContext).Allowed).To(BeTrue()) + }) + + It("denies an interface using an unrelated provider's APIVersion and Kind", func() { + ctx.vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{ + Interfaces: []vmopv1.VirtualMachineNetworkInterfaceSpec{ + { + Name: "eth0", + Network: &common.PartialObjectRef{ + TypeMeta: metav1.TypeMeta{ + APIVersion: unrelatedAPIVersion, + Kind: unrelatedKind, + }, + Name: "my-network", + }, + }, + }, + } + var err error + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vm) + Expect(err).ToNot(HaveOccurred()) + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(BeFalse()) + // Error reasons should list both supported API versions + doValidateWithMsg(primaryAPIVersion, legacyAPIVersion)(response) + }) + + It("denies an interface whose group matches the legacy provider but whose kind belongs to the primary provider", func() { + // legacyAPIVersion group is valid, but primaryKind belongs to a + // different provider's group — the kind check must be scoped to + // the matched group, not the union of all providers' kinds. + ctx.vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{ + Interfaces: []vmopv1.VirtualMachineNetworkInterfaceSpec{ + { + Name: "eth0", + Network: &common.PartialObjectRef{ + TypeMeta: metav1.TypeMeta{ + APIVersion: legacyAPIVersion, + Kind: primaryKind, + }, + Name: "my-network", + }, + }, + }, + } + var err error + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vm) + Expect(err).ToNot(HaveOccurred()) + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(BeFalse()) + // Kind error lists only the kinds valid for the matched group. + doValidateWithMsg(legacyKind)(response) + }) + }, + + Entry("vpc primary, vsphere-distributed legacy", + pkgcfg.NetworkProviderTypeVPC, + pkgcfg.NetworkProviderTypeVDS, + ), + Entry("vpc primary, nsx-tier1 legacy", + pkgcfg.NetworkProviderTypeVPC, + pkgcfg.NetworkProviderTypeNSXT, + ), + ) + Context("when PerNamespaceNetworkProvider is enabled and NetworkSettings is absent", func() { BeforeEach(func() { pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) {