From a4b3388bdaa95ebd31bf5c62a0a111dafe874ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 7 Sep 2023 11:16:26 +0200 Subject: [PATCH] chore: improve unit tests in cli (#8304) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .../policies/cpol-limit-configmap-for-sa.yaml | 55 +++++++ .../_testdata/values/global-values.yaml | 3 + ...valid.yaml => limit-configmap-for-sa.yaml} | 0 .../kubectl-kyverno/commands/apply/command.go | 2 +- cmd/cli/kubectl-kyverno/commands/test/test.go | 2 +- cmd/cli/kubectl-kyverno/log/log_test.go | 9 ++ .../kubectl-kyverno/policy/variables_test.go | 61 ++++++++ cmd/cli/kubectl-kyverno/values/load_test.go | 6 +- cmd/cli/kubectl-kyverno/variables/new_test.go | 6 +- .../kubectl-kyverno/variables/variables.go | 2 +- .../variables/variables_test.go | 140 +++++++++++++++++- 11 files changed, 274 insertions(+), 12 deletions(-) create mode 100644 cmd/cli/kubectl-kyverno/_testdata/policies/cpol-limit-configmap-for-sa.yaml create mode 100644 cmd/cli/kubectl-kyverno/_testdata/values/global-values.yaml rename cmd/cli/kubectl-kyverno/_testdata/values/{valid.yaml => limit-configmap-for-sa.yaml} (100%) create mode 100644 cmd/cli/kubectl-kyverno/log/log_test.go create mode 100644 cmd/cli/kubectl-kyverno/policy/variables_test.go diff --git a/cmd/cli/kubectl-kyverno/_testdata/policies/cpol-limit-configmap-for-sa.yaml b/cmd/cli/kubectl-kyverno/_testdata/policies/cpol-limit-configmap-for-sa.yaml new file mode 100644 index 0000000000..af9a9baeba --- /dev/null +++ b/cmd/cli/kubectl-kyverno/_testdata/policies/cpol-limit-configmap-for-sa.yaml @@ -0,0 +1,55 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: limit-configmap-for-sa + annotations: + policies.kyverno.io/title: Limit ConfigMap to ServiceAccounts for a User + policies.kyverno.io/category: Other + policies.kyverno.io/severity: medium + kyverno.io/kyverno-version: 1.6.0 + kyverno.io/kubernetes-version: "1.20-1.23" + policies.kyverno.io/subject: ConfigMap, ServiceAccount + policies.kyverno.io/description: This policy shows how to restrict certain operations on specific ConfigMaps by ServiceAccounts. +spec: + background: false + validationFailureAction: audit + rules: + - name: limit-configmap-for-sa-developer + match: + any: + - resources: + kinds: + - ConfigMap + subjects: + - kind: ServiceAccount + name: developer + namespace: kube-system + - resources: + kinds: + - ConfigMap + subjects: + - kind: ServiceAccount + name: another-developer + namespace: another-namespace + preconditions: + all: + - key: "{{request.object.metadata.namespace}}" + operator: In + value: + - "any-namespace" + - "another-namespace" + - key: "{{request.object.metadata.name}}" + operator: In + value: + - "any-configmap-name-good" + - "another-configmap-name" + validate: + message: "{{request.object.metadata.namespace}}/{{request.object.kind}}/{{request.object.metadata.name}} resource is protected. Admin or allowed users can change the resource" + deny: + conditions: + all: + - key: "{{request.operation}}" + operator: "In" + value: + - "UPDATE" + - "CREATE" diff --git a/cmd/cli/kubectl-kyverno/_testdata/values/global-values.yaml b/cmd/cli/kubectl-kyverno/_testdata/values/global-values.yaml new file mode 100644 index 0000000000..92d4a627e4 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/_testdata/values/global-values.yaml @@ -0,0 +1,3 @@ +globalValues: + foo: bar + baz: jee diff --git a/cmd/cli/kubectl-kyverno/_testdata/values/valid.yaml b/cmd/cli/kubectl-kyverno/_testdata/values/limit-configmap-for-sa.yaml similarity index 100% rename from cmd/cli/kubectl-kyverno/_testdata/values/valid.yaml rename to cmd/cli/kubectl-kyverno/_testdata/values/limit-configmap-for-sa.yaml diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index eb8117695b..d320c6abc5 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -268,7 +268,7 @@ func (c *ApplyCommandConfig) applyPolicytoResource( } } kindOnwhichPolicyIsApplied := common.GetKindsFromPolicy(pol, vars.Subresources(), dClient) - resourceValues, err := vars.CheckVariableForPolicy(pol.GetName(), resource.GetName(), resource.GetKind(), kindOnwhichPolicyIsApplied, matches...) + resourceValues, err := vars.ComputeVariables(pol.GetName(), resource.GetName(), resource.GetKind(), kindOnwhichPolicyIsApplied, matches...) if err != nil { return &rc, resources, skipInvalidPolicies, responses, sanitizederror.NewWithError(fmt.Sprintf("policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag", pol.GetName(), resource.GetName()), err) } diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go index a0ad4015df..3e44224b89 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/test.go +++ b/cmd/cli/kubectl-kyverno/commands/test/test.go @@ -140,7 +140,7 @@ func runTest(openApiManager openapi.Manager, testCase test.TestCase, auditWarn b kindOnwhichPolicyIsApplied := common.GetKindsFromPolicy(pol, vars.Subresources(), dClient) for _, resource := range uniques { - resourceValues, err := vars.CheckVariableForPolicy(pol.GetName(), resource.GetName(), resource.GetKind(), kindOnwhichPolicyIsApplied, matches...) + resourceValues, err := vars.ComputeVariables(pol.GetName(), resource.GetName(), resource.GetKind(), kindOnwhichPolicyIsApplied, matches...) if err != nil { message := fmt.Sprintf( "policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag", diff --git a/cmd/cli/kubectl-kyverno/log/log_test.go b/cmd/cli/kubectl-kyverno/log/log_test.go new file mode 100644 index 0000000000..bee13202be --- /dev/null +++ b/cmd/cli/kubectl-kyverno/log/log_test.go @@ -0,0 +1,9 @@ +package log + +import "testing" + +func TestConfigure(t *testing.T) { + if err := Configure(); (err != nil) != false { + t.Errorf("Configure() error = %v, wantErr %v", err, false) + } +} diff --git a/cmd/cli/kubectl-kyverno/policy/variables_test.go b/cmd/cli/kubectl-kyverno/policy/variables_test.go new file mode 100644 index 0000000000..8fa69036a4 --- /dev/null +++ b/cmd/cli/kubectl-kyverno/policy/variables_test.go @@ -0,0 +1,61 @@ +package policy + +import ( + "reflect" + "testing" + + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/stretchr/testify/assert" +) + +func TestExtractVariables(t *testing.T) { + loadPolicy := func(path string) kyvernov1.PolicyInterface { + t.Helper() + policies, _, err := Load(nil, "", path) + assert.NoError(t, err) + assert.Equal(t, len(policies), 1) + return policies[0] + + } + tests := []struct { + name string + policy kyvernov1.PolicyInterface + want []string + wantErr bool + }{{ + name: "nil", + policy: nil, + want: nil, + wantErr: false, + }, { + name: "cpol-pod-requirements", + policy: loadPolicy("../_testdata/policies/cpol-pod-requirements.yaml"), + want: nil, + wantErr: false, + }, { + name: "cpol-limit-configmap-for-sa", + policy: loadPolicy("../_testdata/policies/cpol-limit-configmap-for-sa.yaml"), + want: []string{ + "{{request.object.metadata.namespace}}", + "{{request.object.metadata.name}}", + "{{request.object.metadata.namespace}}", + "{{request.object.kind}}", + "{{request.object.metadata.name}}", + "{{request.operation}}", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ExtractVariables(tt.policy) + if (err != nil) != tt.wantErr { + t.Errorf("ExtractVariables() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ExtractVariables() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/cmd/cli/kubectl-kyverno/values/load_test.go b/cmd/cli/kubectl-kyverno/values/load_test.go index e447a7e9bc..c32f43a1ec 100644 --- a/cmd/cli/kubectl-kyverno/values/load_test.go +++ b/cmd/cli/kubectl-kyverno/values/load_test.go @@ -42,8 +42,8 @@ func Test_readFile(t *testing.T) { wantErr: false, }, { name: "valid", - filepath: "../_testdata/values/valid.yaml", - want: mustReadFile("../_testdata/values/valid.yaml"), + filepath: "../_testdata/values/limit-configmap-for-sa.yaml", + want: mustReadFile("../_testdata/values/limit-configmap-for-sa.yaml"), wantErr: false, }, { name: "empty (billy)", @@ -107,7 +107,7 @@ func TestLoad(t *testing.T) { wantErr: true, }, { name: "valid", - filepath: "../_testdata/values/valid.yaml", + filepath: "../_testdata/values/limit-configmap-for-sa.yaml", want: &valuesapi.Values{ NamespaceSelectors: []valuesapi.NamespaceSelector{{ Name: "test1", diff --git a/cmd/cli/kubectl-kyverno/variables/new_test.go b/cmd/cli/kubectl-kyverno/variables/new_test.go index 116bdff47e..90d91e2a1b 100644 --- a/cmd/cli/kubectl-kyverno/variables/new_test.go +++ b/cmd/cli/kubectl-kyverno/variables/new_test.go @@ -89,7 +89,7 @@ func TestNew(t *testing.T) { name: "values file", fs: nil, resourcePath: "", - path: "../_testdata/values/valid.yaml", + path: "../_testdata/values/limit-configmap-for-sa.yaml", vals: nil, vars: nil, want: &Variables{ @@ -121,7 +121,7 @@ func TestNew(t *testing.T) { name: "values file and vars", fs: nil, resourcePath: "", - path: "../_testdata/values/valid.yaml", + path: "../_testdata/values/limit-configmap-for-sa.yaml", vals: nil, vars: []string{ "foo=bar", @@ -167,7 +167,7 @@ func TestNew(t *testing.T) { name: "values and values file", fs: nil, resourcePath: "", - path: "../_testdata/values/valid.yaml", + path: "../_testdata/values/limit-configmap-for-sa.yaml", vals: &valuesapi.Values{ GlobalValues: map[string]string{ "bar": "baz", diff --git a/cmd/cli/kubectl-kyverno/variables/variables.go b/cmd/cli/kubectl-kyverno/variables/variables.go index 3cb45e6b12..9c90dd637d 100644 --- a/cmd/cli/kubectl-kyverno/variables/variables.go +++ b/cmd/cli/kubectl-kyverno/variables/variables.go @@ -55,7 +55,7 @@ func (v Variables) NamespaceSelectors() map[string]Labels { return out } -func (v Variables) CheckVariableForPolicy(policy, resource, kind string, kindMap sets.Set[string], variables ...string) (map[string]interface{}, error) { +func (v Variables) ComputeVariables(policy, resource, kind string, kindMap sets.Set[string], variables ...string) (map[string]interface{}, error) { resourceValues := map[string]interface{}{} // first apply global values if v.values != nil { diff --git a/cmd/cli/kubectl-kyverno/variables/variables_test.go b/cmd/cli/kubectl-kyverno/variables/variables_test.go index 67251eeff2..cfb01d7a18 100644 --- a/cmd/cli/kubectl-kyverno/variables/variables_test.go +++ b/cmd/cli/kubectl-kyverno/variables/variables_test.go @@ -7,6 +7,7 @@ import ( valuesapi "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/apis/values" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/values" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/sets" ) func TestVariables_HasVariables(t *testing.T) { @@ -93,7 +94,7 @@ func TestVariables_Subresources(t *testing.T) { } func TestVariables_NamespaceSelectors(t *testing.T) { - vals, err := values.Load(nil, "../_testdata/values/valid.yaml") + vals, err := values.Load(nil, "../_testdata/values/limit-configmap-for-sa.yaml") assert.NoError(t, err) tests := []struct { name string @@ -134,7 +135,7 @@ func TestVariables_NamespaceSelectors(t *testing.T) { } func TestVariables_SetInStore(t *testing.T) { - vals, err := values.Load(nil, "../_testdata/values/valid.yaml") + vals, err := values.Load(nil, "../_testdata/values/limit-configmap-for-sa.yaml") assert.NoError(t, err) vals.Policies = append(vals.Policies, valuesapi.Policy{ Name: "limit-configmap-for-sa", @@ -177,7 +178,7 @@ func TestVariables_SetInStore(t *testing.T) { } func TestVariables_HasPolicyVariables(t *testing.T) { - vals, err := values.Load(nil, "../_testdata/values/valid.yaml") + vals, err := values.Load(nil, "../_testdata/values/limit-configmap-for-sa.yaml") assert.NoError(t, err) vals.Policies = append(vals.Policies, valuesapi.Policy{ Name: "limit-configmap-for-sa", @@ -234,3 +235,136 @@ func TestVariables_HasPolicyVariables(t *testing.T) { }) } } + +func TestVariables_ComputeVariables(t *testing.T) { + loadValues := func(path string) *valuesapi.Values { + t.Helper() + vals, err := values.Load(nil, path) + assert.NoError(t, err) + return vals + } + type fields struct { + values *valuesapi.Values + variables map[string]string + } + type args struct { + policy string + resource string + kind string + kindMap sets.Set[string] + variables []string + } + tests := []struct { + name string + fields fields + args args + want map[string]interface{} + wantErr bool + }{ + { + name: "nil", + args: args{ + "limit-configmap-for-sa", + "any-configmap-name-good", + "ConfigMap", + nil, + nil, + }, + want: map[string]interface{}{ + "request.operation": "CREATE", + }, + wantErr: false, + }, + { + name: "values", + fields: fields{ + loadValues("../_testdata/values/limit-configmap-for-sa.yaml"), + nil, + }, + args: args{ + "limit-configmap-for-sa", + "any-configmap-name-good", + "ConfigMap", + nil, + nil, + }, + want: map[string]interface{}{ + "request.operation": "UPDATE", + }, + wantErr: false, + }, { + name: "values", + fields: fields{ + loadValues("../_testdata/values/limit-configmap-for-sa.yaml"), + nil, + }, + args: args{ + "test", + "any-configmap-name-good", + "ConfigMap", + nil, + nil, + }, + want: map[string]interface{}{ + "request.operation": "CREATE", + }, + wantErr: false, + }, { + name: "values", + fields: fields{ + loadValues("../_testdata/values/global-values.yaml"), + nil, + }, + args: args{ + "test", + "any-configmap-name-good", + "ConfigMap", + nil, + nil, + }, + want: map[string]interface{}{ + "baz": "jee", + "foo": "bar", + "request.operation": "CREATE", + }, + wantErr: false, + }, { + name: "values and variables", + fields: fields{ + loadValues("../_testdata/values/global-values.yaml"), + map[string]string{ + "request.operation": "DELETE", + }, + }, + args: args{ + "test", + "any-configmap-name-good", + "ConfigMap", + nil, + nil, + }, + want: map[string]interface{}{ + "baz": "jee", + "foo": "bar", + "request.operation": "DELETE", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := Variables{ + values: tt.fields.values, + variables: tt.fields.variables, + } + got, err := v.ComputeVariables(tt.args.policy, tt.args.resource, tt.args.kind, tt.args.kindMap, tt.args.variables...) + if (err != nil) != tt.wantErr { + t.Errorf("Variables.ComputeVariables() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Variables.ComputeVariables() = %v, want %v", got, tt.want) + } + }) + } +}