From f78ca61859a96296cc55d34d8325214b7ad7fb21 Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Thu, 9 Jan 2020 12:24:37 -0800 Subject: [PATCH] generate violation in mutation when substitute path not present --- pkg/engine/mutate/overlay.go | 12 +++++- pkg/engine/mutation.go | 19 ++++++--- pkg/engine/mutation_test.go | 65 +++++++++++++++++++++++++++++ pkg/engine/response/response.go | 12 ++++++ pkg/policyviolation/builder.go | 3 +- pkg/policyviolation/builder_test.go | 60 ++++++++++++++++++++++++++ pkg/webhooks/mutation.go | 5 +++ 7 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 pkg/policyviolation/builder_test.go diff --git a/pkg/engine/mutate/overlay.go b/pkg/engine/mutate/overlay.go index 66f7e7cffb..ff5e71a47f 100644 --- a/pkg/engine/mutate/overlay.go +++ b/pkg/engine/mutate/overlay.go @@ -22,7 +22,7 @@ import ( "github.com/nirmata/kyverno/pkg/engine/variables" ) -// processOverlay processes validation patterns on the resource +// processOverlay processes mutation overlay on the resource func ProcessOverlay(ctx context.EvalInterface, rule kyverno.Rule, resource unstructured.Unstructured) (resp response.RuleResponse, patchedResource unstructured.Unstructured) { startTime := time.Now() glog.V(4).Infof("started applying overlay rule %q (%v)", rule.Name, startTime) @@ -32,6 +32,16 @@ func ProcessOverlay(ctx context.EvalInterface, rule kyverno.Rule, resource unstr resp.RuleStats.ProcessingTime = time.Since(startTime) glog.V(4).Infof("finished applying overlay rule %q (%v)", resp.Name, resp.RuleStats.ProcessingTime) }() + + // if referenced is not present, we skip processing the rule and report violation + if err := variables.ValidateVariables(ctx, rule.Mutation.Overlay); err != nil { + glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), err.Error()) + resp.Success = true + resp.PathNotPresent = true + resp.Message = err.Error() + return resp, resource + } + // substitute variables // first pass we substitute all the JMESPATH substitution for the variable // variable: {{}} diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 9d5700edaf..4bd1262397 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -82,11 +82,20 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { if rule.Mutation.Overlay != nil { var ruleResponse response.RuleResponse ruleResponse, patchedResource = mutate.ProcessOverlay(ctx, rule, patchedResource) - if ruleResponse.Success == true && ruleResponse.Patches == nil { - // overlay pattern does not match the resource conditions - glog.V(4).Infof(ruleResponse.Message) - continue - } else if ruleResponse.Success == true { + if ruleResponse.Success == true { + // - variable substitution path is not present + if ruleResponse.PathNotPresent { + glog.V(4).Infof(ruleResponse.Message) + resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse) + continue + } + + // - overlay pattern does not match the resource conditions + if ruleResponse.Patches == nil { + glog.V(4).Infof(ruleResponse.Message) + continue + } + glog.Infof("Mutate overlay in rule '%s' successfully applied on %s/%s/%s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName()) } diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index cf04432947..cc7a202903 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -88,3 +88,68 @@ func Test_VariableSubstitutionOverlay(t *testing.T) { t.Error("patches dont match") } } + +func Test_variableSubstitutionPathNotExist(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "check-root-user" + }, + "spec": { + "containers": [ + { + "name": "check-root-user", + "image": "nginxinc/nginx-unprivileged", + "securityContext": { + "runAsNonRoot": true + } + } + ] + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "substitue-variable" + }, + "spec": { + "rules": [ + { + "name": "test-path-not-exist", + "match": { + "resources": { + "kinds": [ + "Pod" + ] + } + }, + "mutate": { + "overlay": { + "spec": { + "name": "{{request.object.metadata.name1}}" + } + } + } + } + ] + } + }`) + + var policy kyverno.ClusterPolicy + json.Unmarshal(policyraw, &policy) + resourceUnstructured, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(resourceRaw) + + policyContext := PolicyContext{ + Policy: policy, + Context: ctx, + NewResource: *resourceUnstructured} + er := Mutate(policyContext) + assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent, true) +} diff --git a/pkg/engine/response/response.go b/pkg/engine/response/response.go index 10c714d75d..955d27f52e 100644 --- a/pkg/engine/response/response.go +++ b/pkg/engine/response/response.go @@ -65,6 +65,8 @@ type RuleResponse struct { Success bool `json:"success"` // statistics RuleStats `json:",inline"` + // PathNotPresent indicates whether referenced path in variable substitution exist + PathNotPresent bool `json:"pathNotPresent"` } //ToString ... @@ -119,3 +121,13 @@ func (er EngineResponse) getRules(success bool) []string { } return rules } + +// IsPathNotPresent checks if the referenced path(in variable substitution) exist +func (er EngineResponse) IsPathNotPresent() bool { + for _, r := range er.PolicyResponse.Rules { + if r.PathNotPresent { + return true + } + } + return false +} diff --git a/pkg/policyviolation/builder.go b/pkg/policyviolation/builder.go index 736a0f3380..6694923999 100644 --- a/pkg/policyviolation/builder.go +++ b/pkg/policyviolation/builder.go @@ -15,7 +15,8 @@ func GeneratePVsFromEngineResponse(ers []response.EngineResponse) (pvInfos []Inf glog.V(4).Infof("resource %v, has not been assigned a name, not creating a policy violation for it", er.PolicyResponse.Resource) continue } - if er.IsSuccesful() { + // skip when response succeed AND referenced paths exist + if er.IsSuccesful() && !er.IsPathNotPresent() { continue } glog.V(4).Infof("Building policy violation for engine response %v", er) diff --git a/pkg/policyviolation/builder_test.go b/pkg/policyviolation/builder_test.go new file mode 100644 index 0000000000..d5a5ecf2d6 --- /dev/null +++ b/pkg/policyviolation/builder_test.go @@ -0,0 +1,60 @@ +package policyviolation + +import ( + "testing" + + "github.com/nirmata/kyverno/pkg/engine/response" + "gotest.tools/assert" +) + +func Test_GeneratePVsFromEngineResponse_PathNotExist(t *testing.T) { + ers := []response.EngineResponse{ + response.EngineResponse{ + PolicyResponse: response.PolicyResponse{ + Policy: "test-substitue-variable", + Resource: response.ResourceSpec{ + Kind: "Pod", + Name: "test", + Namespace: "test", + }, + Rules: []response.RuleResponse{ + response.RuleResponse{ + Name: "test-path-not-exist", + Type: "Mutation", + Message: "referenced paths are not present: request.object.metadata.name1", + Success: true, + PathNotPresent: true, + }, + response.RuleResponse{ + Name: "test-path-exist", + Type: "Mutation", + Success: true, + PathNotPresent: false, + }, + }, + }, + }, + response.EngineResponse{ + PolicyResponse: response.PolicyResponse{ + Policy: "test-substitue-variable2", + Resource: response.ResourceSpec{ + Kind: "Pod", + Name: "test", + Namespace: "test", + }, + Rules: []response.RuleResponse{ + response.RuleResponse{ + Name: "test-path-not-exist-accross-policy", + Type: "Mutation", + Message: "referenced paths are not present: request.object.metadata.name1", + Success: true, + PathNotPresent: true, + }, + }, + }, + }, + } + + pvInfos := GeneratePVsFromEngineResponse(ers) + assert.Assert(t, len(pvInfos) == 2) +} diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index f8c0c2d34d..4103b1a5e5 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -8,6 +8,7 @@ import ( "github.com/nirmata/kyverno/pkg/engine/response" engineutils "github.com/nirmata/kyverno/pkg/engine/utils" policyctr "github.com/nirmata/kyverno/pkg/policy" + "github.com/nirmata/kyverno/pkg/policyviolation" "github.com/nirmata/kyverno/pkg/utils" v1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -96,6 +97,10 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou patches = append(patches, annPatches) } + // generate violation when referenced path does not exist + pvInfos := policyviolation.GeneratePVsFromEngineResponse(engineResponses) + ws.pvGenerator.Add(pvInfos...) + // ADD EVENTS events := generateEvents(engineResponses, (request.Operation == v1beta1.Update)) ws.eventGen.Add(events...)