Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image name parsing with tag and digest #4406

Merged
merged 5 commits into from Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 61 additions & 1 deletion api/filters/imagetag/imagetag_test.go
Expand Up @@ -679,7 +679,7 @@ spec:
},
},
},
"mutation tracker": {
"mutation tracker": {
input: `
group: apps
apiVersion: v1
Expand Down Expand Up @@ -767,6 +767,66 @@ spec:
},
},
},
"image with 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.2.1@sha256:46d5b90a7f4e9996351ad893a26bcbd27216676ad4d5316088ce351fb2c2c3dd
`,
filter: Filter{
ImageTag: types.Image{
Name: "nginx",
NewName: "apache",
},
},
fsSlice: []types.FieldSpec{
{
Path: "spec/image",
},
},
},
"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 {
Expand Down
19 changes: 15 additions & 4 deletions api/filters/imagetag/updater.go
Expand Up @@ -30,14 +30,25 @@ 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
// not overriding tag/digest component, keep original
if u.ImageTag.NewTag == "" && u.ImageTag.Digest == "" {
if tag != "" {
tag = ":" + tag
}
if digest != "" {
tag += "@" + digest
}
}
if 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 + "@" + u.ImageTag.Digest
} else if u.ImageTag.NewTag != "" {
tag = ":" + u.ImageTag.NewTag
} else if u.ImageTag.Digest != "" {
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved
tag = "@" + u.ImageTag.Digest
}

Expand Down
56 changes: 36 additions & 20 deletions api/image/image.go
Expand Up @@ -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
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved
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, ""
}
55 changes: 50 additions & 5 deletions api/image/image_test.go
Expand Up @@ -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",
Expand All @@ -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, 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)
})
}
}