addressing feedback

pkg/genericcontrolplane/options/options.go

@@ -114,9 +114,6 @@ func NewServerRunOptions() *ServerRunOptions {
 	// disable the watch cache
 	s.Etcd.EnableWatchCache = false
 
-	// TODO: turn off the admission webhooks for now
-	s.Admission.DefaultOffPlugins.Insert(mutating.PluginName)
-
 	// Overwrite the default for storage data format.
 	s.Etcd.DefaultStorageMediaType = "application/vnd.kubernetes.protobuf"
 

staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go

@@ -21,6 +21,8 @@ import (
 	"sort"
 	"sync/atomic"
 
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
+
 	"k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -120,7 +122,7 @@ func mergeMutatingWebhookConfigurations(configurations []*v1.MutatingWebhookConf
 			n := c.Webhooks[i].Name
 			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
 			names[n]++
-			accessors = append(accessors, webhook.NewMutatingWebhookAccessor(uid, c.Name, &c.Webhooks[i]))
+			accessors = append(accessors, webhook.NewMutatingWebhookAccessor(uid, c.Name, logicalcluster.From(c), &c.Webhooks[i]))
 		}
 	}
 	return accessors

staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go

@@ -21,7 +21,9 @@ import (
 	"sort"
 	"sync/atomic"
 
-	v1 "k8s.io/api/admissionregistration/v1"
+	logicalcluster "github.com/kcp-dev/apimachinery/pkg/logicalcluster"
+
+	"k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
@@ -118,7 +120,7 @@ func mergeValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhook
 			n := c.Webhooks[i].Name
 			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
 			names[n]++
-			accessors = append(accessors, webhook.NewValidatingWebhookAccessor(uid, c.Name, c.ObjectMeta.ClusterName, &c.Webhooks[i]))
+			accessors = append(accessors, webhook.NewValidatingWebhookAccessor(uid, c.Name, logicalcluster.From(c), &c.Webhooks[i]))
 		}
 	}
 	return accessors

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go

@@ -19,6 +19,8 @@ package webhook
 import (
 	"sync"
 
+	logicalcluster "github.com/kcp-dev/apimachinery/pkg/logicalcluster"
+
 	v1 "k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -31,7 +33,7 @@ type WebhookAccessor interface {
 	// GetUID gets a string that uniquely identifies the webhook.
 	GetUID() string
 
-	GetCluster() string
+	GetLogicalCluster() logicalcluster.LogicalCluster
 
 	// GetConfigurationName gets the name of the webhook configuration that owns this webhook.
 	GetConfigurationName() string
@@ -73,14 +75,15 @@ type WebhookAccessor interface {
 }
 
 // NewMutatingWebhookAccessor creates an accessor for a MutatingWebhook.
-func NewMutatingWebhookAccessor(uid, configurationName string, h *v1.MutatingWebhook) WebhookAccessor {
-	return &mutatingWebhookAccessor{uid: uid, configurationName: configurationName, MutatingWebhook: h}
+func NewMutatingWebhookAccessor(uid, configurationName string, cluster logicalcluster.LogicalCluster, h *v1.MutatingWebhook) WebhookAccessor {
+	return &mutatingWebhookAccessor{uid: uid, configurationName: configurationName, cluster: cluster, MutatingWebhook: h}
 }
 
 type mutatingWebhookAccessor struct {
 	*v1.MutatingWebhook
 	uid               string
 	configurationName string
+	cluster           logicalcluster.LogicalCluster
 
 	initObjectSelector sync.Once
 	objectSelector     labels.Selector
@@ -95,8 +98,8 @@ type mutatingWebhookAccessor struct {
 	clientErr  error
 }
 
-func (m *mutatingWebhookAccessor) GetCluster() string {
-	return ""
+func (m *mutatingWebhookAccessor) GetLogicalCluster() logicalcluster.LogicalCluster {
+	return m.cluster
 }
 
 func (m *mutatingWebhookAccessor) GetUID() string {
@@ -177,15 +180,15 @@ func (m *mutatingWebhookAccessor) GetValidatingWebhook() (*v1.ValidatingWebhook,
 }
 
 // NewValidatingWebhookAccessor creates an accessor for a ValidatingWebhook.
-func NewValidatingWebhookAccessor(uid, configurationName, cluster string, h *v1.ValidatingWebhook) WebhookAccessor {
+func NewValidatingWebhookAccessor(uid, configurationName string, cluster logicalcluster.LogicalCluster, h *v1.ValidatingWebhook) WebhookAccessor {
 	return &validatingWebhookAccessor{uid: uid, configurationName: configurationName, cluster: cluster, ValidatingWebhook: h}
 }
 
 type validatingWebhookAccessor struct {
 	*v1.ValidatingWebhook
 	uid               string
 	configurationName string
-	cluster           string
+	cluster           logicalcluster.LogicalCluster
 
 	initObjectSelector sync.Once
 	objectSelector     labels.Selector
@@ -200,7 +203,7 @@ type validatingWebhookAccessor struct {
 	clientErr  error
 }
 
-func (v *validatingWebhookAccessor) GetCluster() string {
+func (v *validatingWebhookAccessor) GetLogicalCluster() logicalcluster.LogicalCluster {
 	return v.cluster
 
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors_test.go

@@ -22,7 +22,8 @@ import (
 	"testing"
 
 	fuzz "github.com/google/gofuzz"
-	"k8s.io/api/admissionregistration/v1"
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
+	v1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/util/diff"
 )
 
@@ -37,7 +38,7 @@ func TestMutatingWebhookAccessor(t *testing.T) {
 			orig.ReinvocationPolicy = nil
 
 			uid := fmt.Sprintf("test.configuration.admission/%s/0", orig.Name)
-			accessor := NewMutatingWebhookAccessor(uid, "test.configuration.admission", orig)
+			accessor := NewMutatingWebhookAccessor(uid, "test.configuration.admission", logicalcluster.New(""), orig)
 			if uid != accessor.GetUID() {
 				t.Errorf("expected GetUID to return %s, but got %s", accessor.GetUID(), uid)
 			}
@@ -77,7 +78,7 @@ func TestValidatingWebhookAccessor(t *testing.T) {
 			orig := &v1.ValidatingWebhook{}
 			f.Fuzz(orig)
 			uid := fmt.Sprintf("test.configuration.admission/%s/0", orig.Name)
-			accessor := NewValidatingWebhookAccessor(uid, "test.configuration.admission", orig)
+			accessor := NewValidatingWebhookAccessor(uid, "test.configuration.admission", logicalcluster.New("testing"), orig)
 			if uid != accessor.GetUID() {
 				t.Errorf("expected GetUID to return %s, but got %s", accessor.GetUID(), uid)
 			}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go

@@ -21,9 +21,10 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
-	v1 "k8s.io/api/admissionregistration/v1"
+	"k8s.io/api/admissionregistration/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
@@ -140,8 +141,8 @@ func (a *Webhook) ValidateInitialization() error {
 
 // ShouldCallHook returns invocation details if the webhook should be called, nil if the webhook should not be called,
 // or an error if an error was encountered during evaluation.
-func (a *Webhook) ShouldCallHook(h webhook.WebhookAccessor, attr admission.Attributes, o admission.ObjectInterfaces, clusterName string) (*WebhookInvocation, *apierrors.StatusError) {
-	if h.GetCluster() != clusterName {
+func (a *Webhook) ShouldCallHook(h webhook.WebhookAccessor, attr admission.Attributes, o admission.ObjectInterfaces, cluster logicalcluster.LogicalCluster) (*WebhookInvocation, *apierrors.StatusError) {
+	if h.GetLogicalCluster() != cluster {
 		return nil, nil
 	}
 	matches, matchNsErr := a.namespaceMatcher.MatchNamespaceSelector(h, attr)

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go

@@ -17,11 +17,13 @@ limitations under the License.
 package generic
 
 import (
+	"context"
 	"fmt"
 	"strings"
 	"testing"
 
-	"k8s.io/api/admissionregistration/v1"
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
+	v1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -69,6 +71,7 @@ func TestShouldCallHook(t *testing.T) {
 
 		webhook *v1.ValidatingWebhook
 		attrs   admission.Attributes
+		cluster logicalcluster.LogicalCluster
 
 		expectCall            bool
 		expectErr             string
@@ -296,7 +299,7 @@ func TestShouldCallHook(t *testing.T) {
 
 	for i, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
-			invocation, err := a.ShouldCallHook(webhook.NewValidatingWebhookAccessor(fmt.Sprintf("webhook-%d", i), fmt.Sprintf("webhook-cfg-%d", i), testcase.webhook), testcase.attrs, interfaces)
+			invocation, err := a.ShouldCallHook(webhook.NewValidatingWebhookAccessor(fmt.Sprintf("webhook-%d", i), fmt.Sprintf("webhook-cfg-%d", i), testcase.cluster, testcase.webhook), testcase.attrs, interfaces, testcase.cluster)
 			if err != nil {
 				if len(testcase.expectErr) == 0 {
 					t.Fatal(err)
@@ -347,6 +350,12 @@ func (f fakeNamespaceLister) Get(name string) (*corev1.Namespace, error) {
 	}
 	return nil, errors.NewNotFound(corev1.Resource("namespaces"), name)
 }
+func (f fakeNamespaceLister) GetWithContext(ctx context.Context, name string) (*corev1.Namespace, error) {
+	return f.Get(name)
+}
+func (f fakeNamespaceLister) ListWithContext(ctx context.Context, selector labels.Selector) ([]*corev1.Namespace, error) {
+	return f.List(selector)
+}
 
 func BenchmarkShouldCallHookWithComplexSelector(b *testing.B) {
 	allScopes := v1.AllScopes
@@ -407,13 +416,13 @@ func BenchmarkShouldCallHookWithComplexSelector(b *testing.B) {
 		},
 	}
 
-	wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb)
+	wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", logicalcluster.New("cluster"), wb)
 	attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil)
 	interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper}
 	a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}}
 
 	for i := 0; i < b.N; i++ {
-		a.ShouldCallHook(wbAccessor, attrs, interfaces)
+		a.ShouldCallHook(wbAccessor, attrs, interfaces, logicalcluster.New("cluster"))
 	}
 }
 
@@ -475,13 +484,13 @@ func BenchmarkShouldCallHookWithComplexRule(b *testing.B) {
 		wb.Rules = append(wb.Rules, rule)
 	}
 
-	wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb)
+	wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", logicalcluster.New("cluster"), wb)
 	attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil)
 	interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper}
 	a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}}
 
 	for i := 0; i < b.N; i++ {
-		a.ShouldCallHook(wbAccessor, attrs, interfaces)
+		a.ShouldCallHook(wbAccessor, attrs, interfaces, logicalcluster.New("cluster"))
 	}
 }
 
@@ -548,12 +557,12 @@ func BenchmarkShouldCallHookWithComplexSelectorAndRule(b *testing.B) {
 		wb.Rules = append(wb.Rules, rule)
 	}
 
-	wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", wb)
+	wbAccessor := webhook.NewValidatingWebhookAccessor("webhook", "webhook-cfg", logicalcluster.New("cluster"), wb)
 	attrs := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{"autoscaling", "v1", "Scale"}, "ns", "name", schema.GroupVersionResource{"apps", "v1", "deployments"}, "scale", admission.Create, &metav1.CreateOptions{}, false, nil)
 	interfaces := &admission.RuntimeObjectInterfaces{EquivalentResourceMapper: mapper}
 	a := &Webhook{namespaceMatcher: &namespace.Matcher{NamespaceLister: namespaceLister}, objectMatcher: &object.Matcher{}}
 
 	for i := 0; i < b.N; i++ {
-		a.ShouldCallHook(wbAccessor, attrs, interfaces)
+		a.ShouldCallHook(wbAccessor, attrs, interfaces, logicalcluster.New("cluster"))
 	}
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher_test.go

@@ -17,9 +17,11 @@ limitations under the License.
 package namespace
 
 import (
+	"context"
 	"reflect"
 	"testing"
 
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
 	registrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -44,6 +46,12 @@ func (f fakeNamespaceLister) Get(name string) (*corev1.Namespace, error) {
 	}
 	return nil, errors.NewNotFound(corev1.Resource("namespaces"), name)
 }
+func (f fakeNamespaceLister) GetWithContext(ctx context.Context, name string) (*corev1.Namespace, error) {
+	return f.Get(name)
+}
+func (f fakeNamespaceLister) ListWithContext(ctx context.Context, selector labels.Selector) ([]*corev1.Namespace, error) {
+	return f.List(selector)
+}
 
 func TestGetNamespaceLabels(t *testing.T) {
 	namespace1Labels := map[string]string{
@@ -120,7 +128,7 @@ func TestNotExemptClusterScopedResource(t *testing.T) {
 	}
 	attr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "mock-name", schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, "", admission.Create, &metav1.CreateOptions{}, false, nil)
 	matcher := Matcher{}
-	matches, err := matcher.MatchNamespaceSelector(webhook.NewValidatingWebhookAccessor("mock-hook", "mock-cfg", hook), attr)
+	matches, err := matcher.MatchNamespaceSelector(webhook.NewValidatingWebhookAccessor("mock-hook", "mock-cfg", logicalcluster.New("mock-cluster"), hook), attr)
 	if err != nil {
 		t.Fatal(err)
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher_test.go

@@ -19,7 +19,8 @@ package object
 import (
 	"testing"
 
-	"k8s.io/api/admissionregistration/v1"
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
+	v1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -115,7 +116,7 @@ func TestObjectSelector(t *testing.T) {
 			}}}
 
 		t.Run(testcase.name, func(t *testing.T) {
-			match, err := matcher.MatchObjectSelector(webhook.NewValidatingWebhookAccessor("mock-hook", "mock-cfg", hook), testcase.attrs)
+			match, err := matcher.MatchObjectSelector(webhook.NewValidatingWebhookAccessor("mock-hook", "mock-cfg", logicalcluster.New("mock-cluster"), hook), testcase.attrs)
 			if err != nil {
 				t.Error(err)
 			}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
 
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
@@ -486,14 +487,14 @@ func TestCreateAdmissionObjects(t *testing.T) {
 		{
 			name: "no supported versions",
 			invocation: &generic.WebhookInvocation{
-				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", &admissionregistrationv1.MutatingWebhook{}),
+				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", logicalcluster.New("cluster"), &admissionregistrationv1.MutatingWebhook{}),
 			},
 			expectErr: "webhook does not accept known AdmissionReview versions",
 		},
 		{
 			name: "no known supported versions",
 			invocation: &generic.WebhookInvocation{
-				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", &admissionregistrationv1.MutatingWebhook{
+				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", logicalcluster.New("cluster"), &admissionregistrationv1.MutatingWebhook{
 					AdmissionReviewVersions: []string{"vX"},
 				}),
 			},
@@ -510,7 +511,7 @@ func TestCreateAdmissionObjects(t *testing.T) {
 				Resource:    schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"},
 				Subresource: "",
 				Kind:        schema.GroupVersionKind{Group: "extensions", Version: "v1beta1", Kind: "Deployment"},
-				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", &admissionregistrationv1.MutatingWebhook{
+				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", logicalcluster.New("cluster"), &admissionregistrationv1.MutatingWebhook{
 					AdmissionReviewVersions: []string{"v1", "v1beta1"},
 				}),
 			},
@@ -553,7 +554,7 @@ func TestCreateAdmissionObjects(t *testing.T) {
 				Resource:    schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"},
 				Subresource: "",
 				Kind:        schema.GroupVersionKind{Group: "extensions", Version: "v1beta1", Kind: "Deployment"},
-				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", &admissionregistrationv1.MutatingWebhook{
+				Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", "mycfg", logicalcluster.New("cluster"), &admissionregistrationv1.MutatingWebhook{
 					AdmissionReviewVersions: []string{"v1beta1", "v1"},
 				}),
 			},

staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/kcp-dev/apimachinery/pkg/logicalcluster"
 	clientv3 "go.etcd.io/etcd/client/v3"
 
 	apitesting "k8s.io/apimachinery/pkg/api/apitesting"
@@ -241,7 +242,7 @@ func TestWatchContextCancel(t *testing.T) {
 	cancel()
 	// When we watch with a canceled context, we should detect that it's context canceled.
 	// We won't take it as error and also close the watcher.
-	w, err := store.watcher.Watch(canceledCtx, "/abc", 0, false, "", false, storage.Everything)
+	w, err := store.watcher.Watch(canceledCtx, "/abc", 0, false, logicalcluster.New(""), false, storage.Everything)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -259,7 +260,7 @@ func TestWatchContextCancel(t *testing.T) {
 func TestWatchErrResultNotBlockAfterCancel(t *testing.T) {
 	origCtx, store, _ := testSetup(t)
 	ctx, cancel := context.WithCancel(origCtx)
-	w := store.watcher.createWatchChan(ctx, "/abc", 0, false, "", false, storage.Everything)
+	w := store.watcher.createWatchChan(ctx, "/abc", 0, false, logicalcluster.New(""), false, storage.Everything)
 	// make resutlChan and errChan blocking to ensure ordering.
 	w.resultChan = make(chan watch.Event)
 	w.errChan = make(chan error)