From c3fc0dafd665e556e947f7a90f2d1f7ec87a0260 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Mon, 27 Nov 2023 01:35:31 -0800 Subject: [PATCH] Remove var check (#8990) * fix excessive logs Signed-off-by: Jim Bugwadia * remove variable check Signed-off-by: Jim Bugwadia --------- Signed-off-by: Jim Bugwadia --- .../kubectl-kyverno/commands/apply/command.go | 12 --- cmd/cli/kubectl-kyverno/commands/test/test.go | 12 --- cmd/cli/kubectl-kyverno/variables/new.go | 2 +- cmd/cli/kubectl-kyverno/variables/utils.go | 22 ----- .../kubectl-kyverno/variables/utils_test.go | 79 --------------- .../kubectl-kyverno/variables/variables.go | 16 ---- .../variables/variables_test.go | 96 ------------------- 7 files changed, 1 insertion(+), 238 deletions(-) delete mode 100644 cmd/cli/kubectl-kyverno/variables/utils.go delete mode 100644 cmd/cli/kubectl-kyverno/variables/utils_test.go diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index 55e63282c0..13f25662de 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -244,18 +244,6 @@ func (c *ApplyCommandConfig) applyPolicytoResource( } continue } - matches, err := policy.ExtractVariables(pol) - if err != nil { - log.Log.Error(err, "skipping invalid policy", "name", pol.GetName()) - continue - } - if !vars.HasVariables() && variables.NeedsVariables(matches...) { - // check policy in variable file - if !vars.HasPolicyVariables(pol.GetName()) { - fmt.Fprintf(out, "test skipped for policy %v (as required variables are not provided by the users) \n \n", pol.GetName()) - continue - } - } validPolicies = append(validPolicies, pol) } var rc processor.ResultCounts diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go index eea1b89f7a..79aaa4b334 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/test.go +++ b/cmd/cli/kubectl-kyverno/commands/test/test.go @@ -120,18 +120,6 @@ func runTest(out io.Writer, testCase test.TestCase, auditWarn bool) ([]engineapi log.Log.Error(err, "skipping invalid policy", "name", pol.GetName()) continue } - matches, err := policy.ExtractVariables(pol) - if err != nil { - log.Log.Error(err, "skipping invalid policy", "name", pol.GetName()) - continue - } - if !vars.HasVariables() && variables.NeedsVariables(matches...) { - // check policy in variable file - if !vars.HasPolicyVariables(pol.GetName()) { - fmt.Fprintln(out, " test skipped for policy", pol.GetName(), "(as required variables are not provided by the users)") - // continue - } - } validPolicies = append(validPolicies, pol) } // execute engine diff --git a/cmd/cli/kubectl-kyverno/variables/new.go b/cmd/cli/kubectl-kyverno/variables/new.go index 378b285034..fd71dad004 100644 --- a/cmd/cli/kubectl-kyverno/variables/new.go +++ b/cmd/cli/kubectl-kyverno/variables/new.go @@ -14,7 +14,7 @@ func New(fs billy.Filesystem, resourcePath string, path string, vals *v1alpha1.V if vals == nil && path != "" { v, err := values.Load(fs, filepath.Join(resourcePath, path)) if err != nil { - return nil, fmt.Errorf("Unable to load variable file: %s (%w)", path, err) + return nil, fmt.Errorf("unable to load variable file: %s (%w)", path, err) } vals = &v.ValuesSpec } diff --git a/cmd/cli/kubectl-kyverno/variables/utils.go b/cmd/cli/kubectl-kyverno/variables/utils.go deleted file mode 100644 index ca737418b9..0000000000 --- a/cmd/cli/kubectl-kyverno/variables/utils.go +++ /dev/null @@ -1,22 +0,0 @@ -package variables - -import ( - "strings" -) - -func NeedsVariable(variable string) bool { - return variable != "" && - !strings.Contains(variable, "request.object") && - !strings.Contains(variable, "request.operation") && - !strings.Contains(variable, "element") && - variable != "elementIndex" -} - -func NeedsVariables(variables ...string) bool { - for _, variable := range variables { - if NeedsVariable(variable) { - return true - } - } - return false -} diff --git a/cmd/cli/kubectl-kyverno/variables/utils_test.go b/cmd/cli/kubectl-kyverno/variables/utils_test.go deleted file mode 100644 index 6323acdfca..0000000000 --- a/cmd/cli/kubectl-kyverno/variables/utils_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package variables - -import ( - "testing" -) - -func TestNeedsVariable(t *testing.T) { - tests := []struct { - name string - want bool - }{{ - name: "", - want: false, - }, { - name: "request.object.spec", - want: false, - }, { - name: "request.operation", - want: false, - }, { - name: "element.spec.container", - want: false, - }, { - name: "elementIndex", - want: false, - }, { - name: "foo", - want: true, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := NeedsVariable(tt.name); got != tt.want { - t.Errorf("NeedsVariable() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestNeedsVariables(t *testing.T) { - tests := []struct { - name string - variables []string - want bool - }{{ - name: "nil", - variables: nil, - want: false, - }, { - name: "empty", - variables: []string{}, - want: false, - }, { - name: "false", - variables: []string{ - "request.object.spec", - "request.operation", - "element.spec.container", - "elementIndex", - }, - want: false, - }, { - name: "true", - variables: []string{ - "request.object.spec", - "request.operation", - "element.spec.container", - "elementIndex", - "foo", - }, - want: true, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := NeedsVariables(tt.variables...); got != tt.want { - t.Errorf("NeedsVariables() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/cmd/cli/kubectl-kyverno/variables/variables.go b/cmd/cli/kubectl-kyverno/variables/variables.go index 7d9901354e..9a62f2cb07 100644 --- a/cmd/cli/kubectl-kyverno/variables/variables.go +++ b/cmd/cli/kubectl-kyverno/variables/variables.go @@ -13,22 +13,6 @@ type Variables struct { variables map[string]string } -func (v Variables) HasVariables() bool { - return len(v.variables) != 0 -} - -func (v Variables) HasPolicyVariables(policy string) bool { - if v.values == nil { - return false - } - for _, pol := range v.values.Policies { - if pol.Name == policy { - return true - } - } - return false -} - func (v Variables) Subresources() []v1alpha1.Subresource { if v.values == nil { return nil diff --git a/cmd/cli/kubectl-kyverno/variables/variables_test.go b/cmd/cli/kubectl-kyverno/variables/variables_test.go index 3021674966..807726d8f2 100644 --- a/cmd/cli/kubectl-kyverno/variables/variables_test.go +++ b/cmd/cli/kubectl-kyverno/variables/variables_test.go @@ -10,43 +10,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" ) -func TestVariables_HasVariables(t *testing.T) { - tests := []struct { - name string - values *v1alpha1.ValuesSpec - variables map[string]string - want bool - }{{ - name: "nil", - values: nil, - variables: nil, - want: false, - }, { - name: "empty", - values: nil, - variables: map[string]string{}, - want: false, - }, { - name: "not empty", - values: nil, - variables: map[string]string{ - "foo": "bar", - }, - want: true, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - v := Variables{ - values: tt.values, - variables: tt.variables, - } - if got := v.HasVariables(); got != tt.want { - t.Errorf("Variables.HasVariables() = %v, want %v", got, tt.want) - } - }) - } -} - func TestVariables_Subresources(t *testing.T) { tests := []struct { name string @@ -177,65 +140,6 @@ func TestVariables_SetInStore(t *testing.T) { } } -func TestVariables_HasPolicyVariables(t *testing.T) { - vals, err := values.Load(nil, "../_testdata/values/limit-configmap-for-sa.yaml") - assert.NoError(t, err) - vals.ValuesSpec.Policies = append(vals.ValuesSpec.Policies, v1alpha1.Policy{ - Name: "limit-configmap-for-sa", - Rules: []v1alpha1.Rule{{ - Name: "rule", - Values: map[string]interface{}{ - "foo": "bar", - }, - ForeachValues: map[string][]interface{}{ - "baz": nil, - }, - }}, - }) - tests := []struct { - name string - values *v1alpha1.ValuesSpec - variables map[string]string - policy string - want bool - }{{ - name: "nil", - values: nil, - variables: nil, - policy: "test", - want: false, - }, { - name: "empty", - values: &v1alpha1.ValuesSpec{}, - variables: nil, - policy: "test", - want: false, - }, { - name: "values - test", - values: &vals.ValuesSpec, - variables: nil, - policy: "test", - want: false, - }, { - name: "values - limit-configmap-for-sa", - values: &vals.ValuesSpec, - variables: nil, - policy: "limit-configmap-for-sa", - want: true, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - v := Variables{ - values: tt.values, - variables: tt.variables, - } - if got := v.HasPolicyVariables(tt.policy); got != tt.want { - t.Errorf("Variables.HasPolicyVariables() = %v, want %v", got, tt.want) - } - }) - } -} - func TestVariables_ComputeVariables(t *testing.T) { loadValues := func(path string) *v1alpha1.ValuesSpec { t.Helper()