Skip to content

Commit 017171a

Browse files
kcp-ci-botkcp-ci-bot
authored
Merge pull request #3414 from gman0/admissionwebhooks-tests-and-docs
Improvements for webhooks docs and tests
2 parents 3037f91 + a2f5d62 commit 017171a

File tree

4 files changed

+131
-24
lines changed

4 files changed

+131
-24
lines changed

‎docs/content/concepts/apis/admission-webhooks.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Admission Webhooks
2+
3+
kcp extends the vanilla [admission plugins](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/) for webhooks, and makes them cluster-aware.
4+
5+
```
6+
┌────────────────────────┐
7+
│ Consumer Workspace ws2 │
8+
├────────────────────────┤
9+
│ │
10+
┌────┼─ Widgets APIBinding │
11+
│ │ │
12+
│ │ Widget a │
13+
┌───────────────────────────────────────────────┐ │ │ Widget b │
14+
│ API Provider Workspace ws1 │ │ │ Widget c │
15+
├───────────────────────────────────────────────┤ │ │ │
16+
│ │ │ └────────────────────────┘
17+
│ Widgets APIExport ◄──────────────┼────┤
18+
│ │ │ │
19+
│ ▼ │ │
20+
│ Widgets APIResourceSchema │ │ ┌────────────────────────┐
21+
│ (widgets.v1.example.org) │ │ │ Consumer Workspace ws3 │
22+
│ ▲ │ │ ├────────────────────────┤
23+
│ │ │ │ │ │
24+
│ ┌───────────────────┴─────────────────────┐ │ └────┼─ Widgets APIBinding │
25+
│ │ Mutating/ValidatingWebhookConfiguration │ │ │ │
26+
│ │ for widgets.v1.example.org │ │ │ Widget a │
27+
│ │ │ │ │ Widget b │
28+
│ │ Handle a from ws2 (APIResourceSchema) │ │ │ Widget c │
29+
│ │ Handle b from ws3 (APIResourceSchema) │ │ │ │
30+
│ │ Handle a from ws1 (CRD) │ │ └────────────────────────┘
31+
│ │ ... │ │
32+
│ └───────────────────┬─────────────────────┘ │
33+
│ │ │
34+
│ ▼ │
35+
│ Widgets CustomResourceDefinition │
36+
│ (widgets.v1.example.org) │
37+
│ │
38+
│ Widget a │
39+
│ │
40+
└───────────────────────────────────────────────┘
41+
```
42+
43+
When an object is to be mutated or validated, the webhook admission plugin ([`apis.kcp.io/MutatingWebhook`](https://github.com/kcp-dev/kcp/tree/main/pkg/admission/mutatingwebhook) and [`apis.kcp.io/ValidatingWebhook`](https://github.com/kcp-dev/kcp/tree/main/pkg/admission/validatingwebhook) respectively) looks for the owner of the resource schema. Once found, it then dispatches the handling for that object in the owner's workspace. There are two such cases in the diagram above:
44+
45+
* **Admitting bound resources.** During the request handling, Widget objects inside the consumer workspaces `ws2` and `ws3` are picked up by the respective webhook admission plugin. The plugin sees the resource's schema comes from an APIBinding, and so it sets up an instance of `{Mutating,Validating}Webhook` to be working with its APIExport's workspace, in `ws1`. Afterwards, normal webhook admission flow continues: the request is dispatched to all eligible webhook configurations inside `ws1` and the object in request is mutated or validated.
46+
* **Admitting local resources.** The second case is when the webhook configuration exists in the same workspace as the object it's handling. The admission plugin sees the resource is not sourced via an APIBinding, and so it looks for eligible webhook configurations locally, and dispatches the request to the webhooks there. The same would of course be true if APIExport and its APIBinding lived in the same workspace: the APIExport would resolve to the same cluster.
47+
48+
Lastly, objects in admission review are annotated with the name of the workspace that owns that object. For example, when Widget `b` from `ws3` is being validated, its caught by `ValidatingWebhookConfiguration` in `ws1`, but the webhook will see `kcp.io/cluster: ws3` annotation on the reviewed object.

‎test/e2e/apibinding/apibinding_webhook_test.go

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
gohttp "net/http"
2424
"path/filepath"
25+
"sync/atomic"
2526
"testing"
2627
"time"
2728

@@ -64,8 +65,8 @@ func TestAPIBindingMutatingWebhook(t *testing.T) {
6465
t.Cleanup(cancel)
6566

6667
orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture.
67-
sourcePath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
68-
targetPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
68+
sourcePath, sourceWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)
69+
targetPath, targetWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)
6970

7071
cfg := server.BaseConfig(t)
7172

@@ -141,10 +142,12 @@ func TestAPIBindingMutatingWebhook(t *testing.T) {
141142
deserializer := codecs.UniversalDeserializer()
142143

143144
t.Logf("Create test server and create mutating webhook for cowboys in both source and target cluster")
145+
var clusterInReviewObject atomic.Value
144146
testWebhooks := map[logicalcluster.Path]*webhookserver.AdmissionWebhookServer{}
145147
for _, cluster := range []logicalcluster.Path{sourcePath, targetPath} {
146148
testWebhooks[cluster] = &webhookserver.AdmissionWebhookServer{
147-
ResponseFn: func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) {
149+
ResponseFn: func(obj runtime.Object, review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) {
150+
clusterInReviewObject.Store(logicalcluster.From(obj.(*v1alpha1.Cowboy)).String())
148151
return &admissionv1.AdmissionResponse{Allowed: true}, nil
149152
},
150153
ObjectGVK: schema.GroupVersionKind{
@@ -205,6 +208,34 @@ func TestAPIBindingMutatingWebhook(t *testing.T) {
205208
return testWebhooks[sourcePath].Calls() >= 1, ""
206209
}, wait.ForeverTestTimeout, 100*time.Millisecond, "failed to create cowboy resource")
207210

211+
t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the target cluster")
212+
require.Equal(t, targetWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")
213+
214+
t.Logf("Create an APIBinding in workspace %q that points to the today-cowboys export", sourcePath)
215+
kcptestinghelpers.Eventually(t, func() (bool, string) {
216+
_, err := kcpClusterClient.Cluster(sourcePath).ApisV1alpha2().APIBindings().Create(ctx, apiBinding, metav1.CreateOptions{})
217+
return err == nil, fmt.Sprintf("Error creating APIBinding: %v", err)
218+
}, wait.ForeverTestTimeout, time.Millisecond*100)
219+
220+
t.Logf("Ensure cowboys are served in %q", sourcePath)
221+
require.Eventually(t, func() bool {
222+
_, err := cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").List(ctx, metav1.ListOptions{})
223+
return err == nil
224+
}, wait.ForeverTestTimeout, 100*time.Millisecond)
225+
t.Logf("Cowboys are served")
226+
227+
sourceWHCalls := testWebhooks[sourcePath].Calls()
228+
229+
t.Logf("Creating cowboy resource in source logical cluster, eventually going through admission webhook")
230+
require.Eventually(t, func() bool {
231+
_, err = cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
232+
require.NoError(t, err)
233+
return testWebhooks[sourcePath].Calls() > sourceWHCalls
234+
}, wait.ForeverTestTimeout, 100*time.Millisecond)
235+
236+
t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the source cluster")
237+
require.Equal(t, sourceWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")
238+
208239
t.Logf("Check that the in-workspace webhook was NOT called")
209240
require.Zero(t, testWebhooks[targetPath].Calls(), "in-workspace webhook should not have been called")
210241
}
@@ -219,8 +250,8 @@ func TestAPIBindingValidatingWebhook(t *testing.T) {
219250
t.Cleanup(cancel)
220251

221252
orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture.
222-
sourcePath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
223-
targetPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
253+
sourcePath, sourceWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)
254+
targetPath, targetWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)
224255

225256
cfg := server.BaseConfig(t)
226257

@@ -297,9 +328,11 @@ func TestAPIBindingValidatingWebhook(t *testing.T) {
297328

298329
t.Logf("Create test server and create validating webhook for cowboys in both source and target cluster")
299330
testWebhooks := map[logicalcluster.Path]*webhookserver.AdmissionWebhookServer{}
331+
var clusterInReviewObject atomic.Value
300332
for _, cluster := range []logicalcluster.Path{sourcePath, targetPath} {
301333
testWebhooks[cluster] = &webhookserver.AdmissionWebhookServer{
302-
ResponseFn: func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) {
334+
ResponseFn: func(obj runtime.Object, review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) {
335+
clusterInReviewObject.Store(logicalcluster.From(obj.(*v1alpha1.Cowboy)).String())
303336
return &admissionv1.AdmissionResponse{Allowed: true}, nil
304337
},
305338
ObjectGVK: schema.GroupVersionKind{
@@ -373,6 +406,34 @@ func TestAPIBindingValidatingWebhook(t *testing.T) {
373406
return testWebhooks[sourcePath].Calls() >= 1
374407
}, wait.ForeverTestTimeout, 100*time.Millisecond)
375408

409+
t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the target cluster")
410+
require.Equal(t, targetWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")
411+
412+
t.Logf("Create an APIBinding in workspace %q that points to the today-cowboys export", sourcePath)
413+
kcptestinghelpers.Eventually(t, func() (bool, string) {
414+
_, err := kcpClients.Cluster(sourcePath).ApisV1alpha2().APIBindings().Create(ctx, apiBinding, metav1.CreateOptions{})
415+
return err == nil, fmt.Sprintf("Error creating APIBinding: %v", err)
416+
}, wait.ForeverTestTimeout, time.Millisecond*100)
417+
418+
t.Logf("Ensure cowboys are served in %q", sourcePath)
419+
require.Eventually(t, func() bool {
420+
_, err := cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").List(ctx, metav1.ListOptions{})
421+
return err == nil
422+
}, wait.ForeverTestTimeout, 100*time.Millisecond)
423+
t.Logf("Cowboys are served")
424+
425+
sourceWHCalls := testWebhooks[sourcePath].Calls()
426+
427+
t.Logf("Creating cowboy resource in source logical cluster, eventually going through admission webhook")
428+
require.Eventually(t, func() bool {
429+
_, err = cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
430+
require.NoError(t, err)
431+
return testWebhooks[sourcePath].Calls() > sourceWHCalls
432+
}, wait.ForeverTestTimeout, 100*time.Millisecond)
433+
434+
t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the source cluster")
435+
require.Equal(t, sourceWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")
436+
376437
t.Logf("Check that the in-workspace webhook was NOT called")
377438
require.Zero(t, testWebhooks[targetPath].Calls(), "in-workspace webhook should not have been called")
378439
}

‎test/e2e/conformance/webhook_test.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package conformance
1818

1919
import (
2020
"context"
21-
"encoding/json"
2221
"path/filepath"
2322
"sync/atomic"
2423
"testing"
@@ -30,7 +29,6 @@ import (
3029
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
3130
"k8s.io/apimachinery/pkg/api/errors"
3231
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
33-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3432
"k8s.io/apimachinery/pkg/runtime"
3533
"k8s.io/apimachinery/pkg/runtime/schema"
3634
"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -75,12 +73,8 @@ func TestMutatingWebhookInWorkspace(t *testing.T) {
7573

7674
var clusterInReviewObject atomic.Value
7775
testWebhook := webhookserver.AdmissionWebhookServer{
78-
ResponseFn: func(review *v1.AdmissionReview) (*v1.AdmissionResponse, error) {
79-
var u unstructured.Unstructured
80-
if err := json.Unmarshal(review.Request.Object.Raw, &u.Object); err != nil {
81-
return nil, err
82-
}
83-
clusterInReviewObject.Store(logicalcluster.From(&u).String())
76+
ResponseFn: func(obj runtime.Object, review *v1.AdmissionReview) (*v1.AdmissionResponse, error) {
77+
clusterInReviewObject.Store(logicalcluster.From(obj.(*v1alpha1.Cowboy)).String())
8478
return &v1.AdmissionResponse{Allowed: true}, nil
8579
},
8680
ObjectGVK: schema.GroupVersionKind{
@@ -197,8 +191,10 @@ func TestValidatingWebhookInWorkspace(t *testing.T) {
197191
codecs := serializer.NewCodecFactory(scheme)
198192
deserializer := codecs.UniversalDeserializer()
199193

194+
var clusterInReviewObject atomic.Value
200195
testWebhook := webhookserver.AdmissionWebhookServer{
201-
ResponseFn: func(review *v1.AdmissionReview) (*v1.AdmissionResponse, error) {
196+
ResponseFn: func(obj runtime.Object, review *v1.AdmissionReview) (*v1.AdmissionResponse, error) {
197+
clusterInReviewObject.Store(logicalcluster.From(obj.(*v1alpha1.Cowboy)).String())
202198
return &v1.AdmissionResponse{Allowed: true}, nil
203199
},
204200
ObjectGVK: schema.GroupVersionKind{
@@ -215,9 +211,10 @@ func TestValidatingWebhookInWorkspace(t *testing.T) {
215211
testWebhook.StartTLS(t, filepath.Join(dirPath, "apiserver.crt"), filepath.Join(dirPath, "apiserver.key"), cfg.Host, port)
216212

217213
orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture.
218-
ws1, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
219-
ws2, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
220-
workspaces := []logicalcluster.Path{ws1, ws2}
214+
ws1Path, ws1 := kcptesting.NewWorkspaceFixture(t, server, orgPath)
215+
ws2Path, ws2 := kcptesting.NewWorkspaceFixture(t, server, orgPath)
216+
paths := []logicalcluster.Path{ws1Path, ws2Path}
217+
workspaces := []*tenancyv1alpha1.Workspace{ws1, ws2}
221218

222219
kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(cfg)
223220
require.NoError(t, err, "failed to construct client for server")
@@ -227,7 +224,7 @@ func TestValidatingWebhookInWorkspace(t *testing.T) {
227224
require.NoError(t, err, "failed to construct apiextensions client for server")
228225

229226
t.Logf("Install the Cowboy resources into logical clusters")
230-
for _, wsPath := range workspaces {
227+
for _, wsPath := range paths {
231228
t.Logf("Bootstrapping Workspace CRDs in logical cluster %s", wsPath)
232229
crdClient := apiExtensionsClients.ApiextensionsV1().CustomResourceDefinitions()
233230
wildwest.Create(t, wsPath, crdClient, metav1.GroupResource{Group: "wildwest.dev", Resource: "cowboys"})
@@ -259,7 +256,7 @@ func TestValidatingWebhookInWorkspace(t *testing.T) {
259256
AdmissionReviewVersions: []string{"v1"},
260257
}},
261258
}
262-
_, err = kubeClusterClient.Cluster(workspaces[0]).AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(ctx, webhook, metav1.CreateOptions{})
259+
_, err = kubeClusterClient.Cluster(paths[0]).AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(ctx, webhook, metav1.CreateOptions{})
263260
require.NoError(t, err, "failed to add validating webhook configurations")
264261

265262
cowboy := v1alpha1.Cowboy{
@@ -271,16 +268,17 @@ func TestValidatingWebhookInWorkspace(t *testing.T) {
271268

272269
t.Logf("Creating cowboy resource in first logical cluster")
273270
require.Eventually(t, func() bool {
274-
_, err = cowbyClusterClient.Cluster(workspaces[0]).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
271+
_, err = cowbyClusterClient.Cluster(paths[0]).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
275272
if err != nil && !errors.IsAlreadyExists(err) {
276273
return false
277274
}
278275
return testWebhook.Calls() == 1
279276
}, wait.ForeverTestTimeout, 100*time.Millisecond)
277+
require.Equal(t, workspaces[0].Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has the kcp.io/cluster annotation set")
280278

281279
// Avoid race condition here by making sure that CRD is served after installing the types into logical clusters
282280
t.Logf("Creating cowboy resource in second logical cluster")
283-
_, err = cowbyClusterClient.Cluster(workspaces[1]).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
281+
_, err = cowbyClusterClient.Cluster(paths[1]).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
284282
require.NoError(t, err, "failed to create cowboy resource in second logical cluster")
285283
require.Equal(t, 1, testWebhook.Calls(), "expected that the webhook is not called for logical cluster where webhook is not installed")
286284
}

‎test/e2e/fixtures/webhook/webhook.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
)
3535

3636
type AdmissionWebhookServer struct {
37-
ResponseFn func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error)
37+
ResponseFn func(obj runtime.Object, review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error)
3838
ObjectGVK schema.GroupVersionKind
3939
Deserializer runtime.Decoder
4040

@@ -156,7 +156,7 @@ func (s *AdmissionWebhookServer) ServeHTTP(resp http.ResponseWriter, req *http.R
156156
responseAdmissionReview := &admissionv1.AdmissionReview{
157157
TypeMeta: requestedAdmissionReview.TypeMeta,
158158
}
159-
r, err := s.ResponseFn(requestedAdmissionReview)
159+
r, err := s.ResponseFn(obj, requestedAdmissionReview)
160160
if err != nil {
161161
s.t.Logf("%v", err)
162162
http.Error(resp, err.Error(), http.StatusInternalServerError)

0 commit comments

Comments
 (0)