From b9d4ee6876865099eaa7b43a322336ae2b4375ef Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Wed, 6 Oct 2021 18:31:20 -0700 Subject: [PATCH 1/5] fix tests Signed-off-by: Jim Bugwadia --- pkg/api/kyverno/v1/policy_types.go | 29 +++++ pkg/engine/mutate/mutation.go | 38 ++++-- pkg/engine/mutation.go | 189 +++++++++++++++++++++-------- pkg/engine/mutation_test.go | 114 +++++++++++++++++ pkg/engine/utils.go | 58 ++++++++- pkg/engine/validation.go | 109 +++++------------ 6 files changed, 395 insertions(+), 142 deletions(-) diff --git a/pkg/api/kyverno/v1/policy_types.go b/pkg/api/kyverno/v1/policy_types.go index 267a41be0e..bdcbacbbee 100755 --- a/pkg/api/kyverno/v1/policy_types.go +++ b/pkg/api/kyverno/v1/policy_types.go @@ -402,6 +402,35 @@ type Mutation struct { // See https://tools.ietf.org/html/rfc6902 and https://kubectl.docs.kubernetes.io/references/kustomize/patchesjson6902/. // +optional PatchesJSON6902 string `json:"patchesJson6902,omitempty" yaml:"patchesJson6902,omitempty"` + + // ForEach applies policy rule changes to nested elements. + ForEachMutation *ForEachMutation `json:"foreach,omitempty" yaml:"foreach,omitempty"` +} + +// ForEach applies policy rule changes to nested elements. +type ForEachMutation struct { + + // List specifies a JMESPath expression that results in one or more elements + // to which the validation logic is applied. + List string `json:"list,omitempty" yaml:"list,omitempty"` + + // Context defines variables and data sources that can be used during rule execution. + // +optional + Context []ContextEntry `json:"context,omitempty" yaml:"context,omitempty"` + + // Preconditions are used to determine if a policy rule should be applied by evaluating a + // set of conditions. The declaration can contain nested `any` or `all` statements. + // See: https://kyverno.io/docs/writing-policies/preconditions/ + // +kubebuilder:validation:XPreserveUnknownFields + // +optional + AnyAllConditions *AnyAllConditions `json:"preconditions,omitempty" yaml:"preconditions,omitempty"` + + // PatchStrategicMerge is a strategic merge patch used to modify resources. + // See https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/ + // and https://kubectl.docs.kubernetes.io/references/kustomize/patchesstrategicmerge/. + // +kubebuilder:validation:XPreserveUnknownFields + // +optional + PatchStrategicMerge apiextensions.JSON `json:"patchStrategicMerge,omitempty" yaml:"patchStrategicMerge,omitempty"` } // +k8s:deepcopy-gen=false diff --git a/pkg/engine/mutate/mutation.go b/pkg/engine/mutate/mutation.go index 067355c573..80ee5a101e 100644 --- a/pkg/engine/mutate/mutation.go +++ b/pkg/engine/mutate/mutation.go @@ -30,6 +30,8 @@ func CreateMutateHandler(ruleName string, mutate *kyverno.Mutation, patchedResou return newPatchStrategicMergeHandler(ruleName, mutate, patchedResource, context, logger) case isPatches(mutate): return newPatchesHandler(ruleName, mutate, patchedResource, context, logger) + case isForEach(mutate): + return newForEachHandler(ruleName, mutate, patchedResource, context, logger) default: return newEmptyHandler(patchedResource) } @@ -58,6 +60,28 @@ func (h patchStrategicMergeHandler) Handle() (response.RuleResponse, unstructure return ProcessStrategicMergePatch(h.ruleName, h.mutation.PatchStrategicMerge, h.patchedResource, h.logger) } +type forEachHandler struct { + ruleName string + mutation *kyverno.Mutation + patchedResource unstructured.Unstructured + evalCtx context.EvalInterface + logger logr.Logger +} + +func newForEachHandler(ruleName string, mutate *kyverno.Mutation, patchedResource unstructured.Unstructured, context context.EvalInterface, logger logr.Logger) Handler { + return forEachHandler{ + ruleName: ruleName, + mutation: mutate, + patchedResource: patchedResource, + evalCtx: context, + logger: logger, + } +} + +func (h forEachHandler) Handle() (response.RuleResponse, unstructured.Unstructured) { + return ProcessStrategicMergePatch(h.ruleName, h.mutation.ForEachMutation.PatchStrategicMerge, h.patchedResource, h.logger) +} + // overlayHandler type overlayHandler struct { ruleName string @@ -156,10 +180,11 @@ func (h emptyHandler) Handle() (response.RuleResponse, unstructured.Unstructured } func isPatchStrategicMerge(mutate *kyverno.Mutation) bool { - if mutate.PatchStrategicMerge != nil { - return true - } - return false + return mutate.PatchStrategicMerge != nil +} + +func isForEach(mutate *kyverno.Mutation) bool { + return mutate.ForEachMutation != nil } func isPatchesJSON6902(mutate *kyverno.Mutation) bool { @@ -170,10 +195,7 @@ func isPatchesJSON6902(mutate *kyverno.Mutation) bool { } func isOverlay(mutate *kyverno.Mutation) bool { - if mutate.Overlay != nil { - return true - } - return false + return mutate.Overlay != nil } func isPatches(mutate *kyverno.Mutation) bool { diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 3cc26f096a..13deaa8acb 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -2,6 +2,8 @@ package engine import ( "fmt" + "github.com/kyverno/kyverno/pkg/engine/context" + "github.com/pkg/errors" "time" "github.com/go-logr/logr" @@ -57,9 +59,7 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { continue } - var ruleResponse response.RuleResponse logger := logger.WithValues("rule", rule.Name) - excludeResource := []string{} if len(policyContext.ExcludeGroupRole) > 0 { excludeResource = policyContext.ExcludeGroupRole @@ -78,10 +78,10 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { policyContext.JSONContext.Reset() if err == nil && resource != nil { if err := ctx.AddResourceAsObject(resource.(map[string]interface{})); err != nil { - logger.WithName("RestoreContext").Error(err, "unable to update resource object") + logger.Error(err, "unable to update resource object") } } else { - logger.WithName("RestoreContext").Error(err, "failed to query resource object") + logger.Error(err, "failed to query resource object") } if err := LoadContext(logger, rule.Context, resCache, policyContext, rule.Name); err != nil { @@ -94,66 +94,149 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { } ruleCopy := rule.DeepCopy() - ruleCopy.AnyAllConditions, err = variables.SubstituteAllInPreconditions(logger, ctx, ruleCopy.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 := transformConditions(ruleCopy.AnyAllConditions) - if err != nil { - logger.V(2).Info("failed to load context", "reason", err.Error()) - continue - } - // evaluate pre-conditions - // - handle variable substitutions - if !variables.EvaluateConditions(logger, ctx, copyConditions) { - logger.V(3).Info("resource fails the preconditions") - continue - } - - if *ruleCopy, err = variables.SubstituteAllInRule(logger, ctx, *ruleCopy); err != nil { - ruleResp := response.RuleResponse{ - Name: ruleCopy.Name, - Type: utils.Mutation.String(), - Message: fmt.Sprintf("variable substitution failed: %s", err.Error()), - Status: response.RuleStatusPass, + var ruleResp *response.RuleResponse + if rule.Mutation.ForEachMutation != nil { + ruleResp, patchedResource = mutateForEachResource(ruleCopy, policyContext, patchedResource, logger) + } else { + skip := false + err, mutateResp := mutateResource(ruleCopy, policyContext.JSONContext, patchedResource, logger) + if err != nil { + if skip { + ruleResp = ruleResponse(&rule, utils.Mutation, err.Error(), response.RuleStatusSkip) + } else { + ruleResp = ruleResponse(&rule, utils.Mutation, err.Error(), response.RuleStatusError) + } + } else { + ruleResp = ruleResponse(&rule, utils.Mutation, "mutated resource", response.RuleStatusPass) + ruleResp.Patches = mutateResp.patches + patchedResource = mutateResp.patchedResource } - - incrementAppliedCount(resp) - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResp) - - logger.Error(err, "failed to substitute variables, skip current rule", "rule name", ruleCopy.Name) - continue } - mutation := ruleCopy.Mutation.DeepCopy() - mutateHandler := mutate.CreateMutateHandler(ruleCopy.Name, mutation, patchedResource, ctx, logger) - ruleResponse, patchedResource = mutateHandler.Handle() - if ruleResponse.Status == response.RuleStatusPass { - // - overlay pattern does not match the resource conditions - if ruleResponse.Patches == nil { - continue + if ruleResp != nil { + resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, *ruleResp) + if ruleResp.Status == response.RuleStatusError { + incrementErrorCount(resp) + } else { + incrementAppliedCount(resp) } - - logger.V(4).Info("mutate rule applied successfully", "ruleName", ruleCopy.Name) } - - if err := ctx.AddResourceAsObject(patchedResource.Object); err != nil { - logger.Error(err, "failed to update resource in the JSON context") - } - - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse) - incrementAppliedRuleCount(resp) } resp.PatchedResource = patchedResource return resp } -func incrementAppliedRuleCount(resp *response.EngineResponse) { - resp.PolicyResponse.RulesAppliedCount++ +func mutateForEachResource(rule *kyverno.Rule, ctx *PolicyContext, resource unstructured.Unstructured, logger logr.Logger) (*response.RuleResponse, unstructured.Unstructured) { + foreach := rule.Mutation.ForEachMutation + if foreach == nil { + return nil, resource + } + + if err := LoadContext(logger, foreach.Context, ctx.ResourceCache, ctx, rule.Name); err != nil { + logger.Error(err, "failed to load context") + return ruleError(rule, utils.Mutation, "failed to load context", err), resource + } + + preconditionsPassed, err := checkPreconditions(logger, ctx, foreach.AnyAllConditions) + if err != nil { + return ruleError(rule, utils.Mutation, "failed to evaluate preconditions", err), resource + } else if !preconditionsPassed { + return ruleResponse(rule, utils.Mutation, "preconditions not met", response.RuleStatusSkip), resource + } + + elements, err := evaluateList(foreach.List, ctx.JSONContext) + if err != nil { + msg := fmt.Sprintf("failed to evaluate list %s", foreach.List) + return ruleError(rule, utils.Mutation, msg, err), resource + } + + ctx.JSONContext.Checkpoint() + defer ctx.JSONContext.Restore() + + applyCount := 0 + patchedResource := resource + allPatches := make([][]byte, 0) + for _, e := range elements { + ctx.JSONContext.Reset() + + ctx := ctx.Copy() + if err := addElementToContext(ctx, e); err != nil { + logger.Error(err, "failed to add element to context") + return ruleError(rule, utils.Mutation, "failed to process foreach", err), resource + } + + var skip = false + err, mutateResp := mutateResource(rule, ctx.JSONContext, patchedResource, logger) + if err != nil && !skip { + return ruleResponse(rule, utils.Mutation, err.Error(), response.RuleStatusError), resource + } + + patchedResource = mutateResp.patchedResource + if len(mutateResp.patches) > 0 { + allPatches = append(allPatches, mutateResp.patches...) + } + + applyCount++ + } + + if applyCount == 0 { + return ruleResponse(rule, utils.Mutation, "0 elements processed", response.RuleStatusSkip), resource + } + + r := ruleResponse(rule, utils.Mutation, fmt.Sprintf("%d elements processed", applyCount), response.RuleStatusPass) + r.Patches = allPatches + return r, patchedResource +} + +type mutateResponse struct { + skip bool + patchedResource unstructured.Unstructured + patches [][]byte +} + +func mutateResource(rule *kyverno.Rule, ctx *context.Context, resource unstructured.Unstructured, logger logr.Logger) (error, *mutateResponse) { + mutateResp := &mutateResponse{false, unstructured.Unstructured{}, nil} + anyAllConditions, err := variables.SubstituteAllInPreconditions(logger, ctx, rule.AnyAllConditions) + if err != nil { + return errors.Wrapf(err, "failed to substitute vars in preconditions"), mutateResp + } + + copyConditions, err := transformConditions(anyAllConditions) + if err != nil { + return errors.Wrapf(err, "failed to load context"), mutateResp + } + + if !variables.EvaluateConditions(logger, ctx, copyConditions) { + return errors.Wrapf(err, "preconditions mismatch"), mutateResp + } + + updatedRule, err := variables.SubstituteAllInRule(logger, ctx, *rule) + if err != nil { + return errors.Wrapf(err, "variable substitution failed"), mutateResp + } + + mutation := updatedRule.Mutation.DeepCopy() + mutateHandler := mutate.CreateMutateHandler(updatedRule.Name, mutation, resource, ctx, logger) + resp, patchedResource := mutateHandler.Handle() + if resp.Status == response.RuleStatusPass { + // - overlay pattern does not match the resource conditions + if resp.Patches == nil { + mutateResp.skip = true + return fmt.Errorf("resource does not match pattern"), mutateResp + } + + mutateResp.skip = false + mutateResp.patchedResource = patchedResource + mutateResp.patches = resp.Patches + logger.V(4).Info("mutate rule applied successfully", "ruleName", rule.Name) + } + + if err := ctx.AddResourceAsObject(patchedResource.Object); err != nil { + logger.Error(err, "failed to update resource in the JSON context") + } + + return nil, mutateResp } func startMutateResultResponse(resp *response.EngineResponse, policy kyverno.ClusterPolicy, resource unstructured.Unstructured) { diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index d5aa5b69b2..d1a5959569 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -2,6 +2,7 @@ package engine import ( "encoding/json" + "github.com/kyverno/kyverno/pkg/engine/response" "reflect" "testing" @@ -88,6 +89,9 @@ func Test_VariableSubstitutionOverlay(t *testing.T) { NewResource: *resourceUnstructured} er := Mutate(policyContext) t.Log(string(expectedPatch)) + + assert.Equal(t, len(er.PolicyResponse.Rules), 1) + assert.Equal(t, len(er.PolicyResponse.Rules[0].Patches), 1) t.Log(string(er.PolicyResponse.Rules[0].Patches[0])) if !reflect.DeepEqual(expectedPatch, er.PolicyResponse.Rules[0].Patches[0]) { t.Error("patches dont match") @@ -254,6 +258,8 @@ func Test_variableSubstitutionCLI(t *testing.T) { } er := Mutate(policyContext) + assert.Equal(t, len(er.PolicyResponse.Rules), 1) + assert.Equal(t, len(er.PolicyResponse.Rules[0].Patches), 1) t.Log(string(expectedPatch)) t.Log(string(er.PolicyResponse.Rules[0].Patches[0])) if !reflect.DeepEqual(expectedPatch, er.PolicyResponse.Rules[0].Patches[0]) { @@ -365,6 +371,10 @@ func Test_chained_rules(t *testing.T) { assert.NilError(t, err) assert.Equal(t, containers[0].(map[string]interface{})["image"], "otherregistry.corp.com/foo/bash:5.0") + assert.Equal(t, len(er.PolicyResponse.Rules), 2) + assert.Equal(t, len(er.PolicyResponse.Rules[0].Patches), 1) + assert.Equal(t, len(er.PolicyResponse.Rules[1].Patches), 1) + assert.Equal(t, string(er.PolicyResponse.Rules[0].Patches[0]), `{"op":"replace","path":"/spec/containers/0/image","value":"myregistry.corp.com/foo/bash:5.0"}`) assert.Equal(t, string(er.PolicyResponse.Rules[1].Patches[0]), `{"op":"replace","path":"/spec/containers/0/image","value":"otherregistry.corp.com/foo/bash:5.0"}`) } @@ -548,3 +558,107 @@ func Test_nonZeroIndexNumberPatchesJson6902(t *testing.T) { t.Error("patches don't match") } } + +func Test_foreach(t *testing.T) { + policyRaw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "replace-image-registry" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "replace-image-registry", + "match": { + "resources": { + "kinds": [ + "Pod" + ] + } + }, + "mutate": { + "foreach": { + "list": "request.object.spec.containers", + "patchStrategicMerge": { + "spec": { + "containers": [ + { + "name": "{{ element.name }}", + "image": "registry.io/{{images.containers.{{element.name}}.path}}:{{images.containers.{{element.name}}.tag}}" + } + ] + } + } + } + } + } + ] + } +}`) + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "test" + }, + "spec": { + "containers": [ + { + "name": "test1", + "image": "foo1/bash1:5.0" + }, + { + "name": "test2", + "image": "foo2/bash2:5.0" + }, + { + "name": "test3", + "image": "foo3/bash3:5.0" + } + ] + } +}`) + var policy kyverno.ClusterPolicy + err := json.Unmarshal(policyRaw, &policy) + assert.NilError(t, err) + + resource, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContext() + err = ctx.AddResourceAsObject(resource.Object) + assert.NilError(t, err) + + policyContext := &PolicyContext{ + Policy: policy, + JSONContext: ctx, + NewResource: *resource, + } + + err = ctx.AddImageInfo(resource) + assert.NilError(t, err) + + err = context.MutateResourceWithImageInfo(resourceRaw, ctx) + assert.NilError(t, err) + + er := Mutate(policyContext) + + assert.Equal(t, len(er.PolicyResponse.Rules), 1) + assert.Equal(t, er.PolicyResponse.Rules[0].Status, response.RuleStatusPass) + + containers, _, err := unstructured.NestedSlice(er.PatchedResource.Object, "spec", "containers") + assert.NilError(t, err) + for _, c := range containers { + ctnr := c.(map[string]interface{}) + switch ctnr["name"] { + case "test1": + assert.Equal(t, ctnr["image"], "registry.io/foo1/bash1:5.0") + case "test2": + assert.Equal(t, ctnr["image"], "registry.io/foo2/bash2:5.0") + case "test3": + assert.Equal(t, ctnr["image"], "registry.io/foo3/bash3:5.0") + } + } +} diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index a2a808b249..1bad375ad8 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -1,8 +1,13 @@ package engine import ( - "errors" "fmt" + "github.com/go-logr/logr" + "github.com/kyverno/kyverno/pkg/engine/context" + "github.com/kyverno/kyverno/pkg/engine/response" + engineUtils "github.com/kyverno/kyverno/pkg/engine/utils" + "github.com/kyverno/kyverno/pkg/engine/variables" + "github.com/pkg/errors" "reflect" "strings" "time" @@ -435,3 +440,54 @@ func ManagedPodResource(policy kyverno.ClusterPolicy, resource unstructured.Unst return false } + +func checkPreconditions(logger logr.Logger, ctx *PolicyContext, anyAllConditions apiextensions.JSON) (bool, error) { + preconditions, err := variables.SubstituteAllInPreconditions(logger, ctx.JSONContext, anyAllConditions) + if err != nil { + return false, errors.Wrapf(err, "failed to substitute variables in preconditions") + } + + typeConditions, err := transformConditions(preconditions) + if err != nil { + return false, errors.Wrapf(err, "failed to parse preconditions") + } + + pass := variables.EvaluateConditions(logger, ctx.JSONContext, typeConditions) + return pass, nil +} + +func evaluateList(jmesPath string, ctx context.EvalInterface) ([]interface{}, error) { + i, err := ctx.Query(jmesPath) + if err != nil { + return nil, err + } + + l, ok := i.([]interface{}) + if !ok { + return []interface{}{i}, nil + } + + return l, nil +} + +func ruleError(rule *kyverno.Rule, ruleType engineUtils.RuleType, msg string, err error) *response.RuleResponse { + msg = fmt.Sprintf("%s: %s", msg, err.Error()) + return ruleResponse(rule, ruleType, msg, response.RuleStatusError) +} + +func ruleResponse(rule *kyverno.Rule, ruleType engineUtils.RuleType, msg string, status response.RuleStatus) *response.RuleResponse { + return &response.RuleResponse{ + Name: rule.Name, + Type: ruleType.String(), + Message: msg, + Status: status, + } +} + +func incrementAppliedCount(resp *response.EngineResponse) { + resp.PolicyResponse.RulesAppliedCount++ +} + +func incrementErrorCount(resp *response.EngineResponse) { + resp.PolicyResponse.RulesErrorCount++ +} \ No newline at end of file diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 3459ca7746..6203450022 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -75,14 +75,6 @@ func buildResponse(ctx *PolicyContext, resp *response.EngineResponse, startTime resp.PolicyResponse.PolicyExecutionTimestamp = startTime.Unix() } -func incrementAppliedCount(resp *response.EngineResponse) { - resp.PolicyResponse.RulesAppliedCount++ -} - -func incrementErrorCount(resp *response.EngineResponse) { - resp.PolicyResponse.RulesErrorCount++ -} - func validateResource(log logr.Logger, ctx *PolicyContext) *response.EngineResponse { resp := &response.EngineResponse{} if ManagedPodResource(ctx.Policy, ctx.NewResource) { @@ -188,19 +180,19 @@ func newForeachValidator(log logr.Logger, ctx *PolicyContext, rule *kyverno.Rule func (v *validator) validate() *response.RuleResponse { if err := v.loadContext(); err != nil { - return ruleError(v.rule, "failed to load context", err) + return ruleError(v.rule, utils.Validation, "failed to load context", err) } - preconditionsPassed, err := v.checkPreconditions() + preconditionsPassed, err := checkPreconditions(v.log, v.ctx, v.anyAllConditions) if err != nil { - return ruleError(v.rule, "failed to evaluate preconditions", err) + return ruleError(v.rule, utils.Validation, "failed to evaluate preconditions", err) } else if !preconditionsPassed { - return ruleResponse(v.rule, "preconditions not met", response.RuleStatusSkip) + return ruleResponse(v.rule, utils.Validation, "preconditions not met", response.RuleStatusSkip) } if v.pattern != nil || v.anyPattern != nil { if err = v.substitutePatterns(); err != nil { - return ruleError(v.rule, "variable substitution failed", err) + return ruleError(v.rule, utils.Validation, "variable substitution failed", err) } ruleResponse := v.validateResourceWithRule() @@ -217,14 +209,14 @@ func (v *validator) validate() *response.RuleResponse { func (v *validator) validateForEach() *response.RuleResponse { if err := v.loadContext(); err != nil { - return ruleError(v.rule, "failed to load context", err) + return ruleError(v.rule, utils.Validation, "failed to load context", err) } - preconditionsPassed, err := v.checkPreconditions() + preconditionsPassed, err := checkPreconditions(v.log, v.ctx, v.anyAllConditions) if err != nil { - return ruleError(v.rule, "failed to evaluate preconditions", err) + return ruleError(v.rule, utils.Validation, "failed to evaluate preconditions", err) } else if !preconditionsPassed { - return ruleResponse(v.rule, "preconditions not met", response.RuleStatusSkip) + return ruleResponse(v.rule, utils.Validation, "preconditions not met", response.RuleStatusSkip) } foreach := v.rule.Validation.ForEachValidation @@ -232,10 +224,10 @@ func (v *validator) validateForEach() *response.RuleResponse { return nil } - elements, err := v.evaluateList(foreach.List) + elements, err := evaluateList(foreach.List, v.ctx.JSONContext) if err != nil { msg := fmt.Sprintf("failed to evaluate list %s", foreach.List) - return ruleError(v.rule, msg, err) + return ruleError(v.rule, utils.Validation, msg, err) } v.ctx.JSONContext.Checkpoint() @@ -248,7 +240,7 @@ func (v *validator) validateForEach() *response.RuleResponse { ctx := v.ctx.Copy() if err := addElementToContext(ctx, e); err != nil { v.log.Error(err, "failed to add element to context") - return ruleError(v.rule, "failed to process foreach", err) + return ruleError(v.rule, utils.Validation, "failed to process foreach", err) } foreachValidator := newForeachValidator(v.log, ctx, v.rule) @@ -261,17 +253,17 @@ func (v *validator) validateForEach() *response.RuleResponse { continue } else if r.Status != response.RuleStatusPass { msg := fmt.Sprintf("validation failed in foreach rule for %v", r.Message) - return ruleResponse(v.rule, msg, r.Status) + return ruleResponse(v.rule, utils.Validation, msg, r.Status) } applyCount++ } if applyCount == 0 { - return ruleResponse(v.rule, "rule skipped", response.RuleStatusSkip) + return ruleResponse(v.rule, utils.Validation, "rule skipped", response.RuleStatusSkip) } - return ruleResponse(v.rule, "rule passed", response.RuleStatusPass) + return ruleResponse(v.rule, utils.Validation, "rule passed", response.RuleStatusPass) } func addElementToContext(ctx *PolicyContext, e interface{}) error { @@ -295,20 +287,6 @@ func addElementToContext(ctx *PolicyContext, e interface{}) error { return nil } -func (v *validator) evaluateList(jmesPath string) ([]interface{}, error) { - i, err := v.ctx.JSONContext.Query(jmesPath) - if err != nil { - return nil, err - } - - l, ok := i.([]interface{}) - if !ok { - return []interface{}{i}, nil - } - - return l, nil -} - func (v *validator) loadContext() error { if err := LoadContext(v.log, v.contextEntries, v.ctx.ResourceCache, v.ctx, v.rule.Name); err != nil { if _, ok := err.(gojmespath.NotFoundError); ok { @@ -323,43 +301,28 @@ func (v *validator) loadContext() error { return nil } -func (v *validator) checkPreconditions() (bool, error) { - preconditions, err := variables.SubstituteAllInPreconditions(v.log, v.ctx.JSONContext, v.anyAllConditions) - if err != nil { - return false, errors.Wrapf(err, "failed to substitute variables in preconditions") - } - - typeConditions, err := transformConditions(preconditions) - if err != nil { - return false, errors.Wrapf(err, "failed to parse preconditions") - } - - pass := variables.EvaluateConditions(v.log, v.ctx.JSONContext, typeConditions) - return pass, nil -} - func (v *validator) validateDeny() *response.RuleResponse { anyAllCond := v.deny.AnyAllConditions anyAllCond, err := variables.SubstituteAll(v.log, v.ctx.JSONContext, anyAllCond) if err != nil { - return ruleError(v.rule, "failed to substitute variables in deny conditions", err) + return ruleError(v.rule, utils.Validation, "failed to substitute variables in deny conditions", err) } if err = v.substituteDeny(); err != nil { - return ruleError(v.rule, "failed to substitute variables in rule", err) + return ruleError(v.rule, utils.Validation, "failed to substitute variables in rule", err) } denyConditions, err := transformConditions(anyAllCond) if err != nil { - return ruleError(v.rule, "invalid deny conditions", err) + return ruleError(v.rule, utils.Validation, "invalid deny conditions", err) } deny := variables.EvaluateConditions(v.log, v.ctx.JSONContext, denyConditions) if deny { - return ruleResponse(v.rule, v.getDenyMessage(deny), response.RuleStatusFail) + return ruleResponse(v.rule, utils.Validation, v.getDenyMessage(deny), response.RuleStatusFail) } - return ruleResponse(v.rule, v.getDenyMessage(deny), response.RuleStatusPass) + return ruleResponse(v.rule, utils.Validation, v.getDenyMessage(deny), response.RuleStatusPass) } func (v *validator) getDenyMessage(deny bool) string { @@ -464,22 +427,22 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon v.log.V(3).Info("validation error", "path", pe.Path, "error", err.Error()) if pe.Skip { - return ruleResponse(v.rule, pe.Error(), response.RuleStatusSkip) + return ruleResponse(v.rule, utils.Validation, pe.Error(), response.RuleStatusSkip) } if pe.Path == "" { - return ruleResponse(v.rule, v.buildErrorMessage(err, ""), response.RuleStatusError) + return ruleResponse(v.rule, utils.Validation, v.buildErrorMessage(err, ""), response.RuleStatusError) } - return ruleResponse(v.rule, v.buildErrorMessage(err, pe.Path), response.RuleStatusFail) + return ruleResponse(v.rule, utils.Validation, v.buildErrorMessage(err, pe.Path), response.RuleStatusFail) } else { - return ruleResponse(v.rule, v.buildErrorMessage(err, pe.Path), response.RuleStatusError) + return ruleResponse(v.rule, utils.Validation, v.buildErrorMessage(err, pe.Path), response.RuleStatusError) } } v.log.V(4).Info("successfully processed rule") msg := fmt.Sprintf("validation rule '%s' passed.", v.rule.Name) - return ruleResponse(v.rule, msg, response.RuleStatusPass) + return ruleResponse(v.rule, utils.Validation, msg, response.RuleStatusPass) } if v.anyPattern != nil { @@ -489,14 +452,14 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon anyPatterns, err := deserializeAnyPattern(v.anyPattern) if err != nil { msg := fmt.Sprintf("failed to deserialize anyPattern, expected type array: %v", err) - return ruleResponse(v.rule, msg, response.RuleStatusError) + return ruleResponse(v.rule, utils.Validation, msg, response.RuleStatusError) } for idx, pattern := range anyPatterns { err := validate.MatchPattern(v.log, resource.Object, pattern) if err == nil { msg := fmt.Sprintf("validation rule '%s' anyPattern[%d] passed.", v.rule.Name, idx) - return ruleResponse(v.rule, msg, response.RuleStatusPass) + return ruleResponse(v.rule, utils.Validation, msg, response.RuleStatusPass) } if pe, ok := err.(*validate.PatternError); ok { @@ -520,11 +483,11 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon v.log.V(4).Info(fmt.Sprintf("Validation rule '%s' failed. %s", v.rule.Name, errorStr)) msg := buildAnyPatternErrorMessage(v.rule, errorStr) - return ruleResponse(v.rule, msg, response.RuleStatusFail) + return ruleResponse(v.rule, utils.Validation, msg, response.RuleStatusFail) } } - return ruleResponse(v.rule, v.rule.Validation.Message, response.RuleStatusPass) + return ruleResponse(v.rule, utils.Validation, v.rule.Validation.Message, response.RuleStatusPass) } func deserializeAnyPattern(anyPattern apiextensions.JSON) ([]interface{}, error) { @@ -621,17 +584,3 @@ func (v *validator) substituteDeny() error { v.deny = i.(*kyverno.Deny) return nil } - -func ruleError(rule *kyverno.Rule, msg string, err error) *response.RuleResponse { - msg = fmt.Sprintf("%s: %s", msg, err.Error()) - return ruleResponse(rule, msg, response.RuleStatusError) -} - -func ruleResponse(rule *kyverno.Rule, msg string, status response.RuleStatus) *response.RuleResponse { - return &response.RuleResponse{ - Name: rule.Name, - Type: utils.Validation.String(), - Message: msg, - Status: status, - } -} From fa1816d6057a4b18d961fb9d8250caf30cad8b51 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Wed, 6 Oct 2021 21:50:26 -0700 Subject: [PATCH 2/5] fix tests Signed-off-by: Jim Bugwadia --- pkg/engine/mutation.go | 13 +++++++++---- pkg/testrunner/scenario.go | 10 ++++++++-- test/output/pod-with-emptydir.yaml | 4 ++-- test/output/pod-with-hostpath.yaml | 2 +- test/resources/pod-with-hostpath.yaml | 1 + .../samples/best_practices/add_safe_to_evict.yaml | 12 ++++++++---- .../samples/best_practices/add_safe_to_evict2.yaml | 4 ++++ .../samples/best_practices/add_safe_to_evict3.yaml | 12 +++++++++++- 8 files changed, 44 insertions(+), 14 deletions(-) diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 13deaa8acb..b9677b9edc 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -98,16 +98,19 @@ func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) { if rule.Mutation.ForEachMutation != nil { ruleResp, patchedResource = mutateForEachResource(ruleCopy, policyContext, patchedResource, logger) } else { - skip := false err, mutateResp := mutateResource(ruleCopy, policyContext.JSONContext, patchedResource, logger) if err != nil { - if skip { + if mutateResp.skip { ruleResp = ruleResponse(&rule, utils.Mutation, err.Error(), response.RuleStatusSkip) } else { ruleResp = ruleResponse(&rule, utils.Mutation, err.Error(), response.RuleStatusError) } } else { - ruleResp = ruleResponse(&rule, utils.Mutation, "mutated resource", response.RuleStatusPass) + if mutateResp.message == "" { + mutateResp.message = "mutated resource" + } + + ruleResp = ruleResponse(&rule, utils.Mutation, mutateResp.message, response.RuleStatusPass) ruleResp.Patches = mutateResp.patches patchedResource = mutateResp.patchedResource } @@ -193,10 +196,11 @@ type mutateResponse struct { skip bool patchedResource unstructured.Unstructured patches [][]byte + message string } func mutateResource(rule *kyverno.Rule, ctx *context.Context, resource unstructured.Unstructured, logger logr.Logger) (error, *mutateResponse) { - mutateResp := &mutateResponse{false, unstructured.Unstructured{}, nil} + mutateResp := &mutateResponse{false, unstructured.Unstructured{}, nil, ""} anyAllConditions, err := variables.SubstituteAllInPreconditions(logger, ctx, rule.AnyAllConditions) if err != nil { return errors.Wrapf(err, "failed to substitute vars in preconditions"), mutateResp @@ -229,6 +233,7 @@ func mutateResource(rule *kyverno.Rule, ctx *context.Context, resource unstructu mutateResp.skip = false mutateResp.patchedResource = patchedResource mutateResp.patches = resp.Patches + mutateResp.message = resp.Message logger.V(4).Info("mutate rule applied successfully", "ruleName", rule.Name) } diff --git a/pkg/testrunner/scenario.go b/pkg/testrunner/scenario.go index 4a719132dc..c20fe5231e 100644 --- a/pkg/testrunner/scenario.go +++ b/pkg/testrunner/scenario.go @@ -230,11 +230,11 @@ func validateResource(t *testing.T, responseResource unstructured.Unstructured, return } - resourcePrint(responseResource, "response resource") - resourcePrint(*expectedResource, "expected resource") // compare the resources if !reflect.DeepEqual(responseResource, *expectedResource) { t.Error("failed: response resource returned does not match expected resource") + resourcePrint(responseResource, "response resource") + resourcePrint(*expectedResource, "expected resource") return } t.Log("success: response resource returned matches expected resource") @@ -339,6 +339,12 @@ func loadPolicyResource(t *testing.T, file string) *unstructured.Unstructured { t.Logf("more than one resource specified in the file %s", file) t.Log("considering the first one for policy application") } + + for _, r := range resources { + metadata := r.UnstructuredContent()["metadata"].(map[string]interface{}) + delete(metadata, "creationTimestamp") + } + return resources[0] } diff --git a/test/output/pod-with-emptydir.yaml b/test/output/pod-with-emptydir.yaml index e7bc8b2897..5593606614 100644 --- a/test/output/pod-with-emptydir.yaml +++ b/test/output/pod-with-emptydir.yaml @@ -3,7 +3,7 @@ kind: Pod metadata: name: pod-with-emptydir annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict: true + cluster-autoscaler.kubernetes.io/safe-to-evict: "true" spec: containers: - image: k8s.gcr.io/test-webserver @@ -13,4 +13,4 @@ spec: name: cache-volume volumes: - name: cache-volume - emptyDir: {} \ No newline at end of file + emptyDir: {} diff --git a/test/output/pod-with-hostpath.yaml b/test/output/pod-with-hostpath.yaml index 30d6801b05..b011aa1e09 100644 --- a/test/output/pod-with-hostpath.yaml +++ b/test/output/pod-with-hostpath.yaml @@ -3,7 +3,7 @@ kind: Pod metadata: name: pod-with-hostpath annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict: true + cluster-autoscaler.kubernetes.io/safe-to-evict: "true" spec: containers: - image: k8s.gcr.io/test-webserver diff --git a/test/resources/pod-with-hostpath.yaml b/test/resources/pod-with-hostpath.yaml index 48f81def6d..6280c40b35 100644 --- a/test/resources/pod-with-hostpath.yaml +++ b/test/resources/pod-with-hostpath.yaml @@ -2,6 +2,7 @@ apiVersion: v1 kind: Pod metadata: name: pod-with-hostpath + annotations: spec: containers: - image: k8s.gcr.io/test-webserver diff --git a/test/scenarios/samples/best_practices/add_safe_to_evict.yaml b/test/scenarios/samples/best_practices/add_safe_to_evict.yaml index 46c39b0381..22a7507b18 100644 --- a/test/scenarios/samples/best_practices/add_safe_to_evict.yaml +++ b/test/scenarios/samples/best_practices/add_safe_to_evict.yaml @@ -15,7 +15,11 @@ expected: namespace: '' name: pod-with-emptydir rules: - - name: annotate-empty-dir - type: Mutation - status: pass - message: "successfully processed strategic merge patch" \ No newline at end of file + - name: annotate-empty-dir + type: Mutation + status: pass + message: "successfully processed strategic merge patch" + - name: annotate-host-path + type: Mutation + status: skip + message: "resource does not match pattern" \ No newline at end of file diff --git a/test/scenarios/samples/best_practices/add_safe_to_evict2.yaml b/test/scenarios/samples/best_practices/add_safe_to_evict2.yaml index 085b3b0185..b3d985abe0 100644 --- a/test/scenarios/samples/best_practices/add_safe_to_evict2.yaml +++ b/test/scenarios/samples/best_practices/add_safe_to_evict2.yaml @@ -15,6 +15,10 @@ expected: namespace: '' name: pod-with-hostpath rules: + - name: annotate-empty-dir + type: Mutation + status: skip + message: "resource does not match pattern" - name: annotate-host-path type: Mutation status: pass diff --git a/test/scenarios/samples/best_practices/add_safe_to_evict3.yaml b/test/scenarios/samples/best_practices/add_safe_to_evict3.yaml index dc4e6fd411..d5e8f74a23 100644 --- a/test/scenarios/samples/best_practices/add_safe_to_evict3.yaml +++ b/test/scenarios/samples/best_practices/add_safe_to_evict3.yaml @@ -14,4 +14,14 @@ expected: apiVersion: v1 namespace: '' name: pod-with-default-volume - rules: \ No newline at end of file + rules: + - name: annotate-empty-dir + type: Mutation + status: skip + message: "resource does not match pattern" + - name: annotate-host-path + type: Mutation + status: skip + message: "resource does not match pattern" + + \ No newline at end of file From 683543d8e2279075b501a243d095358cdd98df93 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Wed, 6 Oct 2021 22:05:28 -0700 Subject: [PATCH 3/5] fmt Signed-off-by: Jim Bugwadia --- pkg/engine/mutation.go | 6 +++--- pkg/engine/utils.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index b9677b9edc..b48020874e 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -193,10 +193,10 @@ func mutateForEachResource(rule *kyverno.Rule, ctx *PolicyContext, resource unst } type mutateResponse struct { - skip bool + skip bool patchedResource unstructured.Unstructured - patches [][]byte - message string + patches [][]byte + message string } func mutateResource(rule *kyverno.Rule, ctx *context.Context, resource unstructured.Unstructured, logger logr.Logger) (error, *mutateResponse) { diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 1bad375ad8..de8be81230 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -490,4 +490,4 @@ func incrementAppliedCount(resp *response.EngineResponse) { func incrementErrorCount(resp *response.EngineResponse) { resp.PolicyResponse.RulesErrorCount++ -} \ No newline at end of file +} From 4c6344202816850b9dbc62444a9a634d03432406 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Wed, 6 Oct 2021 22:19:47 -0700 Subject: [PATCH 4/5] separate MutateResourceWithImageInfo from buildContext and add comments Signed-off-by: Jim Bugwadia --- pkg/engine/context/imageutils.go | 3 +++ pkg/webhooks/server.go | 9 +++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/engine/context/imageutils.go b/pkg/engine/context/imageutils.go index 0fb90f6c05..3c3ec25c15 100644 --- a/pkg/engine/context/imageutils.go +++ b/pkg/engine/context/imageutils.go @@ -192,6 +192,9 @@ func addDefaultDomain(name string) string { return name } +// MutateResourceWithImageInfo will set images to their canonical form so that they can be compared +// in a predictable manner. This sets the default registry as `docker.io` and the tag as `latest` if +// these are missing. func MutateResourceWithImageInfo(raw []byte, ctx *Context) error { images := ctx.ImageInfo() if images == nil { diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 33df5255cb..c189b8e7f1 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -320,6 +320,11 @@ func (ws *WebhookServer) resourceMutation(request *v1beta1.AdmissionRequest) *v1 return failureResponse(err.Error()) } + // update container images to a canonical form + if err := enginectx.MutateResourceWithImageInfo(request.Object.Raw, policyContext.JSONContext); err != nil { + ws.log.Error(err, "failed to patch images info to resource, policies that mutate images may be impacted") + } + mutatePatches := ws.applyMutatePolicies(request, policyContext, mutatePolicies, requestTime, logger) newRequest := patchRequest(mutatePatches, request, logger) @@ -373,10 +378,6 @@ func (ws *WebhookServer) buildPolicyContext(request *v1beta1.AdmissionRequest, a return nil, errors.Wrap(err, "failed to add image information to the policy rule context") } - if err := enginectx.MutateResourceWithImageInfo(request.Object.Raw, ctx); err != nil { - ws.log.Error(err, "failed to patch images info to resource, policies that mutate images may be impacted") - } - policyContext := &engine.PolicyContext{ NewResource: resource, AdmissionInfo: userRequestInfo, From 1c0a30310627ec3f1ee73d24a506f66dc9a059f3 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Wed, 6 Oct 2021 22:48:56 -0700 Subject: [PATCH 5/5] fix merge error Signed-off-by: Jim Bugwadia --- pkg/engine/imageVerify.go | 8 ++++---- pkg/engine/utils/utils.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index 4d5c71f685..b678dc333b 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -175,7 +175,7 @@ func (iv *imageVerifier) attestImage(repository, key string, imageInfo *context. statements, err := cosign.FetchAttestations(image, []byte(key), repository) if err != nil { iv.logger.Info("failed to fetch attestations", "image", image, "error", err, "duration", time.Since(start).Seconds()) - return ruleError(iv.rule, fmt.Sprintf("failed to fetch attestations for %s", image), err) + return ruleError(iv.rule, utils.ImageVerify, fmt.Sprintf("failed to fetch attestations for %s", image), err) } iv.logger.V(3).Info("received attested statements", "statements", statements) @@ -186,13 +186,13 @@ func (iv *imageVerifier) attestImage(repository, key string, imageInfo *context. if ac.PredicateType == predicateType { val, err := iv.checkAttestations(ac, s, imageInfo) if err != nil { - return ruleError(iv.rule, "error while checking attestation", err) + return ruleError(iv.rule, utils.ImageVerify, "error while checking attestation", err) } if !val { msg := fmt.Sprintf("attestation checks failed for %s and predicate %s", imageInfo.String(), predicateType) iv.logger.Info(msg) - return ruleResponse(iv.rule, msg, response.RuleStatusFail) + return ruleResponse(iv.rule, utils.ImageVerify, msg, response.RuleStatusFail) } } } @@ -200,7 +200,7 @@ func (iv *imageVerifier) attestImage(repository, key string, imageInfo *context. msg := fmt.Sprintf("attestation checks passed for %s", imageInfo.String()) iv.logger.V(2).Info(msg) - return ruleResponse(iv.rule, msg, response.RuleStatusPass) + return ruleResponse(iv.rule, utils.ImageVerify, msg, response.RuleStatusPass) } func (iv *imageVerifier) checkAttestations(a *v1.Attestation, s map[string]interface{}, img *context.ImageInfo) (bool, error) { diff --git a/pkg/engine/utils/utils.go b/pkg/engine/utils/utils.go index f9b621060d..4c01a85ba8 100644 --- a/pkg/engine/utils/utils.go +++ b/pkg/engine/utils/utils.go @@ -20,8 +20,8 @@ const ( Validation //Generation type for generation rule Generation - //All type for other rule operations(future) - All + // ImageVerify type for image verification + ImageVerify ) func (ri RuleType) String() string {