From 584f841c1e70dac37f6fb0a04a474f843dffa15c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?=
<charles.edouard@nirmata.com>
Date: Tue, 19 Dec 2023 15:45:53 +0100
Subject: [PATCH] refactor: make CLI store non static (#9200)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* refactor: make CLI store non static
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
* registry access
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
* apply
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
* tests
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
---------
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
---
.../kubectl-kyverno/commands/apply/command.go | 10 +++-
.../kubectl-kyverno/commands/test/command.go | 7 +--
cmd/cli/kubectl-kyverno/commands/test/test.go | 7 ++-
cmd/cli/kubectl-kyverno/processor/generate.go | 8 +--
.../processor/policy_processor.go | 7 ++-
.../processor/policy_processor_test.go | 2 +
.../kubectl-kyverno/store/contextloader.go | 18 +++---
cmd/cli/kubectl-kyverno/store/store.go | 56 +++++++++----------
.../kubectl-kyverno/variables/variables.go | 8 +--
.../variables/variables_test.go | 7 ++-
10 files changed, 73 insertions(+), 57 deletions(-)
diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go
index 13f25662de..4aaebb2b52 100644
--- a/cmd/cli/kubectl-kyverno/commands/apply/command.go
+++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go
@@ -141,7 +141,8 @@ func (c *ApplyCommandConfig) applyCommandHelper(out io.Writer) (*processor.Resul
if err != nil {
return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("failed to decode yaml (%w)", err)
}
- rc, resources1, skipInvalidPolicies, responses1, err, dClient := c.initStoreAndClusterClient(skipInvalidPolicies)
+ var store store.Store
+ rc, resources1, skipInvalidPolicies, responses1, err, dClient := c.initStoreAndClusterClient(&store, skipInvalidPolicies)
if err != nil {
return rc, resources1, skipInvalidPolicies, responses1, err
}
@@ -163,6 +164,7 @@ func (c *ApplyCommandConfig) applyCommandHelper(out io.Writer) (*processor.Resul
}
rc, resources1, responses1, err = c.applyPolicytoResource(
out,
+ &store,
variables,
policies,
resources,
@@ -219,6 +221,7 @@ func (c *ApplyCommandConfig) applyValidatingAdmissionPolicytoResource(
func (c *ApplyCommandConfig) applyPolicytoResource(
out io.Writer,
+ store *store.Store,
vars *variables.Variables,
policies []kyvernov1.PolicyInterface,
resources []*unstructured.Unstructured,
@@ -228,7 +231,7 @@ func (c *ApplyCommandConfig) applyPolicytoResource(
mutateLogPathIsDir bool,
) (*processor.ResultCounts, []*unstructured.Unstructured, []engineapi.EngineResponse, error) {
if vars != nil {
- vars.SetInStore()
+ vars.SetInStore(store)
}
// validate policies
var validPolicies []kyvernov1.PolicyInterface
@@ -250,6 +253,7 @@ func (c *ApplyCommandConfig) applyPolicytoResource(
var responses []engineapi.EngineResponse
for _, resource := range resources {
processor := processor.PolicyProcessor{
+ Store: store,
Policies: validPolicies,
Resource: *resource,
MutateLogPath: c.MutateLogPath,
@@ -336,7 +340,7 @@ func (c *ApplyCommandConfig) loadPolicies(skipInvalidPolicies SkippedInvalidPoli
return nil, nil, skipInvalidPolicies, nil, nil, policies, validatingAdmissionPolicies
}
-func (c *ApplyCommandConfig) initStoreAndClusterClient(skipInvalidPolicies SkippedInvalidPolicies) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error, dclient.Interface) {
+func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, skipInvalidPolicies SkippedInvalidPolicies) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error, dclient.Interface) {
store.SetLocal(true)
store.SetRegistryAccess(c.RegistryAccess)
if c.Cluster {
diff --git a/cmd/cli/kubectl-kyverno/commands/test/command.go b/cmd/cli/kubectl-kyverno/commands/test/command.go
index deb98cebc8..27447c2e7c 100644
--- a/cmd/cli/kubectl-kyverno/commands/test/command.go
+++ b/cmd/cli/kubectl-kyverno/commands/test/command.go
@@ -11,7 +11,6 @@ import (
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/color"
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/output/table"
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/report"
- "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/store"
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/filter"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
"github.com/spf13/cobra"
@@ -31,8 +30,7 @@ func Command() *cobra.Command {
SilenceUsage: true,
RunE: func(cmd *cobra.Command, dirPath []string) (err error) {
color.Init(removeColor)
- store.SetRegistryAccess(registryAccess)
- return testCommandExecute(cmd.OutOrStdout(), dirPath, fileName, gitBranch, testCase, failOnly, detailedResults)
+ return testCommandExecute(cmd.OutOrStdout(), dirPath, fileName, gitBranch, testCase, registryAccess, failOnly, detailedResults)
},
}
cmd.Flags().StringVarP(&fileName, "file-name", "f", "kyverno-test.yaml", "Test filename")
@@ -57,6 +55,7 @@ func testCommandExecute(
fileName string,
gitBranch string,
testCase string,
+ registryAccess bool,
failOnly bool,
detailedResults bool,
) (err error) {
@@ -115,7 +114,7 @@ func testCommandExecute(
continue
}
resourcePath := filepath.Dir(test.Path)
- responses, err := runTest(out, test, false)
+ responses, err := runTest(out, test, registryAccess, false)
if err != nil {
return fmt.Errorf("failed to run test (%w)", err)
}
diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go
index 05b7bcfb7f..e41ccccbfc 100644
--- a/cmd/cli/kubectl-kyverno/commands/test/test.go
+++ b/cmd/cli/kubectl-kyverno/commands/test/test.go
@@ -26,7 +26,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
-func runTest(out io.Writer, testCase test.TestCase, auditWarn bool) ([]engineapi.EngineResponse, error) {
+func runTest(out io.Writer, testCase test.TestCase, registryAccess bool, auditWarn bool) ([]engineapi.EngineResponse, error) {
// don't process test case with errors
if testCase.Err != nil {
return nil, testCase.Err
@@ -73,9 +73,11 @@ func runTest(out io.Writer, testCase test.TestCase, auditWarn bool) ([]engineapi
}
}
// init store
+ var store store.Store
store.SetLocal(true)
+ store.SetRegistryAccess(registryAccess)
if vars != nil {
- vars.SetInStore()
+ vars.SetInStore(&store)
}
fmt.Fprintln(out, " Applying", len(policies)+len(validatingAdmissionPolicies), pluralize.Pluralize(len(policies)+len(validatingAdmissionPolicies), "policy", "policies"), "to", len(uniques), pluralize.Pluralize(len(uniques), "resource", "resources"), "...")
// TODO document the code below
@@ -137,6 +139,7 @@ func runTest(out io.Writer, testCase test.TestCase, auditWarn bool) ([]engineapi
for _, resource := range uniques {
processor := processor.PolicyProcessor{
+ Store: &store,
Policies: validPolicies,
Resource: *resource,
MutateLogPath: "",
diff --git a/cmd/cli/kubectl-kyverno/processor/generate.go b/cmd/cli/kubectl-kyverno/processor/generate.go
index ce767e52b8..073de09578 100644
--- a/cmd/cli/kubectl-kyverno/processor/generate.go
+++ b/cmd/cli/kubectl-kyverno/processor/generate.go
@@ -25,7 +25,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
)
-func handleGeneratePolicy(out io.Writer, generateResponse *engineapi.EngineResponse, policyContext engine.PolicyContext, ruleToCloneSourceResource map[string]string) ([]engineapi.RuleResponse, error) {
+func handleGeneratePolicy(out io.Writer, store *store.Store, generateResponse *engineapi.EngineResponse, policyContext engine.PolicyContext, ruleToCloneSourceResource map[string]string) ([]engineapi.RuleResponse, error) {
newResource := policyContext.NewResource()
objects := []runtime.Object{&newResource}
for _, rule := range generateResponse.PolicyResponse.Rules {
@@ -74,7 +74,7 @@ func handleGeneratePolicy(out io.Writer, generateResponse *engineapi.EngineRespo
}
}
- c, err := initializeMockController(out, listKinds, objects)
+ c, err := initializeMockController(out, store, listKinds, objects)
if err != nil {
fmt.Fprintln(out, "error at controller")
return nil, err
@@ -113,7 +113,7 @@ func handleGeneratePolicy(out io.Writer, generateResponse *engineapi.EngineRespo
return newRuleResponse, nil
}
-func initializeMockController(out io.Writer, gvrToListKind map[schema.GroupVersionResource]string, objects []runtime.Object) (*generate.GenerateController, error) {
+func initializeMockController(out io.Writer, s *store.Store, gvrToListKind map[schema.GroupVersionResource]string, objects []runtime.Object) (*generate.GenerateController, error) {
client, err := dclient.NewFakeClient(runtime.NewScheme(), gvrToListKind, objects...)
if err != nil {
fmt.Fprintf(out, "Failed to mock dynamic client")
@@ -133,7 +133,7 @@ func initializeMockController(out io.Writer, gvrToListKind map[schema.GroupVersi
adapters.Client(client),
nil,
imageverifycache.DisabledImageVerifyCache(),
- store.ContextLoaderFactory(nil),
+ store.ContextLoaderFactory(s, nil),
nil,
"",
))
diff --git a/cmd/cli/kubectl-kyverno/processor/policy_processor.go b/cmd/cli/kubectl-kyverno/processor/policy_processor.go
index 45061d452b..4dbb7059c8 100644
--- a/cmd/cli/kubectl-kyverno/processor/policy_processor.go
+++ b/cmd/cli/kubectl-kyverno/processor/policy_processor.go
@@ -35,6 +35,7 @@ import (
)
type PolicyProcessor struct {
+ Store *store.Store
Policies []kyvernov1.PolicyInterface
Resource unstructured.Unstructured
MutateLogPath string
@@ -70,7 +71,7 @@ func (p *PolicyProcessor) ApplyPoliciesOnResource() ([]engineapi.EngineResponse,
client,
factories.DefaultRegistryClientFactory(adapters.RegistryClient(rclient), nil),
imageverifycache.DisabledImageVerifyCache(),
- store.ContextLoaderFactory(nil),
+ store.ContextLoaderFactory(p.Store, nil),
nil,
"",
)
@@ -177,7 +178,7 @@ func (p *PolicyProcessor) ApplyPoliciesOnResource() ([]engineapi.EngineResponse,
}
generateResponse := eng.ApplyBackgroundChecks(context.TODO(), policyContext)
if !generateResponse.IsEmpty() {
- newRuleResponse, err := handleGeneratePolicy(p.Out, &generateResponse, *policyContext, p.RuleToCloneSourceResource)
+ newRuleResponse, err := handleGeneratePolicy(p.Out, p.Store, &generateResponse, *policyContext, p.RuleToCloneSourceResource)
if err != nil {
log.Log.Error(err, "failed to apply generate policy")
} else {
@@ -205,7 +206,7 @@ func (p *PolicyProcessor) makePolicyContext(
var resourceValues map[string]interface{}
if p.Variables != nil {
kindOnwhichPolicyIsApplied := common.GetKindsFromPolicy(p.Out, policy, p.Variables.Subresources(), p.Client)
- vals, err := p.Variables.ComputeVariables(policy.GetName(), resource.GetName(), resource.GetKind(), kindOnwhichPolicyIsApplied /*matches...*/)
+ vals, err := p.Variables.ComputeVariables(p.Store, policy.GetName(), resource.GetName(), resource.GetKind(), kindOnwhichPolicyIsApplied /*matches...*/)
if err != nil {
return nil, fmt.Errorf("policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag (%w)",
policy.GetName(),
diff --git a/cmd/cli/kubectl-kyverno/processor/policy_processor_test.go b/cmd/cli/kubectl-kyverno/processor/policy_processor_test.go
index 60f7ba6fb1..0b5dd68003 100644
--- a/cmd/cli/kubectl-kyverno/processor/policy_processor_test.go
+++ b/cmd/cli/kubectl-kyverno/processor/policy_processor_test.go
@@ -5,6 +5,7 @@ import (
"testing"
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource"
+ "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/store"
yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
"gotest.tools/assert"
)
@@ -107,6 +108,7 @@ func Test_NamespaceSelector(t *testing.T) {
policyArray, _, _ := yamlutils.GetPolicy(tc.policy)
resourceArray, _ := resource.GetUnstructuredResources(tc.resource)
processor := PolicyProcessor{
+ Store: &store.Store{},
Policies: policyArray,
Resource: *resourceArray[0],
MutateLogPath: "",
diff --git a/cmd/cli/kubectl-kyverno/store/contextloader.go b/cmd/cli/kubectl-kyverno/store/contextloader.go
index ace392914e..c9153608a9 100644
--- a/cmd/cli/kubectl-kyverno/store/contextloader.go
+++ b/cmd/cli/kubectl-kyverno/store/contextloader.go
@@ -10,13 +10,13 @@ import (
"github.com/kyverno/kyverno/pkg/engine/jmespath"
)
-func ContextLoaderFactory(cmResolver engineapi.ConfigmapResolver) engineapi.ContextLoaderFactory {
- if !IsLocal() {
+func ContextLoaderFactory(s *Store, cmResolver engineapi.ConfigmapResolver) engineapi.ContextLoaderFactory {
+ if !s.IsLocal() {
return factories.DefaultContextLoaderFactory(cmResolver)
}
return func(policy kyvernov1.PolicyInterface, rule kyvernov1.Rule) engineapi.ContextLoader {
init := func(jsonContext enginecontext.Interface) error {
- rule := GetPolicyRule(policy.GetName(), rule.Name)
+ rule := s.GetPolicyRule(policy.GetName(), rule.Name)
if rule != nil && len(rule.Values) > 0 {
variables := rule.Values
for key, value := range variables {
@@ -27,7 +27,7 @@ func ContextLoaderFactory(cmResolver engineapi.ConfigmapResolver) engineapi.Cont
}
if rule != nil && len(rule.ForEachValues) > 0 {
for key, value := range rule.ForEachValues {
- if err := jsonContext.AddVariable(key, value[GetForeachElement()]); err != nil {
+ if err := jsonContext.AddVariable(key, value[s.GetForeachElement()]); err != nil {
return err
}
}
@@ -35,11 +35,15 @@ func ContextLoaderFactory(cmResolver engineapi.ConfigmapResolver) engineapi.Cont
return nil
}
factory := factories.DefaultContextLoaderFactory(cmResolver, factories.WithInitializer(init))
- return wrapper{factory(policy, rule)}
+ return wrapper{
+ store: s,
+ inner: factory(policy, rule),
+ }
}
}
type wrapper struct {
+ store *Store
inner engineapi.ContextLoader
}
@@ -51,10 +55,10 @@ func (w wrapper) Load(
contextEntries []kyvernov1.ContextEntry,
jsonContext enginecontext.Interface,
) error {
- if !IsApiCallAllowed() {
+ if !w.store.IsApiCallAllowed() {
client = nil
}
- if !GetRegistryAccess() {
+ if !w.store.GetRegistryAccess() {
rclientFactory = nil
}
return w.inner.Load(ctx, jp, client, rclientFactory, contextEntries, jsonContext)
diff --git a/cmd/cli/kubectl-kyverno/store/store.go b/cmd/cli/kubectl-kyverno/store/store.go
index c59ac677ac..9bceebc764 100644
--- a/cmd/cli/kubectl-kyverno/store/store.go
+++ b/cmd/cli/kubectl-kyverno/store/store.go
@@ -19,56 +19,56 @@ type Rule struct {
ForEachValues map[string][]interface{} `json:"foreachValues"`
}
-var (
+type Store struct {
local bool
registryClient registryclient.Client
allowApiCalls bool
policies []Policy
foreachElement int
-)
+}
// SetLocal sets local (clusterless) execution for the CLI
-func SetLocal(m bool) {
- local = m
+func (s *Store) SetLocal(m bool) {
+ s.local = m
}
// IsLocal returns 'true' if the CLI is in local (clusterless) execution
-func IsLocal() bool {
- return local
+func (s *Store) IsLocal() bool {
+ return s.local
}
-func SetForEachElement(element int) {
- foreachElement = element
+func (s *Store) SetForEachElement(element int) {
+ s.foreachElement = element
}
-func GetForeachElement() int {
- return foreachElement
+func (s *Store) GetForeachElement() int {
+ return s.foreachElement
}
-func SetRegistryAccess(access bool) {
+func (s *Store) SetRegistryAccess(access bool) {
if access {
- registryClient = registryclient.NewOrDie(registryclient.WithLocalKeychain())
+ s.registryClient = registryclient.NewOrDie(registryclient.WithLocalKeychain())
}
}
-func GetRegistryAccess() bool {
- return registryClient != nil
+func (s *Store) GetRegistryAccess() bool {
+ return s.registryClient != nil
}
-func GetRegistryClient() registryclient.Client {
- return registryClient
+func (s *Store) GetRegistryClient() registryclient.Client {
+ return s.registryClient
}
-func SetPolicies(p ...Policy) {
- policies = p
+func (s *Store) SetPolicies(p ...Policy) {
+ s.policies = p
}
-func HasPolicies() bool {
- return len(policies) != 0
+func (s *Store) HasPolicies() bool {
+ return len(s.policies) != 0
}
-func GetPolicy(policyName string) *Policy {
- for _, policy := range policies {
+func (s *Store) GetPolicy(policyName string) *Policy {
+ for _, policy := range s.policies {
if policy.Name == policyName {
return &policy
}
@@ -76,8 +76,8 @@ func GetPolicy(policyName string) *Policy {
return nil
}
-func GetPolicyRule(policyName string, ruleName string) *Rule {
- for _, policy := range policies {
+func (s *Store) GetPolicyRule(policyName string, ruleName string) *Rule {
+ for _, policy := range s.policies {
if policy.Name == policyName {
for _, rule := range policy.Rules {
switch ruleName {
@@ -90,10 +90,10 @@ func GetPolicyRule(policyName string, ruleName string) *Rule {
return nil
}
-func AllowApiCall(allow bool) {
- allowApiCalls = allow
+func (s *Store) AllowApiCall(allow bool) {
+ s.allowApiCalls = allow
}
-func IsApiCallAllowed() bool {
- return allowApiCalls
+func (s *Store) IsApiCallAllowed() bool {
+ return s.allowApiCalls
}
diff --git a/cmd/cli/kubectl-kyverno/variables/variables.go b/cmd/cli/kubectl-kyverno/variables/variables.go
index 9a62f2cb07..9babb7aeab 100644
--- a/cmd/cli/kubectl-kyverno/variables/variables.go
+++ b/cmd/cli/kubectl-kyverno/variables/variables.go
@@ -39,7 +39,7 @@ func (v Variables) NamespaceSelectors() map[string]Labels {
return out
}
-func (v Variables) ComputeVariables(policy, resource, kind string, kindMap sets.Set[string], variables ...string) (map[string]interface{}, error) {
+func (v Variables) ComputeVariables(s *store.Store, 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 {
@@ -73,13 +73,13 @@ func (v Variables) ComputeVariables(policy, resource, kind string, kindMap sets.
}
// skipping the variable check for non matching kind
// TODO remove dependency to store
- if kindMap.Has(kind) && len(variables) > 0 && len(resourceValues) == 0 && store.HasPolicies() {
+ if kindMap.Has(kind) && len(variables) > 0 && len(resourceValues) == 0 && s.HasPolicies() {
return nil, fmt.Errorf("policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag", policy, resource)
}
return resourceValues, nil
}
-func (v Variables) SetInStore() {
+func (v Variables) SetInStore(s *store.Store) {
storePolicies := []store.Policy{}
if v.values != nil {
for _, p := range v.values.Policies {
@@ -97,5 +97,5 @@ func (v Variables) SetInStore() {
storePolicies = append(storePolicies, sp)
}
}
- store.SetPolicies(storePolicies...)
+ s.SetPolicies(storePolicies...)
}
diff --git a/cmd/cli/kubectl-kyverno/variables/variables_test.go b/cmd/cli/kubectl-kyverno/variables/variables_test.go
index 807726d8f2..68b622bd0a 100644
--- a/cmd/cli/kubectl-kyverno/variables/variables_test.go
+++ b/cmd/cli/kubectl-kyverno/variables/variables_test.go
@@ -5,6 +5,7 @@ import (
"testing"
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/apis/v1alpha1"
+ "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/store"
"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/values"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/sets"
@@ -131,11 +132,12 @@ func TestVariables_SetInStore(t *testing.T) {
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
+ var s store.Store
v := Variables{
values: tt.values,
variables: tt.variables,
}
- v.SetInStore()
+ v.SetInStore(&s)
})
}
}
@@ -257,11 +259,12 @@ func TestVariables_ComputeVariables(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
+ var s store.Store
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...)
+ got, err := v.ComputeVariables(&s, 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