From a80ee683c1b51ce01bd28f4a7a5c7e12286d3373 Mon Sep 17 00:00:00 2001 From: Marc Brugger Date: Fri, 16 Dec 2022 14:50:15 +0100 Subject: [PATCH] fix: allow policies from stdin in apply again (#5668) Signed-off-by: bakito Signed-off-by: bakito Co-authored-by: shuting --- cmd/cli/kubectl-kyverno/apply/.gitignore | 1 + .../kubectl-kyverno/apply/apply_command.go | 55 +++--- .../apply/apply_command_test.go | 168 ++++++++++++++---- test/openshift/team-validate-ns-name.yaml | 12 ++ 4 files changed, 177 insertions(+), 59 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/apply/.gitignore create mode 100644 test/openshift/team-validate-ns-name.yaml diff --git a/cmd/cli/kubectl-kyverno/apply/.gitignore b/cmd/cli/kubectl-kyverno/apply/.gitignore new file mode 100644 index 0000000000..5a2fc55300 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/apply/.gitignore @@ -0,0 +1 @@ +/disallow_latest_tag.yaml diff --git a/cmd/cli/kubectl-kyverno/apply/apply_command.go b/cmd/cli/kubectl-kyverno/apply/apply_command.go index 32dd833e4f..acf40303a2 100644 --- a/cmd/cli/kubectl-kyverno/apply/apply_command.go +++ b/cmd/cli/kubectl-kyverno/apply/apply_command.go @@ -67,7 +67,8 @@ type ApplyCommandConfig struct { warnExitCode int } -var applyHelp = ` +var ( + applyHelp = ` To apply on a resource: kyverno apply /path/to/policy.yaml /path/to/folderOfPolicies --resource=/path/to/resource1 --resource=/path/to/resource2 @@ -145,6 +146,10 @@ To apply policy with variables: More info: https://kyverno.io/docs/kyverno-cli/ ` + // allow os.exit to be overwritten during unit tests + osExit = os.Exit +) + func Command() *cobra.Command { var cmd *cobra.Command applyCommandConfig := &ApplyCommandConfig{} @@ -243,43 +248,45 @@ func (c *ApplyCommandConfig) applyCommandHelper() (rc *common.ResultCounts, reso return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("a stdin pipe can be used for either policies or resources, not both", err) } - isGit := common.IsGitSourcePath(c.PolicyPaths) var policies []kyvernov1.PolicyInterface - gitSourceURL, err := url.Parse(c.PolicyPaths[0]) - if err != nil { - fmt.Printf("Error: failed to load policies\nCause: %s\n", err) - os.Exit(1) - } - pathElems := strings.Split(gitSourceURL.Path[1:], "/") - if len(pathElems) <= 1 { - err := fmt.Errorf("invalid URL path %s - expected https:///:owner/:repository/:branch (without --git-branch flag) OR https:///:owner/:repository/:directory (with --git-branch flag)", gitSourceURL.Path) - fmt.Printf("Error: failed to parse URL \nCause: %s\n", err) - os.Exit(1) - } + isGit := common.IsGitSourcePath(c.PolicyPaths) - gitSourceURL.Path = strings.Join([]string{pathElems[0], pathElems[1]}, "/") - repoURL := gitSourceURL.String() - var gitPathToYamls string if isGit { + gitSourceURL, err := url.Parse(c.PolicyPaths[0]) + if err != nil { + fmt.Printf("Error: failed to load policies\nCause: %s\n", err) + osExit(1) + } + + pathElems := strings.Split(gitSourceURL.Path[1:], "/") + if len(pathElems) <= 1 { + err := fmt.Errorf("invalid URL path %s - expected https:///:owner/:repository/:branch (without --git-branch flag) OR https:///:owner/:repository/:directory (with --git-branch flag)", gitSourceURL.Path) + fmt.Printf("Error: failed to parse URL \nCause: %s\n", err) + osExit(1) + } + + gitSourceURL.Path = strings.Join([]string{pathElems[0], pathElems[1]}, "/") + repoURL := gitSourceURL.String() + var gitPathToYamls string c.GitBranch, gitPathToYamls = common.GetGitBranchOrPolicyPaths(c.GitBranch, repoURL, c.PolicyPaths) _, cloneErr := gitutils.Clone(repoURL, fs, c.GitBranch) if cloneErr != nil { fmt.Printf("Error: failed to clone repository \nCause: %s\n", cloneErr) log.Log.V(3).Info(fmt.Sprintf("failed to clone repository %v as it is not valid", repoURL), "error", cloneErr) - os.Exit(1) + osExit(1) } policyYamls, err := gitutils.ListYamls(fs, gitPathToYamls) if err != nil { return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to list YAMLs in repository", err) } - c.PolicyPaths = policyYamls sort.Strings(policyYamls) + c.PolicyPaths = policyYamls } policies, err = common.GetPoliciesFromPaths(fs, c.PolicyPaths, isGit, "") if err != nil { fmt.Printf("Error: failed to load policies\nCause: %s\n", err) - os.Exit(1) + osExit(1) } if len(c.ResourcePaths) == 0 && !c.Cluster { @@ -310,13 +317,13 @@ func (c *ApplyCommandConfig) applyCommandHelper() (rc *common.ResultCounts, reso err = common.PrintMutatedPolicy(policies) if err != nil { - return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to marsal mutated policy", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to marshal mutated policy", err) } resources, err = common.GetResourceAccordingToResourcePath(fs, c.ResourcePaths, c.Cluster, policies, dClient, c.Namespace, c.PolicyReport, false, "") if err != nil { fmt.Printf("Error: failed to load resources\nCause: %s\n", err) - os.Exit(1) + osExit(1) } if (len(resources) > 1 || len(policies) > 1) && c.VariablesString != "" { @@ -330,7 +337,7 @@ func (c *ApplyCommandConfig) applyCommandHelper() (rc *common.ResultCounts, reso userInfo, subjectInfo, err = common.GetUserInfoFromPath(fs, c.UserInfoPath, false, "") if err != nil { fmt.Printf("Error: failed to load request info\nCause: %s\n", err) - os.Exit(1) + osExit(1) } store.SetSubjects(subjectInfo) } @@ -499,9 +506,9 @@ func PrintReportOrViolation(policyReport bool, rc *common.ResultCounts, resource } if rc.Fail > 0 || rc.Error > 0 { - os.Exit(1) + osExit(1) } else if rc.Warn > 0 && warnExitCode != 0 { - os.Exit(warnExitCode) + osExit(warnExitCode) } } diff --git a/cmd/cli/kubectl-kyverno/apply/apply_command_test.go b/cmd/cli/kubectl-kyverno/apply/apply_command_test.go index 488e901a39..48e3e5e2af 100644 --- a/cmd/cli/kubectl-kyverno/apply/apply_command_test.go +++ b/cmd/cli/kubectl-kyverno/apply/apply_command_test.go @@ -1,6 +1,10 @@ package apply import ( + "fmt" + "os" + "path/filepath" + "strings" "testing" preport "github.com/kyverno/kyverno/api/policyreport/v1alpha2" @@ -9,15 +13,17 @@ import ( func Test_Apply(t *testing.T) { type TestCase struct { - PolicyPaths []string - GitBranch string - ResourcePaths []string - Cluster bool + gitBranch string expectedPolicyReports []preport.PolicyReport config ApplyCommandConfig + stdinFile string } + // copy disallow_latest_tag.yaml to local path + localFileName, err := copyFileToThisDir("../../../../test/best_practices/disallow_latest_tag.yaml") + assert.NilError(t, err) + defer func() { _ = os.Remove(localFileName) }() - testcases := []TestCase{ + testcases := []*TestCase{ { config: ApplyCommandConfig{ PolicyPaths: []string{"../../../../test/best_practices/disallow_latest_tag.yaml"}, @@ -36,6 +42,24 @@ func Test_Apply(t *testing.T) { }, }, }, + { + config: ApplyCommandConfig{ + PolicyPaths: []string{localFileName}, + ResourcePaths: []string{"../../../../test/resources/pod_with_version_tag.yaml"}, + PolicyReport: true, + }, + expectedPolicyReports: []preport.PolicyReport{ + { + Summary: preport.PolicyReportSummary{ + Pass: 2, + Fail: 0, + Skip: 0, + Error: 0, + Warn: 0, + }, + }, + }, + }, { config: ApplyCommandConfig{ PolicyPaths: []string{"../../../../test/best_practices/disallow_latest_tag.yaml"}, @@ -72,25 +96,6 @@ func Test_Apply(t *testing.T) { }, }, }, - { - config: ApplyCommandConfig{ - PolicyPaths: []string{"https://github.com/kyverno/policies/openshift/team-validate-ns-name/"}, - GitBranch: "main", - PolicyReport: true, - Cluster: true, - }, - expectedPolicyReports: []preport.PolicyReport{ - { - Summary: preport.PolicyReportSummary{ - Pass: 6, - Fail: 0, - Skip: 0, - Error: 0, - Warn: 0, - }, - }, - }, - }, { config: ApplyCommandConfig{ PolicyPaths: []string{"../../../../test/best_practices/disallow_latest_tag.yaml"}, @@ -110,21 +115,114 @@ func Test_Apply(t *testing.T) { }, }, }, + { + config: ApplyCommandConfig{ + PolicyPaths: []string{"-"}, + ResourcePaths: []string{"../../../../test/resources/pod_with_latest_tag.yaml"}, + PolicyReport: true, + AuditWarn: true, + }, + stdinFile: "../../../../test/best_practices/disallow_latest_tag.yaml", + expectedPolicyReports: []preport.PolicyReport{ + { + Summary: preport.PolicyReportSummary{ + Pass: 1, + Fail: 0, + Skip: 0, + Error: 0, + Warn: 1, + }, + }, + }, + }, + { + config: ApplyCommandConfig{ + PolicyPaths: []string{"../../../../test/best_practices/disallow_latest_tag.yaml"}, + ResourcePaths: []string{"-"}, + PolicyReport: true, + AuditWarn: true, + }, + stdinFile: "../../../../test/resources/pod_with_latest_tag.yaml", + expectedPolicyReports: []preport.PolicyReport{ + { + Summary: preport.PolicyReportSummary{ + Pass: 1, + Fail: 0, + Skip: 0, + Error: 0, + Warn: 1, + }, + }, + }, + }, + { + config: ApplyCommandConfig{ + PolicyPaths: []string{"https://github.com/kyverno/policies/openshift/team-validate-ns-name/"}, + ResourcePaths: []string{"../../../../test/openshift/team-validate-ns-name.yaml"}, + GitBranch: "main", + PolicyReport: true, + }, + expectedPolicyReports: []preport.PolicyReport{ + { + Summary: preport.PolicyReportSummary{ + Pass: 2, + Fail: 0, + Skip: 0, + Error: 0, + Warn: 0, + }, + }, + }, + }, } - compareSummary := func(expected preport.PolicyReportSummary, actual map[string]interface{}) { - assert.Equal(t, actual[preport.StatusPass].(int64), int64(expected.Pass)) - assert.Equal(t, actual[preport.StatusFail].(int64), int64(expected.Fail)) - assert.Equal(t, actual[preport.StatusSkip].(int64), int64(expected.Skip)) - assert.Equal(t, actual[preport.StatusWarn].(int64), int64(expected.Warn)) - assert.Equal(t, actual[preport.StatusError].(int64), int64(expected.Error)) + compareSummary := func(expected preport.PolicyReportSummary, actual map[string]interface{}, desc string) { + assert.Equal(t, actual[preport.StatusPass].(int64), int64(expected.Pass), desc) + assert.Equal(t, actual[preport.StatusFail].(int64), int64(expected.Fail), desc) + assert.Equal(t, actual[preport.StatusSkip].(int64), int64(expected.Skip), desc) + assert.Equal(t, actual[preport.StatusWarn].(int64), int64(expected.Warn), desc) + assert.Equal(t, actual[preport.StatusError].(int64), int64(expected.Error), desc) + } + + verifyTestcase := func(t *testing.T, tc *TestCase, compareSummary func(preport.PolicyReportSummary, map[string]interface{}, string)) { + if tc.stdinFile != "" { + oldStdin := os.Stdin + input, err := os.OpenFile(tc.stdinFile, os.O_RDONLY, 0) + assert.NilError(t, err) + os.Stdin = input + defer func() { + // Restore original Stdin + os.Stdin = oldStdin + _ = input.Close() + }() + } + desc := fmt.Sprintf("Policies: [%s], / Resources: [%s]", strings.Join(tc.config.PolicyPaths, ","), strings.Join(tc.config.ResourcePaths, ",")) + // prevent os.Exit from being called + osExit = func(code int) { + assert.Check(t, false, "os.Exit(%d) should not be called: %s", code, desc) + } + + defer func() { osExit = os.Exit }() + _, _, _, info, err := tc.config.applyCommandHelper() + assert.NilError(t, err, desc) + + resps := buildPolicyReports(info) + assert.Assert(t, len(resps) > 0, "policy reports should not be empty: %s", desc) + for i, resp := range resps { + compareSummary(tc.expectedPolicyReports[i].Summary, resp.UnstructuredContent()["summary"].(map[string]interface{}), desc) + } } for _, tc := range testcases { - _, _, _, info, _ := tc.config.applyCommandHelper() - resps := buildPolicyReports(info) - for i, resp := range resps { - compareSummary(tc.expectedPolicyReports[i].Summary, resp.UnstructuredContent()["summary"].(map[string]interface{})) - } + verifyTestcase(t, tc, compareSummary) } } + +func copyFileToThisDir(sourceFile string) (string, error) { + input, err := os.ReadFile(sourceFile) + if err != nil { + return "", err + } + + return filepath.Base(sourceFile), os.WriteFile(filepath.Base(sourceFile), input, 0644) +} diff --git a/test/openshift/team-validate-ns-name.yaml b/test/openshift/team-validate-ns-name.yaml new file mode 100644 index 0000000000..cf944f1500 --- /dev/null +++ b/test/openshift/team-validate-ns-name.yaml @@ -0,0 +1,12 @@ +### Namespace good +--- +apiVersion: v1 +kind: Namespace +metadata: + name: team1-test +### Namespace bad +--- +apiVersion: v1 +kind: Namespace +metadata: + name: test-namespace