Call the shared website build workflow - #5515
Conversation
test.yml and FlowFuse/flowfuse's docs.yml both carried their own copy of the same pipeline: app token, website and blueprint-library checkouts, Node 24, blueprints, install, build, hyperlink. Two copies of one build, already drifting, and the flowfuse copy threw its output away after the link check. site-preview.yml is now the single definition, called from both sides. The flowfuse caller passes its PR as docs_repository/docs_ref, which lands the checkout next to the website so docs-sync.mjs prefers it over cloning main, and sets deploy_preview so the build is uploaded to Netlify under a per-PR alias instead of being discarded. Two behaviour notes: - Previews build with `build:nuxt` rather than `build:nuxt:skip-images`, because nuxt.config.ts drops the image provider to none when SKIP_IMAGES is set. This matches netlify.toml's deploy-preview context. - The image cache key was static, so the entry was written once and never refreshed, and it pointed at the Eleventy-era `_site/img`. It is now a rolling key over the two directories netlify.toml's cache plugin keeps warm. The upload never passes --prod and never passes --build, so netlify.toml's build command (which reindexes Algolia) does not run against a preview, and a step fails the job if Netlify hands back the production URL. test_website is kept as a gate job because a reusable workflow reports its checks as "caller job / called job", which would rename the context the "protect main" ruleset requires.
… absent A caller that sets deploy_preview without passing the token or site id would otherwise get an opaque netlify-cli error.
✅ Deploy Preview for flowforge-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@dimitrieh Could you elaborate on the skip-images point? The way it's worked so far is just skipping image optimization, images are still "real" and visible either way. The reason we skip it is that it takes longer than necessary for a preview, and with Netlify's recent issue where it drops the cache for no reason, we were getting ~90min builds on random production deploys too, so it's currently disabled for production as well. |
ppawlowski
left a comment
ppawlowski
left a comment
There was a problem hiding this comment.
- Please move the reusable GitHub Actions workflow to the
FlowFuse/github-actions-workflowsrepository - Address my comments while moving
| description: 'Repository holding the website source.' | ||
| type: string | ||
| default: 'FlowFuse/website' | ||
| website_ref: |
There was a problem hiding this comment.
What is the purpose of the parametrisation of a repository name and ref?
There was a problem hiding this comment.
Two callers, two different sources.
actions/checkoutdefaults to the repo that owns the run. Called fromFlowFuse/flowfusethat's flowfuse, not the website, so the website source has to be named explicitly. Now that the workflow sits ingithub-actions-workflowsthat's true for both callers.website_refis what lets this repo's own CI build the commit under test (github.sha) rather thanmain, while flowfuse docs PRs stay on websitemain.
Regarding website_repository, I'm fine hardcoding FlowFuse/website and keeping only the ref if you prefer. Fork PRs can't build anyway since they don't get the secrets. Let me know which you'd rather have.
| # Accepts either the App ID or the Client ID as the JWT issuer, so both callers | ||
| # can pass whichever of the two their own secret holds. |
There was a problem hiding this comment.
| # Accepts either the App ID or the Client ID as the JWT issuer, so both callers | |
| # can pass whichever of the two their own secret holds. |
This is not true.
There was a problem hiding this comment.
Removed.
On the mechanism, for the record: main.js resolves core.getInput("client-id") || core.getInput("app-id") and passes the result straight through as the JWT issuer, and test.yml on main already feeds GH_CI_READONLY_APP_ID into client-id today. What I can't verify is what that secret actually holds, so the comment was claiming more than I can back up. Dropped it.
| # Rolling key. A static key is only ever written once, so the entry went stale and | ||
| # never picked up newly processed images. These are the two directories that | ||
| # netlify.toml's cache plugin keeps warm on Netlify's own builds. | ||
| key: img-pipeline-${{ github.run_id }} |
There was a problem hiding this comment.
This is very expensive, the github.run_id is unique per run, so the exact lookup can never hit. That means the cache restore step always falls through to restore-keys and cache save always fires - a fresh full copy every workflow execution.
Please have in mind that there is a 10G storage limit for cache. I am pretty sure we will hit this limit very quickly with this approach.
There was a problem hiding this comment.
Agreed, that key was wrong. github.run_id is a guaranteed miss, which means a full save every single run.
Two changes:
- The step now only runs when
deploy_previewis true. Everything else builds withSKIP_IMAGES, which setsnuxt.config.ts's image provider tonone, so those runs were writing a cache nothing ever read. - The key is content based now:
hashFilesover the source images undersrc, withrestore-keys: img-pipeline-. Unchanged images give an exact hit and no save at all. A run that touches an image restores from the prefix and writes one entry.
That gets it to one entry per branch per image content, rather than one per run. Good catch on the 10 GB, the output tree is in the tens of megabytes so the old version would have filled it fast.
| client-id: ${{ secrets.ci_app_id }} | ||
| private-key: ${{ secrets.ci_app_key }} | ||
| owner: ${{ github.repository_owner }} | ||
| repositories: blueprint-library |
There was a problem hiding this comment.
Scoping the token to the FlowFuse/blueprint-library will cause operations on the flowfuse/website and flowfuse/flowfuse repositories to fail.
There was a problem hiding this comment.
I don't think it does, but tell me if I'm missing a path.
The app token is referenced exactly once, on the blueprint-library checkout. The website and docs checkouts use the caller's default GITHUB_TOKEN, and both repos are public, so a caller-scoped token reads them fine. The old test.yml listed website because it passed the app token to the website checkout, and this version doesn't.
Happy to add website back if you'd rather the two lists stay identical, but as written it's scope the workflow never uses. I've put a comment on the step saying so.
| # A reusable workflow reports its checks as "caller job / called job", which would rename | ||
| # the test_website context that the "protect main" ruleset requires. This gate keeps that | ||
| # name and fails whenever the build above did not succeed. | ||
| test_website: | ||
| needs: build_site | ||
| if: always() | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Generate a token | ||
| id: generate_token | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 | ||
| with: | ||
| client-id: ${{ secrets.GH_CI_READONLY_APP_ID }} | ||
| private-key: ${{ secrets.GH_CI_READONLY_APP_KEY }} | ||
| owner: ${{ github.repository_owner }} | ||
| repositories: | | ||
| website | ||
| blueprint-library | ||
| - name: Check out website repository | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| path: 'website' | ||
| token: ${{ steps.generate_token.outputs.token }} | ||
| - name: Check out FlowFuse/blueprint-library repository (to access the blueprints) | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: 'FlowFuse/blueprint-library' | ||
| ref: main | ||
| path: 'blueprint-library' | ||
| token: ${{ steps.generate_token.outputs.token }} | ||
| - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 | ||
| with: | ||
| node-version: 24 | ||
| cache: 'npm' | ||
| cache-dependency-path: './website/package-lock.json' | ||
| - run: npm run blueprints | ||
| working-directory: 'website' | ||
| - name: Install Dependencies | ||
| run: npm install | ||
| working-directory: 'website' | ||
| - name: Run unit tests | ||
| run: npm test | ||
| working-directory: 'website' | ||
| - name: Build the forge | ||
| run: npm run build:nuxt:skip-images | ||
| working-directory: 'website' | ||
| - name: Check links | ||
| uses: untitaker/hyperlink@9375bc4063712ad490d5eb3d54df0b6aade15e54 # 0.3.2 | ||
| with: | ||
| args: website/nuxt/dist/ --check-anchors --sources website/src | ||
| - name: Require the site build to have succeeded | ||
| run: | | ||
| echo "build_site result: ${{ needs.build_site.result }}" | ||
| [ "${{ needs.build_site.result }}" = "success" ] |
There was a problem hiding this comment.
Just update the ruleset instead of introducing a separate job that spins up a dedicated runner and consumes minutes.
There was a problem hiding this comment.
Done, the gate job is gone.
The calling job is named test_website, so the required context becomes test_website / Build and check. Can you update the "protect main" ruleset to that name? I'll hold the merge until it's in, otherwise every website PR blocks on a check that never reports.
The build now lives in FlowFuse/github-actions-workflows, so this repository and FlowFuse/flowfuse documentation pull requests share one definition. Review feedback also drops the separate gate job: the calling job is named test_website, so the required context becomes test_website / Build and check and the ruleset moves to that name.
dimitrieh
changed the title
|
Makes sense, that's where it belongs. Both points are done:
Both callers point at the One trade-off worth naming: a build change can't be tested in the PR that makes it any more, since the caller has to point at a merged ref. That's why it started here, but I'd rather follow the convention. Two things I'd like back from you:
|
|
@ppawlowski moved to FlowFuse/github-actions-workflows#211, comments addressed, replies above. This one is just the caller now. |
3 participants
Description
test.ymland FlowFuse/flowfuse'sdocs.ymleach carried their own copy of the same build: app token, website and blueprint-library checkouts, Node 24, blueprints, install, build, hyperlink. That build now lives in FlowFuse/github-actions-workflows asbuild_website.yml, called from both, so changes to the build or the deploy are picked up on the flowfuse side automatically as the Nuxt migration progresses. It also gains an optional Netlify upload, which is what gives documentation PRs a preview URL.This PR is now just the caller.
test_websitekeeps its job name, so the context the "protect main" ruleset requires becomestest_website / Build and check.Merge order. FlowFuse/github-actions-workflows#211 first, then this PR, then FlowFuse/flowfuse#8070. Both callers track the workflow branch until #211 lands and then move to
@main. The ruleset needs updating totest_website / Build and checkbefore this merges.Related Issue(s)
Shared workflow: FlowFuse/github-actions-workflows#211
Caller side: FlowFuse/flowfuse#8070
Checklist