From bb3df218ed41497165a6b82d6a5285db35523b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 29 Aug 2023 00:04:00 +0200 Subject: [PATCH] fix: validate the YAML test file syntactically and schematically (#8145) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: validate the YAML test file syntactically Signed-off-by: Charles-Edouard Brétéché * schema validation Signed-off-by: Charles-Edouard Brétéché * unit tests Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- cmd/cli/kubectl-kyverno/test/load.go | 61 +++++++++++-------- cmd/cli/kubectl-kyverno/test/load_test.go | 63 ++++++++++++++++++++ cmd/cli/kubectl-kyverno/test/test.go | 7 +-- cmd/cli/kubectl-kyverno/test/test_command.go | 25 +++++--- 4 files changed, 116 insertions(+), 40 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/test/load_test.go diff --git a/cmd/cli/kubectl-kyverno/test/load.go b/cmd/cli/kubectl-kyverno/test/load.go index 052872a85a..20f18b82e0 100644 --- a/cmd/cli/kubectl-kyverno/test/load.go +++ b/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 { - bytes []byte +type testCase struct { + test *api.Test resourcePath string } @@ -27,8 +28,8 @@ func loadTests( dirPath []string, fileName string, gitBranch string, -) (billy.Filesystem, []policy, []error) { - var policies []policy +) (billy.Filesystem, []testCase, []error) { + 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) - for _, yamlFilePath := range policyYamls { + sort.Strings(yamlFiles) + 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) - bytes, err := io.ReadAll(file) + resoucePath := strings.Trim(yamlFilePath, fileName) + 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{ - bytes: policyBytes, - resourcePath: policyresoucePath, + tests = append(tests, testCase{ + test: test, + resourcePath: resoucePath, }) } } } } - return fs, policies, errors + return fs, tests, errors } else { path := filepath.Clean(dirPath[0]) - policies, errors = loadLocalTest(path, fileName) - return nil, policies, errors + tests, errors = loadLocalTest(path, fileName) + return nil, tests, errors } } func loadLocalTest( path string, fileName string, -) ([]policy, []error) { - var policies []policy +) ([]testCase, []error) { + 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{ - bytes: valuesBytes, + policies = append(policies, testCase{ + 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 +} diff --git a/cmd/cli/kubectl-kyverno/test/load_test.go b/cmd/cli/kubectl-kyverno/test/load_test.go new file mode 100644 index 0000000000..362d1ad590 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/cmd/cli/kubectl-kyverno/test/test.go b/cmd/cli/kubectl-kyverno/test/test.go index 744a031975..c673cd11ab 100644 --- a/cmd/cli/kubectl-kyverno/test/test.go +++ b/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 { diff --git a/cmd/cli/kubectl-kyverno/test/test_command.go b/cmd/cli/kubectl-kyverno/test/test_command.go index 9d17ef6eb3..76bc6d6185 100644 --- a/cmd/cli/kubectl-kyverno/test/test_command.go +++ b/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 {