Fix image name parsing with tag and digest #4406

cronik · 2022-01-19T22:38:46Z

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 project (see the release.yaml).

k8s-ci-robot · 2022-01-19T22:38:48Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

linux-foundation-easycla · 2022-01-19T22:38:48Z

The committers are authorized under a signed CLA.

✅ Kyle Cronin (678bc0c)

k8s-ci-robot · 2022-01-19T22:38:53Z

Welcome @cronik!

It looks like this is your first PR to kubernetes-sigs/kustomize 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kustomize has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-01-19T22:38:54Z

Hi @cronik. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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).

cronik · 2022-01-20T15:13:44Z

I signed it

jarias-lfx · 2022-01-20T15:42:03Z

/check-cla

natasha41575 · 2022-01-21T19:58:48Z

/ok-to-test

natasha41575 · 2022-01-21T20:02:44Z

api/image/image.go

@@ -24,24 +24,31 @@ func IsImageMatched(s, t string) bool {
 func Split(imageName string) (name string, tag string) {


Can we change this method to return three things - a name, tag, and digest? Below in one of the tests I see tag: :1.2.3@sha256:12345 which doesn't seem correct. We should differentiate the tag and digest where possible to help support cases where there are both.

natasha41575 · 2022-01-21T20:04:09Z

api/image/image_test.go

+			testName: "with domain",
+			value:    "docker.io/nginx:1.2.3",
+			name:     "docker.io/nginx",
+			tag:      ":1.2.3",


IMO it doesn't make sense to include the leading colon in the split. The tag is "1.2.3". Could you change that?

natasha41575 · 2022-01-21T20:05:07Z

api/image/image_test.go

 			name:     "nginx",
-			tag:      "@12345",
+			tag:      "@sha256:12345",


Aligned with the above suggestion to change the Split function to return the tag and digest separately, I'd like to see this test look like the following:

testName: "with digest", value: "nginx@sha256:12345", name: "nginx", digest: "sha256:12345"

k8s-ci-robot · 2022-01-22T17:39:39Z

@cronik: This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cronik · 2022-01-22T17:56:52Z

@natasha41575 updated Split to return digest and remove the delimiter prefixes from the returned values.

image.Split now returns 3 fields: name, tag, and digest. The tag and digest fields no longer include their respective delimiters (`:` and `@`).

cronik · 2022-01-26T00:38:49Z

@natasha41575 I had to make a few commit to resolve merge conflicts, may want to add the label tide/merge-method-squash

natasha41575

Mostly looking good, couple of minor comments

api/filters/imagetag/updater.go

api/image/image.go

natasha41575 · 2022-01-26T02:35:56Z

/lgtm

natasha41575 · 2022-02-16T16:49:14Z

/approve

k8s-ci-robot · 2022-02-16T16:49:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cronik, natasha41575

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [natasha41575]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 19, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 19, 2022

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 19, 2022

k8s-ci-robot requested review from KnVerey and natasha41575 January 19, 2022 22:39

cronik force-pushed the bugfix/image-tag-parsing branch from 678bc0c to 7a3c426 Compare January 20, 2022 14:55

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 20, 2022

cronik changed the title ~~Fix image name parsing with name and digest~~ Fix image name parsing with tag and digest Jan 21, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 21, 2022

natasha41575 reviewed Jan 21, 2022

View reviewed changes

Return digest property from image name parser

727093f

image.Split now returns 3 fields: name, tag, and digest. The tag and digest fields no longer include their respective delimiters (`:` and `@`).

cronik force-pushed the bugfix/image-tag-parsing branch from 286bb9f to 727093f Compare January 22, 2022 23:58

cronik and others added 2 commits January 25, 2022 19:23

Merge branch 'master' into bugfix/image-tag-parsing

a62a229

Fix merge file indentation

1bac439

natasha41575 reviewed Jan 26, 2022

View reviewed changes

api/filters/imagetag/updater.go Show resolved Hide resolved

api/image/image.go Show resolved Hide resolved

natasha41575 added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 26, 2022

Refactor imagetag updater string builder

11d585e

k8s-ci-robot assigned natasha41575 Jan 26, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 16, 2022

k8s-ci-robot merged commit 8dab949 into kubernetes-sigs:master Feb 16, 2022

natasha41575 mentioned this pull request Jul 15, 2022

kustomize edit set image should correctly parse both tag and digest #4713

Open

yuwenma mentioned this pull request Aug 29, 2022

feat: refactor set-image with go sdk GoogleContainerTools/kpt-functions-catalog#904

Merged

This was referenced Sep 7, 2022

bump of kustomize version from 4.4.1 to 4.5.X argoproj/argo-cd#10540

Closed

Upgrade Kustomize to >= 4.5.6 to mitigate CVE-2021-29923 and fix bugs argoproj/argo-cd#10533

Closed

obs-gh-danielmartin mentioned this pull request Apr 18, 2024

Using digest and newTag outputs both when docs indicate newTag is ignored #5664

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix image name parsing with tag and digest #4406

Fix image name parsing with tag and digest #4406

cronik commented Jan 19, 2022

k8s-ci-robot commented Jan 19, 2022

linux-foundation-easycla bot commented Jan 19, 2022 •

edited

k8s-ci-robot commented Jan 19, 2022

k8s-ci-robot commented Jan 19, 2022

cronik commented Jan 20, 2022

jarias-lfx commented Jan 20, 2022

natasha41575 commented Jan 21, 2022

natasha41575 Jan 21, 2022

natasha41575 Jan 21, 2022

natasha41575 Jan 21, 2022

k8s-ci-robot commented Jan 22, 2022

cronik commented Jan 22, 2022

cronik commented Jan 26, 2022

natasha41575 left a comment

natasha41575 commented Jan 26, 2022

natasha41575 commented Feb 16, 2022

k8s-ci-robot commented Feb 16, 2022

		@@ -24,24 +24,31 @@ func IsImageMatched(s, t string) bool {
		func Split(imageName string) (name string, tag string) {

Fix image name parsing with tag and digest #4406

Fix image name parsing with tag and digest #4406

Conversation

cronik commented Jan 19, 2022

k8s-ci-robot commented Jan 19, 2022

linux-foundation-easycla bot commented Jan 19, 2022 • edited

k8s-ci-robot commented Jan 19, 2022

k8s-ci-robot commented Jan 19, 2022

cronik commented Jan 20, 2022

jarias-lfx commented Jan 20, 2022

natasha41575 commented Jan 21, 2022

natasha41575 Jan 21, 2022

Choose a reason for hiding this comment

natasha41575 Jan 21, 2022

Choose a reason for hiding this comment

natasha41575 Jan 21, 2022

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 22, 2022

cronik commented Jan 22, 2022

cronik commented Jan 26, 2022

natasha41575 left a comment

Choose a reason for hiding this comment

natasha41575 commented Jan 26, 2022

natasha41575 commented Feb 16, 2022

k8s-ci-robot commented Feb 16, 2022

linux-foundation-easycla bot commented Jan 19, 2022 •

edited