From 0688c9b369c2a9d4535b51a6831ba77b2a262af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 12 Sep 2023 19:03:37 +0200 Subject: [PATCH] fix: Kyverno apply produces false positives when validating 'empty dangling" tags (#8358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: verifyImages w/ multiple entries is not consistent Signed-off-by: Charles-Edouard Brétéché * clean Signed-off-by: Charles-Edouard Brétéché * fix: Kyverno apply produces false positives when validating 'empty dangling' tags Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- .../_testdata/apply/test-1/policy.yaml | 28 +++++ .../_testdata/apply/test-1/resources.yaml | 101 ++++++++++++++++++ .../kubectl-kyverno/commands/apply/command.go | 9 +- .../commands/apply/command_test.go | 21 +++- 4 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/_testdata/apply/test-1/policy.yaml create mode 100644 cmd/cli/kubectl-kyverno/_testdata/apply/test-1/resources.yaml diff --git a/cmd/cli/kubectl-kyverno/_testdata/apply/test-1/policy.yaml b/cmd/cli/kubectl-kyverno/_testdata/apply/test-1/policy.yaml new file mode 100644 index 0000000000..62ce291ff9 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/_testdata/apply/test-1/policy.yaml @@ -0,0 +1,28 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: test-policy + annotations: + policies.kyverno.io/title: "Lol Security Standards" + policies.kyverno.io/category: "Lol Security Standards" + policies.kyverno.io/severity: "high" + policies.kyverno.io/subject: "Pod" +spec: + background: true + failurePolicy: Fail + rules: + - name: restrict-lol-annotation + match: + any: + - resources: + kinds: + - Pod + validate: + message: >- + If "lol" annotation is present then + its value can be only one of "such lol", "much annotation". + pattern: + # syntax refdoc: https://kyverno.io/docs/writing-policies/validate/#anchors + =(metadata): + =(annotations): + =(lol): such lol | much annotation diff --git a/cmd/cli/kubectl-kyverno/_testdata/apply/test-1/resources.yaml b/cmd/cli/kubectl-kyverno/_testdata/apply/test-1/resources.yaml new file mode 100644 index 0000000000..d672c501d2 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/_testdata/apply/test-1/resources.yaml @@ -0,0 +1,101 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: lol + name: i-will-fail-the-policy-check +spec: + selector: + matchLabels: + app: lol + template: + metadata: + labels: + app: lol + annotations: {} + spec: + containers: + - image: woot + name: woot +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: lol + name: no-annotations-pass +spec: + selector: + matchLabels: + app: lol + template: + metadata: + labels: + app: lol +# annotations: + spec: + containers: + - image: woot + name: woot +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: lol + name: empty-object-pass +spec: + selector: + matchLabels: + app: lol + template: + metadata: + labels: + app: lol + annotations: {} + spec: + containers: + - image: woot + name: woot +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: lol + name: correct-lol-annotation-pass +spec: + selector: + matchLabels: + app: lol + template: + metadata: + labels: + app: lol + annotations: + lol: much annotation + spec: + containers: + - image: woot + name: woot +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: lol + name: unrelated-annotation-pass +spec: + selector: + matchLabels: + app: lol + template: + metadata: + labels: + app: lol + annotations: + much: unrelated + spec: + containers: + - image: woot + name: woot \ No newline at end of file diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index 0eb685d8b2..c7c776d893 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -69,10 +69,11 @@ func Command() *cobra.Command { var removeColor, detailedResults, table bool applyCommandConfig := &ApplyCommandConfig{} cmd := &cobra.Command{ - Use: "apply", - Short: command.FormatDescription(true, websiteUrl, false, description...), - Long: command.FormatDescription(false, websiteUrl, false, description...), - Example: command.FormatExamples(examples...), + Use: "apply", + Short: command.FormatDescription(true, websiteUrl, false, description...), + Long: command.FormatDescription(false, websiteUrl, false, description...), + Example: command.FormatExamples(examples...), + SilenceUsage: true, RunE: func(cmd *cobra.Command, policyPaths []string) (err error) { color.InitColors(removeColor) applyCommandConfig.PolicyPaths = policyPaths diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command_test.go b/cmd/cli/kubectl-kyverno/commands/apply/command_test.go index 3b1f163ef6..c94635c353 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command_test.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command_test.go @@ -9,7 +9,7 @@ import ( policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/report" - "gotest.tools/assert" + "github.com/stretchr/testify/assert" ) func Test_Apply(t *testing.T) { @@ -21,7 +21,7 @@ func Test_Apply(t *testing.T) { } // copy disallow_latest_tag.yaml to local path localFileName, err := copyFileToThisDir("../../../../../test/best_practices/disallow_latest_tag.yaml") - assert.NilError(t, err) + assert.NoError(t, err) defer func() { _ = os.Remove(localFileName) }() testcases := []*TestCase{ @@ -322,7 +322,7 @@ func Test_Apply(t *testing.T) { if tc.stdinFile != "" { oldStdin := os.Stdin input, err := os.OpenFile(tc.stdinFile, os.O_RDONLY, 0) - assert.NilError(t, err) + assert.NoError(t, err) os.Stdin = input defer func() { // Restore original Stdin @@ -333,10 +333,10 @@ func Test_Apply(t *testing.T) { desc := fmt.Sprintf("Policies: [%s], / Resources: [%s]", strings.Join(tc.config.PolicyPaths, ","), strings.Join(tc.config.ResourcePaths, ",")) _, _, _, responses, err := tc.config.applyCommandHelper() - assert.NilError(t, err, desc) + assert.NoError(t, err, desc) clustered, _ := report.ComputePolicyReports(tc.config.AuditWarn, responses...) - assert.Assert(t, len(clustered) > 0, "policy reports should not be empty: %s", desc) + assert.Greater(t, len(clustered), 0, "policy reports should not be empty: %s", desc) combined := []policyreportv1alpha2.ClusterPolicyReport{ report.MergeClusterReports(clustered), } @@ -361,3 +361,14 @@ func copyFileToThisDir(sourceFile string) (string, error) { return filepath.Base(sourceFile), os.WriteFile(filepath.Base(sourceFile), input, 0o644) } + +func TestCommand(t *testing.T) { + cmd := Command() + cmd.SetArgs([]string{ + "../../_testdata/apply/test-1/policy.yaml", + "--resource", + "../../_testdata/apply/test-1/resources.yaml", + }) + err := cmd.Execute() + assert.NoError(t, err) +}