Repository navigation
Deploy PR previews with HarperFast/deploy-action - #797
dawsontoth wants to merge 2 commits into
Conversation
Replace the hand-written pr-preview.yaml with preview.yaml, the action's two-job preview workflow (deploy in `preview`, cleanup in `preview-cleanup`). It authenticates by OIDC only, so the stored username and password are no longer read, and it never checks out PR code; fork PRs get no preview. The site needs a Docusaurus build, which must not run with the preview identity. preview-build.yaml builds each PR's head commit in an unprivileged pull_request run and uploads it; the deploy job waits for that run, downloads the artifact under a config.yaml of its own, and deploys it as a payload preview at the root of its own host. The local preview script follows the build workflow's name and base URL. Co-Authored-By: Claude Opus 5.5 <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates scripts/preview-pr.mjs to query the "Build PR Preview" workflow instead of "Deploy PR Preview" and sets the DOCUSAURUS_BASE_URL to / instead of a PR-specific path. Feedback points out that using the --paginate flag with gh api and --jq can result in multiple JSON documents being returned, which will cause a SyntaxError when parsed. It is recommended to remove the --paginate flag.
| const runs = JSON.parse( | ||
| execSync( | ||
| `gh api repos/HarperFast/documentation/actions/runs --paginate -X GET -f branch=${sanitizedBranch} --jq '.workflow_runs | map(select(.conclusion == "success" and .name == "Deploy PR Preview")) | sort_by(.created_at) | reverse | .[0]'`, | ||
| `gh api repos/HarperFast/documentation/actions/runs --paginate -X GET -f branch=${sanitizedBranch} --jq '.workflow_runs | map(select(.conclusion == "success" and .name == "Build PR Preview")) | sort_by(.created_at) | reverse | .[0]'`, |
There was a problem hiding this comment.
Using --paginate with gh api and --jq can output multiple JSON documents (one per page) if there are multiple pages of workflow runs. When passed to JSON.parse(), this will cause a SyntaxError because it is not a single valid JSON document. Since we only need the latest successful run, which is guaranteed to be on the first page, we should remove the --paginate flag to ensure the output is always a single valid JSON document.
| `gh api repos/HarperFast/documentation/actions/runs --paginate -X GET -f branch=${sanitizedBranch} --jq '.workflow_runs | map(select(.conclusion == "success" and .name == "Build PR Preview")) | sort_by(.created_at) | reverse | .[0]'`, | |
| `gh api repos/HarperFast/documentation/actions/runs -X GET -f branch=${sanitizedBranch} --jq '.workflow_runs | map(select(.conclusion == "success" and .name == "Build PR Preview")) | sort_by(.created_at) | reverse | .[0]'`, |
Co-Authored-By: Claude Opus 5.5 <[email protected]>
⊙ Problem
pr-preview.yamlchecks out each PR's code underpull_request_target(fork PRs too, after an approval inpreview), builds it in a job that holdsHARPER_PREVIEW_USERNAMEandHARPER_PREVIEW_PASSWORD, and deploys apr-<N>component that stays until the PR closes. Previews pile up on the preview host (#695).HarperFast/deploy-actionnow does per-PR previews on Harper 5.4 with OIDC only, and Kris suggested running it here so it's exercised on real PRs (HarperFast/deploy-action#1).💡 Solution
preview.yamlis the action's example workflow: adeployjob in thepreviewenvironment and acleanupjob inpreview-cleanup, each with the fork guard. No checkout, no stored credential, and the action keeps one PR comment and a GitHub Deployment. Previews move from<preview host>/pr-<N>tohttps://<branch>.app.<cluster domain>/and expire on their own (7 days by default).preview-build.yamlbuilds the site. The action's default deploys a PR's commit by reference, but this repo has no Harperconfig.yamland the cluster doesn't run a Docusaurus build. So, as the action's README and the CI guide describe for apps that need a build, an unprivilegedpull_requestrun builds the PR's head commit and uploadspr-<N>-build. The deploy job waits for that run, downloads the artifact intosite/build, writes its ownsite/config.yaml(the same static config production uses), and deployssiteas a payload withby_ref: false.scripts/preview-pr.mjs(npm run preview:pr) looks for the new build workflow's runs, and serves with base URL/.The action is pinned to
v1.0.0by its commit,45fa59b.🤔 Things to look at
pull_request_target, soworkflow_runcan't deploy. The deploy job polls for the build of its exact commit, from a branch of this repository. It deploys nothing if the PR moves on or closes, and fails if the build fails, takes over 30 minutes, or never starts.pull_requestrun for it, so the deploy job fails after 10 minutes and says why. Assemble the v5 reference and open release notes from harper and harper-pro #788 is in that state now. The old workflow built conflicting PRs anyway.npm run preview:pr, subject to the repository's fork-approval setting.docs-site, production's component name, so previews aredocs-site-pr-<N>. Each gets its own database branch (the default). The site has no tables.actions: readon the deploy job, beyond the example, is for reading the build's runs and downloading its artifact.📋 Enabling previews (Dawson)
Nothing here is done yet: this PR changes no cluster or repository settings. A real preview needs this PR merged, since
pull_request_targetrunsmain's workflow and the trust policies pinmain.Before merging
Steps 3a, 3b and 4 don't depend on the cluster, so they can happen in any order.
preview.harper-documentation.harperfabric.com(one instance), upgraded in Fabric. Its version isn't checked yet, because the operations API needs a login.alpha.1has noenable_previewsorharper preview setup.*.app.preview.harper-documentation.harperfabric.comresolves, but the instance's certificate (issued Sep 20, expires Dec 19) doesn't cover it, so a preview host's connection is reset. Fabric adds it when the certificate renews on its own, before Dec 19, unless Fabric renews it sooner.harper preview setup app=docs-site, signed in as asuper_user, from a checkout of this branch (setup reads.github/workflows/preview.yamland checks it).loginprompts for the target,https://preview.harper-documentation.harperfabric.com:9925/.docs-site-ci-previewrole and user and both trust policies, and enables previews fordocs-site. It also sets theHARPER_PREVIEW_TARGETrepository variable, which is separate from today's secret of the same name.previewenvironment: allow onlymainunder Deployment branches and tags. Keep itsdevelopersreviewers until this merges:main's oldpr-preview.yamlbuilds fork PRs with the stored password, and those reviewers are what gate it.preview-cleanupenvironment: create it, with no reviewers and the samemainrule.pull_request_targetfor.github/workflows/preview.yaml. The repository is public, and GitHub blocks the event from December 2, with brownouts on November 9 and 16. The repository has no policies yet. From the action's README:After merging
previewenvironment: remove thedevelopersreviewers, or every docs PR's preview waits for an approval.HARPER_PREVIEW_USERNAME,HARPER_PREVIEW_PASSWORDandHARPER_PREVIEW_TARGETsecrets, and thePREVIEW_CLEANUP_APP_IDvariable andPREVIEW_CLEANUP_APP_PRIVATE_KEYsecret if they're set. Nothing reads them now.pr-<N>environments.pr-<N>components from the preview cluster.Outside this PR
companion-check.yaml, allowingpull_request_target,scheduleandworkflow_dispatch. It also runs onpull_request_target, so without a policy it fails during the brownouts and stops on December 2.✅ Verification
checkPreviewWorkflowFilefrom harpermain(whatharper preview setupruns) finds no problems, nothing unverified, and nothing unsafe inpreview.yaml. Adding a PR-head checkout, or dropping the fork guard, gets refused.validate.yamlin place of the build workflow:run-id.pull_requestrun at all.Relates to #695.
🤖 Generated with Claude Code