diff --git a/pkg/clients/dclient/discovery.go b/pkg/clients/dclient/discovery.go index 7ac81c1caa..93a7a68aca 100644 --- a/pkg/clients/dclient/discovery.go +++ b/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{ - GroupVersionResource: gvk.GroupVersion().WithResource(resource.Name), - }) + gvrs := GroupVersionResourceSubresource{ + 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, diff --git a/pkg/clients/dclient/fake.go b/pkg/clients/dclient/fake.go index f82a29639c..681765b59b 100644 --- a/pkg/clients/dclient/fake.go +++ b/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") } diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go index b0d4ddd285..6862c5a769 100644 --- a/pkg/controllers/webhook/controller.go +++ b/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 { diff --git a/pkg/engine/loadtargets.go b/pkg/engine/loadtargets.go index db1a45a024..5364d38c80 100644 --- a/pkg/engine/loadtargets.go +++ b/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 != "" { diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 3dfb33d93d..2f089602a2 100644 --- a/pkg/policy/validate.go +++ b/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 { - if strings.Contains(gvr.Resource, "/") { + for gvrs := range gvrss { + if strings.Contains(gvrs.Resource, "/") { return fmt.Errorf("background scan enabled with subresource %s", subresource) } } diff --git a/pkg/policycache/cache.go b/pkg/policycache/cache.go index f31fb4a3c0..194c76e7fe 100644 --- a/pkg/policycache/cache.go +++ b/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 diff --git a/pkg/policycache/cache_test.go b/pkg/policycache/cache_test.go index 73010fce31..de82e1797e 100644 --- a/pkg/policycache/cache_test.go +++ b/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 { diff --git a/pkg/policycache/store.go b/pkg/policycache/store.go index 8eeebfd6a0..d4959df812 100644 --- a/pkg/policycache/store.go +++ b/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 { diff --git a/pkg/policycache/test.go b/pkg/policycache/test.go index 1051e8ea4f..9a3f9678aa 100644 --- a/pkg/policycache/test.go +++ b/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) }