Adding e2e tests for new plugins

kcp-dev · Apr 25, 2022 · df499dd · df499dd
1 parent 105943c
commit df499dd
Show file tree

Hide file tree

Showing 8 changed files with 662 additions and 154 deletions.
diff --git a/pkg/admission/mutatingwebhook/plugin.go b/pkg/admission/mutatingwebhook/plugin.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"io"
 
-	"github.com/kcp-dev/kcp/pkg/admission/webhook"
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -31,6 +30,8 @@ import (
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/mutating"
 	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 	"k8s.io/client-go/informers"
+
+	"github.com/kcp-dev/kcp/pkg/admission/webhook"
 )
 
 const (
@@ -101,11 +102,11 @@ func Register(plugins *admission.Plugins) {
 }
 
 func (a *Plugin) Admit(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error {
-	return a.Dispatch(ctx, attr, o)
+	return a.WebhookDispatcher.Dispatch(ctx, attr, o)
 }
 
 // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.
 func (p *Plugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
-	p.WebhookDispatcher.SetExternalKubeInformerFactory(f)
+	p.WebhookDispatcher.SetHookSource(configuration.NewMutatingWebhookConfigurationManager(f))
 	p.Plugin.SetExternalKubeInformerFactory(f)
 }
diff --git a/pkg/admission/plugins.go b/pkg/admission/plugins.go
@@ -22,6 +22,7 @@ import (
 	"k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle"
 	"k8s.io/apiserver/pkg/admission/plugin/resourcequota"
 	mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating"
+	validatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/validating"
 	kubeapiserveroptions "k8s.io/kubernetes/pkg/kubeapiserver/options"
 	certapproval "k8s.io/kubernetes/plugin/pkg/admission/certificates/approval"
 	certsigning "k8s.io/kubernetes/plugin/pkg/admission/certificates/signing"
@@ -110,14 +111,14 @@ var defaultOnPluginsInKcp = sets.NewString(
 // new plugins got added upstream and to react (enable or disable by default). We
 // have a unit test in place to avoid drift.
 var defaultOnKubePluginsInKube = sets.NewString(
-	lifecycle.PluginName,                // NamespaceLifecycle
-	limitranger.PluginName,              // LimitRanger
-	serviceaccount.PluginName,           // ServiceAccount
-	setdefault.PluginName,               // DefaultStorageClass
-	resize.PluginName,                   // PersistentVolumeClaimResize
-	defaulttolerationseconds.PluginName, // DefaultTolerationSeconds
-	//mutatingwebhook.PluginName,              // MutatingAdmissionWebhook
-	//validatingwebhook.PluginName,            // ValidatingAdmissionWebhook
+	lifecycle.PluginName,                    // NamespaceLifecycle
+	limitranger.PluginName,                  // LimitRanger
+	serviceaccount.PluginName,               // ServiceAccount
+	setdefault.PluginName,                   // DefaultStorageClass
+	resize.PluginName,                       // PersistentVolumeClaimResize
+	defaulttolerationseconds.PluginName,     // DefaultTolerationSeconds
+	mutatingwebhook.PluginName,              // MutatingAdmissionWebhook
+	validatingwebhook.PluginName,            // ValidatingAdmissionWebhook
 	resourcequota.PluginName,                // ResourceQuota
 	storageobjectinuseprotection.PluginName, // StorageObjectInUseProtection
 	podpriority.PluginName,                  // PodPriority

diff --git a/pkg/admission/validatingwebhook/plugin.go b/pkg/admission/validatingwebhook/plugin.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"io"
 
-	"github.com/kcp-dev/kcp/pkg/admission/webhook"
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -31,6 +30,8 @@ import (
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/validating"
 	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 	"k8s.io/client-go/informers"
+
+	"github.com/kcp-dev/kcp/pkg/admission/webhook"
 )
 
 const (
@@ -104,6 +105,6 @@ func (a *Plugin) Validate(ctx context.Context, attr admission.Attributes, o admi
 
 // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.
 func (p *Plugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
-	p.WebhookDispatcher.SetExternalKubeInformerFactory(f)
+	p.WebhookDispatcher.SetHookSource(configuration.NewValidatingWebhookConfigurationManager(f))
 	p.Plugin.SetExternalKubeInformerFactory(f)
 }
diff --git a/pkg/admission/webhook/generic_webhook.go b/pkg/admission/webhook/generic_webhook.go
@@ -21,17 +21,17 @@ import (
 	"fmt"
 
 	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
-	kcpinformers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions"
-	apisv1alpha1 "github.com/kcp-dev/kcp/pkg/client/listers/apis/v1alpha1"
 
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/apiserver/pkg/admission/configuration"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/generic"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/rules"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/client-go/informers"
+	"k8s.io/klog/v2"
+
+	kcpinformers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions"
+	apisv1alpha1 "github.com/kcp-dev/kcp/pkg/client/listers/apis/v1alpha1"
 )
 
 type WebhookDispatcher struct {
@@ -70,8 +70,10 @@ func (p *WebhookDispatcher) Dispatch(ctx context.Context, attr admission.Attribu
 	// Determine the type of request, is it api binding or not.
 	if workspace, isAPIBinding := p.getAPIBindingWorkspace(attr, lcluster); isAPIBinding {
 		whAccessor = p.restrictToLogicalCluster(hooks, workspace)
+		klog.V(3).Infof("restricting call to api registration hooks in cluster: %v", workspace)
 	} else {
 		whAccessor = p.restrictToLogicalCluster(hooks, lcluster)
+		klog.V(3).Infof("restricting call to hooks in cluster: %v", lcluster)
 	}
 
 	return p.dispatcher.Dispatch(ctx, attr, o, whAccessor)
@@ -88,7 +90,12 @@ func (p *WebhookDispatcher) getAPIBindingWorkspace(attr admission.Attributes, lc
 		}
 		for _, br := range apiBinding.Status.BoundResources {
 			if br.Group == attr.GetResource().Group && br.Resource == attr.GetResource().Resource {
-				return logicalcluster.New(apiBinding.Status.BoundAPIExport.Workspace.WorkspaceName), true
+				p, hasParent := logicalcluster.From(apiBinding).Parent()
+				if !hasParent {
+					klog.V(3).Infof("unable to find parent for apibinding: cluster:%v resource: %v/%v", apiBinding.GetClusterName(), apiBinding.GetNamespace(), apiBinding.GetName())
+					return logicalcluster.New(""), false
+				}
+				return p.Join(apiBinding.Status.BoundAPIExport.Workspace.WorkspaceName), true
 			}
 		}
 	}
@@ -107,9 +114,8 @@ func (p *WebhookDispatcher) restrictToLogicalCluster(hooks []webhook.WebhookAcce
 	return wh
 }
 
-// SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.
-func (p *WebhookDispatcher) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
-	p.hookSource = configuration.NewValidatingWebhookConfigurationManager(f)
+func (p *WebhookDispatcher) SetHookSource(s generic.Source) {
+	p.hookSource = s
 }
 
 // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.

diff --git a/pkg/admission/webhook/generic_webhook_test.go b/pkg/admission/webhook/generic_webhook_test.go
@@ -22,7 +22,7 @@ import (
 	"testing"
 
 	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
-	"github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
@@ -31,6 +31,8 @@ import (
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/endpoints/request"
+
+	"github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
 )
 
 func attr(gvk schema.GroupVersionKind, name, resource string, op admission.Operation) admission.Attributes {
@@ -127,18 +129,18 @@ func TestDispatch(t *testing.T) {
 				"cowboys",
 				admission.Create,
 			),
-			cluster: "dest-cluster",
+			cluster: "root:org:dest-cluster",
 			expectedHooks: []webhook.WebhookAccessor{
-				webhook.NewValidatingWebhookAccessor("1", "api-registration-hook", logicalcluster.New("source-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("1", "api-registration-hook", logicalcluster.New("root:org:source-cluster"), nil),
 			},
 			hooksInSource: []webhook.WebhookAccessor{
-				webhook.NewValidatingWebhookAccessor("1", "api-registration-hook", logicalcluster.New("source-cluster"), nil),
-				webhook.NewValidatingWebhookAccessor("2", "secrets", logicalcluster.New("dest-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("1", "api-registration-hook", logicalcluster.New("root:org:source-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("2", "secrets", logicalcluster.New("root:org:dest-cluster"), nil),
 			},
 			apiBindings: []*v1alpha1.APIBinding{
 				{
 					ObjectMeta: metav1.ObjectMeta{
-						ClusterName: "dest-cluster",
+						ClusterName: "root:org:dest-cluster",
 					},
 					Status: v1alpha1.APIBindingStatus{
 						BoundResources: []v1alpha1.BoundAPIResource{
@@ -164,14 +166,14 @@ func TestDispatch(t *testing.T) {
 				"cowboys",
 				admission.Create,
 			),
-			cluster: "dest-cluster",
+			cluster: "root:org:dest-cluster",
 			expectedHooks: []webhook.WebhookAccessor{
-				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("dest-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("root:org:dest-cluster"), nil),
 			},
 			hooksInSource: []webhook.WebhookAccessor{
-				webhook.NewValidatingWebhookAccessor("1", "cowboy-hook", logicalcluster.New("source-cluster"), nil),
-				webhook.NewValidatingWebhookAccessor("2", "secrets", logicalcluster.New("source-cluster"), nil),
-				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("dest-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("1", "cowboy-hook", logicalcluster.New("root:org:source-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("2", "secrets", logicalcluster.New("root:org:source-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("root:org:dest-cluster"), nil),
 			},
 		},
 		{
@@ -182,19 +184,19 @@ func TestDispatch(t *testing.T) {
 				"cowboys",
 				admission.Create,
 			),
-			cluster: "dest-cluster",
+			cluster: "root:org:dest-cluster",
 			expectedHooks: []webhook.WebhookAccessor{
-				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("dest-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("root:org:dest-cluster"), nil),
 			},
 			hooksInSource: []webhook.WebhookAccessor{
-				webhook.NewValidatingWebhookAccessor("1", "cowboy-hook", logicalcluster.New("source-cluster"), nil),
-				webhook.NewValidatingWebhookAccessor("2", "secrets", logicalcluster.New("source-cluster"), nil),
-				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("dest-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("1", "cowboy-hook", logicalcluster.New("root:org:source-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("2", "secrets", logicalcluster.New("root:org:source-cluster"), nil),
+				webhook.NewValidatingWebhookAccessor("3", "secrets", logicalcluster.New("root:org:dest-cluster"), nil),
 			},
 			apiBindings: []*v1alpha1.APIBinding{
 				{
 					ObjectMeta: metav1.ObjectMeta{
-						ClusterName: "dest-cluster",
+						ClusterName: "root:org:dest-cluster",
 					},
 					Status: v1alpha1.APIBindingStatus{
 						BoundResources: []v1alpha1.BoundAPIResource{
@@ -212,7 +214,7 @@ func TestDispatch(t *testing.T) {
 				},
 				{
 					ObjectMeta: metav1.ObjectMeta{
-						ClusterName: "dest-cluster-2",
+						ClusterName: "root:org:dest-cluster-2",
 					},
 					Status: v1alpha1.APIBindingStatus{
 						BoundResources: []v1alpha1.BoundAPIResource{
@@ -238,7 +240,7 @@ func TestDispatch(t *testing.T) {
 				"cowboys",
 				admission.Create,
 			),
-			cluster: "dest-cluster",
+			cluster: "root:org:dest-cluster",
 			apiBindingsSynced: func() bool {
 				return false
 			},
@@ -252,7 +254,7 @@ func TestDispatch(t *testing.T) {
 				"cowboys",
 				admission.Create,
 			),
-			cluster:             "dest-cluster",
+			cluster:             "root:org:dest-cluster",
 			hookSourceNotSynced: true,
 			wantErr:             true,
 		},
@@ -278,7 +280,7 @@ func TestDispatch(t *testing.T) {
 			ctx := request.WithCluster(context.Background(), request.Cluster{Name: logicalcluster.New(tc.cluster)})
 
 			if err := o.Dispatch(ctx, tc.attr, nil); (err != nil) != tc.wantErr {
-				t.Fatalf("Validate() error = %v, wantErr %v", err, tc.wantErr)
+				t.Fatalf("Dispatch() error = %v, wantErr %v", err, tc.wantErr)
 			}
 		})
 	}