feat: use pointer in rule (mutation field) (#11078)

* feat: use pointer in rule (mutation field)

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

api/kyverno/v1/rule_types.go

@@ -89,7 +89,7 @@ type Rule struct {
 
 	// Mutation is used to modify matching resources.
 	// +optional
-	Mutation Mutation `json:"mutate,omitempty"`
+	Mutation *Mutation `json:"mutate,omitempty"`
 
 	// Validation is used to validate matching resources.
 	// +optional
@@ -113,7 +113,7 @@ type Rule struct {
 
 // HasMutate checks for mutate rule
 func (r *Rule) HasMutate() bool {
-	return !datautils.DeepEqual(r.Mutation, Mutation{})
+	return r.Mutation != nil && !datautils.DeepEqual(*r.Mutation, Mutation{})
 }
 
 // HasMutateStandard checks for standard admission mutate rule
@@ -121,12 +121,12 @@ func (r *Rule) HasMutateStandard() bool {
 	if r.HasMutateExisting() {
 		return false
 	}
-	return !datautils.DeepEqual(r.Mutation, Mutation{})
+	return r.HasMutate()
 }
 
 // HasMutateExisting checks if the mutate rule applies to existing resources
 func (r *Rule) HasMutateExisting() bool {
-	return r.Mutation.Targets != nil
+	return r.Mutation != nil && r.Mutation.Targets != nil
 }
 
 // HasVerifyImages checks for verifyImages rule

api/kyverno/v1/zz_generated.deepcopy.go

@@ -1425,7 +1425,11 @@ func (in *Rule) DeepCopyInto(out *Rule) {
 		*out = make([]v1beta1.MatchCondition, len(*in))
 		copy(*out, *in)
 	}
-	in.Mutation.DeepCopyInto(&out.Mutation)
+	if in.Mutation != nil {
+		in, out := &in.Mutation, &out.Mutation
+		*out = new(Mutation)
+		(*in).DeepCopyInto(*out)
+	}
 	in.Validation.DeepCopyInto(&out.Validation)
 	if in.Generation != nil {
 		in, out := &in.Generation, &out.Generation

api/kyverno/v2beta1/rule_types.go

@@ -52,7 +52,7 @@ type Rule struct {
 
 	// Mutation is used to modify matching resources.
 	// +optional
-	Mutation kyvernov1.Mutation `json:"mutate,omitempty"`
+	Mutation *kyvernov1.Mutation `json:"mutate,omitempty"`
 
 	// Validation is used to validate matching resources.
 	// +optional
@@ -76,7 +76,7 @@ type Rule struct {
 
 // HasMutate checks for mutate rule
 func (r *Rule) HasMutate() bool {
-	return !datautils.DeepEqual(r.Mutation, kyvernov1.Mutation{})
+	return r.Mutation != nil && !datautils.DeepEqual(*r.Mutation, kyvernov1.Mutation{})
 }
 
 // HasMutate checks for standard admission mutate rule
@@ -84,12 +84,12 @@ func (r *Rule) HasMutateStandard() bool {
 	if r.HasMutateExisting() {
 		return false
 	}
-	return !datautils.DeepEqual(r.Mutation, kyvernov1.Mutation{})
+	return r.HasMutate()
 }
 
 // HasMutateExisting checks if the mutate rule applies to existing resources
 func (r *Rule) HasMutateExisting() bool {
-	return r.Mutation.Targets != nil
+	return r.Mutation != nil && r.Mutation.Targets != nil
 }
 
 // HasVerifyImages checks for verifyImages rule

api/kyverno/v2beta1/zz_generated.deepcopy.go

@@ -742,7 +742,11 @@ func (in *Rule) DeepCopyInto(out *Rule) {
 		*out = make([]admissionregistrationv1.MatchCondition, len(*in))
 		copy(*out, *in)
 	}
-	in.Mutation.DeepCopyInto(&out.Mutation)
+	if in.Mutation != nil {
+		in, out := &in.Mutation, &out.Mutation
+		*out = new(v1.Mutation)
+		(*in).DeepCopyInto(*out)
+	}
 	in.Validation.DeepCopyInto(&out.Validation)
 	if in.Generation != nil {
 		in, out := &in.Generation, &out.Generation

pkg/autogen/autogen.go

@@ -69,13 +69,18 @@ func stripCronJob(controllers string) string {
 func CanAutoGen(spec *kyvernov1.Spec) (applyAutoGen bool, controllers sets.Set[string]) {
 	needed := false
 	for _, rule := range spec.Rules {
-		if rule.Mutation.PatchesJSON6902 != "" || rule.HasGenerate() {
+		if rule.HasGenerate() {
 			return false, sets.New("none")
 		}
-		for _, foreach := range rule.Mutation.ForEachMutation {
-			if foreach.PatchesJSON6902 != "" {
+		if rule.Mutation != nil {
+			if rule.Mutation.PatchesJSON6902 != "" {
 				return false, sets.New("none")
 			}
+			for _, foreach := range rule.Mutation.ForEachMutation {
+				if foreach.PatchesJSON6902 != "" {
+					return false, sets.New("none")
+				}
+			}
 		}
 		match := rule.MatchResources
 		if !checkAutogenSupport(&needed, match.ResourceDescription) {
@@ -225,7 +230,7 @@ func convertRule(rule kyvernoRule, kind string) (*kyvernov1.Rule, error) {
 		out.SetAnyAllConditions(rule.AnyAllConditions.Conditions)
 	}
 	if rule.Mutation != nil {
-		out.Mutation = *rule.Mutation
+		out.Mutation = rule.Mutation
 	}
 	if rule.Validation != nil {
 		out.Validation = *rule.Validation

pkg/autogen/rule.go

@@ -45,7 +45,7 @@ func createRule(rule *kyvernov1.Rule) *kyvernoRule {
 	if rule.ExcludeResources != nil && !datautils.DeepEqual(*rule.ExcludeResources, kyvernov1.MatchResources{}) {
 		jsonFriendlyStruct.ExcludeResources = rule.ExcludeResources.DeepCopy()
 	}
-	if !datautils.DeepEqual(rule.Mutation, kyvernov1.Mutation{}) {
+	if rule.Mutation != nil && !datautils.DeepEqual(*rule.Mutation, kyvernov1.Mutation{}) {
 		jsonFriendlyStruct.Mutation = rule.Mutation.DeepCopy()
 	}
 	if !datautils.DeepEqual(rule.Validation, kyvernov1.Validation{}) {
@@ -95,39 +95,41 @@ func generateRule(name string, rule *kyvernov1.Rule, tplKey, shift string, kinds
 			}
 		}
 	}
-	if target := rule.Mutation.GetPatchStrategicMerge(); target != nil {
-		newMutation := kyvernov1.Mutation{}
-		newMutation.SetPatchStrategicMerge(
-			map[string]interface{}{
-				"spec": map[string]interface{}{
-					tplKey: target,
-				},
-			},
-		)
-		rule.Mutation = newMutation
-		return rule
-	}
-	if len(rule.Mutation.ForEachMutation) > 0 && rule.Mutation.ForEachMutation != nil {
-		var newForEachMutation []kyvernov1.ForEachMutation
-		for _, foreach := range rule.Mutation.ForEachMutation {
-			temp := kyvernov1.ForEachMutation{
-				List:             foreach.List,
-				Context:          foreach.Context,
-				AnyAllConditions: foreach.AnyAllConditions,
-			}
-			temp.SetPatchStrategicMerge(
+	if rule.Mutation != nil {
+		if target := rule.Mutation.GetPatchStrategicMerge(); target != nil {
+			newMutation := &kyvernov1.Mutation{}
+			newMutation.SetPatchStrategicMerge(
 				map[string]interface{}{
 					"spec": map[string]interface{}{
-						tplKey: foreach.GetPatchStrategicMerge(),
+						tplKey: target,
 					},
 				},
 			)
-			newForEachMutation = append(newForEachMutation, temp)
+			rule.Mutation = newMutation
+			return rule
 		}
-		rule.Mutation = kyvernov1.Mutation{
-			ForEachMutation: newForEachMutation,
+		if len(rule.Mutation.ForEachMutation) > 0 && rule.Mutation.ForEachMutation != nil {
+			var newForEachMutation []kyvernov1.ForEachMutation
+			for _, foreach := range rule.Mutation.ForEachMutation {
+				temp := kyvernov1.ForEachMutation{
+					List:             foreach.List,
+					Context:          foreach.Context,
+					AnyAllConditions: foreach.AnyAllConditions,
+				}
+				temp.SetPatchStrategicMerge(
+					map[string]interface{}{
+						"spec": map[string]interface{}{
+							tplKey: foreach.GetPatchStrategicMerge(),
+						},
+					},
+				)
+				newForEachMutation = append(newForEachMutation, temp)
+			}
+			rule.Mutation = &kyvernov1.Mutation{
+				ForEachMutation: newForEachMutation,
+			}
+			return rule
 		}
-		return rule
 	}
 	if target := rule.Validation.GetPattern(); target != nil {
 		newValidate := kyvernov1.Validation{

pkg/controllers/webhook/controller_test.go

@@ -236,7 +236,7 @@ func TestAddOperationsForMutatingtingWebhookConf(t *testing.T) {
 			name: "Test Case 1",
 			rules: []kyverno.Rule{
 				{
-					Mutation: kyverno.Mutation{
+					Mutation: &kyverno.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyverno.MatchResources{
@@ -255,7 +255,7 @@ func TestAddOperationsForMutatingtingWebhookConf(t *testing.T) {
 			name: "Test Case 2",
 			rules: []kyverno.Rule{
 				{
-					Mutation: kyverno.Mutation{
+					Mutation: &kyverno.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyverno.MatchResources{
@@ -278,7 +278,7 @@ func TestAddOperationsForMutatingtingWebhookConf(t *testing.T) {
 			name: "Test Case 3",
 			rules: []kyverno.Rule{
 				{
-					Mutation: kyverno.Mutation{
+					Mutation: &kyverno.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyverno.MatchResources{
@@ -289,7 +289,7 @@ func TestAddOperationsForMutatingtingWebhookConf(t *testing.T) {
 					},
 				},
 				{
-					Mutation: kyverno.Mutation{
+					Mutation: &kyverno.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyverno.MatchResources{
@@ -308,7 +308,7 @@ func TestAddOperationsForMutatingtingWebhookConf(t *testing.T) {
 			name: "Test Case 4",
 			rules: []kyverno.Rule{
 				{
-					Mutation: kyverno.Mutation{
+					Mutation: &kyverno.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyverno.MatchResources{
@@ -319,7 +319,7 @@ func TestAddOperationsForMutatingtingWebhookConf(t *testing.T) {
 					},
 				},
 				{
-					Mutation: kyverno.Mutation{
+					Mutation: &kyverno.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyverno.MatchResources{
@@ -370,8 +370,9 @@ func TestAddOperationsForMutatingtingWebhookConfMultiplePolicies(t *testing.T) {
 					Spec: kyverno.Spec{
 						Rules: []kyverno.Rule{
 							{
-								Mutation: kyverno.Mutation{
-									RawPatchStrategicMerge: &apiextensionsv1.JSON{Raw: []byte(`"nodeSelector": {<"public-ip-type": "elastic"}, +"priorityClassName": "elastic-ip-required"`)}},
+								Mutation: &kyverno.Mutation{
+									RawPatchStrategicMerge: &apiextensionsv1.JSON{Raw: []byte(`"nodeSelector": {<"public-ip-type": "elastic"}, +"priorityClassName": "elastic-ip-required"`)},
+								},
 								MatchResources: kyverno.MatchResources{
 									ResourceDescription: kyverno.ResourceDescription{
 										Kinds: []string{"Pod"},

pkg/controllers/webhook/utils_test.go

@@ -219,7 +219,7 @@ func TestComputeOperationsForMutatingWebhookConf(t *testing.T) {
 			name: "Test Case 1",
 			rules: []kyvernov1.Rule{
 				{
-					Mutation: kyvernov1.Mutation{
+					Mutation: &kyvernov1.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyvernov1.MatchResources{
@@ -237,14 +237,14 @@ func TestComputeOperationsForMutatingWebhookConf(t *testing.T) {
 			name: "Test Case 2",
 			rules: []kyvernov1.Rule{
 				{
-					Mutation: kyvernov1.Mutation{
+					Mutation: &kyvernov1.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources:   kyvernov1.MatchResources{},
 					ExcludeResources: &kyvernov1.MatchResources{},
 				},
 				{
-					Mutation: kyvernov1.Mutation{
+					Mutation: &kyvernov1.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources:   kyvernov1.MatchResources{},
@@ -260,7 +260,7 @@ func TestComputeOperationsForMutatingWebhookConf(t *testing.T) {
 			name: "Test Case 2",
 			rules: []kyvernov1.Rule{
 				{
-					Mutation: kyvernov1.Mutation{
+					Mutation: &kyvernov1.Mutation{
 						PatchesJSON6902: "add",
 					},
 					MatchResources: kyvernov1.MatchResources{},

pkg/engine/mutate/mutation.go

@@ -40,8 +40,11 @@ func Mutate(rule *kyvernov1.Rule, ctx context.Interface, resource unstructured.U
 	if err != nil {
 		return NewErrorResponse("variable substitution failed", err)
 	}
-	m := updatedRule.Mutation
-	patcher := NewPatcher(m.GetPatchStrategicMerge(), m.PatchesJSON6902)
+	mutation := updatedRule.Mutation
+	if mutation == nil {
+		return NewErrorResponse("empty mutate rule", nil)
+	}
+	patcher := NewPatcher(mutation.GetPatchStrategicMerge(), mutation.PatchesJSON6902)
 	if patcher == nil {
 		return NewErrorResponse("empty mutate rule", nil)
 	}

pkg/engine/mutate/mutation_test.go

@@ -79,8 +79,7 @@ func makeRuleWithPatches(t *testing.T, patches []jsonPatch) *types.Rule {
 	if err != nil {
 		t.Errorf("failed to marshal patch: %v", err)
 	}
-
-	mutation := types.Mutation{
+	mutation := &types.Mutation{
 		PatchesJSON6902: string(jsonPatches),
 	}
 	return &types.Rule{

pkg/metrics/parsers.go

@@ -26,7 +26,7 @@ func ParseRuleType(rule kyvernov1.Rule) RuleType {
 	if !datautils.DeepEqual(rule.Validation, kyvernov1.Validation{}) {
 		return Validate
 	}
-	if !datautils.DeepEqual(rule.Mutation, kyvernov1.Mutation{}) {
+	if rule.Mutation != nil && !datautils.DeepEqual(*rule.Mutation, kyvernov1.Mutation{}) {
 		return Mutate
 	}
 	if rule.Generation != nil && !datautils.DeepEqual(*rule.Generation, kyvernov1.Generation{}) {

pkg/policycache/cache_test.go

@@ -894,7 +894,8 @@ func Test_Ns_All(t *testing.T) {
 	//add
 	setPolicy(t, pCache, policy, finder)
 	nspace := policy.GetNamespace()
-	for _, rule := range autogen.ComputeRules(policy, "") {
+	rules := autogen.ComputeRules(policy, "")
+	for _, rule := range rules {
 		for _, kind := range rule.MatchResources.Kinds {
 			group, version, kind, subresource := kubeutils.ParseKindSelector(kind)
 			gvrs, err := finder.FindResources(group, version, kind, subresource)

pkg/utils/fuzz/policy_spec.go

@@ -220,7 +220,7 @@ func createRule(f *fuzz.ConsumeFuzzer) (*kyvernov1.Rule, error) {
 		if err != nil {
 			return rule, err
 		}
-		rule.Mutation = *m
+		rule.Mutation = m
 	}
 
 	setValidation, err := f.GetBool()

pkg/validation/policy/actions.go

@@ -35,7 +35,7 @@ func validateActions(idx int, rule *kyvernov1.Rule, client dclient.Interface, mo
 
 	// Mutate
 	if rule.HasMutate() {
-		checker = mutate.NewMutateFactory(rule.Mutation, client, mock, backgroundSA)
+		checker = mutate.NewMutateFactory(*rule.Mutation, client, mock, backgroundSA)
 		if w, path, err := checker.Validate(context.TODO(), nil); err != nil {
 			return nil, fmt.Errorf("path: spec.rules[%d].mutate.%s.: %v", idx, path, err)
 		} else if w != nil {

pkg/validation/policy/validate.go

@@ -268,12 +268,14 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 
 	for i, rule := range rules {
 		rulePath := rulesPath.Index(i)
-		// check for forward slash
-		if err := validateJSONPatchPathForForwardSlash(rule.Mutation.PatchesJSON6902); err != nil {
-			return warnings, fmt.Errorf("path must begin with a forward slash: spec.rules[%d]: %s", i, err)
-		}
-		if err := validateJSONPatch(rule.Mutation.PatchesJSON6902, i); err != nil {
-			return warnings, fmt.Errorf("%s", err)
+		if rule.Mutation != nil {
+			// check for forward slash
+			if err := validateJSONPatchPathForForwardSlash(rule.Mutation.PatchesJSON6902); err != nil {
+				return warnings, fmt.Errorf("path must begin with a forward slash: spec.rules[%d]: %s", i, err)
+			}
+			if err := validateJSONPatch(rule.Mutation.PatchesJSON6902, i); err != nil {
+				return warnings, fmt.Errorf("%s", err)
+			}
 		}
 
 		if jsonPatchOnPod(rule) {
@@ -561,7 +563,7 @@ func hasInvalidVariables(policy kyvernov1.PolicyInterface, background bool) erro
 		}
 
 		mutateTarget := false
-		if ruleCopy.Mutation.Targets != nil {
+		if ruleCopy.Mutation != nil && ruleCopy.Mutation.Targets != nil {
 			mutateTarget = true
 			withTargetOnly := ruleWithoutPattern(ruleCopy)
 			for i := range ruleCopy.Mutation.Targets {
@@ -605,10 +607,11 @@ func ValidateOnPolicyUpdate(p kyvernov1.PolicyInterface, onPolicyUpdate bool) er
 // for now forbidden sections are match, exclude and
 func ruleForbiddenSectionsHaveVariables(rule *kyvernov1.Rule) error {
 	var err error
-
-	err = jsonPatchPathHasVariables(rule.Mutation.PatchesJSON6902)
-	if err != nil && errors.Is(errOperationForbidden, err) {
-		return fmt.Errorf("rule \"%s\" should not have variables in patchesJSON6902 path section", rule.Name)
+	if rule.Mutation != nil {
+		err = jsonPatchPathHasVariables(rule.Mutation.PatchesJSON6902)
+		if err != nil && errors.Is(errOperationForbidden, err) {
+			return fmt.Errorf("rule \"%s\" should not have variables in patchesJSON6902 path section", rule.Name)
+		}
 	}
 
 	err = objectHasVariables(rule.ExcludeResources)
@@ -718,7 +721,9 @@ func imageRefHasVariables(verifyImages []kyvernov1.ImageVerification) error {
 }
 
 func ruleWithoutPattern(ruleCopy *kyvernov1.Rule) *kyvernov1.Rule {
-	withTargetOnly := new(kyvernov1.Rule)
+	withTargetOnly := &kyvernov1.Rule{
+		Mutation: &kyvernov1.Mutation{},
+	}
 	withTargetOnly.Mutation.Targets = make([]kyvernov1.TargetResourceSpec, len(ruleCopy.Mutation.Targets))
 	withTargetOnly.Context = ruleCopy.Context
 	withTargetOnly.RawAnyAllConditions = ruleCopy.RawAnyAllConditions
@@ -736,11 +741,13 @@ func buildContext(rule *kyvernov1.Rule, background bool, target bool) *enginecon
 	for _, fe := range rule.Validation.ForEachValidation {
 		addContextVariables(fe.Context, ctx)
 	}
-	for _, fe := range rule.Mutation.ForEachMutation {
-		addContextVariables(fe.Context, ctx)
-	}
-	for _, fe := range rule.Mutation.Targets {
-		addContextVariables(fe.Context, ctx)
+	if rule.Mutation != nil {
+		for _, fe := range rule.Mutation.ForEachMutation {
+			addContextVariables(fe.Context, ctx)
+		}
+		for _, fe := range rule.Mutation.Targets {
+			addContextVariables(fe.Context, ctx)
+		}
 	}
 	if rule.HasGenerate() {
 		for _, fe := range rule.Generation.ForEachGeneration {
@@ -917,14 +924,16 @@ func isLabelAndAnnotationsString(rule kyvernov1.Rule) bool {
 }
 
 func ruleOnlyDealsWithResourceMetaData(rule kyvernov1.Rule) bool {
-	patches, _ := rule.Mutation.GetPatchStrategicMerge().(map[string]interface{})
-	for k := range patches {
-		if k != "metadata" {
-			return false
+	if rule.Mutation != nil {
+		patches, _ := rule.Mutation.GetPatchStrategicMerge().(map[string]interface{})
+		for k := range patches {
+			if k != "metadata" {
+				return false
+			}
 		}
 	}
 
-	if rule.Mutation.PatchesJSON6902 != "" {
+	if rule.Mutation != nil && rule.Mutation.PatchesJSON6902 != "" {
 		bytes := []byte(rule.Mutation.PatchesJSON6902)
 		jp, _ := jsonpatch.DecodePatch(bytes)
 		for _, o := range jp {
@@ -1026,7 +1035,7 @@ func validateResources(path *field.Path, rule kyvernov1.Rule) (string, error) {
 		}
 	}
 
-	if len(rule.Mutation.ForEachMutation) != 0 {
+	if rule.Mutation != nil && len(rule.Mutation.ForEachMutation) != 0 {
 		if path, err := validateMutationForEach(rule.Mutation.ForEachMutation, "mutate.foreach"); err != nil {
 			return path, err
 		}
@@ -1479,7 +1488,7 @@ func jsonPatchOnPod(rule kyvernov1.Rule) bool {
 		return false
 	}
 
-	if slices.Contains(rule.MatchResources.Kinds, "Pod") && rule.Mutation.PatchesJSON6902 != "" {
+	if slices.Contains(rule.MatchResources.Kinds, "Pod") && rule.Mutation != nil && rule.Mutation.PatchesJSON6902 != "" {
 		return true
 	}
 

pkg/validation/policy/validate_test.go

@@ -1693,7 +1693,7 @@ func Test_ValidateNamespace(t *testing.T) {
 					{
 						Name:           "require-labels",
 						MatchResources: kyverno.MatchResources{ResourceDescription: kyverno.ResourceDescription{Kinds: []string{"Pod"}}},
-						Mutation: kyverno.Mutation{
+						Mutation: &kyverno.Mutation{
 							RawPatchStrategicMerge: &apiextv1.JSON{Raw: []byte(`"metadata": {"labels": {"app-name": "{{request.object.metadata.name}}"}}`)},
 						},
 					},