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 all 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
90 changes: 90 additions & 0 deletions api/filters/imagetag/imagetag_test.go
Expand Up @@ -767,6 +767,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 {
Expand Down
26 changes: 20 additions & 6 deletions api/filters/imagetag/updater.go
Expand Up @@ -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 != "" {
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved
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) {
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, 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)
})
}
}