fix: map APIResource in FindResources results (#6611)

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

pkg/clients/dclient/discovery.go

@@ -11,7 +11,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/utils/wildcard"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/discovery"
 )
 
@@ -35,7 +34,7 @@ func (gvrs GroupVersionResourceSubresource) WithSubResource(subresource string)
 
 // IDiscovery provides interface to mange Kind and GVR mapping
 type IDiscovery interface {
-	FindResources(group, version, kind, subresource string) ([]GroupVersionResourceSubresource, error)
+	FindResources(group, version, kind, subresource string) (map[GroupVersionResourceSubresource]metav1.APIResource, error)
 	FindResource(groupVersion string, kind string) (apiResource, parentAPIResource *metav1.APIResource, gvr schema.GroupVersionResource, err error)
 	// TODO: there's no mapping from GVK to GVR, this is very error prone
 	GetGVRFromGVK(schema.GroupVersionKind) (schema.GroupVersionResource, error)
@@ -159,7 +158,7 @@ func (c serverResources) FindResource(groupVersion string, kind string) (apiReso
 	return nil, nil, schema.GroupVersionResource{}, err
 }
 
-func (c serverResources) FindResources(group, version, kind, subresource string) ([]GroupVersionResourceSubresource, error) {
+func (c serverResources) FindResources(group, version, kind, subresource string) (map[GroupVersionResourceSubresource]metav1.APIResource, error) {
 	resources, err := c.findResources(group, version, kind, subresource)
 	if err != nil {
 		if !c.cachedClient.Fresh() {
@@ -170,7 +169,7 @@ func (c serverResources) FindResources(group, version, kind, subresource string)
 	return resources, err
 }
 
-func (c serverResources) findResources(group, version, kind, subresource string) ([]GroupVersionResourceSubresource, error) {
+func (c serverResources) findResources(group, version, kind, subresource string) (map[GroupVersionResourceSubresource]metav1.APIResource, error) {
 	_, serverGroupsAndResources, err := c.cachedClient.ServerGroupsAndResources()
 	if err != nil && !strings.Contains(err.Error(), "Got empty response for") {
 		if discovery.IsGroupDiscoveryFailedError(err) {
@@ -195,7 +194,7 @@ func (c serverResources) findResources(group, version, kind, subresource string)
 			Kind:    kind,
 		}
 	}
-	resources := sets.New[GroupVersionResourceSubresource]()
+	resources := map[GroupVersionResourceSubresource]metav1.APIResource{}
 	// first match resouces
 	for _, list := range serverGroupsAndResources {
 		gv, err := schema.ParseGroupVersion(list.GroupVersion)
@@ -206,23 +205,24 @@ func (c serverResources) findResources(group, version, kind, subresource string)
 				if !strings.Contains(resource.Name, "/") {
 					gvk := getGVK(gv, resource.Group, resource.Version, resource.Kind)
 					if wildcard.Match(group, gvk.Group) && wildcard.Match(version, gvk.Version) && wildcard.Match(kind, gvk.Kind) {
-						resources.Insert(GroupVersionResourceSubresource{
+						gvrs := GroupVersionResourceSubresource{
-							GroupVersionResource: gvk.GroupVersion().WithResource(resource.Name),
+							GroupVersionResource: gv.WithResource(resource.Name),
-						})
+						}
+						resources[gvrs] = resource
 					}
 				}
 			}
 		}
 	}
 	// second match subresouces if necessary
-	subresources := sets.New[GroupVersionResourceSubresource]()
+	subresources := map[GroupVersionResourceSubresource]metav1.APIResource{}
 	if subresource != "" {
 		for _, list := range serverGroupsAndResources {
 			for _, resource := range list.APIResources {
 				for parent := range resources {
 					if wildcard.Match(parent.Resource+"/"+subresource, resource.Name) {
 						parts := strings.Split(resource.Name, "/")
-						subresources.Insert(parent.WithSubResource(parts[1]))
+						subresources[parent.WithSubResource(parts[1])] = resource
 						break
 					}
 				}
@@ -230,7 +230,7 @@ func (c serverResources) findResources(group, version, kind, subresource string)
 		}
 	}
 	// third if no resource matched, try again but consider subresources this time
-	if resources.Len() == 0 {
+	if len(resources) == 0 {
 		for _, list := range serverGroupsAndResources {
 			gv, err := schema.ParseGroupVersion(list.GroupVersion)
 			if err != nil {
@@ -240,21 +240,25 @@ func (c serverResources) findResources(group, version, kind, subresource string)
 					gvk := getGVK(gv, resource.Group, resource.Version, resource.Kind)
 					if wildcard.Match(group, gvk.Group) && wildcard.Match(version, gvk.Version) && wildcard.Match(kind, gvk.Kind) {
 						parts := strings.Split(resource.Name, "/")
-						resources.Insert(GroupVersionResourceSubresource{
+						gvrs := GroupVersionResourceSubresource{
 							GroupVersionResource: gv.WithResource(parts[0]),
 							SubResource:          parts[1],
-						})
+						}
+						resources[gvrs] = resource
 					}
 				}
 			}
 		}
 	}
 	if kind == "*" && subresource == "*" {
-		return resources.Union(subresources).UnsortedList(), nil
+		for key, value := range subresources {
+			resources[key] = value
+		}
+		return resources, nil
 	} else if subresource != "" {
-		return subresources.UnsortedList(), nil
+		return subresources, nil
 	}
-	return resources.UnsortedList(), nil
+	return resources, nil
 }
 
 func (c serverResources) findResource(groupVersion string, kind string) (apiResource, parentAPIResource *metav1.APIResource,

pkg/clients/dclient/fake.go

@@ -81,7 +81,7 @@ func (c *fakeDiscoveryClient) FindResource(groupVersion string, kind string) (ap
 	return nil, nil, schema.GroupVersionResource{}, fmt.Errorf("not implemented")
 }
 
-func (c *fakeDiscoveryClient) FindResources(group, version, kind, subresource string) ([]GroupVersionResourceSubresource, error) {
+func (c *fakeDiscoveryClient) FindResources(group, version, kind, subresource string) (map[GroupVersionResourceSubresource]metav1.APIResource, error) {
 	return nil, fmt.Errorf("not implemented")
 }
 

pkg/controllers/webhook/controller.go

@@ -854,7 +854,9 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface
 				logger.Error(err, "unable to find resource", "group", group, "version", version, "kind", kind, "subresource", subresource)
 				continue
 			}
-			gvrsList = append(gvrsList, gvrss...)
+			for gvrs := range gvrss {
+				gvrsList = append(gvrsList, gvrs)
+			}
 		}
 	}
 	for _, gvr := range gvrsList {

pkg/engine/loadtargets.go

@@ -82,7 +82,7 @@ func getTargets(client dclient.Interface, target kyvernov1.ResourceSpec, ctx eng
 	if err != nil {
 		return nil, err
 	}
-	for _, gvrs := range gvrss {
+	for gvrs := range gvrss {
 		dyn := client.GetDynamicInterface().Resource(gvrs.GroupVersionResource)
 		var sub []string
 		if gvrs.SubResource != "" {

pkg/policy/validate.go

@@ -1213,16 +1213,16 @@ func validateKinds(kinds []string, mock, backgroundScanningEnabled, isValidation
 	for _, k := range kinds {
 		if !mock {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(k)
-			gvrs, err := client.Discovery().FindResources(group, version, kind, subresource)
+			gvrss, err := client.Discovery().FindResources(group, version, kind, subresource)
 			if err != nil {
 				return fmt.Errorf("unable to convert GVK to GVR for kinds %s, err: %s", k, err)
 			}
-			if len(gvrs) == 0 {
+			if len(gvrss) == 0 {
 				return fmt.Errorf("unable to convert GVK to GVR for kinds %s", k)
 			}
 			if backgroundScanningEnabled {
-				for _, gvr := range gvrs {
+				for gvrs := range gvrss {
-					if strings.Contains(gvr.Resource, "/") {
+					if strings.Contains(gvrs.Resource, "/") {
 						return fmt.Errorf("background scan enabled with subresource %s", subresource)
 					}
 				}

pkg/policycache/cache.go

@@ -4,10 +4,11 @@ import (
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	"github.com/kyverno/kyverno/pkg/utils/wildcard"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 type ResourceFinder interface {
-	FindResources(group, version, kind, subresource string) ([]dclient.GroupVersionResourceSubresource, error)
+	FindResources(group, version, kind, subresource string) (map[dclient.GroupVersionResourceSubresource]metav1.APIResource, error)
 }
 
 // Cache get method use for to get policy names and mostly use to test cache testcases

pkg/policycache/cache_test.go

@@ -33,7 +33,7 @@ func Test_All(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				// get
 				mutate := pCache.get(Mutate, gvr, "")
 				if len(mutate) != 1 {
@@ -69,7 +69,7 @@ func Test_Add_Duplicate_Policy(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				mutate := pCache.get(Mutate, gvr, "")
 				if len(mutate) != 1 {
 					t.Errorf("expected 1 mutate policy, found %v", len(mutate))
@@ -102,7 +102,7 @@ func Test_Add_Validate_Audit(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				validateEnforce := pCache.get(ValidateEnforce, gvr, "")
 				if len(validateEnforce) != 0 {
 					t.Errorf("expected 0 validate (enforce) policy, found %v", len(validateEnforce))
@@ -899,7 +899,7 @@ func Test_Ns_All(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				// get
 				mutate := pCache.get(Mutate, gvr, nspace)
 				if len(mutate) != 1 {
@@ -935,7 +935,7 @@ func Test_Ns_Add_Duplicate_Policy(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				mutate := pCache.get(Mutate, gvr, nspace)
 				if len(mutate) != 1 {
 					t.Errorf("expected 1 mutate policy, found %v", len(mutate))
@@ -968,7 +968,7 @@ func Test_Ns_Add_Validate_Audit(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				validateEnforce := pCache.get(ValidateEnforce, gvr, nspace)
 				if len(validateEnforce) != 0 {
 					t.Errorf("expected 0 validate (enforce) policy, found %v", len(validateEnforce))
@@ -1011,7 +1011,7 @@ func Test_GVk_Cache(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				generate := pCache.get(Generate, gvr, "")
 				if len(generate) != 1 {
 					t.Errorf("expected 1 generate policy, found %v", len(generate))
@@ -1049,7 +1049,7 @@ func Test_Add_Validate_Enforce(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				validateEnforce := pCache.get(ValidateEnforce, gvr, nspace)
 				if len(validateEnforce) != 1 {
 					t.Errorf("expected 1 validate policy, found %v", len(validateEnforce))
@@ -1090,7 +1090,7 @@ func Test_Mutate_Policy(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				// get
 				mutate := pCache.get(Mutate, gvr, "")
 				if len(mutate) != 1 {
@@ -1112,7 +1112,7 @@ func Test_Generate_Policy(t *testing.T) {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)
 			assert.NilError(t, err)
-			for _, gvr := range gvrs {
+			for gvr := range gvrs {
 				// get
 				generate := pCache.get(Generate, gvr, "")
 				if len(generate) != 1 {

pkg/policycache/store.go

@@ -107,7 +107,9 @@ func (m *policyMap) set(key string, policy kyvernov1.PolicyInterface, client Res
 				logger.Error(err, "failed to fetch resource group versions", "group", group, "version", version, "kind", kind)
 				errs = append(errs, err)
 			} else {
-				entries.Insert(gvrss...)
+				for gvrs := range gvrss {
+					entries.Insert(gvrs)
+				}
 			}
 		}
 		if entries.Len() > 0 {

pkg/policycache/test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
@@ -33,28 +34,29 @@ var (
 
 type TestResourceFinder struct{}
 
-func (TestResourceFinder) FindResources(group, version, kind, subresource string) ([]dclient.GroupVersionResourceSubresource, error) {
+func (TestResourceFinder) FindResources(group, version, kind, subresource string) (map[dclient.GroupVersionResourceSubresource]metav1.APIResource, error) {
+	var dummy metav1.APIResource
 	switch kind {
 	case "Pod":
-		return []dclient.GroupVersionResourceSubresource{podsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{podsGVRS: dummy}, nil
 	case "Namespace":
-		return []dclient.GroupVersionResourceSubresource{namespacesGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{namespacesGVRS: dummy}, nil
 	case "ClusterRole":
-		return []dclient.GroupVersionResourceSubresource{clusterrolesGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{clusterrolesGVRS: dummy}, nil
 	case "Deployment":
-		return []dclient.GroupVersionResourceSubresource{deploymentsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{deploymentsGVRS: dummy}, nil
 	case "StatefulSet":
-		return []dclient.GroupVersionResourceSubresource{statefulsetsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{statefulsetsGVRS: dummy}, nil
 	case "DaemonSet":
-		return []dclient.GroupVersionResourceSubresource{daemonsetsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{daemonsetsGVRS: dummy}, nil
 	case "ReplicaSet":
-		return []dclient.GroupVersionResourceSubresource{replicasetsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{replicasetsGVRS: dummy}, nil
 	case "Job":
-		return []dclient.GroupVersionResourceSubresource{jobsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{jobsGVRS: dummy}, nil
 	case "ReplicationController":
-		return []dclient.GroupVersionResourceSubresource{replicationcontrollersGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{replicationcontrollersGVRS: dummy}, nil
 	case "CronJob":
-		return []dclient.GroupVersionResourceSubresource{cronjobsGVRS}, nil
+		return map[dclient.GroupVersionResourceSubresource]metav1.APIResource{cronjobsGVRS: dummy}, nil
 	}
 	return nil, fmt.Errorf("not found: %s", kind)
 }