feat: separate webhook rules per GVK/rule (#4986)

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

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

pkg/controllers/webhook/controller.go

@@ -646,11 +646,9 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(caBundle []byte)
 			result.Webhooks = append(
 				result.Webhooks,
 				admissionregistrationv1.MutatingWebhook{
-					Name:         config.MutatingWebhookName + "-ignore",
+					Name:                    config.MutatingWebhookName + "-ignore",
-					ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/ignore"),
+					ClientConfig:            c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/ignore"),
-					Rules: []admissionregistrationv1.RuleWithOperations{
+					Rules:                   ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update),
-						ignore.buildRuleWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update),
-					},
 					FailurePolicy:           &ignore.failurePolicy,
 					SideEffects:             &noneOnDryRun,
 					AdmissionReviewVersions: []string{"v1beta1"},
@@ -665,11 +663,9 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(caBundle []byte)
 			result.Webhooks = append(
 				result.Webhooks,
 				admissionregistrationv1.MutatingWebhook{
-					Name:         config.MutatingWebhookName + "-fail",
+					Name:                    config.MutatingWebhookName + "-fail",
-					ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/fail"),
+					ClientConfig:            c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/fail"),
-					Rules: []admissionregistrationv1.RuleWithOperations{
+					Rules:                   fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update),
-						fail.buildRuleWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update),
-					},
 					FailurePolicy:           &fail.failurePolicy,
 					SideEffects:             &noneOnDryRun,
 					AdmissionReviewVersions: []string{"v1beta1"},
@@ -761,11 +757,9 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(caBundle []byte
 			result.Webhooks = append(
 				result.Webhooks,
 				admissionregistrationv1.ValidatingWebhook{
-					Name:         config.ValidatingWebhookName + "-ignore",
+					Name:                    config.ValidatingWebhookName + "-ignore",
-					ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/ignore"),
+					ClientConfig:            c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/ignore"),
-					Rules: []admissionregistrationv1.RuleWithOperations{
+					Rules:                   ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect),
-						ignore.buildRuleWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect),
-					},
 					FailurePolicy:           &ignore.failurePolicy,
 					SideEffects:             sideEffects,
 					AdmissionReviewVersions: []string{"v1beta1"},
@@ -779,11 +773,9 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(caBundle []byte
 			result.Webhooks = append(
 				result.Webhooks,
 				admissionregistrationv1.ValidatingWebhook{
-					Name:         config.ValidatingWebhookName + "-fail",
+					Name:                    config.ValidatingWebhookName + "-fail",
-					ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/fail"),
+					ClientConfig:            c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/fail"),
-					Rules: []admissionregistrationv1.RuleWithOperations{
+					Rules:                   fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect),
-						fail.buildRuleWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect),
-					},
 					FailurePolicy:           &fail.failurePolicy,
 					SideEffects:             sideEffects,
 					AdmissionReviewVersions: []string{"v1beta1"},
@@ -878,20 +870,7 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface
 		}
 	}
 	for _, gvr := range gvrList {
-		dst.groups.Insert(gvr.Group)
+		dst.set(gvr)
-		if gvr.Version == "*" {
-			dst.versions = sets.NewString()
-			dst.versions.Insert(gvr.Version)
-		} else if !dst.versions.Has("*") {
-			dst.versions.Insert(gvr.Version)
-		}
-		dst.resources.Insert(gvr.Resource)
-	}
-	if dst.resources.Has("pods") {
-		dst.resources.Insert("pods/ephemeralcontainers")
-	}
-	if dst.resources.Has("services") {
-		dst.resources.Insert("services/status")
 	}
 	spec := policy.GetSpec()
 	if spec.WebhookTimeoutSeconds != nil {

pkg/controllers/webhook/utils.go

@@ -5,8 +5,10 @@ import (
 
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/utils"
+	"golang.org/x/exp/slices"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -15,40 +17,74 @@ import (
 type webhook struct {
 	maxWebhookTimeout int32
 	failurePolicy     admissionregistrationv1.FailurePolicyType
-	groups            sets.String
+	rules             map[schema.GroupVersionResource]struct{}
-	versions          sets.String
-	resources         sets.String
 }
 
 func newWebhook(timeout int32, failurePolicy admissionregistrationv1.FailurePolicyType) *webhook {
 	return &webhook{
 		maxWebhookTimeout: timeout,
 		failurePolicy:     failurePolicy,
-		groups:            sets.NewString(),
+		rules:             map[schema.GroupVersionResource]struct{}{},
-		versions:          sets.NewString(),
-		resources:         sets.NewString(),
 	}
 }
 
-func (wh *webhook) buildRuleWithOperations(ops ...admissionregistrationv1.OperationType) admissionregistrationv1.RuleWithOperations {
+func (wh *webhook) buildRulesWithOperations(ops ...admissionregistrationv1.OperationType) []admissionregistrationv1.RuleWithOperations {
-	return admissionregistrationv1.RuleWithOperations{
+	var rules []admissionregistrationv1.RuleWithOperations
-		Rule: admissionregistrationv1.Rule{
+	for gvr := range wh.rules {
-			APIGroups:   wh.groups.List(),
+		resources := sets.NewString(gvr.Resource)
-			APIVersions: wh.versions.List(),
+		if resources.Has("pods") {
-			Resources:   wh.resources.List(),
+			resources.Insert("pods/ephemeralcontainers")
-		},
+		}
-		Operations: ops,
+		if resources.Has("services") {
+			resources.Insert("services/status")
+		}
+		rules = append(rules, admissionregistrationv1.RuleWithOperations{
+			Rule: admissionregistrationv1.Rule{
+				APIGroups:   []string{gvr.Group},
+				APIVersions: []string{gvr.Version},
+				Resources:   resources.List(),
+			},
+			Operations: ops,
+		})
 	}
+	less := func(a []string, b []string) (bool, bool) {
+		if len(a) != len(b) {
+			return len(a) < len(b), true
+		}
+		for i := range a {
+			if a[i] != b[i] {
+				return a[i] < b[i], true
+			}
+		}
+		return false, false
+	}
+	slices.SortFunc(rules, func(a admissionregistrationv1.RuleWithOperations, b admissionregistrationv1.RuleWithOperations) bool {
+		if less, match := less(a.APIGroups, b.APIGroups); match {
+			return less
+		}
+		if less, match := less(a.APIVersions, b.APIVersions); match {
+			return less
+		}
+		if less, match := less(a.Resources, b.Resources); match {
+			return less
+		}
+		return false
+	})
+	return rules
+}
+
+func (wh *webhook) set(gvr schema.GroupVersionResource) {
+	wh.rules[gvr] = struct{}{}
 }
 
 func (wh *webhook) isEmpty() bool {
-	return wh.groups.Len() == 0 || wh.versions.Len() == 0 || wh.resources.Len() == 0
+	return len(wh.rules) == 0
 }
 
 func (wh *webhook) setWildcard() {
-	wh.groups = sets.NewString("*")
+	wh.rules = map[schema.GroupVersionResource]struct{}{
-	wh.versions = sets.NewString("*")
+		{Group: "*", Version: "*", Resource: "*/*"}: {},
-	wh.resources = sets.NewString("*/*")
+	}
 }
 
 func hasWildcard(policies ...kyvernov1.PolicyInterface) bool {