From 856449a454765fadb0cece65f670b251a7cb21f8 Mon Sep 17 00:00:00 2001 From: Wenying Dong Date: Mon, 24 Jun 2024 13:28:08 +0800 Subject: [PATCH] Add support for VPC mixed mode 1. Add controller for CR Network to maintain the default Network resource in system Namespaces. 2. Add watch in Pod/Namepsace/Service/NetworkPolicy controller to requeue resources when Network type is switch to VPC. 3. Add precheck on if the default Network type is VPC or not in resource create events. 4. Add webhook in NCP owned resource creations to check if the default Network type is VPC or not in a Namespace. --- build/yaml/webhook/manifests.yaml | 33 + cmd/main.go | 50 +- go.mod | 5 +- go.sum | 4 +- pkg/config/config.go | 9 +- .../namespace/namespace_controller.go | 30 +- .../namespace/namespace_controller_test.go | 8 +- .../networkpolicy/networkpolicy_controller.go | 49 +- pkg/controllers/pod/pod_controller.go | 35 +- .../service/service_lb_controller.go | 92 ++- .../service/service_lb_controller_test.go | 138 +++- .../subnetset/subnetset_controller.go | 9 +- pkg/controllers/vpcnetwork/interfaces.go | 17 + .../vpcnetwork/network_controller.go | 186 +++++ pkg/controllers/vpcnetwork/network_handler.go | 106 +++ pkg/controllers/vpcnetwork/network_test.go | 712 ++++++++++++++++++ pkg/controllers/vpcnetwork/network_webhook.go | 48 ++ .../vpcnetwork/testing/network_testing.go | 26 + pkg/nsx/services/common/types.go | 1 + pkg/util/utils.go | 13 + 20 files changed, 1486 insertions(+), 85 deletions(-) create mode 100644 pkg/controllers/vpcnetwork/interfaces.go create mode 100644 pkg/controllers/vpcnetwork/network_controller.go create mode 100644 pkg/controllers/vpcnetwork/network_handler.go create mode 100644 pkg/controllers/vpcnetwork/network_test.go create mode 100644 pkg/controllers/vpcnetwork/network_webhook.go create mode 100644 pkg/controllers/vpcnetwork/testing/network_testing.go diff --git a/build/yaml/webhook/manifests.yaml b/build/yaml/webhook/manifests.yaml index 71bc17b0b..81ed127d1 100644 --- a/build/yaml/webhook/manifests.yaml +++ b/build/yaml/webhook/manifests.yaml @@ -30,3 +30,36 @@ webhooks: resources: - subnetsets sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: subnetset + namespace: vmware-system-nsx + path: /validate-vpc-enablement + failurePolicy: Fail + name: vpcnetwork.validating.nsx.vmware.com + rules: + - apiGroups: + - nsx.vmware.com + apiVersions: + - v1alpha1 + operations: + - CREATE + resources: + - networkinfos + - nsxserviceaccount + - securitypolicies + - staticroutes + - subnetports + - subnets + - subnetsets + - apiGroups: + - nsx.vmware.com + apiVersions: + - v1alpha2 + operations: + - CREATE + resources: + - ippools + sideEffects: None diff --git a/cmd/main.go b/cmd/main.go index 1abec4603..1c6d9bc2f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -17,6 +17,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" logf "sigs.k8s.io/controller-runtime/pkg/log" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/webhook" "github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha1" "github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha2" @@ -35,6 +36,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnet" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnetport" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnetset" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" "github.com/vmware-tanzu/nsx-operator/pkg/nsx" @@ -133,12 +135,13 @@ func StartNetworkInfoController(mgr ctrl.Manager, vpcService *vpc.VPCService) { } } -func StartNamespaceController(mgr ctrl.Manager, cf *config.NSXOperatorConfig, vpcService common.VPCServiceProvider) { +func StartNamespaceController(mgr ctrl.Manager, cf *config.NSXOperatorConfig, vpcService common.VPCServiceProvider, networkProvider vpcnetwork.VPCNetworkProvider) { nsReconciler := &namespacecontroller.NamespaceReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - NSXConfig: cf, - VPCService: vpcService, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + NSXConfig: cf, + VPCService: vpcService, + NetworkProvider: networkProvider, } if err := nsReconciler.Start(mgr); err != nil { @@ -149,14 +152,26 @@ func StartNamespaceController(mgr ctrl.Manager, cf *config.NSXOperatorConfig, vp func main() { log.Info("starting NSX Operator") - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + mgrOptions := ctrl.Options{ Scheme: scheme, HealthProbeBindAddress: config.ProbeAddr, Metrics: metricsserver.Options{BindAddress: config.MetricsAddr}, LeaderElection: cf.HAEnabled(), LeaderElectionNamespace: nsxOperatorNamespace, LeaderElectionID: "nsx-operator", - }) + } + + enableWebhook := true + if _, err := os.Stat(config.WebhookCertDir); errors.Is(err, os.ErrNotExist) { + log.Error(err, "server cert not found, disabling webhook server", "cert", config.WebhookCertDir) + enableWebhook = false + } else { + mgrOptions.WebhookServer = webhook.NewServer(webhook.Options{ + Port: config.WebhookServerPort, + CertDir: config.WebhookCertDir, + }) + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOptions) if err != nil { log.Error(err, "failed to init manager") os.Exit(1) @@ -181,6 +196,10 @@ func main() { var vpcService *vpc.VPCService if cf.CoeConfig.EnableVPCNetwork { + if !enableWebhook { + log.Error(nil, "Webhook cert is not provided, can't filter out the CRs in a non-VPC namespace") + os.Exit(1) + } // Check NSX version for VPC networking mode if !commonService.NSXClient.NSXCheckVersion(nsx.VPC) { log.Error(nil, "VPC mode cannot be enabled if NSX version is lower than 4.1.1") @@ -218,17 +237,16 @@ func main() { os.Exit(1) } // Start controllers which only supports VPC + var vpcNetworkProvider vpcnetwork.VPCNetworkProvider + if cf.CoeConfig.EnableVPCMixedMode { + vpcNetworkProvider = vpcnetwork.StartNetworkController(mgr) + } StartNetworkInfoController(mgr, vpcService) - StartNamespaceController(mgr, cf, vpcService) + StartNamespaceController(mgr, cf, vpcService, vpcNetworkProvider) // Start subnet/subnetset controller. if err := subnet.StartSubnetController(mgr, subnetService, subnetPortService, vpcService); err != nil { os.Exit(1) } - enableWebhook := true - if _, err := os.Stat(config.WebhookCertDir); errors.Is(err, os.ErrNotExist) { - log.Error(err, "server cert not found, disabling webhook server", "cert", config.WebhookCertDir) - enableWebhook = false - } if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, enableWebhook); err != nil { os.Exit(1) } @@ -236,10 +254,10 @@ func main() { node.StartNodeController(mgr, nodeService) staticroutecontroller.StartStaticRouteController(mgr, staticRouteService) subnetport.StartSubnetPortController(mgr, subnetPortService, subnetService, vpcService) - pod.StartPodController(mgr, subnetPortService, subnetService, vpcService, nodeService) + pod.StartPodController(mgr, subnetPortService, subnetService, vpcService, nodeService, vpcNetworkProvider) StartIPPoolController(mgr, ipPoolService, vpcService) - networkpolicycontroller.StartNetworkPolicyController(mgr, commonService, vpcService) - service.StartServiceLbController(mgr, commonService) + networkpolicycontroller.StartNetworkPolicyController(mgr, commonService, vpcService, vpcNetworkProvider) + service.StartServiceLbController(mgr, commonService, vpcNetworkProvider) } // Start controllers which can run in non-VPC mode securitypolicycontroller.StartSecurityPolicyController(mgr, commonService, vpcService) diff --git a/go.mod b/go.mod index 0fd988151..e3d085ffb 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ replace ( github.com/vmware-tanzu/nsx-operator/pkg/apis => ./pkg/apis github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha1 => ./pkg/apis/v1alpha1 github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha2 => ./pkg/apis/v1alpha2 - github.com/vmware-tanzu/nsx-operator/pkg/client => ./pkg/client + github.com/vmware-tanzu/nsx-operator/pkg/client => ./pkg/client ) require ( @@ -14,6 +14,7 @@ require ( github.com/apparentlymart/go-cidr v1.1.0 github.com/deckarep/golang-set v1.8.0 github.com/go-logr/logr v1.3.0 + github.com/go-logr/zapr v1.2.4 github.com/golang-jwt/jwt v3.2.2+incompatible github.com/golang/mock v1.6.0 github.com/google/uuid v1.3.0 @@ -23,6 +24,7 @@ require ( github.com/prometheus/client_golang v1.16.0 github.com/sirupsen/logrus v1.9.0 github.com/stretchr/testify v1.8.4 + github.com/vmware-tanzu/net-operator-api v0.0.0-20240529180459-ccac5a20bda1 github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20240305035435-c992c623aad3 github.com/vmware-tanzu/nsx-operator/pkg/client v0.0.0-20240102061654-537b080e159f github.com/vmware-tanzu/vm-operator/api v1.8.2 @@ -53,7 +55,6 @@ require ( github.com/evanphx/json-patch/v5 v5.6.0 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/gibson042/canonicaljson-go v1.0.3 // indirect - github.com/go-logr/zapr v1.2.4 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect diff --git a/go.sum b/go.sum index e9a5b9a89..7b467928c 100644 --- a/go.sum +++ b/go.sum @@ -177,8 +177,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/vmware-tanzu/nsx-operator/pkg/client v0.0.0-20240102061654-537b080e159f h1:EV4eiUQr3QpUGfTtqdVph0+bmE+3cj0aNJpd9n2qTdo= -github.com/vmware-tanzu/nsx-operator/pkg/client v0.0.0-20240102061654-537b080e159f/go.mod h1:dzob8tUzpAREQPtbbjQs4b1UyQDR37B2TiIdg8WJSRM= +github.com/vmware-tanzu/net-operator-api v0.0.0-20240529180459-ccac5a20bda1 h1:r8RuSJnLEStOdGTfaZeOjH4rvRB4Gm/N1+qtU16VrI0= +github.com/vmware-tanzu/net-operator-api v0.0.0-20240529180459-ccac5a20bda1/go.mod h1:w6QJGm3crIA16ZIz1FVQXD2NVeJhOgGXxW05RbVTSTo= github.com/vmware-tanzu/vm-operator/api v1.8.2 h1:7cZHVusqAmAMFWvsiU7X5xontxdjasknI/sVfe0p0Z4= github.com/vmware-tanzu/vm-operator/api v1.8.2/go.mod h1:vauVboD3sQxP+pb28TnI9wfrj+0nH2zSEc9Q7AzWJ54= github.com/vmware/govmomi v0.27.4 h1:5kY8TAkhB20lsjzrjE073eRb8+HixBI29PVMG5lxq6I= diff --git a/pkg/config/config.go b/pkg/config/config.go index 2710b69c9..169661ee5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -94,8 +94,9 @@ type DefaultConfig struct { } type CoeConfig struct { - Cluster string `ini:"cluster"` - EnableVPCNetwork bool `ini:"enable_vpc_network"` + Cluster string `ini:"cluster"` + EnableVPCNetwork bool `ini:"enable_vpc_network"` + EnableVPCMixedMode bool `ini:"enable_vpc_mixed_mode"` } type NsxConfig struct { @@ -238,6 +239,10 @@ func (operatorConfig *NSXOperatorConfig) validate() error { if err := operatorConfig.NsxConfig.validate(operatorConfig.CoeConfig.EnableVPCNetwork); err != nil { return err } + if operatorConfig.CoeConfig.EnableVPCMixedMode && !operatorConfig.CoeConfig.EnableVPCNetwork { + configLog.Error("VPC mixed mode is enabled but VPC is not enabled") + return errors.New("VPC mixed mode can't be enabled without VPC network enablement") + } // TODO, verify if user&pwd, cert, jwt has any of them provided return nil } diff --git a/pkg/controllers/namespace/namespace_controller.go b/pkg/controllers/namespace/namespace_controller.go index 44c14309a..1261b704e 100644 --- a/pkg/controllers/namespace/namespace_controller.go +++ b/pkg/controllers/namespace/namespace_controller.go @@ -8,17 +8,21 @@ import ( "errors" "fmt" + netopv1alpha1 "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + apitypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha1" "github.com/vmware-tanzu/nsx-operator/pkg/config" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" _ "github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter" @@ -35,10 +39,11 @@ var ( // Using vpcservice provider instead of vpc service to prevent // invoking method that should be exposed to other module. type NamespaceReconciler struct { - Client client.Client - Scheme *apimachineryruntime.Scheme - NSXConfig *config.NSXOperatorConfig - VPCService types.VPCServiceProvider + Client client.Client + Scheme *apimachineryruntime.Scheme + NSXConfig *config.NSXOperatorConfig + VPCService types.VPCServiceProvider + NetworkProvider vpcnetwork.VPCNetworkProvider } func (r *NamespaceReconciler) getDefaultNetworkConfigName() (string, error) { @@ -179,6 +184,13 @@ func (r *NamespaceReconciler) insertNamespaceNetworkconfigBinding(ns string, ann return nil } +func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + if r.NetworkProvider != nil { + return r.NetworkProvider.ReconcileWithVPCFilters("namespace", ctx, req, r.reconcile) + } + return r.reconcile(ctx, req) +} + /* VPC creation strategy: @@ -193,7 +205,7 @@ We suppose namespace should have following annotations: - If the ns do not have either of the annotation above, then we believe it is using default VPC, try to search default VPC in network config CR store. The default VPC network config CR's name is "default". */ -func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *NamespaceReconciler) reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { obj := &v1.Namespace{} log.Info("reconciling K8s namespace", "namespace", req.NamespacedName) metrics.CounterInc(r.NSXConfig, metrics.ControllerSyncTotal, common.MetricResTypeNamespace) @@ -251,6 +263,14 @@ func (r *NamespaceReconciler) setupWithManager(mgr ctrl.Manager) error { controller.Options{ MaxConcurrentReconciles: common.NumReconcile(), }). + Watches( + &netopv1alpha1.Network{}, + &vpcnetwork.EnqueueRequestForNetwork{Client: r.Client, Lister: func(namespace string) ([]apitypes.NamespacedName, error) { + obj := apitypes.NamespacedName{Name: namespace, Namespace: namespace} + return []apitypes.NamespacedName{obj}, nil + }}, + builder.WithPredicates(vpcnetwork.PredicateFuncsByNetwork), + ). Complete(r) } diff --git a/pkg/controllers/namespace/namespace_controller_test.go b/pkg/controllers/namespace/namespace_controller_test.go index 9053d0797..40037ae0a 100644 --- a/pkg/controllers/namespace/namespace_controller_test.go +++ b/pkg/controllers/namespace/namespace_controller_test.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/vmware-tanzu/nsx-operator/pkg/config" + vpcnetworktesting "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork/testing" "github.com/vmware-tanzu/nsx-operator/pkg/nsx" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc" @@ -40,9 +41,10 @@ func createNameSpaceReconciler() *NamespaceReconciler { } return &NamespaceReconciler{ - Client: fake.NewClientBuilder().Build(), - Scheme: fake.NewClientBuilder().Build().Scheme(), - VPCService: service, + Client: fake.NewClientBuilder().Build(), + Scheme: fake.NewClientBuilder().Build().Scheme(), + VPCService: service, + NetworkProvider: &vpcnetworktesting.FakeVPCNetworkProvider{}, } } diff --git a/pkg/controllers/networkpolicy/networkpolicy_controller.go b/pkg/controllers/networkpolicy/networkpolicy_controller.go index 3e7eda8dc..cb68012d9 100644 --- a/pkg/controllers/networkpolicy/networkpolicy_controller.go +++ b/pkg/controllers/networkpolicy/networkpolicy_controller.go @@ -10,6 +10,7 @@ import ( "os" "sync" + "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" v1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" apimachineryruntime "k8s.io/apimachinery/pkg/runtime" @@ -17,11 +18,13 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" _ "github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter" @@ -41,10 +44,11 @@ var ( // NetworkPolicyReconciler reconciles a NetworkPolicy object type NetworkPolicyReconciler struct { - Client client.Client - Scheme *apimachineryruntime.Scheme - Service *securitypolicy.SecurityPolicyService - Recorder record.EventRecorder + Client client.Client + Scheme *apimachineryruntime.Scheme + Service *securitypolicy.SecurityPolicyService + Recorder record.EventRecorder + NetworkProvider vpcnetwork.VPCNetworkProvider } func updateFail(r *NetworkPolicyReconciler, c *context.Context, o *networkingv1.NetworkPolicy, e *error) { @@ -70,6 +74,13 @@ func deleteSuccess(r *NetworkPolicyReconciler, _ *context.Context, o *networking func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { // Use once.Do to ensure gc is called only once common.GcOnce(r, &once) + if r.NetworkProvider != nil { + return r.NetworkProvider.ReconcileWithVPCFilters("networkpolicy", ctx, req, r.reconcile) + } + return r.reconcile(ctx, req) +} + +func (r *NetworkPolicyReconciler) reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { networkPolicy := &networkingv1.NetworkPolicy{} log.Info("reconciling networkpolicy", "networkpolicy", req.NamespacedName) metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType) @@ -134,6 +145,11 @@ func (r *NetworkPolicyReconciler) setupWithManager(mgr ctrl.Manager) error { controller.Options{ MaxConcurrentReconciles: common.NumReconcile(), }). + Watches( + &v1alpha1.Network{}, + &vpcnetwork.EnqueueRequestForNetwork{Client: r.Client, Lister: r.listNetworkPolicies}, + builder.WithPredicates(vpcnetwork.PredicateFuncsByNetwork), + ). Complete(r) } @@ -180,11 +196,28 @@ func (r *NetworkPolicyReconciler) CollectGarbage(ctx context.Context) { } } -func StartNetworkPolicyController(mgr ctrl.Manager, commonService servicecommon.Service, vpcService servicecommon.VPCServiceProvider) { +func (r *NetworkPolicyReconciler) listNetworkPolicies(ns string) ([]types.NamespacedName, error) { + npList := &networkingv1.NetworkPolicyList{} + err := r.Client.List(context.Background(), npList, client.InNamespace(ns)) + if err != nil { + return nil, err + } + nsNames := make([]types.NamespacedName, 0) + for _, np := range npList.Items { + nsNames = append(nsNames, types.NamespacedName{ + Namespace: np.Namespace, + Name: np.Name, + }) + } + return nsNames, nil +} + +func StartNetworkPolicyController(mgr ctrl.Manager, commonService servicecommon.Service, vpcService servicecommon.VPCServiceProvider, networkProvider vpcnetwork.VPCNetworkProvider) { networkPolicyReconcile := NetworkPolicyReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("networkpolicy-controller"), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("networkpolicy-controller"), + NetworkProvider: networkProvider, } networkPolicyReconcile.Service = securitypolicy.GetSecurityService(commonService, vpcService) if err := networkPolicyReconcile.Start(mgr); err != nil { diff --git a/pkg/controllers/pod/pod_controller.go b/pkg/controllers/pod/pod_controller.go index 0e8b94ba3..460e35e86 100644 --- a/pkg/controllers/pod/pod_controller.go +++ b/pkg/controllers/pod/pod_controller.go @@ -10,6 +10,7 @@ import ( "strings" "sync" + "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v1 "k8s.io/api/core/v1" apimachineryruntime "k8s.io/apimachinery/pkg/runtime" @@ -17,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -24,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" @@ -47,12 +50,20 @@ type PodReconciler struct { VPCService servicecommon.VPCServiceProvider NodeServiceReader servicecommon.NodeServiceReader Recorder record.EventRecorder + + NetworkProvider vpcnetwork.VPCNetworkProvider } func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { // Use once.Do to ensure gc is called only once common.GcOnce(r, &once) + if r.NetworkProvider != nil { + return r.NetworkProvider.ReconcileWithVPCFilters("pod", ctx, req, r.reconcile) + } + return r.reconcile(ctx, req) +} +func (r *PodReconciler) reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { pod := &v1.Pod{} log.Info("reconciling pod", "pod", req.NamespacedName) @@ -170,10 +181,31 @@ func (r *PodReconciler) SetupWithManager(mgr ctrl.Manager) error { controller.Options{ MaxConcurrentReconciles: common.NumReconcile(), }). + Watches( + &v1alpha1.Network{}, + &vpcnetwork.EnqueueRequestForNetwork{Client: r.Client, Lister: r.listPods}, + builder.WithPredicates(vpcnetwork.PredicateFuncsByNetwork), + ). Complete(r) } -func StartPodController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPortService, subnetService servicecommon.SubnetServiceProvider, vpcService servicecommon.VPCServiceProvider, nodeService servicecommon.NodeServiceReader) { +func (r *PodReconciler) listPods(ns string) ([]types.NamespacedName, error) { + podList := &v1.PodList{} + err := r.Client.List(context.Background(), podList, client.InNamespace(ns)) + if err != nil { + return nil, err + } + nsNames := make([]types.NamespacedName, 0) + for _, pod := range podList.Items { + nsNames = append(nsNames, types.NamespacedName{ + Namespace: pod.Namespace, + Name: pod.Name, + }) + } + return nsNames, nil +} + +func StartPodController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPortService, subnetService servicecommon.SubnetServiceProvider, vpcService servicecommon.VPCServiceProvider, nodeService servicecommon.NodeServiceReader, networkProvider vpcnetwork.VPCNetworkProvider) { podPortReconciler := PodReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -182,6 +214,7 @@ func StartPodController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPo VPCService: vpcService, NodeServiceReader: nodeService, Recorder: mgr.GetEventRecorderFor("pod-controller"), + NetworkProvider: networkProvider, } if err := podPortReconciler.Start(mgr); err != nil { log.Error(err, "failed to create controller", "controller", "Pod") diff --git a/pkg/controllers/service/service_lb_controller.go b/pkg/controllers/service/service_lb_controller.go index 48cc9838c..1cddc0567 100644 --- a/pkg/controllers/service/service_lb_controller.go +++ b/pkg/controllers/service/service_lb_controller.go @@ -7,36 +7,43 @@ import ( "context" "os" + "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" v1 "k8s.io/api/core/v1" apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/version" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" _ "github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter" servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + "github.com/vmware-tanzu/nsx-operator/pkg/util" ) var ( - log = &logger.Log - ResultNormal = common.ResultNormal - ResultRequeue = common.ResultRequeue - MetricResType = common.MetricResTypeServiceLb + log = &logger.Log + ResultNormal = common.ResultNormal + ResultRequeue = common.ResultRequeue + MetricResType = common.MetricResTypeServiceLb + LBServiceClassForVPC = "vmware.com/nsx_vpc" ) // ServiceLbReconciler reconciles a Service LoadBalancer object type ServiceLbReconciler struct { - Client client.Client - Scheme *apimachineryruntime.Scheme - Service *servicecommon.Service - Recorder record.EventRecorder + Client client.Client + Scheme *apimachineryruntime.Scheme + Service *servicecommon.Service + Recorder record.EventRecorder + NetworkProvider vpcnetwork.VPCNetworkProvider } func updateSuccess(r *ServiceLbReconciler, c *context.Context, lbService *v1.Service) { @@ -46,6 +53,13 @@ func updateSuccess(r *ServiceLbReconciler, c *context.Context, lbService *v1.Ser } func (r *ServiceLbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + if r.NetworkProvider != nil { + return r.NetworkProvider.ReconcileWithVPCFilters("lb Service", ctx, req, r.reconcile) + } + return r.reconcile(ctx, req) +} + +func (r *ServiceLbReconciler) reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { service := &v1.Service{} if err := r.Client.Get(ctx, req.NamespacedName, service); err != nil { @@ -53,14 +67,28 @@ func (r *ServiceLbReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ResultNormal, client.IgnoreNotFound(err) } - if service.Spec.Type == v1.ServiceTypeLoadBalancer { - log.Info("reconciling lb service", "lbService", req.NamespacedName) - metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType) + if service.Spec.Type != v1.ServiceTypeLoadBalancer { + return ResultNormal, nil + } - if service.ObjectMeta.DeletionTimestamp.IsZero() { - metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateTotal, MetricResType) - updateSuccess(r, &ctx, service) - } + systemNS, err := util.IsVPCSystemNamespace(r.Client, req.Namespace, nil) + if err != nil { + log.Error(err, "unable to check Namespace with system annotation on lb service", "service", req.Namespace) + return ResultNormal, client.IgnoreNotFound(err) + } + + // Ignore the LB Service in system Namespaces if configured with a different LoadBalancerClass from VPC. + if systemNS && !(service.Spec.LoadBalancerClass != nil && *service.Spec.LoadBalancerClass == LBServiceClassForVPC) { + log.Info("LB Service is using a non-vpc class", "req", req, "lbClass", service.Spec.LoadBalancerClass) + return ResultNormal, nil + } + + log.Info("reconciling lb service", "lbService", req.NamespacedName) + metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType) + + if service.ObjectMeta.DeletionTimestamp.IsZero() { + metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateTotal, MetricResType) + updateSuccess(r, &ctx, service) } return ResultNormal, nil @@ -99,6 +127,11 @@ func (r *ServiceLbReconciler) setupWithManager(mgr ctrl.Manager) error { controller.Options{ MaxConcurrentReconciles: common.NumReconcile(), }). + Watches( + &v1alpha1.Network{}, + &vpcnetwork.EnqueueRequestForNetwork{Client: r.Client, Lister: r.listLBServices}, + builder.WithPredicates(vpcnetwork.PredicateFuncsByNetwork), + ). Complete(r) } @@ -112,6 +145,26 @@ func (r *ServiceLbReconciler) Start(mgr ctrl.Manager) error { return nil } +func (r *ServiceLbReconciler) listLBServices(ns string) ([]types.NamespacedName, error) { + serviceList := &v1.ServiceList{} + err := r.Client.List(context.Background(), serviceList, client.InNamespace(ns)) + if err != nil { + return nil, err + } + nsNames := make([]types.NamespacedName, 0) + for _, svc := range serviceList.Items { + if svc.Spec.Type != v1.ServiceTypeLoadBalancer { + continue + } + // Only process LoadBalancer type Service. + nsNames = append(nsNames, types.NamespacedName{ + Namespace: svc.Namespace, + Name: svc.Name, + }) + } + return nsNames, nil +} + func isServiceLbStatusIpModeSupported(c *rest.Config) bool { version129, _ := version.ParseGeneric("v1.29.0") @@ -137,13 +190,14 @@ func isServiceLbStatusIpModeSupported(c *rest.Config) bool { return runningVersion.AtLeast(version129) } -func StartServiceLbController(mgr ctrl.Manager, commonService servicecommon.Service) { +func StartServiceLbController(mgr ctrl.Manager, commonService servicecommon.Service, networkProvider vpcnetwork.VPCNetworkProvider) { if isServiceLbStatusIpModeSupported(mgr.GetConfig()) { serviceLbReconciler := ServiceLbReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("serviceLb-controller"), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("serviceLb-controller"), + NetworkProvider: networkProvider, } serviceLbReconciler.Service = &commonService if err := serviceLbReconciler.Start(mgr); err != nil { diff --git a/pkg/controllers/service/service_lb_controller_test.go b/pkg/controllers/service/service_lb_controller_test.go index 0555aac1f..ecffa3abe 100644 --- a/pkg/controllers/service/service_lb_controller_test.go +++ b/pkg/controllers/service/service_lb_controller_test.go @@ -6,6 +6,7 @@ package service import ( "context" "errors" + "fmt" "testing" "k8s.io/apimachinery/pkg/runtime" @@ -23,6 +24,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/config" + vpcnetworktesting "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork/testing" mock_client "github.com/vmware-tanzu/nsx-operator/pkg/mock/controller-runtime/client" "github.com/vmware-tanzu/nsx-operator/pkg/nsx" _ "github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter" @@ -31,10 +33,11 @@ import ( func NewFakeServiceLbReconciler() *ServiceLbReconciler { return &ServiceLbReconciler{ - Client: fake.NewClientBuilder().Build(), - Scheme: fake.NewClientBuilder().Build().Scheme(), - Service: nil, - Recorder: fakeRecorder{}, + Client: fake.NewClientBuilder().Build(), + Scheme: fake.NewClientBuilder().Build().Scheme(), + Service: nil, + Recorder: fakeRecorder{}, + NetworkProvider: &vpcnetworktesting.FakeVPCNetworkProvider{}, } } @@ -146,30 +149,89 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { } r := &ServiceLbReconciler{ - Client: k8sClient, - Scheme: nil, - Service: service, - Recorder: fakeRecorder{}, + Client: k8sClient, + Scheme: nil, + Service: service, + Recorder: fakeRecorder{}, + NetworkProvider: &vpcnetworktesting.FakeVPCNetworkProvider{}, } ctx := context.Background() req := controllerruntime.Request{NamespacedName: types.NamespacedName{Namespace: "dummy", Name: "dummy"}} // case not found obj errNotFound := errors.New("not found") - k8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(errNotFound) + k8sClient.EXPECT().Get(ctx, gomock.Any(), &v1.Service{}).Return(errNotFound) _, err := r.Reconcile(ctx, req) assert.Equal(t, err, errNotFound) lbService := &v1.Service{} - // case DeletionTimestamp.IsZero = false and service type is LoadBalancer + mockNamespaceGetFunc := func(annotation map[string]string) { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), &v1.Namespace{}).Return(nil).Do( + func(_ context.Context, key client.ObjectKey, obj client.Object, option ...client.GetOption) error { + nsObj := obj.(*v1.Namespace) + nsObj.Namespace = key.Namespace + nsObj.Name = key.Name + nsObj.Annotations = annotation + return nil + }, + ) + } + + // case service type is LoadBalancer, error when checking system Namespace. + k8sClient.EXPECT().Get(ctx, gomock.Any(), lbService).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + v1lbservice := obj.(*v1.Service) + v1lbservice.Spec.Type = v1.ServiceTypeLoadBalancer + return nil + }) + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), &v1.Namespace{}).Return(errors.New("invalid Namespace")) + _, err = r.Reconcile(ctx, req) + assert.EqualError(t, err, "invalid Namespace") + + // case service type is LoadBalancer in system Namespace, loadBalancerClass is not set + k8sClient.EXPECT().Get(ctx, gomock.Any(), lbService).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + v1lbservice := obj.(*v1.Service) + v1lbservice.Spec.Type = v1.ServiceTypeLoadBalancer + return nil + }) + mockNamespaceGetFunc(map[string]string{"nsx.vmware.com/nsx_network_config": "true"}) + _, err = r.Reconcile(ctx, req) + assert.Equal(t, err, nil) + + // case service type is LoadBalancer in system Namespace, loadBalancerClass is not VPC + nonVPCClass := "not-vpc" + k8sClient.EXPECT().Get(ctx, gomock.Any(), lbService).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + v1lbservice := obj.(*v1.Service) + v1lbservice.Spec.Type = v1.ServiceTypeLoadBalancer + v1lbservice.Spec.LoadBalancerClass = &nonVPCClass + return nil + }) + mockNamespaceGetFunc(map[string]string{"nsx.vmware.com/nsx_network_config": "true"}) + _, err = r.Reconcile(ctx, req) + assert.Equal(t, err, nil) + + // case DeletionTimestamp.IsZero = false and service type is LoadBalancer in system Namespace with VPC k8sClient.EXPECT().Get(ctx, gomock.Any(), lbService).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { v1lbservice := obj.(*v1.Service) v1lbservice.Spec.Type = v1.ServiceTypeLoadBalancer + v1lbservice.Spec.LoadBalancerClass = &LBServiceClassForVPC time := metav1.Now() v1lbservice.ObjectMeta.DeletionTimestamp = &time return nil }) + mockNamespaceGetFunc(map[string]string{"nsx.vmware.com/nsx_network_config": "true"}) + _, err = r.Reconcile(ctx, req) + assert.Equal(t, err, nil) + + // case DeletionTimestamp.IsZero = false and service type is LoadBalancer in non-system Namespace. + k8sClient.EXPECT().Get(ctx, gomock.Any(), lbService).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + v1lbservice := obj.(*v1.Service) + v1lbservice.Spec.Type = v1.ServiceTypeLoadBalancer + time := metav1.Now() + v1lbservice.ObjectMeta.DeletionTimestamp = &time + return nil + }) + mockNamespaceGetFunc(nil) _, err = r.Reconcile(ctx, req) assert.Equal(t, err, nil) @@ -191,18 +253,56 @@ func TestSecurityPolicyReconciler_Start(t *testing.T) { service := &servicecommon.Service{} var mgr controllerruntime.Manager r := &ServiceLbReconciler{ - Client: k8sClient, - Scheme: nil, - Service: service, - Recorder: fakeRecorder{}, + Client: k8sClient, + Scheme: nil, + Service: service, + Recorder: fakeRecorder{}, + NetworkProvider: &vpcnetworktesting.FakeVPCNetworkProvider{}, } - // Case Manager is not initialized - err := r.Start(mgr) - assert.NotEqual(t, nil, err) - // Case Manager is initialized mgr, _ = controllerruntime.NewManager(&rest.Config{}, manager.Options{}) - err = r.Start(mgr) + err := r.Start(mgr) assert.Equal(t, nil, err) } + +func TestListLBServices(t *testing.T) { + mockCtl := gomock.NewController(t) + k8sClient := mock_client.NewMockClient(mockCtl) + r := &ServiceLbReconciler{ + Client: k8sClient, + Scheme: nil, + } + testNS := "ns1" + // Validate error case + k8sClient.EXPECT().List(gomock.Any(), &v1.ServiceList{}, gomock.Any()).Return(fmt.Errorf("unable to list Services")) + _, err := r.listLBServices(testNS) + assert.EqualError(t, err, "unable to list Services") + // Validate only LoadBalancer type Services are returned + k8sClient.EXPECT().List(gomock.Any(), &v1.ServiceList{}, gomock.Any()).Return(nil).Do( + func(_ context.Context, obj client.ObjectList, opts ...client.ListOption) error { + netList := obj.(*v1.ServiceList) + netList.Items = []v1.Service{ + { + ObjectMeta: metav1.ObjectMeta{Name: "clusterIP-svc", Namespace: testNS}, + Spec: v1.ServiceSpec{Type: v1.ServiceTypeClusterIP}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "nodePort-svc", Namespace: testNS}, + Spec: v1.ServiceSpec{Type: v1.ServiceTypeNodePort}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "lb-svc", Namespace: testNS}, + Spec: v1.ServiceSpec{Type: v1.ServiceTypeLoadBalancer}, + }, + } + return nil + }) + + objs, err := r.listLBServices(testNS) + assert.NoError(t, err) + assert.Equal(t, 1, len(objs)) + svc := objs[0] + assert.Equal(t, "lb-svc", svc.Name) + assert.Equal(t, testNS, svc.Namespace) +} diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index b50fa42b6..fb9c00bbe 100644 --- a/pkg/controllers/subnetset/subnetset_controller.go +++ b/pkg/controllers/subnetset/subnetset_controller.go @@ -22,7 +22,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha1" - "github.com/vmware-tanzu/nsx-operator/pkg/config" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" "github.com/vmware-tanzu/nsx-operator/pkg/logger" "github.com/vmware-tanzu/nsx-operator/pkg/metrics" @@ -329,13 +328,7 @@ func (r *SubnetSetReconciler) Start(mgr ctrl.Manager, enableWebhook bool) error return err } if enableWebhook { - hookServer := webhook.NewServer(webhook.Options{ - Port: config.WebhookServerPort, - CertDir: config.WebhookCertDir, - }) - if err := mgr.Add(hookServer); err != nil { - return err - } + hookServer := mgr.GetWebhookServer() hookServer.Register("/validate-nsx-vmware-com-v1alpha1-subnetset", &webhook.Admission{ Handler: &SubnetSetValidator{ diff --git a/pkg/controllers/vpcnetwork/interfaces.go b/pkg/controllers/vpcnetwork/interfaces.go new file mode 100644 index 000000000..3981b7d14 --- /dev/null +++ b/pkg/controllers/vpcnetwork/interfaces.go @@ -0,0 +1,17 @@ +/* Copyright © 2024 VMware, Inc. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 */ + +package vpcnetwork + +import ( + "context" + + ctrl "sigs.k8s.io/controller-runtime" +) + +type ReconcileFunc func(ctx context.Context, req ctrl.Request) (ctrl.Result, error) + +type VPCNetworkProvider interface { + IsVPCEnabledOnNamespace(ns string) (bool, error) + ReconcileWithVPCFilters(resource string, ctx context.Context, req ctrl.Request, innerFunc ReconcileFunc) (ctrl.Result, error) +} diff --git a/pkg/controllers/vpcnetwork/network_controller.go b/pkg/controllers/vpcnetwork/network_controller.go new file mode 100644 index 000000000..9ee062e63 --- /dev/null +++ b/pkg/controllers/vpcnetwork/network_controller.go @@ -0,0 +1,186 @@ +/* Copyright © 2024 VMware, Inc. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 */ + +package vpcnetwork + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/go-logr/logr" + "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" + v1 "k8s.io/api/core/v1" + apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/webhook" + + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/logger" + "github.com/vmware-tanzu/nsx-operator/pkg/util" +) + +var ( + log = logger.Log + resultNormal = common.ResultNormal + resultRequeue = common.ResultRequeue + + systemNetworkNamespaces sets.Set[string] +) + +const ( + defaultNetworkLabelKey = "is-default-network" + defaultNetworkLabelValue = "true" + vpcNetworkValidationPath = "/validate-vpc-enablement" +) + +// NetworkReconciler reconciles a WCP Network object +type NetworkReconciler struct { + Client client.Client + Scheme *apimachineryruntime.Scheme +} + +func (r *NetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + // Log the Network create/update event. + log.Info("reconciling network CR", "network", req.NamespacedName) + + ns := req.Namespace + if systemNetworkNamespaces.Has(ns) { + log.Info("Default Network CR in system Namespaces is updated to VPC type", "req", req.NamespacedName) + return resultNormal, nil + } + + // Find the system Network Namespace when creating Network CRs + nsObj := &v1.Namespace{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: ns, Name: ns}, nsObj) + // This shall not happen, adding the below logic for security purpose. + if err != nil { + log.Error(err, "failed to check Namespace CR", "namespace", ns) + return resultRequeue, client.IgnoreNotFound(err) + } + isSystem, _ := util.IsVPCSystemNamespace(r.Client, ns, nsObj) + if isSystem { + systemNetworkNamespaces.Insert(ns) + if systemNetworkNamespaces.Len() == 1 { + log.Info("Default Network in system Namespaces is created", "req", req.NamespacedName) + } else { + // In theory, this shall not happen, adding log here for security. + log.Error(nil, "Multiple default Networks are found in system NSs", "namespaces", systemNetworkNamespaces.Len()) + } + } + return resultNormal, nil +} + +func (r *NetworkReconciler) IsVPCEnabledOnNamespace(ns string) (bool, error) { + nsObj := &v1.Namespace{} + err := r.Client.Get(context.Background(), types.NamespacedName{Namespace: ns, Name: ns}, nsObj) + if err != nil { + log.Error(err, "failed to check Namespace CR", "namespace", ns) + return false, err + } + isSystem, _ := util.IsVPCSystemNamespace(r.Client, ns, nsObj) + if isSystem { + if systemNetworkNamespaces.Len() == 0 { + return false, fmt.Errorf("no shared VPC namespace found with system Namespace %s", ns) + } + ns = systemNetworkNamespaces.UnsortedList()[0] + } + netList := &v1alpha1.NetworkList{} + matchingLabels := client.MatchingLabels{defaultNetworkLabelKey: defaultNetworkLabelValue} + err = r.Client.List(context.Background(), netList, client.InNamespace(ns), matchingLabels) + if err != nil { + log.Error(err, "failed to list default Network in Namespace", "namespace", ns) + return false, err + } + if len(netList.Items) == 0 { + return false, fmt.Errorf("no default network found in Namespace %s", ns) + } + network := netList.Items[0] + return network.Spec.Type == v1alpha1.NetworkTypeNSXTVPC, nil +} + +func (r *NetworkReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Register validation webhook to check VPC is enabled or not when creating CRs in a Namespace. + webhookServer := mgr.GetWebhookServer() + webhookServer.Register(vpcNetworkValidationPath, + &webhook.Admission{ + Handler: r, + }) + return ctrl.NewControllerManagedBy(mgr). + For(&v1alpha1.Network{}). + WithEventFilter(predicate.Funcs{ + CreateFunc: func(createEvent event.CreateEvent) bool { + // Filter the resources only labeled with "is-default-network = true" + return isDefaultNetwork(createEvent.Object) + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + // Ignore the deletion event. + return false + }, + }). + WithOptions( + controller.Options{ + MaxConcurrentReconciles: common.NumReconcile(), + }). + Complete(r) +} + +func newNetworkController(mgr ctrl.Manager) *NetworkReconciler { + systemNetworkNamespaces = sets.New[string]() + return &NetworkReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + } +} + +func StartNetworkController(mgr ctrl.Manager) VPCNetworkProvider { + networkReconcile := newNetworkController(mgr) + err := networkReconcile.SetupWithManager(mgr) + if err != nil { + log.Error(err, "failed to create controller", "controller", "Network") + os.Exit(1) + } + return networkReconcile +} + +// isDefaultNetwork returns true only if the provided network is labeled with "is-default-network=true". +func isDefaultNetwork(network client.Object) bool { + labels := network.GetLabels() + networkLabelValue, found := labels[defaultNetworkLabelKey] + if !found { + return false + } + return strings.EqualFold(networkLabelValue, defaultNetworkLabelValue) +} + +func predictNetworkUpdateEvent(e event.UpdateEvent, log logr.Logger) bool { + oldObj := e.ObjectOld.(*v1alpha1.Network) + newObj := e.ObjectNew.(*v1alpha1.Network) + if !isDefaultNetwork(newObj) { + return false + } + if oldObj.Spec.Type == newObj.Spec.Type { + return false + } + if newObj.Spec.Type != v1alpha1.NetworkTypeNSXTVPC { + log.Info("DefaultNetwork has updated its type to non-VPC, ignore", "namespace", + newObj.Namespace, "name", newObj.Name, "oldType", oldObj.Spec.Type, "newType", newObj.Spec.Type) + return false + } + if !systemNetworkNamespaces.Has(newObj.Namespace) { + log.Info("Ignore the event to update network type as VPC in a non-system Namespaces", "namespace", newObj.Namespace) + return false + } + log.Info("receive network update to VPC event", "name", oldObj.Name, "namespace", oldObj.Name) + return true +} diff --git a/pkg/controllers/vpcnetwork/network_handler.go b/pkg/controllers/vpcnetwork/network_handler.go new file mode 100644 index 000000000..adcfdadee --- /dev/null +++ b/pkg/controllers/vpcnetwork/network_handler.go @@ -0,0 +1,106 @@ +/* Copyright © 2024 VMware, Inc. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 */ + +package vpcnetwork + +import ( + "context" + "fmt" + + "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/util" +) + +type EnqueueRequestForNetwork struct { + Client client.Client + Lister func(namespace string) ([]types.NamespacedName, error) +} + +func (e *EnqueueRequestForNetwork) Create(_ context.Context, _ event.CreateEvent, _ workqueue.RateLimitingInterface) { + log.V(1).Info("namespace create event, do nothing") +} + +func (e *EnqueueRequestForNetwork) Delete(_ context.Context, _ event.DeleteEvent, _ workqueue.RateLimitingInterface) { + log.V(1).Info("namespace delete event, do nothing") +} + +func (e *EnqueueRequestForNetwork) Generic(_ context.Context, _ event.GenericEvent, _ workqueue.RateLimitingInterface) { + log.V(1).Info("namespace generic event, do nothing") +} + +func (e *EnqueueRequestForNetwork) Update(ctx context.Context, updateEvent event.UpdateEvent, q workqueue.RateLimitingInterface) { + network := updateEvent.ObjectNew.(*v1alpha1.Network) + netNS := network.Namespace + objs := make([]types.NamespacedName, 0) + listCRsInNS := func(ns string) error { + crs, err := e.Lister(ns) + if err != nil { + log.Error(err, "Failed to list CRs in the Namespace", "namespace", network.Namespace) + return err + } + objs = append(objs, crs...) + return nil + } + + // List existing CRs in a non-system and add into the queue + if !systemNetworkNamespaces.Has(netNS) { + if err := listCRsInNS(network.Namespace); err != nil { + return + } + } else { + // List CRs in all system Namespaces and all into the queue. + namespaceList := &v1.NamespaceList{} + err := e.Client.List(ctx, namespaceList) + if err != nil { + log.Error(err, "failed to list Namespaces after system network is updated to VPC", "namespace", network.Namespace) + return + } + for i := range namespaceList.Items { + ns := namespaceList.Items[i] + if isSystem, _ := util.IsVPCSystemNamespace(e.Client, ns.Name, &ns); isSystem { + if err = listCRsInNS(ns.Name); err != nil { + return + } + } + } + } + + for _, namespacedname := range objs { + q.Add(reconcile.Request{NamespacedName: namespacedname}) + } +} + +var PredicateFuncsByNetwork = predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return predictNetworkUpdateEvent(e, log.V(1)) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, +} + +func (r *NetworkReconciler) ReconcileWithVPCFilters(resource string, ctx context.Context, req ctrl.Request, innerFunc ReconcileFunc) (ctrl.Result, error) { + vpcEnabled, err := r.IsVPCEnabledOnNamespace(req.Namespace) + if err != nil { + log.Error(err, fmt.Sprintf("failed to check VPC enablement when processing %s", resource), "namespace", req.Namespace, "name", req.Name) + return common.ResultRequeue, err + } + if !vpcEnabled { + log.V(2).Info(fmt.Sprintf("VPC is not enabled, ignore %s in the Namespace", resource), "namespace", req.Namespace) + return common.ResultNormal, nil + } + return innerFunc(ctx, req) +} diff --git a/pkg/controllers/vpcnetwork/network_test.go b/pkg/controllers/vpcnetwork/network_test.go new file mode 100644 index 000000000..00b9d981c --- /dev/null +++ b/pkg/controllers/vpcnetwork/network_test.go @@ -0,0 +1,712 @@ +/* Copyright © 2024 VMware, Inc. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 */ + +package vpcnetwork + +import ( + "context" + "fmt" + "net/http" + "sync" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/vmware-tanzu/net-operator-api/api/v1alpha1" + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/util/workqueue" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + mock_client "github.com/vmware-tanzu/nsx-operator/pkg/mock/controller-runtime/client" + svccommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + systemNS = "kube-system" +) + +func TestPredictNetworkUpdateEvent(t *testing.T) { + oldDNS := []string{"1.1.1.1"} + newDNS := []string{"1.1.1.1", "1.1.1.2"} + nonSystemNS := "ns1" + systemNetworkNamespaces = sets.New[string](systemNS) + + for _, tc := range []struct { + name string + oldNet *v1alpha1.Network + newNet *v1alpha1.Network + expResult bool + }{ + { + name: "Update event on non-default Network", + oldNet: createNetwork("non-default", systemNS, v1alpha1.NetworkTypeNSXT, nil, oldDNS), + newNet: createNetwork("non-default", systemNS, v1alpha1.NetworkTypeNSXTVPC, nil, newDNS), + expResult: false, + }, + { + name: "Update default VDS Network with no change on network type", + oldNet: createVDSDefaultNetwork("default", systemNS, oldDNS), + newNet: createVDSDefaultNetwork("default", systemNS, newDNS), + expResult: false, + }, + { + name: "Update default VPC Network with no change on network type", + oldNet: createVPCDefaultNetwork("default", nonSystemNS, oldDNS), + newNet: createVPCDefaultNetwork("default", nonSystemNS, newDNS), + expResult: false, + }, + { + name: "Update default Network to non-VPC type", + oldNet: createVDSDefaultNetwork("default", nonSystemNS, oldDNS), + newNet: createDefaultNetwork("default", nonSystemNS, v1alpha1.NetworkTypeNSXT, newDNS), + expResult: false, + }, + { + name: "Update default Network in a non-system Namespace", + oldNet: createVDSDefaultNetwork("default", nonSystemNS, oldDNS), + newNet: createVPCDefaultNetwork("default", nonSystemNS, oldDNS), + expResult: false, + }, + { + name: "Update default Network to VPC in system Namespaces", + oldNet: createVDSDefaultNetwork("default", systemNS, oldDNS), + newNet: createVPCDefaultNetwork("default", systemNS, oldDNS), + expResult: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Network create event is ignored. + createResult := PredicateFuncsByNetwork.Create(event.CreateEvent{ + Object: tc.oldNet, + }) + assert.False(t, createResult) + // Network update event is processed. + updateEvent := event.UpdateEvent{ + ObjectOld: tc.oldNet, + ObjectNew: tc.newNet, + } + updateResult := PredicateFuncsByNetwork.Update(updateEvent) + assert.Equal(t, tc.expResult, updateResult) + // Network delete event is ignored. + deleteResult := PredicateFuncsByNetwork.Delete(event.DeleteEvent{ + Object: tc.newNet, + }) + assert.False(t, deleteResult) + }) + } +} + +func TestIsDefaultNetwork(t *testing.T) { + //systemNetworkNamespaces = sets.New[string]() + for _, tc := range []struct { + name string + network *v1alpha1.Network + expResult bool + }{ + { + name: "Network without any labels", + network: createNetwork("net1", "ns1", v1alpha1.NetworkTypeNSXTVPC, nil, nil), + expResult: false, + }, + { + name: "Network without default label key", + network: createNetwork("net2", "ns1", v1alpha1.NetworkTypeNSXTVPC, map[string]string{"invalid-key": "true"}, nil), + expResult: false, + }, + { + name: "Network with default label key and value false", + network: createNetwork("net3", "ns1", v1alpha1.NetworkTypeNSXTVPC, map[string]string{defaultNetworkLabelKey: "false"}, nil), + expResult: false, + }, + { + name: "Network with default label key and value true", + network: createNetwork("net4", "ns1", v1alpha1.NetworkTypeNSXTVPC, map[string]string{defaultNetworkLabelKey: defaultNetworkLabelValue}, nil), + expResult: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + actResult := isDefaultNetwork(tc.network) + assert.Equal(t, tc.expResult, actResult) + }) + } +} + +func TestReconcile(t *testing.T) { + mockCtl := gomock.NewController(t) + k8sClient := mock_client.NewMockClient(mockCtl) + reconciler := &NetworkReconciler{Client: k8sClient} + systemNetworkNamespaces = sets.New[string]() + + ctx := context.Background() + systemReq := types.NamespacedName{ + Namespace: systemNS, + Name: "default-net1", + } + nonSystemReq1 := types.NamespacedName{ + Namespace: "ns1", + Name: "default", + } + nonSystemReq2 := types.NamespacedName{ + Namespace: "ns2", + Name: "default", + } + + for _, tc := range []struct { + name string + existingNSs map[string]bool + request ctrl.Request + mockFunc func() + expErr string + expResult ctrl.Result + addToSystemNS bool + }{ + { + name: "Error when fetching Network's Namespace ", + request: ctrl.Request{NamespacedName: systemReq}, + mockFunc: func() { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("failure to get Namespace")) + }, + expErr: "failure to get Namespace", + expResult: resultRequeue, + }, + { + name: "Network's Namespace is annotated with VPC system false", + request: ctrl.Request{NamespacedName: nonSystemReq1}, + mockFunc: func() { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Do( + func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + copyNamespace(obj, createWorkloadNamespace(nonSystemReq1.Namespace)) + return nil + }, + ) + }, + expResult: resultNormal, + addToSystemNS: false, + }, + { + name: "Network's Namespace has no system annotation key", + request: ctrl.Request{NamespacedName: nonSystemReq2}, + mockFunc: func() { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Do( + func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + copyNamespace(obj, createWorkloadNamespace(nonSystemReq2.Namespace)) + return nil + }, + ) + }, + expResult: resultNormal, + addToSystemNS: false, + }, + { + name: "Network is in system Namespace", + request: ctrl.Request{NamespacedName: systemReq}, + mockFunc: func() { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Do( + func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + copyNamespace(obj, createSystemNamespace(systemReq.Namespace)) + return nil + }, + ) + }, + expResult: resultNormal, + addToSystemNS: true, + }, + { + name: "Update Network type in system Namespace without read Namespace", + request: ctrl.Request{NamespacedName: systemReq}, + expResult: resultNormal, + addToSystemNS: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.mockFunc != nil { + tc.mockFunc() + } + result, err := reconciler.Reconcile(ctx, tc.request) + if tc.expErr != "" { + assert.EqualError(t, err, tc.expErr) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expResult, result) + if tc.addToSystemNS { + assert.Contains(t, systemNetworkNamespaces, tc.request.Namespace) + } else { + assert.NotContains(t, systemNetworkNamespaces, tc.request.Namespace) + } + }) + } +} + +func TestIsVPCEnabledOnNamespace(t *testing.T) { + defer func() { + systemNetworkNamespaces = nil + }() + + mockCtl := gomock.NewController(t) + k8sClient := mock_client.NewMockClient(mockCtl) + reconciler := &NetworkReconciler{Client: k8sClient} + netNSForSystem := "kube-system" + defaultNetworkInSystem := createVPCDefaultNetwork("default", netNSForSystem, nil) + systemNetworkNamespaces = sets.New[string](netNSForSystem) + + for _, tc := range []struct { + name string + ns string + listNs string + netListErr error + isSystemNS bool + nsGetErr error + listNets []v1alpha1.Network + expErr string + expResult bool + }{ + { + name: "Error when checking system Namespaces", + ns: "ns0", + nsGetErr: fmt.Errorf("failed to list Namespaces"), + expErr: "failed to list Namespaces", + expResult: false, + }, + { + name: "Error when listing Network CRs", + ns: "ns1", + listNs: "ns1", + netListErr: fmt.Errorf("failed to list Networks in Namespace"), + expErr: "failed to list Networks in Namespace", + expResult: false, + }, + { + name: "No default network exists in Namespace", + ns: "ns2", + listNs: "ns2", + listNets: []v1alpha1.Network{}, + expErr: "no default network found in Namespace ns2", + expResult: false, + }, + { + name: "Default network type is VDS in Namespace ns3", + ns: "ns3", + listNs: "ns3", + listNets: []v1alpha1.Network{ + *createVDSDefaultNetwork("default-ns3", "ns3", nil), + }, + expResult: false, + }, + { + name: "Default network type is VPC in Namespace ns4", + ns: "ns4", + listNs: "ns4", + listNets: []v1alpha1.Network{ + *createVPCDefaultNetwork("default-ns4", "ns4", nil), + }, + expResult: true, + }, + { + name: "Default system network type is VPC", + ns: netNSForSystem, + isSystemNS: true, + listNs: netNSForSystem, + listNets: []v1alpha1.Network{*defaultNetworkInSystem}, + expResult: true, + }, + { + name: "Check VPC from a different system Namespace", + ns: "system", + isSystemNS: true, + listNs: netNSForSystem, + listNets: []v1alpha1.Network{*defaultNetworkInSystem}, + expResult: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.nsGetErr != nil { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.nsGetErr) + } else { + var namespace *corev1.Namespace + if tc.isSystemNS { + namespace = createSystemNamespace(tc.ns) + } else { + namespace = createWorkloadNamespace(tc.ns) + } + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Namespace: tc.ns, Name: tc.ns}, &corev1.Namespace{}).Return(nil).Do( + func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + copyNamespace(obj, namespace) + return nil + }, + ) + matchingLabels := client.MatchingLabels{defaultNetworkLabelKey: defaultNetworkLabelValue} + listOptions := []client.ListOption{ + client.InNamespace(tc.listNs), matchingLabels, + } + k8sClient.EXPECT().List(gomock.Any(), &v1alpha1.NetworkList{}, listOptions).Return(tc.netListErr).Do( + func(_ context.Context, obj client.ObjectList, opts ...client.ListOption) error { + netList := obj.(*v1alpha1.NetworkList) + netList.Items = tc.listNets + return tc.netListErr + }, + ) + } + + result, err := reconciler.IsVPCEnabledOnNamespace(tc.ns) + if tc.expErr != "" { + assert.EqualError(t, err, tc.expErr) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expResult, result) + }) + } +} + +func TestReconcileWithVPCFilters(t *testing.T) { + mockCtl := gomock.NewController(t) + k8sClient := mock_client.NewMockClient(mockCtl) + reconciler := &NetworkReconciler{Client: k8sClient} + ctx := context.Background() + var innerHandledRequest types.NamespacedName + innerHandled := false + + innerfunc := func(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + innerHandled = true + innerHandledRequest = req.NamespacedName + return resultNormal, nil + } + for _, tc := range []struct { + name string + request ctrl.Request + listErr error + listNets []v1alpha1.Network + expErr string + expResult ctrl.Result + expInnerHandled bool + }{ + { + name: "Error when listing Network CRs", + listErr: fmt.Errorf("failed to list Networks in Namespace"), + expErr: "failed to list Networks in Namespace", + expResult: common.ResultRequeue, + }, + { + name: "Default network type is VDS", + request: ctrl.Request{NamespacedName: types.NamespacedName{Namespace: "ns1", Name: "pod1"}}, + listNets: []v1alpha1.Network{ + {ObjectMeta: v1.ObjectMeta{ + Namespace: "ns1", + Name: "default", + Labels: map[string]string{defaultNetworkLabelKey: defaultNetworkLabelValue}}, + Spec: v1alpha1.NetworkSpec{ + Type: v1alpha1.NetworkTypeVDS, + }}, + }, + expResult: common.ResultNormal, + expInnerHandled: false, + }, + { + name: "Default network type is VPC", + request: ctrl.Request{NamespacedName: types.NamespacedName{Namespace: "ns2", Name: "pod2"}}, + listNets: []v1alpha1.Network{ + {ObjectMeta: v1.ObjectMeta{ + Namespace: "ns2", + Name: "default", + Labels: map[string]string{defaultNetworkLabelKey: defaultNetworkLabelValue}}, + Spec: v1alpha1.NetworkSpec{ + Type: v1alpha1.NetworkTypeNSXTVPC, + }}, + }, + expResult: common.ResultNormal, + expInnerHandled: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + innerHandled = false + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Do( + func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + copyNamespace(obj, createWorkloadNamespace(tc.request.Namespace)) + return nil + }, + ) + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.listErr).Do( + func(_ context.Context, obj client.ObjectList, options ...client.ListOption) error { + netList := obj.(*v1alpha1.NetworkList) + netList.Items = tc.listNets + return tc.listErr + }, + ) + + result, err := reconciler.ReconcileWithVPCFilters("test", ctx, tc.request, innerfunc) + if tc.expErr != "" { + assert.EqualError(t, err, tc.expErr) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expResult, result) + assert.Equal(t, tc.expInnerHandled, innerHandled) + if tc.expInnerHandled { + assert.Equal(t, tc.request.NamespacedName, innerHandledRequest) + } + }) + } +} + +func TestEnqueueRequestForNetwork(t *testing.T) { + mockCtl := gomock.NewController(t) + k8sClient := mock_client.NewMockClient(mockCtl) + nsAdd, nsUpdate, nsDelete, nsGeneric := "add", "update", "delete", "unknown" + systemNS1 := "system" + systemNetworkNamespaces = sets.New[string](systemNS) + + queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) + enquedItems := make([]types.NamespacedName, 0) + wg := sync.WaitGroup{} + wg.Add(1) + + // Start a goroutine to get items from the queue. + go func() { + defer wg.Done() + for { + obj, _ := queue.Get() + item := obj.(reconcile.Request) + if item.Namespace == "stop" { + return + } + enquedItems = append(enquedItems, item.NamespacedName) + queue.Forget(obj) + queue.Done(obj) + } + }() + + ctx := context.Background() + lister := fakeLister{items: []types.NamespacedName{ + {Namespace: nsAdd, Name: "obj11"}, + {Namespace: nsUpdate, Name: "obj21"}, + {Namespace: nsUpdate, Name: "obj22"}, + {Namespace: nsDelete, Name: "obj31"}, + {Namespace: nsGeneric, Name: "obj41"}, + {Namespace: systemNS, Name: "obj-system"}, + {Namespace: systemNS1, Name: "obj-system1"}, + }} + + enqueueRequest := EnqueueRequestForNetwork{Client: k8sClient, Lister: lister.list} + // Call create event + netInAdd := createVPCDefaultNetwork("net1", nsAdd, nil) + enqueueRequest.Create(ctx, event.CreateEvent{Object: netInAdd}, queue) + // Call update event + netInUpdateOld := createVDSDefaultNetwork("net1", nsUpdate, nil) + netInUpdateNew := createVPCDefaultNetwork("net1", nsUpdate, nil) + enqueueRequest.Update(ctx, event.UpdateEvent{ObjectOld: netInUpdateOld, ObjectNew: netInUpdateNew}, queue) + // Call delete event + netInDelete := createVPCDefaultNetwork("net1", nsDelete, nil) + enqueueRequest.Delete(ctx, event.DeleteEvent{Object: netInDelete}, queue) + // Call generic event + netInGeneric := createVPCDefaultNetwork("net1", nsGeneric, nil) + enqueueRequest.Generic(ctx, event.GenericEvent{Object: netInGeneric}, queue) + + // Call network update event in system NS + systemNetUpdateOld := createVDSDefaultNetwork("net1", systemNS, nil) + systemNetUpdateNew := createVPCDefaultNetwork("net1", systemNS, nil) + k8sClient.EXPECT().List(gomock.Any(), &corev1.NamespaceList{}).Return(nil).Do( + func(_ context.Context, obj client.ObjectList, options ...client.ListOption) error { + nsList := obj.(*corev1.NamespaceList) + nsList.Items = []corev1.Namespace{ + *createSystemNamespace(systemNS), + *createSystemNamespace(systemNS1), + *createWorkloadNamespace(nsAdd), + *createWorkloadNamespace(nsUpdate), + *createWorkloadNamespace(nsDelete), + *createWorkloadNamespace(nsGeneric), + } + return nil + }, + ) + enqueueRequest.Update(ctx, event.UpdateEvent{ObjectOld: systemNetUpdateOld, ObjectNew: systemNetUpdateNew}, queue) + // Send stop event + queue.Add(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "stop"}}) + + wg.Wait() + // Validate only update events are enqueue. + assert.Equal(t, 4, len(enquedItems)) + assert.Contains(t, enquedItems, types.NamespacedName{Namespace: nsUpdate, Name: "obj21"}) + assert.Contains(t, enquedItems, types.NamespacedName{Namespace: nsUpdate, Name: "obj22"}) + assert.Contains(t, enquedItems, types.NamespacedName{Namespace: systemNS, Name: "obj-system"}) + assert.Contains(t, enquedItems, types.NamespacedName{Namespace: systemNS1, Name: "obj-system1"}) + queue.ShutDown() +} + +func TestWebhookHandle(t *testing.T) { + mockCtl := gomock.NewController(t) + k8sClient := mock_client.NewMockClient(mockCtl) + reconciler := &NetworkReconciler{Client: k8sClient} + + vpcWlNS := "ns1" + vdsWlNS := "ns2" + vpcNetwork := createVPCDefaultNetwork("net1", vpcWlNS, nil) + vdsNetwork := createVDSDefaultNetwork("net2", vdsWlNS, nil) + networks := map[string][]v1alpha1.Network{ + vpcWlNS: {*vpcNetwork}, + vdsWlNS: {*vdsNetwork}, + } + allowedResp := admission.Allowed("") + disallowedResp := admission.Errored(http.StatusBadRequest, fmt.Errorf("VPC is not enabled in Namespace %s", vdsWlNS)) + systemNetworkNamespaces = sets.New[string]() + + ctx := context.Background() + for _, tc := range []struct { + name string + validatedResource bool + requestKinds []string + requestNS string + isSystemNS bool + listNetInNS bool + req admission.Request + expResponse admission.Response + }{ + { + name: "Allow creating Pod in VPC NSs", + validatedResource: false, + requestKinds: []string{"Pod"}, + requestNS: vpcWlNS, + listNetInNS: true, + expResponse: allowedResp, + }, + { + name: "Allow creating Pod in non-VPC NSs", + validatedResource: false, + requestKinds: []string{"Pod"}, + requestNS: vdsWlNS, + listNetInNS: true, + expResponse: allowedResp, + }, + { + name: "Allow creating resources in VPC NSs", + validatedResource: true, + requestKinds: []string{"IPPool", "NetworkInfo", "NSXServiceAccount", "SecurityPolicy", "StaticRoute", "SubnetPort", "Subnet", "SubnetSet"}, + requestNS: vpcWlNS, + listNetInNS: true, + expResponse: allowedResp, + }, + { + name: "Disallow creating resources in non-VPC NSs", + validatedResource: true, + requestKinds: []string{"IPPool", "NetworkInfo", "NSXServiceAccount", "SecurityPolicy", "StaticRoute", "SubnetPort", "Subnet", "SubnetSet"}, + requestNS: vdsWlNS, + listNetInNS: true, + expResponse: disallowedResp, + }, { + name: "Error when listing resources in a system NS with no Networks", + validatedResource: true, + isSystemNS: true, + requestKinds: []string{"IPPool"}, + requestNS: "kube-system", + listNetInNS: false, + expResponse: admission.Errored(http.StatusBadRequest, + fmt.Errorf("unable to check the default network type in Namespace kube-system: no shared VPC namespace found with system Namespace kube-system")), + }, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.validatedResource { + k8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Do( + func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + if tc.isSystemNS { + copyNamespace(obj, createSystemNamespace(tc.requestNS)) + } else { + copyNamespace(obj, createWorkloadNamespace(tc.requestNS)) + } + return nil + }, + ).Times(len(tc.requestKinds)) + if tc.listNetInNS { + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Do( + func(_ context.Context, obj client.ObjectList, options ...client.ListOption) error { + netList := obj.(*v1alpha1.NetworkList) + netList.Items = networks[tc.requestNS] + return nil + }, + ).Times(len(tc.requestKinds)) + } + } + for _, kind := range tc.requestKinds { + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{Kind: v1.GroupVersionKind{Kind: kind}, Namespace: tc.requestNS}, + } + response := reconciler.Handle(ctx, req) + assert.Equal(t, tc.expResponse, response) + } + }) + } +} + +type fakeLister struct { + items []types.NamespacedName + err error +} + +func (l *fakeLister) list(ns string) ([]types.NamespacedName, error) { + results := make([]types.NamespacedName, 0) + for _, item := range l.items { + if item.Namespace == ns { + results = append(results, item) + } + } + return results, l.err +} + +func createVPCDefaultNetwork(name, ns string, dns []string) *v1alpha1.Network { + return createDefaultNetwork(name, ns, v1alpha1.NetworkTypeNSXTVPC, dns) +} + +func createVDSDefaultNetwork(name, ns string, dns []string) *v1alpha1.Network { + return createDefaultNetwork(name, ns, v1alpha1.NetworkTypeVDS, dns) +} + +func createDefaultNetwork(name, ns string, networkType v1alpha1.NetworkType, dns []string) *v1alpha1.Network { + return createNetwork(name, ns, networkType, map[string]string{defaultNetworkLabelKey: defaultNetworkLabelValue}, dns) +} + +func createNetwork(name, ns string, networkType v1alpha1.NetworkType, labels map[string]string, dns []string) *v1alpha1.Network { + return &v1alpha1.Network{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ns, + Name: name, + Labels: labels, + }, + Spec: v1alpha1.NetworkSpec{ + Type: networkType, + DNS: dns, + }, + } +} + +func copyNamespace(obj client.Object, namespace *corev1.Namespace) { + nsObj := obj.(*corev1.Namespace) + nsObj.Namespace = namespace.Namespace + nsObj.Name = namespace.Name + nsObj.Annotations = namespace.Annotations + nsObj.Labels = namespace.Labels +} + +func createSystemNamespace(name string) *corev1.Namespace { + return createNamespace(name, map[string]string{}) +} + +func createWorkloadNamespace(name string) *corev1.Namespace { + return createNamespace(name, map[string]string{svccommon.LabelWorkloadNamespace: "true"}) +} + +func createNamespace(name string, labels map[string]string) *corev1.Namespace { + return &corev1.Namespace{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + Labels: labels, + }, + } +} diff --git a/pkg/controllers/vpcnetwork/network_webhook.go b/pkg/controllers/vpcnetwork/network_webhook.go new file mode 100644 index 000000000..fe5ea5450 --- /dev/null +++ b/pkg/controllers/vpcnetwork/network_webhook.go @@ -0,0 +1,48 @@ +/* Copyright © 2024 VMware, Inc. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 */ + +package vpcnetwork + +import ( + "context" + "fmt" + "net/http" + + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +func (r *NetworkReconciler) Handle(ctx context.Context, req admission.Request) admission.Response { + objKind := req.Kind.Kind + switch objKind { + case "IPPool": + fallthrough + case "NetworkInfo": + fallthrough + case "NSXServiceAccount": + fallthrough + case "SecurityPolicy": + fallthrough + case "StaticRoute": + fallthrough + case "SubnetPort": + fallthrough + case "Subnet": + fallthrough + case "SubnetSet": + ns := req.Namespace + enabled, err := r.IsVPCEnabledOnNamespace(ns) + if err != nil { + log.Error(err, "failed to check if VPC is enabled when validating CR creation", "Namespace", ns, objKind, req.Namespace+"/"+req.Name) + returnedErr := fmt.Errorf("unable to check the default network type in Namespace %s: %v", ns, err) + return admission.Errored(http.StatusBadRequest, returnedErr) + } + if !enabled { + log.Info("VPC is not enabled in Namespace, reject CR", "Namespace", ns, objKind, req.Namespace+"/"+req.Name) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("VPC is not enabled in Namespace %s", ns)) + } + return admission.Allowed("") + default: + log.Info("Unsupported kind in the validation, allow by default", "kind", objKind) + return admission.Allowed("") + } +} diff --git a/pkg/controllers/vpcnetwork/testing/network_testing.go b/pkg/controllers/vpcnetwork/testing/network_testing.go new file mode 100644 index 000000000..7510ab1cd --- /dev/null +++ b/pkg/controllers/vpcnetwork/testing/network_testing.go @@ -0,0 +1,26 @@ +/* +Copyright © 2024 VMware, Inc. All Rights Reserved. + + SPDX-License-Identifier: Apache-2.0 +*/ +package testing + +import ( + "context" + + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/vpcnetwork" +) + +// FakeVPCNetworkProvider is maintained for test only +type FakeVPCNetworkProvider struct { +} + +func (p *FakeVPCNetworkProvider) IsVPCEnabledOnNamespace(ns string) (bool, error) { + return true, nil +} + +func (p *FakeVPCNetworkProvider) ReconcileWithVPCFilters(resource string, ctx context.Context, req ctrl.Request, innerFunc vpcnetwork.ReconcileFunc) (ctrl.Result, error) { + return innerFunc(ctx, req) +} diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index 08d671dff..914b0fc0b 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -78,6 +78,7 @@ const ( AnnotationAttachmentRef string = "nsx.vmware.com/attachment_ref" AnnotationPodMAC string = "nsx.vmware.com/mac" AnnotationPodAttachment string = "nsx.vmware.com/attachment" + LabelWorkloadNamespace string = "vmware.com/for-user-workloads" TagScopePodName string = "nsx-op/pod_name" TagScopePodUID string = "nsx-op/pod_uid" ValueMajorVersion string = "1" diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 0b537b313..17e33a635 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -147,6 +147,19 @@ func IsSystemNamespace(c client.Client, ns string, obj *v1.Namespace) (bool, err return false, nil } +func IsVPCSystemNamespace(c client.Client, ns string, obj *v1.Namespace) (bool, error) { + nsObj := &v1.Namespace{} + if obj != nil { + nsObj = obj + } else if err := c.Get(context.Background(), types.NamespacedName{Namespace: ns, Name: ns}, nsObj); err != nil { + return false, client.IgnoreNotFound(err) + } + if wlNSLabelVal, ok := nsObj.Labels[common.LabelWorkloadNamespace]; ok && strings.ToLower(wlNSLabelVal) == "true" { + return false, nil + } + return true, nil +} + // CheckPodHasNamedPort checks if the pod has a named port, it filters the pod events // we don't want give concern. func CheckPodHasNamedPort(pod v1.Pod, reason string) bool {