redo variable validation (kyverno#2647)

* redo variable validation

Signed-off-by: Jim Bugwadia <[email protected]>

* handle quotes for JMESPath - escaping

Signed-off-by: Jim Bugwadia <[email protected]>

* fix tests and linter issues

Signed-off-by: Jim Bugwadia <[email protected]>

* fix fmt

Signed-off-by: Jim Bugwadia <[email protected]>

pkg/engine/context/context.go

@@ -2,7 +2,6 @@ package context
 
 import (
 	"encoding/json"
-	"fmt"
 	"strings"
 	"sync"
 
@@ -56,34 +55,21 @@ type Context struct {
 	mutex              sync.RWMutex
 	jsonRaw            []byte
 	jsonRawCheckpoints [][]byte
-	builtInVars        []string
 	images             *Images
 	log                logr.Logger
 }
 
 //NewContext returns a new context
-// builtInVars is the list of known variables (e.g. serviceAccountName)
-func NewContext(builtInVars ...string) *Context {
+func NewContext() *Context {
 	ctx := Context{
 		jsonRaw:            []byte(`{}`), // empty json struct
-		builtInVars:        builtInVars,
 		log:                log.Log.WithName("context"),
 		jsonRawCheckpoints: make([][]byte, 0),
 	}
 
 	return &ctx
 }
 
-// InvalidVariableErr represents error for non-white-listed variables
-type InvalidVariableErr struct {
-	variable  string
-	whiteList []string
-}
-
-func (i InvalidVariableErr) Error() string {
-	return fmt.Sprintf("variable %s cannot be used, allowed variables: %v", i.variable, i.whiteList)
-}
-
 // AddJSON merges json data
 func (ctx *Context) AddJSON(dataRaw []byte) error {
 	var err error
@@ -359,20 +345,3 @@ func (ctx *Context) reset(remove bool) {
 		ctx.jsonRawCheckpoints = ctx.jsonRawCheckpoints[:n]
 	}
 }
-
-// AddBuiltInVars adds given pattern to the builtInVars
-func (ctx *Context) AddBuiltInVars(pattern string) {
-	ctx.mutex.Lock()
-	defer ctx.mutex.Unlock()
-
-	builtInVarsCopy := ctx.builtInVars
-	ctx.builtInVars = append(builtInVarsCopy, pattern)
-}
-
-func (ctx *Context) getBuiltInVars() []string {
-	ctx.mutex.RLock()
-	defer ctx.mutex.RUnlock()
-
-	vars := ctx.builtInVars
-	return vars
-}

pkg/engine/context/evaluate.go

@@ -19,20 +19,14 @@ func (ctx *Context) Query(query string) (interface{}, error) {
 	}
 
 	var emptyResult interface{}
-	// check for white-listed variables
-	if !ctx.isBuiltInVariable(query) {
-		return emptyResult, InvalidVariableErr{
-			variable:  query,
-			whiteList: ctx.getBuiltInVars(),
-		}
-	}
 
 	// compile the query
 	queryPath, err := jmespath.New(query)
 	if err != nil {
 		ctx.log.Error(err, "incorrect query", "query", query)
 		return emptyResult, fmt.Errorf("incorrect query %s: %v", query, err)
 	}
+
 	// search
 	ctx.mutex.RLock()
 	defer ctx.mutex.RUnlock()
@@ -55,18 +49,6 @@ func (ctx *Context) Query(query string) (interface{}, error) {
 	return result, nil
 }
 
-func (ctx *Context) isBuiltInVariable(variable string) bool {
-	if len(ctx.getBuiltInVars()) == 0 {
-		return true
-	}
-	for _, wVar := range ctx.getBuiltInVars() {
-		if strings.HasPrefix(variable, wVar) {
-			return true
-		}
-	}
-	return false
-}
-
 func (ctx *Context) HasChanged(jmespath string) (bool, error) {
 	objData, err := ctx.Query("request.object." + jmespath)
 	if err != nil {

pkg/engine/context/mock_context.go

@@ -0,0 +1,101 @@
+package context
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+	"sync"
+
+	"github.com/kyverno/kyverno/pkg/engine/jmespath"
+	"github.com/minio/pkg/wildcard"
+)
+
+//MockContext is used for testing and validation of variables
+type MockContext struct {
+	mutex           sync.RWMutex
+	re              *regexp.Regexp
+	allowedPatterns []string
+}
+
+//NewMockContext creates a new MockContext that allows variables matching the supplied list of wildcard patterns
+func NewMockContext(re *regexp.Regexp, vars ...string) *MockContext {
+	return &MockContext{re: re, allowedPatterns: vars}
+}
+
+// AddVariable adds given wildcardPattern to the allowed variable patterns
+func (ctx *MockContext) AddVariable(wildcardPattern string) {
+	ctx.mutex.Lock()
+	defer ctx.mutex.Unlock()
+
+	builtInVarsCopy := ctx.allowedPatterns
+	ctx.allowedPatterns = append(builtInVarsCopy, wildcardPattern)
+}
+
+//Query the JSON context with JMESPATH search path
+func (ctx *MockContext) Query(query string) (interface{}, error) {
+	query = strings.TrimSpace(query)
+	if query == "" {
+		return nil, fmt.Errorf("invalid query (nil)")
+	}
+
+	var emptyResult interface{}
+
+	// compile the query
+	_, err := jmespath.New(query)
+	if err != nil {
+		return emptyResult, fmt.Errorf("invalid JMESPath query %s: %v", query, err)
+	}
+
+	// strip escaped quotes from JMESPath variables with dashes e.g. {{ \"my-map.data\".key }}
+	query = strings.Replace(query, "\"", "", -1)
+	if ctx.re != nil && ctx.re.MatchString(query) {
+		return emptyResult, nil
+	}
+
+	if ctx.isVariableDefined(query) {
+		return emptyResult, nil
+	}
+
+	return emptyResult, InvalidVariableErr{
+		variable:        query,
+		re:              ctx.re,
+		allowedPatterns: ctx.allowedPatterns,
+	}
+}
+
+func (ctx *MockContext) isVariableDefined(variable string) bool {
+	for _, pattern := range ctx.getVariables() {
+		if wildcard.Match(pattern, variable) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (ctx *MockContext) getVariables() []string {
+	ctx.mutex.RLock()
+	defer ctx.mutex.RUnlock()
+
+	vars := ctx.allowedPatterns
+	return vars
+}
+
+// InvalidVariableErr represents error for non-white-listed variables
+type InvalidVariableErr struct {
+	variable        string
+	re              *regexp.Regexp
+	allowedPatterns []string
+}
+
+func (i InvalidVariableErr) Error() string {
+	if i.re == nil {
+		return fmt.Sprintf("variable %s must match patterns %v", i.variable, i.allowedPatterns)
+	}
+
+	return fmt.Sprintf("variable %s must match regex \"%s\" or patterns %v", i.variable, i.re.String(), i.allowedPatterns)
+}
+
+func (ctx *MockContext) HasChanged(_ string) (bool, error) {
+	return false, nil
+}

pkg/engine/variables/vars.go

@@ -212,49 +212,10 @@ func substituteReferences(log logr.Logger, rule interface{}) (interface{}, error
 	return jsonUtils.NewTraversal(rule, substituteReferencesIfAny(log)).TraverseJSON()
 }
 
-// ValidateBackgroundModeVars validates variables against the specified context,
-// which contains a list of allowed JMESPath queries in background processing,
-// and throws an error if the variable is not allowed.
-func ValidateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface, rule interface{}) (interface{}, error) {
-	return jsonUtils.NewTraversal(rule, validateBackgroundModeVars(log, ctx)).TraverseJSON()
-}
-
 func ValidateElementInForEach(log logr.Logger, rule interface{}) (interface{}, error) {
 	return jsonUtils.NewTraversal(rule, validateElementInForEach(log)).TraverseJSON()
 }
 
-func validateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface) jsonUtils.Action {
-	return jsonUtils.OnlyForLeafsAndKeys(func(data *jsonUtils.ActionData) (interface{}, error) {
-		value, ok := data.Element.(string)
-		if !ok {
-			return data.Element, nil
-		}
-		vars := RegexVariables.FindAllString(value, -1)
-		for _, v := range vars {
-			initial := len(regexVariableInit.FindAllString(v, -1)) > 0
-
-			if !initial {
-				v = v[1:]
-			}
-
-			variable := replaceBracesAndTrimSpaces(v)
-
-			_, err := ctx.Query(variable)
-			if err != nil {
-				switch err.(type) {
-				case gojmespath.NotFoundError:
-					return nil, nil
-				case context.InvalidVariableErr:
-					return nil, err
-				default:
-					return nil, fmt.Errorf("failed to resolve %v at path %s: %v", variable, data.Path, err)
-				}
-			}
-		}
-		return nil, nil
-	})
-}
-
 func validateElementInForEach(log logr.Logger) jsonUtils.Action {
 	return jsonUtils.OnlyForLeafsAndKeys(func(data *jsonUtils.ActionData) (interface{}, error) {
 		value, ok := data.Element.(string)
@@ -379,7 +340,15 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface, vr Var
 				variable := replaceBracesAndTrimSpaces(v)
 
 				if variable == "@" {
-					variable = strings.Replace(variable, "@", fmt.Sprintf("request.object.%s", getJMESPath(data.Path)), -1)
+					path := getJMESPath(data.Path)
+					var val string
+					if strings.HasPrefix(path, "[") {
+						val = fmt.Sprintf("request.object%s", path)
+					} else {
+						val = fmt.Sprintf("request.object.%s", path)
+					}
+
+					variable = strings.Replace(variable, "@", val, -1)
 				}
 
 				if isDeleteRequest {
@@ -441,12 +410,15 @@ func isDeleteRequest(ctx context.EvalInterface) bool {
 	return false
 }
 
-// getJMESPath converts path to JMES format
+var regexPathDigit = regexp.MustCompile(`\.?([\d])\.?`)
+
+// getJMESPath converts path to JMESPath format
 func getJMESPath(rawPath string) string {
-	tokens := strings.Split(rawPath, "/")[3:] // skip empty element and two non-resource (like mutate.overlay)
+	tokens := strings.Split(rawPath, "/")[3:] // skip "/" + 2 elements (e.g. mutate.overlay | validate.pattern)
 	path := strings.Join(tokens, ".")
-	regex := regexp.MustCompile(`\.([\d])\.`)
-	return string(regex.ReplaceAll([]byte(path), []byte("[$1].")))
+	b := regexPathDigit.ReplaceAll([]byte(path), []byte("[$1]."))
+	result := strings.Trim(string(b), ".")
+	return result
 }
 
 func substituteVarInPattern(prefix, pattern, variable string, value interface{}) (string, error) {

pkg/engine/variables/vars_test.go

@@ -526,7 +526,7 @@ func Test_policyContextValidation(t *testing.T) {
 	err := json.Unmarshal(policyContext, &contextMap)
 	assert.NilError(t, err)
 
-	ctx := context.NewContext("request.object")
+	ctx := context.NewMockContext(nil, "request.object")
 
 	_, err = SubstituteAll(log.Log, ctx, contextMap)
 	assert.Assert(t, err != nil, err)
@@ -603,7 +603,7 @@ func Test_variableSubstitution_array(t *testing.T) {
 	err := json.Unmarshal(ruleRaw, &rule)
 	assert.NilError(t, err)
 
-	ctx := context.NewContext("request.object", "animals")
+	ctx := context.NewContext()
 	ctx.AddJSON(configmapRaw)
 	ctx.AddResource(resourceRaw)
 
@@ -982,7 +982,7 @@ func TestFormAbsolutePath_RelativePathExists(t *testing.T) {
 	assert.Assert(t, result == expectedString)
 }
 
-func TestFormAbsolutePath_RelativePathWithBackToTopInTheBegining(t *testing.T) {
+func TestFormAbsolutePath_RelativePathWithBackToTopInTheBeginning(t *testing.T) {
 	absolutePath := "/spec/containers/0/resources/requests/memory"
 	referencePath := "../../limits/memory"
 	expectedString := "/spec/containers/0/resources/limits/memory"
@@ -1151,3 +1151,9 @@ func Test_ReplacingEscpNestedVariableWhenDeleting(t *testing.T) {
 
 	assert.Equal(t, fmt.Sprintf("%v", pattern), "{{request.object.metadata.annotations.target}}")
 }
+
+func Test_getJMESPath(t *testing.T) {
+	assert.Equal(t, "spec.containers[0]", getJMESPath("/validate/pattern/spec/containers/0"))
+	assert.Equal(t, "spec.containers[0].volumes[1]", getJMESPath("/validate/pattern/spec/containers/0/volumes/1"))
+	assert.Equal(t, "[0]", getJMESPath("/mutate/overlay/0"))
+}

pkg/kyverno/apply/apply_command.go

@@ -280,7 +280,7 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool,
 			continue
 		}
 
-		matches := common.PolicyHasVariables(*policy)
+		matches := common.HasVariables(policy)
 		variable := common.RemoveDuplicateAndObjectVariables(matches)
 		if len(variable) > 0 {
 			if len(variables) == 0 {