Move image pushing into CircleCI #945
Conversation
- Moves/updates the travis deploy script to work on circleCI. - Added DOCKERHUB_TOKEN via web UI to circleCI - runs publishing of images on master branch and on tags like v.* - minor change to naming to clarify job purposes Ref: #926 Signed-off-by: John Schnake <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #945 +/- ##
=======================================
Coverage 47.78% 47.78%
=======================================
Files 76 76
Lines 5226 5226
=======================================
Hits 2497 2497
Misses 2573 2573
Partials 156 156Continue to review full report at Codecov.
|
| - build_and_test | ||
| filters: | ||
| branches: | ||
| only: master |
There was a problem hiding this comment.
I was reading about the filters earlier and came across this post: https://discuss.circleci.com/t/dont-run-deploy-workflow-step-for-forked-pr/23606
It suggests that a PR from the master branch of a fork would pass this filter. We might want to consider expanding the check in the publish script to check for one of the environment variables that are only set for forked PRs: https://circleci.com/docs/2.0/env-vars/#built-in-environment-variables
What do you think?
There was a problem hiding this comment.
Great question.
So you're right on the approach (using env vars) and we do this though by checking the CIRCLE_BRANCH. For pull requests it is pull/945 or whatnot.
However, as I was typing my response I realized that for the tagged portion of the bash script we could hit a problem. E.g. If I make a PR that includes a tag on my commit I may be able to trigger that part of the build.
We can make this check a bit more verbose by checking for another env var value about the PR, but we should acknowledge that a malicious user could still manipulate the scripts/env vars to trigger these. I dont know if we can prevent that.
There was a problem hiding this comment.
I just checked the CIRCLE_BRANCH in a test PR and you're right, it sets the branch correctly to be a pull request branch:
#!/bin/bash -eo pipefail
echo $CIRCLE_BRANCH
echo $CIRCLE_PR_NUMBER
echo $CIRCLE_PR_REPONAME
echo $CIRCLE_PR_USERNAME
pull/946
946
sonobuoy
zubron
My concern was that CIRCLE_BRANCH would be set to the source branch, which could be master.
I'm not sure if there is anything that we can do to prevent malicious PRs. But, given that we've prevented the secrets being passed to forked PRs, they could bypass the checks but the steps shouldn't work because the secrets aren't available.
There was a problem hiding this comment.
Yes, excellent point. The config settings in the UI prevent image pushing from ever actually occurring.
What this PR does / why we need it:
Which issue(s) this PR fixes
Ref: #926
Special notes for your reviewer:
I tried to use circleCI on my own repo to show this behavior to some extent. You can see here: https://circleci.com/workflow-run/9fdeb6cc-04ae-4e11-9173-bc2f06c14f40 that the publishing waits for the tests to pass. In addition, you can see https://circleci.com/workflow-run/9a9a49a3-5279-40ab-8638-9803a92063e1 doesn't show that the publishing would have occurred at all since it wasn't on the master branch.
Release note: