From c0a74fe0d5c02542d8ba394ed35637f790409e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 5 Sep 2023 13:09:45 +0200 Subject: [PATCH] fix: bad test file causes all tests to pass with success (#8258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .../kubectl-kyverno/commands/test/command.go | 62 +++++++++---------- cmd/cli/kubectl-kyverno/commands/test/test.go | 39 ++++++------ cmd/cli/kubectl-kyverno/test/load.go | 6 +- cmd/cli/kubectl-kyverno/test/load_test.go | 26 ++++---- 4 files changed, 65 insertions(+), 68 deletions(-) diff --git a/cmd/cli/kubectl-kyverno/commands/test/command.go b/cmd/cli/kubectl-kyverno/commands/test/command.go index 07d45e01d6..11becddf5d 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/command.go +++ b/cmd/cli/kubectl-kyverno/commands/test/command.go @@ -43,12 +43,7 @@ func Command() *cobra.Command { } }() store.SetRegistryAccess(registryAccess) - _, err = testCommandExecute(dirPath, fileName, gitBranch, testCase, failOnly, detailedResults) - if err != nil { - log.Log.V(3).Info("a directory is required") - return err - } - return nil + return testCommandExecute(dirPath, fileName, gitBranch, testCase, failOnly, detailedResults) }, } cmd.Flags().StringVarP(&fileName, "file-name", "f", "kyverno-test.yaml", "Test filename") @@ -74,10 +69,10 @@ func testCommandExecute( testCase string, failOnly bool, detailedResults bool, -) (rc *resultCounts, err error) { +) (err error) { // check input dir if len(dirPath) == 0 { - return rc, sanitizederror.NewWithError("a directory is required", err) + return sanitizederror.NewWithError("a directory is required", err) } // parse filter filter, errors := filter.ParseFilter(testCase) @@ -85,21 +80,20 @@ func testCommandExecute( fmt.Println() fmt.Println("Filter errors:") for _, e := range errors { - fmt.Printf(" %v \n", e.Error()) + fmt.Println(" Error:", e) } } // init openapi manager openApiManager, err := openapi.NewManager(log.Log) if err != nil { - return rc, fmt.Errorf("unable to create open api controller, %w", err) + return fmt.Errorf("unable to create open api controller, %w", err) } // load tests tests, err := loadTests(dirPath, fileName, gitBranch) if err != nil { fmt.Println() - fmt.Println("Error loading tests:") - fmt.Printf(" %s\n", err) - return rc, err + fmt.Println("Error loading tests:", err) + return err } if len(tests) == 0 { fmt.Println() @@ -109,7 +103,8 @@ func testCommandExecute( fmt.Println() fmt.Println("Test errors:") for _, e := range errs { - fmt.Printf(" %v \n", e.Error()) + fmt.Println(" Path:", e.Path) + fmt.Println(" Error:", e.Err) } } if len(tests) == 0 { @@ -119,25 +114,25 @@ func testCommandExecute( os.Exit(1) } } - rc = &resultCounts{} + rc := &resultCounts{} var table table.Table - for _, p := range tests { - resourcePath := filepath.Dir(p.Path) - if tests, responses, err := applyPoliciesFromPath( - p.Fs, - p.Test, - p.Fs != nil, - resourcePath, - rc, - openApiManager, - filter, - false, - ); err != nil { - return rc, sanitizederror.NewWithError("failed to apply test command", err) - } else if t, err := printTestResult(tests, responses, rc, failOnly, detailedResults, p.Fs, resourcePath); err != nil { - return rc, sanitizederror.NewWithError("failed to print test result:", err) - } else { - table.AddFailed(t.RawRows...) + for _, test := range tests { + if test.Err == nil { + resourcePath := filepath.Dir(test.Path) + if tests, responses, err := applyPoliciesFromPath( + test, + resourcePath, + rc, + openApiManager, + filter, + false, + ); err != nil { + return sanitizederror.NewWithError("failed to apply test command", err) + } else if t, err := printTestResult(tests, responses, rc, failOnly, detailedResults, test.Fs, resourcePath); err != nil { + return sanitizederror.NewWithError("failed to print test result:", err) + } else { + table.AddFailed(t.RawRows...) + } } } if !failOnly { @@ -152,8 +147,7 @@ func testCommandExecute( } os.Exit(1) } - os.Exit(0) - return rc, nil + return nil } func checkResult(test api.TestResults, fs billy.Filesystem, resoucePath string, response engineapi.EngineResponse, rule engineapi.RuleResponse) (bool, string, string) { diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go index bdf7cd9c99..41de7f6d71 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/test.go +++ b/cmd/cli/kubectl-kyverno/commands/test/test.go @@ -4,9 +4,9 @@ import ( "fmt" "os" - "github.com/go-git/go-billy/v5" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/api/kyverno/v1beta1" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/api" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/filter" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/common" @@ -26,38 +26,39 @@ import ( ) func applyPoliciesFromPath( - fs billy.Filesystem, - apiTest *api.Test, - isGit bool, + testCase test.TestCase, policyResourcePath string, rc *resultCounts, openApiManager openapi.Manager, filter filter.Filter, auditWarn bool, ) ([]api.TestResults, []engineapi.EngineResponse, error) { - engineResponses := make([]engineapi.EngineResponse, 0) + var engineResponses []engineapi.EngineResponse + test := testCase.Test + fs := testCase.Fs + isGit := fs != nil var dClient dclient.Interface var resultCounts common.ResultCounts store.SetLocal(true) var filteredResults []api.TestResults - for _, res := range apiTest.Results { + for _, res := range test.Results { if filter.Apply(res) { filteredResults = append(filteredResults, res) } } - apiTest.Results = filteredResults + test.Results = filteredResults - if len(apiTest.Results) == 0 { + if len(test.Results) == 0 { return nil, nil, nil } - fmt.Printf("\nExecuting %s...\n", apiTest.Name) - valuesFile := apiTest.Variables - userInfoFile := apiTest.UserInfo + fmt.Printf("\nExecuting %s...\n", test.Name) + valuesFile := test.Variables + userInfoFile := test.UserInfo - variables, globalValMap, valuesMap, namespaceSelectorMap, subresources, err := common.GetVariable(nil, apiTest.Values, apiTest.Variables, fs, isGit, policyResourcePath) + variables, globalValMap, valuesMap, namespaceSelectorMap, subresources, err := common.GetVariable(nil, test.Values, test.Variables, fs, isGit, policyResourcePath) if err != nil { if !sanitizederror.IsErrorSanitized(err) { return nil, nil, sanitizederror.NewWithError("failed to decode yaml", err) @@ -76,8 +77,8 @@ func applyPoliciesFromPath( } } - policyFullPath := pathutils.GetFullPaths(apiTest.Policies, policyResourcePath, isGit) - resourceFullPath := pathutils.GetFullPaths(apiTest.Resources, policyResourcePath, isGit) + policyFullPath := pathutils.GetFullPaths(test.Policies, policyResourcePath, isGit) + resourceFullPath := pathutils.GetFullPaths(test.Resources, policyResourcePath, isGit) policies, validatingAdmissionPolicies, err := common.GetPoliciesFromPaths(fs, policyFullPath, isGit, policyResourcePath) if err != nil { @@ -87,7 +88,7 @@ func applyPoliciesFromPath( var filteredPolicies []kyvernov1.PolicyInterface for _, p := range policies { - for _, res := range apiTest.Results { + for _, res := range test.Results { if p.GetName() == res.Policy { filteredPolicies = append(filteredPolicies, p) break @@ -97,7 +98,7 @@ func applyPoliciesFromPath( var filteredVAPs []v1alpha1.ValidatingAdmissionPolicy for _, p := range validatingAdmissionPolicies { - for _, res := range apiTest.Results { + for _, res := range test.Results { if p.GetName() == res.Policy { filteredVAPs = append(filteredVAPs, p) break @@ -111,7 +112,7 @@ func applyPoliciesFromPath( var filteredRules []kyvernov1.Rule for _, rule := range autogen.ComputeRules(p) { - for _, res := range apiTest.Results { + for _, res := range test.Results { if res.IsValidatingAdmissionPolicy { continue } @@ -153,7 +154,7 @@ func applyPoliciesFromPath( os.Exit(1) } - checkableResources := selectResourcesForCheck(resources, apiTest) + checkableResources := selectResourcesForCheck(resources, test) msgPolicies := "1 policy" if len(policies)+len(validatingAdmissionPolicies) > 1 { @@ -234,7 +235,7 @@ func applyPoliciesFromPath( engineResponses = append(engineResponses, ers...) } } - return apiTest.Results, engineResponses, nil + return test.Results, engineResponses, nil } func selectResourcesForCheck(resources []*unstructured.Unstructured, values *api.Test) []*unstructured.Unstructured { diff --git a/cmd/cli/kubectl-kyverno/test/load.go b/cmd/cli/kubectl-kyverno/test/load.go index 7a7fa0bbe0..6483e84bb8 100644 --- a/cmd/cli/kubectl-kyverno/test/load.go +++ b/cmd/cli/kubectl-kyverno/test/load.go @@ -19,11 +19,11 @@ type TestCase struct { type TestCases []TestCase -func (tc TestCases) Errors() []error { - var errors []error +func (tc TestCases) Errors() []TestCase { + var errors []TestCase for _, test := range tc { if test.Err != nil { - errors = append(errors, test.Err) + errors = append(errors, test) } } return errors diff --git a/cmd/cli/kubectl-kyverno/test/load_test.go b/cmd/cli/kubectl-kyverno/test/load_test.go index 2a90560751..1d7c1171f3 100644 --- a/cmd/cli/kubectl-kyverno/test/load_test.go +++ b/cmd/cli/kubectl-kyverno/test/load_test.go @@ -13,7 +13,7 @@ func TestTestCases_Errors(t *testing.T) { tests := []struct { name string tc TestCases - want []error + want []TestCase }{{ name: "nil", tc: nil, @@ -31,9 +31,9 @@ func TestTestCases_Errors(t *testing.T) { tc: []TestCase{{ Err: errors.New("error 1"), }}, - want: []error{ - errors.New("error 1"), - }, + want: []TestCase{{ + Err: errors.New("error 1"), + }}, }, { name: "two errors", tc: []TestCase{{ @@ -41,10 +41,11 @@ func TestTestCases_Errors(t *testing.T) { }, { Err: errors.New("error 2"), }}, - want: []error{ - errors.New("error 1"), - errors.New("error 2"), - }, + want: []TestCase{{ + Err: errors.New("error 1"), + }, { + Err: errors.New("error 2"), + }}, }, { name: "mixed", tc: []TestCase{{ @@ -52,10 +53,11 @@ func TestTestCases_Errors(t *testing.T) { }, {}, { Err: errors.New("error 2"), }, {}}, - want: []error{ - errors.New("error 1"), - errors.New("error 2"), - }, + want: []TestCase{{ + Err: errors.New("error 1"), + }, { + Err: errors.New("error 2"), + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {