Enable external contributors to create previews (#1068)

Reorganize preview workflows so that previews can be made for PRs from outside contributors.

.github/workflows/fly-build.yml

@@ -0,0 +1,43 @@
+# fly-deploy will be triggered on completion of this workflow to actually deploy the code to fly.io.
+
+name: fly.io Build
+on:
+  pull_request:
+    branches: [ main ]
+    types: [labeled, opened, synchronize, reopened]
+
+  # Allows running this workflow manually from the Actions tab
+  workflow_dispatch:
+
+jobs:
+  build:
+    name: Build Docker image
+    runs-on: ubuntu-latest
+    # Build when the 'preview' label is added, or when PR is updated with this label present.
+    if: >
+      github.event_name == 'workflow_dispatch' ||
+      (github.event_name == 'pull_request' &&
+      contains(github.event.pull_request.labels.*.name, 'preview'))
+    steps:
+      - uses: actions/checkout@v4
+      - name: Build and export Docker image
+        id: docker-build
+        run: >
+          docker build -t grist-core:preview . &&
+          docker image save grist-core:preview -o grist-core.tar
+      - name: Save PR information
+        run: |
+          echo PR_NUMBER=${{ github.event.number }} >> ./pr-info.txt
+          echo PR_SOURCE=${{ github.event.pull_request.head.repo.full_name }}-${{ github.event.pull_request.head.ref }} >> ./pr-info.txt
+          echo PR_SHASUM=${{ github.event.pull_request.head.sha }} >> ./pr-info.txt
+        # PR_SOURCE looks like <owner>/<repo>-<branch>.
+        # For example, if the GitHub user "foo" forked grist-core as "grist-bar", and makes a PR from their branch named "baz",
+        # it will be "foo/grist-bar-baz". deploy.js later replaces "/" with "-", making it "foo-grist-bar-baz".
+      - name: Upload artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: docker-image
+          path: |
+            ./grist-core.tar
+            ./pr-info.txt
+          if-no-files-found: "error"

.github/workflows/fly-cleanup.yml

@@ -1,4 +1,4 @@
-name: Fly Cleanup
+name: fly.io Cleanup
 on:
   schedule:
     # Once a day, clean up jobs marked as expired
@@ -12,12 +12,12 @@ env:
 
 jobs:
   clean:
-      name: Clean stale deployed apps
-      runs-on: ubuntu-latest
-      if: github.repository_owner == 'gristlabs'
-      steps:
-        - uses: actions/checkout@v3
-        - uses: superfly/flyctl-actions/setup-flyctl@master
-          with:
-            version: 0.1.66
-        - run: node buildtools/fly-deploy.js clean
+    name: Clean stale deployed apps
+    runs-on: ubuntu-latest
+    if: github.repository_owner == 'gristlabs'
+    steps:
+      - uses: actions/checkout@v3
+      - uses: superfly/flyctl-actions/setup-flyctl@master
+        with:
+          version: 0.2.72
+      - run: node buildtools/fly-deploy.js clean

.github/workflows/fly-deploy.yml

@@ -0,0 +1,70 @@
+# Follow-up of fly-build, with access to secrets for making deployments.
+# This workflow runs in the target repo context. It does not, and should never execute user-supplied code.
+# See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+
+name: fly.io Deploy
+on:
+  workflow_run:
+    workflows: ["fly.io Build"]
+    types:
+      - completed
+
+jobs:
+  deploy:
+    name: Deploy app to fly.io
+    runs-on: ubuntu-latest
+    if: |
+      github.event.workflow_run.event == 'pull_request' &&
+      github.event.workflow_run.conclusion == 'success'
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up flyctl
+        uses: superfly/flyctl-actions/setup-flyctl@master
+        with:
+          version: 0.2.72
+      - name: Download artifacts
+        uses: actions/github-script@v7
+        with:
+          script: |
+            var artifacts = await github.rest.actions.listWorkflowRunArtifacts({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               run_id: ${{ github.event.workflow_run.id }},
+            });
+            var matchArtifact = artifacts.data.artifacts.filter((artifact) => {
+              return artifact.name == "docker-image"
+            })[0];
+            var download = await github.rest.actions.downloadArtifact({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               artifact_id: matchArtifact.id,
+               archive_format: 'zip',
+            });
+            var fs = require('fs');
+            fs.writeFileSync('${{github.workspace}}/docker-image.zip', Buffer.from(download.data));
+      - name: Extract artifacts
+        id: extract_artifacts
+        run: |
+          unzip docker-image.zip
+          cat ./pr-info.txt >> $GITHUB_OUTPUT
+      - name: Load Docker image
+        run: docker load --input grist-core.tar
+      - name: Deploy to fly.io
+        id: fly_deploy
+        env:
+          FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
+          BRANCH_NAME: ${{ steps.extract_artifacts.outputs.PR_SOURCE }}
+        run: |
+          node buildtools/fly-deploy.js deploy
+          flyctl config -c ./fly.toml env | awk '/APP_HOME_URL/{print "DEPLOY_URL=" $2}' >> $GITHUB_OUTPUT
+          flyctl config -c ./fly.toml env | awk '/FLY_DEPLOY_EXPIRATION/{print "EXPIRES=" $2}' >> $GITHUB_OUTPUT
+      - name: Comment on PR
+        uses: actions/github-script@v7
+        with:
+          script: |
+            github.rest.issues.createComment({
+              issue_number: ${{ steps.extract_artifacts.outputs.PR_NUMBER }},
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              body: `Deployed commit \`${{ steps.extract_artifacts.outputs.PR_SHASUM }}\` as ${{ steps.fly_deploy.outputs.DEPLOY_URL }} (until ${{ steps.fly_deploy.outputs.EXPIRES }})`
+            })

.github/workflows/fly-destroy.yml

@@ -0,0 +1,36 @@
+# This workflow runs in the target repo context, as it is triggered via pull_request_target.
+# It does not, and should not have access to code in the PR.
+# See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+
+name: fly.io Destroy
+on:
+  pull_request_target:
+    branches: [ main ]
+    types: [unlabeled, closed]
+
+  # Allows running this workflow manually from the Actions tab
+  workflow_dispatch:
+
+jobs:
+  destroy:
+    name: Remove app from fly.io
+    runs-on: ubuntu-latest
+    # Remove the deployment when 'preview' label is removed, or the PR is closed.
+    if: |
+      github.event_name == 'workflow_dispatch' ||
+      (github.event_name == 'pull_request_target' &&
+      (github.event.action == 'closed' ||
+      (github.event.action == 'unlabeled' && github.event.label.name == 'preview')))
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up flyctl
+        uses: superfly/flyctl-actions/setup-flyctl@master
+        with:
+          version: 0.2.72
+      - name: Destroy fly.io app
+        env:
+          FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
+          BRANCH_NAME: ${{ github.event.pull_request.head.repo.full_name }}-${{ github.event.pull_request.head.ref }}
+          # See fly-build for what BRANCH_NAME looks like.
+        id: fly_destroy
+        run: node buildtools/fly-deploy.js destroy

.github/workflows/fly.yml

@@ -1,64 +0,0 @@
-name: Fly Deploy
-on:
-  pull_request:
-    branches: [ main ]
-    types: [labeled, unlabeled, closed, opened, synchronize, reopened]
-
-  # Allows running this workflow manually from the Actions tab
-  workflow_dispatch:
-
-env:
-  FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
-  BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
-
-jobs:
-  deploy:
-    name: Deploy app
-    runs-on: ubuntu-latest
-    # Deploy when the 'preview' label is added, or when PR is updated with this label present.
-    if: |
-          github.repository_owner == 'gristlabs' &&
-          github.event_name == 'pull_request' && (
-            github.event.action == 'labeled' ||
-            github.event.action == 'opened' ||
-            github.event.action == 'synchronize' ||
-            github.event.action == 'reopened'
-          ) &&
-          contains(github.event.pull_request.labels.*.name, 'preview')
-    steps:
-      - uses: actions/checkout@v3
-      - uses: superfly/flyctl-actions/setup-flyctl@master
-        with:
-          version: 0.1.89
-      - id: fly_deploy
-        run: |
-          node buildtools/fly-deploy.js deploy
-          flyctl config -c ./fly.toml env | awk '/APP_HOME_URL/{print "DEPLOY_URL=" $2}' >> $GITHUB_OUTPUT
-          flyctl config -c ./fly.toml env | awk '/FLY_DEPLOY_EXPIRATION/{print "EXPIRES=" $2}' >> $GITHUB_OUTPUT
-
-      - uses: actions/github-script@v6
-        with:
-          script: |
-            github.rest.issues.createComment({
-              issue_number: context.issue.number,
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              body: `Deployed as ${{ steps.fly_deploy.outputs.DEPLOY_URL }} (until ${{ steps.fly_deploy.outputs.EXPIRES }})`
-            })
-
-  destroy:
-    name: Remove app
-    runs-on: ubuntu-latest
-    # Remove the deployment when 'preview' label is removed, or the PR is closed.
-    if: |
-          github.repository_owner == 'gristlabs' &&
-          github.event_name == 'pull_request' &&
-          (github.event.action == 'closed' ||
-           (github.event.action == 'unlabeled' && github.event.label.name == 'preview'))
-    steps:
-      - uses: actions/checkout@v3
-      - uses: superfly/flyctl-actions/setup-flyctl@master
-        with:
-          version: 0.1.89
-      - id: fly_destroy
-        run: node buildtools/fly-deploy.js destroy

buildtools/fly-deploy.js

@@ -1,7 +1,6 @@
 const util = require('util');
 const childProcess = require('child_process');
 const fs = require('fs/promises');
-const {existsSync} = require('fs');
 
 const exec = util.promisify(childProcess.exec);
 
@@ -17,66 +16,81 @@ const getBranchName = () => {
 };
 
 async function main() {
-  if (process.argv[2] === 'deploy') {
-    const appRoot = process.argv[3] || ".";
-    if (!existsSync(`${appRoot}/Dockerfile`)) {
-      console.log(`Dockerfile not found in appRoot of ${appRoot}`);
-      process.exit(1);
-    }
-
-    const name = getAppName();
-    const volName = getVolumeName();
-    if (!await appExists(name)) {
-      await appCreate(name);
-      await volCreate(name, volName);
-    } else {
-      // Check if volume exists, and create it if not. This is needed because there was an API
-      // change in flyctl (mandatory -y flag) and some apps were created without a volume.
-      if (!(await volList(name)).length) {
+  switch (process.argv[2]) {
+    case "deploy": {
+      const name = getAppName();
+      const volName = getVolumeName();
+      if (!await appExists(name)) {
+        await appCreate(name);
         await volCreate(name, volName);
+      } else {
+        // Check if volume exists, and create it if not. This is needed because there was an API
+        // change in flyctl (mandatory -y flag) and some apps were created without a volume.
+        if (!(await volList(name)).length) {
+          await volCreate(name, volName);
+        }
       }
+      await prepConfig(name, volName);
+      await appDeploy(name);
+      break;
     }
-    await prepConfig(name, appRoot, volName);
-    await appDeploy(name, appRoot);
-  } else if (process.argv[2] === 'destroy') {
-    const name = getAppName();
-    if (await appExists(name)) {
-      await appDestroy(name);
+    case "destroy": {
+      const name = getAppName();
+      if (await appExists(name)) {
+        await appDestroy(name);
+      }
+      break;
     }
-  } else if (process.argv[2] === 'clean') {
-    const staleApps = await findStaleApps();
-    for (const appName of staleApps) {
-      await appDestroy(appName);
+    case "clean": {
+      const staleApps = await findStaleApps();
+      for (const appName of staleApps) {
+        await appDestroy(appName);
+      }
+      break;
     }
-  } else {
-    console.log(`Usage:
-  deploy [appRoot]:
-            create (if needed) and deploy fly app grist-{BRANCH_NAME}.
-            appRoot may specify the working directory that contains the Dockerfile to build.
+    default: {
+      console.log(`Usage:
+  deploy:   create (if needed) and deploy fly app grist-{BRANCH_NAME}.
   destroy:  destroy fly app grist-{BRANCH_NAME}
   clean:    destroy all grist-* fly apps whose time has come
             (according to FLY_DEPLOY_EXPIRATION env var set at deploy time)
 
   DRYRUN=1 in environment will show what would be done
 `);
-    process.exit(1);
+      process.exit(1);
+    }
   }
 }
 
+function getDockerTag(name) {
+  return `registry.fly.io/${name}:latest`;
+}
+
 const appExists = (name) => runFetch(`flyctl status -a ${name}`).then(() => true).catch(() => false);
-const appCreate = (name) => runAction(`flyctl launch --auto-confirm --name ${name} -r ewr -o ${org} --vm-memory 1024`);
+// We do not deploy at the create stage, since the Docker image isn't ready yet.
+// Assigning --image prevents flyctl from making inferences based on the codebase and provisioning unnecessary postgres/redis instances.
+const appCreate = (name) => runAction(`flyctl launch --no-deploy --auto-confirm --image ${getDockerTag(name)} --name ${name} -r ewr -o ${org}`);
 const volCreate = (name, vol) => runAction(`flyctl volumes create ${vol} -s 1 -r ewr -y -a ${name}`);
 const volList = (name) => runFetch(`flyctl volumes list -a ${name} -j`).then(({stdout}) => JSON.parse(stdout));
-const appDeploy = (name, appRoot) => runAction(`flyctl deploy ${appRoot} --remote-only --region=ewr --vm-memory 1024`,
-  {shell: true, stdio: 'inherit'});
+const appDeploy = async (name) => {
+  try {
+    await runAction("flyctl auth docker")
+    await runAction(`docker image tag grist-core:preview ${getDockerTag(name)}`);
+    await runAction(`docker push ${getDockerTag(name)}`);
+    await runAction(`flyctl deploy --app ${name} --image ${getDockerTag(name)}`);
+  } catch (e) {
+    console.log(`Error occurred when deploying: ${e}`);
+    process.exit(1);
+  }
+};
 
 async function appDestroy(name) {
   await runAction(`flyctl apps destroy ${name} -y`);
 }
 
-async function prepConfig(name, appRoot, volName) {
-  const configPath = `${appRoot}/fly.toml`;
-  const configTemplatePath = `${appRoot}/buildtools/fly-template.toml`;
+async function prepConfig(name, volName) {
+  const configPath = "./fly.toml";
+  const configTemplatePath = "./buildtools/fly-template.toml";
   const template = await fs.readFile(configTemplatePath, {encoding: 'utf8'});
 
   // Calculate the time when we can destroy the app, used by findStaleApps.

buildtools/fly-template.toml

@@ -48,3 +48,8 @@ processes = []
 [mounts]
 source="{VOLUME_NAME}"
 destination="/persist"
+
+[[vm]]
+  memory = '1gb'
+  cpu_kind = 'shared'
+  cpus = 1