Merge pull request #1866 from treydock/test-error-handling

Improved error handling for test command

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:

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

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 fmt.Errorf("No values found for policy %s rule %s", policyName, ruleName)
+		}
 		variables := rule.Values
 
 		for key, value := range variables {

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 {

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"
@@ -145,83 +146,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{} {
@@ -269,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)
@@ -333,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 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
+			}
 			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
@@ -361,6 +369,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)
@@ -374,7 +383,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 {
@@ -387,6 +396,7 @@ func printTestResult(resps map[string][]interface{}, testResults []TestResults,
 						res.Result = "Pass"
 						rc.pass++
 					} else {
+						res.Result = boldRed.Sprintf("Fail")
 						rc.fail++
 					}
 				}

test/cli/test/simple/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"

test/cli/test/simple/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

test/cli/test/simple/test.yaml

@@ -0,0 +1,14 @@
+name: test-simple
+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

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\" }}"

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}}"

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

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

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