fix: validate pattern premature skip (#9155)

Signed-off-by: Liang Deng <283304489@qq.com>
Co-authored-by: shuting <shuting@nirmata.com>

pkg/engine/anchor/error.go

@@ -83,7 +83,7 @@ func IsConditionalAnchorError(err error) bool {
 	return isError(err, conditionalAnchorErr, conditionalAnchorErrMsg)
 }
 
-// IsGlobalAnchorError checks if error is a global global anchor error
+// IsGlobalAnchorError checks if error is a global anchor error
 func IsGlobalAnchorError(err error) bool {
 	return isError(err, globalAnchorErr, globalAnchorErrMsg)
 }

pkg/engine/anchor/handlers.go

@@ -92,7 +92,7 @@ func newEqualityHandler(anchor Anchor, pattern interface{}, path string) Validat
 	}
 }
 
-// Handle processed condition anchor
+// Handle processed equality anchor
 func (eh equalityHandler) Handle(handler resourceElementHandler, resourceMap map[string]interface{}, originPattern interface{}, ac *AnchorMap) (string, error) {
 	anchorKey := eh.anchor.Key()
 	currentPath := eh.path + anchorKey + "/"

pkg/engine/validate/validate.go

@@ -129,6 +129,8 @@ func validateMap(log logr.Logger, resourceMap, patternMap map[string]interface{}
 	sort.Strings(keys)
 
 	// Evaluate anchors
+	var skipErrors []error
+	var applyCount int
 	for _, key := range keys {
 		patternElement := anchors[key]
 		// get handler for each pattern in the pattern
@@ -137,12 +139,24 @@ func validateMap(log logr.Logger, resourceMap, patternMap map[string]interface{}
 		// - Equality
 		handler := anchor.CreateElementHandler(key, patternElement, path)
 		handlerPath, err := handler.Handle(validateResourceElement, resourceMap, origPattern, ac)
-		// if there are resource values at same level, then anchor acts as conditional instead of a strict check
-		// but if there are none then it's an if-then check
 		if err != nil {
-			// If global anchor fails then we don't process the resource
+			if skip(err) {
+				skipErrors = append(skipErrors, err)
+				continue
+			}
+
 			return handlerPath, err
 		}
+
+		applyCount++
+	}
+
+	if applyCount == 0 && len(skipErrors) > 0 {
+		return path, &PatternError{
+			Err:  multierr.Combine(skipErrors...),
+			Path: path,
+			Skip: true,
+		}
 	}
 
 	// Evaluate resources

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-fail/README.md

@@ -0,0 +1,11 @@
+## Description
+
+This test ensures that when the policy uses a pattern with conditional anchor, the creation of some resources should be failed instead of skipped.
+
+## Expected Behavior
+
+Resource failed to be created due to validate failure.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/8731

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-fail/chainsaw-test.yaml

@@ -0,0 +1,24 @@
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  creationTimestamp: null
+  name: validate-pattern-should-fail
+spec:
+  steps:
+    - name: step-01
+      try:
+        - apply:
+            file: policy.yaml
+        - assert:
+            file: policy-assert.yaml
+    - name: step-02
+      try:
+        - apply:
+            expect:
+              - check:
+                  ($error != null): true
+            file: resource.yaml
+    - name: step-03
+      try:
+        - assert:
+            file: event-assert.yaml

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-fail/event-assert.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Event
+metadata:
+  namespace: default
+involvedObject:
+  apiVersion: kyverno.io/v1
+  kind: ClusterPolicy
+  name: priv
+reason: PolicyViolation
+reportingComponent: kyverno-admission

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-fail/policy-assert.yaml

@@ -0,0 +1,10 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: priv
+spec: {}
+status:
+  conditions:
+    - reason: Succeeded
+      status: "True"
+      type: Ready

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-fail/policy.yaml

@@ -0,0 +1,31 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: priv
+spec:
+  validationFailureAction: Enforce
+  background: true
+  rules:
+    - name: priv-esc
+      match:
+        any:
+          - resources:
+              kinds:
+                - Pod
+      validate:
+        message: >-
+          Lorem ipse
+        pattern:
+          spec:
+            =(ephemeralContainers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"
+            =(initContainers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"
+            =(containers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-fail/resource.yaml

@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-pod
+  labels:
+    app: test-app
+spec:
+  containers:
+    - name: side
+      image: test/foo:1.2.3
+  initContainers:
+    - name: init
+      image: test/bar:1.2.3
+      securityContext:
+        allowPrivilegeEscalation: true

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/README.md

@@ -0,0 +1,11 @@
+## Description
+
+This test ensures that when the policy uses a pattern with conditional anchor, the creation of some resources should be passes instead of skipped.
+
+## Expected Behavior
+
+The creation of resource should be passes instead of skipped.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/8731

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/chainsaw-test.yaml

@@ -0,0 +1,25 @@
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  creationTimestamp: null
+  name: validate-pattern-should-pass
+spec:
+  steps:
+    - name: step-01
+      try:
+        - apply:
+            file: policy.yaml
+        - assert:
+            file: policy-assert.yaml
+    - name: step-02
+      try:
+        - apply:
+            file: resource.yaml
+    - name: step-03
+      try:
+        - assert:
+            file: event-assert.yaml
+    - name: step-04
+      try:
+        - assert:
+            file: report-pass-assert.yaml

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/event-assert.yaml

@@ -0,0 +1,12 @@
+apiVersion: v1
+kind: Event
+metadata:
+  namespace: default
+involvedObject:
+  apiVersion: kyverno.io/v1
+  kind: ClusterPolicy
+  name: priv
+type: Normal
+reason: PolicyApplied
+action: Resource Passed
+reportingComponent: kyverno-admission

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/policy-assert.yaml

@@ -0,0 +1,10 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: priv
+spec: {}
+status:
+  conditions:
+    - reason: Succeeded
+      status: "True"
+      type: Ready

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/policy.yaml

@@ -0,0 +1,31 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: priv
+spec:
+  validationFailureAction: Enforce
+  background: true
+  rules:
+    - name: priv-esc
+      match:
+        any:
+          - resources:
+              kinds:
+                - Pod
+      validate:
+        message: >-
+          Lorem ipse
+        pattern:
+          spec:
+            =(ephemeralContainers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"
+            =(initContainers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"
+            =(containers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/report-pass-assert.yaml

@@ -0,0 +1,23 @@
+apiVersion: wgpolicyk8s.io/v1alpha2
+kind: PolicyReport
+metadata:
+  ownerReferences:
+  - apiVersion: v1
+    kind: Pod
+    name: test-pod
+scope:
+  apiVersion: v1
+  kind: Pod
+  name: test-pod
+results:
+- message: validation rule 'priv-esc' passed.
+  policy: priv
+  result: pass
+  rule: priv-esc
+  source: kyverno
+summary:
+  error: 0
+  fail: 0
+  pass: 1
+  skip: 0
+  warn: 0

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-pass/resource.yaml

@@ -0,0 +1,19 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-pod
+  labels:
+    app: test-app
+spec:
+  containers:
+    - name: main
+      image: test/bar:1.2.3
+      securityContext:
+        allowPrivilegeEscalation: false
+    - name: side
+      image: test/foo:1.2.3
+  initContainers:
+    - name: init
+      image: test/foo:1.2.3
+      securityContext:
+        allowPrivilegeEscalation: true

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-skip/README.md

@@ -0,0 +1,11 @@
+## Description
+
+This test ensures that when the policy uses a pattern with conditional anchor, the creation of some resources should be skipped.
+
+## Expected Behavior
+
+The creation of resource should be skipped.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/8731

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-skip/chainsaw-test.yaml

@@ -0,0 +1,21 @@
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  creationTimestamp: null
+  name: validate-pattern-should-skip
+spec:
+  steps:
+    - name: step-01
+      try:
+        - apply:
+            file: policy.yaml
+        - assert:
+            file: policy-assert.yaml
+    - name: step-02
+      try:
+        - apply:
+            file: resource.yaml
+    - name: step-03
+      try:
+        - assert:
+            file: report-skip-assert.yaml

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-skip/policy-assert.yaml

@@ -0,0 +1,10 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: priv
+spec: {}
+status:
+  conditions:
+    - reason: Succeeded
+      status: "True"
+      type: Ready

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-skip/policy.yaml

@@ -0,0 +1,27 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: priv
+spec:
+  validationFailureAction: Enforce
+  background: true
+  rules:
+    - name: priv-esc
+      match:
+        any:
+          - resources:
+              kinds:
+                - Pod
+      validate:
+        message: >-
+          Lorem ipse
+        pattern:
+          spec:
+            =(initContainers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"
+            =(containers):
+              - (image): "!*/foo:*.*.*"
+                securityContext:
+                  allowPrivilegeEscalation: "false"

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-skip/report-skip-assert.yaml

@@ -0,0 +1,23 @@
+apiVersion: wgpolicyk8s.io/v1alpha2
+kind: PolicyReport
+metadata:
+  ownerReferences:
+  - apiVersion: v1
+    kind: Pod
+    name: test-pod
+scope:
+  apiVersion: v1
+  kind: Pod
+  name: test-pod
+results:
+- message: 'conditional anchor mismatch: resource value ''test/foo:1.2.3'' does not match ''!*/foo:*.*.*'' at path /spec/containers/0/image/; conditional anchor mismatch: resource value ''test/foo:1.2.3'' does not match ''!*/foo:*.*.*'' at path /spec/containers/1/image/; conditional anchor mismatch: resource value ''test/foo:1.2.3'' does not match ''!*/foo:*.*.*'' at path /spec/initContainers/0/image/'
+  policy: priv
+  result: skip
+  rule: priv-esc
+  source: kyverno
+summary:
+  error: 0
+  fail: 0
+  pass: 0
+  skip: 1
+  warn: 0

test/conformance/chainsaw/validate/clusterpolicy/cornercases/validate-pattern-should-skip/resource.yaml

@@ -0,0 +1,19 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-pod
+  labels:
+    app: test-app
+spec:
+  containers:
+    - name: main
+      image: test/foo:1.2.3
+      securityContext:
+        allowPrivilegeEscalation: false
+    - name: side
+      image: test/foo:1.2.3
+  initContainers:
+    - name: init
+      image: test/foo:1.2.3
+      securityContext:
+        allowPrivilegeEscalation: true