Skip to content

Commit 2496f10

Browse files
gran-vmvgran-vmv
authored
[AddressBinding] Add admission webhook to validate CREATE/UPDATE request (vmware-tanzu#782)
Signed-off-by: gran <[email protected]>
1 parent 3cabf3c commit 2496f10

File tree

9 files changed

+253
-23
lines changed

9 files changed

+253
-23
lines changed

‎build/yaml/webhook/certificate.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ metadata:
3131
spec:
3232
# $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize
3333
dnsNames:
34-
- subnetset.vmware-system-nsx.svc
35-
- subnetset.vmware-system-nsx.svc.cluster.local
34+
- vmware-system-nsx-operator-webhook-service.vmware-system-nsx.svc
35+
- vmware-system-nsx-operator-webhook-service.vmware-system-nsx.svc.cluster.local
3636
issuerRef:
3737
kind: Issuer
3838
name: selfsigned-issuer

‎build/yaml/webhook/manifests.yaml‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ webhooks:
99
- v1
1010
clientConfig:
1111
service:
12-
name: subnetset
12+
name: vmware-system-nsx-operator-webhook-service
1313
namespace: vmware-system-nsx
1414
# kubebuilder webhookpath.
1515
path: /validate-nsx-vmware-com-v1alpha1-subnetset
@@ -30,3 +30,23 @@ webhooks:
3030
resources:
3131
- subnetsets
3232
sideEffects: None
33+
- admissionReviewVersions:
34+
- v1
35+
clientConfig:
36+
service:
37+
name: vmware-system-nsx-operator-webhook-service
38+
namespace: vmware-system-nsx
39+
path: /validate-crd-nsx-vmware-com-v1alpha1-addressbinding
40+
failurePolicy: Fail
41+
name: addressbinding.validating.crd.nsx.vmware.com
42+
rules:
43+
- apiGroups:
44+
- crd.nsx.vmware.com
45+
apiVersions:
46+
- v1alpha1
47+
operations:
48+
- CREATE
49+
- UPDATE
50+
resources:
51+
- addressbindings
52+
sideEffects: None

‎build/yaml/webhook/service.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ metadata:
99
app.kubernetes.io/created-by: nsx-operator
1010
app.kubernetes.io/part-of: nsx-operator
1111
app.kubernetes.io/managed-by: kustomize
12-
name: subnetset
12+
name: vmware-system-nsx-operator-webhook-service
1313
namespace: vmware-system-nsx
1414
spec:
1515
ports:

‎cmd/main.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
logf "sigs.k8s.io/controller-runtime/pkg/log"
1919
"sigs.k8s.io/controller-runtime/pkg/manager"
2020
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
21+
"sigs.k8s.io/controller-runtime/pkg/webhook"
2122

2223
"github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1"
2324
crdv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
@@ -213,18 +214,26 @@ func startServiceController(mgr manager.Manager, nsxClient *nsx.Client) {
213214
if err := subnet.StartSubnetController(mgr, subnetService, subnetPortService, vpcService); err != nil {
214215
os.Exit(1)
215216
}
216-
enableWebhook := true
217+
var hookServer webhook.Server
217218
if _, err := os.Stat(config.WebhookCertDir); errors.Is(err, os.ErrNotExist) {
218219
log.Error(err, "server cert not found, disabling webhook server", "cert", config.WebhookCertDir)
219-
enableWebhook = false
220+
} else {
221+
hookServer = webhook.NewServer(webhook.Options{
222+
Port: config.WebhookServerPort,
223+
CertDir: config.WebhookCertDir,
224+
})
225+
if err := mgr.Add(hookServer); err != nil {
226+
log.Error(err, "failed to add hook server")
227+
os.Exit(1)
228+
}
220229
}
221-
if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, enableWebhook); err != nil {
230+
if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, hookServer); err != nil {
222231
os.Exit(1)
223232
}
224233

225234
node.StartNodeController(mgr, nodeService)
226235
staticroutecontroller.StartStaticRouteController(mgr, staticRouteService)
227-
subnetport.StartSubnetPortController(mgr, subnetPortService, subnetService, vpcService)
236+
subnetport.StartSubnetPortController(mgr, subnetPortService, subnetService, vpcService, hookServer)
228237
pod.StartPodController(mgr, subnetPortService, subnetService, vpcService, nodeService)
229238
StartIPAddressAllocationController(mgr, ipAddressAllocationService, vpcService)
230239
networkpolicycontroller.StartNetworkPolicyController(mgr, commonService, vpcService)

‎cmd/webhookcert/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ func generateWebhookCerts() error {
9898
Bytes: caBytes,
9999
})
100100

101-
dnsNames := []string{"subnetset", "subnetset.vmware-system-nsx", "subnetset.vmware-system-nsx.svc"}
102-
commonName := "subnetset.vmware-system-nsx.svc"
101+
dnsNames := []string{"vmware-system-nsx-operator-webhook-service", "vmware-system-nsx-operator-webhook-service.vmware-system-nsx", "vmware-system-nsx-operator-webhook-service.vmware-system-nsx.svc"}
102+
commonName := "vmware-system-nsx-operator-webhook-service.vmware-system-nsx.svc"
103103

104104
serialNumber, err = rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
105105
if err != nil {

‎pkg/controllers/subnetport/addressbinding_webhook.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package subnetport
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
"reflect"
8+
9+
admissionv1 "k8s.io/api/admission/v1"
10+
"sigs.k8s.io/controller-runtime/pkg/client"
11+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
12+
13+
"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
14+
"github.com/vmware-tanzu/nsx-operator/pkg/util"
15+
)
16+
17+
// Create validator instead of using the existing one in controller-runtime because the existing one can't
18+
// inspect admission.Request in Handle function.
19+
20+
//+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
21+
22+
type AddressBindingValidator struct {
23+
Client client.Client
24+
decoder admission.Decoder
25+
}
26+
27+
// Handle handles admission requests.
28+
func (v *AddressBindingValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
29+
ab := &v1alpha1.AddressBinding{}
30+
if req.Operation == admissionv1.Delete {
31+
return admission.Allowed("")
32+
} else {
33+
err := v.decoder.Decode(req, ab)
34+
if err != nil {
35+
log.Error(err, "error while decoding AddressBinding", "AddressBinding", req.Namespace+"/"+req.Name)
36+
return admission.Errored(http.StatusBadRequest, err)
37+
}
38+
}
39+
switch req.Operation {
40+
case admissionv1.Create:
41+
existingAddressBindingList := &v1alpha1.AddressBindingList{}
42+
abIndexValue := fmt.Sprintf("%s/%s", ab.Namespace, ab.Spec.VMName)
43+
err := v.Client.List(context.TODO(), existingAddressBindingList, client.MatchingFields{util.AddressBindingNamespaceVMIndexKey: abIndexValue})
44+
if err != nil {
45+
log.Error(err, "failed to list AddressBinding from cache", "indexValue", abIndexValue)
46+
return admission.Errored(http.StatusInternalServerError, err)
47+
}
48+
for _, existingAddressBinding := range existingAddressBindingList.Items {
49+
if ab.Name != existingAddressBinding.Name && ab.Spec.InterfaceName == existingAddressBinding.Spec.InterfaceName {
50+
return admission.Denied("interface already has AddressBinding")
51+
}
52+
}
53+
case admissionv1.Update:
54+
oldAddressBinding := &v1alpha1.AddressBinding{}
55+
if err := v.decoder.DecodeRaw(req.OldObject, oldAddressBinding); err != nil {
56+
log.Error(err, "error while decoding AddressBinding", "AddressBinding", req.Namespace+"/"+req.Name)
57+
return admission.Errored(http.StatusBadRequest, err)
58+
}
59+
if !reflect.DeepEqual(ab.Spec, oldAddressBinding.Spec) {
60+
return admission.Denied("update AddressBinding is not allowed")
61+
}
62+
}
63+
return admission.Allowed("")
64+
}

‎pkg/controllers/subnetport/addressbinding_webhook_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package subnetport
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
"testing"
8+
9+
"github.com/agiledragon/gomonkey/v2"
10+
"github.com/stretchr/testify/assert"
11+
admissionv1 "k8s.io/api/admission/v1"
12+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
"k8s.io/apimachinery/pkg/util/json"
15+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
16+
"sigs.k8s.io/controller-runtime/pkg/client"
17+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
18+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
19+
20+
"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
21+
"github.com/vmware-tanzu/nsx-operator/pkg/util"
22+
)
23+
24+
func TestAddressBindingValidator_Handle(t *testing.T) {
25+
req1, _ := json.Marshal(&v1alpha1.AddressBinding{
26+
ObjectMeta: v1.ObjectMeta{
27+
Namespace: "ns1",
28+
Name: "ab1",
29+
},
30+
Spec: v1alpha1.AddressBindingSpec{
31+
VMName: "vm1",
32+
InterfaceName: "inf1",
33+
},
34+
})
35+
req1New, _ := json.Marshal(&v1alpha1.AddressBinding{
36+
ObjectMeta: v1.ObjectMeta{
37+
Namespace: "ns1",
38+
Name: "ab1",
39+
},
40+
Spec: v1alpha1.AddressBindingSpec{
41+
VMName: "vm1",
42+
InterfaceName: "inf1new",
43+
},
44+
})
45+
req2, _ := json.Marshal(&v1alpha1.AddressBinding{
46+
ObjectMeta: v1.ObjectMeta{
47+
Namespace: "ns1",
48+
Name: "ab2",
49+
},
50+
Spec: v1alpha1.AddressBindingSpec{
51+
VMName: "vm1",
52+
InterfaceName: "inf2",
53+
},
54+
})
55+
type args struct {
56+
req admission.Request
57+
}
58+
tests := []struct {
59+
name string
60+
args args
61+
prepareFunc func(*testing.T, client.Client, context.Context) *gomonkey.Patches
62+
want admission.Response
63+
}{
64+
{
65+
name: "delete",
66+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Delete}}},
67+
want: admission.Allowed(""),
68+
},
69+
{
70+
name: "create decode error",
71+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create}}},
72+
want: admission.Errored(http.StatusBadRequest, fmt.Errorf("there is no content to decode")),
73+
},
74+
{
75+
name: "create",
76+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: req1}}}},
77+
want: admission.Allowed(""),
78+
},
79+
{
80+
name: "create list error",
81+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: req1}}}},
82+
prepareFunc: func(t *testing.T, client client.Client, ctx context.Context) *gomonkey.Patches {
83+
return gomonkey.ApplyMethodSeq(client, "List", []gomonkey.OutputCell{{
84+
Values: gomonkey.Params{fmt.Errorf("mock error")},
85+
Times: 1,
86+
}})
87+
},
88+
want: admission.Errored(http.StatusInternalServerError, fmt.Errorf("mock error")),
89+
},
90+
{
91+
name: "create dup",
92+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: req2}}}},
93+
want: admission.Denied("interface already has AddressBinding"),
94+
},
95+
{
96+
name: "update decode error",
97+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Update, Object: runtime.RawExtension{Raw: req1}}}},
98+
want: admission.Errored(http.StatusBadRequest, fmt.Errorf("there is no content to decode")),
99+
},
100+
{
101+
name: "update changed",
102+
args: args{req: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Update, Object: runtime.RawExtension{Raw: req1New}, OldObject: runtime.RawExtension{Raw: req1}}}},
103+
want: admission.Denied("update AddressBinding is not allowed"),
104+
},
105+
}
106+
for _, tt := range tests {
107+
t.Run(tt.name, func(t *testing.T) {
108+
scheme := clientgoscheme.Scheme
109+
v1alpha1.AddToScheme(scheme)
110+
client := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&v1alpha1.AddressBinding{}).WithIndex(&v1alpha1.AddressBinding{}, util.AddressBindingNamespaceVMIndexKey, addressBindingNamespaceVMIndexFunc).Build()
111+
decoder := admission.NewDecoder(scheme)
112+
ctx := context.TODO()
113+
client.Create(ctx, &v1alpha1.AddressBinding{
114+
ObjectMeta: v1.ObjectMeta{
115+
Namespace: "ns1",
116+
Name: "ab2a",
117+
},
118+
Spec: v1alpha1.AddressBindingSpec{
119+
VMName: "vm1",
120+
InterfaceName: "inf2",
121+
},
122+
})
123+
if tt.prepareFunc != nil {
124+
patches := tt.prepareFunc(t, client, ctx)
125+
defer patches.Reset()
126+
}
127+
v := &AddressBindingValidator{
128+
Client: client,
129+
decoder: decoder,
130+
}
131+
assert.Equalf(t, tt.want, v.Handle(ctx, tt.args.req), "Handle()")
132+
})
133+
}
134+
}

‎pkg/controllers/subnetport/subnetport_controller.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"sigs.k8s.io/controller-runtime/pkg/handler"
2828
"sigs.k8s.io/controller-runtime/pkg/predicate"
2929
"sigs.k8s.io/controller-runtime/pkg/reconcile"
30+
"sigs.k8s.io/controller-runtime/pkg/webhook"
31+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
3032

3133
"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
3234
"github.com/vmware-tanzu/nsx-operator/pkg/controllers/common"
@@ -256,7 +258,7 @@ func (r *SubnetPortReconciler) vmMapFunc(_ context.Context, vm client.Object) []
256258
return requests
257259
}
258260

259-
func StartSubnetPortController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPortService, subnetService *subnet.SubnetService, vpcService *vpc.VPCService) {
261+
func StartSubnetPortController(mgr ctrl.Manager, subnetPortService *subnetport.SubnetPortService, subnetService *subnet.SubnetService, vpcService *vpc.VPCService, hookServer webhook.Server) {
260262
subnetPortReconciler := SubnetPortReconciler{
261263
Client: mgr.GetClient(),
262264
Scheme: mgr.GetScheme(),
@@ -269,6 +271,15 @@ func StartSubnetPortController(mgr ctrl.Manager, subnetPortService *subnetport.S
269271
log.Error(err, "failed to create controller", "controller", "SubnetPort")
270272
os.Exit(1)
271273
}
274+
if hookServer != nil {
275+
hookServer.Register("/validate-crd-nsx-vmware-com-v1alpha1-addressbinding",
276+
&webhook.Admission{
277+
Handler: &AddressBindingValidator{
278+
Client: mgr.GetClient(),
279+
decoder: admission.NewDecoder(mgr.GetScheme()),
280+
},
281+
})
282+
}
272283
go common.GenericGarbageCollector(make(chan bool), servicecommon.GCInterval, subnetPortReconciler.CollectGarbage)
273284
}
274285

‎pkg/controllers/subnetset/subnetset_controller.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2323

2424
"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
25-
"github.com/vmware-tanzu/nsx-operator/pkg/config"
2625
"github.com/vmware-tanzu/nsx-operator/pkg/controllers/common"
2726
"github.com/vmware-tanzu/nsx-operator/pkg/logger"
2827
"github.com/vmware-tanzu/nsx-operator/pkg/metrics"
@@ -372,7 +371,7 @@ func (r *SubnetSetReconciler) deleteStaleSubnets(ctx context.Context, nsxSubnets
372371

373372
func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetService,
374373
subnetPortService servicecommon.SubnetPortServiceProvider, vpcService servicecommon.VPCServiceProvider,
375-
enableWebhook bool,
374+
hookServer webhook.Server,
376375
) error {
377376
subnetsetReconciler := &SubnetSetReconciler{
378377
Client: mgr.GetClient(),
@@ -382,7 +381,7 @@ func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetServ
382381
VPCService: vpcService,
383382
Recorder: mgr.GetEventRecorderFor("subnetset-controller"),
384383
}
385-
if err := subnetsetReconciler.Start(mgr, enableWebhook); err != nil {
384+
if err := subnetsetReconciler.Start(mgr, hookServer); err != nil {
386385
log.Error(err, "Failed to create controller", "controller", "SubnetSet")
387386
return err
388387
}
@@ -391,19 +390,12 @@ func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetServ
391390
}
392391

393392
// Start setup manager
394-
func (r *SubnetSetReconciler) Start(mgr ctrl.Manager, enableWebhook bool) error {
393+
func (r *SubnetSetReconciler) Start(mgr ctrl.Manager, hookServer webhook.Server) error {
395394
err := r.setupWithManager(mgr)
396395
if err != nil {
397396
return err
398397
}
399-
if enableWebhook {
400-
hookServer := webhook.NewServer(webhook.Options{
401-
Port: config.WebhookServerPort,
402-
CertDir: config.WebhookCertDir,
403-
})
404-
if err := mgr.Add(hookServer); err != nil {
405-
return err
406-
}
398+
if hookServer != nil {
407399
hookServer.Register("/validate-nsx-vmware-com-v1alpha1-subnetset",
408400
&webhook.Admission{
409401
Handler: &SubnetSetValidator{

0 commit comments

Comments
 (0)