From cb364904b6897a476417cc4f735b79b04f627556 Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Mon, 3 May 2021 08:20:22 -0400 Subject: [PATCH 1/8] Improved error handling for test command Signed-off-by: Trey Dockendorf --- Makefile | 1 + pkg/kyverno/test/command.go | 52 ++++++++++++++++-------------------- test/cli/test/policy.yaml | 35 ++++++++++++++++++++++++ test/cli/test/resources.yaml | 21 +++++++++++++++ test/cli/test/test.yaml | 14 ++++++++++ 5 files changed, 94 insertions(+), 29 deletions(-) create mode 100644 test/cli/test/policy.yaml create mode 100644 test/cli/test/resources.yaml create mode 100644 test/cli/test/test.yaml diff --git a/Makefile b/Makefile index 9c1aa99e37..e834cc6116 100644 --- a/Makefile +++ b/Makefile @@ -179,6 +179,7 @@ test-e2e: run_testcmd_policy: go build -o kyvernoctl cmd/cli/kubectl-kyverno/main.go ./kyvernoctl test https://github.com/kyverno/policies/main + ./kyvernoctl test ./test/cli/test # godownloader create downloading script for kyverno-cli godownloader: diff --git a/pkg/kyverno/test/command.go b/pkg/kyverno/test/command.go index fa519d9e81..28346aaa37 100644 --- a/pkg/kyverno/test/command.go +++ b/pkg/kyverno/test/command.go @@ -145,83 +145,77 @@ func testCommandExecute(dirPath []string, valuesFile string, fileName string) (r sort.Strings(policyYamls) for _, yamlFilePath := range policyYamls { file, err := fs.Open(yamlFilePath) + if err != nil { + errors = append(errors, sanitizederror.NewWithError("Error: failed to open file", err)) + continue + } if strings.Contains(file.Name(), fileName) { testYamlCount++ policyresoucePath := strings.Trim(yamlFilePath, fileName) bytes, err := ioutil.ReadAll(file) if err != nil { - sanitizederror.NewWithError("Error: failed to read file", err) + errors = append(errors, sanitizederror.NewWithError("Error: failed to read file", err)) continue } policyBytes, err := yaml.ToJSON(bytes) if err != nil { - sanitizederror.NewWithError("failed to convert to JSON", err) + errors = append(errors, sanitizederror.NewWithError("failed to convert to JSON", err)) continue } if err := applyPoliciesFromPath(fs, policyBytes, valuesFile, true, policyresoucePath, rc); err != nil { return rc, sanitizederror.NewWithError("failed to apply test command", err) } } - if err != nil { - sanitizederror.NewWithError("Error: failed to open file", err) - continue - } + } + if testYamlCount == 0 { + fmt.Printf("\n No test yamls available \n") } } else { path := filepath.Clean(dirPath[0]) - if err != nil { - errors = append(errors, err) - } - err := getLocalDirTestFiles(fs, path, fileName, valuesFile, rc, testYamlCount) - if err != nil { - errors = append(errors, err) - } - if len(errors) > 0 && log.Log.V(1).Enabled() { - fmt.Printf("ignoring errors: \n") - for _, e := range errors { - fmt.Printf(" %v \n", e.Error()) - } + errors = getLocalDirTestFiles(fs, path, fileName, valuesFile, rc) + } + if len(errors) > 0 && log.Log.V(1).Enabled() { + fmt.Printf("ignoring errors: \n") + for _, e := range errors { + fmt.Printf(" %v \n", e.Error()) } } if rc.fail > 0 { os.Exit(1) } - if testYamlCount == 0 { - fmt.Printf("\n No test yamls available \n") - } os.Exit(0) return rc, nil } -func getLocalDirTestFiles(fs billy.Filesystem, path, fileName, valuesFile string, rc *resultCounts, testYamlCount int) error { +func getLocalDirTestFiles(fs billy.Filesystem, path, fileName, valuesFile string, rc *resultCounts) []error { + var errors []error files, err := ioutil.ReadDir(path) if err != nil { - return fmt.Errorf("failed to read %v: %v", path, err.Error()) + return []error{fmt.Errorf("failed to read %v: %v", path, err.Error())} } for _, file := range files { if file.IsDir() { - getLocalDirTestFiles(fs, filepath.Join(path, file.Name()), fileName, valuesFile, rc, testYamlCount) + getLocalDirTestFiles(fs, filepath.Join(path, file.Name()), fileName, valuesFile, rc) continue } if strings.Contains(file.Name(), fileName) { - testYamlCount++ yamlFile, err := ioutil.ReadFile(filepath.Join(path, file.Name())) if err != nil { - sanitizederror.NewWithError("unable to read yaml", err) + errors = append(errors, sanitizederror.NewWithError("unable to read yaml", err)) continue } valuesBytes, err := yaml.ToJSON(yamlFile) if err != nil { - sanitizederror.NewWithError("failed to convert json", err) + errors = append(errors, sanitizederror.NewWithError("failed to convert json", err)) continue } if err := applyPoliciesFromPath(fs, valuesBytes, valuesFile, false, path, rc); err != nil { - sanitizederror.NewWithError("failed to apply test command", err) + errors = append(errors, sanitizederror.NewWithError(fmt.Sprintf("failed to apply test command from file %s", file.Name()), err)) continue } } } - return nil + return errors } func buildPolicyResults(resps []*response.EngineResponse) map[string][]interface{} { diff --git a/test/cli/test/policy.yaml b/test/cli/test/policy.yaml new file mode 100644 index 0000000000..81c9337d55 --- /dev/null +++ b/test/cli/test/policy.yaml @@ -0,0 +1,35 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: disallow-latest-tag + annotations: + policies.kyverno.io/category: Best Practices + 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: audit + rules: + - name: require-image-tag + match: + resources: + kinds: + - Pod + validate: + message: "An image tag is required." + pattern: + spec: + containers: + - image: "*:*" + - name: validate-image-tag + match: + resources: + kinds: + - Pod + validate: + message: "Using a mutable image tag e.g. 'latest' is not allowed." + pattern: + spec: + containers: + - image: "!*:latest" diff --git a/test/cli/test/resources.yaml b/test/cli/test/resources.yaml new file mode 100644 index 0000000000..92ae8d4373 --- /dev/null +++ b/test/cli/test/resources.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: Pod +metadata: + name: test-web + labels: + app: app +spec: + containers: + - name: nginx + image: nginx:latest +--- +apiVersion: v1 +kind: Pod +metadata: + name: test-app + labels: + app: app +spec: + containers: + - name: nginx + image: nginx:1.12 diff --git a/test/cli/test/test.yaml b/test/cli/test/test.yaml new file mode 100644 index 0000000000..f1063ead4d --- /dev/null +++ b/test/cli/test/test.yaml @@ -0,0 +1,14 @@ +name: test +policies: + - policy.yaml +resources: + - resources.yaml +results: + - policy: disallow-latest-tag + rule: validate-image-tag + resource: test-web + status: fail + - policy: disallow-latest-tag + rule: validate-image-tag + resource: test-app + status: pass From bb626ed633ecd993de338e87bb9a189a1b1affd7 Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Mon, 3 May 2021 08:55:04 -0400 Subject: [PATCH 2/8] Print 'Not found' if test defined is not found Signed-off-by: Trey Dockendorf --- pkg/kyverno/test/command.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/kyverno/test/command.go b/pkg/kyverno/test/command.go index 28346aaa37..eeaf2eead8 100644 --- a/pkg/kyverno/test/command.go +++ b/pkg/kyverno/test/command.go @@ -355,6 +355,7 @@ func printTestResult(resps map[string][]interface{}, testResults []TestResults, printer := tableprinter.New(os.Stdout) table := []*Table{} boldRed := color.New(color.FgRed).Add(color.Bold) + boldYellow := color.New(color.FgYellow).Add(color.Bold) boldFgCyan := color.New(color.FgCyan).Add(color.Bold) for i, v := range testResults { res := new(Table) @@ -368,7 +369,7 @@ func printTestResult(resps map[string][]interface{}, testResults []TestResults, } var r []ReportResult json.Unmarshal(valuesBytes, &r) - res.Result = boldRed.Sprintf("Fail") + res.Result = boldYellow.Sprintf("Not found") if len(r) != 0 { var resource TestResults for _, testRes := range r { @@ -381,6 +382,7 @@ func printTestResult(resps map[string][]interface{}, testResults []TestResults, res.Result = "Pass" rc.pass++ } else { + res.Result = boldRed.Sprintf("Fail") rc.fail++ } } From 6cb26d31341c20d2b911a3c97419565bdc0c538e Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Mon, 3 May 2021 15:35:35 -0400 Subject: [PATCH 3/8] Fix path when loading variables during directory tests Signed-off-by: Trey Dockendorf --- pkg/kyverno/common/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kyverno/common/common.go b/pkg/kyverno/common/common.go index 62f09775bc..1a3188d79c 100644 --- a/pkg/kyverno/common/common.go +++ b/pkg/kyverno/common/common.go @@ -334,7 +334,7 @@ func GetVariable(variablesString, valuesFile string, fs billy.Filesystem, isGit } yamlFile, err = ioutil.ReadAll(filep) } else { - yamlFile, err = ioutil.ReadFile(valuesFile) + yamlFile, err = ioutil.ReadFile(filepath.Join(policyresoucePath, valuesFile)) } if err != nil { From d7886bddc9b47fb08858fcd69a14370b0e014198 Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Mon, 3 May 2021 19:54:19 -0400 Subject: [PATCH 4/8] Fix tests with variables to use Mock store Signed-off-by: Trey Dockendorf --- pkg/engine/jsonContext.go | 3 +++ pkg/kyverno/test/command.go | 2 ++ 2 files changed, 5 insertions(+) diff --git a/pkg/engine/jsonContext.go b/pkg/engine/jsonContext.go index 5429cd88aa..144531c475 100644 --- a/pkg/engine/jsonContext.go +++ b/pkg/engine/jsonContext.go @@ -27,6 +27,9 @@ func LoadContext(logger logr.Logger, contextEntries []kyverno.ContextEntry, resC policyName := ctx.Policy.Name if store.GetMock() { rule := store.GetPolicyRuleFromContext(policyName, ruleName) + if len(rule.Values) == 0 { + return errors.New(fmt.Sprintf("No values found for policy %s rule %s", policyName, ruleName)) + } variables := rule.Values for key, value := range variables { diff --git a/pkg/kyverno/test/command.go b/pkg/kyverno/test/command.go index eeaf2eead8..f8b1641433 100644 --- a/pkg/kyverno/test/command.go +++ b/pkg/kyverno/test/command.go @@ -22,6 +22,7 @@ import ( "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/kyverno/common" sanitizederror "github.com/kyverno/kyverno/pkg/kyverno/sanitizedError" + "github.com/kyverno/kyverno/pkg/kyverno/store" "github.com/kyverno/kyverno/pkg/openapi" policy2 "github.com/kyverno/kyverno/pkg/policy" "github.com/kyverno/kyverno/pkg/policyreport" @@ -263,6 +264,7 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, valuesFile s var dClient *client.Client values := &Test{} var variablesString string + store.SetMock(true) if err := json.Unmarshal(policyBytes, values); err != nil { return sanitizederror.NewWithError("failed to decode yaml", err) From 00b8da9219f8f7446177ec42c07879489f63eb37 Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Tue, 4 May 2021 09:39:31 -0400 Subject: [PATCH 5/8] Ensure JSON strings are properly escaped Ensure multiple policies can be tested with variables in same files Signed-off-by: Trey Dockendorf --- pkg/common/common.go | 2 +- pkg/kyverno/test/command.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/common/common.go b/pkg/common/common.go index b46f35a4de..a39e915d4d 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -103,7 +103,7 @@ func VariableToJSON(key, value string) []byte { } } - midString := fmt.Sprintf(`"%s"`, value) + midString := fmt.Sprintf(`"%s"`, strings.Replace(value, `"`, `\"`, -1)) finalString := startString + midString + endString var jsonData = []byte(finalString) return jsonData diff --git a/pkg/kyverno/test/command.go b/pkg/kyverno/test/command.go index f8b1641433..e848a9dac3 100644 --- a/pkg/kyverno/test/command.go +++ b/pkg/kyverno/test/command.go @@ -329,6 +329,18 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, valuesFile s continue } for _, resource := range resources { + var resourcePolicy string + for polName, values := range valuesMap { + for resName := range values { + if resName == resource.GetName() { + resourcePolicy = polName + } + } + } + if resourcePolicy != policy.GetName() { + log.Log.V(3).Info(fmt.Sprintf("Skipping resource, policy names do not match %s != %s", resourcePolicy, policy.GetName())) + continue + } thisPolicyResourceValues := make(map[string]string) if len(valuesMap[policy.GetName()]) != 0 && !reflect.DeepEqual(valuesMap[policy.GetName()][resource.GetName()], Resource{}) { thisPolicyResourceValues = valuesMap[policy.GetName()][resource.GetName()].Values From db4fec0eebb1055076ccc4bb942d59369140c62f Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Tue, 4 May 2021 10:18:24 -0400 Subject: [PATCH 6/8] Add additional e2e tests for 'kyverno test' Signed-off-by: Trey Dockendorf --- test/cli/test/{ => simple}/policy.yaml | 0 test/cli/test/{ => simple}/resources.yaml | 0 test/cli/test/{ => simple}/test.yaml | 2 +- test/cli/test/variables/cm-array-example.yaml | 25 +++++++++++ .../test/variables/cm-variable-example.yaml | 21 +++++++++ test/cli/test/variables/resources.yaml | 43 +++++++++++++++++++ test/cli/test/variables/test.yaml | 24 +++++++++++ test/cli/test/variables/variables.yaml | 25 +++++++++++ 8 files changed, 139 insertions(+), 1 deletion(-) rename test/cli/test/{ => simple}/policy.yaml (100%) rename test/cli/test/{ => simple}/resources.yaml (100%) rename test/cli/test/{ => simple}/test.yaml (93%) create mode 100644 test/cli/test/variables/cm-array-example.yaml create mode 100644 test/cli/test/variables/cm-variable-example.yaml create mode 100644 test/cli/test/variables/resources.yaml create mode 100644 test/cli/test/variables/test.yaml create mode 100644 test/cli/test/variables/variables.yaml diff --git a/test/cli/test/policy.yaml b/test/cli/test/simple/policy.yaml similarity index 100% rename from test/cli/test/policy.yaml rename to test/cli/test/simple/policy.yaml diff --git a/test/cli/test/resources.yaml b/test/cli/test/simple/resources.yaml similarity index 100% rename from test/cli/test/resources.yaml rename to test/cli/test/simple/resources.yaml diff --git a/test/cli/test/test.yaml b/test/cli/test/simple/test.yaml similarity index 93% rename from test/cli/test/test.yaml rename to test/cli/test/simple/test.yaml index f1063ead4d..674f1b00ea 100644 --- a/test/cli/test/test.yaml +++ b/test/cli/test/simple/test.yaml @@ -1,4 +1,4 @@ -name: test +name: test-simple policies: - policy.yaml resources: diff --git a/test/cli/test/variables/cm-array-example.yaml b/test/cli/test/variables/cm-array-example.yaml new file mode 100644 index 0000000000..7415720b80 --- /dev/null +++ b/test/cli/test/variables/cm-array-example.yaml @@ -0,0 +1,25 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: cm-array-example +spec: + validationFailureAction: enforce + background: false + rules: + - name: validate-role-annotation + context: + - name: roles-dictionary + configMap: + name: roles-dictionary + namespace: default + match: + resources: + kinds: + - Pod + validate: + message: "The role {{ request.object.metadata.annotations.role }} is not in the allowed list of roles: {{ \"roles-dictionary\".data.\"allowed-roles\" }}." + deny: + conditions: + - key: "{{ request.object.metadata.annotations.role }}" + operator: NotIn + value: "{{ \"roles-dictionary\".data.\"allowed-roles\" }}" diff --git a/test/cli/test/variables/cm-variable-example.yaml b/test/cli/test/variables/cm-variable-example.yaml new file mode 100644 index 0000000000..7055a66d2d --- /dev/null +++ b/test/cli/test/variables/cm-variable-example.yaml @@ -0,0 +1,21 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: cm-variable-example +spec: + rules: + - name: example-configmap-lookup + context: + - name: dictionary + configMap: + name: some-config-map + namespace: some-namespace + match: + resources: + kinds: + - Pod + validate: + pattern: + metadata: + labels: + my-environment-name: "{{dictionary.data.env}}" diff --git a/test/cli/test/variables/resources.yaml b/test/cli/test/variables/resources.yaml new file mode 100644 index 0000000000..a96522f5f2 --- /dev/null +++ b/test/cli/test/variables/resources.yaml @@ -0,0 +1,43 @@ +apiVersion: v1 +kind: Pod +metadata: + name: test-env-test + labels: + my-environment-name: test +spec: + containers: + - name: nginx + image: nginx:latest +--- +apiVersion: v1 +kind: Pod +metadata: + name: test-env-dev + labels: + my-environment-name: dev +spec: + containers: + - name: nginx + image: nginx:1.12 +--- +apiVersion: v1 +kind: Pod +metadata: + name: test-web + annotations: + role: web +spec: + containers: + - name: nginx + image: nginx:latest +--- +apiVersion: v1 +kind: Pod +metadata: + name: test-app + annotations: + role: app +spec: + containers: + - name: nginx + image: nginx:1.12 diff --git a/test/cli/test/variables/test.yaml b/test/cli/test/variables/test.yaml new file mode 100644 index 0000000000..4473d21506 --- /dev/null +++ b/test/cli/test/variables/test.yaml @@ -0,0 +1,24 @@ +name: test-variables +policies: + - cm-variable-example.yaml + - cm-array-example.yaml +resources: + - resources.yaml +variables: variables.yaml +results: + - policy: cm-variable-example + rule: example-configmap-lookup + resource: test-env-test + status: pass + - policy: cm-variable-example + rule: example-configmap-lookup + resource: test-env-dev + status: fail + - policy: cm-array-example + rule: validate-role-annotation + resource: test-web + status: fail + - policy: cm-array-example + rule: validate-role-annotation + resource: test-app + status: pass diff --git a/test/cli/test/variables/variables.yaml b/test/cli/test/variables/variables.yaml new file mode 100644 index 0000000000..942ddc9ce2 --- /dev/null +++ b/test/cli/test/variables/variables.yaml @@ -0,0 +1,25 @@ +policies: + - name: cm-variable-example + rules: + - name: example-configmap-lookup + values: + dictionary.data.env: test + resources: + - name: test-env-test + values: + request.object.metadata.name: test-env-test + - name: test-env-dev + values: + request.object.metadata.name: test-env-dev + - name: cm-array-example + rules: + - name: validate-role-annotation + values: + roles-dictionary.data.allowed-roles: "[\"app\",\"test\"]" + resources: + - name: test-web + values: + request.object.metadata.annotations.role: web + - name: test-app + values: + request.object.metadata.annotations.role: app From beabeddb816f8c60fe67ae4449058aed7af0cfed Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Tue, 4 May 2021 11:14:07 -0400 Subject: [PATCH 7/8] Fix reviewdog failure Signed-off-by: Trey Dockendorf --- pkg/engine/jsonContext.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/engine/jsonContext.go b/pkg/engine/jsonContext.go index 144531c475..2aefaf401d 100644 --- a/pkg/engine/jsonContext.go +++ b/pkg/engine/jsonContext.go @@ -28,7 +28,7 @@ func LoadContext(logger logr.Logger, contextEntries []kyverno.ContextEntry, resC if store.GetMock() { rule := store.GetPolicyRuleFromContext(policyName, ruleName) if len(rule.Values) == 0 { - return errors.New(fmt.Sprintf("No values found for policy %s rule %s", policyName, ruleName)) + return fmt.Errorf("No values found for policy %s rule %s", policyName, ruleName) } variables := rule.Values From 6407cb4c2d5f817b7c4cee3880a54f81edcfc6f2 Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Tue, 4 May 2021 13:13:23 -0400 Subject: [PATCH 8/8] Only evaluate if policy names match when variables are present Signed-off-by: Trey Dockendorf --- pkg/kyverno/test/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kyverno/test/command.go b/pkg/kyverno/test/command.go index e848a9dac3..ca4cf909b0 100644 --- a/pkg/kyverno/test/command.go +++ b/pkg/kyverno/test/command.go @@ -337,7 +337,7 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, valuesFile s } } } - if resourcePolicy != policy.GetName() { + if len(valuesMap) != 0 && resourcePolicy != policy.GetName() { log.Log.V(3).Info(fmt.Sprintf("Skipping resource, policy names do not match %s != %s", resourcePolicy, policy.GetName())) continue }