Add handling of tags in docker-helper #12943
Conversation
Co-authored-by: Daniel Fangl <daniel.fangl@localstack.cloud>
Helper Script Tests30 tests 30 ✅ 0s ⏱️ Results for commit 9badae1. ♻️ This comment has been updated with latest results. |
Co-authored-by: Daniel Fangl <daniel.fangl@localstack.cloud>
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 20m 4s ⏱️ Results for commit 9badae1. ♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files 2 suites 1h 44m 29s ⏱️ Results for commit 9badae1. ♻️ This comment has been updated with latest results. |
tests/bin/test.docker-helper.bats
Outdated
| export PLATFORM=amd64 | ||
| export TEST_TAG=v1.0.0 | ||
| run bin/docker-helper.sh push | ||
| echo "$output" |
There was a problem hiding this comment.
suggestion: Seems like a leftover from testing?
| @@ -65,15 +65,21 @@ function _get_current_branch() { | |||
| git branch --show-current | |||
| } | |||
|
|
|||
| function _get_current_tag() { | |||
| git describe --tags --exact-match 2> /dev/null || true | |||
There was a problem hiding this comment.
question: Would it make sense to check that this really is a release tag? Is there a scenario where this can slip through and enable the push for something we do not want?
There was a problem hiding this comment.
I think this also leaves a bigger question: Should we allow any tags which are not release tags in the repository at all? This would be a case for the repository protection rules, then.
There was a problem hiding this comment.
That's a good point! But since a few weeks we have branch protection rules in place in the main emulator repos. Only the release action / bot is allowed to create any tags.
Are you suggesting to make the rule even stricter such that the release action is only allowed to push version compliant tag names?
There was a problem hiding this comment.
It would make sense in my opinion, even though it is unlikely that the release action starts to push random tags. I think a ruleset:
- Allowing
v\d+\.\d+\.\d+(not sure of the regex flavor for github, please don't quote me on it) tags only for the release action / bot - Disallowing all other tag pushes
Would make sense. We could also add another check in the docker helper, to feel more safe, but I don't think it is necessary.
There was a problem hiding this comment.
I just want to add here that the pipeline which will use the docker-helper to push the images (aws-main.yml) can only be triggered by tags of this shape:
tags:
- 'v[0-9]+.[0-9]+.[0-9]+'
There was a problem hiding this comment.
I've also added functionality to the docker helper now to refuse using tags without this shape and a test for it
There was a problem hiding this comment.
thought: That's awesome, thanks! Great improvement and cleanup! One follow up question: What happens if the commit is tagged twice?
There was a problem hiding this comment.
I'd assume the workflow would be triggered once for each tag. I can explore that
There was a problem hiding this comment.
Thanks! Would also be very interesting how this function behaves for the two tags. :)
Co-authored-by: Alex Rashed <2796604+alexrashed@users.noreply.github.com>
| @@ -65,15 +65,21 @@ function _get_current_branch() { | |||
| git branch --show-current | |||
| } | |||
|
|
|||
| function _get_current_tag() { | |||
| git describe --tags --exact-match 2> /dev/null || true | |||
There was a problem hiding this comment.
thought: That's awesome, thanks! Great improvement and cleanup! One follow up question: What happens if the commit is tagged twice?
Motivation
With the latest release we still had issues with pushing docker images on pipeline runs triggered by the tag push.
This is because the docker helper assumes that only the
mainbranch should be allowed for pushing the images. Since tags are not bound to a branch, a tag-triggered run will not have a branch ref.We need to add extra provisions for allowing tagged commits to also push images.
/cc @dfangl (and thanks for providing the patch files)
Changes
Testing
To avoid further issues we'll do a run in a forked repository where we push into a different DockerHub account.
TODO
What's left to do: