diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 56a6ce7da..4976fa60b 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -1,9 +1,13 @@ name: CodeQL +# The `selector` job decides whether a pull request runs the analysis, and +# `summary` reports the result (ADR 0003). + on: push: branches: - main + # The trigger paths are in the `selector` job. pull_request: branches: - main @@ -16,8 +20,83 @@ permissions: actions: read jobs: + # The selector job: it decides which jobs a pull request runs. It runs on + # every event and is never skipped, because the gated jobs need it. When it + # cannot decide it fails, and that fails the summary job: a selector that + # answered nothing would skip every gated job, and the summary job counts a + # skipped job as passed (ADR 0003). + # + # On a GitHub-hosted runner because the gated jobs wait for it. No checkout: + # on a pull request, dorny/paths-filter lists the changed files through the + # GitHub API. + selector: + name: Selector + runs-on: ubuntu-latest + timeout-minutes: 10 + # Not the workflow's `security-events: write`: this job writes nothing. + permissions: + contents: read + # Each output is named after the filter that decides it. + # `scripts/__tests__/workflow-selectors.test.mjs` follows a job's `if:` to + # its filter by that name. + outputs: + javascript: ${{ steps.decide.outputs.javascript }} + steps: + # Only on a pull request, which has a base to compare with. Other events + # select every job in the step below. + - id: filter + if: github.event_name == 'pull_request' + uses: dorny/paths-filter@0e4a8c6effa4802afeda77dc8d303f8176d7dfad # v3 + with: + filters: | + # Every file type the CodeQL JavaScript extractor reads, from + # `FileType` in its FileExtractor.java: HTML and templates, + # JavaScript, JSON, TypeScript, and YAML (which includes `.lock`). + javascript: + - "**/*.{htm,html,xhtm,xhtml,vue,hbs,ejs,njk,erb,jsp,dot}" + - "**/*.{js,jsx,mjs,cjs,es6,es,xsjs,xsjslib}" + - "**/*.{ts,tsx,mts,cts}" + - "**/*.json" + - "**/*.{lock,raml,yaml,yml}" + + # Explicit default, not `|| 'true'`, which trips GitHub's inconsistent + # treatment of the string 'false'. On a pull request, an answer other than + # `true` or `false` means the filter did not decide, so the step fails + # rather than write it. + # + # The event and the answers arrive through `env:`, not through a `${{ }}` + # in the script. That lets `scripts/__tests__/workflow-selectors.test.mjs` + # run this script and read back what it wrote. + - id: decide + env: + EVENT_NAME: ${{ github.event_name }} + JAVASCRIPT: ${{ steps.filter.outputs.javascript }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" != "pull_request" ]; then + JAVASCRIPT=true + fi + answers=( + "javascript=$JAVASCRIPT" + ) + for answer in "${answers[@]}"; do + case "${answer#*=}" in + true|false) echo "$answer" >> "$GITHUB_OUTPUT" ;; + *) + echo "::error::The path filter answered '${answer#*=}' for ${answer%%=*}. The selector cannot decide which jobs to run." + exit 1 + ;; + esac + done + analyze: name: Analyze + needs: [selector] + # Written as an exclusion, not a list of events: every event other + # than a pull request runs this job. + if: >- + github.event_name != 'pull_request' + || needs.selector.outputs.javascript == 'true' runs-on: blacksmith-4vcpu-ubuntu-2404 strategy: @@ -39,3 +118,34 @@ jobs: uses: github/codeql-action/init@v4.38.0 with: category: "/language:${{ matrix.language }}" + + # The summary job: the one job of this workflow that branch rules require. + # It has the same name on every event. It passes when every needed job + # passed or was skipped. Skipped counts as passed because the selector skips + # a job whose trigger paths a change does not touch. A failed job still + # fails this one, because it is in `needs` and reports its failure. + summary: + name: CodeQL summary + needs: [selector, analyze] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 10 + # Not the workflow's `security-events: write`: this job writes nothing. + permissions: + contents: read + steps: + - name: Check that every needed job passed or was skipped + env: + RESULTS: ${{ join(needs.*.result, ' ') }} + run: | + set -euo pipefail + echo "Needed results: $RESULTS" + for result in $RESULTS; do + case "$result" in + success|skipped) ;; + *) + echo "::error::A needed job reported '$result'." + exit 1 + ;; + esac + done diff --git a/.github/workflows/lint-release.yml b/.github/workflows/lint-release.yml index adde4c933..cef358422 100644 --- a/.github/workflows/lint-release.yml +++ b/.github/workflows/lint-release.yml @@ -15,35 +15,15 @@ name: Lint release tooling # where an EQL-local copy of this file linted them and ran on nothing — along # with its two unique checks, the `prepare-bindings-assets` shellcheck and that # wrapper's bash unit test, neither of which ran anywhere here until now. +# +# The `selector` job decides whether a pull request runs the lint, and +# `summary` reports the result (ADR 0003). on: - pull_request: - # Exactly what the job reads: the workflows actionlint is pointed at, the - # config it resolves the Blacksmith label from, and the shell scripts the - # later steps check and run. The filter also named scripts/release-gate.mjs, - # scripts/ffi-release-matrix.mjs, scripts/lint-no-workflow-caching.mjs and - # package.json — none of which this job looks at, since it deliberately does - # not run `test:scripts` (see the job comment). Editing one booted a runner - # and downloaded a Go binary to lint four unchanged files, which is how a - # job trains reviewers to ignore it. `tests.yml`'s `lint` job runs - # `test:scripts` unfiltered on every PR, so those four are already covered. - # - # `lint-release-scope.test.mjs` asserts the `.github/workflows/` half of - # this list and the actionlint argument list below stay the same set. - paths: - - .github/workflows/release.yml - - .github/workflows/_build-ffi-artifacts.yml - - .github/workflows/ffi-preflight.yml - - .github/workflows/_build-auth-artifacts.yml - - .github/workflows/auth-preflight.yml - - .github/workflows/_build-eql-sql.yml - - .github/workflows/_build-eql-docs.yml - - .github/workflows/release-plz.yml - - .github/workflows/release-postgres-eql-image.yml - - .github/workflows/lint-release.yml - - .github/actionlint.yaml - - packages/eql/tasks/release/*.sh - - scripts/check-c-library.sh + # No path filter: a workflow that a path filter skips leaves its required + # checks pending, and GitHub's documentation on required status checks says + # that blocks a merge. The trigger paths are in the `selector` job instead. + pull_request: {} workflow_dispatch: {} permissions: @@ -58,6 +38,87 @@ concurrency: cancel-in-progress: true jobs: + # The selector job: it decides which jobs a pull request runs. It runs on + # every event and is never skipped, because the gated jobs need it. When it + # cannot decide it fails, and that fails the summary job: a selector that + # answered nothing would skip every gated job, and the summary job counts a + # skipped job as passed (ADR 0003). + # + # On a GitHub-hosted runner because the gated jobs wait for it. No checkout: + # on a pull request, dorny/paths-filter lists the changed files through the + # GitHub API. + selector: + name: Selector + runs-on: ubuntu-latest + timeout-minutes: 10 + # Each output is named after the filter that decides it. + # `scripts/__tests__/workflow-selectors.test.mjs` follows a job's `if:` to + # its filter by that name. + outputs: + release_lint: ${{ steps.decide.outputs.release_lint }} + steps: + # Only on a pull request, which has a base to compare with. Other events + # select every job in the step below. + - id: filter + if: github.event_name == 'pull_request' + uses: dorny/paths-filter@0e4a8c6effa4802afeda77dc8d303f8176d7dfad # v3 + with: + filters: | + # Exactly what the job reads: the workflows actionlint is pointed at, + # the config it resolves the Blacksmith label from, and the shell + # scripts the later steps check and run. It does not run + # `test:scripts` (see the job comment), so scripts/release-gate.mjs and + # the other release scripts are not here: `tests.yml` runs + # `test:scripts` for them. + # + # `lint-release-scope.test.mjs` asserts the `.github/workflows/` half + # of this list and the actionlint argument list below stay the same + # set. + release_lint: + - ".github/workflows/release.yml" + - ".github/workflows/_build-ffi-artifacts.yml" + - ".github/workflows/ffi-preflight.yml" + - ".github/workflows/_build-auth-artifacts.yml" + - ".github/workflows/auth-preflight.yml" + - ".github/workflows/_build-eql-sql.yml" + - ".github/workflows/_build-eql-docs.yml" + - ".github/workflows/release-plz.yml" + - ".github/workflows/release-postgres-eql-image.yml" + - ".github/workflows/lint-release.yml" + - ".github/actionlint.yaml" + - "packages/eql/tasks/release/*.sh" + - "scripts/check-c-library.sh" + + # Explicit default, not `|| 'true'`, which trips GitHub's inconsistent + # treatment of the string 'false'. On a pull request, an answer other than + # `true` or `false` means the filter did not decide, so the step fails + # rather than write it. + # + # The event and the answers arrive through `env:`, not through a `${{ }}` + # in the script. That lets `scripts/__tests__/workflow-selectors.test.mjs` + # run this script and read back what it wrote. + - id: decide + env: + EVENT_NAME: ${{ github.event_name }} + RELEASE_LINT: ${{ steps.filter.outputs.release_lint }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" != "pull_request" ]; then + RELEASE_LINT=true + fi + answers=( + "release_lint=$RELEASE_LINT" + ) + for answer in "${answers[@]}"; do + case "${answer#*=}" in + true|false) echo "$answer" >> "$GITHUB_OUTPUT" ;; + *) + echo "::error::The path filter answered '${answer#*=}' for ${answer%%=*}. The selector cannot decide which jobs to run." + exit 1 + ;; + esac + done + # No pnpm, no Node, no install: actionlint is a downloaded Go binary and # shellcheck ships in the runner image, so this job is a checkout and a few # commands. @@ -70,6 +131,12 @@ jobs: # this workflow uniquely has is actionlint, and the two shell checks below. lint: name: actionlint (release workflows) + needs: [selector] + # Written as an exclusion, not a list of events: every event other + # than a pull request runs this job. + if: >- + github.event_name != 'pull_request' + || needs.selector.outputs.release_lint == 'true' runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -120,3 +187,31 @@ jobs: # `test:scripts`. - name: shellcheck (C library check) run: shellcheck scripts/check-c-library.sh + + # The summary job: the one job of this workflow that branch rules require. + # It has the same name on every event. It passes when every needed job + # passed or was skipped. Skipped counts as passed because the selector skips + # a job whose trigger paths a change does not touch. A failed job still + # fails this one, because it is in `needs` and reports its failure. + summary: + name: Lint release tooling summary + needs: [selector, lint] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Check that every needed job passed or was skipped + env: + RESULTS: ${{ join(needs.*.result, ' ') }} + run: | + set -euo pipefail + echo "Needed results: $RESULTS" + for result in $RESULTS; do + case "$result" in + success|skipped) ;; + *) + echo "::error::A needed job reported '$result'." + exit 1 + ;; + esac + done diff --git a/.github/workflows/musl-build-image.yml b/.github/workflows/musl-build-image.yml index 89ef361e4..abbb244a6 100644 --- a/.github/workflows/musl-build-image.yml +++ b/.github/workflows/musl-build-image.yml @@ -4,17 +4,22 @@ name: musl build image # version from its index when it publishes the next one. Nothing else builds # the image between releases, so without this a pin that stopped resolving # would first be seen by a release. +# +# The `selector` job decides whether a pull request builds the image, and +# `summary` reports the result (ADR 0003). on: push: branches: - main + # The same files as the `selector` job's filter. + # scripts/__tests__/workflow-paths-filter-parity.test.mjs compares them. paths: - ".github/docker/musl-build/**" - ".github/workflows/musl-build-image.yml" - pull_request: - paths: - - ".github/docker/musl-build/**" - - ".github/workflows/musl-build-image.yml" + # No path filter: a workflow that a path filter skips leaves its required + # checks pending, and GitHub's documentation on required status checks says + # that blocks a merge. The trigger paths are in the `selector` job instead. + pull_request: {} schedule: - cron: "0 7 * * 1" workflow_dispatch: @@ -23,8 +28,74 @@ permissions: contents: read jobs: + # The selector job: it decides which jobs a pull request runs. It runs on + # every event and is never skipped, because the gated jobs need it. When it + # cannot decide it fails, and that fails the summary job: a selector that + # answered nothing would skip every gated job, and the summary job counts a + # skipped job as passed (ADR 0003). + # + # On a GitHub-hosted runner because the gated jobs wait for it. No checkout: + # on a pull request, dorny/paths-filter lists the changed files through the + # GitHub API. + selector: + name: Selector + runs-on: ubuntu-latest + timeout-minutes: 10 + # Each output is named after the filter that decides it. + # `scripts/__tests__/workflow-selectors.test.mjs` follows a job's `if:` to + # its filter by that name. + outputs: + musl_image: ${{ steps.decide.outputs.musl_image }} + steps: + # Only on a pull request, which has a base to compare with. Other events + # select every job in the step below. + - id: filter + if: github.event_name == 'pull_request' + uses: dorny/paths-filter@0e4a8c6effa4802afeda77dc8d303f8176d7dfad # v3 + with: + filters: | + musl_image: + - ".github/docker/musl-build/**" + - ".github/workflows/musl-build-image.yml" + + # Explicit default, not `|| 'true'`, which trips GitHub's inconsistent + # treatment of the string 'false'. On a pull request, an answer other than + # `true` or `false` means the filter did not decide, so the step fails + # rather than write it. + # + # The event and the answers arrive through `env:`, not through a `${{ }}` + # in the script. That lets `scripts/__tests__/workflow-selectors.test.mjs` + # run this script and read back what it wrote. + - id: decide + env: + EVENT_NAME: ${{ github.event_name }} + MUSL_IMAGE: ${{ steps.filter.outputs.musl_image }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" != "pull_request" ]; then + MUSL_IMAGE=true + fi + answers=( + "musl_image=$MUSL_IMAGE" + ) + for answer in "${answers[@]}"; do + case "${answer#*=}" in + true|false) echo "$answer" >> "$GITHUB_OUTPUT" ;; + *) + echo "::error::The path filter answered '${answer#*=}' for ${answer%%=*}. The selector cannot decide which jobs to run." + exit 1 + ;; + esac + done + build: name: Build the Alpine musl image + needs: [selector] + # Written as an exclusion, not a list of events: every event other + # than a pull request runs this job. + if: >- + github.event_name != 'pull_request' + || needs.selector.outputs.musl_image == 'true' runs-on: ubuntu-latest timeout-minutes: 15 # `issues: write` for the last step alone; see there. @@ -61,3 +132,31 @@ jobs: --title "The musl build image no longer builds" \ --label "github-actions" \ --body "The weekly build of .github/docker/musl-build/Dockerfile failed: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID. An apk pin has most likely stopped resolving; re-pin with the command in the Dockerfile's comment." + + # The summary job: the one job of this workflow that branch rules require. + # It has the same name on every event. It passes when every needed job + # passed or was skipped. Skipped counts as passed because the selector skips + # a job whose trigger paths a change does not touch. A failed job still + # fails this one, because it is in `needs` and reports its failure. + summary: + name: musl build image summary + needs: [selector, build] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Check that every needed job passed or was skipped + env: + RESULTS: ${{ join(needs.*.result, ' ') }} + run: | + set -euo pipefail + echo "Needed results: $RESULTS" + for result in $RESULTS; do + case "$result" in + success|skipped) ;; + *) + echo "::error::A needed job reported '$result'." + exit 1 + ;; + esac + done diff --git a/.github/workflows/osv-scanner.yml b/.github/workflows/osv-scanner.yml index 3d009896d..7cfa39ccb 100644 --- a/.github/workflows/osv-scanner.yml +++ b/.github/workflows/osv-scanner.yml @@ -1,9 +1,13 @@ name: OSV-Scanner +# The `selector` job decides whether a pull request runs the scan, and +# `summary` reports the result (ADR 0003). + on: push: branches: - main + # The trigger paths are in the `selector` job. pull_request: branches: - main @@ -17,11 +21,117 @@ permissions: actions: read jobs: + # The selector job: it decides which jobs a pull request runs. It runs on + # every event and is never skipped, because the gated jobs need it. When it + # cannot decide it fails, and that fails the summary job: a selector that + # answered nothing would skip every gated job, and the summary job counts a + # skipped job as passed (ADR 0003). + # + # On a GitHub-hosted runner because the gated jobs wait for it. No checkout: + # on a pull request, dorny/paths-filter lists the changed files through the + # GitHub API. + selector: + name: Selector + runs-on: ubuntu-latest + timeout-minutes: 10 + # Not the workflow's `security-events: write`: this job writes nothing. + permissions: + contents: read + # Each output is named after the filter that decides it. + # `scripts/__tests__/workflow-selectors.test.mjs` follows a job's `if:` to + # its filter by that name. + outputs: + dependencies: ${{ steps.decide.outputs.dependencies }} + steps: + # Only on a pull request, which has a base to compare with. Other events + # select every job in the step below. + - id: filter + if: github.event_name == 'pull_request' + uses: dorny/paths-filter@0e4a8c6effa4802afeda77dc8d303f8176d7dfad # v3 + with: + filters: | + # The lockfiles and manifests OSV-Scanner reads dependencies from, + # and its config. A change to other files cannot change what it + # finds. The weekday schedule catches new advisories. + dependencies: + - "**/{pnpm-lock.yaml,package-lock.json,npm-shrinkwrap.json,yarn.lock,bun.lock,deno.lock}" + - "**/{Cargo.lock,go.mod,requirements*.txt,poetry.lock,Pipfile.lock,uv.lock,pdm.lock}" + - "**/{Gemfile.lock,composer.lock,pom.xml,gradle.lockfile,pubspec.lock,mix.lock}" + - "**/{packages.lock.json,renv.lock,osv-scanner.toml}" + - ".github/workflows/osv-scanner.yml" + + # Explicit default, not `|| 'true'`, which trips GitHub's inconsistent + # treatment of the string 'false'. On a pull request, an answer other than + # `true` or `false` means the filter did not decide, so the step fails + # rather than write it. + # + # The event and the answers arrive through `env:`, not through a `${{ }}` + # in the script. That lets `scripts/__tests__/workflow-selectors.test.mjs` + # run this script and read back what it wrote. + - id: decide + env: + EVENT_NAME: ${{ github.event_name }} + DEPENDENCIES: ${{ steps.filter.outputs.dependencies }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" != "pull_request" ]; then + DEPENDENCIES=true + fi + answers=( + "dependencies=$DEPENDENCIES" + ) + for answer in "${answers[@]}"; do + case "${answer#*=}" in + true|false) echo "$answer" >> "$GITHUB_OUTPUT" ;; + *) + echo "::error::The path filter answered '${answer#*=}' for ${answer%%=*}. The selector cannot decide which jobs to run." + exit 1 + ;; + esac + done + osv-scan: name: OSV Vulnerability Scan + needs: [selector] + # Written as an exclusion, not a list of events: every event other + # than a pull request runs this job. + if: >- + github.event_name != 'pull_request' + || needs.selector.outputs.dependencies == 'true' uses: google/osv-scanner-action/.github/workflows/osv-scanner-reusable.yml@v2.6.0 with: fail-on-vuln: false scan-args: |- --recursive ./ + + # The summary job: the one job of this workflow that branch rules require. + # It has the same name on every event. It passes when every needed job + # passed or was skipped. Skipped counts as passed because the selector skips + # a job whose trigger paths a change does not touch. A failed job still + # fails this one, because it is in `needs` and reports its failure. + summary: + name: OSV-Scanner summary + needs: [selector, osv-scan] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 10 + # Not the workflow's `security-events: write`: this job writes nothing. + permissions: + contents: read + steps: + - name: Check that every needed job passed or was skipped + env: + RESULTS: ${{ join(needs.*.result, ' ') }} + run: | + set -euo pipefail + echo "Needed results: $RESULTS" + for result in $RESULTS; do + case "$result" in + success|skipped) ;; + *) + echo "::error::A needed job reported '$result'." + exit 1 + ;; + esac + done diff --git a/scripts/__tests__/auth-binding-step-order.test.mjs b/scripts/__tests__/auth-binding-step-order.test.mjs index aa1b604ab..82dd4317e 100644 --- a/scripts/__tests__/auth-binding-step-order.test.mjs +++ b/scripts/__tests__/auth-binding-step-order.test.mjs @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { filterCovers } from './lib/paths-filter.mjs' +import { jobTriggerLists } from './lib/selector.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** @@ -84,13 +85,20 @@ describe('the @cipherstash/auth binding is built wherever the SDK runs', () => { const same = (entry, input) => entry === input -/** The `paths:` lists of a workflow's filtered events. */ -const filters = (relPath) => { - const wf = readWorkflow(relPath) - const on = wf?.on ?? wf?.[true] - return ['push', 'pull_request'] - .map((event) => on?.[event]?.paths) - .filter(Array.isArray) +/** + * The path lists that decide whether the given jobs run, once each: the + * workflow's `push` and `pull_request` `paths:` lists, and the selector filter + * each job reads. Every job of the workflow when none are given. + */ +const filters = (relPath, jobNames) => { + const lists = new Map() + for (const jobName of jobNames ?? + Object.keys(readWorkflow(relPath)?.jobs ?? {})) { + for (const { source, paths } of jobTriggerLists(relPath, jobName)) { + lists.set(source, paths) + } + } + return [...lists.values()] } /** @@ -105,14 +113,19 @@ const AUTH_SOURCE_WORKFLOWS = [ ] describe('a change to the auth binding starts the jobs that load it', () => { - const users = [ - ...new Set( - JOBS.filter((job) => job.auth).map((job) => job.id.split(' / ')[0]), - ), - ] + /** Each workflow with a job that builds the binding, and those jobs. */ + const usersByWorkflow = new Map() + for (const job of JOBS.filter((entry) => entry.auth)) { + const [relPath, jobName] = job.id.split(' / ') + usersByWorkflow.set(relPath, [ + ...(usersByWorkflow.get(relPath) ?? []), + jobName, + ]) + } + const users = [...usersByWorkflow.keys()] it.each(users)('%s filters on the action', (relPath) => { - for (const paths of filters(relPath)) { + for (const paths of filters(relPath, usersByWorkflow.get(relPath))) { expect( filterCovers(paths, '.github/actions/build-auth-binding/**', same), ).toBe(true) @@ -133,7 +146,7 @@ describe('a change to the auth binding starts the jobs that load it', () => { '%s filters on the auth sources', (relPath) => { expect(shipped.length).toBeGreaterThanOrEqual(4) - const lists = filters(relPath) + const lists = filters(relPath, usersByWorkflow.get(relPath)) expect(lists.length).toBe(2) for (const paths of lists) { expect( diff --git a/scripts/__tests__/integration-workflow-paths.test.mjs b/scripts/__tests__/integration-workflow-paths.test.mjs index c3b1e812a..a44f65539 100644 --- a/scripts/__tests__/integration-workflow-paths.test.mjs +++ b/scripts/__tests__/integration-workflow-paths.test.mjs @@ -2,6 +2,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { REPO_ROOT } from './lib/repo-root.mjs' +import { jobTriggerLists } from './lib/selector.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** @@ -40,9 +41,6 @@ const STACK_SRC = 'languages/typescript/packages/stack/src' const STACK_MANIFEST = 'languages/typescript/packages/stack/package.json' const CATALOG_MANIFEST = 'pnpm-workspace.yaml' -/** Both trigger events, in the order GitHub evaluates them. */ -const TRIGGER_EVENTS = ['push', 'pull_request'] - /** * The integration workflows, discovered rather than listed: any workflow whose * `CS_IT_SUITE` globs select suites out of `languages/typescript/packages/stack/` is in scope. A @@ -56,14 +54,21 @@ function discoverWorkflows() { } /** - * `on:` parses as the boolean `true` under YAML 1.1 (the "Norway problem"), - * which is why this reads both keys rather than `wf.on`. + * The path lists that decide whether the jobs running a suite run, once each: + * the workflow's `push` and `pull_request` `paths:` lists, and the selector + * filter each such job reads. */ -function triggerBlocks(wf) { - const on = wf.on ?? wf[true] - return TRIGGER_EVENTS.map((event) => on?.[event]).filter( - (block) => block && Array.isArray(block.paths), - ) +function triggerBlocks(relPath) { + const lists = new Map() + for (const [jobId, job] of Object.entries( + readWorkflow(relPath)?.jobs ?? {}, + )) { + if (!job?.env?.CS_IT_SUITE) continue + for (const { source, paths } of jobTriggerLists(relPath, jobId)) { + lists.set(source, { source, paths }) + } + } + return [...lists.values()] } /** Every `CS_IT_SUITE` glob set declared anywhere in the workflow. */ @@ -190,8 +195,9 @@ describe('integration workflow paths filters', () => { for (const relPath of WORKFLOWS) { it(`${relPath} triggers on the manifests pinning its suites' dependencies`, () => { const wf = readWorkflow(relPath) - const blocks = triggerBlocks(wf) - expect(blocks.length).toBe(TRIGGER_EVENTS.length) + const blocks = triggerBlocks(relPath) + // A `push` list, and a `pull_request` list or a selector filter. + expect(blocks.length).toBe(2) const deps = stackDependencies() const files = suiteFiles(suiteGlobs(wf)) @@ -215,7 +221,7 @@ describe('integration workflow paths filters', () => { it(`${relPath} triggers on every source path its suites import`, () => { const wf = readWorkflow(relPath) - const blocks = triggerBlocks(wf) + const blocks = triggerBlocks(relPath) expect(blocks.length).toBeGreaterThan(0) const files = suiteFiles(suiteGlobs(wf)) diff --git a/scripts/__tests__/lib/selector.mjs b/scripts/__tests__/lib/selector.mjs new file mode 100644 index 000000000..ec2f910bb --- /dev/null +++ b/scripts/__tests__/lib/selector.mjs @@ -0,0 +1,361 @@ +import { spawnSync } from 'node:child_process' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import yaml from 'js-yaml' +import { runsWhen } from './expressions.mjs' +import { pathsGlobToRegExp } from './paths-filter.mjs' +import { readWorkflow } from './workflows.mjs' + +/** + * Reading and running a workflow's selector job, for the guards that pin what + * it decides. + * + * The selector job is the first job of a workflow. It compares the files a + * pull request changes with each check's trigger paths, using + * dorny/paths-filter, and writes one output per filter. Each gated job's `if:` + * reads the output named after its filter. ADR 0003 and `.github/CONTEXT.md` + * describe the design. + * + * Everything here runs the workflow's own text: the filter rules as written, + * the decision script as written, and each job's real `if:`. A guard that + * reimplemented the decision would agree with a rewritten one by luck. + */ + +export const SELECTOR_JOB = 'selector' + +/** The events the selector decides nothing for: each one selects every job. */ +export const NON_PULL_REQUEST_EVENTS = [ + 'push', + 'schedule', + 'merge_group', + 'workflow_dispatch', +] + +const DORNY = 'dorny/paths-filter@' + +const stepOutputRef = (expression) => + /steps\.([A-Za-z0-9_-]+)\.outputs\.([A-Za-z0-9_-]+)/.exec( + String(expression ?? ''), + ) + +const selectorOutputRefs = (expression) => + [ + ...String(expression ?? '').matchAll( + new RegExp(`needs\\.${SELECTOR_JOB}\\.outputs\\.([A-Za-z0-9_-]+)`, 'g'), + ), + ].map((match) => match[1]) + +/** + * `{a,b}` expanded into its alternatives, as picomatch reads a brace pattern. + * One level only: a nested brace throws, rather than being read wrongly. + */ +export function expandBraces(pattern) { + const open = pattern.indexOf('{') + if (open === -1) return [pattern] + const close = pattern.indexOf('}', open) + const inner = pattern.slice(open + 1, close) + if (close === -1 || inner.includes('{')) { + throw new Error(`Cannot expand the brace pattern ${pattern}`) + } + const head = pattern.slice(0, open) + return inner + .split(',') + .flatMap((alternative) => + expandBraces(`${head}${alternative}${pattern.slice(close + 1)}`), + ) +} + +/** One dorny/paths-filter rule against one file. A `!` rule negates. */ +export function dornyRuleMatches(rule, file) { + if (rule.startsWith('!')) return !dornyRuleMatches(rule.slice(1), file) + return expandBraces(rule).some((glob) => pathsGlobToRegExp(glob).test(file)) +} + +/** + * Whether a file counts for a filter. Under dorny's default quantifier, `some`, + * one matching rule is enough; under `every`, all of them must match. + */ +export function dornyFilterMatches(rules, quantifier, file) { + return quantifier === 'every' + ? rules.every((rule) => dornyRuleMatches(rule, file)) + : rules.some((rule) => dornyRuleMatches(rule, file)) +} + +/** + * A workflow's selector job, parsed, or `null` when it has none. + * + * `filters` maps each filter name to its rules. A filter that includes an + * anchored list (`- *name`) parses to a nested list, and dorny flattens it; + * so does this. `outputs` maps each selector output to the decision step + * output it reads. + */ +export function readSelectorJob(relPath) { + const workflow = readWorkflow(relPath) + const job = workflow?.jobs?.[SELECTOR_JOB] + if (!job) return null + const steps = Array.isArray(job.steps) ? job.steps : [] + const filterSteps = steps.filter((step) => + String(step?.uses ?? '').startsWith(DORNY), + ) + const filterStep = filterSteps.length === 1 ? filterSteps[0] : null + const filters = Object.fromEntries( + Object.entries(yaml.load(filterStep?.with?.filters ?? '') ?? {}).map( + ([name, rules]) => [name, [rules].flat(Infinity).map(String)], + ), + ) + const outputs = Object.fromEntries( + Object.entries(job.outputs ?? {}).map(([name, expression]) => [ + name, + stepOutputRef(expression), + ]), + ) + const decideIds = [ + ...new Set(Object.values(outputs).map((ref) => ref?.[1])), + ].filter(Boolean) + const decideStep = + decideIds.length === 1 + ? steps.find((step) => step?.id === decideIds[0]) + : null + return { + workflow, + job, + filterSteps, + filterStep, + quantifier: filterStep?.with?.['predicate-quantifier'] ?? 'some', + filters, + outputs: Object.fromEntries( + Object.entries(outputs).map(([name, ref]) => [name, ref?.[2]]), + ), + decideStep, + } +} + +/** The selector outputs a job's `if:` reads. */ +export function selectorOutputsRead(job) { + return [...new Set(selectorOutputRefs(job?.if))] +} + +/** + * Runs the selector's decision script the way GitHub would: `answers` stands + * in for the filter step's outputs, keyed by filter name. An env entry that + * names no answer gets the empty string, as GitHub substitutes for an + * expression that resolves to nothing. + * + * Returns the exit status, the log, and the selector job's outputs. + */ +export function runSelectorDecision(relPath, eventName, answers) { + const selector = readSelectorJob(relPath) + const scratch = mkdtempSync(join(tmpdir(), 'selector-')) + try { + const outputFile = join(scratch, 'outputs.txt') + writeFileSync(outputFile, '') + const env = { PATH: process.env.PATH, GITHUB_OUTPUT: outputFile } + for (const [key, value] of Object.entries( + selector?.decideStep?.env ?? {}, + )) { + const text = String(value) + if (/github\.event_name/.test(text)) env[key] = eventName + else env[key] = answers[stepOutputRef(text)?.[2]] ?? '' + } + const run = spawnSync( + 'bash', + ['-c', String(selector?.decideStep?.run ?? 'exit 99')], + { cwd: scratch, env, encoding: 'utf8' }, + ) + const written = Object.fromEntries( + readFileSync(outputFile, 'utf8') + .split('\n') + .filter(Boolean) + .map((line) => [ + line.slice(0, line.indexOf('=')), + line.slice(line.indexOf('=') + 1), + ]), + ) + return { + status: run.status, + log: `${run.stdout}${run.stderr}`, + outputs: Object.fromEntries( + Object.entries(selector?.outputs ?? {}).map(([name, stepOutput]) => [ + name, + written[stepOutput], + ]), + ), + } + } finally { + rmSync(scratch, { recursive: true, force: true }) + } +} + +/** What each filter answers for a pull request that changes `files`. */ +export function filterAnswers(relPath, files) { + const selector = readSelectorJob(relPath) + return Object.fromEntries( + Object.entries(selector?.filters ?? {}).map(([name, rules]) => [ + name, + String( + files.some((file) => + dornyFilterMatches(rules, selector.quantifier, file), + ), + ), + ]), + ) +} + +const needsOf = (job) => [job?.needs ?? []].flat() + +/** + * The jobs that run for a same-repository pull request changing `files`, + * sorted. The selector job's outputs come from its own script; each job's + * `if:` is evaluated as GitHub does, including the implicit `success()` that + * skips a job when one it needs did not succeed. + * + * `github` adds to the context, for a job whose `if:` reads more than the + * event and the selector (such as `head_ref`). + */ +export function selectedJobs(relPath, files, { github = {} } = {}) { + const selector = readSelectorJob(relPath) + const decision = runSelectorDecision( + relPath, + 'pull_request', + filterAnswers(relPath, files), + ) + if (decision.status !== 0) { + throw new Error(`The selector failed:\n${decision.log}`) + } + const repository = 'cipherstash/stack' + const context = { + github: { + event_name: 'pull_request', + repository, + head_ref: 'feature', + event: { + pull_request: { + draft: false, + head: { repo: { full_name: repository } }, + user: { type: 'User' }, + }, + }, + ...github, + }, + needs: {}, + vars: {}, + } + const jobs = selector.workflow.jobs + const results = { + [SELECTOR_JOB]: { result: 'success', outputs: decision.outputs }, + } + const resolve = (id) => { + if (results[id]) return results[id].result + const job = jobs[id] + const needs = needsOf(job) + const needResults = needs.map(resolve) + const condition = String(job?.if ?? '') + const statusFunction = /\b(always|failure|cancelled|success)\(\)/.test( + condition, + ) + let runs + if (!statusFunction && needResults.some((result) => result !== 'success')) { + runs = false + } else if (!condition) { + runs = true + } else { + runs = runsWhen(condition, { + ...context, + needs: Object.fromEntries(needs.map((need) => [need, results[need]])), + }) + } + results[id] = { result: runs ? 'success' : 'skipped', outputs: {} } + return results[id].result + } + return Object.keys(jobs) + .filter((id) => resolve(id) === 'success') + .sort() +} + +// GitHub reads a `paths:` list in order, and the last matching entry decides. +export function pathsListSelects(paths, file) { + let selected = false + for (const entry of paths) { + if (entry.startsWith('!')) { + if (pathsGlobToRegExp(entry.slice(1)).test(file)) selected = false + } else if (pathsGlobToRegExp(entry).test(file)) { + selected = true + } + } + return selected +} + +/** + * A filter as a GitHub `paths:` list: entries in order, a `!` entry excluding + * what came before it. Lets the guards that check a workflow's `paths:` + * coverage read a selector filter with the same code. + * + * Under `every`, the one positive rule's brace alternatives come first and the + * `!` rules after, which selects exactly the files dorny selects. Under + * `some`, a filter has no `!` rule (`workflow-selectors.test.mjs`), so it is + * already that list. + */ +export function filterAsPathsList(rules, quantifier) { + if (quantifier !== 'every') return rules.flatMap(expandBraces) + const positive = rules.filter((rule) => !rule.startsWith('!')) + const negative = rules.filter((rule) => rule.startsWith('!')) + return [...positive.flatMap(expandBraces), ...negative] +} + +/** + * The path lists that decide whether a job runs on a change, as GitHub + * `paths:` lists: the workflow's `on.push.paths` and `on.pull_request.paths` + * where they exist, and each selector filter the job's `if:` reads. + * + * A job in a selector workflow that reads no filter runs on every pull + * request, so only the `push` list, if any, decides it. + */ +export function jobTriggerLists(relPath, jobId) { + const workflow = readWorkflow(relPath) + const on = workflow?.on ?? workflow?.[true] + const lists = ['push', 'pull_request'] + .filter((event) => Array.isArray(on?.[event]?.paths)) + .map((event) => ({ + source: `on.${event}.paths`, + paths: on[event].paths.map(String), + })) + const selector = readSelectorJob(relPath) + for (const name of selectorOutputsRead(workflow?.jobs?.[jobId])) { + const rules = selector?.filters?.[name] + if (!rules) continue + lists.push({ + source: `selector filter "${name}"`, + paths: filterAsPathsList(rules, selector.quantifier), + }) + } + return lists +} + +/** + * Runs the summary job's check with the given results of the jobs it needs, + * and returns its exit status. The results stand in for + * `join(needs.*.result, ' ')`, wherever the step reads it. + */ +export function runSummaryCheck(relPath, summaryId, results) { + const workflow = readWorkflow(relPath) + const steps = workflow?.jobs?.[summaryId]?.steps ?? [] + const joined = results.join(' ') + const resultsExpression = /\$\{\{\s*join\(needs\.\*\.result,\s*' '\)\s*\}\}/g + let status = 0 + for (const step of steps) { + if (typeof step?.run !== 'string') continue + const env = { PATH: process.env.PATH } + for (const [key, value] of Object.entries(step.env ?? {})) { + env[key] = String(value).replace(resultsExpression, joined) + } + const run = spawnSync( + 'bash', + ['-c', step.run.replace(resultsExpression, joined)], + { env, encoding: 'utf8' }, + ) + status = run.status + if (status !== 0) break + } + return status +} diff --git a/scripts/__tests__/lint-release-scope.test.mjs b/scripts/__tests__/lint-release-scope.test.mjs index fa9f67142..9333c2e21 100644 --- a/scripts/__tests__/lint-release-scope.test.mjs +++ b/scripts/__tests__/lint-release-scope.test.mjs @@ -2,10 +2,11 @@ import { readFileSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { REPO_ROOT } from './lib/repo-root.mjs' +import { jobTriggerLists } from './lib/selector.mjs' import { readWorkflow, WORKFLOW_DIR, workflowFiles } from './lib/workflows.mjs' /** - * `lint-release.yml` states its scope TWICE — once as the `pull_request` paths + * `lint-release.yml` states its scope TWICE — once as the selector job's paths * filter that decides when the job runs, once as the argument list actionlint * is actually pointed at. Nothing bound the two. * @@ -22,13 +23,9 @@ import { readWorkflow, WORKFLOW_DIR, workflowFiles } from './lib/workflows.mjs' const LINT_RELEASE = `${WORKFLOW_DIR}/lint-release.yml` -const workflow = readWorkflow(LINT_RELEASE) -// `on:` parses as the boolean `true` under YAML 1.1 — the Norway problem. -const triggers = workflow.on ?? workflow[true] - -const filtered = triggers.pull_request.paths.filter((path) => - path.startsWith(`${WORKFLOW_DIR}/`), -) +const filtered = jobTriggerLists(LINT_RELEASE, 'lint') + .flatMap(({ paths }) => paths) + .filter((path) => path.startsWith(`${WORKFLOW_DIR}/`)) const source = readFileSync(join(REPO_ROOT, LINT_RELEASE), 'utf8') const linted = [ diff --git a/scripts/__tests__/musl-build-image.test.mjs b/scripts/__tests__/musl-build-image.test.mjs index 84188b357..454cb9d3d 100644 --- a/scripts/__tests__/musl-build-image.test.mjs +++ b/scripts/__tests__/musl-build-image.test.mjs @@ -3,6 +3,7 @@ import { join } from 'node:path' import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' import { REPO_ROOT } from './lib/repo-root.mjs' +import { jobTriggerLists } from './lib/selector.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** @@ -238,8 +239,12 @@ describe('keeping the image current', () => { const wf = readWorkflow(relPath) const on = wf?.on ?? wf?.[true] expect(on?.schedule).toEqual([{ cron: '0 7 * * 1' }]) - for (const trigger of ['push', 'pull_request']) { - expect(on?.[trigger]?.paths).toEqual( + // `push` lists the paths itself; a pull request takes them from the + // selector job's filter. + const lists = jobTriggerLists(relPath, 'build').map(({ source }) => source) + expect(lists).toEqual(['on.push.paths', 'selector filter "musl_image"']) + for (const { paths } of jobTriggerLists(relPath, 'build')) { + expect(paths).toEqual( expect.arrayContaining([`${DOCKERFILE_DIR}/**`, relPath]), ) } diff --git a/scripts/__tests__/other-selectors.test.mjs b/scripts/__tests__/other-selectors.test.mjs new file mode 100644 index 000000000..514beecbb --- /dev/null +++ b/scripts/__tests__/other-selectors.test.mjs @@ -0,0 +1,100 @@ +import { describe, expect, it } from 'vitest' +import { readSelectorJob, selectedJobs } from './lib/selector.mjs' + +/** + * What the selector jobs of the release lint, the musl image build, CodeQL and + * OSV-Scanner decide for a pull request. + * + * CodeQL analyses JavaScript and TypeScript only, so its trigger paths are the + * file types the JavaScript extractor reads. OSV-Scanner reads dependencies + * from lockfiles and manifests, so its trigger paths are those files. + */ + +const WORKFLOWS = { + lintRelease: '.github/workflows/lint-release.yml', + musl: '.github/workflows/musl-build-image.yml', + codeql: '.github/workflows/codeql.yml', + osv: '.github/workflows/osv-scanner.yml', +} + +const runs = (relPath, job, file) => selectedJobs(relPath, [file]).includes(job) + +describe('the other pull request workflows', () => { + it.each(Object.values(WORKFLOWS))('%s has a selector job', (relPath) => { + expect(readSelectorJob(relPath)).not.toBeNull() + }) +}) + +describe('the release lint selector', () => { + it('runs for the release workflows and the scripts it checks', () => { + for (const file of [ + '.github/workflows/release.yml', + '.github/actionlint.yaml', + 'packages/eql/tasks/release/prepare-bindings-assets.sh', + ]) { + expect(runs(WORKFLOWS.lintRelease, 'lint', file), file).toBe(true) + } + expect( + runs(WORKFLOWS.lintRelease, 'lint', 'scripts/release-gate.mjs'), + ).toBe(false) + }) +}) + +describe('the musl image selector', () => { + it('builds the image for a change to the Dockerfile only', () => { + expect( + runs(WORKFLOWS.musl, 'build', '.github/docker/musl-build/Dockerfile'), + ).toBe(true) + expect(runs(WORKFLOWS.musl, 'build', 'Cargo.lock')).toBe(false) + }) +}) + +describe('the CodeQL selector', () => { + it('runs the analysis for JavaScript, TypeScript and the files they read', () => { + for (const file of [ + 'languages/typescript/packages/stack/src/index.ts', + 'scripts/release-gate.mjs', + 'languages/typescript/packages/stack/package.json', + '.github/workflows/codeql.yml', + ]) { + expect(runs(WORKFLOWS.codeql, 'analyze', file), file).toBe(true) + } + }) + + it('skips the analysis for Rust, Go, SQL and Markdown', () => { + for (const file of [ + 'packages/stack-encrypt/src/lib.rs', + 'languages/golang/encrypt/encrypt.go', + 'packages/eql/src/v3/schema.sql', + 'README.md', + ]) { + expect(runs(WORKFLOWS.codeql, 'analyze', file), file).toBe(false) + } + }) +}) + +describe('the OSV-Scanner selector', () => { + it('runs the scan for a lockfile or manifest anywhere in the tree', () => { + for (const file of [ + 'pnpm-lock.yaml', + 'Cargo.lock', + 'packages/eql/Cargo.lock', + 'languages/golang/go.mod', + '.github/impact-gate/requirements.txt', + 'languages/typescript/examples/deno/deno.lock', + ]) { + expect(runs(WORKFLOWS.osv, 'osv-scan', file), file).toBe(true) + } + }) + + it('skips the scan for source code', () => { + expect( + runs( + WORKFLOWS.osv, + 'osv-scan', + 'languages/typescript/packages/stack/src/index.ts', + ), + ).toBe(false) + expect(runs(WORKFLOWS.osv, 'osv-scan', 'Cargo.toml')).toBe(false) + }) +}) diff --git a/scripts/__tests__/wasm-build-inputs-paths-filter.test.mjs b/scripts/__tests__/wasm-build-inputs-paths-filter.test.mjs index 28d96ad94..8f28725ad 100644 --- a/scripts/__tests__/wasm-build-inputs-paths-filter.test.mjs +++ b/scripts/__tests__/wasm-build-inputs-paths-filter.test.mjs @@ -4,6 +4,7 @@ import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' import { filterCovers } from './lib/paths-filter.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' +import { jobTriggerLists } from './lib/selector.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** @@ -57,9 +58,6 @@ const ACTION = '.github/actions/build-ffi-binding/action.yml' const ACTION_USES = './.github/actions/build-ffi-binding' const CACHE_ACTION = /^actions\/cache(\/(restore|save))?@/ -/** Both trigger events that carry a `paths:` filter, in GitHub's own order. */ -const FILTERED_EVENTS = ['push', 'pull_request'] - /** The package whose Rust both cache keys are about. */ const FFI_PACKAGE = 'languages/typescript/packages/protect-ffi' const FFI_PACKAGE_NAME = '@cipherstash/protect-ffi' @@ -276,21 +274,34 @@ function stepDir(wf, job, step) { /** Every job step of a workflow, paired with the job that owns it. */ function steps(wf) { - return Object.values(wf?.jobs ?? {}).flatMap((job) => - (Array.isArray(job?.steps) ? job.steps : []).map((step) => ({ job, step })), + return Object.entries(wf?.jobs ?? {}).flatMap(([jobId, job]) => + (Array.isArray(job?.steps) ? job.steps : []).map((step) => ({ + jobId, + job, + step, + })), ) } -/** Runs `./.github/actions/build-ffi-binding`, optionally with `wasm: 'true'`. */ -function usesAction(relPath, { wasmOnly = false } = {}) { - return steps(readWorkflow(relPath)).some( - ({ step }) => - typeof step?.uses === 'string' && - step.uses.trim() === ACTION_USES && - (!wasmOnly || String(step?.with?.wasm) === 'true'), +const jobIdsOf = (entries) => [...new Set(entries.map(({ jobId }) => jobId))] + +/** + * The jobs that run `./.github/actions/build-ffi-binding`, optionally with + * `wasm: 'true'`. + */ +function actionJobs(relPath, { wasmOnly = false } = {}) { + return jobIdsOf( + steps(readWorkflow(relPath)).filter( + ({ step }) => + typeof step?.uses === 'string' && + step.uses.trim() === ACTION_USES && + (!wasmOnly || String(step?.with?.wasm) === 'true'), + ), ) } +const usesAction = (relPath, options) => actionJobs(relPath, options).length > 0 + /** * Compiles this package's Rust — through the action, or directly. * @@ -300,10 +311,9 @@ function usesAction(relPath, { wasmOnly = false } = {}) { * EQL workspace has its own, with its own workflows — is not mistaken for this * one. */ -function compilesFfiRust(relPath) { - if (usesAction(relPath)) return true +function ffiRustJobs(relPath) { const wf = readWorkflow(relPath) - return steps(wf).some(({ job, step }) => { + const direct = steps(wf).filter(({ job, step }) => { const run = String(step?.run ?? '') if (!run) return false const targetsPackage = @@ -316,24 +326,28 @@ function compilesFfiRust(relPath) { (inPackage && mentionsAny(run, CARGO_TASKS)) ) }) + return [...new Set([...actionJobs(relPath), ...jobIdsOf(direct)])] } +const compilesFfiRust = (relPath) => ffiRustJobs(relPath).length > 0 + /** - * The `paths:` filters a workflow declares, one entry per filtered event. + * The path lists that decide whether the given jobs run: the workflow's `push` + * and `pull_request` `paths:` lists, and the selector filter each job reads. + * One entry per list, labelled with where it comes from. * - * `on:` parses as the boolean `true` under YAML 1.1 (the "Norway problem"), - * hence `wf.on ?? wf[true]`. An event with no `paths:` is UNFILTERED and is - * skipped rather than failed — it already runs on every diff, so it cannot miss - * one. `tests.yml` and `tests-rust.yml` are both half-cases: `push` is - * `branches: [main]` with no filter, so only their `pull_request` lists are - * here. `tests.yml`'s is `'**'` with one negation, which `filterCovers` reads. + * A job that no list decides is skipped rather than failed: it runs on every + * diff, so it cannot miss one. A selector filter is read as a `paths:` list, + * so a `'**'` with negations is read the same way in either. */ -function filteredEvents(relPath) { - const wf = readWorkflow(relPath) - const on = wf?.on ?? wf?.[true] - return FILTERED_EVENTS.filter((event) => - Array.isArray(on?.[event]?.paths), - ).map((event) => ({ relPath, event, paths: on[event].paths })) +function filteredEvents(relPath, jobIds) { + const lists = new Map() + for (const jobId of jobIds) { + for (const { source, paths } of jobTriggerLists(relPath, jobId)) { + lists.set(source, { relPath, event: source, paths }) + } + } + return [...lists.values()] } const ALL_WORKFLOWS = workflowFiles() @@ -381,6 +395,7 @@ const KEYS = [ what: '`index.node`, the binding every credentialed suite encrypts through', reaches: 'compiles the crate graph behind `index.node`', workflows: ALL_WORKFLOWS.filter(compilesFfiRust), + jobs: ffiRustJobs, expected: [ '.github/workflows/_build-ffi-artifacts.yml', '.github/workflows/integration-drizzle.yml', @@ -401,6 +416,7 @@ const KEYS = [ workflows: ALL_WORKFLOWS.filter((relPath) => usesAction(relPath, { wasmOnly: true }), ), + jobs: (relPath) => actionJobs(relPath, { wasmOnly: true }), expected: [ '.github/workflows/integration-drizzle.yml', '.github/workflows/integration-protect-ffi.yml', @@ -447,7 +463,9 @@ describe('the FFI cache keys and the filters that trigger them agree', () => { for (const key of KEYS) { const inputs = buildInputs(key.step) - const filtered = key.workflows.flatMap(filteredEvents) + const filtered = key.workflows.flatMap((relPath) => + filteredEvents(relPath, key.jobs(relPath)), + ) describe(`the ${key.id} key`, () => { it('hashes build inputs that name real files', () => { @@ -506,7 +524,7 @@ describe('the FFI cache keys and the filters that trigger them agree', () => { ) expect( uncovered, - `These files are hashed into the ${key.id} cache key of \`${ACTION}\`, so an edit to one MISSES the cache and rebuilds ${key.what}. ${relPath} ${key.reaches}, but its \`${event}\` \`paths:\` filter does not list them, so a PR touching only one of them rebuilds the artifact and starts no suite that exercises it. Nothing goes red; nothing ran.\nAdd each to BOTH copies of the filter — GitHub Actions has no YAML anchors, and \`workflow-paths-filter-parity.test.mjs\` fails a one-sided edit.`, + `These files are hashed into the ${key.id} cache key of \`${ACTION}\`, so an edit to one MISSES the cache and rebuilds ${key.what}. ${relPath} ${key.reaches}, but its \`${event}\` \`paths:\` filter does not list them, so a PR touching only one of them rebuilds the artifact and starts no suite that exercises it. Nothing goes red; nothing ran.\nAdd each to every copy of the list — GitHub Actions has no YAML anchors, and \`workflow-paths-filter-parity.test.mjs\` fails a one-sided edit.`, ).toEqual([]) }) } @@ -521,7 +539,7 @@ describe('the FFI cache keys and the filters that trigger them agree', () => { KEYS.flatMap((key) => { const inputs = buildInputs(key.step) return key.workflows - .flatMap(filteredEvents) + .flatMap((relPath) => filteredEvents(relPath, key.jobs(relPath))) .flatMap(({ relPath, paths }) => inputs .filter((pattern) => !filterCovers(paths, pattern, entryCovers)) diff --git a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs index e727e71fe..d7a6ea95e 100644 --- a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs +++ b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs @@ -180,7 +180,11 @@ function extractForkClause(condition) { * run. */ const PERMISSIVE_NEEDS = { - selector: { outputs: { eql: 'true', rust_crates: 'true' } }, + // Every selector output, whatever its name: each workflow's selector names + // its outputs after its own filters. + selector: { + outputs: new Proxy({}, { has: () => true, get: () => 'true' }), + }, // The release workflows' gates, none of which is about how the run was // triggered. Held open, or "skips because there is nothing to publish" would // be indistinguishable from "skips on a dispatch". diff --git a/scripts/__tests__/workflow-paths-filter-parity.test.mjs b/scripts/__tests__/workflow-paths-filter-parity.test.mjs index 74ab3af62..50be41fea 100644 --- a/scripts/__tests__/workflow-paths-filter-parity.test.mjs +++ b/scripts/__tests__/workflow-paths-filter-parity.test.mjs @@ -1,4 +1,11 @@ +import { execFileSync } from 'node:child_process' import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { + dornyFilterMatches, + pathsListSelects, + readSelectorJob, +} from './lib/selector.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** @@ -57,6 +64,14 @@ function pathsFilters(relPath) { ) } +/** Every tracked file: what a `paths:` list or a filter can select. */ +const TRACKED = execFileSync('git', ['ls-files', '-z'], { + cwd: REPO_ROOT, + encoding: 'utf8', +}) + .split('\0') + .filter(Boolean) + const CLASSIFIED = workflowFiles().map((relPath) => { const filters = pathsFilters(relPath) const declared = FILTERED_EVENTS.filter((event) => filters[event] !== null) @@ -66,8 +81,23 @@ const CLASSIFIED = workflowFiles().map((relPath) => { /** Workflows writing the list twice — the ones the parity check applies to. */ const PARITY = CLASSIFIED.filter((entry) => entry.declared.length === 2) +/** + * Workflows whose selector job decides what a pull request runs, and whose + * `push` keeps a `paths:` list. Their `pull_request` has no filter, by design + * (ADR 0003), so the second copy of the list is the selector's filters. The + * check below compares the two by what they select. + */ +const SELECTOR_PUSH = CLASSIFIED.filter( + (entry) => + entry.filters.push !== null && + entry.filters.pull_request === null && + readSelectorJob(entry.relPath) !== null, +) + /** Workflows filtering one event and not the other. */ -const ASYMMETRIC = CLASSIFIED.filter((entry) => entry.declared.length === 1) +const ASYMMETRIC = CLASSIFIED.filter( + (entry) => entry.declared.length === 1 && !SELECTOR_PUSH.includes(entry), +) /** * The guard on the scan, in the shape the rest of this directory uses. Every @@ -129,40 +159,12 @@ const EXPECTED_ASYMMETRIES = new Map([ // `pull_request` one is unreachable when `push` is unfiltered. 'push is deliberately unfiltered (branches: [main]), so main runs a superset of what PRs run', ], - [ - '.github/workflows/lint-release.yml', - // There is no `push:` trigger at all — this gate exists to catch a broken - // release workflow BEFORE it merges, and a copy running after the merge - // would report the same finding at the moment it is no longer useful. With - // one event there is no second list to drift from, and the direction that - // would hurt (a narrower `pull_request` filter, so the PR that breaks the - // release machinery never runs the check) is what the single list IS. - 'pull_request is the only trigger; a post-merge copy would report a release-blocking finding too late to act on', - ], [ '.github/workflows/require-auth-npm-changeset.yml', // Its list is held to the job's own `git diff` pathspec by // check-auth-npm-changeset.test.mjs, which is the second copy here. 'pull_request is the only trigger; the check diffs a pull request against its base, which a push does not have', ], - [ - '.github/workflows/test-eql.yml', - // `pull_request` is deliberately UNFILTERED and `push: branches: [main]` - // carries the list, which is the same shape as tests-rust.yml with the two - // events swapped. Unfiltered on the PR side because this workflow's - // `ci-required` is the check branch protection is meant to reference, and a - // workflow skipped by a path filter leaves a required check stuck Pending; - // relevance is applied per-job via `if:` instead. Filtered on the push side - // because nothing waits on a push run, so skipping one costs nothing. - // - // The direction that would hurt — a `pull_request` filter narrower than the - // `push` one, so the PR introducing a change never runs the suite — is - // unreachable when `pull_request` has no filter at all. The push list is - // not unchecked: `eql-workflow-filters.test.mjs` compares it against the - // union of the selector job's filters, which is the second copy of the - // same list and the one this file cannot see. - 'pull_request is deliberately unfiltered (per-job relevance instead), so PRs run a superset of what push runs; the push list is compared against the selector job filters by eql-workflow-filters.test.mjs', - ], [ '.github/workflows/bench-eql.yml', // The mirror image of lint-release.yml: `push: branches: [main]` with a @@ -235,6 +237,27 @@ describe('paths filters are written twice, identically', () => { ).toEqual([]) }) + for (const { relPath, filters } of SELECTOR_PUSH) { + it(`${relPath} selects on push what its selector selects on a pull request`, () => { + // A selector workflow writes its list twice as well: as the `push` + // `paths:` list, and split across the selector job's filters. The two + // are written in different syntaxes, so they are compared by what they + // select from the tracked files, not by their text. + const selector = readSelectorJob(relPath) + const differ = TRACKED.filter( + (file) => + pathsListSelects(filters.push, file) !== + Object.values(selector.filters).some((rules) => + dornyFilterMatches(rules, selector.quantifier, file), + ), + ) + expect( + differ, + `In ${relPath}, the \`push\` \`paths:\` list and the union of the selector job's filters disagree about these files. Change both in the same edit.`, + ).toEqual([]) + }) + } + for (const { relPath } of PARITY) { it(`${relPath} filters push and pull_request identically`, () => { const filters = pathsFilters(relPath) diff --git a/scripts/__tests__/workflow-selectors.test.mjs b/scripts/__tests__/workflow-selectors.test.mjs new file mode 100644 index 000000000..341b95f8d --- /dev/null +++ b/scripts/__tests__/workflow-selectors.test.mjs @@ -0,0 +1,220 @@ +import { describe, expect, it } from 'vitest' +import { runsWhen } from './lib/expressions.mjs' +import { + NON_PULL_REQUEST_EVENTS, + readSelectorJob, + runSelectorDecision, + runSummaryCheck, + SELECTOR_JOB, + selectorOutputsRead, +} from './lib/selector.mjs' +import { workflowFiles } from './lib/workflows.mjs' + +/** + * Every workflow with a selector job follows the same design (ADR 0003): the + * selector job decides which checks a pull request runs, each gated job reads + * the selector's answer in its `if:`, and a summary job ends the workflow. + * `.github/CONTEXT.md` defines the terms. + * + * The workflows are discovered, not listed, so a workflow that gains a + * selector job is held to this the day it lands. What each selector decides + * for particular changes is pinned per workflow, in the `*-selector*.test.mjs` + * files. + * + * Whether every job is decided by the selector, or has a written reason to run + * on every change, is the failsafe's question, not this file's. + */ + +const SELECTOR_WORKFLOWS = workflowFiles().filter( + (relPath) => readSelectorJob(relPath) !== null, +) + +/** + * The guard on the scan. A minimum: a workflow that gains a selector job must + * not fail this. Each workflow's own `*-selector*.test.mjs` file checks that + * its selector job exists. + */ +const EXPECTED_SELECTOR_WORKFLOWS = ['.github/workflows/test-eql.yml'] + +const triggersOf = (workflow) => workflow?.on ?? workflow?.[true] + +const needsOf = (job) => [job?.needs ?? []].flat() + +/** The jobs whose `if:` reads a selector output. */ +function gatedJobs(workflow) { + return Object.entries(workflow?.jobs ?? {}) + .map(([id, job]) => ({ id, job, reads: selectorOutputsRead(job) })) + .filter((entry) => entry.reads.length > 0) +} + +describe('the workflows with a selector job', () => { + it('include the converted workflows', () => { + expect(SELECTOR_WORKFLOWS).toEqual( + expect.arrayContaining(EXPECTED_SELECTOR_WORKFLOWS), + ) + }) +}) + +for (const relPath of SELECTOR_WORKFLOWS) { + const selector = readSelectorJob(relPath) + const { workflow } = selector + const jobIds = Object.keys(workflow.jobs) + const summaryId = jobIds.at(-1) + const filterNames = Object.keys(selector.filters) + const outputNames = Object.keys(selector.outputs) + + describe(relPath, () => { + it('has no path filter on pull_request', () => { + // A workflow that a path filter skips leaves its required checks + // pending, and GitHub's documentation on required status checks says + // that blocks a merge. The selector job decides instead. + const pullRequest = triggersOf(workflow)?.pull_request ?? {} + expect(Object.keys(pullRequest)).not.toContain('paths') + expect(Object.keys(pullRequest)).not.toContain('paths-ignore') + }) + + it('starts with the selector job, on a GitHub-hosted runner', () => { + // ADR 0003: every gated job waits for it, and a self-provisioned + // machine takes longer to start. + expect(jobIds[0]).toBe(SELECTOR_JOB) + expect(String(selector.job['runs-on'])).toMatch(/^ubuntu-/) + expect(selector.job.needs).toBeUndefined() + expect(selector.job.if).toBeUndefined() + }) + + it('has one path filter step, run only on a pull request', () => { + expect(selector.filterSteps).toHaveLength(1) + expect(String(selector.filterStep?.if)).toBe( + "github.event_name == 'pull_request'", + ) + expect(filterNames).not.toEqual([]) + }) + + it('writes no `!` rule that dorny would read as matching everything', () => { + // Under dorny's default quantifier, `some`, a file counts when ANY rule + // matches, so `!**/*.md` matches every file that is not Markdown. A `!` + // rule only excludes under `every`, where a file must match every rule, + // and there a second positive rule would select nothing at all. + expect(['some', 'every']).toContain(selector.quantifier) + for (const [name, rules] of Object.entries(selector.filters)) { + const positive = rules.filter((rule) => !rule.startsWith('!')) + const negative = rules.filter((rule) => rule.startsWith('!')) + if (selector.quantifier === 'every') { + expect(positive, `filter "${name}"`).toHaveLength(1) + } else { + expect(negative, `filter "${name}"`).toEqual([]) + } + } + }) + + it('names each output after the filter that decides it', () => { + // The guards follow a job's `if:` to its filter by this name. + expect(outputNames.sort()).toEqual([...filterNames].sort()) + for (const [name, stepOutput] of Object.entries(selector.outputs)) { + expect(stepOutput, `output "${name}"`).toBe(name) + } + expect( + selector.decideStep, + 'No single step writes the selector outputs.', + ).toBeTruthy() + }) + + it('selects every job on events other than pull_request', () => { + for (const event of NON_PULL_REQUEST_EVENTS) { + const { status, log, outputs } = runSelectorDecision(relPath, event, {}) + expect(status, log).toBe(0) + for (const name of outputNames) { + expect(outputs[name], `${name} on ${event}`).toBe('true') + } + } + }) + + it('passes the filter answers through on a pull request', () => { + for (const answer of ['true', 'false']) { + const answers = Object.fromEntries( + filterNames.map((name) => [name, answer]), + ) + const { status, log, outputs } = runSelectorDecision( + relPath, + 'pull_request', + answers, + ) + expect(status, log).toBe(0) + expect(outputs).toEqual(answers) + } + }) + + it('fails when a pull request filter gives no answer', () => { + // A missing answer would skip every gated job, and the summary job + // counts skipped as passed: a green run that ran nothing. + for (const name of filterNames) { + for (const bad of ['', 'maybe']) { + const answers = Object.fromEntries( + filterNames.map((other) => [other, other === name ? bad : 'true']), + ) + const { status } = runSelectorDecision( + relPath, + 'pull_request', + answers, + ) + expect(status, `filter "${name}" answering '${bad}'`).not.toBe(0) + } + } + }) + + it('gates jobs only on outputs the selector writes, and needs it', () => { + const gated = gatedJobs(workflow) + expect(gated).not.toEqual([]) + for (const { id, job, reads } of gated) { + expect(needsOf(job), `job "${id}"`).toContain(SELECTOR_JOB) + for (const name of reads) { + expect(outputNames, `job "${id}" reads "${name}"`).toContain(name) + } + } + }) + + it("writes each gated job's `if:` as an exclusion of pull_request", () => { + // Outside a pull request the selector's answers must not matter: a job + // written as a list of allowed events skips on the event nobody listed. + for (const { id, job } of gatedJobs(workflow)) { + for (const event of NON_PULL_REQUEST_EVENTS) { + const verdict = (answer) => + runsWhen(job.if, { + github: { + event_name: event, + repository: 'cipherstash/stack', + event: {}, + }, + needs: { + [SELECTOR_JOB]: { + result: 'success', + outputs: Object.fromEntries( + outputNames.map((name) => [name, answer]), + ), + }, + }, + vars: {}, + }) + expect(verdict('false'), `job "${id}" on ${event}`).toBe( + verdict('true'), + ) + } + } + }) + + it('ends with a summary job that needs every other job', () => { + const summary = workflow.jobs[summaryId] + expect(String(summary.if)).toBe('always()') + expect([...needsOf(summary)].sort()).toEqual( + jobIds.filter((id) => id !== summaryId).sort(), + ) + }) + + it('passes the summary only when every needed job passed or skipped', () => { + const status = (results) => runSummaryCheck(relPath, summaryId, results) + expect(status(['success', 'skipped', 'success'])).toBe(0) + expect(status(['success', 'failure'])).not.toBe(0) + expect(status(['skipped', 'cancelled'])).not.toBe(0) + }) + }) +}