From 668cfbbc7c5496f0e36de9d80658270bc0946ad6 Mon Sep 17 00:00:00 2001
From: Bradon Kanyid <rattboi@gmail.com>
Date: Thu, 5 May 2022 21:52:58 -0700
Subject: [PATCH] fix: docker references with both tag and digest

fix #48

This solution is taken almost directly from CRI-O, as referenced here: https://github.com/cri-o/cri-o/pull/3060/files
---
 pkg/webhook/image_swapper.go              | 30 +++++++++++++++++++++--
 pkg/webhook/image_swapper_test.go         | 16 +++++++++++-
 test/requests/admissionreview-simple.json | 15 ++++++++++++
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/pkg/webhook/image_swapper.go b/pkg/webhook/image_swapper.go
index ea66c0b0..2788ac49 100644
--- a/pkg/webhook/image_swapper.go
+++ b/pkg/webhook/image_swapper.go
@@ -138,6 +138,26 @@ func NewImageSwapperWebhook(registryClient registry.Client, imagePullSecretProvi
 	return kwhmutating.NewWebhook(mcfg)
 }
 
+// imageNamesWithDigestOrTag strips the tag from ambiguous image references that have a digest as well (e.g. `image:tag@sha256:123...`).
+// Such image references are supported by docker but, due to their ambiguity,
+// explicitly not by containers/image.
+func imageNamesWithDigestOrTag(imageName string) (string, error) {
+	ref, err := reference.ParseNormalizedNamed(imageName)
+	if err != nil {
+		return "", err
+	}
+	_, isTagged := ref.(reference.NamedTagged)
+	canonical, isDigested := ref.(reference.Canonical)
+	if isTagged && isDigested {
+		canonical, err = reference.WithDigest(reference.TrimNamed(ref), canonical.Digest())
+		if err != nil {
+			return "", err
+		}
+		imageName = canonical.String()
+	}
+	return imageName, nil
+}
+
 // Mutate replaces the image ref. Satisfies mutating.Mutator interface.
 func (p *ImageSwapper) Mutate(ctx context.Context, ar *kwhmodel.AdmissionReview, obj metav1.Object) (*kwhmutating.MutatorResult, error) {
 	pod, ok := obj.(*corev1.Pod)
@@ -159,9 +179,15 @@ func (p *ImageSwapper) Mutate(ctx context.Context, ar *kwhmodel.AdmissionReview,
 	for _, containerSet := range containerSets {
 		containers := *containerSet
 		for i, container := range containers {
-			srcRef, err := alltransports.ParseImageName("docker://" + container.Image)
+			normalizedName, err := imageNamesWithDigestOrTag(container.Image)
+			if err != nil {
+				log.Ctx(lctx).Warn().Msgf("unable to normalize source name %s: %v", container.Image, err)
+				continue
+			}
+
+			srcRef, err := alltransports.ParseImageName("docker://" + normalizedName)
 			if err != nil {
-				log.Ctx(lctx).Warn().Msgf("invalid source name %s: %v", container.Image, err)
+				log.Ctx(lctx).Warn().Msgf("invalid source name %s: %v", normalizedName, err)
 				continue
 			}
 
diff --git a/pkg/webhook/image_swapper_test.go b/pkg/webhook/image_swapper_test.go
index c1803e6a..7e288a29 100644
--- a/pkg/webhook/image_swapper_test.go
+++ b/pkg/webhook/image_swapper_test.go
@@ -263,6 +263,19 @@ func TestImageSwapper_Mutate(t *testing.T) {
 				Value: aws.String("k8s-image-swapper"),
 			}},
 		}).Return(mock.Anything)
+	ecrClient.On(
+		"CreateRepository",
+		&ecr.CreateRepositoryInput{
+			ImageScanningConfiguration: &ecr.ImageScanningConfiguration{
+				ScanOnPush: aws.Bool(true),
+			},
+			ImageTagMutability: aws.String("MUTABLE"),
+			RepositoryName:     aws.String("k8s.gcr.io/ingress-nginx/controller"),
+			Tags: []*ecr.Tag{{
+				Key:   aws.String("CreatedBy"),
+				Value: aws.String("k8s-image-swapper"),
+			}},
+		}).Return(mock.Anything)
 
 	registryClient, _ := registry.NewMockECRClient(ecrClient, "ap-southeast-2", "123456789.dkr.ecr.ap-southeast-2.amazonaws.com")
 
@@ -283,7 +296,8 @@ func TestImageSwapper_Mutate(t *testing.T) {
 
 	expected := `[
 		{"op":"replace","path":"/spec/initContainers/0/image","value":"123456789.dkr.ecr.ap-southeast-2.amazonaws.com/docker.io/library/init-container:latest"},
-		{"op":"replace","path":"/spec/containers/0/image","value":"123456789.dkr.ecr.ap-southeast-2.amazonaws.com/docker.io/library/nginx:latest"}
+		{"op":"replace","path":"/spec/containers/0/image","value":"123456789.dkr.ecr.ap-southeast-2.amazonaws.com/docker.io/library/nginx:latest"},
+		{"op":"replace","path":"/spec/containers/1/image","value":"123456789.dkr.ecr.ap-southeast-2.amazonaws.com/k8s.gcr.io/ingress-nginx/controller@sha256:9bba603b99bf25f6d117cf1235b6598c16033ad027b143c90fa5b3cc583c5713"}
 	]`
 
 	assert.JSONEq(t, expected, string(resp.(*model.MutatingAdmissionResponse).JSONPatchPatch))
diff --git a/test/requests/admissionreview-simple.json b/test/requests/admissionreview-simple.json
index 07008cab..7ee6637d 100644
--- a/test/requests/admissionreview-simple.json
+++ b/test/requests/admissionreview-simple.json
@@ -36,6 +36,21 @@
                 "readOnly": true
               }
             ]
+          },
+          {
+            "image": "k8s.gcr.io/ingress-nginx/controller:v0.43.0@sha256:9bba603b99bf25f6d117cf1235b6598c16033ad027b143c90fa5b3cc583c5713",
+            "imagePullPolicy": "Always",
+            "name": "ingress-nginx28",
+            "resources": {},
+            "terminationMessagePath": "/dev/termination-log",
+            "terminationMessagePolicy": "File",
+            "volumeMounts": [
+              {
+                "mountPath": "/var/run/secrets/kubernetes.io/serviceaccount",
+                "name": "default-token-fjtvr",
+                "readOnly": true
+              }
+            ]
           }
         ],
         "dnsPolicy": "ClusterFirst",