refactor: mutate checks (#9255)

* refactor

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix

Signed-off-by: ShutingZhao <shuting@nirmata.com>

---------

Signed-off-by: ShutingZhao <shuting@nirmata.com>

api/kyverno/v1/policy_types.go

@@ -54,7 +54,7 @@ func (p *Policy) HasAutoGenAnnotation() bool {
 // HasMutateOrValidateOrGenerate checks for rule types
 func (p *Policy) HasMutateOrValidateOrGenerate() bool {
 	for _, rule := range p.Spec.Rules {
-		if rule.HasMutate() || rule.HasValidate() || rule.HasGenerate() {
+		if rule.HasMutateStandard() || rule.HasValidate() || rule.HasGenerate() {
 			return true
 		}
 	}

api/kyverno/v1/rule_types.go

@@ -112,6 +112,19 @@ func (r *Rule) HasMutate() bool {
 	return !datautils.DeepEqual(r.Mutation, Mutation{})
 }
 
+// HasMutateStandard checks for standard admission mutate rule
+func (r *Rule) HasMutateStandard() bool {
+	if r.HasMutateExisting() {
+		return false
+	}
+	return !datautils.DeepEqual(r.Mutation, Mutation{})
+}
+
+// HasMutateExisting checks if the mutate rule applies to existing resources
+func (r *Rule) HasMutateExisting() bool {
+	return r.Mutation.Targets != nil
+}
+
 // HasVerifyImages checks for verifyImages rule
 func (r *Rule) HasVerifyImages() bool {
 	for _, verifyImage := range r.VerifyImages {
@@ -157,11 +170,6 @@ func (r *Rule) HasGenerate() bool {
 	return !datautils.DeepEqual(r.Generation, Generation{})
 }
 
-// IsMutateExisting checks if the mutate rule applies to existing resources
-func (r *Rule) IsMutateExisting() bool {
-	return r.Mutation.Targets != nil
-}
-
 func (r *Rule) IsPodSecurity() bool {
 	return r.Validation.PodSecurity != nil
 }
@@ -369,7 +377,7 @@ func (r *Rule) ValidateMatchExcludeConflict(path *field.Path) (errs field.ErrorL
 
 // ValidateMutationRuleTargetNamespace checks if the targets are scoped to the policy's namespace
 func (r *Rule) ValidateMutationRuleTargetNamespace(path *field.Path, namespaced bool, policyNamespace string) (errs field.ErrorList) {
-	if r.HasMutate() && namespaced {
+	if r.HasMutateExisting() && namespaced {
 		for idx, target := range r.Mutation.Targets {
 			if target.Namespace != "" && target.Namespace != policyNamespace {
 				errs = append(errs, field.Invalid(path.Child("targets").Index(idx).Child("namespace"), target.Namespace, "This field can be ignored or should have value of the namespace where the policy is being created"))

api/kyverno/v1/spec_types.go

@@ -147,6 +147,26 @@ func (s *Spec) HasMutate() bool {
 	return false
 }
 
+// HasMutateStandard checks for standard admission mutate rule
+func (s *Spec) HasMutateStandard() bool {
+	for _, rule := range s.Rules {
+		if rule.HasMutateStandard() {
+			return true
+		}
+	}
+	return false
+}
+
+// HasMutateExisting checks for mutate existing rule types
+func (s *Spec) HasMutateExisting() bool {
+	for _, rule := range s.Rules {
+		if rule.HasMutateExisting() {
+			return true
+		}
+	}
+	return false
+}
+
 // HasValidate checks for validate rule types
 func (s *Spec) HasValidate() bool {
 	for _, rule := range s.Rules {
@@ -214,16 +234,6 @@ func (s *Spec) BackgroundProcessingEnabled() bool {
 	return *s.Background
 }
 
-// IsMutateExisting checks if the mutate policy applies to existing resources
-func (s *Spec) IsMutateExisting() bool {
-	for _, rule := range s.Rules {
-		if rule.IsMutateExisting() {
-			return true
-		}
-	}
-	return false
-}
-
 // GetMutateExistingOnPolicyUpdate return MutateExistingOnPolicyUpdate set value
 func (s *Spec) GetMutateExistingOnPolicyUpdate() bool {
 	return s.MutateExistingOnPolicyUpdate
@@ -286,7 +296,7 @@ func (s *Spec) validateDeprecatedFields(path *field.Path) (errs field.ErrorList)
 }
 
 func (s *Spec) validateMutateTargets(path *field.Path) (errs field.ErrorList) {
-	if s.MutateExistingOnPolicyUpdate {
+	if s.GetMutateExistingOnPolicyUpdate() {
 		for i, rule := range s.Rules {
 			if !rule.HasMutate() {
 				continue

api/kyverno/v2beta1/rule_types.go

@@ -79,6 +79,19 @@ func (r *Rule) HasMutate() bool {
 	return !datautils.DeepEqual(r.Mutation, kyvernov1.Mutation{})
 }
 
+// HasMutate checks for standard admission mutate rule
+func (r *Rule) HasMutateStandard() bool {
+	if r.HasMutateExisting() {
+		return false
+	}
+	return !datautils.DeepEqual(r.Mutation, kyvernov1.Mutation{})
+}
+
+// HasMutateExisting checks if the mutate rule applies to existing resources
+func (r *Rule) HasMutateExisting() bool {
+	return r.Mutation.Targets != nil
+}
+
 // HasVerifyImages checks for verifyImages rule
 func (r *Rule) HasVerifyImages() bool {
 	for _, verifyImage := range r.VerifyImages {
@@ -124,11 +137,6 @@ func (r *Rule) HasGenerate() bool {
 	return !datautils.DeepEqual(r.Generation, kyvernov1.Generation{})
 }
 
-// IsMutateExisting checks if the mutate rule applies to existing resources
-func (r *Rule) IsMutateExisting() bool {
-	return r.Mutation.Targets != nil
-}
-
 func (r *Rule) GetGenerateTypeAndSync() (_ kyvernov1.GenerateType, sync bool) {
 	if !r.HasGenerate() {
 		return

api/kyverno/v2beta1/spec_types.go

@@ -109,6 +109,26 @@ func (s *Spec) HasMutate() bool {
 	return false
 }
 
+// HasMutate checks for standard admission mutate rule
+func (s *Spec) HasMutateStandard() bool {
+	for _, rule := range s.Rules {
+		if rule.HasMutateStandard() {
+			return true
+		}
+	}
+	return false
+}
+
+// HasMutate checks for mutate existing rule types
+func (s *Spec) HasMutateExisting() bool {
+	for _, rule := range s.Rules {
+		if rule.HasMutateExisting() {
+			return true
+		}
+	}
+	return false
+}
+
 // HasValidate checks for validate rule types
 func (s *Spec) HasValidate() bool {
 	for _, rule := range s.Rules {
@@ -182,16 +202,6 @@ func (s *Spec) BackgroundProcessingEnabled() bool {
 	return *s.Background
 }
 
-// IsMutateExisting checks if the mutate policy applies to existing resources
-func (s *Spec) IsMutateExisting() bool {
-	for _, rule := range s.Rules {
-		if rule.IsMutateExisting() {
-			return true
-		}
-	}
-	return false
-}
-
 // GetMutateExistingOnPolicyUpdate return MutateExistingOnPolicyUpdate set value
 func (s *Spec) GetMutateExistingOnPolicyUpdate() bool {
 	return s.MutateExistingOnPolicyUpdate

cmd/cli/kubectl-kyverno/processor/policy_processor.go

@@ -100,7 +100,7 @@ func (p *PolicyProcessor) ApplyPoliciesOnResource() ([]engineapi.EngineResponse,
 	var responses []engineapi.EngineResponse
 	// mutate
 	for _, policy := range p.Policies {
-		if !policyHasMutate(policy) {
+		if !policy.GetSpec().HasMutate() {
 			continue
 		}
 		policyContext, err := p.makePolicyContext(jp, cfg, resource, policy, namespaceLabels, gvk, subresource)
@@ -117,7 +117,7 @@ func (p *PolicyProcessor) ApplyPoliciesOnResource() ([]engineapi.EngineResponse,
 	}
 	// verify images
 	for _, policy := range p.Policies {
-		if !policyHasVerifyImages(policy) {
+		if !policy.GetSpec().HasVerifyImages() {
 			continue
 		}
 		policyContext, err := p.makePolicyContext(jp, cfg, resource, policy, namespaceLabels, gvk, subresource)
@@ -172,7 +172,7 @@ func (p *PolicyProcessor) ApplyPoliciesOnResource() ([]engineapi.EngineResponse,
 	}
 	// generate
 	for _, policy := range p.Policies {
-		if policyHasGenerate(policy) {
+		if policy.GetSpec().HasGenerate() {
 			policyContext, err := p.makePolicyContext(jp, cfg, resource, policy, namespaceLabels, gvk, subresource)
 			if err != nil {
 				return responses, err

cmd/cli/kubectl-kyverno/processor/utils.go

@@ -4,24 +4,6 @@ import (
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 )
 
-func policyHasGenerate(policy kyvernov1.PolicyInterface) bool {
-	for _, rule := range policy.GetSpec().Rules {
-		if rule.HasGenerate() {
-			return true
-		}
-	}
-	return false
-}
-
-func policyHasMutate(policy kyvernov1.PolicyInterface) bool {
-	for _, rule := range policy.GetSpec().Rules {
-		if rule.HasMutate() {
-			return true
-		}
-	}
-	return false
-}
-
 func policyHasValidateOrVerifyImageChecks(policy kyvernov1.PolicyInterface) bool {
 	for _, rule := range policy.GetSpec().Rules {
 		//  engine.validate handles both validate and verifyImageChecks atm
@@ -31,12 +13,3 @@ func policyHasValidateOrVerifyImageChecks(policy kyvernov1.PolicyInterface) bool
 	}
 	return false
 }
-
-func policyHasVerifyImages(policy kyvernov1.PolicyInterface) bool {
-	for _, rule := range policy.GetSpec().Rules {
-		if rule.HasVerifyImages() {
-			return true
-		}
-	}
-	return false
-}

pkg/background/mutate/mutate.go

@@ -87,7 +87,7 @@ func (c *mutateExistingController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) e
 	}
 
 	for _, rule := range policy.GetSpec().Rules {
-		if !rule.IsMutateExisting() || ur.Spec.Rule != rule.Name {
+		if !rule.HasMutateExisting() || ur.Spec.Rule != rule.Name {
 			continue
 		}
 

pkg/controllers/webhook/controller.go

@@ -644,7 +644,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(ctx context.Conte
 		for _, p := range policies {
 			if p.AdmissionProcessingEnabled() {
 				spec := p.GetSpec()
-				if spec.HasMutate() || spec.HasVerifyImages() {
+				if spec.HasMutateStandard() || spec.HasVerifyImages() {
 					if spec.GetFailurePolicy(ctx) == kyvernov1.Ignore {
 						c.mergeWebhook(ignore, p, false)
 					} else {
@@ -770,7 +770,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(ctx context.Con
 		for _, p := range policies {
 			if p.AdmissionProcessingEnabled() {
 				spec := p.GetSpec()
-				if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasVerifyImageChecks() || spec.HasVerifyManifests() {
+				if spec.HasValidate() || spec.HasGenerate() || spec.HasMutateExisting() || spec.HasVerifyImageChecks() || spec.HasVerifyManifests() {
 					if spec.GetFailurePolicy(ctx) == kyvernov1.Ignore {
 						c.mergeWebhook(ignore, p, true)
 					} else {
@@ -867,8 +867,8 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface
 			continue
 		}
 		if (updateValidate && rule.HasValidate() || rule.HasVerifyImageChecks()) ||
-			(updateValidate && rule.HasMutate() && rule.IsMutateExisting()) ||
-			(!updateValidate && rule.HasMutate()) && !rule.IsMutateExisting() ||
+			(updateValidate && rule.HasMutateExisting()) ||
+			(!updateValidate && rule.HasMutateStandard()) ||
 			(!updateValidate && rule.HasVerifyImages()) || (!updateValidate && rule.HasVerifyManifests()) {
 			matchedGVK = append(matchedGVK, rule.MatchResources.GetKinds()...)
 		}

pkg/engine/background.go

@@ -52,7 +52,7 @@ func (e *engine) filterRule(
 	logger logr.Logger,
 	policyContext engineapi.PolicyContext,
 ) *engineapi.RuleResponse {
-	if !rule.HasGenerate() && !rule.IsMutateExisting() {
+	if !rule.HasGenerate() && !rule.HasMutateExisting() {
 		return nil
 	}
 

pkg/engine/mutate/mutation.go

@@ -63,7 +63,7 @@ func Mutate(rule *kyvernov1.Rule, ctx context.Interface, resource unstructured.U
 	if err := patchedResource.UnmarshalJSON(patchedBytes); err != nil {
 		return NewErrorResponse("failed to unmarshal patched resource", err)
 	}
-	if rule.IsMutateExisting() {
+	if rule.HasMutateExisting() {
 		if err := ctx.SetTargetResource(patchedResource.Object); err != nil {
 			return NewErrorResponse("failed to update patched target resource in the JSON context", err)
 		}

pkg/engine/mutation.go

@@ -35,7 +35,7 @@ func (e *engine) mutate(
 			if !rule.HasMutate() {
 				return nil, nil
 			}
-			if !policyContext.AdmissionOperation() && rule.IsMutateExisting() {
+			if !policyContext.AdmissionOperation() && rule.HasMutateExisting() {
 				return mutation.NewMutateExistingHandler(e.client)
 			}
 			return mutation.NewMutateResourceHandler()

pkg/policy/mutate.go

@@ -16,7 +16,7 @@ func (pc *policyController) handleMutate(policyKey string, policy kyvernov1.Poli
 	logger.Info("update URs on policy event")
 	for _, rule := range policy.GetSpec().Rules {
 		var ruleType kyvernov1beta1.RequestType
-		if rule.IsMutateExisting() {
+		if rule.HasMutateExisting() {
 			ruleType = kyvernov1beta1.Mutate
 			triggers := generateTriggers(pc.client, rule, pc.log)
 			for _, trigger := range triggers {

pkg/policy/policy_controller.go

@@ -145,7 +145,7 @@ func NewPolicyController(
 
 func (pc *policyController) canBackgroundProcess(p kyvernov1.PolicyInterface) bool {
 	logger := pc.log.WithValues("policy", p.GetName())
-	if !p.GetSpec().HasGenerate() && !p.GetSpec().IsMutateExisting() {
+	if !p.GetSpec().HasGenerate() && !p.GetSpec().HasMutateExisting() {
 		logger.V(4).Info("policy does not have background rules for reconciliation")
 		return false
 	}
@@ -155,7 +155,7 @@ func (pc *policyController) canBackgroundProcess(p kyvernov1.PolicyInterface) bo
 		return false
 	}
 
-	if p.GetSpec().IsMutateExisting() {
+	if p.GetSpec().HasMutateExisting() {
 		val := os.Getenv("BACKGROUND_SCAN_INTERVAL")
 		interval, err := time.ParseDuration(val)
 		if err != nil {

pkg/validation/policy/background.go

@@ -25,7 +25,7 @@ func containsUserVariables(policy kyvernov1.PolicyInterface, vars [][]string) er
 		}
 	}
 	for _, rule := range policy.GetSpec().Rules {
-		if rule.IsMutateExisting() {
+		if rule.HasMutateExisting() {
 			return nil
 		}
 	}

pkg/webhooks/resource/mutation/mutation.go

@@ -86,7 +86,7 @@ func (v *mutationHandler) applyMutations(
 
 	for _, policy := range policies {
 		spec := policy.GetSpec()
-		if !spec.HasMutate() {
+		if !spec.HasMutateStandard() {
 			continue
 		}
 

pkg/webhooks/resource/updaterequest.go

@@ -31,7 +31,7 @@ func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr
 
 	var engineResponses []*engineapi.EngineResponse
 	for _, policy := range policies {
-		if !policy.GetSpec().IsMutateExisting() {
+		if !policy.GetSpec().HasMutateExisting() {
 			continue
 		}