fix: validate the YAML test file syntactically and schematically (#8145)

* fix: validate the YAML test file syntactically

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* schema validation

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* unit tests

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

cmd/cli/kubectl-kyverno/test/load.go

@@ -12,14 +12,15 @@ import (
 
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/memfs"
+	"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/api"
 	sanitizederror "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/sanitizedError"
 	gitutils "github.com/kyverno/kyverno/pkg/utils/git"
 	"k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-type policy struct {
+type testCase struct {
-	bytes        []byte
+	test         *api.Test
 	resourcePath string
 }
 
@@ -27,8 +28,8 @@ func loadTests(
 	dirPath []string,
 	fileName string,
 	gitBranch string,
-) (billy.Filesystem, []policy, []error) {
+) (billy.Filesystem, []testCase, []error) {
-	var policies []policy
+	var tests []testCase
 	var errors []error
 	if strings.Contains(dirPath[0], "https://") {
 		fs := memfs.New()
@@ -69,49 +70,49 @@ func loadTests(
 				log.Log.V(3).Info(fmt.Sprintf("failed to clone repository  %v as it is not valid", repoURL), "error", cloneErr)
 				os.Exit(1)
 			}
-			if policyYamls, err := gitutils.ListYamls(fs, gitPathToYamls); err != nil {
+			if yamlFiles, err := gitutils.ListYamls(fs, gitPathToYamls); err != nil {
 				errors = append(errors, sanitizederror.NewWithError("failed to list YAMLs in repository", err))
 			} else {
-				sort.Strings(policyYamls)
+				sort.Strings(yamlFiles)
-				for _, yamlFilePath := range policyYamls {
+				for _, yamlFilePath := range yamlFiles {
 					file, err := fs.Open(yamlFilePath)
 					if err != nil {
 						errors = append(errors, sanitizederror.NewWithError("Error: failed to open file", err))
 						continue
 					}
 					if path.Base(file.Name()) == fileName {
-						policyresoucePath := strings.Trim(yamlFilePath, fileName)
+						resoucePath := strings.Trim(yamlFilePath, fileName)
-						bytes, err := io.ReadAll(file)
+						yamlBytes, err := io.ReadAll(file)
 						if err != nil {
-							errors = append(errors, sanitizederror.NewWithError("Error: failed to read file", err))
+							errors = append(errors, fmt.Errorf("failed to read file (%s)", err))
 							continue
 						}
-						policyBytes, err := yaml.ToJSON(bytes)
+						test, err := loadTest(yamlBytes)
 						if err != nil {
-							errors = append(errors, sanitizederror.NewWithError("failed to convert to JSON", err))
+							errors = append(errors, fmt.Errorf("failed to load test file (%s)", err))
 							continue
 						}
-						policies = append(policies, policy{
+						tests = append(tests, testCase{
-							bytes:        policyBytes,
+							test:         test,
-							resourcePath: policyresoucePath,
+							resourcePath: resoucePath,
 						})
 					}
 				}
 			}
 		}
-		return fs, policies, errors
+		return fs, tests, errors
 	} else {
 		path := filepath.Clean(dirPath[0])
-		policies, errors = loadLocalTest(path, fileName)
+		tests, errors = loadLocalTest(path, fileName)
-		return nil, policies, errors
+		return nil, tests, errors
 	}
 }
 
 func loadLocalTest(
 	path string,
 	fileName string,
-) ([]policy, []error) {
+) ([]testCase, []error) {
-	var policies []policy
+	var policies []testCase
 	var errors []error
 	files, err := os.ReadDir(path)
 	if err != nil {
@@ -124,18 +125,18 @@ func loadLocalTest(
 				errors = append(errors, errs...)
 			} else if file.Name() == fileName {
 				// We accept the risk of including files here as we read the test dir only.
-				yamlFile, err := os.ReadFile(filepath.Join(path, file.Name())) // #nosec G304
+				yamlBytes, err := os.ReadFile(filepath.Join(path, file.Name())) // #nosec G304
 				if err != nil {
-					errors = append(errors, sanitizederror.NewWithError("unable to read yaml", err))
+					errors = append(errors, fmt.Errorf("unable to read yaml (%s)", err))
 					continue
 				}
-				valuesBytes, err := yaml.ToJSON(yamlFile)
+				test, err := loadTest(yamlBytes)
 				if err != nil {
-					errors = append(errors, sanitizederror.NewWithError("failed to convert json", err))
+					errors = append(errors, fmt.Errorf("failed to load test file (%s)", err))
 					continue
 				}
-				policies = append(policies, policy{
+				policies = append(policies, testCase{
-					bytes:        valuesBytes,
+					test:         test,
 					resourcePath: path,
 				})
 			}
@@ -143,3 +144,11 @@ func loadLocalTest(
 	}
 	return policies, errors
 }
+
+func loadTest(data []byte) (*api.Test, error) {
+	var test api.Test
+	if err := yaml.UnmarshalStrict(data, &test); err != nil {
+		return nil, err
+	}
+	return &test, nil
+}

cmd/cli/kubectl-kyverno/test/load_test.go

@@ -0,0 +1,63 @@
+package test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/api"
+)
+
+func Test_loadTest(t *testing.T) {
+	tests := []struct {
+		name    string
+		data    []byte
+		want    *api.Test
+		wantErr bool
+	}{{
+		name: "invalid schema",
+		data: []byte(`
+- name: mytest
+  policies:
+    - pol.yaml
+  resources:
+    - pod.yaml
+  results:
+  - policy: evil-policy-match-foreign-pods
+    rule: evil-validation
+    resource: nginx
+    status: pass
+`),
+		want:    nil,
+		wantErr: true,
+	}, {
+		name: "unknown field",
+		data: []byte(`
+name: mytest
+policies:
+  - pol.yaml
+resources:
+  - pod.yaml
+results:
+- policy: evil-policy-match-foreign-pods
+  rule: evil-validation
+  resource: nginx
+  foo: bar
+  result: pass
+`),
+		want:    nil,
+		wantErr: true,
+	},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := loadTest(tt.data)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("loadTest() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("loadTest() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

cmd/cli/kubectl-kyverno/test/test.go

@@ -1,7 +1,6 @@
 package test
 
 import (
-	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -33,7 +32,7 @@ import (
 
 func applyPoliciesFromPath(
 	fs billy.Filesystem,
-	policyBytes []byte,
+	values *api.Test,
 	isGit bool,
 	policyResourcePath string,
 	rc *resultCounts,
@@ -43,13 +42,9 @@ func applyPoliciesFromPath(
 ) (map[string]policyreportv1alpha2.PolicyReportResult, []api.TestResults, error) {
 	engineResponses := make([]engineapi.EngineResponse, 0)
 	var dClient dclient.Interface
-	values := &api.Test{}
 	var resultCounts common.ResultCounts
 
 	store.SetLocal(true)
-	if err := json.Unmarshal(policyBytes, values); err != nil {
-		return nil, nil, sanitizederror.NewWithError("failed to decode yaml", err)
-	}
 
 	var filteredResults []api.TestResults
 	for _, res := range values.Results {

cmd/cli/kubectl-kyverno/test/test_command.go

@@ -94,14 +94,29 @@ func testCommandExecute(
 	// load tests
 	fs, policies, errors := loadTests(dirPath, fileName, gitBranch)
 	if len(policies) == 0 {
-		fmt.Printf("\n No test yamls available \n")
+		fmt.Println()
+		fmt.Println("No test yamls available")
+	}
+	if len(errors) > 0 {
+		fmt.Println()
+		fmt.Println("Test errors:")
+		for _, e := range errors {
+			fmt.Printf("    %v \n", e.Error())
+		}
+	}
+	if len(policies) == 0 {
+		if len(errors) == 0 {
+			os.Exit(0)
+		} else {
+			os.Exit(1)
+		}
 	}
 	rc = &resultCounts{}
 	var table table.Table
 	for _, p := range policies {
 		if reports, tests, err := applyPoliciesFromPath(
 			fs,
-			p.bytes,
+			p.test,
 			fs != nil,
 			p.resourcePath,
 			rc,
@@ -116,12 +131,6 @@ func testCommandExecute(
 			table.AddFailed(t.RawRows...)
 		}
 	}
-	if len(errors) > 0 && log.Log.V(1).Enabled() {
-		fmt.Println("test errors:")
-		for _, e := range errors {
-			fmt.Printf("    %v \n", e.Error())
-		}
-	}
 	if !failOnly {
 		fmt.Printf("\nTest Summary: %d tests passed and %d tests failed\n", rc.Pass+rc.Skip, rc.Fail)
 	} else {