From 5a44ab3e1662a97ba4ed9ef20d4440acdc4dccda Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Thu, 9 Jan 2020 17:44:11 -0800 Subject: [PATCH] generate violation in validate when substitute path not present --- pkg/engine/mutate/overlay.go | 6 +- pkg/engine/validate/common.go | 17 + pkg/engine/validate/validate.go | 15 +- pkg/engine/validation.go | 117 +++--- pkg/engine/validation_test.go | 348 +++++++++++++++++- pkg/engine/variables/validatevariables.go | 8 +- .../variables/validatevariables_test.go | 8 +- 7 files changed, 458 insertions(+), 61 deletions(-) diff --git a/pkg/engine/mutate/overlay.go b/pkg/engine/mutate/overlay.go index ff5e71a47f..262c61d04e 100644 --- a/pkg/engine/mutate/overlay.go +++ b/pkg/engine/mutate/overlay.go @@ -34,11 +34,11 @@ func ProcessOverlay(ctx context.EvalInterface, rule kyverno.Rule, resource unstr }() // 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()) + if invalidPaths := variables.ValidateVariables(ctx, rule.Mutation.Overlay); len(invalidPaths) != 0 { resp.Success = true resp.PathNotPresent = true - resp.Message = err.Error() + resp.Message = fmt.Sprintf("referenced path not present: %s", invalidPaths) + glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resp.Message) return resp, resource } diff --git a/pkg/engine/validate/common.go b/pkg/engine/validate/common.go index 79580d88eb..109831a89a 100644 --- a/pkg/engine/validate/common.go +++ b/pkg/engine/validate/common.go @@ -7,6 +7,13 @@ import ( "github.com/nirmata/kyverno/pkg/engine/operator" ) +type ValidationFailureReason int + +const ( + PathNotPresent ValidationFailureReason = iota + Rulefailure +) + func isStringIsReference(str string) bool { if len(str) < len(operator.ReferenceSign) { return false @@ -66,3 +73,13 @@ func getRawKeyIfWrappedWithAttributes(str string) string { return str } } + +type ValidationError struct { + StatusCode ValidationFailureReason + ErrorMsg string +} + +// newValidatePatternError returns an validation error using the ValidationFailureReason and errorMsg +func newValidatePatternError(reason ValidationFailureReason, msg string) ValidationError { + return ValidationError{StatusCode: reason, ErrorMsg: msg} +} diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index 58345d7ad0..73e973348a 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -17,13 +17,22 @@ import ( // validateResourceWithPattern is a start of element-by-element validation process // It assumes that validation is started from root, so "/" is passed -//TODO: for failure, we return the path at which it failed along with error -func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern interface{}) (string, error) { +func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern interface{}) (string, ValidationError) { + // if referenced path is not present, we skip processing the rule and report violation + if invalidPaths := variables.ValidateVariables(ctx, pattern); len(invalidPaths) != 0 { + return "", newValidatePatternError(PathNotPresent, invalidPaths) + } + // first pass we substitute all the JMESPATH substitution for the variable // variable: {{}} // if a JMESPATH fails, we dont return error but variable is substitured with nil and error log pattern = variables.SubstituteVariables(ctx, pattern) - return validateResourceElement(resource, pattern, pattern, "/") + path, err := validateResourceElement(resource, pattern, pattern, "/") + if err != nil { + return path, newValidatePatternError(Rulefailure, err.Error()) + } + + return "", ValidationError{} } // validateResourceElement detects the element type (map, array, nil, string, int, bool, float) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index b9670335cc..1999d0b09a 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -1,6 +1,7 @@ package engine import ( + "errors" "fmt" "reflect" "strings" @@ -17,29 +18,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func startResultResponse(resp *response.EngineResponse, policy kyverno.ClusterPolicy, newR unstructured.Unstructured) { - // set policy information - resp.PolicyResponse.Policy = policy.Name - // resource details - resp.PolicyResponse.Resource.Name = newR.GetName() - resp.PolicyResponse.Resource.Namespace = newR.GetNamespace() - resp.PolicyResponse.Resource.Kind = newR.GetKind() - resp.PolicyResponse.Resource.APIVersion = newR.GetAPIVersion() - resp.PolicyResponse.ValidationFailureAction = policy.Spec.ValidationFailureAction - -} - -func endResultResponse(resp *response.EngineResponse, startTime time.Time) { - resp.PolicyResponse.ProcessingTime = time.Since(startTime) - glog.V(4).Infof("Finished applying validation rules policy %v (%v)", resp.PolicyResponse.Policy, resp.PolicyResponse.ProcessingTime) - glog.V(4).Infof("Validation Rules appplied succesfully count %v for policy %q", resp.PolicyResponse.RulesAppliedCount, resp.PolicyResponse.Policy) -} - -func incrementAppliedCount(resp *response.EngineResponse) { - // rules applied succesfully count - resp.PolicyResponse.RulesAppliedCount++ -} - //Validate applies validation rules from policy on the resource func Validate(policyContext PolicyContext) (resp response.EngineResponse) { startTime := time.Now() @@ -87,6 +65,29 @@ func Validate(policyContext PolicyContext) (resp response.EngineResponse) { return response.EngineResponse{} } +func startResultResponse(resp *response.EngineResponse, policy kyverno.ClusterPolicy, newR unstructured.Unstructured) { + // set policy information + resp.PolicyResponse.Policy = policy.Name + // resource details + resp.PolicyResponse.Resource.Name = newR.GetName() + resp.PolicyResponse.Resource.Namespace = newR.GetNamespace() + resp.PolicyResponse.Resource.Kind = newR.GetKind() + resp.PolicyResponse.Resource.APIVersion = newR.GetAPIVersion() + resp.PolicyResponse.ValidationFailureAction = policy.Spec.ValidationFailureAction + +} + +func endResultResponse(resp *response.EngineResponse, startTime time.Time) { + resp.PolicyResponse.ProcessingTime = time.Since(startTime) + glog.V(4).Infof("Finished applying validation rules policy %v (%v)", resp.PolicyResponse.Policy, resp.PolicyResponse.ProcessingTime) + glog.V(4).Infof("Validation Rules appplied succesfully count %v for policy %q", resp.PolicyResponse.RulesAppliedCount, resp.PolicyResponse.Policy) +} + +func incrementAppliedCount(resp *response.EngineResponse) { + // rules applied succesfully count + resp.PolicyResponse.RulesAppliedCount++ +} + func validateResource(ctx context.EvalInterface, policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo) *response.EngineResponse { resp := &response.EngineResponse{} for _, rule := range policy.Spec.Rules { @@ -178,14 +179,23 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu // either pattern or anyPattern can be specified in Validation rule if rule.Validation.Pattern != nil { path, err := validate.ValidateResourceWithPattern(ctx, resource.Object, rule.Validation.Pattern) - if err != nil { - // rule application failed - glog.V(4).Infof("Validation rule '%s' failed at '%s' for resource %s/%s/%s. %s: %v", rule.Name, path, resource.GetKind(), resource.GetNamespace(), resource.GetName(), rule.Validation.Message, err) - resp.Success = false - resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", - rule.Validation.Message, rule.Name, path) + if !reflect.DeepEqual(err, validate.ValidationError{}) { + switch err.StatusCode { + case validate.PathNotPresent: + resp.Success = true + resp.PathNotPresent = true + resp.Message = fmt.Sprintf("referenced path not present: %s", err.ErrorMsg) + glog.V(4).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resp.Message) + case validate.Rulefailure: + // rule application failed + glog.V(4).Infof("Validation rule '%s' failed at '%s' for resource %s/%s/%s. %s: %v", rule.Name, path, resource.GetKind(), resource.GetNamespace(), resource.GetName(), rule.Validation.Message, err) + resp.Success = false + resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", + rule.Validation.Message, rule.Name, path) + } return resp } + // rule application succesful glog.V(4).Infof("rule %s pattern validated succesfully on resource %s/%s/%s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName()) resp.Success = true @@ -194,39 +204,54 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu } // using anyPattern we can define multiple patterns and only one of them has to be succesfully validated + // return directly if one pattern succeed + // if none succeed, report violation / policyerror(TODO) if rule.Validation.AnyPattern != nil { - var errs []error - var failedPaths []string + var ruleFailureErrs []error + var failedPaths, invalidPaths []string for index, pattern := range rule.Validation.AnyPattern { path, err := validate.ValidateResourceWithPattern(ctx, resource.Object, pattern) - if err == nil { - // this pattern was succesfully validated + // this pattern was succesfully validated + if reflect.DeepEqual(err, validate.ValidationError{}) { glog.V(4).Infof("anyPattern %v succesfully validated on resource %s/%s/%s", pattern, resource.GetKind(), resource.GetNamespace(), resource.GetName()) resp.Success = true resp.Message = fmt.Sprintf("Validation rule '%s' anyPattern[%d] succeeded.", rule.Name, index) return resp } - if err != nil { + + switch err.StatusCode { + case validate.PathNotPresent: + invalidPaths = append(invalidPaths, err.ErrorMsg) + case validate.Rulefailure: glog.V(4).Infof("Validation error: %s; Validation rule %s anyPattern[%d] failed at path %s for %s/%s/%s", rule.Validation.Message, rule.Name, index, path, resource.GetKind(), resource.GetNamespace(), resource.GetName()) - errs = append(errs, err) + ruleFailureErrs = append(ruleFailureErrs, errors.New(err.ErrorMsg)) failedPaths = append(failedPaths, path) } } - // If none of the anyPatterns are validated - if len(errs) > 0 { - glog.V(4).Infof("none of anyPattern were processed: %v", errs) - resp.Success = false - var errorStr []string - for index, err := range errs { - glog.V(4).Infof("anyPattern[%d] failed at path %s: %v", index, failedPaths[index], err) - str := fmt.Sprintf("Validation rule %s anyPattern[%d] failed at path %s.", rule.Name, index, failedPaths[index]) - errorStr = append(errorStr, str) - } - resp.Message = fmt.Sprintf("Validation error: %s; %s", rule.Validation.Message, strings.Join(errorStr, ";")) + // PathNotPresent + if len(invalidPaths) != 0 { + resp.Success = true + resp.PathNotPresent = true + resp.Message = fmt.Sprintf("referenced path not present: %s", strings.Join(invalidPaths, ";")) + glog.V(4).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resp.Message) return resp } + + // none of the anyPatterns succeed: len(ruleFailureErrs) > 0 + glog.V(4).Infof("none of anyPattern comply with resource: %v", ruleFailureErrs) + resp.Success = false + var errorStr []string + for index, err := range ruleFailureErrs { + glog.V(4).Infof("anyPattern[%d] failed at path %s: %v", index, failedPaths[index], err) + str := fmt.Sprintf("Validation rule %s anyPattern[%d] failed at path %s.", rule.Name, index, failedPaths[index]) + errorStr = append(errorStr, str) + } + + resp.Message = fmt.Sprintf("Validation error: %s; %s", rule.Validation.Message, strings.Join(errorStr, " ")) + return resp } + return response.RuleResponse{} } diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index f277663efa..3a37d2f268 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -5,6 +5,7 @@ import ( "testing" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/utils" "gotest.tools/assert" ) @@ -483,7 +484,7 @@ func TestValidate_Fail_anyPattern(t *testing.T) { resourceUnstructured, err := utils.ConvertToUnstructured(rawResource) assert.NilError(t, err) er := Validate(PolicyContext{Policy: policy, NewResource: *resourceUnstructured}) - msgs := []string{"Validation error: A namespace is required; Validation rule check-default-namespace anyPattern[0] failed at path /metadata/namespace/.;Validation rule check-default-namespace anyPattern[1] failed at path /metadata/namespace/."} + msgs := []string{"Validation error: A namespace is required; Validation rule check-default-namespace anyPattern[0] failed at path /metadata/namespace/. Validation rule check-default-namespace anyPattern[1] failed at path /metadata/namespace/."} for index, r := range er.PolicyResponse.Rules { assert.Equal(t, r.Message, msgs[index]) } @@ -1431,3 +1432,348 @@ func TestValidate_negationAnchor_pass(t *testing.T) { } assert.Assert(t, er.IsSuccesful()) } + +func Test_VariableSubstitutionPathNotExistInPattern(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "check-root-user" + }, + "spec": { + "containers": [ + { + "name": "check-root-user-a", + "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" + ] + } + }, + "validate": { + "pattern": { + "spec": { + "containers": [ + { + "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 := Validate(policyContext) + assert.Assert(t, er.PolicyResponse.Rules[0].Success, true) + assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent, true) +} + +func Test_VariableSubstitutionPathNotExistInAnyPattern_OnePatternStatisfies(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": { + "name": "test" + }, + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "test-pod", + "image": "nginxinc/nginx-unprivileged" + } + ] + } + } + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "substitue-variable" + }, + "spec": { + "rules": [ + { + "name": "test-path-not-exist", + "match": { + "resources": { + "kinds": [ + "Deployment" + ] + } + }, + "validate": { + "anyPattern": [ + { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "{{request.object.metadata.name1}}*" + } + ] + } + } + } + }, + { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "{{request.object.metadata.name}}*" + } + ] + } + } + } + } + ] + } + } + ] + } + }`) + + var policy kyverno.ClusterPolicy + assert.NilError(t, 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 := Validate(policyContext) + assert.Assert(t, er.PolicyResponse.Rules[0].Success == true) + assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent == false) + expectMsg := "Validation rule 'test-path-not-exist' anyPattern[1] succeeded." + assert.Assert(t, er.PolicyResponse.Rules[0].Message == expectMsg) +} + +func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathNotPresent(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": { + "name": "test" + }, + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "test-pod", + "image": "nginxinc/nginx-unprivileged" + } + ] + } + } + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "substitue-variable" + }, + "spec": { + "rules": [ + { + "name": "test-path-not-exist", + "match": { + "resources": { + "kinds": [ + "Deployment" + ] + } + }, + "validate": { + "anyPattern": [ + { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "{{request.object.metadata.name1}}*" + } + ] + } + } + } + }, + { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "{{request.object.metadata.name2}}*" + } + ] + } + } + } + } + ] + } + } + ] + } + }`) + + var policy kyverno.ClusterPolicy + assert.NilError(t, 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 := Validate(policyContext) + assert.Assert(t, er.PolicyResponse.Rules[0].Success, true) + assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent, true) +} + +func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathPresent_NonePatternSatisfy(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": { + "name": "test" + }, + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "pod-test-pod", + "image": "nginxinc/nginx-unprivileged" + } + ] + } + } + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "substitue-variable" + }, + "spec": { + "rules": [ + { + "name": "test-path-not-exist", + "match": { + "resources": { + "kinds": [ + "Deployment" + ] + } + }, + "validate": { + "anyPattern": [ + { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "{{request.object.metadata.name}}*" + } + ] + } + } + } + }, + { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "{{request.object.metadata.name}}*" + } + ] + } + } + } + } + ] + } + } + ] + } + }`) + + var policy kyverno.ClusterPolicy + assert.NilError(t, 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 := Validate(policyContext) + + expectedMsg := "Validation error: ; Validation rule test-path-not-exist anyPattern[0] failed at path /spec/template/spec/containers/0/name/. Validation rule test-path-not-exist anyPattern[1] failed at path /spec/template/spec/containers/0/name/." + assert.Assert(t, er.PolicyResponse.Rules[0].Success == false) + assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent == false) + assert.Assert(t, er.PolicyResponse.Rules[0].Message == expectedMsg) +} diff --git a/pkg/engine/variables/validatevariables.go b/pkg/engine/variables/validatevariables.go index 5789224a22..f5c428ee01 100644 --- a/pkg/engine/variables/validatevariables.go +++ b/pkg/engine/variables/validatevariables.go @@ -1,7 +1,6 @@ package variables import ( - "fmt" "regexp" "strings" @@ -9,7 +8,8 @@ import ( ) // ValidateVariables validates if referenced path is present -func ValidateVariables(ctx context.EvalInterface, pattern interface{}) error { +// return empty string if all paths are valid, otherwise return invalid path +func ValidateVariables(ctx context.EvalInterface, pattern interface{}) string { var pathsNotPresent []string variableList := extractVariables(pattern) for i := 0; i < len(variableList)-1; i = i + 2 { @@ -22,10 +22,10 @@ func ValidateVariables(ctx context.EvalInterface, pattern interface{}) error { } if len(variableList) != 0 && len(pathsNotPresent) != 0 { - return fmt.Errorf("referenced paths are not present: %s", strings.Join(pathsNotPresent, ";")) + return strings.Join(pathsNotPresent, ";") } - return nil + return "" } // extractVariables extracts variables in the pattern diff --git a/pkg/engine/variables/validatevariables_test.go b/pkg/engine/variables/validatevariables_test.go index 9ced311f5c..ec25c9414a 100644 --- a/pkg/engine/variables/validatevariables_test.go +++ b/pkg/engine/variables/validatevariables_test.go @@ -99,8 +99,8 @@ func Test_ValidateVariables_NoVariable(t *testing.T) { ctx.AddResource(resourceRaw) ctx.AddUserInfo(userReqInfo) - err := ValidateVariables(ctx, pattern) - assert.NilError(t, err) + invalidPaths := ValidateVariables(ctx, pattern) + assert.Assert(t, len(invalidPaths) == 0) } func Test_ValidateVariables(t *testing.T) { @@ -155,6 +155,6 @@ func Test_ValidateVariables(t *testing.T) { ctx.AddResource(resourceRaw) ctx.AddUserInfo(userReqInfo) - err := ValidateVariables(ctx, pattern) - assert.Assert(t, err != nil) + invalidPaths := ValidateVariables(ctx, pattern) + assert.Assert(t, len(invalidPaths) > 0) }