Skip to content

Improvements for webhooks docs and tests #3414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 30, 2025
Merged

Improvements for webhooks docs and tests #3414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb43661
e2e/fixtures/webhook: pass the parsed obj to ResponseFn
gman0 May 16, 2025
2b7b2fe
e2e: added checks for logicalcluster annotation for in-review objs in…
gman0 May 16, 2025
5b723c6
e2e/apibinding: test webhook in source ws too
gman0 May 19, 2025
a2f5d62
docs: added description for admission webhooks
gman0 May 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions docs/content/concepts/apis/admission-webhooks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Admission Webhooks

kcp extends the vanilla [admission plugins](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/) for webhooks, and makes them cluster-aware.

```
┌────────────────────────┐
│ Consumer Workspace ws2 │
├────────────────────────┤
│ │
┌────┼─ Widgets APIBinding │
│ │ │
│ │ Widget a │
┌───────────────────────────────────────────────┐ │ │ Widget b │
│ API Provider Workspace ws1 │ │ │ Widget c │
├───────────────────────────────────────────────┤ │ │ │
│ │ │ └────────────────────────┘
│ Widgets APIExport ◄──────────────┼────┤
│ │ │ │
│ ▼ │ │
│ Widgets APIResourceSchema │ │ ┌────────────────────────┐
│ (widgets.v1.example.org) │ │ │ Consumer Workspace ws3 │
│ ▲ │ │ ├────────────────────────┤
│ │ │ │ │ │
│ ┌───────────────────┴─────────────────────┐ │ └────┼─ Widgets APIBinding │
│ │ Mutating/ValidatingWebhookConfiguration │ │ │ │
│ │ for widgets.v1.example.org │ │ │ Widget a │
│ │ │ │ │ Widget b │
│ │ Handle a from ws2 (APIResourceSchema) │ │ │ Widget c │
│ │ Handle b from ws3 (APIResourceSchema) │ │ │ │
│ │ Handle a from ws1 (CRD) │ │ └────────────────────────┘
│ │ ... │ │
│ └───────────────────┬─────────────────────┘ │
│ │ │
│ ▼ │
│ Widgets CustomResourceDefinition │
│ (widgets.v1.example.org) │
│ │
│ Widget a │
│ │
└───────────────────────────────────────────────┘
```

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:

* **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.
* **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.

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.
73 changes: 67 additions & 6 deletions test/e2e/apibinding/apibinding_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
gohttp "net/http"
"path/filepath"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -64,8 +65,8 @@ func TestAPIBindingMutatingWebhook(t *testing.T) {
t.Cleanup(cancel)

orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture.
sourcePath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
targetPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
sourcePath, sourceWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)
targetPath, targetWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)

cfg := server.BaseConfig(t)

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

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

t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the target cluster")
require.Equal(t, targetWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")

t.Logf("Create an APIBinding in workspace %q that points to the today-cowboys export", sourcePath)
kcptestinghelpers.Eventually(t, func() (bool, string) {
_, err := kcpClusterClient.Cluster(sourcePath).ApisV1alpha2().APIBindings().Create(ctx, apiBinding, metav1.CreateOptions{})
return err == nil, fmt.Sprintf("Error creating APIBinding: %v", err)
}, wait.ForeverTestTimeout, time.Millisecond*100)

t.Logf("Ensure cowboys are served in %q", sourcePath)
require.Eventually(t, func() bool {
_, err := cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").List(ctx, metav1.ListOptions{})
return err == nil
}, wait.ForeverTestTimeout, 100*time.Millisecond)
t.Logf("Cowboys are served")

sourceWHCalls := testWebhooks[sourcePath].Calls()

t.Logf("Creating cowboy resource in source logical cluster, eventually going through admission webhook")
require.Eventually(t, func() bool {
_, err = cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
require.NoError(t, err)
return testWebhooks[sourcePath].Calls() > sourceWHCalls
}, wait.ForeverTestTimeout, 100*time.Millisecond)

t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the source cluster")
require.Equal(t, sourceWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")

t.Logf("Check that the in-workspace webhook was NOT called")
require.Zero(t, testWebhooks[targetPath].Calls(), "in-workspace webhook should not have been called")
}
Expand All @@ -219,8 +250,8 @@ func TestAPIBindingValidatingWebhook(t *testing.T) {
t.Cleanup(cancel)

orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture.
sourcePath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
targetPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
sourcePath, sourceWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)
targetPath, targetWS := kcptesting.NewWorkspaceFixture(t, server, orgPath)

cfg := server.BaseConfig(t)

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

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

t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the target cluster")
require.Equal(t, targetWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")

t.Logf("Create an APIBinding in workspace %q that points to the today-cowboys export", sourcePath)
kcptestinghelpers.Eventually(t, func() (bool, string) {
_, err := kcpClients.Cluster(sourcePath).ApisV1alpha2().APIBindings().Create(ctx, apiBinding, metav1.CreateOptions{})
return err == nil, fmt.Sprintf("Error creating APIBinding: %v", err)
}, wait.ForeverTestTimeout, time.Millisecond*100)

t.Logf("Ensure cowboys are served in %q", sourcePath)
require.Eventually(t, func() bool {
_, err := cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").List(ctx, metav1.ListOptions{})
return err == nil
}, wait.ForeverTestTimeout, 100*time.Millisecond)
t.Logf("Cowboys are served")

sourceWHCalls := testWebhooks[sourcePath].Calls()

t.Logf("Creating cowboy resource in source logical cluster, eventually going through admission webhook")
require.Eventually(t, func() bool {
_, err = cowbyClusterClient.Cluster(sourcePath).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
require.NoError(t, err)
return testWebhooks[sourcePath].Calls() > sourceWHCalls
}, wait.ForeverTestTimeout, 100*time.Millisecond)

t.Logf("Check that the logicalcluster annotation on the object that triggered webhook is matching the source cluster")
require.Equal(t, sourceWS.Spec.Cluster, clusterInReviewObject.Load(), "expected that the object passed to the webhook has correct kcp.io/cluster annotation set")

t.Logf("Check that the in-workspace webhook was NOT called")
require.Zero(t, testWebhooks[targetPath].Calls(), "in-workspace webhook should not have been called")
}
30 changes: 14 additions & 16 deletions test/e2e/conformance/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package conformance

import (
"context"
"encoding/json"
"path/filepath"
"sync/atomic"
"testing"
Expand All @@ -30,7 +29,6 @@ import (
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
Expand Down Expand Up @@ -75,12 +73,8 @@ func TestMutatingWebhookInWorkspace(t *testing.T) {

var clusterInReviewObject atomic.Value
testWebhook := webhookserver.AdmissionWebhookServer{
ResponseFn: func(review *v1.AdmissionReview) (*v1.AdmissionResponse, error) {
var u unstructured.Unstructured
if err := json.Unmarshal(review.Request.Object.Raw, &u.Object); err != nil {
return nil, err
}
clusterInReviewObject.Store(logicalcluster.From(&u).String())
ResponseFn: func(obj runtime.Object, review *v1.AdmissionReview) (*v1.AdmissionResponse, error) {
clusterInReviewObject.Store(logicalcluster.From(obj.(*v1alpha1.Cowboy)).String())
return &v1.AdmissionResponse{Allowed: true}, nil
},
ObjectGVK: schema.GroupVersionKind{
Expand Down Expand Up @@ -197,8 +191,10 @@ func TestValidatingWebhookInWorkspace(t *testing.T) {
codecs := serializer.NewCodecFactory(scheme)
deserializer := codecs.UniversalDeserializer()

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

orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture.
ws1, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
ws2, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath)
workspaces := []logicalcluster.Path{ws1, ws2}
ws1Path, ws1 := kcptesting.NewWorkspaceFixture(t, server, orgPath)
ws2Path, ws2 := kcptesting.NewWorkspaceFixture(t, server, orgPath)
paths := []logicalcluster.Path{ws1Path, ws2Path}
workspaces := []*tenancyv1alpha1.Workspace{ws1, ws2}

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

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

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

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

// Avoid race condition here by making sure that CRD is served after installing the types into logical clusters
t.Logf("Creating cowboy resource in second logical cluster")
_, err = cowbyClusterClient.Cluster(workspaces[1]).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
_, err = cowbyClusterClient.Cluster(paths[1]).WildwestV1alpha1().Cowboys("default").Create(ctx, &cowboy, metav1.CreateOptions{})
require.NoError(t, err, "failed to create cowboy resource in second logical cluster")
require.Equal(t, 1, testWebhook.Calls(), "expected that the webhook is not called for logical cluster where webhook is not installed")
}
4 changes: 2 additions & 2 deletions test/e2e/fixtures/webhook/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
)

type AdmissionWebhookServer struct {
ResponseFn func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error)
ResponseFn func(obj runtime.Object, review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error)
ObjectGVK schema.GroupVersionKind
Deserializer runtime.Decoder

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