Support Previewing PR using Surge.sh #1487

raysonkoh · 2021-02-24T09:13:43Z

What is the purpose of this pull request?

Related to #1466

Currently, we have provided instructions for deploying PR previews using Netlify. However, as Netlify have a limited amount of free build minutes, there may be other users who are thinking of using other PR preview services. This PR aims to add instructions for previewing PR using Surge which has unlimited build minutes as of now.

Overview of changes:

Add 2 new workflows for previewing PR on Surge.

The first workflow is "Receive Markbind PR" which builds the markbind site and uploads it as an Artifact.

The next workflow is "Deploy PR Preview", which is only triggered upon successful completion of the first workflow and it deploys the Markbind site to Surge.sh using with the link as such: https://pr-[PR_NUMBER]-[REPO_SLUG].surge.sh. At the end of this workflow, a github-actions bot will auto-comment the link of the PR preview for easier accessibility for the PR reviewer.

Anything you'd like to highlight / discuss:
Some relevant context: reposense/RepoSense#1105, reposense/RepoSense#1099 , reposense/RepoSense#1411, reposense/RepoSense#1434

The main reason for having 2 separate workflows is due to security reasons. Github-Actions does not expose repo secrets for workflows that are triggered by PRs from a forked repo, link to docs. Based on my research so far, there are four possible ways to get around this.

Using the pull_request_target event, the workflow that are triggered by PRs are able to get access to repo secrets. More info in the github release doc. However, by design, it does not run the code in the PR as it may be malicious. One workaround that people have found would be to use pull_request_target and then checking out the HEAD of the PR. However, this poses a serious security risk as the secrets would be leaked.
Using the workflow_run event, together with a separate initial workflow which is then used to trigger the main workflow. This is the approach that Reposense is currently using and the approach that I adopted for this PR. I feel that this is much more secure than the first option as we can control which section of the PR code to extract to the main workflow (that is deploys the Markbind site to Surge).
Ensure every user sending a PR from their forked repo has their own SURGE_TOKEN
Merging the PR into a separate branch in the main repo, and sending a PR from that branch to master.

Option 2 is also recommended by Github here: https://securitylab.github.com/research/github-actions-preventing-pwn-requests. Option 3 and 4 may be too tedious and inconvenient.

Testing instructions:

Create a Surge account by first installing Surge using npm install --global surge and typing surge on the terminal
Next, get type surge token to generate your surge token.
In your markbind site repo, create a new secret by going to "Settings"->"Secrets" and name the key as SURGE_TOKEN and set the value as the generated surge token.
Commit and push the following 2 files into your markbind site repo, in the directory ./github/workflows/.

receivePR.yml

name: Receive Markbind PR

# read-only
# no access to secrets
on:
  pull_request:

jobs:
  build:
    runs-on: ubuntu-latest
    steps:        
      - uses: actions/checkout@v2
      - name: Install Node
        uses: actions/setup-node@v2
        with:
          node-version: 10
      - name: Build Markbind website
        if: ${{ success() && github.event_name == 'pull_request' }}
        run: |
          npm install -g markbind-cli
          markbind build
      - name: Save PR number and HEAD commit
        if: ${{ success() && github.event_name == 'pull_request' }}
        run: |
          mkdir -p ./pr
          echo ${{ github.event.number }} > ./pr/NUMBER
          echo ${{ github.event.pull_request.head.sha }} > ./pr/SHA
      - name: Upload artifacts 
        if: ${{ success() && github.event_name == 'pull_request' }}
        uses: actions/upload-artifact@v2
        with:
          name: markbind-deployment
          path: |
            ./_site
            ./pr

previewPR.yml

name: Deploy PR Preview

# Runs after PR is received and build by markbind-cli
# Has access to repo secrets
on:
  workflow_run:
    workflows: ["Receive Markbind PR"]
    types:
      - completed

jobs:
  build:
    runs-on: ubuntu-latest
    name: Deploying to surge
    steps:
      - uses: actions/checkout@v2
      - name: Download artifact
        uses: dawidd6/action-download-artifact@v2
        with:
          workflow: receivePR.yml
          run_id: ${{ github.event.workflow_run.id }}
          name: markbind-deployment
          path: .
      - name: Display structure of downloaded files
        run: ls -R
      - name: Extract PR number
        id: pr-number
        run: echo '::set-output name=ACTIONS_PR_NUMBER::'$(cat ./pr/NUMBER)
      - name: Install Node
        uses: actions/setup-node@v2
        with:
          node-version: 10
      - name: Inject slug/short variables
        uses: rlespinasse/github-slug-action@v3.x
      - name: Build PR preview url
        id: pr-url
        run: echo '::set-output name=ACTIONS_PREVIEW_URL::'https://pr-${{ steps.pr-number.outputs.ACTIONS_PR_NUMBER }}-${{ env.GITHUB_REPOSITORY_SLUG_URL }}.surge.sh/
      - name: Install surge and deploy PR to surge
        run: |
          npm i -g surge
          surge --project ./_site --domain ${{ steps.pr-url.outputs.ACTIONS_PREVIEW_URL }}
        env:
          SURGE_TOKEN: ${{ secrets.SURGE_TOKEN }}
      - name: Find PR preview link comment
        uses: peter-evans/find-comment@v1
        id: fc
        with:
          issue-number: ${{ steps.pr-number.outputs.ACTIONS_PR_NUMBER }}
          body-includes: Your PR can be previewed
      - name: Comment PR preview link in PR
        if: ${{ steps.fc.outputs.comment-id == 0 }}
        uses: peter-evans/create-or-update-comment@v1
        with:
          issue-number: ${{ steps.pr-number.outputs.ACTIONS_PR_NUMBER }}
          body: |
            Thank you for submitting the Pull Request! :thumbsup: 

            Your PR can be previewed [here](${{ steps.pr-url.outputs.ACTIONS_PREVIEW_URL }})

Test the workflow by creating PRs from a branch and from a forked repo, if everything goes well, the github-actions bot should auto-comment the link of the PR preview and the link should render the updated Markbind site.

Showcase:

Proposed commit message: (wrap lines at 72 characters)

Add support for PR previews using Surge

Netlify's deploy preview feature may not be suitable for certain
Markbind users due to its limited build minutes.

Expanding support for PR previews via Surge would cater to such
users due to its unlimited build minutes.

Let's add instructions in the userGuide for setting up PR
previews with Surge.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No blatantly unrelated changes
Pinged someone for a review!

.github/workflows/receivePR.yml

ang-zeyu · 2021-02-24T09:50:07Z

Thanks a lot for the work here! @raysonkoh

Could I double check the resolution in the issue was per @damithc's comment here?

i.e. we're not actually going to switch to surge for this repo, just provide instructions / pointers (documentation) for ways of doing pr previews, which sounds good.

if this pr is meant as a poc toward those documentation changes / some other purpose however please ignore this 😅

raysonkoh · 2021-02-24T09:59:16Z

if this pr is meant as a poc toward those documentation changes / some other purpose however please ignore this 😅

Thanks for the comment @ang-zeyu Yes, I was thinking along this line.

Would this be the right approach or do you think updating the userGuide with the instructions is enough?

ang-zeyu · 2021-02-24T10:20:26Z

Thanks for the comment @ang-zeyu Yes, I was thinking along this line.

Would this be the right approach or do you think updating the userGuide with the instructions is enough?

The example here looks really robust, and can serve as a good accompanying example somewhere.

I was thinking of adding a separate supporting top level section / page first (something like Deployment for PR previews?) under https://markbind-master.netlify.app/userguide/deployingthesite?

Something along this structure:

general pointers / things to take note for pr previews with MarkBind, if any
- pr previews with surge (this pr)
- netlify
- possibly github actions once we have Support site versioning #1009 done

On those lines since we only have instructions for deploying to github pages, netlify, a prior pr for a deploying to surge guide (in non-pr preview context) based on the first workflow here could be done as well to reduce the content repitition.

What do you think? @raysonkoh

raysonkoh · 2021-02-24T16:53:20Z

Thanks for the suggestion! Just to clarify, you are suggesting to have another PR for adding instructions for deploying a Markbind site to Surge? Actually the steps are roughly the same, with an additional step of adding a CNAME file to the root of the repo. Is that what you mean by reducing content repetition?

Based on what you said, I think a good page structure for deployingTheSite would be:

Deploying to Github Pages
1. Using the markbind deploy command
2. Using CI tools
  1. Github Actions
  2. Travis CI
  3. Appveyor CI
  4. Circle CI
Deploying to Netlify
Previewing PRs for Markbind sites
1. Using Netlify's Deploy Previews feature
2. Using Surge.sh

What do you think @ang-zeyu ?

ang-zeyu · 2021-02-24T17:16:59Z

Actually the steps are roughly the same, with an additional step of adding a CNAME file to the root of the repo. Is that what you mean by reducing content repetition?

I suppose so. I'm not too familiar with Surge however, I'll leave you to judge whether its worth it here 😅

Deploying to Netlify

Deploying to Surge (added)

Previewing PRs for Markbind sites

Using Netlify's Deploy Previews feature

Using Surge.sh

With the above addition*, if it makes sense

raysonkoh · 2021-02-25T00:08:40Z

Actually the steps are roughly the same, with an additional step of adding a CNAME file to the root of the repo. Is that what you mean by reducing content repetition?

I suppose so. I'm not too familiar with Surge however, I'll leave you to judge whether its worth it here 😅

Deploying to Netlify

Deploying to Surge (added)

Previewing PRs for Markbind sites

Using Netlify's Deploy Previews feature

Using Surge.sh

With the above addition*, if it makes sense

Actually, I feel that we can put this (including a section for Deploying a Markbind site to Surge for production) aside for now, because there isn't really anything that Surge provides that Github Pages doesn't provide and setting up Surge is more troublesome than setting up Github Pages. Cost-wise, both services are free also.

raysonkoh · 2021-02-25T00:10:53Z

I will make a separate PR to reorganize the page structure of deployingTheSite first, so that the changes for this PR will solely be adding instructions for previewing PR using Surge.

jonahtanjz

Good work on this :) The instructions are clear and straightforward. Managed to set up my surge preview very quickly following the steps 👍. Just two minor nits and the rest looks good.

docs/userGuide/deployingTheSite.md

jonahtanjz · 2021-03-01T07:03:21Z

docs/userGuide/deployingTheSite.md

+
+    <include src="screenshot.md" boilerplate var-alt="Create Surge account" var-file="surgeCreateAccount.png" inline />
+
+1. Proceed to create a Surge account.


Perhaps can also include instruction to inform the user to press 'Enter' to proceed to use the default values of project and domain when setting up the account? New users may be confused by this step.

Actually this part is not needed - the user can just CTRL-C out of this process (after logging in / creating an account). I have added a clarification in the userGuide on this. Could you help to check to see if it is clearer?

Actually this part is not needed - the user can just CTRL-C out of this process (after logging in / creating an account).

Ahh I see. Yup it's much clearer now 👍 Thanks for updating :)

- Fix gh-action workflow directory path - Make the steps in setting up Surge clearer

ang-zeyu

Nice work with the docs! @raysonkoh

The main reason for having 2 separate workflows is due to security reasons. Github-Actions does not expose repo secrets for workflows that are triggered by PRs from a forked repo, link to docs. Based on my research so far, there are four possible ways to get around this.

Agree with the approach here, thanks for the research.

Just a few suggestions on the workflow examples:

docs/userGuide/deployingTheSite.md

ang-zeyu · 2021-03-05T14:52:44Z

docs/userGuide/deployingTheSite.md

+          run_id: ${{ github.event.workflow_run.id }}
+          name: markbind-deployment
+          path: .
+      - name: Display structure of downloaded files


should we remove this step? since devs can include it as needed if preferred

Yup make sense, will remove it

docs/userGuide/deployingTheSite.md

ang-zeyu · 2021-03-05T14:58:12Z

docs/userGuide/deployingTheSite.md

+        uses: rlespinasse/github-slug-action@v3.x
+      - name: Build PR preview url
+        id: pr-url
+        run: echo '::set-output name=ACTIONS_PREVIEW_URL::'https://pr-${{ steps.pr-number.outputs.ACTIONS_PR_NUMBER }}-${{ env.GITHUB_REPOSITORY_SLUG_URL }}.surge.sh/


should we remove rlespinasse/github-slug-action@v3.x and simplify the surge url?

also a small comment / note to change this as appropriate, since the url is project-dependent

Hmm I think that if we remove rlespinasse/github-slug-action@v3.x, it will add some more complexity since there needs to be some regex script to extract the repoSlug. I am not sure if there is a way to simplify the surge url while ensuring that it is still as general as possible.

I can add a note to say that the user can make modifications to the surge URL as needed to a simpler one, together with a short code snippet showing the relevant changes. What do you think @ang-zeyu ?

not too familiar with surge's specifics - is ${{ env.GITHUB_REPOSITORY_SLUG_URL }} necessary / convention for pr previewing in surge?

my view is that this could be easily configured to something unique if the user wants to (not limited to owner + repo format).
i.e. just https://pr-${{ steps.pr-number.outputs.ACTIONS_PR_NUMBER }}-xxxx-anything-xxxx.surge.sh.
Would that work?

One way is that the user can have a SURGE_URL secret, so the PR URL would be something like https://pr-${{ steps.pr-number.outputs.ACTIONS_PR_NUMBER }}-${{ secrets.SURGE_URL }}.surge.sh.

This would eliminate the need for rlespinasse/github-slug-action@v3.x in the code. However, this would be slightly more inconvenient for the user.

What do you think about this approach?

Hmm was thinking of just highlighting that the specific line in the .yml should be edited per the project as needed before committing. (comment / warning box / highlight)
Since folks who are using pull requests, github actions, github likely have the know how =P

Okay I have added in the userGuide a new variable PR_URL which the user can modify. I have also added a note to highlight to the user that they have to edit the PR_URL according to their needs.

ang-zeyu · 2021-03-05T15:05:27Z

docs/userGuide/deployingTheSite.md

-Additionally, **when contributors make a pull request to your GitHub repo, you can _preview_ the updated site** at the bottom of the pull request by clicking on `details` link in the PR:
+## Previewing Pull Requests for MarkBind sites
+
+There may be more than one user making changes to your MarkBind site. In such cases, it might be prudent to have a workflow that is centered around pull requests (PRs) such as a [feature branch workflow](https://www.atlassian.com/git/tutorials/comparing-workflows/feature-branch-workflow) or a [forking workflow](https://www.atlassian.com/git/tutorials/comparing-workflows/forking-workflow). 


Let's preface a little bit more who this section is for (users having MarkBind projects on github)

Okay I have added an info box below ## Previewing Pull Requests for MarkBind sites

Co-authored-by: Ang Ze Yu <angzeyu@gmail.com>

ang-zeyu

thanks! one last nit:

ang-zeyu · 2021-03-08T11:39:00Z

docs/userGuide/deployingTheSite.md

+
+<box type="info">
+
+This section is meant for users who are $$hosting their Markbind project on **Github**$$.


let's keep this to 2nd person for consistency

Okay, I have updated this to 2nd person

ang-zeyu

Lgtm 👍

Add initial PR preview workflow

c4e1577

raysonkoh commented Feb 24, 2021

View reviewed changes

.github/workflows/receivePR.yml Outdated Show resolved Hide resolved

raysonkoh added 2 commits February 25, 2021 10:25

Fix markbind deploy cli reference

15cd96b

Reorganize deployingTheSite.md

504add8

raysonkoh mentioned this pull request Feb 25, 2021

Reorganize deployingTheSite page #1489

Merged

10 tasks

raysonkoh added 5 commits February 25, 2021 11:01

Merge branch 'fix-1487' into fix-1466-surge

b7d334f

Add instructions for Surge PR previews

4fb09cc

Merge branch 'master' into fix-1466-surge

8a999a4

Fix grammer

533d82f

Remove poc gh-action workflows

312ed43

raysonkoh marked this pull request as ready for review March 1, 2021 05:52

raysonkoh changed the title ~~[WIP] Support Previewing PR using Surge.sh~~ Support Previewing PR using Surge.sh Mar 1, 2021

Fix indentation

0a6c0fe

jonahtanjz reviewed Mar 1, 2021

View reviewed changes

raysonkoh added 2 commits March 2, 2021 09:00

Update according to PR comments

b1c7e5e

- Fix gh-action workflow directory path - Make the steps in setting up Surge clearer

Remove code block line-numbers

cb68477

ang-zeyu reviewed Mar 5, 2021

View reviewed changes

raysonkoh and others added 5 commits March 7, 2021 13:31

Update docs/userGuide/deployingTheSite.md

7953704

Co-authored-by: Ang Ze Yu <angzeyu@gmail.com>

Update docs/userGuide/deployingTheSite.md

d3bf2c5

Co-authored-by: Ang Ze Yu <angzeyu@gmail.com>

Update according to PR comments

b20d671

Add targeted user info for PR preview

5b87233

Update according to PR comments

5b5591c

ang-zeyu approved these changes Mar 8, 2021

View reviewed changes

Update according to PR comments

f015421

ang-zeyu approved these changes Mar 8, 2021

View reviewed changes

ang-zeyu added this to the v3.0 milestone Mar 8, 2021

ang-zeyu merged commit 1ecbf9a into MarkBind:master Mar 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Previewing PR using Surge.sh #1487

Support Previewing PR using Surge.sh #1487

raysonkoh commented Feb 24, 2021 •

edited

Loading

ang-zeyu commented Feb 24, 2021

raysonkoh commented Feb 24, 2021 •

edited

Loading

ang-zeyu commented Feb 24, 2021

raysonkoh commented Feb 24, 2021

ang-zeyu commented Feb 24, 2021

raysonkoh commented Feb 25, 2021

raysonkoh commented Feb 25, 2021

jonahtanjz left a comment

jonahtanjz Mar 1, 2021 •

edited

Loading

raysonkoh Mar 2, 2021 •

edited

Loading

jonahtanjz Mar 2, 2021

ang-zeyu left a comment

ang-zeyu Mar 5, 2021

raysonkoh Mar 7, 2021

ang-zeyu Mar 5, 2021

raysonkoh Mar 7, 2021

ang-zeyu Mar 7, 2021

raysonkoh Mar 7, 2021

ang-zeyu Mar 7, 2021

raysonkoh Mar 7, 2021

ang-zeyu Mar 5, 2021

raysonkoh Mar 7, 2021

ang-zeyu left a comment

ang-zeyu Mar 8, 2021

raysonkoh Mar 8, 2021

ang-zeyu left a comment


		<include src="screenshot.md" boilerplate var-alt="Create Surge account" var-file="surgeCreateAccount.png" inline />

		1. Proceed to create a Surge account.


		<box type="info">

		This section is meant for users who are $$hosting their Markbind project on Github$$.

Support Previewing PR using Surge.sh #1487

Support Previewing PR using Surge.sh #1487

Conversation

raysonkoh commented Feb 24, 2021 • edited Loading

ang-zeyu commented Feb 24, 2021

raysonkoh commented Feb 24, 2021 • edited Loading

ang-zeyu commented Feb 24, 2021

raysonkoh commented Feb 24, 2021

ang-zeyu commented Feb 24, 2021

raysonkoh commented Feb 25, 2021

raysonkoh commented Feb 25, 2021

jonahtanjz left a comment

Choose a reason for hiding this comment

jonahtanjz Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

raysonkoh Mar 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ang-zeyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ang-zeyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ang-zeyu left a comment

Choose a reason for hiding this comment

raysonkoh commented Feb 24, 2021 •

edited

Loading

raysonkoh commented Feb 24, 2021 •

edited

Loading

jonahtanjz Mar 1, 2021 •

edited

Loading

raysonkoh Mar 2, 2021 •

edited

Loading