Fix image name parsing with tag and digest (#4406)

* Fix image name parsing with name and digest Image names may contain both tag name and digest. For example `nginx:1.21.5@sha256:7826426c9d8d310c62fc68bcd5e8dde70cb39d4fbbd30eda3b1bd03e35fbde29`. Kustomizations with image transforms will not match these image because the image parser assumes either a tag or digest, but not both. For a real life example of kuberenetes deployments that might need to perform these types of transforms is from the [tekton-pipelines](https://github.com/tektoncd/pipeline) project (see the release.yaml). * Return digest property from image name parser image.Split now returns 3 fields: name, tag, and digest. The tag and digest fields no longer include their respective delimiters (`:` and `@`). * Fix merge file indentation * Refactor imagetag updater string builder
kubernetes-sigs · Feb 16, 2022 · 8dab949 · 8dab949
1 parent ff40460
commit 8dab949
Show file tree

Hide file tree

Showing 4 changed files with 196 additions and 31 deletions.
diff --git a/api/filters/imagetag/imagetag_test.go b/api/filters/imagetag/imagetag_test.go
@@ -750,6 +750,96 @@ spec:
 				},
 			},
 		},
+		"image with tag and digest new name": {
+			input: `
+apiVersion: example.com/v1
+kind: Foo
+metadata:
+  name: instance
+spec:
+  image: nginx:1.2.1@sha256:46d5b90a7f4e9996351ad893a26bcbd27216676ad4d5316088ce351fb2c2c3dd
+`,
+			expectedOutput: `
+apiVersion: example.com/v1
+kind: Foo
+metadata:
+  name: instance
+spec:
+  image: apache:1.2.1@sha256:46d5b90a7f4e9996351ad893a26bcbd27216676ad4d5316088ce351fb2c2c3dd
+`,
+			filter: Filter{
+				ImageTag: types.Image{
+					Name:    "nginx",
+					NewName: "apache",
+				},
+			},
+			fsSlice: []types.FieldSpec{
+				{
+					Path: "spec/image",
+				},
+			},
+		},
+		"image with tag and digest new name new tag": {
+			input: `
+apiVersion: example.com/v1
+kind: Foo
+metadata:
+  name: instance
+spec:
+  image: nginx:1.2.1@sha256:46d5b90a7f4e9996351ad893a26bcbd27216676ad4d5316088ce351fb2c2c3dd
+`,
+			expectedOutput: `
+apiVersion: example.com/v1
+kind: Foo
+metadata:
+  name: instance
+spec:
+  image: apache:1.3.0
+`,
+			filter: Filter{
+				ImageTag: types.Image{
+					Name:    "nginx",
+					NewName: "apache",
+					NewTag:  "1.3.0",
+				},
+			},
+			fsSlice: []types.FieldSpec{
+				{
+					Path: "spec/image",
+				},
+			},
+		},
+		"image with tag and digest new name new tag and digest": {
+			input: `
+apiVersion: example.com/v1
+kind: Foo
+metadata:
+  name: instance
+spec:
+  image: nginx:1.2.1@sha256:46d5b90a7f4e9996351ad893a26bcbd27216676ad4d5316088ce351fb2c2c3dd
+`,
+			expectedOutput: `
+apiVersion: example.com/v1
+kind: Foo
+metadata:
+  name: instance
+spec:
+  image: apache:1.3.0@sha256:xyz
+`,
+			filter: Filter{
+				ImageTag: types.Image{
+					Name:    "nginx",
+					NewName: "apache",
+					NewTag:  "1.3.0",
+					Digest:  "sha256:xyz",
+				},
+			},
+			fsSlice: []types.FieldSpec{
+				{
+					Path: "spec/image",
+				},
+			},
+		},
 	}
 
 	for tn, tc := range testCases {

diff --git a/api/filters/imagetag/updater.go b/api/filters/imagetag/updater.go
@@ -30,18 +30,32 @@ func (u imageTagUpdater) SetImageValue(rn *yaml.RNode) error {
 		return nil
 	}
 
-	name, tag := image.Split(value)
+	name, tag, digest := image.Split(value)
 	if u.ImageTag.NewName != "" {
 		name = u.ImageTag.NewName
 	}
-	if u.ImageTag.NewTag != "" {
-		tag = ":" + u.ImageTag.NewTag
+
+	// overriding tag or digest will replace both original tag and digest values
+	if u.ImageTag.NewTag != "" && u.ImageTag.Digest != "" {
+		tag = u.ImageTag.NewTag
+		digest = u.ImageTag.Digest
+	} else if u.ImageTag.NewTag != "" {
+		tag = u.ImageTag.NewTag
+		digest = ""
+	} else if u.ImageTag.Digest != "" {
+		tag = ""
+		digest = u.ImageTag.Digest
+	}
+
+	// build final image name
+	if tag != "" {
+		name += ":" + tag
 	}
-	if u.ImageTag.Digest != "" {
-		tag = "@" + u.ImageTag.Digest
+	if digest != "" {
+		name += "@" + digest
 	}
 
-	return u.trackableSetter.SetScalar(name + tag)(rn)
+	return u.trackableSetter.SetScalar(name)(rn)
 }
 
 func (u imageTagUpdater) Filter(rn *yaml.RNode) (*yaml.RNode, error) {

diff --git a/api/image/image.go b/api/image/image.go
@@ -14,37 +14,53 @@ func IsImageMatched(s, t string) bool {
 	// Tag values are limited to [a-zA-Z0-9_.{}-].
 	// Some tools like Bazel rules_k8s allow tag patterns with {} characters.
 	// More info: https://github.com/bazelbuild/rules_k8s/pull/423
-	pattern, _ := regexp.Compile("^" + t + "(@sha256)?(:[a-zA-Z0-9_.{}-]*)?$")
+	pattern, _ := regexp.Compile("^" + t + "(:[a-zA-Z0-9_.{}-]*)?(@sha256:[a-zA-Z0-9_.{}-]*)?$")
 	return pattern.MatchString(s)
 }
 
 // Split separates and returns the name and tag parts
 // from the image string using either colon `:` or at `@` separators.
-// Note that the returned tag keeps its separator.
-func Split(imageName string) (name string, tag string) {
+// image reference pattern: [[host[:port]/]component/]component[:tag][@digest]
+func Split(imageName string) (name string, tag string, digest string) {
 	// check if image name contains a domain
 	// if domain is present, ignore domain and check for `:`
-	ic := -1
-	if slashIndex := strings.Index(imageName, "/"); slashIndex < 0 {
-		ic = strings.LastIndex(imageName, ":")
+	searchName := imageName
+	slashIndex := strings.Index(imageName, "/")
+	if slashIndex > 0 {
+		searchName = imageName[slashIndex:]
 	} else {
-		lastIc := strings.LastIndex(imageName[slashIndex:], ":")
-		// set ic only if `:` is present
-		if lastIc > 0 {
-			ic = slashIndex + lastIc
-		}
+		slashIndex = 0
 	}
-	ia := strings.LastIndex(imageName, "@")
-	if ic < 0 && ia < 0 {
-		return imageName, ""
+
+	id := strings.Index(searchName, "@")
+	ic := strings.Index(searchName, ":")
+
+	// no tag or digest
+	if ic < 0 && id < 0 {
+		return imageName, "", ""
+	}
+
+	// digest only
+	if id >= 0 && (id < ic || ic < 0) {
+		id += slashIndex
+		name = imageName[:id]
+		digest = strings.TrimPrefix(imageName[id:], "@")
+		return name, "", digest
 	}
 
-	i := ic
-	if ia > 0 {
-		i = ia
+	// tag and digest
+	if id >= 0 && ic >= 0 {
+		id += slashIndex
+		ic += slashIndex
+		name = imageName[:ic]
+		tag = strings.TrimPrefix(imageName[ic:id], ":")
+		digest = strings.TrimPrefix(imageName[id:], "@")
+		return name, tag, digest
 	}
 
-	name = imageName[:i]
-	tag = imageName[i:]
-	return
+	// tag only
+	ic += slashIndex
+	name = imageName[:ic]
+	tag = strings.TrimPrefix(imageName[ic:], ":")
+	return name, tag, ""
 }
diff --git a/api/image/image_test.go b/api/image/image_test.go
@@ -23,11 +23,23 @@ func TestIsImageMatched(t *testing.T) {
 			isMatched: true,
 		},
 		{
-			testName:  "name is match",
+			testName:  "name is match with tag",
 			value:     "nginx:12345",
 			name:      "nginx",
 			isMatched: true,
 		},
+		{
+			testName:  "name is match with digest",
+			value:     "nginx@sha256:xyz",
+			name:      "nginx",
+			isMatched: true,
+		},
+		{
+			testName:  "name is match with tag and digest",
+			value:     "nginx:12345@sha256:xyz",
+			name:      "nginx",
+			isMatched: true,
+		},
 		{
 			testName:  "name is not a match",
 			value:     "apache:12345",
@@ -49,32 +61,65 @@ func TestSplit(t *testing.T) {
 		value    string
 		name     string
 		tag      string
+		digest   string
 	}{
 		{
 			testName: "no tag",
 			value:    "nginx",
 			name:     "nginx",
 			tag:      "",
+			digest:   "",
 		},
 		{
 			testName: "with tag",
 			value:    "nginx:1.2.3",
 			name:     "nginx",
-			tag:      ":1.2.3",
+			tag:      "1.2.3",
+			digest:   "",
 		},
 		{
 			testName: "with digest",
-			value:    "nginx@12345",
+			value:    "nginx@sha256:12345",
+			name:     "nginx",
+			tag:      "",
+			digest:   "sha256:12345",
+		},
+		{
+			testName: "with tag and digest",
+			value:    "nginx:1.2.3@sha256:12345",
 			name:     "nginx",
-			tag:      "@12345",
+			tag:      "1.2.3",
+			digest:   "sha256:12345",
+		},
+		{
+			testName: "with domain",
+			value:    "docker.io/nginx:1.2.3",
+			name:     "docker.io/nginx",
+			tag:      "1.2.3",
+			digest:   "",
+		},
+		{
+			testName: "with domain and port",
+			value:    "foo.com:443/nginx:1.2.3",
+			name:     "foo.com:443/nginx",
+			tag:      "1.2.3",
+			digest:   "",
+		},
+		{
+			testName: "with domain, port, tag and digest",
+			value:    "foo.com:443/nginx:1.2.3@sha256:12345",
+			name:     "foo.com:443/nginx",
+			tag:      "1.2.3",
+			digest:   "sha256:12345",
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.testName, func(t *testing.T) {
-			name, tag := Split(tc.value)
+			name, tag, digest := Split(tc.value)
 			assert.Equal(t, tc.name, name)
 			assert.Equal(t, tc.tag, tag)
+			assert.Equal(t, tc.digest, digest)
 		})
 	}
 }