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

Add option to build&push image on PR automatically #611

Merged
merged 9 commits into from Jun 21, 2022

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented Jun 6, 2022

ISSUE TYPE
  • Feature Pull Request
SUMMARY

Add an option to push the BotKube image automatically on PR. It's alternative approach for #604.

This PR will solve the problem with manual PR builds, e.g. we had that issue here:

Example run: https://github.com/mszostok/botkube/runs/6714112689?check_suite_focus=true

Fixes #590

To ensure that secrets won't be available for untrusted code, first we need to build the image and share it with the second job, which doesn't check out the untrusted code and can safely push an artifact to ghcr.io.

The flow is as follows:

Job1: image build -> image save -> artifact upload 
Job2: artifact download -> image load -> image push

Job1—runs untrusted code but without write repo perms
Job2—push built image with package write perms

Security

This article describes it well: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

@mszostok mszostok added enhancement New feature or request ci Related to CI pipelines labels Jun 6, 2022
@pkosiec pkosiec self-assigned this Jun 6, 2022
@pkosiec pkosiec self-requested a review June 6, 2022 12:35
Copy link
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

Just a few comments 🙂

hack/goreleaser.sh Outdated Show resolved Hide resolved
.github/workflows/pr-image.yaml Outdated Show resolved Hide resolved
.github/workflows/pr-image.yaml Outdated Show resolved Hide resolved
hack/goreleaser.sh Show resolved Hide resolved
hack/goreleaser.sh Show resolved Hide resolved
.github/workflows/pr-image.yaml Outdated Show resolved Hide resolved
Copy link
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

Please apply these two small comments and run the job again on your fork, just to confirm it works. We'll be ready to merge this (ofc if @PrasadG193 agrees with this approach) 👍

.github/workflows/pr-image.yaml Outdated Show resolved Hide resolved
.github/workflows/pr-image.yaml Outdated Show resolved Hide resolved
@pkosiec
Copy link
Member

pkosiec commented Jun 21, 2022

Hi @mszostok,

As #615 will require that job with image build, maybe let's merge this? Generally it's LGTM from my side, but I would like to see it running on fork with the applied improvements before merge.

Even after merge, we can apply any other suggestions if necessary 👍 We will refine the solution over time anyway.
Thanks!

@mszostok
Copy link
Contributor Author

mszostok commented Jun 21, 2022

Hi @pkosiec

Branch was rebased with the newest develop. I applied minor changes (628c1c7 5619734)

NOTE: second commit fixes problem reported by goreleaser:
⨯release failed after 0.03serror=failed to parse tag 'PR-6' as semver: Invalid Semantic Version
I prefer PR-{x} syntax but I had to go with v{x}-PR to solve the issue. Other option is to ignore the error on goreleaser side with --skip-validate.

The example run is here: https://github.com/mszostok/botkube/actions/runs/2534149331
To check locally, run: docker run ghcr.io/mszostok/pr/botkube:v6-PR

Copy link
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

LGTM - but please remove v prefix if possible, to have a bit nicer tag. Thanks!

@mergify mergify bot merged commit 67c3775 into kubeshop:develop Jun 21, 2022
@mszostok mszostok deleted the build-images-on-prs-v2 branch August 24, 2022 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to CI pipelines enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build Docker image on PRs
3 participants