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 PR preview deployments #5131

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Add PR preview deployments #5131

merged 2 commits into from
Sep 20, 2024

Conversation

lucasmerlin
Copy link
Collaborator

This adds preview deployments that will deploy a version of egui_demo_app for each pull request, making things easier to review / test.

Some notes on security:
The preview deployment is split in two workflows, preview_build and preview_deploy.
preview_build runs on pull_request, so it won't have any access to the repositories secrets, so it is safe to
build / execute untrusted code.
preview_deploy has access to the repositories secrets (so it can push to the pr preview repo) but won't run
any untrusted code (it will just extract the build artifact and push it to the pages branch where it will
automatically be deployed).

To set this up, a DEPLOY_KEY secret needs to be added, which allows the action to push the compiled artifacts into this repository: https://github.com/egui-pr-preview/pr
The deploy key is the private key part of a key generated via ssh-keygen. The public key is set as a deploy key in that repo.
I have created the repo on a separate github org, so it won't be directly associated with emil or egui in case someone pushes something naughty.

I have set this up in my fork of egui to show how this works:

  • I created a PR: Pr preview demo lucasmerlin/egui#2
  • The code will be compiled and pushed to the egui-pr-preview/pr repo and deployed via github pages
  • The bot leaves a comment on the pr with a link to the preview
  • The preview is available at https://egui-pr-preview.github.io/pr/2-pr-preview-demo/
    (It's unfortunately only available a couple seconds after the bot writes the comment, because the pages deployment action is run independently on the other repository)
  • Once the PR is merged / closed the preview will be cleaned up (unfortunately the empty folder will remain, it seems like it's not possible to remove that via the JamesIves/github-pages-deploy-action action I use, but I don't think that it's a big issue)

I'll leave the PR in draft until the DEPLOY_KEY is set up

@lucasmerlin lucasmerlin added the CI Continues Integration test label Sep 19, 2024
@lucasmerlin lucasmerlin marked this pull request as ready for review September 19, 2024 12:02
Comment on lines +6 to +8
# `preview_deploy` has access to the repositories secrets (so it can push to the pr preview repo) but won't run
# any untrusted code (it will just extract the build artifact and push it to the pages branch where it will
# automatically be deployed).
Copy link
Owner

Choose a reason for hiding this comment

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

Very nice!

Copy link
Owner

Choose a reason for hiding this comment

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

…but I guess it doesn't prevent a malicious user from simply changing the .yml in the PR.

But there should be some way to configure it so that it is always the .yml from master that is run? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The pull_request trigger will run workflows from the pr branch, so yes, they could modify the yml. But the workflow won't get any access to secrets and will only get a read only GITHUB_TOKEN, so it wouldn't be a problem.

Worst thing they could do is change the output to meta.json to point at a different branch and overwrite someone elses preview deployment that way 🤔

It's possible to run the workflow from master via pull_request_target but then it would be unsafe to build the untrusted code since workflows using that have access to secrets.

target-folder: ${{ env.URL_SLUG }}
ssh-key: ${{ secrets.DEPLOY_KEY }}
commit-message: "Remove preview for PR ${{ env.URL_SLUG }}"
single-commit: true
Copy link
Owner

Choose a reason for hiding this comment

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

nit: hot tip is to configure your editor to

A) ensure a final newline on save
B) trim trailing whitespace on save

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we can add a https://editorconfig.org/ to egui so everyone has the same config

@emilk emilk merged commit 0f290b4 into master Sep 20, 2024
40 checks passed
@emilk emilk deleted the lucas/pr-preview-deployment branch September 20, 2024 07:08
@Spiral-Memory
Copy link

Hey @lucasmerlin ,

Can we set it up so that a PR is deployed only if it has been approved? We are currently using a similar build workflow that triggers with pull_request instead of pull_request_target and uploads artifacts.

Then, with on workflow_run, we'll deploy to the same repository under the gh-pages branch. This will work fine, but we want to introduce another safety check: the deployment should only happen if a maintainer or collaborator has approved the changes. I think on workflow_run does have write permissions, but the pull_request_review trigger doesn't have write permissions.

Can we do that? If so, please let me know.

@lucasmerlin
Copy link
Collaborator Author

Hi @Spiral-Memory, I think this should be possible somehow but I haven't looked into that. In the workflow_run you should be able check whether the PR has been approved via the API and you should also be able to trigger the workflow run task via a pull_request_review task. I think this way you should be able to wire this up.

@Spiral-Memory
Copy link

Yep, i was thinking about the same approach, thanks for your help / confirmation

I'll try implementing that and let you know if it works

Thanks !

hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
This adds preview deployments that will deploy a version of
egui_demo_app for each pull request, making things easier to review /
test.

Some notes on security:
The preview deployment is split in two workflows, preview_build and
preview_deploy.
`preview_build` runs on pull_request, so it won't have any access to the
repositories secrets, so it is safe to
build / execute untrusted code.
`preview_deploy` has access to the repositories secrets (so it can push
to the pr preview repo) but won't run
any untrusted code (it will just extract the build artifact and push it
to the pages branch where it will
automatically be deployed).

To set this up, a DEPLOY_KEY secret needs to be added, which allows the
action to push the compiled artifacts into this repository:
https://github.com/egui-pr-preview/pr
The deploy key is the private key part of a key generated via
ssh-keygen. The public key is set as a deploy key in that repo.
I have created the repo on a separate github org, so it won't be
directly associated with emil or egui in case someone pushes something
naughty.

I have set this up in my fork of egui to show how this works:
- I created a PR: lucasmerlin#2
- The code will be compiled and pushed to the egui-pr-preview/pr repo
and deployed via github pages
- The bot leaves a comment on the pr with a link to the preview
- The preview is available at
https://egui-pr-preview.github.io/pr/2-pr-preview-demo/
(It's unfortunately only available a couple seconds after the bot writes
the comment, because the pages deployment action is run independently on
the other repository)
- Once the PR is merged / closed the preview will be cleaned up
(unfortunately the empty folder will remain, it seems like it's not
possible to remove that via the JamesIves/github-pages-deploy-action
action I use, but I don't think that it's a big issue)

I'll leave the PR in draft until the DEPLOY_KEY is set up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continues Integration test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants