fix: update match logic for old object validation (#11427)

* fix: update match logic for old object validation

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: linter

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: failing test due to user info

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: debug logs

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>

pkg/engine/handlers/validation/utils.go

@@ -3,45 +3,32 @@ package validation
 import (
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	kyvernov2 "github.com/kyverno/kyverno/api/kyverno/v2"
-	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
+	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
-	"github.com/kyverno/kyverno/pkg/utils/match"
+	authenticationv1 "k8s.io/api/authentication/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-func matchResource(resource unstructured.Unstructured, rule kyvernov1.Rule) bool {
+func matchResource(resource unstructured.Unstructured, rule kyvernov1.Rule, namespaceLabels map[string]string, policyNamespace string, operation kyvernov1.AdmissionOperation) bool {
-	if rule.MatchResources.All != nil || rule.MatchResources.Any != nil {
+	// cannot use admission info from the current request as the user can be different, if the rule matches on old request user info, it should skip
-		matched := match.CheckMatchesResources(
+	admissionInfo := kyvernov2.RequestInfo{
-			resource,
+		Roles:        []string{"kyverno:invalidrole"},
-			kyvernov2beta1.MatchResources{
+		ClusterRoles: []string{"kyverno:invalidrole"},
-				Any: rule.MatchResources.Any,
+		AdmissionUserInfo: authenticationv1.UserInfo{
-				All: rule.MatchResources.All,
+			Username: "kyverno:kyverno-invalid-controller",
-			},
+			UID:      "kyverno:invaliduid",
-			make(map[string]string),
+			Groups:   []string{"kyverno:invalidgroup"},
-			kyvernov2.RequestInfo{},
+		},
-			resource.GroupVersionKind(),
-			"",
-		)
-		if matched != nil {
-			return false
-		}
 	}
-	if rule.ExcludeResources != nil {
+
-		if rule.ExcludeResources.All != nil || rule.ExcludeResources.Any != nil {
+	err := engineutils.MatchesResourceDescription(
-			excluded := match.CheckMatchesResources(
+		resource,
-				resource,
+		rule,
-				kyvernov2beta1.MatchResources{
+		admissionInfo,
-					Any: rule.ExcludeResources.Any,
+		namespaceLabels,
-					All: rule.ExcludeResources.All,
+		policyNamespace,
-				},
+		resource.GroupVersionKind(),
-				make(map[string]string),
+		"",
-				kyvernov2.RequestInfo{},
+		operation,
-				resource.GroupVersionKind(),
+	)
-				"",
+	return err == nil
-			)
-			if excluded == nil {
-				return false
-			}
-		}
-	}
-	return true
 }

pkg/engine/handlers/validation/validate_assert.go

@@ -156,7 +156,7 @@ func validateOldObject(ctx context.Context, policyContext engineapi.PolicyContex
 
 	oldResource := policyContext.OldResource()
 
-	if ok := matchResource(oldResource, rule); !ok {
+	if ok := matchResource(oldResource, rule, policyContext.NamespaceLabels(), policyContext.Policy().GetNamespace(), kyvernov1.Create); !ok {
 		return nil, nil
 	}
 

pkg/engine/handlers/validation/validate_pss.go

@@ -181,7 +181,7 @@ func (h validatePssHandler) validateOldObject(
 	oldResource := policyContext.OldResource()
 	emptyResource := unstructured.Unstructured{}
 
-	if ok := matchResource(oldResource, rule); !ok {
+	if ok := matchResource(oldResource, rule, policyContext.NamespaceLabels(), policyContext.Policy().GetNamespace(), kyvernov1.Create); !ok {
 		return nil, nil
 	}
 	if err := policyContext.SetResources(emptyResource, oldResource); err != nil {

pkg/engine/handlers/validation/validate_resource.go

@@ -163,11 +163,9 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
 				return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "failed to validate old object", ruleResponse.Properties())
 			}
 
-			if engineutils.IsSameRuleResponse(ruleResponse, priorResp) {
+			// when an existing resource violates, and the updated resource also violates, then skip
-				v.log.V(2).Info("warning: skipping the rule evaluation as pre-existing violations are allowed")
+			if ruleResponse.Status() == engineapi.RuleStatusFail && priorResp.Status() == engineapi.RuleStatusFail { //
-				if ruleResponse.Status() == engineapi.RuleStatusPass {
+				v.log.V(2).Info("warning: skipping the rule evaluation as pre-existing violations are allowed", "ruleResponse", ruleResponse, "priorResp", priorResp)
-					return ruleResponse
-				}
 				return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "skipping the rule evaluation as pre-existing violations are allowed", v.rule.ReportProperties)
 			}
 		}
@@ -185,7 +183,7 @@ func (v *validator) validateOldObject(ctx context.Context) (*engineapi.RuleRespo
 	oldResource := v.policyContext.OldResource()
 	emptyResource := unstructured.Unstructured{}
 
-	if ok := matchResource(oldResource, v.rule); !ok {
+	if ok := matchResource(oldResource, v.rule, v.policyContext.NamespaceLabels(), v.policyContext.Policy().GetNamespace(), kyvernov1.Create); !ok {
 		return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "resource not matched", v.rule.ReportProperties), nil
 	}