From 71803c46d81160b065a2f7ce5d6906404d4e69ff Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Thu, 12 Aug 2021 23:26:35 +0530 Subject: [PATCH 1/6] GVK format --- pkg/policymutation/policymutation.go | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index a5d83a7b26..ebdd7f31ad 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -60,6 +60,17 @@ func GenerateJSONPatchesForDefaults(policy *kyverno.ClusterPolicy, log logr.Logg } patches = append(patches, convertPatch...) + formatedGVK, errs := checkForGVKFormatPatch(policy, log) + if len(errs) > 0 { + var errMsgs []string + for _, err := range errs { + errMsgs = append(errMsgs, err.Error()) + log.Error(err, "failed to format the kind") + } + updateMsgs = append(updateMsgs, strings.Join(errMsgs, ";")) + } + patches = append(patches, formatedGVK...) + overlaySMPPatches, errs := convertOverlayToStrategicMerge(policy, log) if len(errs) > 0 { var errMsgs []string @@ -74,6 +85,42 @@ func GenerateJSONPatchesForDefaults(policy *kyverno.ClusterPolicy, log logr.Logg return utils.JoinPatches(patches), updateMsgs } +func checkForGVKFormatPatch(policy *kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) { + patches = make([][]byte, 0) + for i, rule := range policy.Spec.Rules { + kindList := []string{} + for _, k := range rule.MatchResources.Kinds { + kindList = append(kindList, getFormatedKind(k)) + } + jsonPatch := struct { + Path string `json:"path"` + Op string `json:"op"` + Value []string `json:"value"` + }{ + fmt.Sprintf("/spec/rules/%s/match/resources/kinds", strconv.Itoa(i)), + "replace", + kindList, + } + patchByte, err := json.Marshal(jsonPatch) + if err != nil { + errs = append(errs, fmt.Errorf("failed to convert policy '%s': %v", policy.Name, err)) + } + patches = append(patches, patchByte) + } + return patches, errs +} + +func getFormatedKind(str string) (kind string) { + if strings.Count(str, "/") == 0 { + return strings.Title(strings.ToLower(str)) + } + splitString := strings.Split(str, "/") + if strings.Count(str, "/") == 1 { + return splitString[0] + "/" + strings.Title(strings.ToLower(splitString[1])) + } + return splitString[0] + "/" + splitString[1] + "/" + strings.Title(strings.ToLower(splitString[2])) +} + func convertPatchToJSON6902(policy *kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) { patches = make([][]byte, 0) From 8e2033cc9520360f39667b19060cbe46d9d6fcdb Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Fri, 13 Aug 2021 10:40:47 +0530 Subject: [PATCH 2/6] fix test case --- test/e2e/mutate/mutate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/mutate/mutate_test.go b/test/e2e/mutate/mutate_test.go index 368578dc63..b8eadd67e9 100644 --- a/test/e2e/mutate/mutate_test.go +++ b/test/e2e/mutate/mutate_test.go @@ -107,7 +107,7 @@ func Test_Mutate_Sets(t *testing.T) { cmRes, err := e2eClient.GetNamespacedResource(cmGVR, tests.ResourceNamespace, "target") c, _ := json.Marshal(cmRes) - By(fmt.Sprintf("configMap : %s", string(c))) + By(fmt.Sprintf("ConfigMap : %s", string(c))) Expect(err).NotTo(HaveOccurred()) Expect(cmRes.GetLabels()["kyverno.key/copy-me"]).To(Equal("sample-value")) From cc957e2c6ef35b8bbcf1771b5603d670d11a02b9 Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Fri, 13 Aug 2021 14:08:18 +0530 Subject: [PATCH 3/6] fix issue --- pkg/policymutation/policymutation.go | 6 +++--- test/e2e/mutate/mutate_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index ebdd7f31ad..cb3bd98af2 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -112,13 +112,13 @@ func checkForGVKFormatPatch(policy *kyverno.ClusterPolicy, log logr.Logger) (pat func getFormatedKind(str string) (kind string) { if strings.Count(str, "/") == 0 { - return strings.Title(strings.ToLower(str)) + return strings.Title(str) } splitString := strings.Split(str, "/") if strings.Count(str, "/") == 1 { - return splitString[0] + "/" + strings.Title(strings.ToLower(splitString[1])) + return splitString[0] + "/" + strings.Title(splitString[1]) } - return splitString[0] + "/" + splitString[1] + "/" + strings.Title(strings.ToLower(splitString[2])) + return splitString[0] + "/" + splitString[1] + "/" + strings.Title(splitString[2]) } func convertPatchToJSON6902(policy *kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) { diff --git a/test/e2e/mutate/mutate_test.go b/test/e2e/mutate/mutate_test.go index b8eadd67e9..368578dc63 100644 --- a/test/e2e/mutate/mutate_test.go +++ b/test/e2e/mutate/mutate_test.go @@ -107,7 +107,7 @@ func Test_Mutate_Sets(t *testing.T) { cmRes, err := e2eClient.GetNamespacedResource(cmGVR, tests.ResourceNamespace, "target") c, _ := json.Marshal(cmRes) - By(fmt.Sprintf("ConfigMap : %s", string(c))) + By(fmt.Sprintf("configMap : %s", string(c))) Expect(err).NotTo(HaveOccurred()) Expect(cmRes.GetLabels()["kyverno.key/copy-me"]).To(Equal("sample-value")) From 2ee32214f9cfcf1dfe532e37f443866a817d76b6 Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Thu, 2 Sep 2021 12:40:40 +0530 Subject: [PATCH 4/6] Dynamic process of GVK --- pkg/common/common.go | 14 +++++++++++++- pkg/engine/utils.go | 1 + pkg/kyverno/common/fetch.go | 3 ++- pkg/openapi/validation.go | 3 ++- pkg/policy/existing.go | 3 ++- pkg/policycache/cache.go | 4 +++- pkg/policymutation/policymutation.go | 13 +------------ 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/pkg/common/common.go b/pkg/common/common.go index 37a8155777..317c48cfdc 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -3,10 +3,11 @@ package common import ( "encoding/json" "fmt" - apierrors "k8s.io/apimachinery/pkg/api/errors" "strings" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" dclient "github.com/kyverno/kyverno/pkg/dclient" @@ -201,3 +202,14 @@ func removePolicyFromLabels(pName string, labels map[string]string) (bool, map[s return false, labels } + +func GetFormatedKind(str string) (kind string) { + if strings.Count(str, "/") == 0 { + return strings.Title(str) + } + splitString := strings.Split(str, "/") + if strings.Count(str, "/") == 1 { + return splitString[0] + "/" + strings.Title(splitString[1]) + } + return splitString[0] + "/" + splitString[1] + "/" + strings.Title(splitString[2]) +} diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 53b27678ae..14229fd3cc 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -30,6 +30,7 @@ type EngineStats struct { func checkKind(kinds []string, resource unstructured.Unstructured) bool { for _, kind := range kinds { + kind = strings.Title(kind) SplitGVK := strings.Split(kind, "/") if len(SplitGVK) == 1 { if resource.GetKind() == kind { diff --git a/pkg/kyverno/common/fetch.go b/pkg/kyverno/common/fetch.go index 627b015719..58e7923442 100644 --- a/pkg/kyverno/common/fetch.go +++ b/pkg/kyverno/common/fetch.go @@ -10,6 +10,7 @@ import ( "github.com/go-git/go-billy/v5" v1 "github.com/kyverno/kyverno/pkg/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/common" client "github.com/kyverno/kyverno/pkg/dclient" engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/utils" @@ -33,7 +34,7 @@ func GetResources(policies []*v1.ClusterPolicy, resourcePaths []string, dClient for _, policy := range policies { for _, rule := range policy.Spec.Rules { for _, kind := range rule.MatchResources.Kinds { - resourceTypesMap[kind] = true + resourceTypesMap[common.GetFormatedKind(kind)] = true } } } diff --git a/pkg/openapi/validation.go b/pkg/openapi/validation.go index abffd4a173..c0cacc3776 100644 --- a/pkg/openapi/validation.go +++ b/pkg/openapi/validation.go @@ -12,6 +12,7 @@ import ( openapiv2 "github.com/googleapis/gnostic/openapiv2" data "github.com/kyverno/kyverno/api" v1 "github.com/kyverno/kyverno/pkg/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/common" "github.com/kyverno/kyverno/pkg/engine" "github.com/kyverno/kyverno/pkg/utils" cmap "github.com/orcaman/concurrent-map" @@ -144,7 +145,7 @@ func (o *Controller) ValidatePolicyMutation(policy v1.ClusterPolicy) error { for _, rule := range policy.Spec.Rules { if rule.HasMutate() { for _, kind := range rule.MatchResources.Kinds { - kindToRules[kind] = append(kindToRules[kind], rule) + kindToRules[kind] = append(kindToRules[common.GetFormatedKind(kind)], rule) } } } diff --git a/pkg/policy/existing.go b/pkg/policy/existing.go index 2bbbe53a66..08cad2ba36 100644 --- a/pkg/policy/existing.go +++ b/pkg/policy/existing.go @@ -29,7 +29,8 @@ func (pc *PolicyController) processExistingResources(policy *kyverno.ClusterPoli continue } - for _, k := range rule.MatchResources.Kinds { + for _, kind := range rule.MatchResources.Kinds { + k := common.GetFormatedKind(kind) logger = logger.WithValues("rule", rule.Name, "kind", k) namespaced, err := pc.rm.GetScope(k) if err != nil { diff --git a/pkg/policycache/cache.go b/pkg/policycache/cache.go index f742d6b747..9c390c9141 100644 --- a/pkg/policycache/cache.go +++ b/pkg/policycache/cache.go @@ -1,6 +1,7 @@ package policycache import ( + "strings" "sync" "github.com/go-logr/logr" @@ -141,7 +142,8 @@ func (m *pMap) add(policy *kyverno.ClusterPolicy) { func addCacheHelper(rmr kyverno.ResourceFilter, m *pMap, rule kyverno.Rule, mutateMap map[string]bool, pName string, enforcePolicy bool, validateEnforceMap map[string]bool, validateAuditMap map[string]bool, generateMap map[string]bool, imageVerifyMap map[string]bool) { for _, gvk := range rmr.Kinds { - _, kind := common.GetKindFromGVK(gvk) + _, k := common.GetKindFromGVK(gvk) + kind := strings.Title(k) _, ok := m.kindDataMap[kind] if !ok { m.kindDataMap[kind] = make(map[PolicyType][]string) diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index cb3bd98af2..d4407f4870 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -90,7 +90,7 @@ func checkForGVKFormatPatch(policy *kyverno.ClusterPolicy, log logr.Logger) (pat for i, rule := range policy.Spec.Rules { kindList := []string{} for _, k := range rule.MatchResources.Kinds { - kindList = append(kindList, getFormatedKind(k)) + kindList = append(kindList, common.GetFormatedKind(k)) } jsonPatch := struct { Path string `json:"path"` @@ -110,17 +110,6 @@ func checkForGVKFormatPatch(policy *kyverno.ClusterPolicy, log logr.Logger) (pat return patches, errs } -func getFormatedKind(str string) (kind string) { - if strings.Count(str, "/") == 0 { - return strings.Title(str) - } - splitString := strings.Split(str, "/") - if strings.Count(str, "/") == 1 { - return splitString[0] + "/" + strings.Title(splitString[1]) - } - return splitString[0] + "/" + splitString[1] + "/" + strings.Title(splitString[2]) -} - func convertPatchToJSON6902(policy *kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) { patches = make([][]byte, 0) From 601fb711e566850889830ef1e438d05240cb8c1c Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Fri, 3 Sep 2021 10:32:09 +0530 Subject: [PATCH 5/6] fix unit test issue --- pkg/engine/utils.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 14229fd3cc..8b7929be18 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -30,18 +30,17 @@ type EngineStats struct { func checkKind(kinds []string, resource unstructured.Unstructured) bool { for _, kind := range kinds { - kind = strings.Title(kind) SplitGVK := strings.Split(kind, "/") if len(SplitGVK) == 1 { - if resource.GetKind() == kind { + if resource.GetKind() == strings.Title(kind) { return true } } else if len(SplitGVK) == 2 { - if resource.GroupVersionKind().Kind == SplitGVK[1] && resource.GroupVersionKind().Version == SplitGVK[0] { + if resource.GroupVersionKind().Kind == strings.Title(SplitGVK[1]) && resource.GroupVersionKind().Version == SplitGVK[0] { return true } } else { - if resource.GroupVersionKind().Group == SplitGVK[0] && resource.GroupVersionKind().Kind == SplitGVK[2] && (resource.GroupVersionKind().Version == SplitGVK[1] || resource.GroupVersionKind().Version == "*") { + if resource.GroupVersionKind().Group == SplitGVK[0] && resource.GroupVersionKind().Kind == strings.Title(SplitGVK[2]) && (resource.GroupVersionKind().Version == SplitGVK[1] || resource.GroupVersionKind().Version == "*") { return true } } From 28cceb9229b57a2c6f6d6a4a919cbf8172cbeb94 Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Fri, 3 Sep 2021 10:42:43 +0530 Subject: [PATCH 6/6] add unit test cases --- pkg/engine/utils_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/engine/utils_test.go b/pkg/engine/utils_test.go index 7e1a2dd289..11340244cc 100644 --- a/pkg/engine/utils_test.go +++ b/pkg/engine/utils_test.go @@ -869,6 +869,24 @@ func TestMatchesResourceDescription(t *testing.T) { Policy: []byte(`{ "apiVersion": "kyverno.io/v1", "kind": "ClusterPolicy", "metadata": { "name": "check-host-path" }, "spec": { "validationFailureAction": "enforce", "background": true, "rules": [ { "name": "check-host-path", "match": { "resources": { "kinds": [ "rbac.authorization.k8s.io/v1beta1/ClusterRole" ] } }, "validate": { "message": "Host path is not allowed", "pattern": { "spec": { "volumes": [ { "name": "*", "hostPath": { "path": "" } } ] } } } } ] } }`), areErrorsExpected: true, }, + { + Description: "Test for GVK case sensitive", + AdmissionInfo: kyverno.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ "apiVersion": "v1", "kind": "Pod", "metadata": { "name": "myapp-pod2", "labels": { "app": "myapp2" } }, "spec": { "containers": [ { "name": "nginx", "image": "nginx" } ] } }`), + Policy: []byte(`{ "apiVersion": "kyverno.io/v1", "kind": "ClusterPolicy", "metadata": { "name": "disallow-latest-tag", "annotations": { "policies.kyverno.io/category": "Workload Isolation", "policies.kyverno.io/description": "The ':latest' tag is mutable and can lead to unexpected errors if the image changes. A best practice is to use an immutable tag that maps to a specific version of an application pod." } }, "spec": { "validationFailureAction": "enforce", "rules": [ { "name": "require-image-tag", "match": { "resources": { "kinds": [ "pod" ] } }, "validate": { "message": "An image tag is required", "pattern": { "spec": { "containers": [ { "image": "*:*" } ] } } } } ] } }`), + areErrorsExpected: false, + }, + { + Description: "Test should pass for GVK case sensitive", + AdmissionInfo: kyverno.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": { "creationTimestamp": "2020-09-21T12:56:35Z", "name": "qos-demo", "labels": { "test": "qos" } }, "spec": { "replicas": 1, "selector": { "matchLabels": { "app": "nginx" } }, "template": { "metadata": { "creationTimestamp": "2020-09-21T12:56:35Z", "labels": { "app": "nginx" } }, "spec": { "containers": [ { "name": "nginx", "image": "nginx:latest", "resources": { "limits": { "cpu": "50m" } } } ]}}}}`), + Policy: []byte(`{ "apiVersion": "kyverno.io/v1", "kind": "ClusterPolicy", "metadata": { "name": "policy-qos" }, "spec": { "validationFailureAction": "enforce", "rules": [ { "name": "add-memory-limit", "match": { "resources": { "kinds": [ "apps/v1/deployment" ], "selector": { "matchLabels": { "test": "qos" } } } }, "mutate": { "overlay": { "spec": { "template": { "spec": { "containers": [ { "(name)": "*", "resources": { "limits": { "+(memory)": "300Mi", "+(cpu)": "100" } } } ] } } } } } }, { "name": "check-cpu-memory-limits", "match": { "resources": { "kinds": [ "apps/v1/Deployment" ], "selector": { "matchLabels": { "test": "qos" } } } }, "validate": { "message": "Resource limits are required for CPU and memory", "pattern": { "spec": { "template": { "spec": { "containers": [ { "(name)": "*", "resources": { "limits": { "memory": "?*", "cpu": "?*" } } } ] } } } } } } ] } }`), + areErrorsExpected: false, + }, } for i, tc := range tcs {