diff --git a/docs/content/concepts/apis/admission-webhooks.md b/docs/content/concepts/apis/admission-webhooks.md new file mode 100644 index 00000000000..765bc6788b3 --- /dev/null +++ b/docs/content/concepts/apis/admission-webhooks.md @@ -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. diff --git a/test/e2e/apibinding/apibinding_webhook_test.go b/test/e2e/apibinding/apibinding_webhook_test.go index f8bf029807c..a87855420dd 100644 --- a/test/e2e/apibinding/apibinding_webhook_test.go +++ b/test/e2e/apibinding/apibinding_webhook_test.go @@ -22,6 +22,7 @@ import ( "fmt" gohttp "net/http" "path/filepath" + "sync/atomic" "testing" "time" @@ -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) @@ -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{ @@ -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") } @@ -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) @@ -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{ @@ -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") } diff --git a/test/e2e/conformance/webhook_test.go b/test/e2e/conformance/webhook_test.go index 5268b639d2e..a2e921854dd 100644 --- a/test/e2e/conformance/webhook_test.go +++ b/test/e2e/conformance/webhook_test.go @@ -18,7 +18,6 @@ package conformance import ( "context" - "encoding/json" "path/filepath" "sync/atomic" "testing" @@ -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" @@ -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{ @@ -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{ @@ -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") @@ -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"}) @@ -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{ @@ -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") } diff --git a/test/e2e/fixtures/webhook/webhook.go b/test/e2e/fixtures/webhook/webhook.go index 560c4588024..b8fe89423b7 100644 --- a/test/e2e/fixtures/webhook/webhook.go +++ b/test/e2e/fixtures/webhook/webhook.go @@ -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 @@ -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)