Merge pull request #113349 from maxsmythe/refactor-matcher-interface

Refactor matcher interface

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

@@ -22,12 +22,19 @@ import (
 	"k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object"
 	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 	"k8s.io/client-go/rest"
 )
 
 // WebhookAccessor provides a common interface to both mutating and validating webhook types.
 type WebhookAccessor interface {
+	// This accessor provides the methods needed to support matching against webhook
+	// predicates
+	namespace.NamespaceSelectorProvider
+	object.ObjectSelectorProvider
+
 	// GetUID gets a string that uniquely identifies the webhook.
 	GetUID() string
 
@@ -36,10 +43,6 @@ type WebhookAccessor interface {
 
 	// GetRESTClient gets the webhook client
 	GetRESTClient(clientManager *webhookutil.ClientManager) (*rest.RESTClient, error)
-	// GetParsedNamespaceSelector gets the webhook NamespaceSelector field.
-	GetParsedNamespaceSelector() (labels.Selector, error)
-	// GetParsedObjectSelector gets the webhook ObjectSelector field.
-	GetParsedObjectSelector() (labels.Selector, error)
 
 	// GetName gets the webhook Name field. Note that the name is scoped to the webhook
 	// configuration and does not provide a globally unique identity, if a unique identity is

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

@@ -30,9 +30,9 @@ import (
 	genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/config"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/namespace"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/object"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/rules"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules"
 	webhookutil "k8s.io/apiserver/pkg/util/webhook"
 	"k8s.io/client-go/informers"
 	clientset "k8s.io/client-go/kubernetes"

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

@@ -30,8 +30,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/namespace"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook/object"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object"
 )
 
 func TestShouldCallHook(t *testing.T) {

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

@@ -17,4 +17,4 @@ limitations under the License.
 // Package namespace defines the utilities that are used by the webhook
 // plugin to decide if a webhook should be applied to an object based on its
 // namespace.
-package namespace // import "k8s.io/apiserver/pkg/admission/plugin/webhook/namespace"
+package namespace // import "k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace"

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

@@ -26,11 +26,15 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	clientset "k8s.io/client-go/kubernetes"
 	corelisters "k8s.io/client-go/listers/core/v1"
 )
 
+type NamespaceSelectorProvider interface {
+	// GetNamespaceSelector gets the webhook NamespaceSelector field.
+	GetParsedNamespaceSelector() (labels.Selector, error)
+}
+
 // Matcher decides if a request is exempted by the NamespaceSelector of a
 // webhook configuration.
 type Matcher struct {
@@ -87,7 +91,7 @@ func (m *Matcher) GetNamespaceLabels(attr admission.Attributes) (map[string]stri
 
 // MatchNamespaceSelector decideds whether the request matches the
 // namespaceSelctor of the webhook. Only when they match, the webhook is called.
-func (m *Matcher) MatchNamespaceSelector(h webhook.WebhookAccessor, attr admission.Attributes) (bool, *apierrors.StatusError) {
+func (m *Matcher) MatchNamespaceSelector(p NamespaceSelectorProvider, attr admission.Attributes) (bool, *apierrors.StatusError) {
 	namespaceName := attr.GetNamespace()
 	if len(namespaceName) == 0 && attr.GetResource().Resource != "namespaces" {
 		// If the request is about a cluster scoped resource, and it is not a
@@ -96,7 +100,7 @@ func (m *Matcher) MatchNamespaceSelector(h webhook.WebhookAccessor, attr admissi
 		// Also update the comment in types.go
 		return true, nil
 	}
-	selector, err := h.GetParsedNamespaceSelector()
+	selector, err := p.GetParsedNamespaceSelector()
 	if err != nil {
 		return false, apierrors.NewInternalError(err)
 	}

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

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package namespace
+package namespace_test
 
 import (
 	"reflect"
@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace"
 )
 
 type fakeNamespaceLister struct {
@@ -100,7 +101,7 @@ func TestGetNamespaceLabels(t *testing.T) {
 			expectedLabels: namespace1Labels,
 		},
 	}
-	matcher := Matcher{
+	matcher := namespace.Matcher{
 		NamespaceLister: namespaceLister,
 	}
 	for _, tt := range tests {
@@ -119,7 +120,7 @@ func TestNotExemptClusterScopedResource(t *testing.T) {
 		NamespaceSelector: &metav1.LabelSelector{},
 	}
 	attr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "mock-name", schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, "", admission.Create, &metav1.CreateOptions{}, false, nil)
-	matcher := Matcher{}
+	matcher := namespace.Matcher{}
 	matches, err := matcher.MatchNamespaceSelector(webhook.NewValidatingWebhookAccessor("mock-hook", "mock-cfg", hook), attr)
 	if err != nil {
 		t.Fatal(err)

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

@@ -17,4 +17,4 @@ limitations under the License.
 // Package object defines the utilities that are used by the webhook plugin to
 // decide if a webhook should run, as long as either the old object or the new
 // object has labels matching the webhook config's objectSelector.
-package object // import "k8s.io/apiserver/pkg/admission/plugin/webhook/object"
+package object // import "k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object"

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

@@ -22,10 +22,14 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	"k8s.io/klog/v2"
 )
 
+type ObjectSelectorProvider interface {
+	// GetObjectSelector gets the webhook ObjectSelector field.
+	GetParsedObjectSelector() (labels.Selector, error)
+}
+
 // Matcher decides if a request selected by the ObjectSelector.
 type Matcher struct {
 }
@@ -45,8 +49,8 @@ func matchObject(obj runtime.Object, selector labels.Selector) bool {
 
 // MatchObjectSelector decideds whether the request matches the ObjectSelector
 // of the webhook. Only when they match, the webhook is called.
-func (m *Matcher) MatchObjectSelector(h webhook.WebhookAccessor, attr admission.Attributes) (bool, *apierrors.StatusError) {
-	selector, err := h.GetParsedObjectSelector()
+func (m *Matcher) MatchObjectSelector(p ObjectSelectorProvider, attr admission.Attributes) (bool, *apierrors.StatusError) {
+	selector, err := p.GetParsedObjectSelector()
 	if err != nil {
 		return false, apierrors.NewInternalError(err)
 	}

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

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package object
+package object_test
 
 import (
 	"testing"
@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object"
 )
 
 func TestObjectSelector(t *testing.T) {
@@ -51,7 +52,7 @@ func TestObjectSelector(t *testing.T) {
 			},
 		},
 	}
-	matcher := &Matcher{}
+	matcher := &object.Matcher{}
 	allScopes := v1.AllScopes
 	testcases := []struct {
 		name string

vendor/modules.txt

@@ -1451,10 +1451,10 @@ k8s.io/apiserver/pkg/admission/plugin/webhook/errors
 k8s.io/apiserver/pkg/admission/plugin/webhook/generic
 k8s.io/apiserver/pkg/admission/plugin/webhook/initializer
 k8s.io/apiserver/pkg/admission/plugin/webhook/mutating
-k8s.io/apiserver/pkg/admission/plugin/webhook/namespace
-k8s.io/apiserver/pkg/admission/plugin/webhook/object
+k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace
+k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object
+k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules
 k8s.io/apiserver/pkg/admission/plugin/webhook/request
-k8s.io/apiserver/pkg/admission/plugin/webhook/rules
 k8s.io/apiserver/pkg/admission/plugin/webhook/validating
 k8s.io/apiserver/pkg/admission/testing
 k8s.io/apiserver/pkg/apis/apiserver