diff --git a/pkg/admission/mutatingwebhook/plugin.go b/pkg/admission/mutatingwebhook/plugin.go index e1fcb3321154..75f4c60e6ba4 100644 --- a/pkg/admission/mutatingwebhook/plugin.go +++ b/pkg/admission/mutatingwebhook/plugin.go @@ -28,6 +28,7 @@ import ( kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes" "github.com/kcp-dev/logicalcluster/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" @@ -38,6 +39,7 @@ import ( kcpinitializers "github.com/kcp-dev/kcp/pkg/admission/initializers" apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + "github.com/kcp-dev/kcp/sdk/apis/core" kcpinformers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions" ) @@ -122,6 +124,21 @@ func (p *Plugin) Admit(ctx context.Context, attr admission.Attributes, o admissi return fmt.Errorf("error validating MutatingWebhook initialization: %w", err) } + // Add cluster annotation on create + if attr.GetOperation() == admission.Create { + u, ok := attr.GetObject().(metav1.Object) + if !ok { + return fmt.Errorf("unexpected type %T", attr.GetObject()) + } + + annotations := u.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[core.LogicalClusterPathAnnotationKey] = clusterName.String() + u.SetAnnotations(annotations) + } + return plugin.Admit(ctx, attr, o) } diff --git a/pkg/admission/validatingwebhook/plugin.go b/pkg/admission/validatingwebhook/plugin.go index 503ccb4dc82b..fc8e855ff4d7 100644 --- a/pkg/admission/validatingwebhook/plugin.go +++ b/pkg/admission/validatingwebhook/plugin.go @@ -28,6 +28,7 @@ import ( kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes" "github.com/kcp-dev/logicalcluster/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" @@ -38,6 +39,7 @@ import ( kcpinitializers "github.com/kcp-dev/kcp/pkg/admission/initializers" apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + "github.com/kcp-dev/kcp/sdk/apis/core" kcpinformers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions" ) @@ -122,6 +124,21 @@ func (p *Plugin) Validate(ctx context.Context, attr admission.Attributes, o admi return fmt.Errorf("error validating ValidatingAdmissionWebhook initialization: %w", err) } + // Add cluster annotation on create + if attr.GetOperation() == admission.Create { + u, ok := attr.GetObject().(metav1.Object) + if !ok { + return fmt.Errorf("unexpected type %T", attr.GetObject()) + } + + annotations := u.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[core.LogicalClusterPathAnnotationKey] = clusterName.String() + u.SetAnnotations(annotations) + } + return plugin.Validate(ctx, attr, o) } diff --git a/pkg/admission/webhook/OWNERS b/pkg/admission/webhook/OWNERS deleted file mode 100644 index 453288f74b98..000000000000 --- a/pkg/admission/webhook/OWNERS +++ /dev/null @@ -1,3 +0,0 @@ -reviewers: -- shawn-hurley -- stevekuznetsov diff --git a/pkg/admission/webhook/generic_webhook.go b/pkg/admission/webhook/generic_webhook.go deleted file mode 100644 index 9d79f6f7e209..000000000000 --- a/pkg/admission/webhook/generic_webhook.go +++ /dev/null @@ -1,187 +0,0 @@ -/* -Copyright 2022 The KCP Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package webhook - -import ( - "context" - "fmt" - "sync" - - "github.com/kcp-dev/logicalcluster/v3" - - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/admission/plugin/webhook" - "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" - "k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules" - genericapirequest "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/client-go/tools/cache" - "k8s.io/klog/v2" - - "github.com/kcp-dev/kcp/pkg/admission/initializers" - "github.com/kcp-dev/kcp/pkg/indexers" - apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" - kcpinformers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions" -) - -type ClusterAwareSource interface { - Webhooks(cluster logicalcluster.Name) []webhook.WebhookAccessor - HasSynced() bool -} - -type clusterAwareSource struct { - factory func(cluster logicalcluster.Name) generic.Source - hasSynced func() bool - - lock sync.RWMutex - sources map[logicalcluster.Name]generic.Source -} - -func (c *clusterAwareSource) Webhooks(cluster logicalcluster.Name) []webhook.WebhookAccessor { - var source generic.Source - var found bool - c.lock.RLock() - source, found = c.sources[cluster] - c.lock.RUnlock() - if found { - return source.Webhooks() - } - - c.lock.Lock() - defer c.lock.Unlock() - source, found = c.sources[cluster] - if found { - return source.Webhooks() - } - - source = c.factory(cluster) - c.sources[cluster] = source - return source.Webhooks() -} - -func (c *clusterAwareSource) HasSynced() bool { - return c.hasSynced() -} - -var _ initializers.WantsKcpInformers = &WebhookDispatcher{} - -type WebhookDispatcher struct { - *admission.Handler - - dispatcher generic.Dispatcher - hookSource ClusterAwareSource - - getAPIBindings func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) - - informersHaveSynced func() bool -} - -func NewWebhookDispatcher() *WebhookDispatcher { - d := &WebhookDispatcher{ - Handler: admission.NewHandler(admission.Connect, admission.Create, admission.Delete, admission.Update), - } - - return d -} - -func (p *WebhookDispatcher) HasSynced() bool { - return p.hookSource.HasSynced() && p.informersHaveSynced() -} - -func (p *WebhookDispatcher) SetDispatcher(dispatch generic.Dispatcher) { - p.dispatcher = dispatch -} - -func (p *WebhookDispatcher) Dispatch(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error { - // If the object is a Webhook configuration, do not call webhooks - // This is because we need some way to recover if a webhook is preventing a cluster resources from being updated - if rules.IsExemptAdmissionConfigurationResource(attr) { - return nil - } - lcluster, err := genericapirequest.ClusterNameFrom(ctx) - if err != nil { - return err - } - if !p.WaitForReady() { - return admission.NewForbidden(attr, fmt.Errorf("not yet ready to handle request")) - } - - var whAccessor []webhook.WebhookAccessor - - // Determine the type of request, is it api binding or not. - if workspace, err := p.getAPIExportCluster(attr, lcluster); err != nil { - return err - } else if !workspace.Empty() { - whAccessor = p.hookSource.Webhooks(workspace) - attr.SetCluster(workspace) - klog.FromContext(ctx).V(7).WithValues("cluster", workspace).Info("restricting call to api registration hooks in cluster") - } else { - whAccessor = p.hookSource.Webhooks(lcluster) - attr.SetCluster(lcluster) - klog.FromContext(ctx).V(7).WithValues("cluster", lcluster).Info("restricting call to hooks in cluster") - } - - return p.dispatcher.Dispatch(ctx, attr, o, whAccessor) -} - -func (p *WebhookDispatcher) getAPIExportCluster(attr admission.Attributes, clusterName logicalcluster.Name) (logicalcluster.Name, error) { - objs, err := p.getAPIBindings(clusterName) - if err != nil { - return "", err - } - for _, apiBinding := range objs { - for _, br := range apiBinding.Status.BoundResources { - if br.Group == attr.GetResource().Group && br.Resource == attr.GetResource().Resource { - return logicalcluster.Name(apiBinding.Status.APIExportClusterName), nil - } - } - } - return "", nil -} - -func (p *WebhookDispatcher) SetHookSource(factory func(cluster logicalcluster.Name) generic.Source, hasSynced func() bool) { - p.hookSource = &clusterAwareSource{ - hasSynced: hasSynced, - factory: factory, - - lock: sync.RWMutex{}, - sources: map[logicalcluster.Name]generic.Source{}, - } -} - -// SetKcpInformers implements the WantsExternalKcpInformerFactory interface. -func (p *WebhookDispatcher) SetKcpInformers(local, global kcpinformers.SharedInformerFactory) { - p.getAPIBindings = func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) { - return local.Apis().V1alpha1().APIBindings().Lister().Cluster(clusterName).List(labels.Everything()) - } - - synced := func() bool { - return local.Apis().V1alpha1().APIBindings().Informer().HasSynced() && - local.Apis().V1alpha1().APIExports().Informer().HasSynced() && - global.Apis().V1alpha1().APIExports().Informer().HasSynced() - } - p.SetReadyFunc(synced) - p.informersHaveSynced = synced - - indexers.AddIfNotPresentOrDie(local.Apis().V1alpha1().APIExports().Informer().GetIndexer(), cache.Indexers{ - indexers.ByLogicalClusterPathAndName: indexers.IndexByLogicalClusterPathAndName, - }) - - indexers.AddIfNotPresentOrDie(global.Apis().V1alpha1().APIExports().Informer().GetIndexer(), cache.Indexers{ - indexers.ByLogicalClusterPathAndName: indexers.IndexByLogicalClusterPathAndName, - }) -} diff --git a/pkg/admission/webhook/generic_webhook_test.go b/pkg/admission/webhook/generic_webhook_test.go deleted file mode 100644 index 3021f784b3f9..000000000000 --- a/pkg/admission/webhook/generic_webhook_test.go +++ /dev/null @@ -1,305 +0,0 @@ -/* -Copyright 2022 The KCP Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package webhook - -import ( - "context" - "fmt" - "strings" - "testing" - - "github.com/kcp-dev/logicalcluster/v3" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/admission/plugin/webhook" - "k8s.io/apiserver/pkg/authentication/user" - "k8s.io/apiserver/pkg/endpoints/request" - - apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" - "github.com/kcp-dev/kcp/sdk/apis/core" -) - -func attr(gvk schema.GroupVersionKind, name, resource string, op admission.Operation) admission.Attributes { - obj := unstructured.Unstructured{} - obj.SetGroupVersionKind(gvk) - obj.SetName(name) - return admission.NewAttributesRecord( - &obj, - nil, - obj.GroupVersionKind(), - "", - obj.GetName(), - obj.GroupVersionKind().GroupVersion().WithResource(resource), - "", - op, - &metav1.CreateOptions{}, - false, - &user.DefaultInfo{}, - ) -} - -type validatingDispatcher struct { - hooks map[logicalcluster.Name][]webhook.WebhookAccessor -} - -func (d *validatingDispatcher) Dispatch(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces, hooks []webhook.WebhookAccessor) error { - if len(hooks) != len(d.hooks) { - return fmt.Errorf("invalid number of hooks sent to dispatcher") - } - uidMatches := map[string]*struct{}{} - for _, h := range hooks { - for _, allHooks := range d.hooks { - for _, expectedHook := range allHooks { - if h.GetUID() == expectedHook.GetUID() { - uidMatches[h.GetUID()] = &struct{}{} - } - } - } - } - if len(uidMatches) != len(d.hooks) { - return fmt.Errorf("hooks UID did not match expected") - } - return nil -} - -type fakeHookSource struct { - hooks map[logicalcluster.Name][]webhook.WebhookAccessor - hasSynced bool -} - -func (f fakeHookSource) Webhooks(cluster logicalcluster.Name) []webhook.WebhookAccessor { - return f.hooks[cluster] -} -func (f fakeHookSource) HasSynced() bool { - return f.hasSynced -} - -func TestDispatch(t *testing.T) { - tests := []struct { - name string - attr admission.Attributes - cluster logicalcluster.Name - expectedHooks map[logicalcluster.Name][]webhook.WebhookAccessor - hooksInSource map[logicalcluster.Name][]webhook.WebhookAccessor - hookSourceNotSynced bool - apiBindings []*apisv1alpha1.APIBinding - apiExports []*apisv1alpha1.APIExport - informersHaveSynced func() bool - wantErr bool - }{ - { - name: "call for APIBinding only calls hooks in api registration logical cluster", - attr: attr( - schema.GroupVersionKind{Kind: "Cowboy", Group: "wildwest.dev", Version: "v1"}, - "bound-resource", - "cowboys", - admission.Create, - ), - cluster: "root-org-dest", - expectedHooks: map[logicalcluster.Name][]webhook.WebhookAccessor{ - logicalcluster.Name("root-org-source"): {webhook.NewValidatingWebhookAccessor("1", "api-registration-hook", nil)}, - }, - hooksInSource: map[logicalcluster.Name][]webhook.WebhookAccessor{ - logicalcluster.Name("root-org-source"): {webhook.NewValidatingWebhookAccessor("1", "api-registration-hook", nil)}, - logicalcluster.Name("root-org-dest"): {webhook.NewValidatingWebhookAccessor("2", "secrets", nil)}, - }, - apiBindings: []*apisv1alpha1.APIBinding{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "one", - Annotations: map[string]string{ - logicalcluster.AnnotationKey: "root-org-dest", - }, - }, - Spec: apisv1alpha1.APIBindingSpec{ - Reference: apisv1alpha1.BindingReference{ - Export: &apisv1alpha1.ExportBindingReference{ - Path: "root:org:source", - Name: "someExport", - }, - }, - }, - Status: apisv1alpha1.APIBindingStatus{ - BoundResources: []apisv1alpha1.BoundAPIResource{ - { - Group: "wildwest.dev", - Resource: "cowboys", - }, - }, - APIExportClusterName: "root-org-source", - }, - }, - }, - apiExports: []*apisv1alpha1.APIExport{ - newAPIExport(logicalcluster.NewPath("root:org:source"), "someExport").APIExport, - }, - }, - { - name: "call for resource only calls hooks in logical cluster", - attr: attr( - schema.GroupVersionKind{Kind: "Cowboy", Group: "wildwest.dev", Version: "v1"}, - "bound-resource", - "cowboys", - admission.Create, - ), - cluster: "root-org-dest", - expectedHooks: map[logicalcluster.Name][]webhook.WebhookAccessor{ - logicalcluster.Name("root-org-dest"): {webhook.NewValidatingWebhookAccessor("3", "secrets", nil)}, - }, - hooksInSource: map[logicalcluster.Name][]webhook.WebhookAccessor{ - logicalcluster.Name("root-org-source"): { - webhook.NewValidatingWebhookAccessor("1", "cowboy-hook", nil), - webhook.NewValidatingWebhookAccessor("2", "secrets", nil), - }, - logicalcluster.Name("root-org-dest"): {webhook.NewValidatingWebhookAccessor("3", "secrets", nil)}, - }, - }, - { - name: "API Bindings for other logical cluster call webhooks for dest cluster", - attr: attr( - schema.GroupVersionKind{Kind: "Cowboy", Group: "wildwest.dev", Version: "v1"}, - "bound-resource", - "cowboys", - admission.Create, - ), - cluster: "root-org-dest", - expectedHooks: map[logicalcluster.Name][]webhook.WebhookAccessor{ - logicalcluster.Name("root-org-dest"): {webhook.NewValidatingWebhookAccessor("3", "secrets", nil)}, - }, - hooksInSource: map[logicalcluster.Name][]webhook.WebhookAccessor{ - logicalcluster.Name("root-org-source"): { - webhook.NewValidatingWebhookAccessor("1", "cowboy-hook", nil), - webhook.NewValidatingWebhookAccessor("2", "secrets", nil), - }, - logicalcluster.Name("root-org-dest"): {webhook.NewValidatingWebhookAccessor("3", "secrets", nil)}, - }, - apiBindings: []*apisv1alpha1.APIBinding{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "two", - Annotations: map[string]string{ - logicalcluster.AnnotationKey: "root-org-dest", - }, - }, - Status: apisv1alpha1.APIBindingStatus{ - BoundResources: []apisv1alpha1.BoundAPIResource{ - { - Group: "wildwest.dev", - Resource: "Horses", - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "one", - Annotations: map[string]string{ - logicalcluster.AnnotationKey: "root-org-dest-2", - }, - }, - Status: apisv1alpha1.APIBindingStatus{ - BoundResources: []apisv1alpha1.BoundAPIResource{ - { - Group: "wildwest.dev", - Resource: "Cowboys", - }, - }, - }, - }, - }, - }, - { - name: "API Bindings lister not synced", - attr: attr( - schema.GroupVersionKind{Kind: "Cowboy", Group: "wildwest.dev", Version: "v1"}, - "bound-resource", - "cowboys", - admission.Create, - ), - cluster: "root-org-dest", - informersHaveSynced: func() bool { - return false - }, - wantErr: true, - }, - { - name: "hook source not synced", - attr: attr( - schema.GroupVersionKind{Kind: "Cowboy", Group: "wildwest.dev", Version: "v1"}, - "bound-resource", - "cowboys", - admission.Create, - ), - cluster: "root-org-dest", - hookSourceNotSynced: true, - wantErr: true, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - ctx, cancelFn := context.WithCancel(context.Background()) - t.Cleanup(cancelFn) - - o := &WebhookDispatcher{ - Handler: admission.NewHandler(admission.Connect, admission.Create, admission.Delete, admission.Update), - dispatcher: &validatingDispatcher{hooks: tc.expectedHooks}, - hookSource: &fakeHookSource{hooks: tc.hooksInSource, hasSynced: !tc.hookSourceNotSynced}, - informersHaveSynced: tc.informersHaveSynced, - getAPIBindings: func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) { - return tc.apiBindings, nil - }, - } - - if tc.informersHaveSynced == nil { - o.informersHaveSynced = func() bool { return true } - } - - // Want to make sure that ready would fail based on these. - o.SetReadyFunc(func() bool { - return o.informersHaveSynced() && o.hookSource.HasSynced() - }) - - ctx = request.WithCluster(ctx, request.Cluster{Name: tc.cluster}) - if err := o.Dispatch(ctx, tc.attr, nil); (err != nil) != tc.wantErr { - t.Fatalf("Dispatch() error = %v, wantErr %v", err, tc.wantErr) - } - }) - } -} - -type apiExportBuilder struct { - APIExport *apisv1alpha1.APIExport -} - -func newAPIExport(path logicalcluster.Path, name string) apiExportBuilder { - clusterName := strings.ReplaceAll(path.String(), ":", "-") - return apiExportBuilder{ - APIExport: &apisv1alpha1.APIExport{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Annotations: map[string]string{ - logicalcluster.AnnotationKey: clusterName, - core.LogicalClusterPathAnnotationKey: path.String(), - }, - }, - }, - } -} diff --git a/test/e2e/apibinding/apibinding_webhook_test.go b/test/e2e/apibinding/apibinding_webhook_test.go index 1e19d1ad7d61..6de987aea734 100644 --- a/test/e2e/apibinding/apibinding_webhook_test.go +++ b/test/e2e/apibinding/apibinding_webhook_test.go @@ -30,7 +30,7 @@ import ( "github.com/kcp-dev/logicalcluster/v3" "github.com/stretchr/testify/require" - v1 "k8s.io/api/admission/v1" + admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -118,7 +118,7 @@ func TestAPIBindingMutatingWebhook(t *testing.T) { scheme := runtime.NewScheme() err = admissionregistrationv1.AddToScheme(scheme) require.NoError(t, err, "failed to add admission registration v1 scheme") - err = v1.AddToScheme(scheme) + err = admissionv1.AddToScheme(scheme) require.NoError(t, err, "failed to add admission v1 scheme") err = v1alpha1.AddToScheme(scheme) require.NoError(t, err, "failed to add cowboy v1alpha1 to scheme") @@ -131,8 +131,8 @@ func TestAPIBindingMutatingWebhook(t *testing.T) { testWebhooks := map[logicalcluster.Path]*webhookserver.AdmissionWebhookServer{} for _, cluster := range []logicalcluster.Path{sourcePath, targetPath} { testWebhooks[cluster] = &webhookserver.AdmissionWebhookServer{ - Response: v1.AdmissionResponse{ - Allowed: true, + ResponseFn: func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) { + return &admissionv1.AdmissionResponse{Allowed: true}, nil }, ObjectGVK: schema.GroupVersionKind{ Group: "wildwest.dev", @@ -265,7 +265,7 @@ func TestAPIBindingValidatingWebhook(t *testing.T) { scheme := runtime.NewScheme() err = admissionregistrationv1.AddToScheme(scheme) require.NoError(t, err, "failed to add admission registration v1 scheme") - err = v1.AddToScheme(scheme) + err = admissionv1.AddToScheme(scheme) require.NoError(t, err, "failed to add admission v1 scheme") err = v1alpha1.AddToScheme(scheme) require.NoError(t, err, "failed to add cowboy v1alpha1 to scheme") @@ -278,8 +278,8 @@ func TestAPIBindingValidatingWebhook(t *testing.T) { testWebhooks := map[logicalcluster.Path]*webhookserver.AdmissionWebhookServer{} for _, cluster := range []logicalcluster.Path{sourcePath, targetPath} { testWebhooks[cluster] = &webhookserver.AdmissionWebhookServer{ - Response: v1.AdmissionResponse{ - Allowed: true, + ResponseFn: func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) { + return &admissionv1.AdmissionResponse{Allowed: true}, nil }, ObjectGVK: schema.GroupVersionKind{ Group: "wildwest.dev", diff --git a/test/e2e/conformance/webhook_test.go b/test/e2e/conformance/webhook_test.go index 6a41bf485d26..825ab95e7a47 100644 --- a/test/e2e/conformance/webhook_test.go +++ b/test/e2e/conformance/webhook_test.go @@ -18,7 +18,9 @@ package conformance import ( "context" + "encoding/json" "path/filepath" + "sync/atomic" "testing" "time" @@ -31,11 +33,13 @@ 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" "k8s.io/apimachinery/pkg/util/wait" + tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" webhookserver "github.com/kcp-dev/kcp/test/e2e/fixtures/webhook" "github.com/kcp-dev/kcp/test/e2e/fixtures/wildwest" "github.com/kcp-dev/kcp/test/e2e/fixtures/wildwest/apis/wildwest/v1alpha1" @@ -66,9 +70,15 @@ func TestMutatingWebhookInWorkspace(t *testing.T) { codecs := serializer.NewCodecFactory(scheme) deserializer := codecs.UniversalDeserializer() + var clusterInReviewObject atomic.Value testWebhook := webhookserver.AdmissionWebhookServer{ - Response: v1.AdmissionResponse{ - Allowed: true, + 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()) + return &v1.AdmissionResponse{Allowed: true}, nil }, ObjectGVK: schema.GroupVersionKind{ Group: "wildwest.dev", @@ -84,9 +94,10 @@ func TestMutatingWebhookInWorkspace(t *testing.T) { testWebhook.StartTLS(t, filepath.Join(dirPath, "apiserver.crt"), filepath.Join(dirPath, "apiserver.key"), port) orgPath, _ := framework.NewOrganizationFixture(t, server) - ws1Path, _ := framework.NewWorkspaceFixture(t, server, orgPath) - ws2Path, _ := framework.NewWorkspaceFixture(t, server, orgPath) - workspaces := []logicalcluster.Path{ws1Path, ws2Path} + ws1Path, ws1 := framework.NewWorkspaceFixture(t, server, orgPath) + ws2Path, ws2 := framework.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") @@ -96,7 +107,7 @@ func TestMutatingWebhookInWorkspace(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"}) @@ -128,7 +139,7 @@ func TestMutatingWebhookInWorkspace(t *testing.T) { AdmissionReviewVersions: []string{"v1"}, }}, } - _, err = kubeClusterClient.Cluster(workspaces[0]).AdmissionregistrationV1().MutatingWebhookConfigurations().Create(ctx, webhook, metav1.CreateOptions{}) + _, err = kubeClusterClient.Cluster(paths[0]).AdmissionregistrationV1().MutatingWebhookConfigurations().Create(ctx, webhook, metav1.CreateOptions{}) require.NoError(t, err, "failed to add mutating webhook configurations") cowboy := v1alpha1.Cowboy{ @@ -140,17 +151,18 @@ func TestMutatingWebhookInWorkspace(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") require.Eventually(t, func() bool { - _, 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{}) if err != nil && !errors.IsAlreadyExists(err) { return false } @@ -183,8 +195,8 @@ func TestValidatingWebhookInWorkspace(t *testing.T) { deserializer := codecs.UniversalDeserializer() testWebhook := webhookserver.AdmissionWebhookServer{ - Response: v1.AdmissionResponse{ - Allowed: true, + ResponseFn: func(review *v1.AdmissionReview) (*v1.AdmissionResponse, error) { + return &v1.AdmissionResponse{Allowed: true}, nil }, ObjectGVK: schema.GroupVersionKind{ Group: "wildwest.dev", diff --git a/test/e2e/fixtures/webhook/webhook.go b/test/e2e/fixtures/webhook/webhook.go index 880ccec423ba..0e44bbe530af 100644 --- a/test/e2e/fixtures/webhook/webhook.go +++ b/test/e2e/fixtures/webhook/webhook.go @@ -31,7 +31,7 @@ import ( ) type AdmissionWebhookServer struct { - Response admissionv1.AdmissionResponse + ResponseFn func(review *admissionv1.AdmissionReview) (*admissionv1.AdmissionResponse, error) ObjectGVK schema.GroupVersionKind Deserializer runtime.Decoder @@ -137,7 +137,13 @@ func (s *AdmissionWebhookServer) ServeHTTP(resp http.ResponseWriter, req *http.R responseAdmissionReview := &admissionv1.AdmissionReview{ TypeMeta: requestedAdmissionReview.TypeMeta, } - responseAdmissionReview.Response = &s.Response + r, err := s.ResponseFn(requestedAdmissionReview) + if err != nil { + s.t.Logf("%v", err) + http.Error(resp, err.Error(), http.StatusInternalServerError) + return + } + responseAdmissionReview.Response = r responseAdmissionReview.Response.UID = requestedAdmissionReview.Request.UID respBytes, err := json.Marshal(responseAdmissionReview)