Skip to content

Commit

Permalink
[AddressBinding] Add admission webhook to validate CREATE/UPDATE request
Browse files Browse the repository at this point in the history
Signed-off-by: gran <[email protected]>
  • Loading branch information
gran-vmv authored and Ran Gu committed Oct 11, 2024
1 parent 762863a commit 61041d4
Show file tree
Hide file tree
Showing 6 changed files with 252 additions and 18 deletions.
20 changes: 20 additions & 0 deletions build/yaml/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 13 additions & 4 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
68 changes: 68 additions & 0 deletions pkg/controllers/subnetport/addressbinding_webhook.go
Original file line number Diff line number Diff line change
@@ -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("")
}
134 changes: 134 additions & 0 deletions pkg/controllers/subnetport/addressbinding_webhook_test.go
Original file line number Diff line number Diff line change
@@ -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()")
})
}
}
13 changes: 12 additions & 1 deletion pkg/controllers/subnetport/subnetport_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand All @@ -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)
}

Expand Down
18 changes: 5 additions & 13 deletions pkg/controllers/subnetset/subnetset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand All @@ -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
}
Expand All @@ -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(),
Expand Down

0 comments on commit 61041d4

Please sign in to comment.