From fb3a90c7037e627019411860515ec094b7992608 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?=
<charled.breteche@gmail.com>
Date: Mon, 27 Feb 2023 14:45:00 +0100
Subject: [PATCH] refactor: remove MutateResourceWithImageInfo (#6397)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* 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 | 1 +
.../kubectl-kyverno/utils/common/common.go | 4 ---
pkg/engine/context/imageutils.go | 32 -------------------
pkg/engine/mutation_test.go | 17 +---------
pkg/webhooks/resource/handlers.go | 5 ---
pkg/webhooks/resource/handlers_test.go | 2 +-
.../e2e/patchesjson6902-simple/README.md | 2 +-
.../resource-mutated.yaml | 2 +-
8 files changed, 5 insertions(+), 60 deletions(-)
delete mode 100644 pkg/engine/context/imageutils.go
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c3ed6c28ae..d4f2cfcf7a 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/cmd/cli/kubectl-kyverno/utils/common/common.go b/cmd/cli/kubectl-kyverno/utils/common/common.go
index 0c74234603..bded6663d1 100644
--- a/cmd/cli/kubectl-kyverno/utils/common/common.go
+++ b/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
diff --git a/pkg/engine/context/imageutils.go b/pkg/engine/context/imageutils.go
deleted file mode 100644
index c6859982cd..0000000000
--- a/pkg/engine/context/imageutils.go
+++ /dev/null
@@ -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)
-}
diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go
index 54dadf406f..dda6455adc 100644
--- a/pkg/engine/mutation_test.go
+++ b/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
}
diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go
index 4cb077c3b8..ea3cc93594 100644
--- a/pkg/webhooks/resource/handlers.go
+++ b/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 {
diff --git a/pkg/webhooks/resource/handlers_test.go b/pkg/webhooks/resource/handlers_test.go
index 588c88062a..32aec953eb 100644
--- a/pkg/webhooks/resource/handlers_test.go
+++ b/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' )}}"
}
]
}
diff --git a/test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/README.md b/test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/README.md
index dc76b8b972..6e808bfeb4 100644
--- a/test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/README.md
+++ b/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)
diff --git a/test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/resource-mutated.yaml b/test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/resource-mutated.yaml
index 5ec53a9a7f..8d1da7023a 100644
--- a/test/conformance/kuttl/mutate/e2e/patchesjson6902-simple/resource-mutated.yaml
+++ b/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: