From 39ac9cac5992896da3ae0c0f49c832516d11b016 Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Mon, 11 Sep 2023 12:29:03 +0530 Subject: [PATCH 1/8] dummy commit to trigger ci --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index e837b7c16ebc..90a9667b5b9d 100644 --- a/README.md +++ b/README.md @@ -76,3 +76,4 @@ Made with [contributors-img](https://contrib.rocks). Copyright 2023, the Kyverno project. All rights reserved. Kyverno is licensed under the [Apache License 2.0](LICENSE). Kyverno is a [Cloud Native Computing Foundation (CNCF) Incubating project](https://www.cncf.io/projects/) and was contributed by [Nirmata](https://nirmata.com/?utm_source=github&utm_medium=repository). + From 923913235427a3165216a1e2a44e07da5ddeadee Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Fri, 8 Sep 2023 12:48:56 -0700 Subject: [PATCH 2/8] skip other checks if operations do not match Signed-off-by: Jim Bugwadia --- pkg/engine/utils/match.go | 73 ++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/pkg/engine/utils/match.go b/pkg/engine/utils/match.go index 239cabb5e218..d76681a8799f 100644 --- a/pkg/engine/utils/match.go +++ b/pkg/engine/utils/match.go @@ -59,8 +59,15 @@ func doesResourceMatchConditionBlock( subresource string, operation kyvernov1.AdmissionOperation, ) []error { - var errs []error + if len(conditionBlock.Operations) > 0 { + if !slices.Contains(conditionBlock.Operations, operation) { + // if operation does not match, return immediately + err := fmt.Errorf("operation does not match") + return []error{err} + } + } + var errs []error if len(conditionBlock.Kinds) > 0 { // Matching on ephemeralcontainers even when they are not explicitly specified for backward compatibility. if !matchutils.CheckKind(conditionBlock.Kinds, gvk, subresource, true) { @@ -130,12 +137,6 @@ func doesResourceMatchConditionBlock( } } - if len(conditionBlock.Operations) > 0 { - if !slices.Contains(conditionBlock.Operations, operation) { - errs = append(errs, fmt.Errorf("operation does not match")) - } - } - var userInfoErrors []error if len(userInfo.Roles) > 0 { if !datautils.SliceContains(userInfo.Roles, admissionInfo.Roles...) { @@ -165,24 +166,21 @@ func matchSubjects(ruleSubjects []rbacv1.Subject, userInfo authenticationv1.User // matchesResourceDescription checks if the resource matches resource description of the rule or not func MatchesResourceDescription( - resourceRef unstructured.Unstructured, - ruleRef kyvernov1.Rule, - admissionInfoRef kyvernov1beta1.RequestInfo, + resource unstructured.Unstructured, + rule kyvernov1.Rule, + admissionInfo kyvernov1beta1.RequestInfo, namespaceLabels map[string]string, policyNamespace string, gvk schema.GroupVersionKind, subresource string, operation kyvernov1.AdmissionOperation, ) error { - if resourceRef.Object == nil { + if resource.Object == nil { return fmt.Errorf("resource is empty") } - rule := ruleRef.DeepCopy() - resource := *resourceRef.DeepCopy() - admissionInfo := *admissionInfoRef.DeepCopy() var reasonsForFailure []error - if policyNamespace != "" && policyNamespace != resourceRef.GetNamespace() { + if policyNamespace != "" && policyNamespace != resource.GetNamespace() { return fmt.Errorf("policy and resource namespaces mismatch") } @@ -210,32 +208,35 @@ func MatchesResourceDescription( reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) } - if len(rule.ExcludeResources.Any) > 0 { - // exclude the object if ANY of the criteria match - for _, rer := range rule.ExcludeResources.Any { - reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) - } - } else if len(rule.ExcludeResources.All) > 0 { - // exclude the object if ALL the criteria match - excludedByAll := true - for _, rer := range rule.ExcludeResources.All { - // we got no errors inplying a resource did NOT exclude it - // "matchesResourceDescriptionExcludeHelper" returns errors if resource is excluded by a filter - if len(matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)) == 0 { - excludedByAll = false - break + // check exlude conditions only if match succeeds + if len(reasonsForFailure) == 0 { + if len(rule.ExcludeResources.Any) > 0 { + // exclude the object if ANY of the criteria match + for _, rer := range rule.ExcludeResources.Any { + reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) } + } else if len(rule.ExcludeResources.All) > 0 { + // exclude the object if ALL the criteria match + excludedByAll := true + for _, rer := range rule.ExcludeResources.All { + // we got no errors inplying a resource did NOT exclude it + // "matchesResourceDescriptionExcludeHelper" returns errors if resource is excluded by a filter + if len(matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)) == 0 { + excludedByAll = false + break + } + } + if excludedByAll { + reasonsForFailure = append(reasonsForFailure, fmt.Errorf("resource excluded since the combination of all criteria exclude it")) + } + } else { + rer := kyvernov1.ResourceFilter{UserInfo: rule.ExcludeResources.UserInfo, ResourceDescription: rule.ExcludeResources.ResourceDescription} + reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) } - if excludedByAll { - reasonsForFailure = append(reasonsForFailure, fmt.Errorf("resource excluded since the combination of all criteria exclude it")) - } - } else { - rer := kyvernov1.ResourceFilter{UserInfo: rule.ExcludeResources.UserInfo, ResourceDescription: rule.ExcludeResources.ResourceDescription} - reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) } // creating final error - errorMessage := fmt.Sprintf("rule %s not matched:", ruleRef.Name) + errorMessage := fmt.Sprintf("rule %s not matched:", rule.Name) for i, reasonForFailure := range reasonsForFailure { if reasonForFailure != nil { errorMessage += "\n " + fmt.Sprint(i+1) + ". " + reasonForFailure.Error() From 82ba157308337a074956e54fae255bcda2850e39 Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Tue, 12 Sep 2023 18:06:50 +0530 Subject: [PATCH 3/8] point cli test to nirmata policies fork --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2121caede7f8..13f97fbc754b 100644 --- a/Makefile +++ b/Makefile @@ -645,7 +645,7 @@ test-cli: test-cli-policies test-cli-local test-cli-local-mutate test-cli-local- .PHONY: test-cli-policies test-cli-policies: $(CLI_BIN) @echo Testing against branch $(TEST_GIT_BRANCH)... - @$(CLI_BIN) test https://github.com/kyverno/policies/$(TEST_GIT_BRANCH) + @$(CLI_BIN) test https://github.com/nirmata/policies/$(TEST_GIT_BRANCH) .PHONY: test-cli-local test-cli-local: $(CLI_BIN) From 6e8ae1512e66f49e13a72779f2c0a5cbf414d12c Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Wed, 13 Sep 2023 12:07:04 +0530 Subject: [PATCH 4/8] jmespath reuse interpreter 1.10-n4k changes for https://github.com/nirmata/kyverno/pull/24/files --- go.mod | 4 +- go.sum | 11 ++--- pkg/engine/jmespath/arithmetic_test.go | 12 ++--- pkg/engine/jmespath/functions_test.go | 62 +++++++++++++------------- pkg/engine/jmespath/interface.go | 19 ++++---- pkg/engine/jmespath/new.go | 31 ++++++++++--- pkg/engine/jmespath/new_test.go | 2 +- pkg/engine/jmespath/time_test.go | 12 ++--- 8 files changed, 86 insertions(+), 67 deletions(-) diff --git a/go.mod b/go.mod index 20b0c3b9c584..02dbac770562 100644 --- a/go.mod +++ b/go.mod @@ -42,7 +42,7 @@ require ( github.com/sigstore/k8s-manifest-sigstore v0.4.4 github.com/sigstore/sigstore v1.6.3 github.com/spf13/cobra v1.7.0 - github.com/stretchr/testify v1.8.2 + github.com/stretchr/testify v1.8.4 github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.40.0 go.opentelemetry.io/otel v1.14.0 @@ -329,6 +329,6 @@ require ( ) replace ( - github.com/jmespath/go-jmespath => github.com/kyverno/go-jmespath v0.4.1-0.20230204162932-3ee946b9433d + github.com/jmespath/go-jmespath => github.com/kyverno/go-jmespath v0.4.1-0.20230906134905-62fa64b71f91 github.com/sigstore/cosign => github.com/nirmata/cosign v1.13.2-0.20230726092108-615d4da057d8 ) diff --git a/go.sum b/go.sum index 0401ff6e3bdd..f4e842463edb 100644 --- a/go.sum +++ b/go.sum @@ -849,8 +849,6 @@ github.com/jingyugao/rowserrcheck v0.0.0-20210315055705-d907ca737bb1/go.mod h1:T github.com/jinzhu/copier v0.3.5 h1:GlvfUwHk62RokgqVNvYsku0TATCF7bAHVwEXoBh3iJg= github.com/jinzhu/copier v0.3.5/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg= github.com/jirfag/go-printf-func-name v0.0.0-20200119135958-7558a9eaa5af/go.mod h1:HEWGJkRDzjJY2sqdDwxccsGicWEf9BQOZsq2tV+xzM0= -github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= -github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/jmhodges/clock v1.2.0 h1:eq4kys+NI0PLngzaHEe7AmPT90XMGIEySD1JfV1PDIs= github.com/jmoiron/jsonq v0.0.0-20150511023944-e874b168d07e h1:ZZCvgaRDZg1gC9/1xrsgaJzQUCQgniKtw0xjWywWAOE= github.com/jmoiron/jsonq v0.0.0-20150511023944-e874b168d07e/go.mod h1:+rHyWac2R9oAZwFe1wGY2HBzFJJy++RHBg1cU23NkD8= @@ -914,8 +912,10 @@ github.com/kunwardeep/paralleltest v1.0.2/go.mod h1:ZPqNm1fVHPllh5LPVujzbVz1JN2G github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/kyoh86/exportloopref v0.1.8/go.mod h1:1tUcJeiioIs7VWe5gcOObrux3lb66+sBqGZrRkMwPgg= -github.com/kyverno/go-jmespath v0.4.1-0.20230204162932-3ee946b9433d h1:g63VNwOo6yYRY1n3mgF2ou4cjnwyonsIKqnbBM9pTRA= -github.com/kyverno/go-jmespath v0.4.1-0.20230204162932-3ee946b9433d/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= +github.com/kyverno/go-jmespath v0.4.1-0.20230906134905-62fa64b71f91 h1:n63aowZk61f65e6OJxuql8BS/hCv8LZxz/eCO4+2NfM= +github.com/kyverno/go-jmespath v0.4.1-0.20230906134905-62fa64b71f91/go.mod h1:yzDHaKovQy16rjN4kFnjF+IdNoN4p1ndw+va6+B8zUU= +github.com/kyverno/go-jmespath/internal/testify v1.5.2-0.20230630133209-945021c749d9 h1:lL311dF3a2aeNibJj8v+uhFU3XkvRHZmCtAdSPOrQYY= +github.com/kyverno/go-jmespath/internal/testify v1.5.2-0.20230630133209-945021c749d9/go.mod h1:XRxUGHIiCy1WYma1CdfdO1WOhIe8dLPTENaZr5D1ex4= github.com/ldez/gomoddirectives v0.2.1/go.mod h1:sGicqkRgBOg//JfpXwkB9Hj0X5RyJ7mlACM5B9f6Me4= github.com/ldez/tagliatelle v0.2.0/go.mod h1:8s6WJQwEYHbKZDsp/LjArytKOG8qaMrKQQ3mFukHs88= github.com/lensesio/tableprinter v0.0.0-20201125135848-89e81fc956e7 h1:k/1ku0yehLCPqERCHkIHMDqDg1R02AcCScRuHbamU3s= @@ -1344,8 +1344,9 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/subosito/gotenv v1.4.2 h1:X1TuBLAMDFbaTAChgCBLu3DU3UPyELpnF2jjJ2cz/S8= github.com/subosito/gotenv v1.4.2/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0= diff --git a/pkg/engine/jmespath/arithmetic_test.go b/pkg/engine/jmespath/arithmetic_test.go index 4b4c56fdd5d3..919bc8f76010 100644 --- a/pkg/engine/jmespath/arithmetic_test.go +++ b/pkg/engine/jmespath/arithmetic_test.go @@ -82,7 +82,7 @@ func Test_Add(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.test) + jp, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) result, err := jp.Search("") @@ -228,7 +228,7 @@ func Test_Sum(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.test) + jp, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) result, err := jp.Search("") @@ -327,7 +327,7 @@ func Test_Subtract(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.test) + jp, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) result, err := jp.Search("") @@ -426,7 +426,7 @@ func Test_Multiply(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.test) + jp, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) result, err := jp.Search("") @@ -588,7 +588,7 @@ func Test_Divide(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.test) + jp, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) result, err := jp.Search("") @@ -744,7 +744,7 @@ func Test_Modulo(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.test) + jp, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) result, err := jp.Search("") diff --git a/pkg/engine/jmespath/functions_test.go b/pkg/engine/jmespath/functions_test.go index 9eb31d547efb..4f8c5654abcd 100644 --- a/pkg/engine/jmespath/functions_test.go +++ b/pkg/engine/jmespath/functions_test.go @@ -11,7 +11,7 @@ import ( "gotest.tools/assert" ) -var cfg = config.NewDefaultConfiguration(false) +var jmespathInterface = newImplementation(config.NewDefaultConfiguration(false)) func Test_Compare(t *testing.T) { testCases := []struct { @@ -33,7 +33,7 @@ func Test_Compare(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -60,7 +60,7 @@ func Test_ParseJsonSerde(t *testing.T) { } for _, tc := range testCases { t.Run(tc, func(t *testing.T) { - jp, err := newJMESPath(cfg, fmt.Sprintf(`to_string(parse_json('%s'))`, tc)) + jp, err := jmespathInterface.Query(fmt.Sprintf(`to_string(parse_json('%s'))`, tc)) assert.NilError(t, err) result, err := jp.Search("") @@ -91,7 +91,7 @@ func Test_ParseJsonComplex(t *testing.T) { } for _, tc := range testCases { t.Run(tc.input, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.input) + jp, err := jmespathInterface.Query(tc.input) assert.NilError(t, err) result, err := jp.Search("") @@ -168,7 +168,7 @@ bar: null } for _, tc := range testCases { t.Run(tc.input, func(t *testing.T) { - jp, err := newJMESPath(cfg, fmt.Sprintf(`parse_yaml('%s')`, tc.input)) + jp, err := jmespathInterface.Query(fmt.Sprintf(`parse_yaml('%s')`, tc.input)) assert.NilError(t, err) result, err := jp.Search("") assert.NilError(t, err) @@ -197,7 +197,7 @@ func Test_EqualFold(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -234,7 +234,7 @@ func Test_Replace(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -248,7 +248,7 @@ func Test_Replace(t *testing.T) { } func Test_ReplaceAll(t *testing.T) { - jp, err := newJMESPath(cfg, "replace_all('Lorem ipsum dolor sit amet', 'ipsum', 'muspi')") + jp, err := jmespathInterface.Query("replace_all('Lorem ipsum dolor sit amet', 'ipsum', 'muspi')") assert.NilError(t, err) result, err := jp.Search("") @@ -279,7 +279,7 @@ func Test_ToUpper(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -312,7 +312,7 @@ func Test_ToLower(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -326,7 +326,7 @@ func Test_ToLower(t *testing.T) { } func Test_Trim(t *testing.T) { - jp, err := newJMESPath(cfg, "trim('¡¡¡Hello, Gophers!!!', '!¡')") + jp, err := jmespathInterface.Query("trim('¡¡¡Hello, Gophers!!!', '!¡')") assert.NilError(t, err) result, err := jp.Search("") @@ -397,7 +397,7 @@ func Test_TrimPrefix(t *testing.T) { } func Test_Split(t *testing.T) { - jp, err := newJMESPath(cfg, "split('Hello, Gophers', ', ')") + jp, err := jmespathInterface.Query("split('Hello, Gophers', ', ')") assert.NilError(t, err) result, err := jp.Search("") @@ -410,7 +410,7 @@ func Test_Split(t *testing.T) { } func Test_HasPrefix(t *testing.T) { - jp, err := newJMESPath(cfg, "starts_with('Gophers', 'Go')") + jp, err := jmespathInterface.Query("starts_with('Gophers', 'Go')") assert.NilError(t, err) result, err := jp.Search("") @@ -422,7 +422,7 @@ func Test_HasPrefix(t *testing.T) { } func Test_HasSuffix(t *testing.T) { - jp, err := newJMESPath(cfg, "ends_with('Amigo', 'go')") + jp, err := jmespathInterface.Query("ends_with('Amigo', 'go')") assert.NilError(t, err) result, err := jp.Search("") @@ -437,7 +437,7 @@ func Test_RegexMatch(t *testing.T) { data := make(map[string]interface{}) data["foo"] = "hgf'b1a2r'b12g" - query, err := newJMESPath(cfg, "regex_match('12.*', foo)") + query, err := jmespathInterface.Query("regex_match('12.*', foo)") assert.NilError(t, err) result, err := query.Search(data) @@ -449,7 +449,7 @@ func Test_RegexMatchWithNumber(t *testing.T) { data := make(map[string]interface{}) data["foo"] = -12.0 - query, err := newJMESPath(cfg, "regex_match('12.*', abs(foo))") + query, err := jmespathInterface.Query("regex_match('12.*', abs(foo))") assert.NilError(t, err) result, err := query.Search(data) @@ -461,7 +461,7 @@ func Test_PatternMatch(t *testing.T) { data := make(map[string]interface{}) data["foo"] = "prefix-foo" - query, err := newJMESPath(cfg, "pattern_match('prefix-*', foo)") + query, err := jmespathInterface.Query("pattern_match('prefix-*', foo)") assert.NilError(t, err) result, err := query.Search(data) @@ -473,7 +473,7 @@ func Test_PatternMatchWithNumber(t *testing.T) { data := make(map[string]interface{}) data["foo"] = -12.0 - query, err := newJMESPath(cfg, "pattern_match('12*', abs(foo))") + query, err := jmespathInterface.Query("pattern_match('12*', abs(foo))") assert.NilError(t, err) result, err := query.Search(data) @@ -500,7 +500,7 @@ func Test_RegexReplaceAll(t *testing.T) { var resource interface{} err := json.Unmarshal(resourceRaw, &resource) assert.NilError(t, err) - query, err := newJMESPath(cfg, `regex_replace_all('([Hh]e|G)l', spec.field, '${2}G')`) + query, err := jmespathInterface.Query(`regex_replace_all('([Hh]e|G)l', spec.field, '${2}G')`) assert.NilError(t, err) res, err := query.Search(resource) @@ -531,7 +531,7 @@ func Test_RegexReplaceAllLiteral(t *testing.T) { err := json.Unmarshal(resourceRaw, &resource) assert.NilError(t, err) - query, err := newJMESPath(cfg, `regex_replace_all_literal('[Hh]el?', spec.field, 'G')`) + query, err := jmespathInterface.Query(`regex_replace_all_literal('[Hh]el?', spec.field, 'G')`) assert.NilError(t, err) res, err := query.Search(resource) @@ -586,7 +586,7 @@ func Test_LabelMatch(t *testing.T) { err := json.Unmarshal(tc.resource, &resource) assert.NilError(t, err) - query, err := newJMESPath(cfg, "label_match(`"+tc.test+"`, metadata.labels)") + query, err := jmespathInterface.Query("label_match(`" + tc.test + "`, metadata.labels)") assert.NilError(t, err) res, err := query.Search(resource) @@ -630,7 +630,7 @@ func Test_JpToBoolean(t *testing.T) { } func Test_Base64Decode(t *testing.T) { - jp, err := newJMESPath(cfg, "base64_decode('SGVsbG8sIHdvcmxkIQ==')") + jp, err := jmespathInterface.Query("base64_decode('SGVsbG8sIHdvcmxkIQ==')") assert.NilError(t, err) result, err := jp.Search("") @@ -642,7 +642,7 @@ func Test_Base64Decode(t *testing.T) { } func Test_Base64Encode(t *testing.T) { - jp, err := newJMESPath(cfg, "base64_encode('Hello, world!')") + jp, err := jmespathInterface.Query("base64_encode('Hello, world!')") assert.NilError(t, err) result, err := jp.Search("") @@ -672,7 +672,7 @@ func Test_Base64Decode_Secret(t *testing.T) { err := json.Unmarshal(resourceRaw, &resource) assert.NilError(t, err) - query, err := newJMESPath(cfg, `base64_decode(data.example1)`) + query, err := jmespathInterface.Query(`base64_decode(data.example1)`) assert.NilError(t, err) res, err := query.Search(resource) @@ -747,7 +747,7 @@ func Test_PathCanonicalize(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -796,7 +796,7 @@ func Test_Truncate(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -833,7 +833,7 @@ func Test_SemverCompare(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -880,7 +880,7 @@ func Test_Items(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, "items(`"+tc.object+"`,`"+tc.keyName+"`,`"+tc.valName+"`)") + query, err := jmespathInterface.Query("items(`" + tc.object + "`,`" + tc.keyName + "`,`" + tc.valName + "`)") assert.NilError(t, err) res, err := query.Search("") @@ -931,7 +931,7 @@ func Test_ObjectFromLists(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, "object_from_lists(`"+tc.keys+"`,`"+tc.values+"`)") + query, err := jmespathInterface.Query("object_from_lists(`" + tc.keys + "`,`" + tc.values + "`)") assert.NilError(t, err) res, err := query.Search("") assert.NilError(t, err) @@ -1048,7 +1048,7 @@ ZDGRs55xuoeLDJ/ZRFf9bI+IaCUd1YrfYcHIl3G87Av+r49YVwqRDT0VDV7uLgqn } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") @@ -1464,7 +1464,7 @@ func Test_ImageNormalize(t *testing.T) { } for _, tc := range testCases { t.Run(tc.jmesPath, func(t *testing.T) { - jp, err := newJMESPath(cfg, tc.jmesPath) + jp, err := jmespathInterface.Query(tc.jmesPath) assert.NilError(t, err) result, err := jp.Search("") if tc.wantErr { diff --git a/pkg/engine/jmespath/interface.go b/pkg/engine/jmespath/interface.go index ef9e9395ca54..076d8f03b72a 100644 --- a/pkg/engine/jmespath/interface.go +++ b/pkg/engine/jmespath/interface.go @@ -1,6 +1,9 @@ package jmespath -import "github.com/kyverno/kyverno/pkg/config" +import ( + gojmespath "github.com/jmespath/go-jmespath" + "github.com/kyverno/kyverno/pkg/config" +) type Query interface { Search(interface{}) (interface{}, error) @@ -12,23 +15,17 @@ type Interface interface { } type implementation struct { - configuration config.Configuration + interpreter gojmespath.Interpreter } func New(configuration config.Configuration) Interface { - return implementation{ - configuration: configuration, - } + return newImplementation(configuration) } func (i implementation) Query(query string) (Query, error) { - return newJMESPath(i.configuration, query) + return newJMESPath(i.interpreter, query) } func (i implementation) Search(query string, data interface{}) (interface{}, error) { - if query, err := i.Query(query); err != nil { - return nil, err - } else { - return query.Search(data) - } + return newExecution(i.interpreter, query, data) } diff --git a/pkg/engine/jmespath/new.go b/pkg/engine/jmespath/new.go index cefec5896f2b..3dc4cba253ee 100644 --- a/pkg/engine/jmespath/new.go +++ b/pkg/engine/jmespath/new.go @@ -5,13 +5,34 @@ import ( "github.com/kyverno/kyverno/pkg/config" ) -func newJMESPath(configuration config.Configuration, query string) (*gojmespath.JMESPath, error) { - jp, err := gojmespath.Compile(query) +func newJMESPath(intr gojmespath.Interpreter, query string) (*gojmespath.JMESPath, error) { + parser := gojmespath.NewParser() + ast, err := parser.Parse(query) if err != nil { return nil, err } - for _, function := range GetFunctions(configuration) { - jp.Register(function.FunctionEntry) + + return gojmespath.NewJMESPath(ast, intr), nil +} + +func newImplementation(configuration config.Configuration) Interface { + i := gojmespath.NewInterpreter() + functions := GetFunctions(configuration) + for _, f := range functions { + i.Register(f.FunctionEntry) } - return jp, nil + + return implementation{ + interpreter: i, + } +} + +func newExecution(intr gojmespath.Interpreter, query string, data interface{}) (interface{}, error) { + parser := gojmespath.NewParser() + ast, err := parser.Parse(query) + if err != nil { + return nil, err + } + + return intr.Execute(ast, data) } diff --git a/pkg/engine/jmespath/new_test.go b/pkg/engine/jmespath/new_test.go index f6f44708dadc..9b4044e32b46 100644 --- a/pkg/engine/jmespath/new_test.go +++ b/pkg/engine/jmespath/new_test.go @@ -25,7 +25,7 @@ func TestNew(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := newJMESPath(cfg, tt.args.query) + _, err := jmespathInterface.Query(tt.args.query) if (err != nil) != tt.wantErr { t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/engine/jmespath/time_test.go b/pkg/engine/jmespath/time_test.go index 13a89080292a..c0935567527a 100644 --- a/pkg/engine/jmespath/time_test.go +++ b/pkg/engine/jmespath/time_test.go @@ -25,7 +25,7 @@ func Test_TimeSince(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, tc.test) + query, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) res, err := query.Search("") @@ -55,7 +55,7 @@ func Test_TimeToCron(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, tc.test) + query, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) res, err := query.Search("") @@ -85,7 +85,7 @@ func Test_TimeAdd(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, tc.test) + query, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) res, err := query.Search("") @@ -115,7 +115,7 @@ func Test_TimeParse(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, tc.test) + query, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) res, err := query.Search("") @@ -145,7 +145,7 @@ func Test_TimeUtc(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, tc.test) + query, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) res, err := query.Search("") @@ -171,7 +171,7 @@ func Test_TimeDiff(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - query, err := newJMESPath(cfg, tc.test) + query, err := jmespathInterface.Query(tc.test) assert.NilError(t, err) res, err := query.Search("") From 52ac4e7893c3aacd9f484ca451800b4162145544 Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Thu, 14 Sep 2023 20:20:51 +0530 Subject: [PATCH 5/8] optimize JSON context processing using in memory maps Signed-off-by: Jim Bugwadia --- pkg/engine/context/context.go | 172 ++++++++++++++++--------- pkg/engine/context/deferred_test.go | 8 +- pkg/engine/context/evaluate.go | 9 +- pkg/engine/context/evaluate_test.go | 21 +++ pkg/engine/context/loaders/variable.go | 5 +- pkg/engine/context/mock_context.go | 10 ++ pkg/engine/context/utils.go | 66 ++++++++-- pkg/engine/context/utils_test.go | 107 +++++++++++++++ pkg/engine/jsonutils/convert.go | 22 ++++ pkg/engine/jsonutils/traverse_test.go | 1 - pkg/engine/utils/foreach.go | 4 +- pkg/engine/utils/utils.go | 10 +- pkg/engine/variables/variables_test.go | 1 - pkg/engine/variables/vars.go | 35 ++--- pkg/engine/variables/vars_test.go | 69 ++++++---- pkg/validation/policy/validate.go | 2 +- 16 files changed, 395 insertions(+), 147 deletions(-) create mode 100644 pkg/engine/context/utils_test.go create mode 100644 pkg/engine/jsonutils/convert.go diff --git a/pkg/engine/context/context.go b/pkg/engine/context/context.go index 92b64abbf8bd..c162d7a5d67d 100644 --- a/pkg/engine/context/context.go +++ b/pkg/engine/context/context.go @@ -1,23 +1,28 @@ package context import ( - "encoding/json" "fmt" + "regexp" "strings" - "sync" - jsonpatch "github.com/evanphx/json-patch/v5" + jsoniter "github.com/json-iterator/go" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/engine/jmespath" + "github.com/kyverno/kyverno/pkg/engine/jsonutils" "github.com/kyverno/kyverno/pkg/logging" apiutils "github.com/kyverno/kyverno/pkg/utils/api" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ) -var logger = logging.WithName("context") +var ( + logger = logging.WithName("context") + json = jsoniter.ConfigCompatibleWithStandardLibrary + ReservedKeys = regexp.MustCompile(`request|serviceAccountName|serviceAccountNamespace|element|elementIndex|@|images|image|([a-z_0-9]+\()[^{}]`) +) // EvalInterface is used to query and inspect context data // TODO: move to contextapi to prevent circular dependencies @@ -25,6 +30,9 @@ type EvalInterface interface { // Query accepts a JMESPath expression and returns matching data Query(query string) (interface{}, error) + // Operation returns the admission operation i.e. "request.operation" + QueryOperation() string + // HasChanged accepts a JMESPath expression and compares matching data in the // request.object and request.oldObject context fields. If the data has changed // it return `true`. If the data has not changed it returns false. If either @@ -99,50 +107,69 @@ type Interface interface { EvalInterface - // AddJSON merges the json with context - addJSON(dataRaw []byte) error + // AddJSON merges the json map with context + addJSON(dataMap map[string]interface{}) error } // Context stores the data resources as JSON type context struct { jp jmespath.Interface - mutex sync.RWMutex - jsonRaw []byte - jsonRawCheckpoints [][]byte + jsonRaw map[string]interface{} + jsonRawCheckpoints []map[string]interface{} images map[string]map[string]apiutils.ImageInfo + operation kyvernov1.AdmissionOperation deferred DeferredLoaders } // NewContext returns a new context func NewContext(jp jmespath.Interface) Interface { - return NewContextFromRaw(jp, []byte(`{}`)) + return NewContextFromRaw(jp, map[string]interface{}{}) } // NewContextFromRaw returns a new context initialized with raw data -func NewContextFromRaw(jp jmespath.Interface, raw []byte) Interface { +func NewContextFromRaw(jp jmespath.Interface, raw map[string]interface{}) Interface { return &context{ jp: jp, jsonRaw: raw, - jsonRawCheckpoints: make([][]byte, 0), + jsonRawCheckpoints: make([]map[string]interface{}, 0), deferred: NewDeferredLoaders(), } } // addJSON merges json data -func (ctx *context) addJSON(dataRaw []byte) error { - ctx.mutex.Lock() - defer ctx.mutex.Unlock() - json, err := jsonpatch.MergeMergePatches(ctx.jsonRaw, dataRaw) - if err != nil { - return fmt.Errorf("failed to merge JSON data: %w", err) - } - ctx.jsonRaw = json +func (ctx *context) addJSON(dataMap map[string]interface{}) error { + mergeMaps(dataMap, ctx.jsonRaw) return nil } +func (ctx *context) QueryOperation() string { + if ctx.operation != "" { + return string(ctx.operation) + } + + if requestMap, val := ctx.jsonRaw["request"].(map[string]interface{}); val { + if op, val := requestMap["operation"].(string); val { + return op + } + } + + return "" +} + // AddRequest adds an admission request to context func (ctx *context) AddRequest(request admissionv1.AdmissionRequest) error { - return addToContext(ctx, request, "request") + // an AdmissionRequest needs to be marshaled / unmarshaled as + // JSON to properly convert types of runtime.RawExtension + mapObj, err := jsonutils.DocumentToUntyped(request) + if err != nil { + return err + } + if err := addToContext(ctx, mapObj, "request"); err != nil { + return err + } + + ctx.operation = kyvernov1.AdmissionOperation(request.Operation) + return nil } func (ctx *context) AddVariable(key string, value interface{}) error { @@ -193,12 +220,21 @@ func (ctx *context) SetTargetResource(data map[string]interface{}) error { // AddOperation data at path: request.operation func (ctx *context) AddOperation(data string) error { - return addToContext(ctx, data, "request", "operation") + if err := addToContext(ctx, data, "request", "operation"); err != nil { + return err + } + + ctx.operation = kyvernov1.AdmissionOperation(data) + return nil } // AddUserInfo adds userInfo at path request.userInfo func (ctx *context) AddUserInfo(userRequestInfo kyvernov1beta1.RequestInfo) error { - return addToContext(ctx, userRequestInfo, "request") + if data, err := toUnstructured(&userRequestInfo); err == nil { + return addToContext(ctx, data, "request") + } else { + return err + } } // AddServiceAccount removes prefix 'system:serviceaccount:' and namespace, then loads only SA name and SA namespace @@ -218,33 +254,14 @@ func (ctx *context) AddServiceAccount(userName string) error { saName = groups[1] saNamespace = groups[0] } - saNameObj := struct { - SA string `json:"serviceAccountName"` - }{ - SA: saName, - } - saNameRaw, err := json.Marshal(saNameObj) - if err != nil { - logger.Error(err, "failed to marshal the SA") - return err + data := map[string]interface{}{ + "serviceAccountName": saName, + "serviceAccountNamespace": saNamespace, } - if err := ctx.addJSON(saNameRaw); err != nil { + if err := ctx.addJSON(data); err != nil { return err } - saNsObj := struct { - SA string `json:"serviceAccountNamespace"` - }{ - SA: saNamespace, - } - saNsRaw, err := json.Marshal(saNsObj) - if err != nil { - logger.Error(err, "failed to marshal the SA namespace") - return err - } - if err := ctx.addJSON(saNsRaw); err != nil { - return err - } logger.V(4).Info("Adding service account", "service account name", saName, "service account namespace", saNamespace) return nil } @@ -260,8 +277,8 @@ func (ctx *context) AddElement(data interface{}, index, nesting int) error { data = map[string]interface{}{ "element": data, nestedElement: data, - "elementIndex": index, - nestedElementIndex: index, + "elementIndex": int64(index), + nestedElementIndex: int64(index), } return addToContext(ctx, data) } @@ -288,9 +305,33 @@ func (ctx *context) AddImageInfos(resource *unstructured.Unstructured, cfg confi return nil } ctx.images = images + utm, err := convertImagesToUntyped(images) + if err != nil { + return err + } + + logging.V(4).Info("updated image info", "images", utm) + return addToContext(ctx, utm, "images") +} - logging.V(4).Info("updated image info", "images", images) - return addToContext(ctx, images, "images") +func convertImagesToUntyped(images map[string]map[string]apiutils.ImageInfo) (map[string]interface{}, error) { + results := map[string]interface{}{} + for containerType, v := range images { + imgMap := map[string]interface{}{} + for containerName, imageInfo := range v { + img, err := toUnstructured(&imageInfo.ImageInfo) + if err != nil { + return nil, err + } + + img["jsonPointer"] = imageInfo.Pointer + imgMap[containerName] = img + } + + results[containerType] = imgMap + } + + return results, nil } func (ctx *context) GenerateCustomImageInfo(resource *unstructured.Unstructured, imageExtractorConfigs kyvernov1.ImageExtractorConfigs, cfg config.Configuration) (map[string]map[string]apiutils.ImageInfo, error) { @@ -314,13 +355,23 @@ func (ctx *context) ImageInfo() map[string]map[string]apiutils.ImageInfo { // Checkpoint creates a copy of the current internal state and // pushes it into a stack of stored states. func (ctx *context) Checkpoint() { - ctx.mutex.Lock() - defer ctx.mutex.Unlock() - jsonRawCheckpoint := make([]byte, len(ctx.jsonRaw)) - copy(jsonRawCheckpoint, ctx.jsonRaw) + jsonRawCheckpoint := ctx.copyContext(ctx.jsonRaw) ctx.jsonRawCheckpoints = append(ctx.jsonRawCheckpoints, jsonRawCheckpoint) } +func (ctx *context) copyContext(in map[string]interface{}) map[string]interface{} { + out := make(map[string]interface{}, len(in)) + for k, v := range in { + if ReservedKeys.MatchString(k) { + out[k] = v + } else { + out[k] = runtime.DeepCopyJSONValue(v) + } + } + + return out +} + // Restore sets the internal state to the last checkpoint, and removes the checkpoint. func (ctx *context) Restore() { ctx.reset(true) @@ -337,20 +388,19 @@ func (ctx *context) reset(restore bool) { } } -func (ctx *context) resetCheckpoint(removeCheckpoint bool) bool { - ctx.mutex.Lock() - defer ctx.mutex.Unlock() - +func (ctx *context) resetCheckpoint(restore bool) bool { if len(ctx.jsonRawCheckpoints) == 0 { return false } n := len(ctx.jsonRawCheckpoints) - 1 jsonRawCheckpoint := ctx.jsonRawCheckpoints[n] - ctx.jsonRaw = make([]byte, len(jsonRawCheckpoint)) - copy(ctx.jsonRaw, jsonRawCheckpoint) - if removeCheckpoint { + + if restore { ctx.jsonRawCheckpoints = ctx.jsonRawCheckpoints[:n] + ctx.jsonRaw = jsonRawCheckpoint + } else { + ctx.jsonRaw = ctx.copyContext(jsonRawCheckpoint) } return true diff --git a/pkg/engine/context/deferred_test.go b/pkg/engine/context/deferred_test.go index 7a08c8d76714..7a6f3fdc7f58 100644 --- a/pkg/engine/context/deferred_test.go +++ b/pkg/engine/context/deferred_test.go @@ -94,8 +94,8 @@ func TestDeferredLoaderMismatch(t *testing.T) { func newContext() *context { return &context{ jp: jp, - jsonRaw: []byte(`{}`), - jsonRawCheckpoints: make([][]byte, 0), + jsonRaw: make(map[string]interface{}), + jsonRawCheckpoints: make([]map[string]interface{}, 0), deferred: NewDeferredLoaders(), } } @@ -289,7 +289,7 @@ func TestDeferredCheckpointRestore(t *testing.T) { func TestDeferredForloop(t *testing.T) { ctx := newContext() - addDeferred(ctx, "value", -1) + addDeferred(ctx, "value", float64(-1)) ctx.Checkpoint() for i := 0; i < 5; i++ { @@ -298,7 +298,7 @@ func TestDeferredForloop(t *testing.T) { assert.Equal(t, float64(i-1), val) ctx.Reset() - mock, _ := addDeferred(ctx, "value", i) + mock, _ := addDeferred(ctx, "value", float64(i)) val, err = ctx.Query("value") assert.NilError(t, err) assert.Equal(t, float64(i), val) diff --git a/pkg/engine/context/evaluate.go b/pkg/engine/context/evaluate.go index 1624ea1b5915..a8a70096781b 100644 --- a/pkg/engine/context/evaluate.go +++ b/pkg/engine/context/evaluate.go @@ -1,7 +1,6 @@ package context import ( - "encoding/json" "fmt" "strings" @@ -25,13 +24,7 @@ func (ctx *context) Query(query string) (interface{}, error) { return nil, fmt.Errorf("incorrect query %s: %v", query, err) } // search - ctx.mutex.RLock() - defer ctx.mutex.RUnlock() - var data interface{} - if err := json.Unmarshal(ctx.jsonRaw, &data); err != nil { - return nil, fmt.Errorf("failed to unmarshal context: %w", err) - } - result, err := queryPath.Search(data) + result, err := queryPath.Search(ctx.jsonRaw) if err != nil { return nil, fmt.Errorf("JMESPath query failed: %w", err) } diff --git a/pkg/engine/context/evaluate_test.go b/pkg/engine/context/evaluate_test.go index c41c57e90dfd..6612d5e6bbe6 100644 --- a/pkg/engine/context/evaluate_test.go +++ b/pkg/engine/context/evaluate_test.go @@ -3,6 +3,7 @@ package context import ( "testing" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/stretchr/testify/assert" admissionv1 "k8s.io/api/admission/v1" ) @@ -65,3 +66,23 @@ func createTestContext(obj, oldObj string) Interface { ctx.AddRequest(request) return ctx } + +func TestQueryOperation(t *testing.T) { + ctx := createTestContext(`{"a": {"b": 1, "c": 2}, "d": 3}`, `{"a": {"b": 2, "c": 2}, "d": 4}`) + assert.Equal(t, ctx.QueryOperation(), "UPDATE") + request := admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + } + + err := ctx.AddRequest(request) + assert.Nil(t, err) + assert.Equal(t, ctx.QueryOperation(), "DELETE") + + err = ctx.AddOperation(string(kyvernov1.Connect)) + assert.Nil(t, err) + assert.Equal(t, ctx.QueryOperation(), "CONNECT") + + err = ctx.AddRequest(admissionv1.AdmissionRequest{}) + assert.Nil(t, err) + assert.Equal(t, ctx.QueryOperation(), "") +} diff --git a/pkg/engine/context/loaders/variable.go b/pkg/engine/context/loaders/variable.go index 355fdaa689b3..c4cc69c678e6 100644 --- a/pkg/engine/context/loaders/variable.go +++ b/pkg/engine/context/loaders/variable.go @@ -8,6 +8,7 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/jmespath" + "github.com/kyverno/kyverno/pkg/engine/jsonutils" "github.com/kyverno/kyverno/pkg/engine/variables" ) @@ -58,7 +59,7 @@ func (vl *variableLoader) loadVariable() (err error) { var defaultValue interface{} = nil if entry.Variable.Default != nil { - value, err := variables.DocumentToUntyped(entry.Variable.Default) + value, err := jsonutils.DocumentToUntyped(entry.Variable.Default) if err != nil { return fmt.Errorf("invalid default for variable %s", entry.Name) } @@ -71,7 +72,7 @@ func (vl *variableLoader) loadVariable() (err error) { var output interface{} = defaultValue if entry.Variable.Value != nil { - value, _ := variables.DocumentToUntyped(entry.Variable.Value) + value, _ := jsonutils.DocumentToUntyped(entry.Variable.Value) variable, err := variables.SubstituteAll(logger, ctx, value) if err != nil { return fmt.Errorf("failed to substitute variables in context entry %s %s: %v", entry.Name, entry.Variable.Value, err) diff --git a/pkg/engine/context/mock_context.go b/pkg/engine/context/mock_context.go index a3f66505fba8..6fdff695b93e 100644 --- a/pkg/engine/context/mock_context.go +++ b/pkg/engine/context/mock_context.go @@ -64,6 +64,16 @@ func (ctx *MockContext) Query(query string) (interface{}, error) { } } +func (ctx *MockContext) QueryOperation() string { + if op, err := ctx.Query("request.operation"); err != nil { + if op != nil { + return op.(string) + } + } + + return "" +} + func (ctx *MockContext) isVariableDefined(variable string) bool { for _, pattern := range ctx.getVariables() { if wildcard.Match(pattern, variable) { diff --git a/pkg/engine/context/utils.go b/pkg/engine/context/utils.go index a07d9c07f0ff..6724c69a22f9 100644 --- a/pkg/engine/context/utils.go +++ b/pkg/engine/context/utils.go @@ -1,16 +1,14 @@ package context import ( - "encoding/json" + "reflect" + + "k8s.io/apimachinery/pkg/runtime" ) // AddJSONObject merges json data -func AddJSONObject(ctx Interface, data interface{}) error { - jsonBytes, err := json.Marshal(data) - if err != nil { - return err - } - return ctx.addJSON(jsonBytes) +func AddJSONObject(ctx Interface, data map[string]interface{}) error { + return ctx.addJSON(data) } func AddResource(ctx Interface, dataRaw []byte) error { @@ -32,19 +30,61 @@ func AddOldResource(ctx Interface, dataRaw []byte) error { } func addToContext(ctx *context, data interface{}, tags ...string) error { - dataRaw, err := json.Marshal(push(data, tags...)) - if err != nil { - logger.Error(err, "failed to marshal the resource") + if v, err := convertStructs(data); err != nil { return err + } else { + dataRaw := push(v, tags...) + return ctx.addJSON(dataRaw) } - return ctx.addJSON(dataRaw) } -func push(data interface{}, tags ...string) interface{} { +// convertStructs converts structs, and pointers-to-structs, to map[string]interface{} +func convertStructs(value interface{}) (interface{}, error) { + if value != nil { + v := reflect.ValueOf(value) + if v.Kind() == reflect.Struct { + return toUnstructured(value) + } + + if v.Kind() == reflect.Ptr { + ptrVal := v.Elem() + if ptrVal.Kind() == reflect.Struct { + return toUnstructured(value) + } + } + } + + return value, nil +} + +func push(data interface{}, tags ...string) map[string]interface{} { for i := len(tags) - 1; i >= 0; i-- { data = map[string]interface{}{ tags[i]: data, } } - return data + + return data.(map[string]interface{}) +} + +// mergeMaps merges srcMap entries into destMap +func mergeMaps(srcMap, destMap map[string]interface{}) { + for k, v := range srcMap { + if nextSrcMap, ok := v.(map[string]interface{}); ok { + if nextDestMap, ok := destMap[k].(map[string]interface{}); ok { + mergeMaps(nextSrcMap, nextDestMap) + } else { + destMap[k] = nextSrcMap + } + } else { + destMap[k] = v + } + } +} + +// toUnstructured converts a struct with JSON tags to a map[string]interface{} +func toUnstructured(typedStruct interface{}) (map[string]interface{}, error) { + converter := runtime.DefaultUnstructuredConverter + u, err := converter.ToUnstructured(typedStruct) + return u, err } diff --git a/pkg/engine/context/utils_test.go b/pkg/engine/context/utils_test.go new file mode 100644 index 000000000000..f404bc7a95f3 --- /dev/null +++ b/pkg/engine/context/utils_test.go @@ -0,0 +1,107 @@ +package context + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeMaps(t *testing.T) { + map1 := map[string]interface{}{ + "strVal": "bar1", + "strVal2": "bar2", + "intVal": 2, + "arrayVal": []string{"1", "2", "3"}, + "mapVal": map[string]interface{}{ + "foo": "bar", + }, + "mapVal2": map[string]interface{}{ + "foo2": map[string]interface{}{ + "foo3": 3, + }, + }, + } + + map2 := map[string]interface{}{ + "strVal": "bar2", + "intVal": 3, + "intVal2": 3, + "arrayVal": []string{"1", "2", "3", "4"}, + "mapVal": map[string]interface{}{ + "foo1": "bar1", + "foo2": "bar2", + }, + } + + mergeMaps(map1, map2) + + assert.Equal(t, "bar1", map2["strVal"]) + assert.Equal(t, "bar2", map2["strVal2"]) + assert.Equal(t, 2, map2["intVal"]) + assert.Equal(t, 3, map2["intVal2"]) + assert.Equal(t, []string{"1", "2", "3"}, map2["arrayVal"]) + assert.Equal(t, map[string]interface{}{"foo": "bar", "foo1": "bar1", "foo2": "bar2"}, map2["mapVal"]) + assert.Equal(t, map1["mapVal2"], map2["mapVal2"]) + + requestObj := map[string]interface{}{ + "request": map[string]interface{}{ + "object": map[string]interface{}{ + "foo": "bar", + }, + }, + } + + ctxMap := map[string]interface{}{} + mergeMaps(requestObj, ctxMap) + + r := ctxMap["request"].(map[string]interface{}) + o := r["object"].(map[string]interface{}) + assert.Equal(t, o["foo"], "bar") + + requestObj2 := map[string]interface{}{ + "request": map[string]interface{}{ + "object": map[string]interface{}{ + "foo": "bar2", + "foo2": "bar2", + }, + }, + } + + mergeMaps(requestObj2, ctxMap) + r2 := ctxMap["request"].(map[string]interface{}) + o2 := r2["object"].(map[string]interface{}) + assert.Equal(t, "bar2", o2["foo"]) + assert.Equal(t, "bar2", o2["foo"]) + + request3 := map[string]interface{}{ + "request": map[string]interface{}{ + "userInfo": "user1", + }, + } + + mergeMaps(request3, ctxMap) + r3 := ctxMap["request"].(map[string]interface{}) + o3 := r3["object"].(map[string]interface{}) + assert.NotNil(t, o3) + assert.Equal(t, "bar2", o2["foo"]) + assert.Equal(t, "bar2", o2["foo"]) + assert.Equal(t, "user1", r3["userInfo"]) +} + +func TestStructToUntypedMap(t *testing.T) { + type SampleStuct struct { + Name string `json:"name"` + ID int32 `json:"identifier"` + } + + sample := &SampleStuct{ + Name: "user1", + ID: 12345, + } + + result, err := toUnstructured(sample) + assert.Nil(t, err) + + assert.Equal(t, "user1", result["name"]) + assert.Equal(t, int64(12345), result["identifier"]) +} diff --git a/pkg/engine/jsonutils/convert.go b/pkg/engine/jsonutils/convert.go new file mode 100644 index 000000000000..6b38dfc77679 --- /dev/null +++ b/pkg/engine/jsonutils/convert.go @@ -0,0 +1,22 @@ +package jsonutils + +import jsoniter "github.com/json-iterator/go" + +var json = jsoniter.ConfigCompatibleWithStandardLibrary + +// DocumentToUntyped converts a typed object to JSON data +// i.e. string, []interface{}, map[string]interface{} +func DocumentToUntyped(doc interface{}) (interface{}, error) { + jsonDoc, err := json.Marshal(doc) + if err != nil { + return nil, err + } + + var untyped interface{} + err = json.Unmarshal(jsonDoc, &untyped) + if err != nil { + return nil, err + } + + return untyped, nil +} diff --git a/pkg/engine/jsonutils/traverse_test.go b/pkg/engine/jsonutils/traverse_test.go index e32c13a1698c..b01defe54a9d 100644 --- a/pkg/engine/jsonutils/traverse_test.go +++ b/pkg/engine/jsonutils/traverse_test.go @@ -1,7 +1,6 @@ package jsonutils import ( - "encoding/json" "testing" "gotest.tools/assert" diff --git a/pkg/engine/utils/foreach.go b/pkg/engine/utils/foreach.go index 1c88048f6a1a..c0e49607809c 100644 --- a/pkg/engine/utils/foreach.go +++ b/pkg/engine/utils/foreach.go @@ -5,7 +5,7 @@ import ( engineapi "github.com/kyverno/kyverno/pkg/engine/api" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" - "github.com/kyverno/kyverno/pkg/engine/variables" + "github.com/kyverno/kyverno/pkg/engine/jsonutils" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -31,7 +31,7 @@ func InvertedElement(elements []interface{}) { } func AddElementToContext(ctx engineapi.PolicyContext, element interface{}, index, nesting int, elementScope *bool) error { - data, err := variables.DocumentToUntyped(element) + data, err := jsonutils.DocumentToUntyped(element) if err != nil { return err } diff --git a/pkg/engine/utils/utils.go b/pkg/engine/utils/utils.go index 1bab7ee793eb..3e0e1e03fe8e 100644 --- a/pkg/engine/utils/utils.go +++ b/pkg/engine/utils/utils.go @@ -14,8 +14,16 @@ import ( ) func IsDeleteRequest(ctx engineapi.PolicyContext) bool { + if ctx == nil { + return false + } + + if op := ctx.Operation(); string(op) != "" { + return op == kyvernov1.Delete + } + + // if the NewResource is empty, the request is a DELETE newResource := ctx.NewResource() - // if the OldResource is not empty, and the NewResource is empty, the request is a DELETE return IsEmptyUnstructured(&newResource) } diff --git a/pkg/engine/variables/variables_test.go b/pkg/engine/variables/variables_test.go index a59295c3d063..80bcfa78ade1 100644 --- a/pkg/engine/variables/variables_test.go +++ b/pkg/engine/variables/variables_test.go @@ -1,7 +1,6 @@ package variables import ( - "encoding/json" "reflect" "testing" diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index c8f2c28ddfa4..ea8187c383a6 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -1,7 +1,6 @@ package variables import ( - "encoding/json" "errors" "fmt" "path" @@ -9,6 +8,7 @@ import ( "github.com/go-logr/logr" gojmespath "github.com/jmespath/go-jmespath" + jsoniter "github.com/json-iterator/go" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/anchor" "github.com/kyverno/kyverno/pkg/engine/context" @@ -19,6 +19,8 @@ import ( "github.com/kyverno/kyverno/pkg/utils/jsonpointer" ) +var json = jsoniter.ConfigCompatibleWithStandardLibrary + // ReplaceAllVars replaces all variables with the value defined in the replacement function // This is used to avoid validation errors func ReplaceAllVars(src string, repl func(string) string) string { @@ -58,7 +60,7 @@ func SubstituteAll(log logr.Logger, ctx context.EvalInterface, document interfac } func SubstituteAllInPreconditions(log logr.Logger, ctx context.EvalInterface, document interface{}) (interface{}, error) { - untypedDoc, err := DocumentToUntyped(document) + untypedDoc, err := jsonUtils.DocumentToUntyped(document) if err != nil { return nil, err } @@ -66,7 +68,7 @@ func SubstituteAllInPreconditions(log logr.Logger, ctx context.EvalInterface, do } func SubstituteAllInType[T any](log logr.Logger, ctx context.EvalInterface, t *T) (*T, error) { - untyped, err := DocumentToUntyped(t) + untyped, err := jsonUtils.DocumentToUntyped(t) if err != nil { return nil, err } @@ -99,23 +101,6 @@ func SubstituteAllInRule(log logr.Logger, ctx context.EvalInterface, rule kyvern return *result, nil } -// DocumentToUntyped converts a typed object to JSON data i.e. -// string, []interface{}, map[string]interface{} -func DocumentToUntyped(doc interface{}) (interface{}, error) { - jsonDoc, err := json.Marshal(doc) - if err != nil { - return nil, err - } - - var untyped interface{} - err = json.Unmarshal(jsonDoc, &untyped) - if err != nil { - return nil, err - } - - return untyped, nil -} - func untypedToTyped[T any](untyped interface{}) (*T, error) { jsonRule, err := json.Marshal(untyped) if err != nil { @@ -184,7 +169,7 @@ func substituteAll(log logr.Logger, ctx context.EvalInterface, document interfac func SubstituteAllForceMutate(log logr.Logger, ctx context.Interface, typedRule kyvernov1.Rule) (_ kyvernov1.Rule, err error) { var rule interface{} - rule, err = DocumentToUntyped(typedRule) + rule, err = jsonUtils.DocumentToUntyped(typedRule) if err != nil { return kyvernov1.Rule{}, err } @@ -409,12 +394,12 @@ func IsDeleteRequest(ctx context.EvalInterface) bool { return false } - operation, err := ctx.Query("request.operation") - if err == nil && operation == "DELETE" { - return true + if op := ctx.QueryOperation(); op != "" { + return ctx.QueryOperation() == "DELETE" } - return false + operation, err := ctx.Query("request.operation") + return err == nil && operation == "DELETE" } func substituteVarInPattern(prefix, pattern, variable string, value interface{}) (string, error) { diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go index efea0182cef8..9da6959f6cbe 100644 --- a/pkg/engine/variables/vars_test.go +++ b/pkg/engine/variables/vars_test.go @@ -1,8 +1,6 @@ package variables import ( - "bytes" - "encoding/json" "fmt" "strings" "testing" @@ -208,11 +206,6 @@ func Test_subVars_with_JMESPath_At(t *testing.T) { }`) var err error - - expected := new(bytes.Buffer) - err = json.Compact(expected, expectedRaw) - assert.NilError(t, err) - var pattern, resource interface{} err = json.Unmarshal(patternMap, &pattern) assert.NilError(t, err) @@ -227,7 +220,7 @@ func Test_subVars_with_JMESPath_At(t *testing.T) { assert.NilError(t, err) out, err := json.Marshal(output) assert.NilError(t, err) - assert.Equal(t, string(out), expected.String()) + assert.Equal(t, string(out), compact(t, expectedRaw)) } func Test_subVars_withRegexMatch(t *testing.T) { @@ -268,10 +261,6 @@ func Test_subVars_withRegexMatch(t *testing.T) { var err error - expected := new(bytes.Buffer) - err = json.Compact(expected, expectedRaw) - assert.NilError(t, err) - var pattern, resource interface{} err = json.Unmarshal(patternMap, &pattern) assert.NilError(t, err) @@ -286,7 +275,7 @@ func Test_subVars_withRegexMatch(t *testing.T) { assert.NilError(t, err) out, err := json.Marshal(output) assert.NilError(t, err) - assert.Equal(t, string(out), expected.String()) + assert.Equal(t, string(out), compact(t, expectedRaw)) } func Test_subVars_withMerge(t *testing.T) { @@ -298,10 +287,6 @@ func Test_subVars_withMerge(t *testing.T) { var err error - expected := new(bytes.Buffer) - err = json.Compact(expected, expectedRaw) - assert.NilError(t, err) - var pattern, resource interface{} err = json.Unmarshal(patternMap, &pattern) assert.NilError(t, err) @@ -316,7 +301,17 @@ func Test_subVars_withMerge(t *testing.T) { assert.NilError(t, err) out, err := json.Marshal(output) assert.NilError(t, err) - assert.Equal(t, string(out), expected.String()) + assert.Equal(t, string(out), compact(t, expectedRaw)) +} + +func compact(t *testing.T, in []byte) string { + var tmp map[string]interface{} + err := json.Unmarshal(in, &tmp) + assert.NilError(t, err) + + out, err := json.Marshal(tmp) + assert.NilError(t, err) + return string(out) } func Test_subVars_withRegexReplaceAll(t *testing.T) { @@ -393,10 +388,12 @@ func Test_ReplacingPathWhenDeleting(t *testing.T) { var pattern interface{} var err error err = json.Unmarshal(patternRaw, &pattern) - if err != nil { - t.Error(err) - } - ctx := context.NewContextFromRaw(jp, resourceRaw) + assert.NilError(t, err) + + ctxMap, err := unmarshalToMap(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContextFromRaw(jp, ctxMap) assert.NilError(t, err) pattern, err = SubstituteAll(logr.Discard(), ctx, pattern) @@ -405,6 +402,15 @@ func Test_ReplacingPathWhenDeleting(t *testing.T) { assert.Equal(t, fmt.Sprintf("%v", pattern), "bar") } +func unmarshalToMap(jsonBytes []byte) (map[string]interface{}, error) { + var data map[string]interface{} + if err := json.Unmarshal(jsonBytes, &data); err != nil { + return nil, err + } + + return data, nil +} + func Test_ReplacingNestedVariableWhenDeleting(t *testing.T) { patternRaw := []byte(`"{{request.object.metadata.annotations.{{request.object.metadata.annotations.targetnew}}}}"`) @@ -428,12 +434,12 @@ func Test_ReplacingNestedVariableWhenDeleting(t *testing.T) { var pattern interface{} var err error err = json.Unmarshal(patternRaw, &pattern) - if err != nil { - t.Error(err) - } - ctx := context.NewContextFromRaw(jp, resourceRaw) assert.NilError(t, err) + ctxMap, err := unmarshalToMap(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContextFromRaw(jp, ctxMap) pattern, err = SubstituteAll(logr.Discard(), ctx, pattern) assert.NilError(t, err) @@ -633,7 +639,10 @@ func Test_variableSubstitution_array(t *testing.T) { err := json.Unmarshal(ruleRaw, &rule) assert.NilError(t, err) - ctx := context.NewContextFromRaw(jp, configmapRaw) + ctxMap, err := unmarshalToMap(configmapRaw) + assert.NilError(t, err) + + ctx := context.NewContextFromRaw(jp, ctxMap) context.AddResource(ctx, resourceRaw) vars, err := SubstituteAllInRule(logr.Discard(), ctx, rule) @@ -1171,7 +1180,11 @@ func Test_ReplacingEscpNestedVariableWhenDeleting(t *testing.T) { if err != nil { t.Error(err) } - ctx := context.NewContextFromRaw(jp, resourceRaw) + + ctxMap, err := unmarshalToMap(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContextFromRaw(jp, ctxMap) assert.NilError(t, err) pattern, err = SubstituteAll(logr.Discard(), ctx, pattern) diff --git a/pkg/validation/policy/validate.go b/pkg/validation/policy/validate.go index e6ea81d92789..fdcdc9e20e68 100644 --- a/pkg/validation/policy/validate.go +++ b/pkg/validation/policy/validate.go @@ -38,7 +38,7 @@ import ( ) var ( - allowedVariables = regexp.MustCompile(`request\.|serviceAccountName|serviceAccountNamespace|element|elementIndex|@|images\.|image\.|([a-z_0-9]+\()[^{}]`) + allowedVariables = enginecontext.ReservedKeys allowedVariablesBackground = regexp.MustCompile(`request\.|element|elementIndex|@|images\.|image\.|([a-z_0-9]+\()[^{}]`) allowedVariablesInTarget = regexp.MustCompile(`request\.|serviceAccountName|serviceAccountNamespace|element|elementIndex|@|images\.|image\.|target\.|([a-z_0-9]+\()[^{}]`) allowedVariablesBackgroundInTarget = regexp.MustCompile(`request\.|element|elementIndex|@|images\.|image\.|target\.|([a-z_0-9]+\()[^{}]`) From a5139641a1ad268a21b3a1d510b96d01bd1e33a1 Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Thu, 14 Sep 2023 20:21:11 +0530 Subject: [PATCH 6/8] Fix test case --- pkg/engine/validation_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index ae417e4ed48b..61e0e9d12fff 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -3199,9 +3199,8 @@ func Test_delete_ignore_pattern(t *testing.T) { assert.Equal(t, len(engineResponseCreate.PolicyResponse.Rules), 1) assert.Equal(t, engineResponseCreate.PolicyResponse.Rules[0].Status(), engineapi.RuleStatusFail) - policyContextDelete := NewPolicyContextWithJsonContext(kyvernov1.Create, ctx). - WithPolicy(&policy). - WithOldResource(*resourceUnstructured) + policyContextDelete := NewPolicyContextWithJsonContext(kyvernov1.Delete, ctx). + WithPolicy(&policy) engineResponseDelete := testValidate(context.TODO(), registryclient.NewOrDie(), policyContextDelete, cfg, nil) assert.Equal(t, len(engineResponseDelete.PolicyResponse.Rules), 0) From 222c90011318c08c29ec3a5e58d5e7b8a325cba8 Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Thu, 14 Sep 2023 22:57:48 +0530 Subject: [PATCH 7/8] Initial changes for configure webhook dynamically Signed-off-by: anushkamittal2001 Minor correction in comparison Signed-off-by: anushkamittal2001 Changes in mutatingWebhookConfiguration Signed-off-by: anushkamittal2001 Add minor change in variable declaration Signed-off-by: anushkamittal2001 Correct comparison in controller.go Signed-off-by: anushkamittal2001 --- pkg/controllers/webhook/controller.go | 79 +++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go index 2d7e880adae9..ab049d490fcb 100644 --- a/pkg/controllers/webhook/controller.go +++ b/pkg/controllers/webhook/controller.go @@ -627,6 +627,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config return nil, err } c.recordPolicyState(config.MutatingWebhookConfigurationName, policies...) + operationStatusMap := getOperationStatusMap() for _, p := range policies { spec := p.GetSpec() if spec.HasMutate() || spec.HasVerifyImages() { @@ -636,7 +637,10 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config c.mergeWebhook(fail, p, false) } } + rules := p.GetSpec().Rules + operationStatusMap = analyseOperationStatusMapMutatingWebhookConfiguration(rules, operationStatusMap) } + operationReq := getMinimumOperations(operationStatusMap) webhookCfg := config.WebhookConfig{} webhookCfgs := cfg.GetWebhooks() if len(webhookCfgs) > 0 { @@ -649,7 +653,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config admissionregistrationv1.MutatingWebhook{ Name: config.MutatingWebhookName + "-ignore", ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/ignore"), - Rules: ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update), + Rules: ignore.buildRulesWithOperations(operationReq...), FailurePolicy: &ignore.failurePolicy, SideEffects: &noneOnDryRun, AdmissionReviewVersions: []string{"v1"}, @@ -667,7 +671,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config admissionregistrationv1.MutatingWebhook{ Name: config.MutatingWebhookName + "-fail", ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/fail"), - Rules: fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update), + Rules: fail.buildRulesWithOperations(operationReq...), FailurePolicy: &fail.failurePolicy, SideEffects: &noneOnDryRun, AdmissionReviewVersions: []string{"v1"}, @@ -736,6 +740,68 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(cfg conf nil } +func getOperationStatusMap() map[string]bool { + operationStatusMap := make(map[string]bool) + operationStatusMap["CREATE"] = false + operationStatusMap["UPDATE"] = false + operationStatusMap["DELETE"] = false + operationStatusMap["CONNECT"] = false + return operationStatusMap +} + +func analyseOperationStatusMapValidatingWebhookConfiguration(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool { + for _, r := range rules { + if r.MatchResources.ResourceDescription.Operations != nil { + for _, o := range r.MatchResources.ResourceDescription.Operations { + operationStatusMap[string(o)] = true + } + } + if r.ExcludeResources.ResourceDescription.Operations != nil { + for _, o := range r.MatchResources.ResourceDescription.Operations { + operationStatusMap[string(o)] = true + } + } + if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil { + operationStatusMap["CREATE"] = true + operationStatusMap["UPDATE"] = true + operationStatusMap["DELETE"] = true + operationStatusMap["CONNECT"] = true + } + } + return operationStatusMap +} + +func analyseOperationStatusMapMutatingWebhookConfiguration(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool { + for _, r := range rules { + if r.MatchResources.ResourceDescription.Operations != nil { + for _, o := range r.MatchResources.ResourceDescription.Operations { + operationStatusMap[string(o)] = true + } + } + if r.ExcludeResources.ResourceDescription.Operations != nil { + for _, o := range r.MatchResources.ResourceDescription.Operations { + operationStatusMap[string(o)] = true + } + } + if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil { + operationStatusMap["CREATE"] = true + operationStatusMap["UPDATE"] = true + } + } + return operationStatusMap +} + +func getMinimumOperations(operationStatusMap map[string]bool) []admissionregistrationv1.OperationType { + operationReq := make([]admissionregistrationv1.OperationType, 0, 4) + for k, v := range operationStatusMap { + if v { + var oper admissionregistrationv1.OperationType = admissionregistrationv1.OperationType(k) + operationReq = append(operationReq, oper) + } + } + return operationReq +} + func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Configuration, caBundle []byte) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) { result := admissionregistrationv1.ValidatingWebhookConfiguration{ ObjectMeta: objectMeta(config.ValidatingWebhookConfigurationName, cfg.GetWebhookAnnotations(), c.buildOwner()...), @@ -749,6 +815,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf return nil, err } c.recordPolicyState(config.ValidatingWebhookConfigurationName, policies...) + operationStatusMap := getOperationStatusMap() for _, p := range policies { spec := p.GetSpec() if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasVerifyImageChecks() || spec.HasVerifyManifests() { @@ -758,7 +825,11 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf c.mergeWebhook(fail, p, true) } } + rules := p.GetSpec().Rules + operationStatusMap = analyseOperationStatusMapValidatingWebhookConfiguration(rules, operationStatusMap) } + operationReq := getMinimumOperations(operationStatusMap) + webhookCfg := config.WebhookConfig{} webhookCfgs := cfg.GetWebhooks() if len(webhookCfgs) > 0 { @@ -775,7 +846,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf admissionregistrationv1.ValidatingWebhook{ Name: config.ValidatingWebhookName + "-ignore", ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/ignore"), - Rules: ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect), + Rules: ignore.buildRulesWithOperations(operationReq...), FailurePolicy: &ignore.failurePolicy, SideEffects: sideEffects, AdmissionReviewVersions: []string{"v1"}, @@ -792,7 +863,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf admissionregistrationv1.ValidatingWebhook{ Name: config.ValidatingWebhookName + "-fail", ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/fail"), - Rules: fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect), + Rules: fail.buildRulesWithOperations(operationReq...), FailurePolicy: &fail.failurePolicy, SideEffects: sideEffects, AdmissionReviewVersions: []string{"v1"}, From 419e903b3846ccadbd1fbf9d846fa454015d47d6 Mon Sep 17 00:00:00 2001 From: Parikshit Samant Date: Thu, 14 Sep 2023 23:54:05 +0530 Subject: [PATCH 8/8] consider generate rules for delete, minor cleanup --- pkg/controllers/webhook/controller.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go index ab049d490fcb..bb7b179d6610 100644 --- a/pkg/controllers/webhook/controller.go +++ b/pkg/controllers/webhook/controller.go @@ -51,6 +51,10 @@ const ( IdleDeadline = tickerInterval * 10 maxRetries = 10 tickerInterval = 10 * time.Second + webhookCreate = "CREATE" + webhookUpdate = "UPDATE" + webhookDelete = "DELETE" + webhookConnect = "CONNECT" ) var ( @@ -742,10 +746,10 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(cfg conf func getOperationStatusMap() map[string]bool { operationStatusMap := make(map[string]bool) - operationStatusMap["CREATE"] = false - operationStatusMap["UPDATE"] = false - operationStatusMap["DELETE"] = false - operationStatusMap["CONNECT"] = false + operationStatusMap[webhookCreate] = false + operationStatusMap[webhookUpdate] = false + operationStatusMap[webhookDelete] = false + operationStatusMap[webhookConnect] = false return operationStatusMap } @@ -762,10 +766,12 @@ func analyseOperationStatusMapValidatingWebhookConfiguration(rules []kyvernov1.R } } if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil { - operationStatusMap["CREATE"] = true - operationStatusMap["UPDATE"] = true - operationStatusMap["DELETE"] = true - operationStatusMap["CONNECT"] = true + operationStatusMap[webhookCreate] = true + operationStatusMap[webhookUpdate] = true + + if r.HasGenerate() { + operationStatusMap[webhookDelete] = true + } } } return operationStatusMap @@ -784,8 +790,8 @@ func analyseOperationStatusMapMutatingWebhookConfiguration(rules []kyvernov1.Rul } } if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil { - operationStatusMap["CREATE"] = true - operationStatusMap["UPDATE"] = true + operationStatusMap[webhookCreate] = true + operationStatusMap[webhookUpdate] = true } } return operationStatusMap