refactor: remove MutateResourceWithImageInfo (#6397)

* refactor: remove new resource from policy context

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

* fallback

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

* test something else

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

* fix test

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

* fix test

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

* fix test

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>

* fix

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

* fix kuttl test

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

* fix cli tests

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

* clean

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

* changelog

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

---------

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

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Removed `GenerateRequest` CRD.
 - Refactored `kyverno` chart, migration instructions are available in chart `README.md`.
+- Image references in the json context are not mutated to canonical form anymore, do not assume a registry domain is always present.
 
 ## v1.9.0-rc.1
 

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

@@ -457,10 +457,6 @@ OuterLoop:
 		}
 	}
 
-	if err := engineContext.MutateResourceWithImageInfo(resourceRaw, ctx); err != nil {
-		log.Log.Error(err, "failed to add image variables to context")
-	}
-
 	subresources := make([]engineapi.SubResource, 0)
 
 	// If --cluster flag is not set, then we need to add subresources to the context

pkg/engine/context/imageutils.go

@@ -1,32 +0,0 @@
-package context
-
-import (
-	"fmt"
-
-	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
-)
-
-// MutateResourceWithImageInfo will set images to their canonical form so that they can be compared
-// in a predictable manner. This sets the default registry as `docker.io` and the tag as `latest` if
-// these are missing.
-func MutateResourceWithImageInfo(raw []byte, ctx Interface) error {
-	images := ctx.ImageInfo()
-	if images == nil {
-		return nil
-	}
-	var patches [][]byte
-	buildJSONPatch := func(op, path, value string) []byte {
-		p := fmt.Sprintf(`{ "op": "%s", "path": "%s", "value":"%s" }`, op, path, value)
-		return []byte(p)
-	}
-	for _, infoMaps := range images {
-		for _, info := range infoMaps {
-			patches = append(patches, buildJSONPatch("replace", info.Pointer, info.String()))
-		}
-	}
-	patchedResource, err := engineutils.ApplyPatches(raw, patches)
-	if err != nil {
-		return err
-	}
-	return AddResource(ctx, patchedResource)
-}

pkg/engine/mutation_test.go

@@ -329,7 +329,7 @@ func Test_chained_rules(t *testing.T) {
               "containers": [
                 {
                   "(name)": "*",
-                  "image": "{{regex_replace_all('^[^/]+','{{@}}','myregistry.corp.com')}}"
+                  "image": "{{regex_replace_all('^([^/]+\\.[^/]+/)?(.*)$','{{@}}','myregistry.corp.com/$2')}}"
                 }
               ]
             }
@@ -396,9 +396,6 @@ func Test_chained_rules(t *testing.T) {
 	err = ctx.AddImageInfos(resource, cfg)
 	assert.NilError(t, err)
 
-	err = enginecontext.MutateResourceWithImageInfo(resourceRaw, ctx)
-	assert.NilError(t, err)
-
 	er := testMutate(context.TODO(), nil, registryclient.NewOrDie(), policyContext, nil)
 	containers, _, err := unstructured.NestedSlice(er.PatchedResource.Object, "spec", "containers")
 	assert.NilError(t, err)
@@ -673,9 +670,6 @@ func Test_foreach(t *testing.T) {
 	err = ctx.AddImageInfos(resource, cfg)
 	assert.NilError(t, err)
 
-	err = enginecontext.MutateResourceWithImageInfo(resourceRaw, ctx)
-	assert.NilError(t, err)
-
 	er := testMutate(context.TODO(), nil, registryclient.NewOrDie(), policyContext, nil)
 
 	assert.Equal(t, len(er.PolicyResponse.Rules), 1)
@@ -780,9 +774,6 @@ func Test_foreach_element_mutation(t *testing.T) {
 	err = ctx.AddImageInfos(resource, cfg)
 	assert.NilError(t, err)
 
-	err = enginecontext.MutateResourceWithImageInfo(resourceRaw, ctx)
-	assert.NilError(t, err)
-
 	er := testMutate(context.TODO(), nil, registryclient.NewOrDie(), policyContext, nil)
 
 	assert.Equal(t, len(er.PolicyResponse.Rules), 1)
@@ -906,9 +897,6 @@ func Test_Container_InitContainer_foreach(t *testing.T) {
 	err = ctx.AddImageInfos(resource, cfg)
 	assert.NilError(t, err)
 
-	err = enginecontext.MutateResourceWithImageInfo(resourceRaw, ctx)
-	assert.NilError(t, err)
-
 	er := testMutate(context.TODO(), nil, registryclient.NewOrDie(), policyContext, nil)
 
 	assert.Equal(t, len(er.PolicyResponse.Rules), 1)
@@ -1056,9 +1044,6 @@ func testApplyPolicyToResource(t *testing.T, policyRaw, resourceRaw []byte) *eng
 	err = ctx.AddImageInfos(resource, cfg)
 	assert.NilError(t, err)
 
-	err = enginecontext.MutateResourceWithImageInfo(resourceRaw, ctx)
-	assert.NilError(t, err)
-
 	er := testMutate(context.TODO(), nil, registryclient.NewOrDie(), policyContext, nil)
 	return er
 }

pkg/webhooks/resource/handlers.go

@@ -16,7 +16,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	"github.com/kyverno/kyverno/pkg/config"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
-	enginectx "github.com/kyverno/kyverno/pkg/engine/context"
 	"github.com/kyverno/kyverno/pkg/event"
 	"github.com/kyverno/kyverno/pkg/metrics"
 	"github.com/kyverno/kyverno/pkg/openapi"
@@ -164,10 +163,6 @@ func (h *handlers) Mutate(ctx context.Context, logger logr.Logger, request *admi
 		logger.Error(err, "failed to build policy context")
 		return admissionutils.Response(request.UID, err)
 	}
-	// update container images to a canonical form
-	if err := enginectx.MutateResourceWithImageInfo(request.Object.Raw, policyContext.JSONContext()); err != nil {
-		logger.Error(err, "failed to patch images info to resource, policies that mutate images may be impacted")
-	}
 	mh := mutation.NewMutationHandler(logger, h.engine, h.eventGen, h.openApiManager, h.nsLister, h.metricsConfig)
 	mutatePatches, mutateWarnings, err := mh.HandleMutation(ctx, request, mutatePolicies, policyContext, startTime)
 	if err != nil {

pkg/webhooks/resource/handlers_test.go

@@ -166,7 +166,7 @@ var policyMutateAndVerify = `
                                     "containers": [
                                         {
                                             "name": "{{ element.name }}",
-                                            "image": "{{ regex_replace_all_literal('.*(.*)/', '{{element.image}}', 'ghcr.io/kyverno/' )}}"
+                                            "image": "{{ regex_replace_all('^([^/]+\\.[^/]+/)?(.*)$', '{{element.image}}', 'ghcr.io/kyverno/$2' )}}"
                                         }
                                     ]
                                 }

test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/README.md

@@ -4,7 +4,7 @@ This is a migrated test from e2e. It checks that simple JSON patches function pr
 
 ## Expected Behavior
 
-If the Pod has a second environment variable added with the name `K8S_IMAGE` with value equal to `docker.io/busybox:1.11` then the test succeeds. If it does not, the test fails. Note that there is an initContainer present which based upon the policy definition should NOT be mutated.
+If the Pod has a second environment variable added with the name `K8S_IMAGE` with value equal to `busybox:1.11` then the test succeeds. If it does not, the test fails. Note that there is an initContainer present which based upon the policy definition should NOT be mutated.
 
 ## Reference Issue(s)
 

test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/resource-mutated.yaml

@@ -11,7 +11,7 @@ spec:
     - name: FOO
       value: bar
     - name: K8S_IMAGE
-      value: docker.io/busybox:1.11
+      value: busybox:1.11
     image: busybox:1.11
     name: busybox
     securityContext: