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: Invalid image reference format when using digest strategy with helm charts #317

Merged
merged 3 commits into from
Dec 16, 2021
Merged

Conversation

agorgl
Copy link
Contributor

@agorgl agorgl commented Dec 12, 2021

Fixes #259 #210

When using digest update strategy, the helm image.tag parameter now composes a full tag of both the tag and the digest in the form:

sometag@sha256:<somelonghashstring>

This avoids invalid references in almost all helm charts that construct the image ref as image.repository:image.tag, that so far resulted to the invalid references of the form:

some/image:sha256:<somelonghashstring>

…arts

Signed-off-by: Agorgianitis Loukas <agorglouk@gmail.com>
@CLAassistant
Copy link

CLAassistant commented Dec 12, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, @elartista - this is useful!

I only have one cosmetic concern, please see below.

Also, can you please provide unit tests for the method?

pkg/image/image.go Outdated Show resolved Hide resolved
@agorgl
Copy link
Contributor Author

agorgl commented Dec 16, 2021

Added unit tests, I believe this is now ready!

Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks a lot @elartista !

@jannfis jannfis merged commit f1ebca5 into argoproj-labs:master Dec 16, 2021
@dwi-mfbcs
Copy link

Waiting for this fix to be released. Do we have an ETA for a release that will include this fix? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Digest is not working (Digest, Tag, Helm)
4 participants