From 731fdb3e0708078d0c234435f80711bec0cee2a6 Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Thu, 9 Jan 2020 12:23:05 -0800 Subject: [PATCH] validate paths in variable substitution is present --- pkg/engine/context/evaluate.go | 11 +- pkg/engine/variables/validatevariables.go | 80 +++++++++ .../variables/validatevariables_test.go | 160 ++++++++++++++++++ pkg/engine/variables/variables.go | 4 +- pkg/engine/variables/variables_check.go | 2 +- 5 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 pkg/engine/variables/validatevariables.go create mode 100644 pkg/engine/variables/validatevariables_test.go diff --git a/pkg/engine/context/evaluate.go b/pkg/engine/context/evaluate.go index aa522e4fab..70df8ba9b1 100644 --- a/pkg/engine/context/evaluate.go +++ b/pkg/engine/context/evaluate.go @@ -2,9 +2,10 @@ package context import ( "encoding/json" + "fmt" "github.com/golang/glog" - "github.com/jmespath/go-jmespath" + jmespath "github.com/jmespath/go-jmespath" ) //Query the JSON context with JMESPATH search path @@ -14,7 +15,7 @@ func (ctx *Context) Query(query string) (interface{}, error) { queryPath, err := jmespath.Compile(query) if err != nil { glog.V(4).Infof("incorrect query %s: %v", query, err) - return emptyResult, err + return emptyResult, fmt.Errorf("incorrect query %s: %v", query, err) } // search ctx.mu.RLock() @@ -22,14 +23,14 @@ func (ctx *Context) Query(query string) (interface{}, error) { var data interface{} if err := json.Unmarshal(ctx.jsonRaw, &data); err != nil { - glog.V(4).Infof("failed to unmarshall context") - return emptyResult, err + glog.V(4).Infof("failed to unmarshall context: %v", err) + return emptyResult, fmt.Errorf("failed to unmarshall context: %v", err) } result, err := queryPath.Search(data) if err != nil { glog.V(4).Infof("failed to search query %s: %v", query, err) - return emptyResult, err + return emptyResult, fmt.Errorf("failed to search query %s: %v", query, err) } return result, nil } diff --git a/pkg/engine/variables/validatevariables.go b/pkg/engine/variables/validatevariables.go new file mode 100644 index 0000000000..5789224a22 --- /dev/null +++ b/pkg/engine/variables/validatevariables.go @@ -0,0 +1,80 @@ +package variables + +import ( + "fmt" + "regexp" + "strings" + + "github.com/nirmata/kyverno/pkg/engine/context" +) + +// ValidateVariables validates if referenced path is present +func ValidateVariables(ctx context.EvalInterface, pattern interface{}) error { + var pathsNotPresent []string + variableList := extractVariables(pattern) + for i := 0; i < len(variableList)-1; i = i + 2 { + p := variableList[i+1] + val, err := ctx.Query(p) + // reference path is not present + if err == nil && val == nil { + pathsNotPresent = append(pathsNotPresent, p) + } + } + + if len(variableList) != 0 && len(pathsNotPresent) != 0 { + return fmt.Errorf("referenced paths are not present: %s", strings.Join(pathsNotPresent, ";")) + } + + return nil +} + +// extractVariables extracts variables in the pattern +func extractVariables(pattern interface{}) []string { + switch typedPattern := pattern.(type) { + case map[string]interface{}: + return extractMap(typedPattern) + case []interface{}: + return extractArray(typedPattern) + case string: + return extractValue(typedPattern) + default: + return nil + } +} + +func extractMap(patternMap map[string]interface{}) []string { + var variableList []string + + for _, patternElement := range patternMap { + if vars := extractVariables(patternElement); vars != nil { + variableList = append(variableList, vars...) + } + } + return variableList +} + +func extractArray(patternList []interface{}) []string { + var variableList []string + + for _, patternElement := range patternList { + if vars := extractVariables(patternElement); vars != nil { + variableList = append(variableList, vars...) + } + } + return variableList +} + +func extractValue(valuePattern string) []string { + operatorVariable := getOperator(valuePattern) + variable := valuePattern[len(operatorVariable):] + return extractValueVariable(variable) +} + +func extractValueVariable(valuePattern string) []string { + variableRegex := regexp.MustCompile(variableRegex) + groups := variableRegex.FindStringSubmatch(valuePattern) + if len(groups)%2 != 0 { + return nil + } + return groups +} diff --git a/pkg/engine/variables/validatevariables_test.go b/pkg/engine/variables/validatevariables_test.go new file mode 100644 index 0000000000..9ced311f5c --- /dev/null +++ b/pkg/engine/variables/validatevariables_test.go @@ -0,0 +1,160 @@ +package variables + +import ( + "encoding/json" + "reflect" + "testing" + + kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/engine/context" + "gotest.tools/assert" + authenticationv1 "k8s.io/api/authentication/v1" +) + +func Test_ExtractVariables(t *testing.T) { + patternRaw := []byte(` + { + "name": "ns-owner-{{request.userInfo.username}}", + "data": { + "rules": [ + { + "apiGroups": [ + "" + ], + "resources": [ + "namespaces" + ], + "verbs": [ + "*" + ], + "resourceNames": [ + "{{request.object.metadata.name}}" + ] + } + ] + } + } + `) + + var pattern interface{} + json.Unmarshal(patternRaw, &pattern) + + vars := extractVariables(pattern) + result := []string{"{{request.userInfo.username}}", "request.userInfo.username", "{{request.object.metadata.name}}", "request.object.metadata.name"} + + if !reflect.DeepEqual(vars, result) { + t.Errorf("result does not match, var: %s", vars) + } +} + +func Test_ValidateVariables_NoVariable(t *testing.T) { + patternRaw := []byte(` +{ + "name": "ns-owner", + "data": { + "rules": [ + { + "apiGroups": [ + "" + ], + "resources": [ + "namespaces" + ], + "verbs": [ + "*" + ], + "resourceNames": [ + "Pod" + ] + } + ] + } +} +`) + + resourceRaw := []byte(` + { + "metadata": { + "name": "temp", + "namespace": "n1" + }, + "spec": { + "namespace": "n1", + "name": "temp1" + } + } + `) + + // userInfo + userReqInfo := kyverno.RequestInfo{ + AdmissionUserInfo: authenticationv1.UserInfo{ + Username: "user1", + }, + } + var pattern, resource interface{} + assert.NilError(t, json.Unmarshal(patternRaw, &pattern)) + assert.NilError(t, json.Unmarshal(resourceRaw, &resource)) + + ctx := context.NewContext() + ctx.AddResource(resourceRaw) + ctx.AddUserInfo(userReqInfo) + + err := ValidateVariables(ctx, pattern) + assert.NilError(t, err) +} + +func Test_ValidateVariables(t *testing.T) { + patternRaw := []byte(` +{ + "name": "ns-owner-{{request.userInfo.username}}", + "data": { + "rules": [ + { + "apiGroups": [ + "" + ], + "resources": [ + "namespaces" + ], + "verbs": [ + "*" + ], + "resourceNames": [ + "{{request.object.metadata.name1}}" + ] + } + ] + } +} +`) + + resourceRaw := []byte(` + { + "metadata": { + "name": "temp", + "namespace": "n1" + }, + "spec": { + "namespace": "n1", + "name": "temp1" + } + } + `) + + // userInfo + userReqInfo := kyverno.RequestInfo{ + AdmissionUserInfo: authenticationv1.UserInfo{ + Username: "user1", + }, + } + var pattern, resource interface{} + assert.NilError(t, json.Unmarshal(patternRaw, &pattern)) + assert.NilError(t, json.Unmarshal(resourceRaw, &resource)) + + ctx := context.NewContext() + ctx.AddResource(resourceRaw) + ctx.AddUserInfo(userReqInfo) + + err := ValidateVariables(ctx, pattern) + assert.Assert(t, err != nil) +} diff --git a/pkg/engine/variables/variables.go b/pkg/engine/variables/variables.go index 63934a7d94..7d9b352e4e 100644 --- a/pkg/engine/variables/variables.go +++ b/pkg/engine/variables/variables.go @@ -9,6 +9,8 @@ import ( "github.com/nirmata/kyverno/pkg/engine/operator" ) +const variableRegex = `\{\{([^{}]*)\}\}` + //SubstituteVariables substitutes the JMESPATH with variable substitution // supported substitutions // - no operator + variable(string,object) @@ -73,7 +75,7 @@ func substituteValue(ctx context.EvalInterface, valuePattern string) interface{} func getValueQuery(ctx context.EvalInterface, valuePattern string) interface{} { var emptyInterface interface{} // extract variable {{}} - validRegex := regexp.MustCompile(`\{\{([^{}]*)\}\}`) + validRegex := regexp.MustCompile(variableRegex) groups := validRegex.FindAllStringSubmatch(valuePattern, -1) // can have multiple variables in a single value pattern // var Map diff --git a/pkg/engine/variables/variables_check.go b/pkg/engine/variables/variables_check.go index c9023169d6..46aa56b0fe 100644 --- a/pkg/engine/variables/variables_check.go +++ b/pkg/engine/variables/variables_check.go @@ -49,7 +49,7 @@ func checkValue(valuePattern string, variables []string, path string) error { } func checkValueVariable(valuePattern string, variables []string) bool { - variableRegex := regexp.MustCompile("^{{(.*)}}$") + variableRegex := regexp.MustCompile(variableRegex) groups := variableRegex.FindStringSubmatch(valuePattern) if len(groups) < 2 { return false