From 8dab94964f7a3e4e9700ba9bd3eaba333c29be37 Mon Sep 17 00:00:00 2001 From: Kyle Cronin Date: Wed, 16 Feb 2022 12:02:37 -0500 Subject: [PATCH] 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 --- api/filters/imagetag/imagetag_test.go | 90 +++++++++++++++++++++++++++ api/filters/imagetag/updater.go | 28 ++++++--- api/image/image.go | 58 ++++++++++------- api/image/image_test.go | 55 ++++++++++++++-- 4 files changed, 198 insertions(+), 33 deletions(-) diff --git a/api/filters/imagetag/imagetag_test.go b/api/filters/imagetag/imagetag_test.go index d1f0bc1de..98051741b 100644 --- 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 index 50c0dcdc8..45c6c748d 100644 --- 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 - } - if u.ImageTag.Digest != "" { - tag = "@" + u.ImageTag.Digest + + // 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 } - return u.trackableSetter.SetScalar(name + tag)(rn) + // build final image name + if tag != "" { + name += ":" + tag + } + if digest != "" { + name += "@" + digest + } + + 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 index 059999062..4a88050b4 100644 --- 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 - } - } - ia := strings.LastIndex(imageName, "@") - if ic < 0 && ia < 0 { - return imageName, "" + slashIndex = 0 } - i := ic - if ia > 0 { - i = ia + id := strings.Index(searchName, "@") + ic := strings.Index(searchName, ":") + + // no tag or digest + if ic < 0 && id < 0 { + return imageName, "", "" } - name = imageName[:i] - tag = imageName[i:] - return + // digest only + if id >= 0 && (id < ic || ic < 0) { + id += slashIndex + name = imageName[:id] + digest = strings.TrimPrefix(imageName[id:], "@") + return name, "", digest + } + + // 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 + } + + // 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 index c3526490e..9c3cd3b0a 100644 --- 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: "@12345", + tag: "", + digest: "sha256:12345", + }, + { + testName: "with tag and digest", + value: "nginx:1.2.3@sha256:12345", + name: "nginx", + 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) }) } }