1314 validate rule (#1368)

* fixes 1314

* fix panic

pkg/engine/common/anchorKey.go

@@ -1,16 +1,68 @@
 package common
 
 import (
+	"errors"
+	"fmt"
+	"strings"
+
 	"github.com/kyverno/kyverno/pkg/engine/anchor/common"
 )
 
+// IsConditionalAnchorError checks if error message has conditional anchor error string
+func IsConditionalAnchorError(msg string) bool {
+	if strings.Contains(msg, ConditionalAnchorErrMsg) {
+		return true
+	}
+	return false
+}
+
+// NewConditionalAnchorError returns a new instance of ConditionalAnchorError
+func NewConditionalAnchorError(msg string) ValidateAnchorError {
+	return ValidateAnchorError{
+		Err:     ConditionalAnchorErr,
+		Message: fmt.Sprintf("%s: %s", ConditionalAnchorErrMsg, msg),
+	}
+}
+
+// IsConditionAnchorError ...
+func (e ValidateAnchorError) IsConditionAnchorError() bool {
+	if e.Err == ConditionalAnchorErr {
+		return true
+	}
+	return false
+}
+
+// IsNil ...
+func (e ValidateAnchorError) IsNil() bool {
+	return e == ValidateAnchorError{}
+}
+
+func (e ValidateAnchorError) Error() error {
+	return errors.New(e.Message)
+}
+
+// AnchorError is the const specification of anchor errors
+type AnchorError int
+
+// ConditionalAnchorErr ...
+const ConditionalAnchorErr AnchorError = iota
+
+// ValidateAnchorError represents the error type of validation anchors
+type ValidateAnchorError struct {
+	Err     AnchorError
+	Message string
+}
+
+// ConditionalAnchorErrMsg - the error message for conditional anchor error
+var ConditionalAnchorErrMsg = "conditionalAnchorError"
+
 // AnchorKey - contains map of anchors
 type AnchorKey struct {
 	// anchorMap - for each anchor key in the patterns it will maintains information if the key exists in the resource
 	// if anchor key of the pattern exists in the resource then (key)=true else (key)=false
 	anchorMap map[string]bool
 	// AnchorError - used in validate to break execution of the recursion when if condition fails
-	AnchorError error
+	AnchorError ValidateAnchorError
 }
 
 // NewAnchorMap -initialize anchorMap

pkg/engine/generation.go

@@ -15,7 +15,7 @@ import (
 )
 
 // Generate checks for validity of generate rule on the resource
-// 1. validate variables to be susbtitute in the general ruleInfo (match,exclude,condition)
+// 1. validate variables to be substitute in the general ruleInfo (match,exclude,condition)
 //    - the caller has to check the ruleResponse to determine whether the path exist
 // 2. returns the list of rules that are applicable on this policy and resource, if 1 succeed
 func Generate(policyContext PolicyContext) (resp response.EngineResponse) {

pkg/engine/validate/utils.go

@@ -37,8 +37,9 @@ func getSortedNestedAnchorResource(resources map[string]interface{}) *list.List
 	for k, v := range resources {
 		if hasNestedAnchors(v) {
 			sortedResourceKeys.PushFront(k)
+		} else {
+			sortedResourceKeys.PushBack(k)
 		}
-		sortedResourceKeys.PushBack(k)
 	}
 	return sortedResourceKeys
 }

pkg/engine/validate/validate.go

@@ -3,7 +3,6 @@ package validate
 import (
 	"errors"
 	"fmt"
-	"github.com/kyverno/kyverno/pkg/engine/wildcards"
 	"path"
 	"reflect"
 	"strconv"
@@ -14,6 +13,7 @@ import (
 	commonAnchors "github.com/kyverno/kyverno/pkg/engine/anchor/common"
 	"github.com/kyverno/kyverno/pkg/engine/common"
 	"github.com/kyverno/kyverno/pkg/engine/operator"
+	"github.com/kyverno/kyverno/pkg/engine/wildcards"
 )
 
 // ValidateResourceWithPattern is a start of element-by-element validation process
@@ -23,6 +23,10 @@ func ValidateResourceWithPattern(log logr.Logger, resource, pattern interface{})
 	ac := common.NewAnchorMap()
 	elemPath, err := validateResourceElement(log, resource, pattern, pattern, "/", ac)
 	if err != nil {
+		if common.IsConditionalAnchorError(err.Error()) {
+			return "", nil
+		}
+
 		if !ac.IsAnchorError() {
 			return elemPath, err
 		}
@@ -102,19 +106,14 @@ func validateMap(log logr.Logger, resourceMap, patternMap map[string]interface{}
 		if err != nil {
 			// If Conditional anchor fails then we don't process the resources
 			if commonAnchors.IsConditionAnchor(key) {
-				ac.AnchorError = err
-				log.Error(err, "condition anchor did not satisfy, wont process the resource")
-				return "", nil
+				ac.AnchorError = common.NewConditionalAnchorError(fmt.Sprintf("condition anchor did not satisfy: %s", err.Error()))
+				log.V(3).Info(ac.AnchorError.Message)
+				return "", ac.AnchorError.Error()
 			}
 			return handlerPath, err
 		}
 	}
 
-	// If anchor fails then succeed validate and skip further validation of recursion
-	if ac.AnchorError != nil {
-		return "", nil
-	}
-
 	// Evaluate resources
 	// getSortedNestedAnchorResource - keeps the anchor key to start of the list
 	sortedResourceKeys := getSortedNestedAnchorResource(resources)
@@ -150,6 +149,9 @@ func validateArray(log logr.Logger, resourceArray, patternArray []interface{}, o
 				currentPath := path + strconv.Itoa(i) + "/"
 				elemPath, err := validateResourceElement(log, resourceArray[i], patternElement, originPattern, currentPath, ac)
 				if err != nil {
+					if common.IsConditionalAnchorError(err.Error()) {
+						continue
+					}
 					return elemPath, err
 				}
 			}
@@ -283,6 +285,9 @@ func validateArrayOfMaps(log logr.Logger, resourceMapArray []interface{}, patter
 		currentPath := path + strconv.Itoa(i) + "/"
 		returnpath, err := validateResourceElement(log, resourceElement, patternMap, originPattern, currentPath, ac)
 		if err != nil {
+			if common.IsConditionalAnchorError(err.Error()) {
+				continue
+			}
 			return returnpath, err
 		}
 	}

pkg/engine/validate/validate_test.go

@@ -2,6 +2,7 @@ package validate
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/kyverno/kyverno/pkg/engine/common"
@@ -1418,3 +1419,162 @@ func testValidationPattern(t *testing.T, num string, patternBytes []byte, resour
 		assert.Assert(t, err != nil, num)
 	}
 }
+
+func TestConditionalAnchorWithMultiplePatterns(t *testing.T) {
+	testCases := []struct {
+		name     string
+		pattern  []byte
+		resource []byte
+		nilErr   bool
+	}{
+		{
+			name:     "test-1",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-2",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-x",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "!*:* | *:latest","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-3",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-4",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx", "imagePullPolicy": "Never"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-5",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Never"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-6",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Never"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-7",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx", "imagePullPolicy": "Always"},{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-8",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Always"},{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-9",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Always"},{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-10",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx", "imagePullPolicy": "Never"},{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-11",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Never"},{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-12",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Never"},{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-13",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-14",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-15",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-16",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx", "imagePullPolicy": "Never"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-17",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Never"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-18",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.28", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Never"}]}}`),
+			nilErr:   true,
+		},
+		{
+			name:     "test-19",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-20",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:latest", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx:latest", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   false,
+		},
+		{
+			name:     "test-21",
+			pattern:  []byte(`{"spec": {"containers": [{"name": "*","(image)": "*:latest | !*:*","imagePullPolicy": "!Always"}]}}`),
+			resource: []byte(`{"spec": {"containers": [{"name": "busybox","image": "busybox:1.2.3", "imagePullPolicy": "Always"},{"name": "nginx","image": "nginx:1.2.3", "imagePullPolicy": "Always"}]}}`),
+			nilErr:   true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		var pattern, resource interface{}
+		err := json.Unmarshal(testCase.pattern, &pattern)
+		assert.NilError(t, err)
+		err = json.Unmarshal(testCase.resource, &resource)
+		assert.NilError(t, err)
+
+		_, err = ValidateResourceWithPattern(log.Log, resource, pattern)
+		if testCase.nilErr {
+			assert.NilError(t, err, fmt.Sprintf("\ntest: %s\npattern: %s\nresource: %s\n", testCase.name, pattern, resource))
+		} else {
+			assert.Assert(t,
+				err != nil,
+				fmt.Sprintf("\ntest: %s\npattern: %s\nresource: %s\nmsg: %v", testCase.name, pattern, resource, err))
+		}
+	}
+}

pkg/engine/validation.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
+	"github.com/kyverno/kyverno/pkg/engine/common"
 	"github.com/kyverno/kyverno/pkg/engine/context"
 	"github.com/kyverno/kyverno/pkg/engine/response"
 	"github.com/kyverno/kyverno/pkg/engine/utils"
@@ -197,6 +198,10 @@ func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno
 
 		if rule.Validation.Pattern != nil || rule.Validation.AnyPattern != nil {
 			ruleResponse := validatePatterns(log, ctx, resource, rule)
+			if common.IsConditionalAnchorError(ruleResponse.Message) {
+				continue
+			}
+
 			incrementAppliedCount(resp)
 			resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse)
 		}

pkg/policy/validate_controller.go

@@ -395,7 +395,9 @@ func (pc *PolicyController) syncPolicy(key string) error {
 	} else {
 		var nspolicy *kyverno.Policy
 		nspolicy, err = pc.npLister.Policies(namespace).Get(key)
-		policy = ConvertPolicyToClusterPolicy(nspolicy)
+		if err == nil && nspolicy != nil {
+			policy = ConvertPolicyToClusterPolicy(nspolicy)
+		}
 	}
 
 	if err != nil {