diff --git a/build/yaml/webhook/manifests.yaml b/build/yaml/webhook/manifests.yaml index 71bc17b0b..6bdb1c44f 100644 --- a/build/yaml/webhook/manifests.yaml +++ b/build/yaml/webhook/manifests.yaml @@ -30,3 +30,23 @@ webhooks: resources: - subnetsets sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: subnetset + namespace: vmware-system-nsx + path: /validate-crd-nsx-vmware-com-v1alpha1-addressbinding + failurePolicy: Fail + name: addressbinding.validating.crd.nsx.vmware.com + rules: + - apiGroups: + - crd.nsx.vmware.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - addressbindings + sideEffects: None diff --git a/cmd/main.go b/cmd/main.go index cb72b6190..428f05015 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -18,6 +18,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/webhook" "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" crdv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" @@ -213,18 +214,26 @@ func startServiceController(mgr manager.Manager, nsxClient *nsx.Client) { if err := subnet.StartSubnetController(mgr, subnetService, subnetPortService, vpcService); err != nil { os.Exit(1) } - enableWebhook := true + var hookServer webhook.Server 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 { + hookServer = webhook.NewServer(webhook.Options{ + Port: config.WebhookServerPort, + CertDir: config.WebhookCertDir, + }) + if err := mgr.Add(hookServer); err != nil { + log.Error(err, "failed to add hook server") + os.Exit(1) + } } - if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, enableWebhook); err != nil { + if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, hookServer); err != nil { os.Exit(1) } node.StartNodeController(mgr, nodeService) staticroutecontroller.StartStaticRouteController(mgr, staticRouteService) - subnetport.StartSubnetPortController(mgr, subnetPortService, subnetService, vpcService) + subnetport.StartSubnetPortController(mgr, subnetPortService, subnetService, vpcService, hookServer) pod.StartPodController(mgr, subnetPortService, subnetService, vpcService, nodeService) StartIPAddressAllocationController(mgr, ipAddressAllocationService, vpcService) networkpolicycontroller.StartNetworkPolicyController(mgr, commonService, vpcService) diff --git a/pkg/controllers/subnetport/addressbinding_webhook.go b/pkg/controllers/subnetport/addressbinding_webhook.go new file mode 100644 index 000000000..9c7a3c983 --- /dev/null +++ b/pkg/controllers/subnetport/addressbinding_webhook.go @@ -0,0 +1,68 @@ +package subnetport + +import ( + "context" + "fmt" + "net/http" + "reflect" + + admissionv1 "k8s.io/api/admission/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/util" +) + +// log is for logging in this package. +var addressbindinglog = logf.Log.WithName("addressbinding-webhook") + +// Create validator instead of using the existing one in controller-runtime because the existing one can't +// inspect admission.Request in Handle function. + +//+kubebuilder:webhook:path=/validate-crd-nsx-vmware-com-v1alpha1-addressbinding,mutating=false,failurePolicy=fail,sideEffects=None,groups=crd.nsx.vmware.com,resources=addressbindings,verbs=create;update,versions=v1alpha1,name=addressbinding.validating.crd.nsx.vmware.com,admissionReviewVersions=v1 + +type AddressBindingValidator struct { + Client client.Client + decoder admission.Decoder +} + +// Handle handles admission requests. +func (v *AddressBindingValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + ab := &v1alpha1.AddressBinding{} + if req.Operation == admissionv1.Delete { + return admission.Allowed("") + } else { + err := v.decoder.Decode(req, ab) + if err != nil { + addressbindinglog.Error(err, "error while decoding AddressBinding", "AddressBinding", req.Namespace+"/"+req.Name) + return admission.Errored(http.StatusBadRequest, err) + } + } + switch req.Operation { + case admissionv1.Create: + existingAddressBindingList := &v1alpha1.AddressBindingList{} + abIndexValue := fmt.Sprintf("%s/%s", ab.Namespace, ab.Spec.VMName) + err := v.Client.List(context.TODO(), existingAddressBindingList, client.MatchingFields{util.AddressBindingNamespaceVMIndexKey: abIndexValue}) + if err != nil { + addressbindinglog.Error(err, "failed to list AddressBinding from cache", "indexValue", abIndexValue) + return admission.Errored(http.StatusInternalServerError, err) + } + for _, existingAddressBinding := range existingAddressBindingList.Items { + if ab.Name != existingAddressBinding.Name && ab.Spec.InterfaceName == existingAddressBinding.Spec.InterfaceName { + return admission.Denied("interface already has AddressBinding") + } + } + case admissionv1.Update: + oldAddressBinding := &v1alpha1.AddressBinding{} + if err := v.decoder.DecodeRaw(req.OldObject, oldAddressBinding); err != nil { + addressbindinglog.Error(err, "error while decoding AddressBinding", "AddressBinding", req.Namespace+"/"+req.Name) + return admission.Errored(http.StatusBadRequest, err) + } + if !reflect.DeepEqual(ab.Spec, oldAddressBinding.Spec) { + return admission.Denied("update AddressBinding is not allowed") + } + } + return admission.Allowed("") +} diff --git a/pkg/controllers/subnetport/addressbinding_webhook_test.go b/pkg/controllers/subnetport/addressbinding_webhook_test.go new file mode 100644 index 000000000..9fa8f7791 --- /dev/null +++ b/pkg/controllers/subnetport/addressbinding_webhook_test.go @@ -0,0 +1,134 @@ +package subnetport + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/agiledragon/gomonkey/v2" + "github.com/stretchr/testify/assert" + admissionv1 "k8s.io/api/admission/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/json" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/util" +) + +func TestAddressBindingValidator_Handle(t *testing.T) { + req1, _ := json.Marshal(&v1alpha1.AddressBinding{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "ns1", + Name: "ab1", + }, + Spec: v1alpha1.AddressBindingSpec{ + VMName: "vm1", + InterfaceName: "inf1", + }, + }) + req1New, _ := json.Marshal(&v1alpha1.AddressBinding{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "ns1", + Name: "ab1", + }, + Spec: v1alpha1.AddressBindingSpec{ + VMName: "vm1", + InterfaceName: "inf1new", + }, + }) + req2, _ := json.Marshal(&v1alpha1.AddressBinding{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "ns1", + Name: "ab2", + }, + Spec: v1alpha1.AddressBindingSpec{ + VMName: "vm1", + InterfaceName: "inf2", + }, + }) + type args struct { + req admission.Request + } + tests := []struct { + name string + args args + prepareFunc func(*testing.T, client.Client, context.Context) *gomonkey.Patches + want admission.Response + }{ + { + name: "delete", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Delete}}}, + want: admission.Allowed(""), + }, + { + name: "create decode error", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create}}}, + want: admission.Errored(http.StatusBadRequest, fmt.Errorf("there is no content to decode")), + }, + { + name: "create", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: req1}}}}, + want: admission.Allowed(""), + }, + { + name: "create list error", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: req1}}}}, + prepareFunc: func(t *testing.T, client client.Client, ctx context.Context) *gomonkey.Patches { + return gomonkey.ApplyMethodSeq(client, "List", []gomonkey.OutputCell{{ + Values: gomonkey.Params{fmt.Errorf("mock error")}, + Times: 1, + }}) + }, + want: admission.Errored(http.StatusInternalServerError, fmt.Errorf("mock error")), + }, + { + name: "create dup", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: req2}}}}, + want: admission.Denied("interface already has AddressBinding"), + }, + { + name: "update decode error", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Update, Object: runtime.RawExtension{Raw: req1}}}}, + want: admission.Errored(http.StatusBadRequest, fmt.Errorf("there is no content to decode")), + }, + { + name: "update changed", + args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Update, Object: runtime.RawExtension{Raw: req1New}, OldObject: runtime.RawExtension{Raw: req1}}}}, + want: admission.Denied("update AddressBinding is not allowed"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := clientgoscheme.Scheme + v1alpha1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&v1alpha1.AddressBinding{}).WithIndex(&v1alpha1.AddressBinding{}, util.AddressBindingNamespaceVMIndexKey, addressBindingNamespaceVMIndexFunc).Build() + decoder := admission.NewDecoder(scheme) + ctx := context.TODO() + client.Create(ctx, &v1alpha1.AddressBinding{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "ns1", + Name: "ab2a", + }, + Spec: v1alpha1.AddressBindingSpec{ + VMName: "vm1", + InterfaceName: "inf2", + }, + }) + if tt.prepareFunc != nil { + patches := tt.prepareFunc(t, client, ctx) + defer patches.Reset() + } + v := &AddressBindingValidator{ + Client: client, + decoder: decoder, + } + assert.Equalf(t, tt.want, v.Handle(ctx, tt.args.req), "Handle()") + }) + } +} diff --git a/pkg/controllers/subnetport/subnetport_controller.go b/pkg/controllers/subnetport/subnetport_controller.go index ee6a15cfc..eb422bfed 100644 --- a/pkg/controllers/subnetport/subnetport_controller.go +++ b/pkg/controllers/subnetport/subnetport_controller.go @@ -28,6 +28,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" @@ -257,7 +259,7 @@ func (r *SubnetPortReconciler) vmMapFunc(_ context.Context, vm client.Object) [] return requests } -func StartSubnetPortController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPortService, subnetService *subnet.SubnetService, vpcService *vpc.VPCService) { +func StartSubnetPortController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPortService, subnetService *subnet.SubnetService, vpcService *vpc.VPCService, hookServer webhook.Server) { subnetPortReconciler := SubnetPortReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -270,6 +272,15 @@ func StartSubnetPortController(mgr ctrl.Manager, subnetPortService *subnetport.S log.Error(err, "failed to create controller", "controller", "SubnetPort") os.Exit(1) } + if hookServer != nil { + hookServer.Register("/validate-crd-nsx-vmware-com-v1alpha1-addressbinding", + &webhook.Admission{ + Handler: &AddressBindingValidator{ + Client: mgr.GetClient(), + decoder: admission.NewDecoder(mgr.GetScheme()), + }, + }) + } go common.GenericGarbageCollector(make(chan bool), servicecommon.GCInterval, subnetPortReconciler.CollectGarbage) } diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index 7c182c2d7..a4cb4c27d 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/vpc/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" @@ -369,7 +368,7 @@ func (r *SubnetSetReconciler) deleteStaleSubnets(ctx context.Context, nsxSubnets func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnetPortService servicecommon.SubnetPortServiceProvider, vpcService servicecommon.VPCServiceProvider, - enableWebhook bool, + hookServer webhook.Server, ) error { subnetsetReconciler := &SubnetSetReconciler{ Client: mgr.GetClient(), @@ -379,7 +378,7 @@ func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetServ VPCService: vpcService, Recorder: mgr.GetEventRecorderFor("subnetset-controller"), } - if err := subnetsetReconciler.Start(mgr, enableWebhook); err != nil { + if err := subnetsetReconciler.Start(mgr, hookServer); err != nil { log.Error(err, "Failed to create controller", "controller", "SubnetSet") return err } @@ -388,20 +387,13 @@ func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetServ } // Start setup manager -func (r *SubnetSetReconciler) Start(mgr ctrl.Manager, enableWebhook bool) error { +func (r *SubnetSetReconciler) Start(mgr ctrl.Manager, hookServer webhook.Server) error { err := r.setupWithManager(mgr) if err != nil { 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.Register("/validate-nsx-vmware-com-v1alpha1-subnetset", + if hookServer != nil { + hookServer.Register("/validate-crd-nsx-vmware-com-v1alpha1-subnetset", &webhook.Admission{ Handler: &SubnetSetValidator{ Client: mgr.GetClient(),