From 37bbf33bd5e04858ade2711880ac2eb61f53956a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 14 Sep 2023 02:30:23 +0200 Subject: [PATCH] fix: CLI test command should validate the policy under test (#8387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- Makefile | 2 +- .../_testdata/policies/invalid-schema.yaml | 43 +++++++++++++++++++ cmd/cli/kubectl-kyverno/policy/load_test.go | 38 ++++++++++++++++ pkg/utils/yaml/loadpolicy.go | 8 ++-- pkg/utils/yaml/loadpolicy_test.go | 12 +++--- .../scenario_mutate_endpoint/policy.yaml | 6 +-- .../kyverno-test.yaml | 7 +-- 7 files changed, 96 insertions(+), 20 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/_testdata/policies/invalid-schema.yaml create mode 100644 cmd/cli/kubectl-kyverno/policy/load_test.go diff --git a/Makefile b/Makefile index b71f0133a9..0611fbc0f5 100644 --- a/Makefile +++ b/Makefile @@ -521,7 +521,7 @@ codegen-cli-docs: $(CLI_BIN) ## Generate CLI docs .PHONY: codegen-cli-tests codegen-cli-tests: $(CLI_BIN) ## Fix CLI test files @echo Fix CLI test files... >&2 - @KYVERNO_EXPERIMENTAL=true $(CLI_BIN) fix test ./test/cli --save + @KYVERNO_EXPERIMENTAL=true $(CLI_BIN) fix test ./test/cli --save --compress .PHONY: codegen-docs-all codegen-docs-all: codegen-helm-docs codegen-cli-docs codegen-api-docs ## Generate all docs diff --git a/cmd/cli/kubectl-kyverno/_testdata/policies/invalid-schema.yaml b/cmd/cli/kubectl-kyverno/_testdata/policies/invalid-schema.yaml new file mode 100644 index 0000000000..8ce91ef6a2 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/_testdata/policies/invalid-schema.yaml @@ -0,0 +1,43 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: pod-requirements + annotations: + pod-policies.kyverno.io/autogen-controllers: none + policies.kyverno.io/severity: medium + policies.kyverno.io/category: Pod Security Standards (Restricted) +spec: + background: false + validationFailureAction: audit + rules: + - name: pods-require-account + match: + resources: + kinds: + - Pod + namespaceSelector: + matchLabels: + istio/rev: "default" + validate: + message: User pods must include an account for charging + pattern: + metadata: + labels: + account: "*?" + - name: pods-require-limits + match: + resources: + kinds: + - Pod + validate: + message: CPU and memory resource requests and limits are required for user pods + pattern: + spec: + containers: + - resources: + requests: + memory: "?*" + cpu: "?*" + limits: + memory: "?*" + cpu: "?*" diff --git a/cmd/cli/kubectl-kyverno/policy/load_test.go b/cmd/cli/kubectl-kyverno/policy/load_test.go new file mode 100644 index 0000000000..33a6af2f8a --- /dev/null +++ b/cmd/cli/kubectl-kyverno/policy/load_test.go @@ -0,0 +1,38 @@ +package policy + +import ( + "testing" + + "github.com/go-git/go-billy/v5" +) + +func TestLoad(t *testing.T) { + tests := []struct { + name string + fs billy.Filesystem + resourcePath string + paths []string + wantErr bool + }{{ + name: "cpol-limit-configmap-for-sa", + fs: nil, + resourcePath: "", + paths: []string{"../_testdata/policies/cpol-limit-configmap-for-sa.yaml"}, + wantErr: false, + }, { + name: "invalid-schema", + fs: nil, + resourcePath: "", + paths: []string{"../_testdata/policies/invalid-schema.yaml"}, + wantErr: true, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := Load(tt.fs, tt.resourcePath, tt.paths...) + if (err != nil) != tt.wantErr { + t.Errorf("Load() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} diff --git a/pkg/utils/yaml/loadpolicy.go b/pkg/utils/yaml/loadpolicy.go index 3433493f3c..9f683b70d3 100644 --- a/pkg/utils/yaml/loadpolicy.go +++ b/pkg/utils/yaml/loadpolicy.go @@ -54,9 +54,9 @@ func addPolicy(policies []kyvernov1.PolicyInterface, validatingAdmissionPolicies kind := us.GetKind() if strings.Compare(kind, "ValidatingAdmissionPolicy") == 0 { - validatingAdmissionPolicy := &v1alpha1.ValidatingAdmissionPolicy{} + validatingAdmissionPolicy := v1alpha1.ValidatingAdmissionPolicy{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(us.Object, validatingAdmissionPolicy); err != nil { + if err := runtime.DefaultUnstructuredConverter.FromUnstructuredWithValidation(us.Object, &validatingAdmissionPolicy, true); err != nil { return policies, nil, fmt.Errorf("failed to decode policy: %v", err) } @@ -65,7 +65,7 @@ func addPolicy(policies []kyvernov1.PolicyInterface, validatingAdmissionPolicies return policies, validatingAdmissionPolicies, nil } - validatingAdmissionPolicies = append(validatingAdmissionPolicies, *validatingAdmissionPolicy) + validatingAdmissionPolicies = append(validatingAdmissionPolicies, validatingAdmissionPolicy) } else { var policy kyvernov1.PolicyInterface if us.GetKind() == "ClusterPolicy" { @@ -74,7 +74,7 @@ func addPolicy(policies []kyvernov1.PolicyInterface, validatingAdmissionPolicies policy = &kyvernov1.Policy{} } - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(us.Object, policy); err != nil { + if err := runtime.DefaultUnstructuredConverter.FromUnstructuredWithValidation(us.Object, policy, true); err != nil { return nil, validatingAdmissionPolicies, fmt.Errorf("failed to decode policy: %v", err) } diff --git a/pkg/utils/yaml/loadpolicy_test.go b/pkg/utils/yaml/loadpolicy_test.go index 55c95bf9a9..00222107a7 100644 --- a/pkg/utils/yaml/loadpolicy_test.go +++ b/pkg/utils/yaml/loadpolicy_test.go @@ -314,8 +314,8 @@ spec: apiVersions: ["v1"] operations: ["CREATE", "UPDATE"] resources: ["deployments"] - validations: - - expression: "object.spec.replicas <= 5" + validations: + - expression: "object.spec.replicas <= 5" `), }, validatingAdmissionPolicies: []policy{ {"ValidatingAdmissionPolicy", ""}, @@ -337,8 +337,8 @@ spec: apiVersions: ["v1"] operations: ["CREATE", "UPDATE"] resources: ["deployments"] - validations: - - expression: "object.spec.replicas <= 5" + validations: + - expression: "object.spec.replicas <= 5" --- apiVersion: kyverno.io/v1 kind: Policy @@ -391,8 +391,8 @@ spec: apiVersions: ["v1"] operations: ["CREATE", "UPDATE"] resources: ["deployments"] - validations: - - expression: "object.spec.replicas <= 5" + validations: + - expression: "object.spec.replicas <= 5" --- apiVersion: kyverno.io/v1 kind: ClusterPolicy diff --git a/test/cli/scenarios_to_cli/other/scenario_mutate_endpoint/policy.yaml b/test/cli/scenarios_to_cli/other/scenario_mutate_endpoint/policy.yaml index a401dd951c..1daea413f7 100644 --- a/test/cli/scenarios_to_cli/other/scenario_mutate_endpoint/policy.yaml +++ b/test/cli/scenarios_to_cli/other/scenario_mutate_endpoint/policy.yaml @@ -10,9 +10,9 @@ spec: - resources: kinds: - Endpoints - selector: - matchLabels: - label: test + selector: + matchLabels: + label: test mutate: patchesJson6902: |- [ diff --git a/test/cli/test/deny-modify-platform-label-2/kyverno-test.yaml b/test/cli/test/deny-modify-platform-label-2/kyverno-test.yaml index abb7a17412..b27e1ea7a2 100644 --- a/test/cli/test/deny-modify-platform-label-2/kyverno-test.yaml +++ b/test/cli/test/deny-modify-platform-label-2/kyverno-test.yaml @@ -3,7 +3,6 @@ policies: - deny-modify-platform-label.yaml resources: - resource.yaml -variables: variables.yaml results: - kind: Role policy: deny-modify-platform-label @@ -15,11 +14,7 @@ results: policy: deny-modify-platform-label resources: - my-role-without-platform - result: skip - rule: deny-modify-platform-role -- kind: Role - policy: deny-modify-platform-label - resources: - my-role-with-platform-false result: skip rule: deny-modify-platform-role +variables: variables.yaml