744 fixing policy validation

pkg/engine/variables/vars.go

@@ -17,16 +17,14 @@ const (
 //SubstituteVars replaces the variables with the values defined in the context
 // - if any variable is invaid or has nil value, it is considered as a failed varable substitution
 func SubstituteVars(log logr.Logger, ctx context.EvalInterface, pattern interface{}) (interface{}, error) {
-	errs := []error{}
-	pattern = subVars(log, ctx, pattern, "", &errs)
-	if len(errs) == 0 {
-		// no error while parsing the pattern
-		return pattern, nil
+	pattern, err := subVars(log, ctx, pattern, "")
+	if err != nil {
+		return pattern, err
 	}
-	return pattern, fmt.Errorf("%v", errs)
+	return pattern, nil
 }
 
-func subVars(log logr.Logger, ctx context.EvalInterface, pattern interface{}, path string, errs *[]error) interface{} {
+func subVars(log logr.Logger, ctx context.EvalInterface, pattern interface{}, path string) (interface{}, error) {
 	switch typedPattern := pattern.(type) {
 	case map[string]interface{}:
 		mapCopy := make(map[string]interface{})
@@ -34,48 +32,47 @@ func subVars(log logr.Logger, ctx context.EvalInterface, pattern interface{}, pa
 			mapCopy[k] = v
 		}
 
-		return subMap(log, ctx, mapCopy, path, errs)
+		return subMap(log, ctx, mapCopy, path)
 	case []interface{}:
 		sliceCopy := make([]interface{}, len(typedPattern))
 		copy(sliceCopy, typedPattern)
 
-		return subArray(log, ctx, sliceCopy, path, errs)
+		return subArray(log, ctx, sliceCopy, path)
 	case string:
-		return subValR(ctx, typedPattern, path, errs)
+		return subValR(ctx, typedPattern, path)
 	default:
-		return pattern
+		return pattern, nil
 	}
 }
 
-func subMap(log logr.Logger, ctx context.EvalInterface, patternMap map[string]interface{}, path string, errs *[]error) map[string]interface{} {
+func subMap(log logr.Logger, ctx context.EvalInterface, patternMap map[string]interface{}, path string) (map[string]interface{}, error) {
 	for key, patternElement := range patternMap {
 		curPath := path + "/" + key
-		value := subVars(log, ctx, patternElement, curPath, errs)
+		value, err := subVars(log, ctx, patternElement, curPath)
+		if err != nil {
+			return nil, err
+		}
 		patternMap[key] = value
 
 	}
-	return patternMap
+	return patternMap, nil
 }
 
-func subArray(log logr.Logger, ctx context.EvalInterface, patternList []interface{}, path string, errs *[]error) []interface{} {
+func subArray(log logr.Logger, ctx context.EvalInterface, patternList []interface{}, path string) ([]interface{}, error) {
 	for idx, patternElement := range patternList {
 		curPath := path + "/" + strconv.Itoa(idx)
-		value := subVars(log, ctx, patternElement, curPath, errs)
+		value, err := subVars(log, ctx, patternElement, curPath)
+		if err != nil {
+			return nil, err
+		}
 		patternList[idx] = value
 	}
-	return patternList
+	return patternList, nil
 }
 
 // subValR resolves the variables if defined
-func subValR(ctx context.EvalInterface, valuePattern string, path string, errs *[]error) interface{} {
+func subValR(ctx context.EvalInterface, valuePattern string, path string) (interface{}, error) {
 	originalPattern := valuePattern
-	var failedVars []string
-
-	defer func() {
-		if len(failedVars) > 0 {
-			*errs = append(*errs, fmt.Errorf("failed to resolve %v at path %s", failedVars, path))
-		}
-	}()
 
 	regex := regexp.MustCompile(`\{\{([^{}]*)\}\}`)
 	for {
@@ -84,23 +81,18 @@ func subValR(ctx context.EvalInterface, valuePattern string, path string, errs *
 				underlyingVariable := strings.ReplaceAll(strings.ReplaceAll(variable, "}}", ""), "{{", "")
 				substitutedVar, err := ctx.Query(underlyingVariable)
 				if err != nil {
-					failedVars = append(failedVars, underlyingVariable)
-					return nil
+					return nil, fmt.Errorf("failed to resolve %v at path %s", underlyingVariable, path)
 				}
 				if val, ok := substitutedVar.(string); ok {
-					if val == "" {
-						failedVars = append(failedVars, underlyingVariable)
-						return nil
-					}
 					valuePattern = strings.Replace(valuePattern, variable, val, -1)
 				} else {
 					if substitutedVar != nil {
 						if originalPattern == variable {
-							return substitutedVar
+							return substitutedVar, nil
 						}
+						return nil, fmt.Errorf("failed to resolve %v at path %s", underlyingVariable, path)
 					}
-					failedVars = append(failedVars, underlyingVariable)
-					return nil
+					return nil, fmt.Errorf("could not find variable %v at path %v", underlyingVariable, path)
 				}
 			}
 		} else {
@@ -108,5 +100,5 @@ func subValR(ctx context.EvalInterface, valuePattern string, path string, errs *
 		}
 	}
 
-	return valuePattern
+	return valuePattern, nil
 }

pkg/engine/variables/vars_test.go

@@ -152,6 +152,5 @@ func Test_SubvarRecursive(t *testing.T) {
 
 	ctx := context.NewContext()
 	assert.Assert(t, ctx.AddResource(resourceRaw))
-	errs := []error{}
-	subValR(ctx, string(patternRaw), "/", &errs)
+	subValR(ctx, string(patternRaw), "/")
 }

pkg/policy/background.go

@@ -2,6 +2,7 @@ package policy
 
 import (
 	"fmt"
+	"strings"
 
 	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
 	"github.com/nirmata/kyverno/pkg/engine/context"
@@ -24,35 +25,40 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error {
 		filterVars := []string{"request.object"}
 		ctx := context.NewContext(filterVars...)
 		for condIdx, condition := range rule.Conditions {
-			if condition.Key, err = variables.SubstituteVars(log.Log, ctx, condition.Key); err != nil {
+			if condition.Key, err = variables.SubstituteVars(log.Log, ctx, condition.Key); !checkNotFoundErr(err) {
 				return fmt.Errorf("invalid variable used at spec/rules[%d]/condition[%d]/key", idx, condIdx)
 			}
 
-			if condition.Value, err = variables.SubstituteVars(log.Log, ctx, condition.Value); err != nil {
+			if condition.Value, err = variables.SubstituteVars(log.Log, ctx, condition.Value); !checkNotFoundErr(err) {
 				return fmt.Errorf("invalid variable used at spec/rules[%d]/condition[%d]/value", idx, condIdx)
 			}
 		}
 
-		if rule.Mutation.Overlay, err = variables.SubstituteVars(log.Log, ctx, rule.Mutation.Overlay); err != nil {
-			return fmt.Errorf("invalid variable used at spec/rules[%d]/mutate/overlay", idx)
+		if rule.Mutation.Overlay != nil {
+			if rule.Mutation.Overlay, err = variables.SubstituteVars(log.Log, ctx, rule.Mutation.Overlay); !checkNotFoundErr(err) {
+				return fmt.Errorf("invalid variable used at spec/rules[%d]/mutate/overlay", idx)
+			}
 		}
-		if rule.Validation.Pattern, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Pattern); err != nil {
-			return fmt.Errorf("invalid variable used at spec/rules[%d]/validate/pattern", idx)
+
+		if rule.Validation.Pattern != nil {
+			if rule.Validation.Pattern, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Pattern); !checkNotFoundErr(err) {
+				return fmt.Errorf("invalid variable used at spec/rules[%d]/validate/pattern", idx)
+			}
 		}
 		for idx2, pattern := range rule.Validation.AnyPattern {
-			if rule.Validation.AnyPattern[idx2], err = variables.SubstituteVars(log.Log, ctx, pattern); err != nil {
+			if rule.Validation.AnyPattern[idx2], err = variables.SubstituteVars(log.Log, ctx, pattern); !checkNotFoundErr(err) {
 				return fmt.Errorf("invalid variable used at spec/rules[%d]/validate/anyPattern[%d]", idx, idx2)
 			}
 		}
-		if _, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Message); err != nil {
+		if _, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Message); !checkNotFoundErr(err) {
 			return fmt.Errorf("invalid variable used at spec/rules[%d]/validate/message", idx)
 		}
 		if rule.Validation.Deny != nil {
 			for i := range rule.Validation.Deny.Conditions {
-				if _, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Deny.Conditions[i].Key); err != nil {
+				if _, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Deny.Conditions[i].Key); !checkNotFoundErr(err) {
 					return fmt.Errorf("invalid variable used at spec/rules[%d]/validate/deny/conditions[%d]/key", idx, i)
 				}
-				if _, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Deny.Conditions[i].Value); err != nil {
+				if _, err = variables.SubstituteVars(log.Log, ctx, rule.Validation.Deny.Conditions[i].Value); !checkNotFoundErr(err) {
 					return fmt.Errorf("invalid variable used at spec/rules[%d]/validate/deny/conditions[%d]/value", idx, i)
 				}
 			}
@@ -61,6 +67,17 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error {
 	return nil
 }
 
+func checkNotFoundErr(err error) bool {
+	if err != nil {
+		if strings.HasPrefix(err.Error(), "could not find variable") {
+			return true
+		} else {
+			return false
+		}
+	}
+	return true
+}
+
 func userInfoDefined(ui kyverno.UserInfo) string {
 	if len(ui.Roles) > 0 {
 		return "roles"