From b4ee0c0574932b99a9e877c84d880a5f00fdd3cc Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Tue, 25 Oct 2022 16:28:16 -0700 Subject: [PATCH 1/4] Move webhook scoping rules into a predicates directory Signed-off-by: Max Smythe --- .../pkg/admission/plugin/webhook/generic/webhook.go | 6 +++--- .../pkg/admission/plugin/webhook/generic/webhook_test.go | 4 ++-- .../plugin/webhook/{ => predicates}/namespace/doc.go | 0 .../plugin/webhook/{ => predicates}/namespace/matcher.go | 0 .../webhook/{ => predicates}/namespace/matcher_test.go | 0 .../admission/plugin/webhook/{ => predicates}/object/doc.go | 0 .../plugin/webhook/{ => predicates}/object/matcher.go | 0 .../plugin/webhook/{ => predicates}/object/matcher_test.go | 0 .../plugin/webhook/{ => predicates}/rules/rules.go | 0 .../plugin/webhook/{ => predicates}/rules/rules_test.go | 0 10 files changed, 5 insertions(+), 5 deletions(-) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/namespace/doc.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/namespace/matcher.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/namespace/matcher_test.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/object/doc.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/object/matcher.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/object/matcher_test.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/rules/rules.go (100%) rename staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/{ => predicates}/rules/rules_test.go (100%) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go index c04225e94f7..e3297aed8f5 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go +++ b/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" diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go index 26dbefc1991..c564a45e954 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go +++ b/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) { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/doc.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/doc.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/doc.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/doc.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher_test.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher_test.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/doc.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/doc.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/doc.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/doc.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher_test.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/object/matcher_test.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules_test.go similarity index 100% rename from staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules_test.go rename to staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/rules/rules_test.go From 00ebe0bf623295dc589e43e8c299003f9e939f65 Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Tue, 25 Oct 2022 16:34:06 -0700 Subject: [PATCH 2/4] Make interface for webhook predicates more specific Signed-off-by: Max Smythe --- .../pkg/admission/plugin/webhook/accessors.go | 11 +++++++---- .../plugin/webhook/predicates/namespace/matcher.go | 10 +++++++--- .../webhook/predicates/namespace/matcher_test.go | 7 ++++--- .../plugin/webhook/predicates/object/matcher.go | 10 +++++++--- .../plugin/webhook/predicates/object/matcher_test.go | 5 +++-- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go index 0cbc21c8267..bbe355f318e 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go +++ b/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 diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher.go index 64935b3f5c3..bb7948973b6 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher.go +++ b/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) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher_test.go index 331f0dd9228..7cd235c9149 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/matcher_test.go +++ b/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) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher.go index 0dccb5bb3ae..aadf9073277 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher.go +++ b/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) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher_test.go index 33d64d12e1b..3149dfb3f64 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/matcher_test.go +++ b/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 From 003fbae25bf4c76b8b71d56206b51e1ee6e80812 Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Tue, 25 Oct 2022 20:40:27 -0700 Subject: [PATCH 3/4] Fix canonical imports Signed-off-by: Max Smythe --- .../pkg/admission/plugin/webhook/predicates/namespace/doc.go | 2 +- .../pkg/admission/plugin/webhook/predicates/object/doc.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/doc.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/doc.go index d1a28533836..660001dff78 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/namespace/doc.go +++ b/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" diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/doc.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/doc.go index 93c47344095..8964afa6c5f 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/predicates/object/doc.go +++ b/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" From 5f0e54eea66e8d4fe951c448b58e75036e0a3d6a Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Tue, 25 Oct 2022 21:06:30 -0700 Subject: [PATCH 4/4] Update vendor directory Signed-off-by: Max Smythe --- vendor/modules.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/modules.txt b/vendor/modules.txt index 77e7f0f7dff..4b51a47849f 100644 --- a/vendor/modules.txt +++ b/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