fix: allow policies from stdin in apply again (#5668)

Signed-off-by: bakito <github@bakito.ch>

Signed-off-by: bakito <github@bakito.ch>
Co-authored-by: shuting <shuting@nirmata.com>

cmd/cli/kubectl-kyverno/apply/.gitignore

@@ -0,0 +1 @@
+/disallow_latest_tag.yaml

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://<any_git_source_domain>/:owner/:repository/:branch (without --git-branch flag) OR https://<any_git_source_domain>/: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://<any_git_source_domain>/:owner/:repository/:branch (without --git-branch flag) OR https://<any_git_source_domain>/: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)
 	}
 }
 

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)
+}

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