feat: change webhook configuration to better support wildcards (#6534)

* feat: change webhook configuration to better support wildcards

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* kuttl

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

pkg/controllers/webhook/controller.go

@@ -629,19 +629,13 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(caBundle []byte)
 			return nil, err
 		}
 		c.recordPolicyState(config.MutatingWebhookConfigurationName, policies...)
-		// TODO: shouldn't be per failure policy, depending of the policy/rules that apply ?
-		if hasWildcard(policies...) {
-			ignore.setWildcard()
-			fail.setWildcard()
-		} else {
-			for _, p := range policies {
-				spec := p.GetSpec()
-				if spec.HasMutate() || spec.HasVerifyImages() {
-					if spec.GetFailurePolicy() == kyvernov1.Ignore {
-						c.mergeWebhook(ignore, p, false)
-					} else {
-						c.mergeWebhook(fail, p, false)
-					}
+		for _, p := range policies {
+			spec := p.GetSpec()
+			if spec.HasMutate() || spec.HasVerifyImages() {
+				if spec.GetFailurePolicy() == kyvernov1.Ignore {
+					c.mergeWebhook(ignore, p, false)
+				} else {
+					c.mergeWebhook(fail, p, false)
 				}
 			}
 		}
@@ -736,19 +730,13 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(caBundle []byte
 			return nil, err
 		}
 		c.recordPolicyState(config.ValidatingWebhookConfigurationName, policies...)
-		// TODO: shouldn't be per failure policy, depending of the policy/rules that apply ?
-		if hasWildcard(policies...) {
-			ignore.setWildcard()
-			fail.setWildcard()
-		} else {
-			for _, p := range policies {
-				spec := p.GetSpec()
-				if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasImagesValidationChecks() || spec.HasYAMLSignatureVerify() {
-					if spec.GetFailurePolicy() == kyvernov1.Ignore {
-						c.mergeWebhook(ignore, p, true)
-					} else {
-						c.mergeWebhook(fail, p, true)
-					}
+		for _, p := range policies {
+			spec := p.GetSpec()
+			if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasImagesValidationChecks() || spec.HasYAMLSignatureVerify() {
+				if spec.GetFailurePolicy() == kyvernov1.Ignore {
+					c.mergeWebhook(ignore, p, true)
+				} else {
+					c.mergeWebhook(fail, p, true)
 				}
 			}
 		}
@@ -848,14 +836,23 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface
 			gvkMap[gvk] = 1
 			// NOTE: webhook stores GVR in its rules while policy stores GVK in its rules definition
 			group, version, kind, subresource := kubeutils.ParseKindSelector(gvk)
-			gvrs, err := c.discoveryClient.FindResources(group, version, kind, subresource)
-			if err != nil {
-				logger.Error(err, "unable to find resource", "group", group, "version", version, "kind", kind, "subresource", subresource)
-				continue
-			}
-			for _, gvr := range gvrs {
-				logger.V(4).Info("configuring webhook", "GVK", gvk, "GVR", gvr)
-				gvrList = append(gvrList, gvr)
+			// if kind is `*` no need to lookup resources
+			if kind == "*" && subresource == "*" {
+				gvrList = append(gvrList, schema.GroupVersionResource{Group: group, Version: version, Resource: "*/*"})
+			} else if kind == "*" && subresource == "" {
+				gvrList = append(gvrList, schema.GroupVersionResource{Group: group, Version: version, Resource: "*"})
+			} else if kind == "*" && subresource != "" {
+				gvrList = append(gvrList, schema.GroupVersionResource{Group: group, Version: version, Resource: "*/" + subresource})
+			} else {
+				gvrs, err := c.discoveryClient.FindResources(group, version, kind, subresource)
+				if err != nil {
+					logger.Error(err, "unable to find resource", "group", group, "version", version, "kind", kind, "subresource", subresource)
+					continue
+				}
+				for _, gvr := range gvrs {
+					logger.V(4).Info("configuring webhook", "GVK", gvk, "GVR", gvr)
+					gvrList = append(gvrList, gvr)
+				}
 			}
 		}
 	}

pkg/controllers/webhook/utils.go

@@ -32,7 +32,7 @@ func (wh *webhook) buildRulesWithOperations(ops ...admissionregistrationv1.Opera
 	var rules []admissionregistrationv1.RuleWithOperations
 	for gv, resources := range wh.rules {
 		// if we have pods, we add pods/ephemeralcontainers by default
-		if gv.Group == "" && gv.Version == "v1" && resources.Has("pods") {
+		if (gv.Group == "" || gv.Group == "*") && (gv.Version == "v1" || gv.Version == "*") && (resources.Has("pods") || resources.Has("*")) {
 			resources.Insert("pods/ephemeralcontainers")
 		}
 		rules = append(rules, admissionregistrationv1.RuleWithOperations{
@@ -84,24 +84,6 @@ func (wh *webhook) isEmpty() bool {
 	return len(wh.rules) == 0
 }
 
-func (wh *webhook) setWildcard() {
-	wh.rules = map[schema.GroupVersion]sets.Set[string]{
-		{Group: "*", Version: "*"}: sets.New("*/*"),
-	}
-}
-
-func hasWildcard(policies ...kyvernov1.PolicyInterface) bool {
-	for _, policy := range policies {
-		spec := policy.GetSpec()
-		for _, rule := range spec.Rules {
-			if kinds := rule.MatchResources.GetKinds(); slices.Contains(kinds, "*") {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 func objectMeta(name string, owner ...metav1.OwnerReference) metav1.ObjectMeta {
 	return metav1.ObjectMeta{
 		Name: name,

pkg/controllers/webhook/utils_test.go

@@ -9,13 +9,14 @@ import (
 
 	"gotest.tools/assert"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 func Test_webhook_isEmpty(t *testing.T) {
 	empty := newWebhook(DefaultWebhookTimeout, admissionregistrationv1.Ignore)
 	assert.Equal(t, empty.isEmpty(), true)
 	notEmpty := newWebhook(DefaultWebhookTimeout, admissionregistrationv1.Ignore)
-	notEmpty.setWildcard()
+	notEmpty.set(schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"})
 	assert.Equal(t, notEmpty.isEmpty(), false)
 }
 

test/conformance/kuttl/webhooks/all-scale/webhooks.yaml

@@ -7,28 +7,14 @@ metadata:
 webhooks:
 - rules:
   - apiGroups:
-    - ""
+    - '*'
     apiVersions:
-    - v1
+    - '*'
     operations:
     - CREATE
     - UPDATE
     - DELETE
     - CONNECT
     resources:
-    - replicationcontrollers/scale
-    scope: '*'
-  - apiGroups:
-    - apps
-    apiVersions:
-    - v1
-    operations:
-    - CREATE
-    - UPDATE
-    - DELETE
-    - CONNECT
-    resources:
-    - deployments/scale
-    - replicasets/scale
-    - statefulsets/scale
+    - '*/scale'
     scope: '*'

test/conformance/kuttl/webhooks/double-wildcard/01-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-assert.yaml

test/conformance/kuttl/webhooks/double-wildcard/02-webhooks.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+assert:
+- webhooks.yaml

test/conformance/kuttl/webhooks/double-wildcard/README.md

@@ -0,0 +1,9 @@
+## Description
+
+This test verifies the resource validation webhook is configured correctly when a policy targets all `*/*` resources and subresources.
+
+## Steps
+
+1.  - Create a policy targeting `*/*`
+    - Assert policy gets ready
+1.  - Assert that the resource validation webhook is configured correctly

test/conformance/kuttl/webhooks/double-wildcard/policy-assert.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: require-labels
+status:
+  conditions:
+    - reason: Succeeded
+      status: "True"
+      type: Ready

test/conformance/kuttl/webhooks/double-wildcard/policy.yaml

@@ -0,0 +1,22 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: require-labels
+  annotations:
+    pod-policies.kyverno.io/autogen-controllers: none
+spec:
+  validationFailureAction: Audit
+  background: false
+  rules:
+    - name: require-team
+      match:
+        any:
+          - resources:
+              kinds:
+                - '*/*'
+      validate:
+        message: 'The label `team` is required.'
+        pattern:
+          metadata:
+            labels:
+              team: '?*'

test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml

@@ -0,0 +1,21 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  labels:
+    webhook.kyverno.io/managed-by: kyverno
+  name: kyverno-resource-validating-webhook-cfg
+webhooks:
+- failurePolicy: Fail
+  rules:
+  - apiGroups:
+    - '*'
+    apiVersions:
+    - '*'
+    operations:
+    - CREATE
+    - UPDATE
+    - DELETE
+    - CONNECT
+    resources:
+    - '*/*'
+    scope: '*'

test/conformance/kuttl/webhooks/wildcard/README.md

@@ -1,6 +1,6 @@
 ## Description
 
-This test verifies the resource validation webhook is configured correctly when a policy targets all `*` resource.
+This test verifies the resource validation webhook is configured correctly when a policy targets all `*` resources.
 
 ## Steps
 

test/conformance/kuttl/webhooks/wildcard/webhooks.yaml

@@ -5,20 +5,6 @@ metadata:
     webhook.kyverno.io/managed-by: kyverno
   name: kyverno-resource-validating-webhook-cfg
 webhooks:
-- failurePolicy: Ignore
-  rules:
-  - apiGroups:
-    - '*'
-    apiVersions:
-    - '*'
-    operations:
-    - CREATE
-    - UPDATE
-    - DELETE
-    - CONNECT
-    resources:
-    - '*/*'
-    scope: '*'
 - failurePolicy: Fail
   rules:
   - apiGroups:
@@ -31,5 +17,6 @@ webhooks:
     - DELETE
     - CONNECT
     resources:
-    - '*/*'
+    - '*'
+    - pods/ephemeralcontainers
     scope: '*'