diff --git a/cmd/cli/kubectl-kyverno/apply/apply_command.go b/cmd/cli/kubectl-kyverno/apply/apply_command.go index 777fe73782..47afb21b36 100644 --- a/cmd/cli/kubectl-kyverno/apply/apply_command.go +++ b/cmd/cli/kubectl-kyverno/apply/apply_command.go @@ -341,7 +341,7 @@ func (c *ApplyCommandConfig) applyCommandHelper() (rc *common.ResultCounts, reso fmt.Printf("Error: failed to load request info\nCause: %s\n", err) osExit(1) } - store.SetSubjects(subjectInfo) + store.SetSubject(subjectInfo.Subject) } if c.VariablesString != "" { diff --git a/cmd/cli/kubectl-kyverno/test/test_command.go b/cmd/cli/kubectl-kyverno/test/test_command.go index d42212b64c..b58cccef56 100644 --- a/cmd/cli/kubectl-kyverno/test/test_command.go +++ b/cmd/cli/kubectl-kyverno/test/test_command.go @@ -778,7 +778,7 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, isGit bool, fmt.Printf("Error: failed to load request info\nCause: %s\n", err) os.Exit(1) } - store.SetSubjects(subjectInfo) + store.SetSubject(subjectInfo.Subject) } policyFullPath := getFullPath(values.Policies, policyResourcePath, isGit) diff --git a/cmd/cli/kubectl-kyverno/utils/common/common.go b/cmd/cli/kubectl-kyverno/utils/common/common.go index 9e1c2a4fe9..8885182a40 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/common.go +++ b/cmd/cli/kubectl-kyverno/utils/common/common.go @@ -362,9 +362,7 @@ func GetVariable(variablesString, valuesFile string, fs billy.Filesystem, isGit }) } - store.SetContext(store.Context{ - Policies: storePolicies, - }) + store.SetPolicies(storePolicies...) return variables, globalValMap, valuesMapResource, namespaceSelectorMap, subresources, nil } @@ -855,9 +853,7 @@ func SetInStoreContext(mutatedPolicies []kyvernov1.PolicyInterface, variables ma }) } - store.SetContext(store.Context{ - Policies: storePolicies, - }) + store.SetPolicies(storePolicies...) return variables } @@ -965,7 +961,7 @@ func CheckVariableForPolicy(valuesMap map[string]map[string]Resource, globalValM // skipping the variable check for non matching kind if _, ok := kindOnwhichPolicyIsApplied[resourceKind]; ok { - if len(variable) > 0 && len(thisPolicyResourceValues) == 0 && len(store.GetContext().Policies) == 0 { + if len(variable) > 0 && len(thisPolicyResourceValues) == 0 && store.HasPolicies() { return thisPolicyResourceValues, sanitizederror.NewWithError(fmt.Sprintf("policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag", policyName, resourceName), nil) } } diff --git a/cmd/cli/kubectl-kyverno/utils/store/store.go b/cmd/cli/kubectl-kyverno/utils/store/store.go index 678d075422..06cbbfeffe 100644 --- a/cmd/cli/kubectl-kyverno/utils/store/store.go +++ b/cmd/cli/kubectl-kyverno/utils/store/store.go @@ -5,75 +5,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" ) -var ( - Mock bool - registryClient registryclient.Client - AllowApiCalls bool - ContextVar Context - ForeachElement int - Subjects Subject -) - -func SetMock(mock bool) { - Mock = mock -} - -func GetMock() bool { - return Mock -} - -func SetForEachElement(foreachElement int) { - ForeachElement = foreachElement -} - -func GetForeachElement() int { - return ForeachElement -} - -func SetRegistryAccess(access bool) { - if access { - registryClient = registryclient.NewOrDie(registryclient.WithLocalKeychain()) - } -} - -func GetRegistryAccess() bool { - return registryClient != nil -} - -func GetRegistryClient() registryclient.Client { - return registryClient -} - -func SetContext(context Context) { - ContextVar = context -} - -func GetContext() Context { - return ContextVar -} - -func GetPolicyFromContext(policyName string) *Policy { - for _, policy := range ContextVar.Policies { - if policy.Name == policyName { - return &policy - } - } - return nil -} - -func GetPolicyRuleFromContext(policyName string, ruleName string) *Rule { - for _, policy := range ContextVar.Policies { - if policy.Name == policyName { - for _, rule := range policy.Rules { - if rule.Name == ruleName { - return &rule - } - } - } - } - return nil -} - type Context struct { Policies []Policy `json:"policies"` } @@ -89,22 +20,92 @@ type Rule struct { ForEachValues map[string][]interface{} `json:"foreachValues"` } -func SetSubjects(subjects Subject) { - Subjects = subjects -} - -func GetSubjects() Subject { - return Subjects -} - type Subject struct { Subject rbacv1.Subject `json:"subject,omitempty" yaml:"subject,omitempty"` } -func AllowApiCall(allow bool) { - AllowApiCalls = allow +var ( + mock bool + registryClient registryclient.Client + allowApiCalls bool + policies []Policy + // contextVar Context + foreachElement int + subject rbacv1.Subject +) + +func SetMock(m bool) { + mock = m } -func IsAllowApiCall() bool { - return AllowApiCalls +func IsMock() bool { + return mock +} + +func SetForEachElement(element int) { + foreachElement = element +} + +func GetForeachElement() int { + return foreachElement +} + +func SetRegistryAccess(access bool) { + if access { + registryClient = registryclient.NewOrDie(registryclient.WithLocalKeychain()) + } +} + +func GetRegistryAccess() bool { + return registryClient != nil +} + +func GetRegistryClient() registryclient.Client { + return registryClient +} + +func SetPolicies(p ...Policy) { + policies = p +} + +func HasPolicies() bool { + return len(policies) != 0 +} + +func GetPolicy(policyName string) *Policy { + for _, policy := range policies { + if policy.Name == policyName { + return &policy + } + } + return nil +} + +func GetPolicyRule(policyName string, ruleName string) *Rule { + for _, policy := range policies { + if policy.Name == policyName { + for _, rule := range policy.Rules { + if rule.Name == ruleName { + return &rule + } + } + } + } + return nil +} + +func SetSubject(s rbacv1.Subject) { + subject = s +} + +func GetSubject() rbacv1.Subject { + return subject +} + +func AllowApiCall(allow bool) { + allowApiCalls = allow +} + +func IsApiCallAllowed() bool { + return allowApiCalls } diff --git a/pkg/engine/jsonContext.go b/pkg/engine/jsonContext.go index e1d3263b40..d9dd6cac10 100644 --- a/pkg/engine/jsonContext.go +++ b/pkg/engine/jsonContext.go @@ -22,8 +22,8 @@ func LoadContext(ctx context.Context, logger logr.Logger, rclient registryclient } policyName := enginectx.policy.GetName() - if store.GetMock() { - rule := store.GetPolicyRuleFromContext(policyName, ruleName) + if store.IsMock() { + rule := store.GetPolicyRule(policyName, ruleName) if rule != nil && len(rule.Values) > 0 { variables := rule.Values for key, value := range variables { @@ -46,7 +46,7 @@ func LoadContext(ctx context.Context, logger logr.Logger, rclient registryclient if err := loadVariable(logger, entry, enginectx); err != nil { return err } - } else if entry.APICall != nil && store.IsAllowApiCall() { + } else if entry.APICall != nil && store.IsApiCallAllowed() { if err := loadAPIData(ctx, logger, entry, enginectx); err != nil { return err } @@ -55,7 +55,7 @@ func LoadContext(ctx context.Context, logger logr.Logger, rclient registryclient if rule != nil && len(rule.ForEachValues) > 0 { for key, value := range rule.ForEachValues { - if err := enginectx.jsonContext.AddVariable(key, value[store.ForeachElement]); err != nil { + if err := enginectx.jsonContext.AddVariable(key, value[store.GetForeachElement()]); err != nil { return err } } diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 87b032152a..626889293b 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -9,7 +9,6 @@ import ( "github.com/go-logr/logr" gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" - "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/store" "github.com/kyverno/kyverno/pkg/autogen" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/mutate" @@ -264,33 +263,30 @@ func (f *forEachMutator) mutateElements(ctx context.Context, foreach kyvernov1.F invertedElement(elements) } - for i, e := range elements { - if e == nil { + for index, element := range elements { + if element == nil { continue } f.policyContext.JSONContext().Reset() policyContext := f.policyContext.Copy() - // TODO - this needs to be refactored. The engine should not have a dependency to the CLI code - store.SetForEachElement(i) - falseVar := false - if err := addElementToContext(policyContext, e, i, f.nesting, &falseVar); err != nil { - return mutate.NewErrorResponse(fmt.Sprintf("failed to add element to mutate.foreach[%d].context", i), err) + if err := addElementToContext(policyContext, element, index, f.nesting, &falseVar); err != nil { + return mutate.NewErrorResponse(fmt.Sprintf("failed to add element to mutate.foreach[%d].context", index), err) } if err := LoadContext(ctx, f.log, f.rclient, foreach.Context, policyContext, f.rule.Name); err != nil { - return mutate.NewErrorResponse(fmt.Sprintf("failed to load to mutate.foreach[%d].context", i), err) + return mutate.NewErrorResponse(fmt.Sprintf("failed to load to mutate.foreach[%d].context", index), err) } preconditionsPassed, err := checkPreconditions(f.log, policyContext, foreach.AnyAllConditions) if err != nil { - return mutate.NewErrorResponse(fmt.Sprintf("failed to evaluate mutate.foreach[%d].preconditions", i), err) + return mutate.NewErrorResponse(fmt.Sprintf("failed to evaluate mutate.foreach[%d].preconditions", index), err) } if !preconditionsPassed { - f.log.Info("mutate.foreach.preconditions not met", "elementIndex", i) + f.log.Info("mutate.foreach.preconditions not met", "elementIndex", index) continue } diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index 2d1a460721..34c7aafbb0 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -230,25 +230,21 @@ func Test_variableSubstitutionCLI(t *testing.T) { } }`) - configMapVariableContext := store.Context{ - Policies: []store.Policy{ - { - Name: "cm-variable-example", - Rules: []store.Rule{ - { - Name: "example-configmap-lookup", - Values: map[string]interface{}{ - "dictionary.data.env": "dev1", - }, + expectedPatch := []byte(`{"op":"add","path":"/metadata/labels","value":{"my-environment-name":"dev1"}}`) + + store.SetPolicies( + store.Policy{ + Name: "cm-variable-example", + Rules: []store.Rule{ + { + Name: "example-configmap-lookup", + Values: map[string]interface{}{ + "dictionary.data.env": "dev1", }, }, }, }, - } - - expectedPatch := []byte(`{"op":"add","path":"/metadata/labels","value":{"my-environment-name":"dev1"}}`) - - store.SetContext(configMapVariableContext) + ) store.SetMock(true) var policy kyverno.ClusterPolicy err := json.Unmarshal(policyRaw, &policy) diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 4372001289..bbc0aa8de5 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -161,8 +161,8 @@ func doesResourceMatchConditionBlock(subresourceGVKToAPIResource map[string]*met // matchSubjects return true if one of ruleSubjects exist in userInfo func matchSubjects(ruleSubjects []rbacv1.Subject, userInfo authenticationv1.UserInfo, dynamicConfig []string) bool { - if store.GetMock() { - mockSubject := store.GetSubjects().Subject + if store.IsMock() { + mockSubject := store.GetSubject() for _, subject := range ruleSubjects { switch subject.Kind { case "ServiceAccount": diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index c9d737f446..d8788a25d0 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -12,7 +12,6 @@ import ( gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" kyvernov2alpha1 "github.com/kyverno/kyverno/api/kyverno/v2alpha1" - "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/store" "github.com/kyverno/kyverno/pkg/autogen" "github.com/kyverno/kyverno/pkg/config" engineapi "github.com/kyverno/kyverno/pkg/engine/api" @@ -323,17 +322,14 @@ func (v *validator) validateElements(ctx context.Context, rclient registryclient defer v.policyContext.jsonContext.Restore() applyCount := 0 - for i, e := range elements { - if e == nil { + for index, element := range elements { + if element == nil { continue } - // TODO - this needs to be refactored. The engine should not have a dependency to the CLI code - store.SetForEachElement(i) - v.policyContext.JSONContext().Reset() policyContext := v.policyContext.Copy() - if err := addElementToContext(policyContext, e, i, v.nesting, elementScope); err != nil { + if err := addElementToContext(policyContext, element, index, v.nesting, elementScope); err != nil { v.log.Error(err, "failed to add element to context") return ruleError(v.rule, engineapi.Validation, "failed to process foreach", err), applyCount } @@ -353,7 +349,7 @@ func (v *validator) validateElements(ctx context.Context, rclient registryclient continue } else if r.Status != engineapi.RuleStatusPass { if r.Status == engineapi.RuleStatusError { - if i < len(elements)-1 { + if index < len(elements)-1 { continue } msg := fmt.Sprintf("validation failure: %v", r.Message) @@ -369,13 +365,13 @@ func (v *validator) validateElements(ctx context.Context, rclient registryclient return ruleResponse(*v.rule, engineapi.Validation, "", engineapi.RuleStatusPass), applyCount } -func addElementToContext(ctx *PolicyContext, e interface{}, elementIndex, nesting int, elementScope *bool) error { - data, err := variables.DocumentToUntyped(e) +func addElementToContext(ctx *PolicyContext, element interface{}, index, nesting int, elementScope *bool) error { + data, err := variables.DocumentToUntyped(element) if err != nil { return err } - if err := ctx.JSONContext().AddElement(data, elementIndex, nesting); err != nil { - return errors.Wrapf(err, "failed to add element (%v) to JSON context", e) + if err := ctx.JSONContext().AddElement(data, index, nesting); err != nil { + return errors.Wrapf(err, "failed to add element (%v) to JSON context", element) } dataMap, ok := data.(map[string]interface{}) // We set scoped to true by default if the data is a map diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 050896a923..b6ee8a5487 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -2198,23 +2198,19 @@ func TestValidate_context_variable_substitution_CLI(t *testing.T) { } `) - configMapVariableContext := store.Context{ - Policies: []store.Policy{ - { - Name: "restrict-pod-count", - Rules: []store.Rule{ - { - Name: "restrict-pod-count", - Values: map[string]interface{}{ - "podcounts": "12", - }, + store.SetPolicies( + store.Policy{ + Name: "restrict-pod-count", + Rules: []store.Rule{ + { + Name: "restrict-pod-count", + Values: map[string]interface{}{ + "podcounts": "12", }, }, }, }, - } - - store.SetContext(configMapVariableContext) + ) store.SetMock(true) var policy kyverno.ClusterPolicy @@ -2712,24 +2708,20 @@ func Test_foreach_context_preconditions(t *testing.T) { } }`) - configMapVariableContext := store.Context{ - Policies: []store.Policy{ - { - Name: "test", - Rules: []store.Rule{ - { - Name: "test", - Values: map[string]interface{}{ - "img.data.podvalid": "nginx/nginx:v1", - "img.data.podinvalid": "nginx/nginx:v2", - }, + store.SetPolicies( + store.Policy{ + Name: "test", + Rules: []store.Rule{ + { + Name: "test", + Values: map[string]interface{}{ + "img.data.podvalid": "nginx/nginx:v1", + "img.data.podinvalid": "nginx/nginx:v2", }, }, }, }, - } - - store.SetContext(configMapVariableContext) + ) store.SetMock(true) testForEach(t, policyraw, resourceRaw, "", engineapi.RuleStatusPass) @@ -2806,24 +2798,20 @@ func Test_foreach_context_preconditions_fail(t *testing.T) { } }`) - configMapVariableContext := store.Context{ - Policies: []store.Policy{ - { - Name: "test", - Rules: []store.Rule{ - { - Name: "test", - Values: map[string]interface{}{ - "img.data.podvalid": "nginx/nginx:v1", - "img.data.podinvalid": "nginx/nginx:v1", - }, + store.SetPolicies( + store.Policy{ + Name: "test", + Rules: []store.Rule{ + { + Name: "test", + Values: map[string]interface{}{ + "img.data.podvalid": "nginx/nginx:v1", + "img.data.podinvalid": "nginx/nginx:v1", }, }, }, }, - } - - store.SetContext(configMapVariableContext) + ) store.SetMock(true) testForEach(t, policyraw, resourceRaw, "", engineapi.RuleStatusFail)