fix: range through all resources to build webhook

Signed-off-by: anushkamittal20 <anumittal4641@gmail.com>
anushkamittal2001 · Sep 16, 2024 · 26bda94 · 26bda94
1 parent ba7aaac
commit 26bda94
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 42 deletions.
diff --git a/pkg/controllers/webhook/controller_test.go b/pkg/controllers/webhook/controller_test.go
@@ -52,6 +52,42 @@ func TestAddOperationsForValidatingWebhookConfMultiplePolicies(t *testing.T) {
 			expectedResult: map[string][]admissionregistrationv1.OperationType{
 				"ConfigMap": {"CREATE", "UPDATE", "DELETE", "CONNECT"},
 			},
+		}, {
+			name: "test-2",
+			policies: []kyverno.ClusterPolicy{
+				{
+					Spec: kyverno.Spec{
+						Rules: []kyverno.Rule{
+							{
+								MatchResources: kyverno.MatchResources{
+									ResourceDescription: kyverno.ResourceDescription{
+										Kinds:      []string{"Role"},
+										Operations: []kyverno.AdmissionOperation{"DELETE"},
+									},
+								},
+							},
+						},
+					},
+				},
+				{
+					Spec: kyverno.Spec{
+						Rules: []kyverno.Rule{
+							{
+								MatchResources: kyverno.MatchResources{
+									ResourceDescription: kyverno.ResourceDescription{
+										Kinds:      []string{"Secrets"},
+										Operations: []kyverno.AdmissionOperation{"CONNECT"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedResult: map[string][]admissionregistrationv1.OperationType{
+				"Role":    {"DELETE"},
+				"Secrets": {"CONNECT"},
+			},
 		},
 	}
 

diff --git a/pkg/controllers/webhook/utils.go b/pkg/controllers/webhook/utils.go
@@ -2,6 +2,7 @@ package webhook
 
 import (
 	"cmp"
+	"reflect"
 	"slices"
 	"strings"
 
@@ -75,30 +76,35 @@ func (wh *webhook) buildRulesWithOperations(final map[string][]admissionregistra
 	rules := make([]admissionregistrationv1.RuleWithOperations, 0, len(wh.rules))
 
 	for gv, resources := range wh.rules {
-		firstResource := sets.List(resources)[0]
-		// if we have pods, we add pods/ephemeralcontainers by default
-		if (gv.Group == "" || gv.Group == "*") && (gv.Version == "v1" || gv.Version == "*") && (resources.Has("pods") || resources.Has("*")) {
-			resources.Insert("pods/ephemeralcontainers")
-		}
-
-		operations := findKeyContainingSubstring(final, firstResource, defaultOpn)
-		if len(operations) == 0 {
-			continue
+		ruleforset := make([]admissionregistrationv1.RuleWithOperations, 0, len(resources))
+		for res := range resources {
+			resource := sets.New(res)
+			// if we have pods, we add pods/ephemeralcontainers by default
+			if (gv.Group == "" || gv.Group == "*") && (gv.Version == "v1" || gv.Version == "*") && (resource.Has("pods") || resource.Has("*")) {
+				resource.Insert("pods/ephemeralcontainers")
+			}
+			operations := findKeyContainingSubstring(final, res, defaultOpn)
+			if len(operations) == 0 {
+				continue
+			}
+			slices.SortFunc(operations, func(a, b admissionregistrationv1.OperationType) int {
+				return cmp.Compare(a, b)
+			})
+			var added bool
+			ruleforset, added = appendResourceInRule(resource, operations, ruleforset)
+			if !added {
+				ruleforset = append(ruleforset, admissionregistrationv1.RuleWithOperations{
+					Rule: admissionregistrationv1.Rule{
+						APIGroups:   []string{gv.Group},
+						APIVersions: []string{gv.Version},
+						Resources:   sets.List(resource),
+						Scope:       ptr.To(gv.scopeType),
+					},
+					Operations: operations,
+				})
+			}
 		}
-
-		slices.SortFunc(operations, func(a, b admissionregistrationv1.OperationType) int {
-			return cmp.Compare(a, b)
-		})
-
-		rules = append(rules, admissionregistrationv1.RuleWithOperations{
-			Rule: admissionregistrationv1.Rule{
-				APIGroups:   []string{gv.Group},
-				APIVersions: []string{gv.Version},
-				Resources:   sets.List(resources),
-				Scope:       ptr.To(gv.scopeType),
-			},
-			Operations: operations,
-		})
+		rules = append(rules, ruleforset...)
 	}
 	less := func(a []string, b []string) (int, bool) {
 		if x := cmp.Compare(len(a), len(b)); x != 0 {
@@ -129,6 +135,16 @@ func (wh *webhook) buildRulesWithOperations(final map[string][]admissionregistra
 	return rules
 }
 
+func appendResourceInRule(resource sets.Set[string], operations []admissionregistrationv1.OperationType, ruleforset []admissionregistrationv1.RuleWithOperations) ([]admissionregistrationv1.RuleWithOperations, bool) {
+	for i, rule := range ruleforset {
+		if reflect.DeepEqual(rule.Operations, operations) {
+			ruleforset[i].Rule.Resources = append(rule.Rule.Resources, resource.UnsortedList()...)
+			return ruleforset, true
+		}
+	}
+	return ruleforset, false
+}
+
 func scanResourceFilterForResources(resFilter kyvernov1.ResourceFilters) []string {
 	var resources []string
 	for _, rf := range resFilter {

diff --git a/pkg/controllers/webhook/utils_test.go b/pkg/controllers/webhook/utils_test.go
@@ -35,7 +35,6 @@ var policy = `
         "name": "disallow-unsigned-images"
     },
     "spec": {
-        "validationFailureAction": "enforce",
         "background": false,
         "rules": [
             {
@@ -375,12 +374,6 @@ func TestBuildRulesWithOperations(t *testing.T) {
 		{
 			name: "Test Case 1",
 			rules: map[groupVersionScope]sets.Set[string]{
-				groupVersionScope{
-					GroupVersion: corev1.SchemeGroupVersion,
-					scopeType:    admissionregistrationv1.AllScopes,
-				}: {
-					"namespaces": sets.Empty{},
-				},
 				groupVersionScope{
 					GroupVersion: corev1.SchemeGroupVersion,
 					scopeType:    admissionregistrationv1.NamespacedScope,
@@ -390,16 +383,24 @@ func TestBuildRulesWithOperations(t *testing.T) {
 				},
 			},
 			mapResourceToOpnType: map[string][]admissionregistrationv1.OperationType{
-				"Namespace": {},
-				"Pod":       {webhookCreate, webhookUpdate},
+				"Pod":        {webhookCreate, webhookUpdate},
+				"ConfigMaps": {webhookCreate},
 			},
 			expectedResult: []admissionregistrationv1.RuleWithOperations{
 				{
+					Operations: []admissionregistrationv1.OperationType{webhookCreate},
+					Rule: admissionregistrationv1.Rule{
+						APIGroups:   []string{""},
+						APIVersions: []string{"v1"},
+						Resources:   []string{"configmaps"},
+						Scope:       ptr.To(admissionregistrationv1.NamespacedScope),
+					},
+				}, {
 					Operations: []admissionregistrationv1.OperationType{webhookCreate, webhookUpdate},
 					Rule: admissionregistrationv1.Rule{
 						APIGroups:   []string{""},
 						APIVersions: []string{"v1"},
-						Resources:   []string{"configmaps", "pods", "pods/ephemeralcontainers"},
+						Resources:   []string{"pods", "pods/ephemeralcontainers"},
 						Scope:       ptr.To(admissionregistrationv1.NamespacedScope),
 					},
 				},

diff --git a/test/conformance/chainsaw/webhooks/pod-all-subresources/webhooks.yaml b/test/conformance/chainsaw/webhooks/pod-all-subresources/webhooks.yaml
@@ -16,13 +16,13 @@ webhooks:
     - DELETE
     - UPDATE
     resources:
-    - pods/attach
-    - pods/binding
-    - pods/ephemeralcontainers
-    - pods/eviction
-    - pods/exec
-    - pods/log
-    - pods/portforward
-    - pods/proxy
-    - pods/status
+      (contains(@,'pods/status')): true
+      (contains(@,'pods/log')): true
+      (contains(@,'pods/attach')): true
+      (contains(@,'pods/ephemeralcontainers')): true
+      (contains(@,'pods/eviction')): true
+      (contains(@,'pods/exec')): true
+      (contains(@,'pods/portforward')): true
+      (contains(@,'pods/binding')): true
+      (contains(@,'pods/proxy')): true
     scope: 'Namespaced'