diff --git a/pkg/engine/context/evaluate.go b/pkg/engine/context/evaluate.go index d102fd7703..41d259d35d 100644 --- a/pkg/engine/context/evaluate.go +++ b/pkg/engine/context/evaluate.go @@ -3,10 +3,11 @@ package context import ( "encoding/json" "fmt" - "github.com/pkg/errors" "reflect" "strings" + "github.com/pkg/errors" + jmespath "github.com/kyverno/kyverno/pkg/engine/jmespath" ) diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index 866f62249f..e7a5589ab2 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -60,6 +60,7 @@ func filterRule(rule kyverno.Rule, policyContext *PolicyContext) *response.RuleR return nil } + var err error startTime := time.Now() policy := policyContext.Policy @@ -74,10 +75,10 @@ func filterRule(rule kyverno.Rule, policyContext *PolicyContext) *response.RuleR logger := log.Log.WithName("Generate").WithValues("policy", policy.Name, "kind", newResource.GetKind(), "namespace", newResource.GetNamespace(), "name", newResource.GetName()) - if err := MatchesResourceDescription(newResource, rule, admissionInfo, excludeGroupRole, namespaceLabels); err != nil { + if err = MatchesResourceDescription(newResource, rule, admissionInfo, excludeGroupRole, namespaceLabels); err != nil { // if the oldResource matched, return "false" to delete GR for it - if err := MatchesResourceDescription(oldResource, rule, admissionInfo, excludeGroupRole, namespaceLabels); err == nil { + if err = MatchesResourceDescription(oldResource, rule, admissionInfo, excludeGroupRole, namespaceLabels); err == nil { return &response.RuleResponse{ Name: rule.Name, Type: "Generation", @@ -95,11 +96,17 @@ func filterRule(rule kyverno.Rule, policyContext *PolicyContext) *response.RuleR policyContext.JSONContext.Checkpoint() defer policyContext.JSONContext.Restore() - if err := LoadContext(logger, rule.Context, resCache, policyContext, rule.Name); err != nil { + if err = LoadContext(logger, rule.Context, resCache, policyContext, rule.Name); err != nil { logger.V(4).Info("cannot add external data to the context", "reason", err.Error()) return nil } + rule.AnyAllConditions, err = variables.SubstituteAllInPreconditions(logger, ctx, rule.AnyAllConditions) + if err != nil { + logger.V(4).Info("failed to substitute vars in preconditions, skip current rule", "rule name", rule.Name) + return nil + } + // operate on the copy of the conditions, as we perform variable substitution copyConditions, err := copyConditions(rule.AnyAllConditions) if err != nil { @@ -108,7 +115,7 @@ func filterRule(rule kyverno.Rule, policyContext *PolicyContext) *response.RuleR } // evaluate pre-conditions - if !variables.EvaluateConditions(logger, ctx, copyConditions, true) { + if !variables.EvaluateConditions(logger, ctx, copyConditions) { logger.V(4).Info("preconditions not satisfied, skipping rule", "rule", rule.Name) return nil } diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 98fbe5aa09..f0fc0b5048 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -50,6 +50,8 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { policyContext.JSONContext.Checkpoint() defer policyContext.JSONContext.Restore() + var err error + for _, rule := range policy.Spec.Rules { if !rule.HasMutate() { continue @@ -63,7 +65,7 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { excludeResource = policyContext.ExcludeGroupRole } - if err := MatchesResourceDescription(patchedResource, rule, policyContext.AdmissionInfo, excludeResource, policyContext.NamespaceLabels); err != nil { + if err = MatchesResourceDescription(patchedResource, rule, policyContext.AdmissionInfo, excludeResource, policyContext.NamespaceLabels); err != nil { logger.V(4).Info("rule not matched", "reason", err.Error()) continue } @@ -91,6 +93,12 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { continue } + rule.AnyAllConditions, err = variables.SubstituteAllInPreconditions(logger, ctx, rule.AnyAllConditions) + if err != nil { + logger.V(3).Info("failed to substitute vars in preconditions, skip current rule", "rule name", rule.Name) + continue + } + // operate on the copy of the conditions, as we perform variable substitution copyConditions, err := copyConditions(rule.AnyAllConditions) if err != nil { @@ -99,7 +107,7 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { } // evaluate pre-conditions // - handle variable substitutions - if !variables.EvaluateConditions(logger, ctx, copyConditions, true) { + if !variables.EvaluateConditions(logger, ctx, copyConditions) { logger.V(3).Info("resource fails the preconditions") continue } diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index ff4f8b5b3d..9c1bf441fb 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -111,6 +111,12 @@ func validateResource(log logr.Logger, ctx *PolicyContext) *response.EngineRespo log.V(3).Info("matched validate rule") + rule.AnyAllConditions, err = variables.SubstituteAllInPreconditions(log, ctx.JSONContext, rule.AnyAllConditions) + if err != nil { + log.V(4).Info("failed to substitute vars in preconditions, skip current rule", "rule name", rule.Name) + return nil + } + // operate on the copy of the conditions, as we perform variable substitution preconditionsCopy, err := copyConditions(rule.AnyAllConditions) if err != nil { @@ -119,32 +125,16 @@ func validateResource(log logr.Logger, ctx *PolicyContext) *response.EngineRespo } // evaluate pre-conditions - if !variables.EvaluateConditions(log, ctx.JSONContext, preconditionsCopy, true) { + if !variables.EvaluateConditions(log, ctx.JSONContext, preconditionsCopy) { log.V(4).Info("resource fails the preconditions") continue } - if rule, err = variables.SubstituteAllInRule(log, ctx.JSONContext, rule); err != nil { - ruleResp := response.RuleResponse{ - Name: rule.Name, - Type: utils.Validation.String(), - Message: fmt.Sprintf("variable substitution failed for rule %s: %s", rule.Name, err.Error()), - Success: true, - } - - incrementAppliedCount(resp) - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResp) - - switch err.(type) { - case gojmespath.NotFoundError: - log.V(2).Info("failed to substitute variables, skip current rule", "info", err.Error(), "rule name", rule.Name) - default: - log.Error(err, "failed to substitute variables, skip current rule", "rule name", rule.Name) - } - continue - } - if rule.Validation.Pattern != nil || rule.Validation.AnyPattern != nil { + if rule, err = substituteAll(log, ctx, rule, resp); err != nil { + continue + } + ruleResponse := validateResourceWithRule(log, ctx, rule) if ruleResponse != nil { if !common.IsConditionalAnchorError(ruleResponse.Message) { @@ -153,12 +143,22 @@ func validateResource(log logr.Logger, ctx *PolicyContext) *response.EngineRespo } } } else if rule.Validation.Deny != nil { + rule.Validation.Deny.AnyAllConditions, err = variables.SubstituteAllInPreconditions(log, ctx.JSONContext, rule.Validation.Deny.AnyAllConditions) + if err != nil { + log.V(4).Info("failed to substitute vars in preconditions, skip current rule", "rule name", rule.Name) + continue + } + + if rule, err = substituteAll(log, ctx, rule, resp); err != nil { + continue + } + denyConditionsCopy, err := copyConditions(rule.Validation.Deny.AnyAllConditions) if err != nil { log.V(2).Info("wrongfully configured data", "reason", err.Error()) continue } - deny := variables.EvaluateConditions(log, ctx.JSONContext, denyConditionsCopy, false) + deny := variables.EvaluateConditions(log, ctx.JSONContext, denyConditionsCopy) ruleResp := response.RuleResponse{ Name: rule.Name, Type: utils.Validation.String(), @@ -329,3 +329,29 @@ func buildAnyPatternErrorMessage(rule kyverno.Rule, errors []string) string { return fmt.Sprintf("validation error: %s. %s", rule.Validation.Message, errStr) } + +func substituteAll(log logr.Logger, ctx *PolicyContext, rule kyverno.Rule, resp *response.EngineResponse) (kyverno.Rule, error) { + var err error + if rule, err = variables.SubstituteAllInRule(log, ctx.JSONContext, rule); err != nil { + ruleResp := response.RuleResponse{ + Name: rule.Name, + Type: utils.Validation.String(), + Message: fmt.Sprintf("variable substitution failed for rule %s: %s", rule.Name, err.Error()), + Success: true, + } + + incrementAppliedCount(resp) + resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResp) + + switch err.(type) { + case gojmespath.NotFoundError: + log.V(2).Info("failed to substitute variables, skip current rule", "info", err.Error(), "rule name", rule.Name) + default: + log.Error(err, "failed to substitute variables, skip current rule", "rule name", rule.Name) + } + + return rule, err + } + + return rule, nil +} diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index c3ef5bd249..c3a49c176b 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -1929,8 +1929,8 @@ func Test_Flux_Kustomization_PathNotPresent(t *testing.T) { policyRaw: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"flux-multi-tenancy"},"spec":{"validationFailureAction":"enforce","rules":[{"name":"serviceAccountName","exclude":{"resources":{"namespaces":["flux-system"]}},"match":{"resources":{"kinds":["Kustomization","HelmRelease"]}},"validate":{"message":".spec.serviceAccountName is required","pattern":{"spec":{"serviceAccountName":"?*"}}}},{"name":"sourceRefNamespace","exclude":{"resources":{"namespaces":["flux-system"]}},"match":{"resources":{"kinds":["Kustomization","HelmRelease"]}},"validate":{"message":"spec.sourceRef.namespace must be the same as metadata.namespace","deny":{"conditions":[{"key":"{{request.object.spec.sourceRef.namespace}}","operator":"NotEquals","value":"{{request.object.metadata.namespace}}"}]}}}]}}`), // referred variable path not present resourceRaw: []byte(`{"apiVersion":"kustomize.toolkit.fluxcd.io/v1beta1","kind":"Kustomization","metadata":{"name":"dev-team","namespace":"apps"},"spec":{"serviceAccountName":"dev-team","interval":"5m","sourceRef":{"kind":"GitRepository","name":"dev-team"},"prune":true,"validation":"client"}}`), - expectedResult: true, - expectedMessage: "variable substitution failed for rule sourceRefNamespace: Unknown key \"namespace\" in path", + expectedResult: false, + expectedMessage: "spec.sourceRef.namespace must be the same as metadata.namespace", }, { name: "resource-with-violation", @@ -2242,3 +2242,178 @@ func TestValidate_context_variable_substitution_CLI(t *testing.T) { } assert.Assert(t, !er.IsSuccessful()) } + +func Test_EmptyStringInDenyCondition(t *testing.T) { + policyRaw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "annotations": { + "meta.helm.sh/release-name": "kyverno-policies", + "meta.helm.sh/release-namespace": "kyverno", + "pod-policies.kyverno.io/autogen-controllers": "none" + }, + "labels": { + "app.kubernetes.io/managed-by": "Helm" + }, + "name": "if-baltic-restrict-external-load-balancer" + }, + "spec": { + "background": true, + "rules": [ + { + "match": { + "resources": { + "kinds": [ + "Service" + ] + } + }, + "name": "match-service-type", + "preconditions": [ + { + "key": "{{request.object.spec.type}}", + "operator": "Equals", + "value": "LoadBalancer" + } + ], + "validate": { + "deny": { + "conditions": [ + { + "key": "{{ request.object.metadata.annotations.\"service.beta.kubernetes.io/azure-load-balancer-internal\"}}", + "operator": "NotEquals", + "value": "true" + } + ] + } + } + } + ], + "validationFailureAction": "enforce" + } + }`) + + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "name": "example-service" + }, + "spec": { + "selector": { + "app": "example" + }, + "ports": [ + { + "port": 8765, + "targetPort": 9376 + } + ], + "type": "LoadBalancer" + } + }`) + + var policy kyverno.ClusterPolicy + err := json.Unmarshal(policyRaw, &policy) + assert.NilError(t, err) + + ctx := context.NewContext() + err = ctx.AddResource(resourceRaw) + assert.NilError(t, err) + + resourceUnstructured, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + er := Validate(&PolicyContext{Policy: policy, NewResource: *resourceUnstructured, JSONContext: ctx}) + assert.Assert(t, !er.IsSuccessful()) +} + +func Test_StringInDenyCondition(t *testing.T) { + policyRaw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "annotations": { + "meta.helm.sh/release-name": "kyverno-policies", + "meta.helm.sh/release-namespace": "kyverno", + "pod-policies.kyverno.io/autogen-controllers": "none" + }, + "labels": { + "app.kubernetes.io/managed-by": "Helm" + }, + "name": "if-baltic-restrict-external-load-balancer" + }, + "spec": { + "background": true, + "rules": [ + { + "match": { + "resources": { + "kinds": [ + "Service" + ] + } + }, + "name": "match-service-type", + "preconditions": [ + { + "key": "{{request.object.spec.type}}", + "operator": "Equals", + "value": "LoadBalancer" + } + ], + "validate": { + "deny": { + "conditions": [ + { + "key": "{{ request.object.metadata.annotations.\"service.beta.kubernetes.io/azure-load-balancer-internal\"}}", + "operator": "NotEquals", + "value": "true" + } + ] + } + } + } + ], + "validationFailureAction": "enforce" + } + }`) + + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "name": "example-service", + "annotations": { + "service.beta.kubernetes.io/azure-load-balancer-internal": "true" + } + }, + "spec": { + "selector": { + "app": "example" + }, + "ports": [ + { + "port": 8765, + "targetPort": 9376 + } + ], + "type": "LoadBalancer" + } + }`) + + var policy kyverno.ClusterPolicy + err := json.Unmarshal(policyRaw, &policy) + assert.NilError(t, err) + + ctx := context.NewContext() + err = ctx.AddResource(resourceRaw) + assert.NilError(t, err) + + resourceUnstructured, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + er := Validate(&PolicyContext{Policy: policy, NewResource: *resourceUnstructured, JSONContext: ctx}) + assert.Assert(t, er.IsSuccessful()) +} diff --git a/pkg/engine/variables/evaluate.go b/pkg/engine/variables/evaluate.go index fa76321670..b9bfb00d89 100644 --- a/pkg/engine/variables/evaluate.go +++ b/pkg/engine/variables/evaluate.go @@ -8,28 +8,28 @@ import ( ) //Evaluate evaluates the condition -func Evaluate(log logr.Logger, ctx context.EvalInterface, condition kyverno.Condition, isPreCondition bool) bool { +func Evaluate(log logr.Logger, ctx context.EvalInterface, condition kyverno.Condition) bool { // get handler for the operator - handle := operator.CreateOperatorHandler(log, ctx, condition.Operator, SubstituteAll) + handle := operator.CreateOperatorHandler(log, ctx, condition.Operator) if handle == nil { return false } - return handle.Evaluate(condition.Key, condition.Value, isPreCondition) + return handle.Evaluate(condition.Key, condition.Value) } //EvaluateConditions evalues all the conditions present in a slice, in a backwards compatible way -func EvaluateConditions(log logr.Logger, ctx context.EvalInterface, conditions interface{}, isPreCondition bool) bool { +func EvaluateConditions(log logr.Logger, ctx context.EvalInterface, conditions interface{}) bool { switch typedConditions := conditions.(type) { case kyverno.AnyAllConditions: - return evaluateAnyAllConditions(log, ctx, typedConditions, isPreCondition) + return evaluateAnyAllConditions(log, ctx, typedConditions) case []kyverno.Condition: // backwards compatibility - return evaluateOldConditions(log, ctx, typedConditions, isPreCondition) + return evaluateOldConditions(log, ctx, typedConditions) } return false } //evaluateAnyAllConditions evaluates multiple conditions as a logical AND (all) or OR (any) operation depending on the conditions -func evaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, conditions kyverno.AnyAllConditions, isPreCondition bool) bool { +func evaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, conditions kyverno.AnyAllConditions) bool { anyConditions, allConditions := conditions.AnyConditions, conditions.AllConditions anyConditionsResult, allConditionsResult := true, true @@ -37,7 +37,7 @@ func evaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, condit if anyConditions != nil { anyConditionsResult = false for _, condition := range anyConditions { - if Evaluate(log, ctx, condition, isPreCondition) { + if Evaluate(log, ctx, condition) { anyConditionsResult = true break } @@ -47,7 +47,7 @@ func evaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, condit // update the allConditionsResult if they are present if allConditions != nil { for _, condition := range allConditions { - if !Evaluate(log, ctx, condition, isPreCondition) { + if !Evaluate(log, ctx, condition) { allConditionsResult = false break } @@ -59,9 +59,9 @@ func evaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, condit } //evaluateOldConditions evaluates multiple conditions when those conditions are provided in the old manner i.e. without 'any' or 'all' -func evaluateOldConditions(log logr.Logger, ctx context.EvalInterface, conditions []kyverno.Condition, isPreCondition bool) bool { +func evaluateOldConditions(log logr.Logger, ctx context.EvalInterface, conditions []kyverno.Condition) bool { for _, condition := range conditions { - if !Evaluate(log, ctx, condition, isPreCondition) { + if !Evaluate(log, ctx, condition) { return false } } diff --git a/pkg/engine/variables/evaluate_test.go b/pkg/engine/variables/evaluate_test.go index 4531aecb3b..90cf38423f 100644 --- a/pkg/engine/variables/evaluate_test.go +++ b/pkg/engine/variables/evaluate_test.go @@ -6,6 +6,7 @@ import ( kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/context" + "github.com/stretchr/testify/assert" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -19,7 +20,7 @@ func Test_Eval_Equal_Const_String_Pass(t *testing.T) { Value: "name", } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -33,7 +34,7 @@ func Test_Eval_Equal_Const_String_Fail(t *testing.T) { Value: "name1", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -47,7 +48,7 @@ func Test_Eval_NoEqual_Const_String_Pass(t *testing.T) { Value: "name1", } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -61,7 +62,7 @@ func Test_Eval_NoEqual_Const_String_Fail(t *testing.T) { Value: "name", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -73,7 +74,7 @@ func Test_Eval_GreaterThanOrEquals_Const_string_Equal_Pass(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: 1.0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -85,7 +86,7 @@ func Test_Eval_GreaterThanOrEquals_Const_string_Greater_Pass(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: 0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -97,7 +98,7 @@ func Test_Eval_GreaterThanOrEquals_Const_string_Fail(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: "2", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -109,7 +110,7 @@ func Test_Eval_GreaterThan_Const_string_Equal_Fail(t *testing.T) { Operator: kyverno.GreaterThan, Value: 1.0, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -121,7 +122,7 @@ func Test_Eval_GreaterThan_Const_string_Greater_Pass(t *testing.T) { Operator: kyverno.GreaterThan, Value: 0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -133,7 +134,7 @@ func Test_Eval_GreaterThan_Const_string_Fail(t *testing.T) { Operator: kyverno.GreaterThan, Value: "2", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -145,7 +146,7 @@ func Test_Eval_LessThanOrEquals_Const_string_Equal_Pass(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: 1.0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -157,7 +158,7 @@ func Test_Eval_LessThanOrEquals_Const_string_Less_Pass(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: 1, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -169,7 +170,7 @@ func Test_Eval_LessThanOrEquals_Const_string_Fail(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: "1.1", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -181,7 +182,7 @@ func Test_Eval_LessThan_Const_string_Equal_Pass(t *testing.T) { Operator: kyverno.LessThan, Value: 1.0, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -193,7 +194,7 @@ func Test_Eval_LessThan_Const_string_Less_Pass(t *testing.T) { Operator: kyverno.LessThan, Value: 1, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -205,7 +206,7 @@ func Test_Eval_LessThan_Const_string_Fail(t *testing.T) { Operator: kyverno.LessThan, Value: "1.1", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -221,7 +222,7 @@ func Test_Eval_Equal_Const_Bool_Pass(t *testing.T) { Value: true, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -235,7 +236,7 @@ func Test_Eval_Equal_Const_Bool_Fail(t *testing.T) { Value: false, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -249,7 +250,7 @@ func Test_Eval_NoEqual_Const_Bool_Pass(t *testing.T) { Value: false, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -263,7 +264,7 @@ func Test_Eval_NoEqual_Const_Bool_Fail(t *testing.T) { Value: true, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -278,7 +279,7 @@ func Test_Eval_Equal_Const_int_Pass(t *testing.T) { Value: 1, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -292,7 +293,7 @@ func Test_Eval_Equal_Const_int_Fail(t *testing.T) { Value: 2, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -306,7 +307,7 @@ func Test_Eval_NoEqual_Const_int_Pass(t *testing.T) { Value: 2, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -320,7 +321,7 @@ func Test_Eval_NoEqual_Const_int_Fail(t *testing.T) { Value: 1, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -332,7 +333,7 @@ func Test_Eval_GreaterThanOrEquals_Const_int_Equal_Pass(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: 1.0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -344,7 +345,7 @@ func Test_Eval_GreaterThanOrEquals_Const_int_Greater_Pass(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: 0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -356,7 +357,7 @@ func Test_Eval_GreaterThanOrEquals_Const_int_Fail(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: "2", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -368,7 +369,7 @@ func Test_Eval_GreaterThan_Const_int_Equal_Fail(t *testing.T) { Operator: kyverno.GreaterThan, Value: 1.0, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -380,7 +381,7 @@ func Test_Eval_GreaterThan_Const_int_Greater_Pass(t *testing.T) { Operator: kyverno.GreaterThan, Value: 0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -392,7 +393,7 @@ func Test_Eval_GreaterThan_Const_int_Fail(t *testing.T) { Operator: kyverno.GreaterThan, Value: "2", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -404,7 +405,7 @@ func Test_Eval_LessThanOrEquals_Const_int_Equal_Pass(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: 1.0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -416,7 +417,7 @@ func Test_Eval_LessThanOrEquals_Const_int_Less_Pass(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: 1, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -428,7 +429,7 @@ func Test_Eval_LessThanOrEquals_Const_int_Fail(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: "1", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -440,7 +441,7 @@ func Test_Eval_LessThan_Const_int_Equal_Fail(t *testing.T) { Operator: kyverno.LessThan, Value: 1.0, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -452,7 +453,7 @@ func Test_Eval_LessThan_Const_int_Less_Pass(t *testing.T) { Operator: kyverno.LessThan, Value: 1, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -464,7 +465,7 @@ func Test_Eval_LessThan_Const_int_Fail(t *testing.T) { Operator: kyverno.LessThan, Value: "1", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -479,7 +480,7 @@ func Test_Eval_Equal_Const_int64_Pass(t *testing.T) { Value: int64(1), } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -493,7 +494,7 @@ func Test_Eval_Equal_Const_int64_Fail(t *testing.T) { Value: int64(2), } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -507,7 +508,7 @@ func Test_Eval_NoEqual_Const_int64_Pass(t *testing.T) { Value: int64(2), } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -521,7 +522,7 @@ func Test_Eval_NoEqual_Const_int64_Fail(t *testing.T) { Value: int64(1), } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -537,7 +538,7 @@ func Test_Eval_Equal_Const_float64_Pass(t *testing.T) { Value: 1.5, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -551,7 +552,7 @@ func Test_Eval_Equal_Const_float64_Fail(t *testing.T) { Value: 1.6, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -565,7 +566,7 @@ func Test_Eval_NoEqual_Const_float64_Pass(t *testing.T) { Value: 1.6, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -579,7 +580,7 @@ func Test_Eval_NoEqual_Const_float64_Fail(t *testing.T) { Value: 1.5, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -591,7 +592,7 @@ func Test_Eval_GreaterThanOrEquals_Const_float64_Equal_Pass(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: 1.0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -603,7 +604,7 @@ func Test_Eval_GreaterThanOrEquals_Const_float64_Greater_Pass(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: 0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -615,7 +616,7 @@ func Test_Eval_GreaterThanOrEquals_Const_float64_Fail(t *testing.T) { Operator: kyverno.GreaterThanOrEquals, Value: "2", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -627,7 +628,7 @@ func Test_Eval_GreaterThan_Const_float64_Equal_Fail(t *testing.T) { Operator: kyverno.GreaterThan, Value: 1.0, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -639,7 +640,7 @@ func Test_Eval_GreaterThan_Const_float64_Greater_Pass(t *testing.T) { Operator: kyverno.GreaterThan, Value: "0", } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -651,7 +652,7 @@ func Test_Eval_GreaterThan_Const_float64_Fail(t *testing.T) { Operator: kyverno.GreaterThan, Value: "2.5", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -663,7 +664,7 @@ func Test_Eval_LessThanOrEquals_Const_float64_Equal_Pass(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: 1.0, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -675,7 +676,7 @@ func Test_Eval_LessThanOrEquals_Const_float64_Less_Pass(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: 1, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -687,7 +688,7 @@ func Test_Eval_LessThanOrEquals_Const_float64_Fail(t *testing.T) { Operator: kyverno.LessThanOrEquals, Value: "1.95", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -699,7 +700,7 @@ func Test_Eval_LessThan_Const_float64_Equal_Fail(t *testing.T) { Operator: kyverno.LessThan, Value: 1.0, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -711,7 +712,7 @@ func Test_Eval_LessThan_Const_float64_Less_Pass(t *testing.T) { Operator: kyverno.LessThan, Value: "1.5", } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -723,7 +724,7 @@ func Test_Eval_LessThan_Const_float64_Fail(t *testing.T) { Operator: kyverno.LessThan, Value: 1.95, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -753,7 +754,7 @@ func Test_Eval_Equal_Const_object_Pass(t *testing.T) { Value: obj2, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -781,7 +782,7 @@ func Test_Eval_Equal_Const_object_Fail(t *testing.T) { Value: obj2, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -809,7 +810,7 @@ func Test_Eval_NotEqual_Const_object_Pass(t *testing.T) { Value: obj2, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -837,7 +838,7 @@ func Test_Eval_NotEqual_Const_object_Fail(t *testing.T) { Value: obj2, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -867,7 +868,7 @@ func Test_Eval_Equal_Const_list_Pass(t *testing.T) { Value: obj2, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -893,7 +894,7 @@ func Test_Eval_Equal_Const_list_Fail(t *testing.T) { Value: obj2, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -919,7 +920,7 @@ func Test_Eval_NotEqual_Const_list_Pass(t *testing.T) { Value: obj2, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -945,7 +946,7 @@ func Test_Eval_NotEqual_Const_list_Fail(t *testing.T) { Value: obj2, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -978,9 +979,20 @@ func Test_Eval_Equal_Var_Pass(t *testing.T) { Value: "temp", } - if !Evaluate(log.Log, ctx, condition, true) { - t.Error("expected to pass") - } + conditionJSON, err := json.Marshal(condition) + assert.Nil(t, err) + + var conditionMap interface{} + err = json.Unmarshal(conditionJSON, &conditionMap) + assert.Nil(t, err) + + conditionWithResolvedVars, err := SubstituteAllInPreconditions(log.Log, ctx, conditionMap) + conditionJSON, err = json.Marshal(conditionWithResolvedVars) + assert.Nil(t, err) + + err = json.Unmarshal(conditionJSON, &condition) + assert.Nil(t, err) + assert.True(t, Evaluate(log.Log, ctx, condition)) } func Test_Eval_Equal_Var_Fail(t *testing.T) { @@ -1009,7 +1021,7 @@ func Test_Eval_Equal_Var_Fail(t *testing.T) { Value: "temp1", } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -1036,7 +1048,7 @@ func Test_Eval_In_String_Set_Pass(t *testing.T) { Value: valueInterface, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -1061,7 +1073,7 @@ func Test_Eval_In_String_Set_Fail(t *testing.T) { Value: valueInterface, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } @@ -1086,7 +1098,7 @@ func Test_Eval_NotIn_String_Set_Pass(t *testing.T) { Value: valueInterface, } - if !Evaluate(log.Log, ctx, condition, true) { + if !Evaluate(log.Log, ctx, condition) { t.Error("expected to pass") } } @@ -1111,7 +1123,7 @@ func Test_Eval_NotIn_String_Set_Fail(t *testing.T) { Value: valueInterface, } - if Evaluate(log.Log, ctx, condition, true) { + if Evaluate(log.Log, ctx, condition) { t.Error("expected to fail") } } diff --git a/pkg/engine/variables/operator/equal.go b/pkg/engine/variables/operator/equal.go index df92dfe1bb..4f226cd5c5 100644 --- a/pkg/engine/variables/operator/equal.go +++ b/pkg/engine/variables/operator/equal.go @@ -13,45 +13,21 @@ import ( ) //NewEqualHandler returns handler to manage Equal operations -func NewEqualHandler(log logr.Logger, ctx context.EvalInterface, subHandler VariableSubstitutionHandler) OperatorHandler { +func NewEqualHandler(log logr.Logger, ctx context.EvalInterface) OperatorHandler { return EqualHandler{ - ctx: ctx, - subHandler: subHandler, - log: log, + ctx: ctx, + log: log, } } //EqualHandler provides implementation to handle NotEqual Operator type EqualHandler struct { - ctx context.EvalInterface - subHandler VariableSubstitutionHandler - log logr.Logger + ctx context.EvalInterface + log logr.Logger } //Evaluate evaluates expression with Equal Operator -func (eh EqualHandler) Evaluate(key, value interface{}, isPreCondition bool) bool { - var err error - //TODO: decouple variables from evaluation - // substitute the variables - if key, err = eh.subHandler(eh.log, eh.ctx, key); err != nil { - // Failed to resolve the variable - if isPreCondition { - eh.log.Info("Failed to resolve variable", "info", err.Error(), "variable", key) - } else { - eh.log.Error(err, "Failed to resolve variable", "variable", key) - } - return false - } - if value, err = eh.subHandler(eh.log, eh.ctx, value); err != nil { - // Failed to resolve the variable - if isPreCondition { - eh.log.Info("Failed to resolve variable", "info", err.Error(), "variable", value) - } else { - eh.log.Error(err, "Failed to resolve variable", "variable", value) - } - return false - } - +func (eh EqualHandler) Evaluate(key, value interface{}) bool { // key and value need to be of same type switch typedKey := key.(type) { case bool: diff --git a/pkg/engine/variables/operator/in.go b/pkg/engine/variables/operator/in.go index b05d14f8d1..f7642204b6 100644 --- a/pkg/engine/variables/operator/in.go +++ b/pkg/engine/variables/operator/in.go @@ -11,43 +11,21 @@ import ( ) //NewInHandler returns handler to manage In operations -func NewInHandler(log logr.Logger, ctx context.EvalInterface, subHandler VariableSubstitutionHandler) OperatorHandler { +func NewInHandler(log logr.Logger, ctx context.EvalInterface) OperatorHandler { return InHandler{ - ctx: ctx, - subHandler: subHandler, - log: log, + ctx: ctx, + log: log, } } //InHandler provides implementation to handle In Operator type InHandler struct { - ctx context.EvalInterface - subHandler VariableSubstitutionHandler - log logr.Logger + ctx context.EvalInterface + log logr.Logger } //Evaluate evaluates expression with In Operator -func (in InHandler) Evaluate(key, value interface{}, isPreCondition bool) bool { - var err error - // substitute the variables - if key, err = in.subHandler(in.log, in.ctx, key); err != nil { - if isPreCondition { - in.log.Info("Failed to resolve variable", "info", err.Error(), "variable", key) - } else { - in.log.Error(err, "Failed to resolve variable", "variable", key) - } - return false - } - - if value, err = in.subHandler(in.log, in.ctx, value); err != nil { - if isPreCondition { - in.log.Info("Failed to resolve variable", "info", err.Error(), "variable", value) - } else { - in.log.Error(err, "Failed to resolve variable", "variable", value) - } - return false - } - +func (in InHandler) Evaluate(key, value interface{}) bool { switch typedKey := key.(type) { case string: return in.validateValueWithStringPattern(typedKey, value) diff --git a/pkg/engine/variables/operator/notequal.go b/pkg/engine/variables/operator/notequal.go index 4020e4dbfe..f2e4f72885 100644 --- a/pkg/engine/variables/operator/notequal.go +++ b/pkg/engine/variables/operator/notequal.go @@ -13,44 +13,21 @@ import ( ) //NewNotEqualHandler returns handler to manage NotEqual operations -func NewNotEqualHandler(log logr.Logger, ctx context.EvalInterface, subHandler VariableSubstitutionHandler) OperatorHandler { +func NewNotEqualHandler(log logr.Logger, ctx context.EvalInterface) OperatorHandler { return NotEqualHandler{ - ctx: ctx, - subHandler: subHandler, - log: log, + ctx: ctx, + log: log, } } //NotEqualHandler provides implementation to handle NotEqual Operator type NotEqualHandler struct { - ctx context.EvalInterface - subHandler VariableSubstitutionHandler - log logr.Logger + ctx context.EvalInterface + log logr.Logger } //Evaluate evaluates expression with NotEqual Operator -func (neh NotEqualHandler) Evaluate(key, value interface{}, isPreCondition bool) bool { - var err error - //TODO: decouple variables from evaluation - // substitute the variables - if key, err = neh.subHandler(neh.log, neh.ctx, key); err != nil { - // Failed to resolve the variable - if isPreCondition { - neh.log.Info("Failed to resolve variable", "info", err.Error(), "variable", key) - } else { - neh.log.Error(err, "Failed to resolve variable", "variable", key) - } - return false - } - if value, err = neh.subHandler(neh.log, neh.ctx, value); err != nil { - // Failed to resolve the variable - if isPreCondition { - neh.log.Info("Failed to resolve variable", "info", err.Error(), "variable", value) - } else { - neh.log.Error(err, "Failed to resolve variable", "variable", value) - } - return false - } +func (neh NotEqualHandler) Evaluate(key, value interface{}) bool { // key and value need to be of same type switch typedKey := key.(type) { case bool: diff --git a/pkg/engine/variables/operator/notin.go b/pkg/engine/variables/operator/notin.go index e9cab9f0bb..2a066eb5ff 100644 --- a/pkg/engine/variables/operator/notin.go +++ b/pkg/engine/variables/operator/notin.go @@ -8,44 +8,21 @@ import ( ) //NewNotInHandler returns handler to manage NotIn operations -func NewNotInHandler(log logr.Logger, ctx context.EvalInterface, subHandler VariableSubstitutionHandler) OperatorHandler { +func NewNotInHandler(log logr.Logger, ctx context.EvalInterface) OperatorHandler { return NotInHandler{ - ctx: ctx, - subHandler: subHandler, - log: log, + ctx: ctx, + log: log, } } //NotInHandler provides implementation to handle NotIn Operator type NotInHandler struct { - ctx context.EvalInterface - subHandler VariableSubstitutionHandler - log logr.Logger + ctx context.EvalInterface + log logr.Logger } //Evaluate evaluates expression with NotIn Operator -func (nin NotInHandler) Evaluate(key, value interface{}, isPreCondition bool) bool { - var err error - - // substitute the variables - if key, err = nin.subHandler(nin.log, nin.ctx, key); err != nil { - if isPreCondition { - nin.log.Info("Failed to resolve variable", "info", err.Error(), "variable", key) - } else { - nin.log.Error(err, "Failed to resolve variable", "variable", key) - } - return false - } - - if value, err = nin.subHandler(nin.log, nin.ctx, value); err != nil { - if isPreCondition { - nin.log.Info("Failed to resolve variable", "info", err.Error(), "variable", value) - } else { - nin.log.Error(err, "Failed to resolve variable", "variable", value) - } - return false - } - +func (nin NotInHandler) Evaluate(key, value interface{}) bool { switch typedKey := key.(type) { case string: return nin.validateValueWithStringPattern(typedKey, value) diff --git a/pkg/engine/variables/operator/numeric.go b/pkg/engine/variables/operator/numeric.go index b1f57836e2..815d191f16 100644 --- a/pkg/engine/variables/operator/numeric.go +++ b/pkg/engine/variables/operator/numeric.go @@ -10,21 +10,19 @@ import ( ) //NewNumericOperatorHandler returns handler to manage the provided numeric operations (>, >=, <=, <) -func NewNumericOperatorHandler(log logr.Logger, ctx context.EvalInterface, subHandler VariableSubstitutionHandler, op kyverno.ConditionOperator) OperatorHandler { +func NewNumericOperatorHandler(log logr.Logger, ctx context.EvalInterface, op kyverno.ConditionOperator) OperatorHandler { return NumericOperatorHandler{ - ctx: ctx, - subHandler: subHandler, - log: log, - condition: op, + ctx: ctx, + log: log, + condition: op, } } //NumericOperatorHandler provides implementation to handle Numeric Operations associated with policies type NumericOperatorHandler struct { - ctx context.EvalInterface - subHandler VariableSubstitutionHandler - log logr.Logger - condition kyverno.ConditionOperator + ctx context.EvalInterface + log logr.Logger + condition kyverno.ConditionOperator } // compareByCondition compares a float64 key with a float64 value on the basis of the provided operator @@ -44,27 +42,7 @@ func compareByCondition(key float64, value float64, op kyverno.ConditionOperator } } -func (noh NumericOperatorHandler) Evaluate(key, value interface{}, isPreCondition bool) bool { - var err error - if key, err = noh.subHandler(noh.log, noh.ctx, key); err != nil { - // Failed to resolve the variable - if isPreCondition { - noh.log.Info("Failed to resolve variable", "info", err.Error(), "variable", key) - } else { - noh.log.Error(err, "Failed to resolve variable", "variable", key) - } - return false - } - if value, err = noh.subHandler(noh.log, noh.ctx, value); err != nil { - // Failed to resolve the variable - if isPreCondition { - noh.log.Info("Failed to resolve variable", "info", err.Error(), "variable", value) - } else { - noh.log.Error(err, "Failed to resolve variable", "variable", value) - } - return false - } - +func (noh NumericOperatorHandler) Evaluate(key, value interface{}) bool { switch typedKey := key.(type) { case int: return noh.validateValueWithIntPattern(int64(typedKey), value) diff --git a/pkg/engine/variables/operator/operator.go b/pkg/engine/variables/operator/operator.go index 3d125e334e..561ea9ac00 100644 --- a/pkg/engine/variables/operator/operator.go +++ b/pkg/engine/variables/operator/operator.go @@ -10,7 +10,7 @@ import ( //OperatorHandler provides interface to manage types type OperatorHandler interface { - Evaluate(key, value interface{}, isPreCondition bool) bool + Evaluate(key, value interface{}) bool validateValueWithStringPattern(key string, value interface{}) bool validateValueWithBoolPattern(key bool, value interface{}) bool validateValueWithIntPattern(key int64, value interface{}) bool @@ -23,33 +23,33 @@ type OperatorHandler interface { type VariableSubstitutionHandler = func(log logr.Logger, ctx context.EvalInterface, pattern interface{}) (interface{}, error) //CreateOperatorHandler returns the operator handler based on the operator used in condition -func CreateOperatorHandler(log logr.Logger, ctx context.EvalInterface, op kyverno.ConditionOperator, subHandler VariableSubstitutionHandler) OperatorHandler { +func CreateOperatorHandler(log logr.Logger, ctx context.EvalInterface, op kyverno.ConditionOperator) OperatorHandler { str := strings.ToLower(string(op)) switch str { case strings.ToLower(string(kyverno.Equal)): - return NewEqualHandler(log, ctx, subHandler) + return NewEqualHandler(log, ctx) case strings.ToLower(string(kyverno.Equals)): - return NewEqualHandler(log, ctx, subHandler) + return NewEqualHandler(log, ctx) case strings.ToLower(string(kyverno.NotEqual)): - return NewNotEqualHandler(log, ctx, subHandler) + return NewNotEqualHandler(log, ctx) case strings.ToLower(string(kyverno.NotEquals)): - return NewNotEqualHandler(log, ctx, subHandler) + return NewNotEqualHandler(log, ctx) case strings.ToLower(string(kyverno.In)): - return NewInHandler(log, ctx, subHandler) + return NewInHandler(log, ctx) case strings.ToLower(string(kyverno.NotIn)): - return NewNotInHandler(log, ctx, subHandler) + return NewNotInHandler(log, ctx) case strings.ToLower(string(kyverno.GreaterThanOrEquals)), strings.ToLower(string(kyverno.GreaterThan)), strings.ToLower(string(kyverno.LessThanOrEquals)), strings.ToLower(string(kyverno.LessThan)): - return NewNumericOperatorHandler(log, ctx, subHandler, op) + return NewNumericOperatorHandler(log, ctx, op) default: log.Info("operator not supported", "operator", str) diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index fd8f00eee9..ac5b52bfe9 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -44,7 +44,30 @@ func SubstituteAll(log logr.Logger, ctx context.EvalInterface, document interfac return kyverno.Rule{}, err } - return substituteVars(log, ctx, document) + return substituteVars(log, ctx, document, DefaultVariableResolver) +} + +func newPreconditionsVariableResolver(log logr.Logger) VariableResolver { + // PreconditionsVariableResolver is used to substitute vars in preconditions. + // It returns empty string if error occured during substitution + return func(ctx context.EvalInterface, variable string) (interface{}, error) { + value, err := DefaultVariableResolver(ctx, variable) + if err != nil { + log.Info(fmt.Sprintf("Variable \"%s\" is not resolved in preconditions. Considering it as an empty string", variable)) + return "", nil + } + + return value, nil + } +} + +func SubstituteAllInPreconditions(log logr.Logger, ctx context.EvalInterface, document interface{}) (_ interface{}, err error) { + document, err = substituteReferences(log, document) + if err != nil { + return kyverno.Rule{}, err + } + + return substituteVars(log, ctx, document, newPreconditionsVariableResolver(log)) } func SubstituteAllForceMutate(log logr.Logger, ctx context.EvalInterface, typedRule kyverno.Rule) (_ kyverno.Rule, err error) { @@ -63,7 +86,7 @@ func SubstituteAllForceMutate(log logr.Logger, ctx context.EvalInterface, typedR if ctx == nil { rule = replaceSubstituteVariables(rule) } else { - rule, err = substituteVars(log, ctx, rule) + rule, err = substituteVars(log, ctx, rule, DefaultVariableResolver) if err != nil { return kyverno.Rule{}, err } @@ -74,8 +97,8 @@ func SubstituteAllForceMutate(log logr.Logger, ctx context.EvalInterface, typedR //SubstituteVars replaces the variables with the values defined in the context // - if any variable is invalid or has nil value, it is considered as a failed variable substitution -func substituteVars(log logr.Logger, ctx context.EvalInterface, rule interface{}) (interface{}, error) { - return jsonUtils.NewTraversal(rule, substituteVariablesIfAny(log, ctx)).TraverseJSON() +func substituteVars(log logr.Logger, ctx context.EvalInterface, rule interface{}, vr VariableResolver) (interface{}, error) { + return jsonUtils.NewTraversal(rule, substituteVariablesIfAny(log, ctx, vr)).TraverseJSON() } func substituteReferences(log logr.Logger, rule interface{}) (interface{}, error) { @@ -164,7 +187,15 @@ func substituteReferencesIfAny(log logr.Logger) jsonUtils.Action { }) } -func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUtils.Action { +//VariableResolver defines the handler function for variable substitution +type VariableResolver = func(ctx context.EvalInterface, variable string) (interface{}, error) + +// DefaultVariableResolver is used in all variable substitutions except preconditions +func DefaultVariableResolver(ctx context.EvalInterface, variable string) (interface{}, error) { + return ctx.Query(variable) +} + +func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface, vr VariableResolver) jsonUtils.Action { return jsonUtils.OnlyForLeafs(func(data *jsonUtils.ActionData) (interface{}, error) { value, ok := data.Element.(string) if !ok { @@ -187,7 +218,8 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUt variable = strings.ReplaceAll(variable, "request.object", "request.oldObject") } - substitutedVar, err := ctx.Query(variable) + substitutedVar, err := vr(ctx, variable) + if err != nil { switch err.(type) { case context.InvalidVariableErr, gojmespath.NotFoundError: @@ -354,7 +386,7 @@ func SubstituteAllInRule(log logr.Logger, ctx context.EvalInterface, typedRule k return typedRule, err } - rule, err = substituteVars(log, ctx, rule) + rule, err = substituteVars(log, ctx, rule, DefaultVariableResolver) if err != nil { return typedRule, err } diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go index 146cf37b67..1ea7320311 100644 --- a/pkg/engine/variables/vars_test.go +++ b/pkg/engine/variables/vars_test.go @@ -436,7 +436,7 @@ func Test_SubstituteSuccess(t *testing.T) { patternRaw := []byte(`"{{request.object.metadata.annotations.test}}"`) assert.Assert(t, json.Unmarshal(patternRaw, &pattern)) - action := substituteVariablesIfAny(log.Log, ctx) + action := substituteVariablesIfAny(log.Log, ctx, DefaultVariableResolver) results, err := action(&ju.ActionData{ Document: nil, Element: string(patternRaw), @@ -460,7 +460,7 @@ func Test_SubstituteRecursiveErrors(t *testing.T) { patternRaw := []byte(`"{{request.object.metadata.{{request.object.metadata.annotations.test2}}}}"`) assert.Assert(t, json.Unmarshal(patternRaw, &pattern)) - action := substituteVariablesIfAny(log.Log, ctx) + action := substituteVariablesIfAny(log.Log, ctx, DefaultVariableResolver) results, err := action(&ju.ActionData{ Document: nil, Element: string(patternRaw), @@ -473,7 +473,7 @@ func Test_SubstituteRecursiveErrors(t *testing.T) { patternRaw = []byte(`"{{request.object.metadata2.{{request.object.metadata.annotations.test}}}}"`) assert.Assert(t, json.Unmarshal(patternRaw, &pattern)) - action = substituteVariablesIfAny(log.Log, ctx) + action = substituteVariablesIfAny(log.Log, ctx, DefaultVariableResolver) results, err = action(&ju.ActionData{ Document: nil, Element: string(patternRaw), @@ -492,7 +492,7 @@ func Test_SubstituteRecursive(t *testing.T) { patternRaw := []byte(`"{{request.object.metadata.{{request.object.metadata.annotations.test}}}}"`) assert.Assert(t, json.Unmarshal(patternRaw, &pattern)) - action := substituteVariablesIfAny(log.Log, ctx) + action := substituteVariablesIfAny(log.Log, ctx, DefaultVariableResolver) results, err := action(&ju.ActionData{ Document: nil, Element: string(patternRaw),