diff --git a/pkg/engine/mutate/mutation.go b/pkg/engine/mutate/mutation.go index 4c54e4fd5f..3dd5ab42dd 100644 --- a/pkg/engine/mutate/mutation.go +++ b/pkg/engine/mutate/mutation.go @@ -6,7 +6,6 @@ import ( "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/response" "github.com/kyverno/kyverno/pkg/engine/utils" - "github.com/kyverno/kyverno/pkg/engine/variables" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -56,21 +55,7 @@ func newpatchStrategicMergeHandler(ruleName string, mutate *kyverno.Mutation, pa } func (h patchStrategicMergeHandler) Handle() (response.RuleResponse, unstructured.Unstructured) { - var ruleResponse response.RuleResponse - PatchStrategicMerge := h.mutation.PatchStrategicMerge - log := h.logger - - // substitute the variables - var err error - if PatchStrategicMerge, err = variables.SubstituteAll(log, h.evalCtx, PatchStrategicMerge); err != nil { - // variable subsitution failed - ruleResponse.Name = h.ruleName - ruleResponse.Success = false - ruleResponse.Message = err.Error() - return ruleResponse, h.patchedResource - } - - return ProcessStrategicMergePatch(h.ruleName, PatchStrategicMerge, h.patchedResource, log) + return ProcessStrategicMergePatch(h.ruleName, h.mutation.PatchStrategicMerge, h.patchedResource, h.logger) } // overlayHandler @@ -126,19 +111,7 @@ func (h patchesJSON6902Handler) Handle() (resp response.RuleResponse, patchedRes } func (h overlayHandler) Handle() (response.RuleResponse, unstructured.Unstructured) { - var ruleResponse response.RuleResponse - overlay := h.mutation.Overlay - - // substitute the variables - var err error - if overlay, err = variables.SubstituteAll(h.logger, h.evalCtx, overlay); err != nil { - // variable substitution failed - ruleResponse.Success = false - ruleResponse.Message = err.Error() - return ruleResponse, h.patchedResource - } - - return ProcessOverlay(h.logger, h.ruleName, overlay, h.patchedResource) + return ProcessOverlay(h.logger, h.ruleName, h.mutation.Overlay, h.patchedResource) } // patchesHandler diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index ce6c8e9100..9e84f7552c 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -1,12 +1,14 @@ package engine import ( + "fmt" "time" "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/mutate" "github.com/kyverno/kyverno/pkg/engine/response" + "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/engine/variables" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/log" @@ -88,6 +90,19 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { continue } + if rule, err = variables.SubstituteAllInRule(logger, policyContext.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: false, + } + + incrementAppliedCount(resp) + resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResp) + continue + } + mutation := rule.Mutation.DeepCopy() mutateHandler := mutate.CreateMutateHandler(rule.Name, mutation, patchedResource, ctx, logger) ruleResponse, patchedResource = mutateHandler.Handle() diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index d03163d65c..8bd55a3103 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -160,7 +160,7 @@ func Test_variableSubstitutionPathNotExist(t *testing.T) { JSONContext: ctx, NewResource: *resourceUnstructured} er := Mutate(policyContext) - expectedErrorStr := "variable request.object.metadata.name1 not resolved at path /spec/name" + expectedErrorStr := "variable substitution failed for rule test-path-not-exist: variable request.object.metadata.name1 not resolved at path /mutate/overlay/spec/name" t.Log(er.PolicyResponse.Rules[0].Message) assert.Equal(t, er.PolicyResponse.Rules[0].Message, expectedErrorStr) } diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 88ae9936f5..17ad9d5a36 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -60,16 +60,6 @@ func buildResponse(logger logr.Logger, ctx *PolicyContext, resp *response.Engine resp.PatchedResource = resource } - for i := range resp.PolicyResponse.Rules { - messageInterface, err := variables.SubstituteAll(logger, ctx.JSONContext, resp.PolicyResponse.Rules[i].Message) - if err != nil { - logger.V(4).Info("failed to substitute variables", "error", err.Error()) - continue - } - - resp.PolicyResponse.Rules[i].Message, _ = messageInterface.(string) - } - resp.PolicyResponse.Policy = ctx.Policy.Name resp.PolicyResponse.Resource.Name = resp.PatchedResource.GetName() resp.PolicyResponse.Resource.Namespace = resp.PatchedResource.GetNamespace() @@ -94,6 +84,8 @@ func validateResource(log logr.Logger, ctx *PolicyContext) *response.EngineRespo defer ctx.JSONContext.Restore() for _, rule := range ctx.Policy.Spec.Rules { + var err error + if !rule.HasValidate() { continue } @@ -124,6 +116,20 @@ func validateResource(log logr.Logger, ctx *PolicyContext) *response.EngineRespo 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: false, + } + + incrementAppliedCount(resp) + resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResp) + + continue + } + if rule.Validation.Pattern != nil || rule.Validation.AnyPattern != nil { ruleResponse := validateResourceWithRule(log, ctx, rule) if ruleResponse != nil { @@ -225,13 +231,6 @@ func validatePatterns(log logr.Logger, ctx context.EvalInterface, resource unstr logger.V(4).Info("finished processing rule", "processingTime", resp.RuleStats.ProcessingTime.String()) }() - var err error - if rule, err = variables.SubstituteAllInRule(logger, ctx, rule); err != nil { - resp.Success = false - resp.Message = fmt.Sprintf("variable substitution failed for rule %s: %s", rule.Name, err.Error()) - return resp - } - validationRule := rule.Validation.DeepCopy() if validationRule.Pattern != nil { pattern := validationRule.Pattern diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 88545fc6cf..33cc252c54 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -1599,6 +1599,104 @@ func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathPresent_NonePatter "validation error: Rule test-path-not-exist[0] failed at path /spec/template/spec/containers/0/name/. Rule test-path-not-exist[1] failed at path /spec/template/spec/containers/0/name/.") } +func Test_VariableSubstitutionValidate_VariablesInMessageAreResolved(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "busybox", + "labels": { + "app": "busybox", + "color": "red", + "animal": "cow", + "food": "pizza", + "car": "jeep", + "env": "qa" + } + }, + "spec": { + "replicas": 1, + "selector": { + "matchLabels": { + "app": "busybox" + } + }, + "template": { + "metadata": { + "labels": { + "app": "busybox" + } + }, + "spec": { + "containers": [ + { + "image": "busybox:1.28", + "name": "busybox", + "command": [ + "sleep", + "9999" + ] + } + ] + } + } + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "cm-array-example" + }, + "spec": { + "validationFailureAction": "enforce", + "background": false, + "rules": [ + { + "name": "validate-role-annotation", + "match": { + "resources": { + "kinds": [ + "Deployment" + ] + } + }, + "validate": { + "message": "The animal {{ request.object.metadata.labels.animal }} is not in the allowed list of animals.", + "deny": { + "conditions": [ + { + "key": "{{ request.object.metadata.labels.animal }}", + "operator": "NotIn", + "value": "abcde" + } + ] + } + } + } + ] + } + }`) + + var policy kyverno.ClusterPolicy + assert.NilError(t, json.Unmarshal(policyraw, &policy)) + resourceUnstructured, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContext() + err = ctx.AddResource(resourceRaw) + assert.NilError(t, err) + + policyContext := &PolicyContext{ + Policy: policy, + JSONContext: ctx, + NewResource: *resourceUnstructured} + er := Validate(policyContext) + assert.Assert(t, er.PolicyResponse.Rules[0].Success) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, "The animal cow is not in the allowed list of animals.") +} + type testCase struct { description string policy []byte diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 9c9cb7a42f..bf80d13f4c 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -199,6 +199,7 @@ func (c *Controller) applyGeneratePolicy(log logr.Logger, policyContext *engine. ruleNameToProcessingTime := make(map[string]time.Duration) for _, rule := range policy.Spec.Rules { + var err error if !rule.HasGenerate() { continue } @@ -221,7 +222,12 @@ func (c *Controller) applyGeneratePolicy(log logr.Logger, policyContext *engine. // add configmap json data to context if err := engine.LoadContext(log, rule.Context, resCache, policyContext); err != nil { - log.Info("cannot add configmaps to context", "reason", err.Error()) + log.Error(err, "cannot add configmaps to context") + return nil, err + } + + if rule, err = variables.SubstituteAllInRule(log, policyContext.JSONContext, rule); err != nil { + log.Error(err, "variable substitution failed for rule %s", rule.Name) return nil, err } @@ -316,16 +322,6 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou return noGenResource, err } - // Variable substitutions - // format : {{ results in error and rule is not applied - // - valid variables are replaced with the values - object, err := variables.SubstituteAll(log, ctx, genUnst.Object) - if err != nil { - return noGenResource, err - } - - genUnst.Object, _ = object.(map[string]interface{}) genKind, genName, genNamespace, genAPIVersion, err := getResourceInfo(genUnst.Object) if err != nil { return noGenResource, err diff --git a/pkg/policy/background.go b/pkg/policy/background.go index c459b1a981..0b36202240 100644 --- a/pkg/policy/background.go +++ b/pkg/policy/background.go @@ -44,10 +44,6 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error { return fmt.Errorf("variable substitution failed for rule %s: %s", rule.Name, err.Error()) } - if rule, err = variables.SubstituteAllInRule(log.Log, ctx, rule); !checkNotFoundErr(err) { - return fmt.Errorf("variable substitution failed for rule %s: %s", rule.Name, err.Error()) - } - if rule.AnyAllConditions != nil { if err = validatePreConditions(idx, ctx, rule.AnyAllConditions); !checkNotFoundErr(err) { return err