fix: reduce dependency from engine to cli (#6134)

* fix: remove dependency from engine to cli

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* refactor

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

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 != "" {

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)

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{
+	store.SetPolicies(storePolicies...)
-		Policies: storePolicies,
-	})
 
 	return variables, globalValMap, valuesMapResource, namespaceSelectorMap, subresources, nil
 }
@@ -855,9 +853,7 @@ func SetInStoreContext(mutatedPolicies []kyvernov1.PolicyInterface, variables ma
 		})
 	}
 
-	store.SetContext(store.Context{
+	store.SetPolicies(storePolicies...)
-		Policies: 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)
 		}
 	}

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) {
+var (
-	AllowApiCalls = allow
+	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 {
+func IsMock() bool {
-	return AllowApiCalls
+	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
 }

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() {
+	if store.IsMock() {
-		rule := store.GetPolicyRuleFromContext(policyName, ruleName)
+		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
 				}
 			}

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 {
+	for index, element := range elements {
-		if e == nil {
+		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 {
+		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", i), err)
+			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
 		}
 

pkg/engine/mutation_test.go

@@ -230,25 +230,21 @@ func Test_variableSubstitutionCLI(t *testing.T) {
   }
 }`)
 
-	configMapVariableContext := store.Context{
+	expectedPatch := []byte(`{"op":"add","path":"/metadata/labels","value":{"my-environment-name":"dev1"}}`)
-		Policies: []store.Policy{
+
-			{
+	store.SetPolicies(
-				Name: "cm-variable-example",
+		store.Policy{
-				Rules: []store.Rule{
+			Name: "cm-variable-example",
-					{
+			Rules: []store.Rule{
-						Name: "example-configmap-lookup",
+				{
-						Values: map[string]interface{}{
+					Name: "example-configmap-lookup",
-							"dictionary.data.env": "dev1",
+					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)

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() {
+	if store.IsMock() {
-		mockSubject := store.GetSubjects().Subject
+		mockSubject := store.GetSubject()
 		for _, subject := range ruleSubjects {
 			switch subject.Kind {
 			case "ServiceAccount":

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 {
+	for index, element := range elements {
-		if e == nil {
+		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 {
+func addElementToContext(ctx *PolicyContext, element interface{}, index, nesting int, elementScope *bool) error {
-	data, err := variables.DocumentToUntyped(e)
+	data, err := variables.DocumentToUntyped(element)
 	if err != nil {
 		return err
 	}
-	if err := ctx.JSONContext().AddElement(data, elementIndex, nesting); err != nil {
+	if err := ctx.JSONContext().AddElement(data, index, nesting); err != nil {
-		return errors.Wrapf(err, "failed to add element (%v) to JSON context", e)
+		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

pkg/engine/validation_test.go

@@ -2198,23 +2198,19 @@ func TestValidate_context_variable_substitution_CLI(t *testing.T) {
 	  }
 	`)
 
-	configMapVariableContext := store.Context{
+	store.SetPolicies(
-		Policies: []store.Policy{
+		store.Policy{
-			{
+			Name: "restrict-pod-count",
-				Name: "restrict-pod-count",
+			Rules: []store.Rule{
-				Rules: []store.Rule{
+				{
-					{
+					Name: "restrict-pod-count",
-						Name: "restrict-pod-count",
+					Values: map[string]interface{}{
-						Values: map[string]interface{}{
+						"podcounts": "12",
-							"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{
+	store.SetPolicies(
-		Policies: []store.Policy{
+		store.Policy{
-			{
+			Name: "test",
-				Name: "test",
+			Rules: []store.Rule{
-				Rules: []store.Rule{
+				{
-					{
+					Name: "test",
-						Name: "test",
+					Values: map[string]interface{}{
-						Values: map[string]interface{}{
+						"img.data.podvalid":   "nginx/nginx:v1",
-							"img.data.podvalid":   "nginx/nginx:v1",
+						"img.data.podinvalid": "nginx/nginx:v2",
-							"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{
+	store.SetPolicies(
-		Policies: []store.Policy{
+		store.Policy{
-			{
+			Name: "test",
-				Name: "test",
+			Rules: []store.Rule{
-				Rules: []store.Rule{
+				{
-					{
+					Name: "test",
-						Name: "test",
+					Values: map[string]interface{}{
-						Values: map[string]interface{}{
+						"img.data.podvalid":   "nginx/nginx:v1",
-							"img.data.podvalid":   "nginx/nginx:v1",
+						"img.data.podinvalid": "nginx/nginx:v1",
-							"img.data.podinvalid": "nginx/nginx:v1",
-						},
 					},
 				},
 			},
 		},
-	}
+	)
-
-	store.SetContext(configMapVariableContext)
 	store.SetMock(true)
 
 	testForEach(t, policyraw, resourceRaw, "", engineapi.RuleStatusFail)