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
Port the auto tag feature from docker plugin #36
Conversation
…/drone-docker The logic is forked from https://github.com/drone-plugins/drone-docker code base with necessary modification. I've tested it e2e for DockerHub on my Drone server via this plugin image https://hub.docker.com/repository/docker/15cm/drone-kaniko, for both tag pushes and commit pushes. With this change the .drone.yml in this repo should work as intended. Other changes: - Rename the existing "auto tag" flags/code to "expand tag" for a less misleading naming. - ATTENTION: make a breaking change to set default value of "--tags" to empty. Rationale is to expect most users to use the auto tagging feature. When power users want to specify tags, they should always explicitly set tags instead of being surprised by the default "latest" tag.
We cannot make any breaking changes until and unless they are security vulnerabilities. Auto tag was already supported in this plugin. Can you update what functionality was earlier missing for auto-tags? |
The existing auto tag feature behaves differently from the auto tag feature of the drone docker plugin. Comparison:
The reason I make breaking change on the AUTO_TAG flag instead of creating a new flag is that, the current auto tag is not "auto". I have to specify the tag, which means an update of .drome.yml per tag push. Moreover, as a user who comes from the drone docker plugin (I believe I'm not the only one) that supports the auto tag feature that behaves the same as this PR, I was surprised by the behavior of the current auto tag feature and struggled to see if it could auto detect the tag of git commits. In summary, naming the feature in this PR as AUTO_TAG:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@15cm Sounds good. I have added a comment to the PR.
The --auto-tag has to be a breaking change. This commit limit the breaking impact to the users who enable the flag. Behaviors of flag combination after this commit: * --auto-tag=false: No changes. * --auto-tag=false,--expand-tag=true,tags=1.0.0: * Old behavior: Should not happen. --expand-tag didn't exist. * New Behavior: Build with [1,1.0,1.0.0] tags. * --auto-tag=true * Old behavior: Build with the "latest" tag. * New behavior: Build with auto detected tags. Abort if auto detection failed. * --auto-tag=true,tags=latest: same as "--auto-tag=true". * --auto-tag=true,tags=1.0.0: * Old behavior: Build with [1,1.0,1.0.0] tags. * New behavior: Abort the build with an error message. * --auto-tag=true,--expand-tag=true,tags=1.0.0: Abort the build with an error message. Also added a test for the integration of the BUILD struct and the tagger package, which is used by kaniko.go.
Thanks a lot @15cm for your contribution. |
Thanks for taking care of this PR @shubham149. Sorry a link in the README added by this PR is malformatted. I created another PR as a minor fix #38. |
No worries. I have merged that PR. |
Support the feature in the "Autotag" section of docker plugin.
The logic is forked from https://github.com/drone-plugins/drone-docker code base with necessary modification. I've tested it e2e for DockerHub on my Drone server via this plugin image https://hub.docker.com/repository/docker/15cm/drone-kaniko, for both tag pushes and commit pushes.
With this change the .drone.yml in this repo should work as intended.
Other changes:
Rationale is to expect most users to use the auto tagging feature. When power users want to specify tags, they should always explicitly set tags instead of being surprised by the default "latest" tag.