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

Throw error with explicit latest tag #384

Merged
merged 25 commits into from
Mar 17, 2021
Merged

Conversation

faust64
Copy link
Contributor

@faust64 faust64 commented Mar 5, 2021

following-up on #360

latest tag handling.

Fixes #321

Copy link
Contributor

@jakubhajek jakubhajek left a comment

Choose a reason for hiding this comment

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

I tested that change locally, no issues occurred.

@SantoDE
Copy link
Contributor

SantoDE commented Mar 10, 2021

@faust64 what do you think of the comment in #321? Do you think that would be sufficient? :)

@faust64
Copy link
Contributor Author

faust64 commented Mar 10, 2021

Sure, one should never use latest tag. At least when you're not pulling from a registry that expunges older images, or some rare exceptions when you need some missing feature that's not in an official release, ... doesn't really apply here.
Then again, good practices are rarely observed.
Another take on it may be to throw an error instead.

@SantoDE
Copy link
Contributor

SantoDE commented Mar 10, 2021

Like to throw an error if you are trying to run it with latest? I would like that :)

@faust64
Copy link
Contributor Author

faust64 commented Mar 10, 2021

allright, that should do it

Copy link
Contributor

@jakubhajek jakubhajek left a comment

Choose a reason for hiding this comment

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

It works as expected preventing a user to deploy the latest tag.

@ldez ldez removed the bot/no-merge label Mar 17, 2021
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez changed the title fix(tags): handle non-semvers tag Throw error with explicit latest tag Mar 17, 2021
@traefiker traefiker merged commit 4110682 into traefik:master Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ingressclass template causes chart install to fail when using non-semver values in image.tag
5 participants