From 4d65e8984a1e50d9a437f3236a6193d9968cd99e Mon Sep 17 00:00:00 2001 From: localhost Date: Mon, 7 Sep 2026 07:23:05 +0000 Subject: [PATCH] chore(ci): Implement /query-regression command handling and admission workflow (#8975) * Implement `/query-regression` command handling and admission workflow - Add `query-regression-slash.py` script for processing `/query-regression` commands in PR comments, validating case arguments, and checking permissions. - Update `checks.yml` to include tests for the new slash command functionality. - Modify `query-regression-comment.yml` to trigger on the new `Query Regression Command` workflow. - Create `query-regression-slash.yml` to handle the dispatched command, validate allowlist and permissions, and initiate the regression workflow. - Enhance `query-regression.yml` to support additional inputs for PR admission and SHA verification. - Introduce `slash-command-dispatch.yml` to parse and dispatch commands from PR comments. - Document the new command admission process in `AGENTS.md` and `README.md`. - Add unit tests in `test_query_regression_slash.py` to cover command parsing and admission logic. * refactor: enhance query-regression command handling with comment validation and identity checks * feat: implement admission identity handling for query regression workflows * refactor: update PR admission logic in query regression workflow * refactor: update token usage in slash command dispatch and README for clarity * test: add cases for handling re-run failed jobs and stale runner artifacts * refactor: improve repository metadata handling in query regression scripts * chore: enable overwrite for artifact uploads to handle re-run failed jobs * chore: enable overwrite for query regression admission uploads * feat: enhance query-regression admission with HMAC signing and verification - Introduced HMAC signing for admission markers in query-regression workflows to ensure integrity and authenticity. - Updated `query-regression-comment.test.cjs` to include tests for signing and verifying admission markers. - Modified `query-regression-slash.py` to handle admission marker signing and verification, including checks for dispatch sender and head SHA consistency. - Enhanced workflows to securely manage admission markers and HMAC secrets, ensuring they are not exposed to untrusted contexts. - Improved documentation to clarify the admission process and the role of HMAC in securing the workflow. * test: add case to find newly posted marker among newer comments * test: add case to verify multiline output handling in write_outputs function --- .../query-regression/README.md | 58 +- .github/scripts/query-regression-comment.cjs | 284 ++++-- .../scripts/query-regression-comment.test.cjs | 808 +++++++++++++++++- .github/scripts/query-regression-slash.py | 696 +++++++++++++++ .github/workflows/checks.yml | 1 + .../workflows/query-regression-comment.yml | 44 +- .github/workflows/query-regression-slash.yml | 121 +++ .github/workflows/query-regression.yml | 122 +-- .github/workflows/slash-command-dispatch.yml | 36 + tests/perf/AGENTS.md | 15 +- tests/perf/README.md | 48 +- tests/perf/test_query_regression_slash.py | 475 ++++++++++ 12 files changed, 2515 insertions(+), 193 deletions(-) create mode 100644 .github/scripts/query-regression-slash.py create mode 100644 .github/workflows/query-regression-slash.yml create mode 100644 .github/workflows/slash-command-dispatch.yml create mode 100644 tests/perf/test_query_regression_slash.py diff --git a/.github/runner-scale-sets/query-regression/README.md b/.github/runner-scale-sets/query-regression/README.md index c0e34a4c40..3e0159af60 100644 --- a/.github/runner-scale-sets/query-regression/README.md +++ b/.github/runner-scale-sets/query-regression/README.md @@ -15,9 +15,9 @@ data disk. Dispatching with any other `runner` value uses it as a literal self-hosted runner label, which is how a manually prepared host (see -`ecs-image/bootstrap-runner-host.sh`) runs the workflow. For PR labels, set -the repository variable `QUERY_REGRESSION_PR_RUNNER` to such a label to -redirect PR runs away from ECS. +`ecs-image/bootstrap-runner-host.sh`) runs the workflow. For PR comment +admission, set the repository variable `QUERY_REGRESSION_PR_RUNNER` to such +a label to redirect those runs away from ECS. The office ARC scale set `perf-regression-8-cores` that previously ran this workflow is retired; see git history for its values files and pause/deploy @@ -44,10 +44,12 @@ Configuration lives in repository variables/secrets: | Kind | Name | Purpose | | --- | --- | --- | | secret | `ALICLOUD_ECS_ACCESS_KEY_ID` / `ALICLOUD_ECS_ACCESS_KEY_SECRET` | RAM user scoped to ECS RunInstances/DeleteInstances/Describe*/CreateImage/RunCommand. Used only by provision/teardown jobs on `ubuntu-latest`; never reaches the ECS instance. | -| secret | `GH_PERSONAL_ACCESS_TOKEN` | Creates the short-lived runner registration token (shared with the jsonbench EC2 path). | +| secret | `GH_PERSONAL_ACCESS_TOKEN` | Creates the short-lived runner registration token (shared with the jsonbench EC2 path). Slash-command-dispatch uses `github.token` with `contents: write` for same-repo `repository_dispatch`. | +| secret | `QUERY_REGRESSION_ADMISSION_HMAC` | HMAC-SHA256 key for the hidden admission marker comment. Injected only into the ubuntu-latest parse job and the sticky-comment workflow. Never reference it in `query-regression.yml` (the ECS job would inherit it). Empty fails closed: admit refuses, comments are skipped. | | vars | `ALIYUN_ECS_REGION_ID` / `ALIYUN_ECS_VSWITCH_ID` / `ALIYUN_ECS_SECURITY_GROUP_ID` | Network placement. The security group should allow egress only; no inbound rules are needed. The vSwitch pins the zone. | | vars | `ALIYUN_ECS_INSTANCE_TYPE` | Dedicated (non-burstable, non-shared) instance family. Prefer 32 GiB (e.g. `ecs.g8i.2xlarge`); `ecs.c9i.2xlarge` is 8c16g and nightly thin-LTO of greptime peaks above that. Both base and candidate clusters run on the same machine, so noisy neighbors break thresholds. | | vars | `QUERY_REGRESSION_ECS_IMAGE_ID` | Custom image built by `ecs-image/build-ecs-image.py`. | +| vars | `QUERY_REGRESSION_COMMENT_ALLOWLIST` | Comma/whitespace-separated GitHub logins allowed to comment `/query-regression` on a PR. Each login must also have repository `admin` permission. Empty denies all comment commands. | The system disk is 50 GiB, which covers the image, a 16 GiB swapfile, the checkout, and cold build caches (target dir, cargo registry, sccache). ENOSPC @@ -104,23 +106,37 @@ agent, which Aliyun public Ubuntu images include. ## Trust admission for PR runs -A maintainer applying the `query-regression` or `heavy-regression` label is -**trust admission for that exact PR revision**. `query-regression` runs the -six routine default cases; `heavy-regression` runs only the high-cardinality -`prom_remote_write_7913` remote-write case. `pull_request: labeled` is the only PR -trigger: the label event snapshots its merge, head, and base SHAs. A queued -job fetches that immutable event merge SHA directly, verifies it is a -two-parent merge whose parents include the snapshotted head exactly once, and -uses its other parent as the actual base build revision. The snapshotted -event base is retained for audit only, so a difference from the merge's -non-head parent is not a failure. The job never follows a newer mutable PR -merge ref. An unavailable event merge, or one that does not contain exactly -one snapshotted head parent, fails closed. A later PR head change does not -retarget an already queued run: it may execute only its previously trusted -event revision if that revision remains fetchable. To run the new revision, -the maintainer must review it, remove the label, and re-add the desired -regression label; cancel the old run if it is no longer wanted. An existing -label does not automatically rerun the benchmark. +An allowlisted repository admin commenting `/query-regression` on the PR is +**trust admission for that exact PR revision**. `/query-regression` runs the +six routine default cases; `/query-regression heavy` runs only the +high-cardinality `prom_remote_write_7913` remote-write case. +`slash-command-dispatch.yml` (`issue_comment` on the default branch) parses +the command and `repository_dispatch`es; `query-regression-slash.yml` admits +the revision so secrets work for fork PRs. Dispatch must come from +`github-actions[bot]`. The handler requires the current PR head to equal the +head SHA snapshotted in that dispatch (comment time, not handler start). If +the head moved while queued, admission denies; comment `/query-regression` +again after reviewing the new revision. It then snapshots merge, head, and +base SHAs at admission. A queued job fetches that immutable event merge SHA +directly, verifies it is a two-parent merge whose parents include the +snapshotted head exactly once, and uses its other parent as the actual base +build revision. The ubuntu-latest admission job posts a hidden HMAC-signed +marker comment on the admitted PR and uploads `query-regression-admission` as +a lookup hint. The sticky-comment workflow verifies that marker (and that the +runner artifact matches it) before posting. Candidate code on ECS shares the +run and can overwrite artifacts, but it cannot forge the marker: the reusable +workflow's `GITHUB_TOKEN` is `contents: read` only, and +`QUERY_REGRESSION_ADMISSION_HMAC` is never referenced there so it never +reaches ECS. The snapshotted event base is retained for audit only, so a +difference from the merge's non-head +parent is not a failure. The job never +follows a newer mutable PR merge ref. An unavailable event merge, or one that +does not contain exactly one snapshotted head parent, fails closed. An +already-admitted run is not retargeted by a later push; cancel it if it is no +longer wanted. + +The commenter must be in `QUERY_REGRESSION_COMMENT_ALLOWLIST` and have +repository `admin` permission. Admission does not relax runner hardening or GitHub permissions. Keep the ECS instance free of cloud credentials and long-lived tokens, keep the security diff --git a/.github/scripts/query-regression-comment.cjs b/.github/scripts/query-regression-comment.cjs index 2d6c9c40dc..08bd71e99f 100644 --- a/.github/scripts/query-regression-comment.cjs +++ b/.github/scripts/query-regression-comment.cjs @@ -14,6 +14,19 @@ const fs = require('fs'); const path = require('path'); +const crypto = require('crypto'); + +const MARKER_PREFIX = ''; +const ADMISSION_MAC_FIELDS = [ + 'run_id', + 'pr_number', + 'head_sha', + 'head_repo', + 'base_repo', + 'candidate_sha', + 'base_sha', +]; function skip(core, message) { core.info(message); @@ -300,95 +313,212 @@ function renderSummaryTable(rows) { return details === '' ? table : `${table}\n\n${details}`; } +function admissionMacMessage(identity) { + return ADMISSION_MAC_FIELDS.map(field => { + const value = String(identity[field] ?? ''); + return field.endsWith('_sha') ? value.toLowerCase() : value; + }).join('|'); +} + +function admissionMac(secret, identity) { + return crypto.createHmac('sha256', secret).update(admissionMacMessage(identity)).digest('hex'); +} + +function verifyAdmissionMac(secret, identity, mac) { + if (!secret || !mac) { + return false; + } + const expected = admissionMac(secret, identity); + const actual = String(mac); + if (expected.length !== actual.length) { + return false; + } + try { + return crypto.timingSafeEqual(Buffer.from(expected, 'utf8'), Buffer.from(actual, 'utf8')); + } catch { + return false; + } +} + +function formatAdmissionMarker(identity) { + return `${MARKER_PREFIX}\n${JSON.stringify(identity, Object.keys(identity).sort())}\n${MARKER_SUFFIX}\n`; +} + +function parseAdmissionMarker(body) { + const start = String(body || '').indexOf(MARKER_PREFIX); + if (start < 0) { + return null; + } + const rest = String(body).slice(start + MARKER_PREFIX.length); + const end = rest.indexOf(MARKER_SUFFIX); + if (end < 0) { + return null; + } + try { + const payload = JSON.parse(rest.slice(0, end).trim()); + return payload && typeof payload === 'object' && !Array.isArray(payload) ? payload : null; + } catch { + return null; + } +} + +function isAdmissionIdentity(payload) { + return ADMISSION_MAC_FIELDS.every(field => { + const value = payload?.[field]; + return value !== undefined && value !== null && String(value) !== ''; + }); +} + +async function loadAdmissionMarker(github, { owner, repo, prNumber, runId, secret }) { + // Newest first: the marker is posted at admit time, so it is among the + // latest comments. Oldest-first with a page cap misses it on busy PRs. + for (let page = 1; ; page += 1) { + const { data } = await github.rest.issues.listComments({ + owner, + repo, + issue_number: prNumber, + per_page: 100, + page, + sort: 'created', + direction: 'desc', + }); + if (!Array.isArray(data) || data.length === 0) { + return null; + } + for (const comment of data) { + const payload = parseAdmissionMarker(comment.body); + if (!payload || !isAdmissionIdentity(payload)) { + continue; + } + if (Number(payload.run_id) !== Number(runId)) { + continue; + } + if (!verifyAdmissionMac(secret, payload, payload.mac)) { + continue; + } + return { id: Number(comment.id), payload }; + } + if (data.length < 100) { + return null; + } + } +} + +function identitiesMatch(admission, metadata) { + return ADMISSION_MAC_FIELDS.every(field => { + const left = String(admission[field] ?? ''); + const right = String(metadata[field] ?? ''); + if (field === 'pr_number' || field === 'run_id') { + return Number(left) === Number(right); + } + if (field.endsWith('_sha')) { + return left.toLowerCase() === right.toLowerCase(); + } + return left === right; + }); +} + module.exports = async function validateQueryRegressionComment({ github, context, core }) { const artifactDir = 'query-regression-comment'; + const admissionPath = path.join('query-regression-admission', 'query-regression-admission.json'); const metadataPath = path.join(artifactDir, 'query-regression-pr.json'); const summaryPath = path.join(artifactDir, 'query-regression-summary.md'); + if (!fs.existsSync(admissionPath)) { + return skip(core, 'Missing trusted admission identity; skipping sticky comment.'); + } if (!fs.existsSync(metadataPath)) { return skip(core, 'Missing query-regression-pr.json; skipping sticky comment.'); } + let admission; let metadata; try { + admission = JSON.parse(fs.readFileSync(admissionPath, 'utf8')); metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf8')); } catch (error) { - core.warning(`Invalid PR metadata JSON: ${error.message}`); - return skip(core, 'Invalid PR metadata JSON; skipping sticky comment.'); + core.warning(`Invalid admission or PR metadata JSON: ${error.message}`); + return skip(core, 'Invalid admission or PR metadata JSON; skipping.'); + } + + // Artifact identity is a lookup hint only. Candidate code on ECS shares this + // run and can overwrite artifacts; the signed PR comment is the source of truth. + if (!identitiesMatch(admission, metadata)) { + return skip(core, 'Runner artifact identity does not match admission artifact; skipping.'); } const expectedRunId = Number(process.env.WORKFLOW_RUN_ID); const expectedRunAttempt = Number(process.env.WORKFLOW_RUN_ATTEMPT); - if (metadata.run_id !== expectedRunId || metadata.run_attempt !== expectedRunAttempt) { - return skip(core, 'Artifact metadata does not match this workflow_run; skipping.'); + // parse is not rerun on "Re-run failed jobs", so admission keeps attempt 1. + // Bind it to the stable run id; the runner artifact must match this attempt. + if (Number(admission.run_id) !== expectedRunId) { + return skip(core, 'Trusted admission does not match this workflow_run; skipping.'); } - - if (metadata.base_repo !== `${context.repo.owner}/${context.repo.repo}`) { - return skip(core, `PR targets ${metadata.base_repo}, not this repository; skipping.`); - } - - const prNumber = Number(metadata.pr_number); - if (!Number.isInteger(prNumber) || prNumber <= 0) { - return skip(core, 'Invalid PR number in metadata; skipping.'); + if ( + Number(metadata.run_id) !== expectedRunId || + Number(metadata.run_attempt) !== expectedRunAttempt + ) { + return skip(core, 'Runner artifact does not match this workflow_run attempt; skipping.'); } const run = context.payload.workflow_run; - if (run.event !== 'pull_request') { - return skip(core, `Workflow run event is ${run.event}, not pull_request; skipping.`); - } - if (run.head_sha !== metadata.head_sha) { - return skip(core, 'Workflow run head SHA differs from artifact metadata; skipping.'); - } - const runHeadRepo = run.head_repository?.full_name; - if (!runHeadRepo) { - return skip(core, 'Workflow run head repository is missing; skipping.'); - } - if (runHeadRepo !== metadata.head_repo) { - return skip(core, 'Workflow run head repository differs from artifact metadata; skipping.'); + // Command-handler runs execute from repository_dispatch on the default + // branch, so workflow_run head SHA/repo are that commit, not the PR. + if (run.event !== 'repository_dispatch') { + return skip(core, `Workflow run event is ${run.event}, not repository_dispatch; skipping.`); } - // GitHub leaves workflow_run.pull_requests empty for fork PRs. When present, - // use it as an extra guard; otherwise resolve the unique open PR from trusted - // workflow_run head repo/branch/SHA metadata before accepting the artifact PR. - const workflowPrNumbers = new Set( - (run.pull_requests || []).map(pr => Number(pr.number)).filter(Number.isInteger) - ); - if (workflowPrNumbers.size > 0) { - if (!workflowPrNumbers.has(prNumber)) { - return skip(core, `PR #${prNumber} is not listed in workflow_run ${run.id}; skipping.`); - } - } else { - const runHeadOwner = run.head_repository?.owner?.login; - const runHeadBranch = run.head_branch; - if (!runHeadOwner || !runHeadBranch) { - return skip(core, 'Workflow run head owner or branch is missing; skipping.'); - } + const hmacSecret = (process.env.QUERY_REGRESSION_ADMISSION_HMAC || '').trim(); + if (!hmacSecret) { + return skip(core, 'QUERY_REGRESSION_ADMISSION_HMAC is unset; skipping sticky comment.'); + } - let matchingPrs; - try { - const { data: pullRequests } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - head: `${runHeadOwner}:${runHeadBranch}`, - per_page: 100, - }); - matchingPrs = pullRequests.filter(pr => ( - pr.head.repo?.full_name === runHeadRepo && - pr.head.sha === run.head_sha && - pr.base.repo?.full_name === metadata.base_repo - )); - } catch (error) { - core.warning(`Could not resolve PR from workflow_run metadata: ${error.message}`); - return skip(core, 'Could not resolve PR from workflow_run metadata; skipping.'); - } + const hintedPrNumber = Number(admission.pr_number); + if (!Number.isInteger(hintedPrNumber) || hintedPrNumber <= 0) { + return skip(core, 'Invalid PR number in admission artifact; skipping.'); + } - if (matchingPrs.length !== 1) { - return skip(core, `Workflow run matched ${matchingPrs.length} open PRs; skipping.`); - } - if (Number(matchingPrs[0].number) !== prNumber) { - return skip(core, `Artifact PR #${prNumber} does not match workflow_run PR #${matchingPrs[0].number}; skipping.`); - } + let marker; + try { + marker = await loadAdmissionMarker(github, { + owner: context.repo.owner, + repo: context.repo.repo, + prNumber: hintedPrNumber, + runId: expectedRunId, + secret: hmacSecret, + }); + } catch (error) { + core.warning(`Could not list admission markers on PR #${hintedPrNumber}: ${error.message}`); + return skip(core, `Could not list admission markers on PR #${hintedPrNumber}; skipping.`); + } + if (!marker) { + return skip( + core, + 'No signed admission marker for this run on the hinted PR; skipping.', + ); + } + + admission = { + pr_number: marker.payload.pr_number, + head_sha: marker.payload.head_sha, + head_repo: marker.payload.head_repo, + base_repo: marker.payload.base_repo, + candidate_sha: marker.payload.candidate_sha, + base_sha: marker.payload.base_sha, + run_id: marker.payload.run_id, + }; + + if (!identitiesMatch(admission, metadata)) { + return skip(core, 'Runner artifact identity does not match signed admission marker; skipping.'); + } + + if (admission.base_repo !== `${context.repo.owner}/${context.repo.repo}`) { + return skip(core, `PR targets ${admission.base_repo}, not this repository; skipping.`); + } + + const prNumber = Number(admission.pr_number); + if (!Number.isInteger(prNumber) || prNumber <= 0) { + return skip(core, 'Invalid PR number in signed admission marker; skipping.'); } let pull; @@ -406,11 +536,14 @@ module.exports = async function validateQueryRegressionComment({ github, context if (pull.state !== 'open') { return skip(core, `PR #${prNumber} is ${pull.state}; skipping.`); } - if (pull.base.repo.full_name !== metadata.base_repo || pull.head.repo.full_name !== metadata.head_repo) { - return skip(core, 'Current PR repository metadata does not match artifact; skipping.'); + if ( + pull.base?.repo?.full_name !== admission.base_repo || + pull.head?.repo?.full_name !== admission.head_repo + ) { + return skip(core, 'Current PR repository metadata does not match trusted admission; skipping.'); } - if (pull.head.sha !== metadata.head_sha) { - return skip(core, 'Current PR head SHA differs from artifact; skipping stale run.'); + if (pull.head?.sha !== admission.head_sha) { + return skip(core, 'Current PR head SHA differs from trusted admission; skipping stale run.'); } const reportPaths = findReports(artifactDir); @@ -422,9 +555,9 @@ module.exports = async function validateQueryRegressionComment({ github, context '', `- **Workflow run:** ${serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${expectedRunId}`, `- **Built base SHA:** \`${text(metadata.built_base_sha)}\``, - `- **Event base SHA:** \`${text(metadata.event_base_sha)}\``, - `- **Head SHA:** \`${text(metadata.head_sha)}\``, - `- **Candidate merge SHA:** \`${text(metadata.candidate_sha)}\``, + `- **Event base SHA:** \`${text(admission.base_sha)}\``, + `- **Head SHA:** \`${text(admission.head_sha)}\``, + `- **Candidate merge SHA:** \`${text(admission.candidate_sha)}\``, '', ].join('\n'); @@ -451,4 +584,11 @@ module.exports = async function validateQueryRegressionComment({ github, context core.setOutput('summary_path', summaryPath); }; -module.exports._test = { collectReportRows, renderSummaryTable }; +module.exports._test = { + collectReportRows, + renderSummaryTable, + admissionMac, + verifyAdmissionMac, + formatAdmissionMarker, + parseAdmissionMarker, +}; diff --git a/.github/scripts/query-regression-comment.test.cjs b/.github/scripts/query-regression-comment.test.cjs index 4b3d1a5cbf..d37c56ac68 100644 --- a/.github/scripts/query-regression-comment.test.cjs +++ b/.github/scripts/query-regression-comment.test.cjs @@ -5,7 +5,16 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const handler = require('./query-regression-comment.cjs'); -const { collectReportRows, renderSummaryTable } = handler._test; +const { + collectReportRows, + renderSummaryTable, + admissionMac, + verifyAdmissionMac, + formatAdmissionMarker, + parseAdmissionMarker, +} = handler._test; + +const HMAC_SECRET = 'test-admission-hmac'; function report(name, measurements, thresholds = []) { return { @@ -19,10 +28,41 @@ function report(name, measurements, thresholds = []) { }; } +function markerComment(identity, { id = 1, secret = HMAC_SECRET } = {}) { + const payload = { ...identity, mac: admissionMac(secret, identity) }; + return { id, body: formatAdmissionMarker(payload) }; +} + +function githubApi({ identity, pull, comments }) { + return { + rest: { + issues: { + listComments: async () => ({ data: comments ?? [markerComment(identity)] }), + }, + pulls: { + get: async () => ({ + data: pull ?? { + state: 'open', + base: { repo: { full_name: identity.base_repo } }, + head: { repo: { full_name: identity.head_repo }, sha: identity.head_sha }, + }, + }), + }, + }, + }; +} + test('keeps the default export callable and exposes only the test seam', () => { assert.equal(typeof handler, 'function'); assert.equal(handler.constructor.name, 'AsyncFunction'); - assert.deepEqual(Object.keys(handler._test).sort(), ['collectReportRows', 'renderSummaryTable']); + assert.deepEqual(Object.keys(handler._test).sort(), [ + 'admissionMac', + 'collectReportRows', + 'formatAdmissionMarker', + 'parseAdmissionMarker', + 'renderSummaryTable', + 'verifyAdmissionMac', + ]); }); test('renders every case in one summary table without per-case separators', () => { @@ -305,18 +345,87 @@ test('escapes Markdown table content, including bare carriage returns', () => { assert.doesNotMatch(table, /hidden|comment|drop|\r/); }); -test('writes the explicit no-report summary without an empty table', async () => { +test('posts a comment-command report without treating workflow_run as the PR head', async () => { const originalCwd = process.cwd(); const originalRunId = process.env.WORKFLOW_RUN_ID; const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); const artifactDir = path.join(temporaryDir, 'query-regression-comment'); const outputs = new Map(); try { fs.mkdirSync(artifactDir); - fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify({ - run_id: 101, + const metadata = { + run_id: 202, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + built_base_sha: 'base-sha', + event_base_sha: 'event-base-sha', + candidate_sha: 'merge-sha', + base_sha: 'event-base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '202'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info() {}, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: githubApi({ identity: metadata }), + }); + + assert.equal(outputs.get('should_post'), 'true'); + assert.equal(outputs.get('pr_number'), '42'); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('skips reports produced by a pull_request workflow_run', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 303, run_attempt: 1, base_repo: 'owner/repo', pr_number: 42, @@ -325,14 +434,21 @@ test('writes the explicit no-report summary without an empty table', async () => built_base_sha: 'base-sha', event_base_sha: 'event-base-sha', candidate_sha: 'candidate-sha', - })); + base_sha: 'event-base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); process.chdir(temporaryDir); - process.env.WORKFLOW_RUN_ID = '101'; + process.env.WORKFLOW_RUN_ID = '303'; process.env.WORKFLOW_RUN_ATTEMPT = '1'; await handler({ core: { - info() {}, + info(message) { infos.push(message); }, warning() {}, setOutput(name, value) { outputs.set(name, value); }, }, @@ -347,19 +463,73 @@ test('writes the explicit no-report summary without an empty table', async () => }, }, }, - github: { - rest: { - pulls: { - get: async () => ({ - data: { - state: 'open', - base: { repo: { full_name: 'owner/repo' } }, - head: { repo: { full_name: 'fork/repo' }, sha: 'head-sha' }, - }, - }), + github: { rest: { pulls: { get: async () => { throw new Error('should not fetch PR'); } } } }, + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /not repository_dispatch/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('writes the explicit no-report summary without an empty table', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 101, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'head-sha', + head_repo: 'fork/repo', + built_base_sha: 'base-sha', + event_base_sha: 'event-base-sha', + candidate_sha: 'candidate-sha', + base_sha: 'event-base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '101'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info() {}, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], }, }, }, + github: githubApi({ identity: metadata }), }); const summary = fs.readFileSync(path.join(artifactDir, 'query-regression-summary.md'), 'utf8'); @@ -372,6 +542,608 @@ test('writes the explicit no-report summary without an empty table', async () => else process.env.WORKFLOW_RUN_ID = originalRunId; if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('skips when the runner artifact forges a different PR number', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const admission = { + run_id: 404, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(admission), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify({ + ...admission, + pr_number: 99, + })); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '404'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + + await handler({ + core: { + info(message) { infos.push(message); }, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: { rest: { pulls: { get: async () => { throw new Error('should not fetch PR'); } } } }, + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /does not match admission artifact/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('posts after Re-run failed jobs when admission stays on attempt 1', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + + try { + fs.mkdirSync(artifactDir); + const admission = { + run_id: 505, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + const metadata = { + ...admission, + run_attempt: 2, + built_base_sha: 'base-sha', + event_base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(admission), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '505'; + process.env.WORKFLOW_RUN_ATTEMPT = '2'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info() {}, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: githubApi({ identity: admission }), + }); + + assert.equal(outputs.get('should_post'), 'true'); + assert.equal(outputs.get('pr_number'), '42'); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('skips a stale runner artifact from a previous attempt', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const admission = { + run_id: 606, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(admission), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify({ + ...admission, + built_base_sha: 'base-sha', + event_base_sha: 'base-sha', + })); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '606'; + process.env.WORKFLOW_RUN_ATTEMPT = '2'; + + await handler({ + core: { + info(message) { infos.push(message); }, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: { rest: { pulls: { get: async () => { throw new Error('should not fetch PR'); } } } }, + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /does not match this workflow_run attempt/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('skips when the current PR head repository is missing', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 707, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + built_base_sha: 'base-sha', + event_base_sha: 'base-sha', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '707'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info(message) { infos.push(message); }, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: githubApi({ + identity: metadata, + pull: { + state: 'open', + base: { repo: { full_name: 'owner/repo' } }, + head: { repo: null, sha: 'pr-head-sha' }, + }, + }), + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /does not match trusted admission/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('signs and verifies admission markers', () => { + const identity = { + run_id: 808, + pr_number: 42, + head_sha: 'HEADSHA', + head_repo: 'fork/repo', + base_repo: 'owner/repo', + candidate_sha: 'MERGESHA', + base_sha: 'BASESHA', + }; + const mac = admissionMac(HMAC_SECRET, identity); + assert.equal(verifyAdmissionMac(HMAC_SECRET, identity, mac), true); + assert.equal(verifyAdmissionMac('other', identity, mac), false); + assert.equal(verifyAdmissionMac(HMAC_SECRET, { ...identity, pr_number: 99 }, mac), false); + const parsed = parseAdmissionMarker(`noise\n${formatAdmissionMarker({ ...identity, mac })}\n`); + assert.equal(parsed.pr_number, 42); + assert.equal(verifyAdmissionMac(HMAC_SECRET, parsed, parsed.mac), true); +}); + +test('skips when the HMAC secret is unset', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 808, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '808'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + + await handler({ + core: { + info(message) { infos.push(message); }, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: githubApi({ identity: metadata }), + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /QUERY_REGRESSION_ADMISSION_HMAC is unset/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('skips when the hinted PR has no signed marker for this run', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 909, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 99, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '909'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info(message) { infos.push(message); }, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: githubApi({ identity: metadata, comments: [{ id: 1, body: 'unrelated' }] }), + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /No signed admission marker/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('skips a marker whose HMAC does not match', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const infos = []; + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 910, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '910'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info(message) { infos.push(message); }, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: githubApi({ + identity: metadata, + comments: [markerComment(metadata, { secret: 'forged-secret' })], + }), + }); + + assert.equal(outputs.get('should_post'), 'false'); + assert.match(infos.join('\n'), /No signed admission marker/); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; + fs.rmSync(temporaryDir, { recursive: true, force: true }); + } +}); + +test('finds a newly posted marker after a full page of newer comments', async () => { + const originalCwd = process.cwd(); + const originalRunId = process.env.WORKFLOW_RUN_ID; + const originalRunAttempt = process.env.WORKFLOW_RUN_ATTEMPT; + const originalHmac = process.env.QUERY_REGRESSION_ADMISSION_HMAC; + const temporaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'query-regression-comment-')); + const artifactDir = path.join(temporaryDir, 'query-regression-comment'); + const outputs = new Map(); + const pages = []; + + try { + fs.mkdirSync(artifactDir); + const metadata = { + run_id: 911, + run_attempt: 1, + base_repo: 'owner/repo', + pr_number: 42, + head_sha: 'pr-head-sha', + head_repo: 'fork/repo', + candidate_sha: 'merge-sha', + base_sha: 'base-sha', + }; + fs.mkdirSync(path.join(temporaryDir, 'query-regression-admission')); + fs.writeFileSync( + path.join(temporaryDir, 'query-regression-admission', 'query-regression-admission.json'), + JSON.stringify(metadata), + ); + fs.writeFileSync(path.join(artifactDir, 'query-regression-pr.json'), JSON.stringify(metadata)); + process.chdir(temporaryDir); + process.env.WORKFLOW_RUN_ID = '911'; + process.env.WORKFLOW_RUN_ATTEMPT = '1'; + process.env.QUERY_REGRESSION_ADMISSION_HMAC = HMAC_SECRET; + + await handler({ + core: { + info() {}, + warning() {}, + setOutput(name, value) { outputs.set(name, value); }, + }, + context: { + repo: { owner: 'owner', repo: 'repo' }, + payload: { + workflow_run: { + event: 'repository_dispatch', + head_sha: 'default-branch-sha', + head_repository: { full_name: 'owner/repo' }, + pull_requests: [], + }, + }, + }, + github: { + rest: { + issues: { + listComments: async ({ page, direction }) => { + pages.push({ page, direction }); + if (page === 1) { + return { + data: Array.from({ length: 100 }, (_, index) => ({ + id: 10_000 - index, + body: 'unrelated', + })), + }; + } + return { data: [markerComment(metadata, { id: 50 })] }; + }, + }, + pulls: { + get: async () => ({ + data: { + state: 'open', + base: { repo: { full_name: 'owner/repo' } }, + head: { repo: { full_name: 'fork/repo' }, sha: 'pr-head-sha' }, + }, + }), + }, + }, + }, + }); + + assert.equal(outputs.get('should_post'), 'true'); + assert.deepEqual(pages, [ + { page: 1, direction: 'desc' }, + { page: 2, direction: 'desc' }, + ]); + } finally { + process.chdir(originalCwd); + if (originalRunId === undefined) delete process.env.WORKFLOW_RUN_ID; + else process.env.WORKFLOW_RUN_ID = originalRunId; + if (originalRunAttempt === undefined) delete process.env.WORKFLOW_RUN_ATTEMPT; + else process.env.WORKFLOW_RUN_ATTEMPT = originalRunAttempt; + if (originalHmac === undefined) delete process.env.QUERY_REGRESSION_ADMISSION_HMAC; + else process.env.QUERY_REGRESSION_ADMISSION_HMAC = originalHmac; fs.rmSync(temporaryDir, { recursive: true, force: true }); } }); diff --git a/.github/scripts/query-regression-slash.py b/.github/scripts/query-regression-slash.py new file mode 100644 index 0000000000..bfb9a72060 --- /dev/null +++ b/.github/scripts/query-regression-slash.py @@ -0,0 +1,696 @@ +#!/usr/bin/env python3 +# Copyright 2023 Greptime Team +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Admit a dispatched `/query-regression` command as a query-regression run. + +slash-command-dispatch owns comment parsing and admin permission. This script +requires the dispatch sender to be github-actions[bot], re-fetches the +triggering comment by id (so a forged repository_dispatch payload cannot +spoof the actor or PR), requires the current PR head to match the +dispatcher-snapshotted head SHA, then validates case args, the +QUERY_REGRESSION_COMMENT_ALLOWLIST subset, and the PR's current merge commit. +On admit it posts a hidden HMAC-signed marker comment +(QUERY_REGRESSION_ADMISSION_HMAC) that the sticky-comment workflow verifies; +the ECS job cannot forge that marker. +""" + +from __future__ import annotations + +import argparse +import hashlib +import hmac +import json +import os +import re +import sys +import time +import uuid +import urllib.error +import urllib.parse +import urllib.request +from dataclasses import dataclass +from typing import Any + + +COMMAND = "/query-regression" +ALLOWED_PERMISSIONS = frozenset({"admin"}) +ALLOWED_DISPATCH_SENDERS = frozenset({"github-actions[bot]"}) +FULL_SHA = re.compile(r"^[0-9a-fA-F]{40}$") +CASE_TOKEN = re.compile(r"^(?:all|heavy|tests/perf/query_cases/[A-Za-z0-9_][A-Za-z0-9_./-]*)$") +COMMAND_LINE = re.compile(rf"^{re.escape(COMMAND)}(?:\s+(.*))?$") +ISSUE_URL = re.compile(r"/repos/([^/]+/[^/]+)/issues/(\d+)$") + + +@dataclass(frozen=True) +class CommandParse: + matched: bool + case: str = "" + error: str = "" + + +@dataclass(frozen=True) +class Decision: + skip: bool + reason: str + case: str = "" + pr_number: str = "" + base_sha: str = "" + candidate_sha: str = "" + head_sha: str = "" + head_repo: str = "" + base_repo: str = "" + reply: str = "" + + +@dataclass(frozen=True) +class CommentIdentity: + actor: str = "" + pr_number: int = 0 + command: CommandParse = CommandParse(matched=False) + error: str = "" + + +def parse_case_args(raw_args: str) -> CommandParse: + tokens = [part for part in re.split(r"[\s,]+", (raw_args or "").strip()) if part] + if not tokens: + return CommandParse(matched=True, case="all") + if "all" in tokens and len(tokens) > 1: + return CommandParse(matched=True, error="'all' cannot be mixed with other case selectors") + for token in tokens: + if ".." in token or not CASE_TOKEN.fullmatch(token): + return CommandParse( + matched=True, + error=( + "case selector must be 'all', 'heavy', or tests/perf/query_cases/... " + f"paths without '..'; got {token!r}" + ), + ) + return CommandParse(matched=True, case=",".join(tokens)) + + +def parse_command(body: str) -> CommandParse: + first = (body or "").replace("\r\n", "\n").replace("\r", "\n").split("\n", 1)[0].strip() + match = COMMAND_LINE.fullmatch(first) + if match is None: + return CommandParse(matched=False) + return parse_case_args(match.group(1) or "") + + +def parse_allowlist(raw: str) -> frozenset[str]: + names: set[str] = set() + for part in re.split(r"[\s,]+", raw or ""): + login = part.strip().lstrip("@") + if login: + names.add(login.lower()) + return frozenset(names) + + +def is_full_sha(value: str) -> bool: + return bool(FULL_SHA.fullmatch(value or "")) + + +def nested_str(payload: Any, *keys: str) -> str: + current: Any = payload + for key in keys: + if not isinstance(current, dict): + return "" + current = current.get(key) + if current is None: + return "" + return str(current) + + +MARKER_PREFIX = "" + + +def admission_mac_message(identity: dict[str, Any]) -> str: + return "|".join( + [ + str(identity["run_id"]), + str(identity["pr_number"]), + str(identity["head_sha"]).lower(), + str(identity["head_repo"]), + str(identity["base_repo"]), + str(identity["candidate_sha"]).lower(), + str(identity["base_sha"]).lower(), + ] + ) + + +def sign_admission(secret: str, identity: dict[str, Any]) -> str: + return hmac.new( + secret.encode("utf-8"), + admission_mac_message(identity).encode("utf-8"), + hashlib.sha256, + ).hexdigest() + + +def verify_admission_mac(secret: str, identity: dict[str, Any], mac: str) -> bool: + if not secret or not mac: + return False + expected = sign_admission(secret, identity) + try: + return hmac.compare_digest(expected, mac) + except (TypeError, ValueError): + return False + + +def format_admission_marker(identity: dict[str, Any]) -> str: + body = json.dumps(identity, sort_keys=True) + return f"{MARKER_PREFIX}\n{body}\n{MARKER_SUFFIX}\n" + + +def parse_admission_marker(body: str) -> dict[str, Any] | None: + start = (body or "").find(MARKER_PREFIX) + if start < 0: + return None + rest = body[start + len(MARKER_PREFIX) :] + end = rest.find(MARKER_SUFFIX) + if end < 0: + return None + try: + payload = json.loads(rest[:end].strip()) + except json.JSONDecodeError: + return None + return payload if isinstance(payload, dict) else None + + +def parse_github_id(value: str) -> int | None: + stripped = (value or "").strip() + if not stripped.isdigit(): + return None + number = int(stripped) + return number if number > 0 else None + + +def github_request( + token: str, + api_url: str, + path: str, + *, + method: str = "GET", + payload: dict[str, Any] | None = None, +) -> dict[str, Any]: + data = None if payload is None else json.dumps(payload).encode("utf-8") + headers = { + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": "2022-11-28", + } + if data is not None: + headers["Content-Type"] = "application/json" + request = urllib.request.Request( + f"{api_url.rstrip('/')}{path}", + method=method, + data=data, + headers=headers, + ) + try: + with urllib.request.urlopen(request, timeout=30) as response: + raw = response.read().decode("utf-8") + return json.loads(raw) if raw else {} + except urllib.error.HTTPError as error: + body = error.read().decode("utf-8", "replace") + raise SystemExit( + f"GitHub API {method} {path} failed: HTTP {error.code}: {body}" + ) from error + + +def github_get(token: str, api_url: str, path: str) -> dict[str, Any]: + return github_request(token, api_url, path) + + +def fetch_comment(token: str, api_url: str, repo: str, comment_id: int) -> dict[str, Any]: + return github_get(token, api_url, f"/repos/{repo}/issues/comments/{comment_id}") + + +def identity_from_comment(comment: dict[str, Any], expected_repo: str) -> CommentIdentity: + login = str((comment.get("user") or {}).get("login") or "") + issue_url = str(comment.get("issue_url") or "") + match = ISSUE_URL.search(issue_url) + if match is None: + return CommentIdentity(error="comment issue_url is missing or invalid") + repo, pr_number = match.group(1), int(match.group(2)) + if repo.lower() != expected_repo.lower(): + return CommentIdentity( + actor=login, + pr_number=pr_number, + error=f"comment targets {repo}, not {expected_repo}", + ) + command = parse_command(str(comment.get("body") or "")) + if not command.matched: + return CommentIdentity( + actor=login, + pr_number=pr_number, + command=command, + error="comment is not a query-regression command", + ) + if command.error: + return CommentIdentity( + actor=login, + pr_number=pr_number, + command=command, + error=command.error, + ) + return CommentIdentity(actor=login, pr_number=pr_number, command=command) + + +def payload_matches_comment( + identity: CommentIdentity, + *, + actor: str, + pr_number: str, + command_args: str, +) -> str: + if actor and actor.strip().lstrip("@").lower() != identity.actor.strip().lstrip("@").lower(): + return "payload actor does not match comment author" + if pr_number.strip(): + parsed = parse_github_id(pr_number) + if parsed is None: + return "payload PR number is not a valid integer" + if parsed != identity.pr_number: + return "payload PR number does not match comment" + payload_command = parse_case_args(command_args) + if payload_command.error: + return payload_command.error + if payload_command.case != identity.command.case: + return "payload command args do not match comment" + return "" + + +def dispatch_sender_ok(sender: str) -> str: + """Empty if repository_dispatch came from Actions; otherwise a deny reason.""" + login = (sender or "").strip().lstrip("@").lower() + if login not in ALLOWED_DISPATCH_SENDERS: + return "repository_dispatch sender is not github-actions[bot]" + return "" + + +def dispatch_head_matches(pull: dict[str, Any], snapshot_sha: str) -> str: + """Empty if the current PR head is the dispatcher snapshot; otherwise a deny reason.""" + snapshot = (snapshot_sha or "").strip().lower() + if not is_full_sha(snapshot): + return "dispatch payload is missing an immutable PR head SHA" + current = nested_str(pull, "head", "sha").lower() + if current != snapshot: + return "PR head changed since the slash command was dispatched" + return "" + + +def fetch_pull( + token: str, + api_url: str, + repo: str, + pr_number: int, + *, + attempts: int = 5, +) -> dict[str, Any]: + path = f"/repos/{repo}/pulls/{pr_number}" + payload: dict[str, Any] = {} + for attempt in range(1, attempts + 1): + payload = github_get(token, api_url, path) + if payload.get("mergeable") is not None or payload.get("draft"): + return payload + if attempt < attempts: + time.sleep(2 ** (attempt - 1)) + return payload + + +def fetch_permission(token: str, api_url: str, repo: str, username: str) -> str: + encoded = urllib.parse.quote(username) + try: + payload = github_get( + token, + api_url, + f"/repos/{repo}/collaborators/{encoded}/permission", + ) + except SystemExit as error: + message = str(error) + if "HTTP 404" in message: + return "" + raise + permission = str(payload.get("permission") or "") + return permission if permission in ALLOWED_PERMISSIONS else "denied" + + +def deny(reason: str, *, reply: str = "", pr_number: str = "") -> Decision: + return Decision(skip=True, reason=reason, reply=reply, pr_number=pr_number) + + +def admit_pull( + pull: dict[str, Any], + *, + actor: str, + allowlist: frozenset[str], + permission: str, + command: CommandParse, + expected_repo: str, + pr_number: str = "", +) -> Decision: + def reject(reason: str, *, reply: str = "") -> Decision: + return deny(reason, reply=reply, pr_number=pr_number) + + if not command.matched: + return reject("comment is not a query-regression command") + if command.error: + return reject( + command.error, + reply=f"Query regression command ignored: {command.error}.", + ) + + actor_key = actor.strip().lstrip("@").lower() + if not allowlist: + return reject( + "QUERY_REGRESSION_COMMENT_ALLOWLIST is unset", + reply=( + "Query regression command ignored: repository variable " + "`QUERY_REGRESSION_COMMENT_ALLOWLIST` is empty." + ), + ) + if actor_key not in allowlist: + return reject( + "commenter is not on QUERY_REGRESSION_COMMENT_ALLOWLIST", + reply="Query regression command ignored: you are not on the allowlist.", + ) + if permission not in ALLOWED_PERMISSIONS: + return reject( + "commenter is not a repository admin", + reply="Query regression command ignored: repository admin permission is required.", + ) + + if pull.get("draft"): + return reject( + "PR is a draft", + reply="Query regression command ignored: draft PRs are not admitted.", + ) + if pull.get("state") != "open": + return reject( + f"PR is {pull.get('state')}", + reply="Query regression command ignored: the pull request is not open.", + ) + if pull.get("merged"): + return reject( + "PR is already merged", + reply="Query regression command ignored: the pull request is already merged.", + ) + if pull.get("mergeable") is False: + return reject( + "PR has merge conflicts", + reply=( + "Query regression command ignored: the pull request is not mergeable. " + "Resolve conflicts and comment `/query-regression` again." + ), + ) + if pull.get("mergeable") is None: + return reject( + "PR mergeability is not yet computed", + reply=( + "Query regression command ignored: GitHub has not computed mergeability yet. " + "Retry in a few seconds." + ), + ) + + base_repo = nested_str(pull, "base", "repo", "full_name") + head_repo = nested_str(pull, "head", "repo", "full_name") + base_sha = nested_str(pull, "base", "sha") + head_sha = nested_str(pull, "head", "sha") + merge_sha = nested_str(pull, "merge_commit_sha") + pr_number = str(pull.get("number") or pr_number) + + if not head_repo: + return reject( + "PR head repository is missing", + reply=( + "Query regression command ignored: the pull request head repository " + "is unavailable (the fork may have been deleted)." + ), + ) + if base_repo != expected_repo: + return reject( + f"PR targets {base_repo}, not {expected_repo}", + reply="Query regression command ignored: pull request is not against this repository.", + ) + if not is_full_sha(base_sha) or not is_full_sha(head_sha) or not is_full_sha(merge_sha): + return reject( + "PR is missing an immutable merge, head, or base SHA", + reply=( + "Query regression command ignored: GitHub did not provide a full merge SHA. " + "Retry once the PR is mergeable." + ), + ) + + return Decision( + skip=False, + reason="", + case=command.case, + pr_number=pr_number, + base_sha=base_sha.lower(), + candidate_sha=merge_sha.lower(), + head_sha=head_sha.lower(), + head_repo=head_repo, + base_repo=base_repo, + ) + + +def write_outputs(decision: Decision) -> None: + values = { + "skip": "true" if decision.skip else "false", + "reason": decision.reason, + "case": decision.case, + "pr_number": decision.pr_number, + "base_sha": decision.base_sha, + "candidate_sha": decision.candidate_sha, + "head_sha": decision.head_sha, + "head_repo": decision.head_repo, + "base_repo": decision.base_repo, + "reply": decision.reply, + } + output_path = os.environ.get("GITHUB_OUTPUT") + if output_path: + with open(output_path, "a", encoding="utf-8") as handle: + for key, value in values.items(): + delimiter = f"ghadelimiter_{uuid.uuid4().hex}" + handle.write(f"{key}<<{delimiter}\n{value}\n{delimiter}\n") + for key, value in values.items(): + print(f"{key}={value}") + + +def build_admission_identity(decision: Decision) -> dict[str, Any] | None: + """PR identity from the trusted admission job. MAC is added at persist time.""" + if decision.skip: + return None + run_id = parse_github_id(os.environ.get("GITHUB_RUN_ID") or "") + pr_number = parse_github_id(decision.pr_number) + if run_id is None or pr_number is None: + return None + if not all( + [ + decision.head_sha, + decision.head_repo, + decision.base_repo, + decision.candidate_sha, + decision.base_sha, + ] + ): + return None + return { + "pr_number": pr_number, + "head_sha": decision.head_sha, + "head_repo": decision.head_repo, + "base_repo": decision.base_repo, + "candidate_sha": decision.candidate_sha, + "base_sha": decision.base_sha, + "run_id": run_id, + "run_attempt": parse_github_id(os.environ.get("GITHUB_RUN_ATTEMPT") or "1") or 1, + } + + +def post_admission_marker( + token: str, + api_url: str, + repo: str, + identity: dict[str, Any], +) -> None: + """Hide a signed identity on the admitted PR. ECS cannot forge this.""" + github_request( + token, + api_url, + f"/repos/{repo}/issues/{identity['pr_number']}/comments", + method="POST", + payload={"body": format_admission_marker(identity)}, + ) + + +def persist_admission_identity( + decision: Decision, + *, + token: str, + api_url: str, + repo: str, + secret: str, +) -> str: + """Write the lookup artifact and post the HMAC marker. Empty string on success.""" + if decision.skip: + return "" + if not secret.strip(): + return "QUERY_REGRESSION_ADMISSION_HMAC is unset" + identity = build_admission_identity(decision) + if identity is None: + return "could not persist admission identity" + payload = {**identity, "mac": sign_admission(secret, identity)} + with open("query-regression-admission.json", "w", encoding="utf-8") as handle: + json.dump(payload, handle, sort_keys=True) + handle.write("\n") + post_admission_marker(token, api_url, repo, payload) + return "" + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", default=os.environ.get("GITHUB_REPOSITORY") or "") + parser.add_argument( + "--token", + default=os.environ.get("GITHUB_TOKEN") or os.environ.get("GH_TOKEN") or "", + ) + parser.add_argument("--api-url", default=os.environ.get("GITHUB_API_URL") or "https://api.github.com") + parser.add_argument("--actor", default=os.environ.get("COMMENT_ACTOR") or "") + parser.add_argument("--args", default=os.environ.get("COMMAND_ARGS") or "") + parser.add_argument("--pr-number", default=os.environ.get("PR_NUMBER") or "") + parser.add_argument("--comment-id", default=os.environ.get("COMMENT_ID") or "") + parser.add_argument( + "--allowlist", + default=os.environ.get("QUERY_REGRESSION_COMMENT_ALLOWLIST") or "", + ) + parser.add_argument( + "--dispatch-sender", + default=os.environ.get("DISPATCH_SENDER") or "", + ) + parser.add_argument( + "--dispatch-head-sha", + default=os.environ.get("DISPATCH_HEAD_SHA") or "", + ) + return parser.parse_args(argv) + + +def admit_dispatched_command(args: argparse.Namespace, comment_id: int) -> int: + comment = fetch_comment(args.token, args.api_url, args.repo, comment_id) + identity = identity_from_comment(comment, args.repo) + pr_number = str(identity.pr_number or args.pr_number or "") + if identity.error: + write_outputs( + deny( + identity.error, + reply=f"Query regression command ignored: {identity.error}.", + pr_number=pr_number, + ) + ) + return 0 + mismatch = payload_matches_comment( + identity, + actor=args.actor, + pr_number=args.pr_number, + command_args=args.args, + ) + if mismatch: + write_outputs( + deny( + mismatch, + reply=f"Query regression command ignored: {mismatch}.", + pr_number=str(identity.pr_number), + ) + ) + return 0 + + allowlist = parse_allowlist(args.allowlist) + permission = fetch_permission(args.token, args.api_url, args.repo, identity.actor) + pull = fetch_pull(args.token, args.api_url, args.repo, identity.pr_number) + head_mismatch = dispatch_head_matches(pull, args.dispatch_head_sha) + if head_mismatch: + write_outputs( + deny( + head_mismatch, + reply=( + "Query regression command ignored: the PR head changed since the " + "command was dispatched. Review the current revision and comment " + "`/query-regression` again." + ), + pr_number=str(identity.pr_number), + ) + ) + return 0 + decision = admit_pull( + pull, + actor=identity.actor, + allowlist=allowlist, + permission=permission, + command=identity.command, + expected_repo=args.repo, + pr_number=str(identity.pr_number), + ) + if not decision.skip: + persist_error = persist_admission_identity( + decision, + token=args.token, + api_url=args.api_url, + repo=args.repo, + secret=os.environ.get("QUERY_REGRESSION_ADMISSION_HMAC") or "", + ) + if persist_error: + write_outputs( + deny( + persist_error, + reply=f"Query regression command ignored: {persist_error}.", + pr_number=decision.pr_number, + ) + ) + print(persist_error, file=sys.stderr) + return 1 + write_outputs(decision) + return 0 + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv) + comment_id = parse_github_id(str(args.comment_id)) + if not args.repo or not args.token or comment_id is None: + print("--repo, --token, and a numeric --comment-id are required.", file=sys.stderr) + return 2 + + sender_error = dispatch_sender_ok(args.dispatch_sender) + if sender_error: + # Do not reply: the payload PR number is untrusted when the sender is not Actions. + write_outputs(deny(sender_error)) + return 0 + + try: + return admit_dispatched_command(args, comment_id) + except SystemExit as error: + write_outputs( + deny( + f"admission failed: {error}", + reply=( + "Query regression command ignored: GitHub API error while " + "admitting; please retry." + ), + pr_number=str(args.pr_number or ""), + ) + ) + print(error, file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 8d7a37080b..682a2e5921 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -87,6 +87,7 @@ jobs: python3 tests/perf/test_query_regression_summary_otlp.py python3 tests/perf/test_query_regression_case_selection.py python3 tests/perf/test_query_regression_nightly_refs.py + python3 tests/perf/test_query_regression_slash.py python3 tests/perf/test_aliyun_ecs_runner_scripts.py check: diff --git a/.github/workflows/query-regression-comment.yml b/.github/workflows/query-regression-comment.yml index 21592f6fbc..54afa5368e 100644 --- a/.github/workflows/query-regression-comment.yml +++ b/.github/workflows/query-regression-comment.yml @@ -2,20 +2,22 @@ name: Query Regression Comment on: workflow_run: - workflows: ["Query Regression"] + workflows: + - Query Regression Command types: [completed] permissions: contents: read actions: read + issues: read pull-requests: write jobs: comment: if: >- - ${{ github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion != 'cancelled' && - github.event.workflow_run.conclusion != 'skipped' }} + ${{ github.event.workflow_run.conclusion != 'cancelled' && + github.event.workflow_run.conclusion != 'skipped' && + github.event.workflow_run.event == 'repository_dispatch' }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -25,7 +27,7 @@ jobs: ref: ${{ github.event.repository.default_branch }} persist-credentials: false - - name: Locate query regression comment artifact + - name: Locate query regression artifacts id: artifact uses: actions/github-script@v7 with: @@ -38,24 +40,41 @@ jobs: run_id, per_page: 100, }); - const artifact = data.artifacts.find( + const admission = data.artifacts.find( + item => item.name === 'query-regression-admission' && !item.expired + ); + const comment = data.artifacts.find( item => item.name === 'query-regression-comment' && !item.expired ); - if (!artifact) { - core.info('No query-regression-comment artifact found; skipping.'); + if (!admission || !comment) { + core.info('Trusted admission identity or comment artifact missing; skipping.'); core.setOutput('found', 'false'); return; } core.setOutput('found', 'true'); - core.setOutput('id', String(artifact.id)); + core.setOutput('admission_id', String(admission.id)); + core.setOutput('comment_id', String(comment.id)); - - name: Download query regression comment artifact - id: download + - name: Download admission identity + id: download-admission if: ${{ steps.artifact.outputs.found == 'true' }} uses: actions/download-artifact@v4 continue-on-error: true with: - artifact-ids: ${{ steps.artifact.outputs.id }} + artifact-ids: ${{ steps.artifact.outputs.admission_id }} + path: query-regression-admission + repository: ${{ github.repository }} + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + merge-multiple: true + + - name: Download query regression comment artifact + id: download + if: ${{ steps.download-admission.outcome == 'success' }} + uses: actions/download-artifact@v4 + continue-on-error: true + with: + artifact-ids: ${{ steps.artifact.outputs.comment_id }} path: query-regression-comment repository: ${{ github.repository }} github-token: ${{ secrets.GITHUB_TOKEN }} @@ -69,6 +88,7 @@ jobs: env: WORKFLOW_RUN_ID: ${{ github.event.workflow_run.id }} WORKFLOW_RUN_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} + QUERY_REGRESSION_ADMISSION_HMAC: ${{ secrets.QUERY_REGRESSION_ADMISSION_HMAC }} with: script: | const validate = require('./.github/scripts/query-regression-comment.cjs'); diff --git a/.github/workflows/query-regression-slash.yml b/.github/workflows/query-regression-slash.yml new file mode 100644 index 0000000000..945cfb851b --- /dev/null +++ b/.github/workflows/query-regression-slash.yml @@ -0,0 +1,121 @@ +name: Query Regression Command + +# Handler for `/query-regression`, dispatched by slash-command-dispatch.yml. +# Requires github-actions[bot] as the dispatch sender, re-fetches the +# comment by id, requires the current PR head to match the dispatcher +# snapshot, admits an allowlisted admin, snapshots merge/head/base SHAs, +# and calls the reusable Query Regression workflow. + +on: + repository_dispatch: + types: [query-regression-command] + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + parse: + name: Admit dispatched query-regression command + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + skip: ${{ steps.admit.outputs.skip }} + reason: ${{ steps.admit.outputs.reason }} + case: ${{ steps.admit.outputs.case }} + pr_number: ${{ steps.admit.outputs.pr_number }} + base_sha: ${{ steps.admit.outputs.base_sha }} + candidate_sha: ${{ steps.admit.outputs.candidate_sha }} + head_sha: ${{ steps.admit.outputs.head_sha }} + head_repo: ${{ steps.admit.outputs.head_repo }} + base_repo: ${{ steps.admit.outputs.base_repo }} + steps: + - name: Checkout trusted admission script + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Admit dispatched command + id: admit + env: + GITHUB_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + GITHUB_API_URL: ${{ github.api_url }} + COMMENT_ID: ${{ github.event.client_payload.github.payload.comment.id }} + COMMENT_ACTOR: ${{ github.event.client_payload.github.payload.comment.user.login }} + COMMAND_ARGS: ${{ github.event.client_payload.slash_command.args.unnamed.all || '' }} + PR_NUMBER: ${{ github.event.client_payload.github.payload.issue.number }} + DISPATCH_SENDER: ${{ github.event.sender.login }} + DISPATCH_HEAD_SHA: ${{ github.event.client_payload.pull_request.head.sha }} + QUERY_REGRESSION_COMMENT_ALLOWLIST: ${{ vars.QUERY_REGRESSION_COMMENT_ALLOWLIST }} + # Never reference this secret in query-regression.yml: the ECS job + # would inherit it and could forge the admission marker. + QUERY_REGRESSION_ADMISSION_HMAC: ${{ secrets.QUERY_REGRESSION_ADMISSION_HMAC }} + run: python3 .github/scripts/query-regression-slash.py + + - name: Summarize admission + if: always() + env: + SKIP: ${{ steps.admit.outputs.skip }} + REASON: ${{ steps.admit.outputs.reason }} + CASE: ${{ steps.admit.outputs.case }} + PR_NUMBER: ${{ steps.admit.outputs.pr_number }} + BASE_SHA: ${{ steps.admit.outputs.base_sha }} + CANDIDATE_SHA: ${{ steps.admit.outputs.candidate_sha }} + HEAD_SHA: ${{ steps.admit.outputs.head_sha }} + run: | + set -euo pipefail + if [[ "${SKIP}" == "true" ]]; then + printf 'Skipping query-regression command: %s\n' "${REASON}" | tee -a "${GITHUB_STEP_SUMMARY}" + elif [[ "${SKIP}" == "false" ]]; then + { + printf 'Admitted `/query-regression` (`%s`) on PR #%s\n' "${CASE}" "${PR_NUMBER}" + printf -- '- Head SHA: `%s`\n' "${HEAD_SHA}" + printf -- '- Merge SHA: `%s`\n' "${CANDIDATE_SHA}" + printf -- '- Base SHA: `%s`\n' "${BASE_SHA}" + } | tee -a "${GITHUB_STEP_SUMMARY}" + else + printf 'Admission failed without a decision (the admit step crashed); see its logs.\n' \ + | tee -a "${GITHUB_STEP_SUMMARY}" >&2 + exit 1 + fi + + - name: Upload admission identity + if: ${{ steps.admit.outputs.skip == 'false' && steps.admit.outputs.candidate_sha != '' }} + uses: actions/upload-artifact@v4 + with: + name: query-regression-admission + path: query-regression-admission.json + if-no-files-found: error + retention-days: 7 + overwrite: true + + - name: Reply with admission result + if: ${{ !cancelled() && steps.admit.outputs.reply != '' }} + env: + GH_TOKEN: ${{ github.token }} + REPLY: ${{ steps.admit.outputs.reply }} + PR_NUMBER: ${{ steps.admit.outputs.pr_number || github.event.client_payload.github.payload.issue.number }} + run: | + set -euo pipefail + printf '%s\n' "${REPLY}" | gh pr comment "${PR_NUMBER}" --body-file - + + query-regression: + name: Query regression from comment + needs: [parse] + if: ${{ needs.parse.outputs.skip == 'false' && needs.parse.outputs.candidate_sha != '' }} + uses: ./.github/workflows/query-regression.yml + # inherit is for Aliyun/GH PAT used by provision/teardown. Do not + # reference QUERY_REGRESSION_ADMISSION_HMAC in the reusable workflow. + secrets: inherit + with: + case: ${{ needs.parse.outputs.case }} + base_ref: ${{ needs.parse.outputs.base_sha }} + candidate_ref: ${{ needs.parse.outputs.candidate_sha }} + cargo_profile: nightly + runner: ${{ vars.QUERY_REGRESSION_PR_RUNNER || 'aliyun-ecs' }} + pr_number: ${{ needs.parse.outputs.pr_number }} + head_sha: ${{ needs.parse.outputs.head_sha }} + head_repo: ${{ needs.parse.outputs.head_repo }} + base_repo: ${{ needs.parse.outputs.base_repo }} diff --git a/.github/workflows/query-regression.yml b/.github/workflows/query-regression.yml index e94cd1f95d..c9db9ed5c5 100644 --- a/.github/workflows/query-regression.yml +++ b/.github/workflows/query-regression.yml @@ -36,6 +36,28 @@ on: required: false type: string default: aliyun-ecs + pr_number: + description: >- + When set, treat this reusable call as PR admission (merge-SHA + verification, trusted helper scripts, sticky comment artifact) + required: false + type: string + default: "" + head_sha: + description: PR head SHA snapshotted at admission (required with pr_number) + required: false + type: string + default: "" + head_repo: + description: PR head repository full_name (required with pr_number) + required: false + type: string + default: "" + base_repo: + description: PR base repository full_name (required with pr_number) + required: false + type: string + default: "" workflow_dispatch: inputs: case: @@ -82,21 +104,20 @@ on: required: false type: boolean default: false - pull_request: - types: [labeled] permissions: + # Load-bearing: every job, including ECS, gets this GITHUB_TOKEN. + # Do not grant issues or pull-requests write; candidate code could + # otherwise forge the HMAC admission marker on a victim PR. contents: read jobs: test-tooling: # Stdlib unittests for the workflow Python (case selection, report - # helpers, nightly SHA picking, and rendered ECS user-data). They do - # not talk to Aliyun or the Actions runner process; running them on - # ubuntu-latest fails fast before any ECS spend. Not gated on the - # regression labels: those labels boot a VM, these tests should not. - # Ordinary PRs also run the same tests from checks.yml, because this - # workflow only starts on `labeled` (or dispatch / workflow_call). + # helpers, nightly SHA picking, comment-command admission, and rendered + # ECS user-data). They do not talk to Aliyun or the Actions runner + # process; running them on ubuntu-latest fails fast before any ECS spend. + # Ordinary PRs also run the same tests from checks.yml. runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -112,24 +133,16 @@ jobs: python3 tests/perf/test_query_regression_summary_otlp.py python3 tests/perf/test_query_regression_case_selection.py python3 tests/perf/test_query_regression_nightly_refs.py + python3 tests/perf/test_query_regression_slash.py python3 tests/perf/test_aliyun_ecs_runner_scripts.py provision: - # Runs when the aliyun-ecs path is selected: explicitly via the runner - # input, or for PR labels by default (a QUERY_REGRESSION_PR_RUNNER - # repository variable set to another value redirects PRs to that literal - # runner label instead). Uses trusted scripts from the PR base (or the - # dispatched ref), never from candidate code. Waits for test-tooling so - # a broken user-data template does not still create a VM. + # Runs when the aliyun-ecs path is selected via the runner input (PR + # comment admission can redirect with QUERY_REGRESSION_PR_RUNNER). Uses + # trusted scripts from the caller ref, never from candidate code. Waits + # for test-tooling so a broken user-data template does not still create a VM. needs: [test-tooling] - if: >- - ${{ !failure() && !cancelled() && - ((github.event_name != 'pull_request' && inputs.runner == 'aliyun-ecs') || - (github.event_name == 'pull_request' && - !github.event.pull_request.draft && - (github.event.label.name == 'query-regression' || - github.event.label.name == 'heavy-regression') && - (vars.QUERY_REGRESSION_PR_RUNNER || 'aliyun-ecs') == 'aliyun-ecs')) }} + if: ${{ !failure() && !cancelled() && inputs.runner == 'aliyun-ecs' }} runs-on: ubuntu-latest timeout-minutes: 45 outputs: @@ -141,7 +154,7 @@ jobs: uses: actions/checkout@v4 with: repository: ${{ github.repository }} - ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.sha }} + ref: ${{ github.sha }} persist-credentials: false - name: Install uv @@ -170,20 +183,11 @@ jobs: # lets the job run when provision was intentionally skipped because a # literal runner label was selected; a failed or cancelled provision still # blocks the run because no ECS runner would be waiting. - if: >- - ${{ !failure() && !cancelled() && - (github.event_name != 'pull_request' || - (github.event_name == 'pull_request' && - !github.event.pull_request.draft && - (github.event.label.name == 'query-regression' || - github.event.label.name == 'heavy-regression'))) }} - runs-on: >- - ${{ needs.provision.outputs.label || - (github.event_name != 'pull_request' && inputs.runner || - (vars.QUERY_REGRESSION_PR_RUNNER || 'aliyun-ecs')) }} + if: ${{ !failure() && !cancelled() }} + runs-on: ${{ needs.provision.outputs.label || inputs.runner }} timeout-minutes: 180 env: - CARGO_PROFILE: ${{ github.event_name == 'pull_request' && 'nightly' || inputs.cargo_profile }} + CARGO_PROFILE: ${{ inputs.cargo_profile }} CARGO_HOME: /home/runner/.cargo UV_CACHE_DIR: /home/runner/.cargo/uv-cache RUSTUP_HOME: /opt/rustup @@ -198,11 +202,18 @@ jobs: CARGO_INCREMENTAL: "0" RUSTFLAGS: -D warnings -C link-arg=-fuse-ld=mold QUERY_REGRESSION_CACHE_EPOCH: "1" - BASE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || inputs.base_ref }} - CANDIDATE_REF: ${{ github.event_name != 'pull_request' && (inputs.candidate_ref || github.ref) || '' }} - EVENT_MERGE_SHA: ${{ github.event_name == 'pull_request' && github.sha || '' }} - EVENT_HEAD_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || '' }} - EVENT_BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || '' }} + # Reusable workflows keep the caller's github.event_name (slash is + # repository_dispatch; nightly is workflow_run), never "workflow_call". + # PR admission is the explicit pr_number input from the slash wrapper. + PR_ADMISSION: ${{ inputs.pr_number != '' }} + PR_NUMBER: ${{ inputs.pr_number }} + HEAD_REPO: ${{ inputs.head_repo }} + BASE_REPO: ${{ inputs.base_repo }} + BASE_REF: ${{ inputs.base_ref }} + CANDIDATE_REF: ${{ inputs.candidate_ref || github.ref }} + EVENT_MERGE_SHA: ${{ inputs.pr_number != '' && inputs.candidate_ref || '' }} + EVENT_HEAD_SHA: ${{ inputs.head_sha }} + EVENT_BASE_SHA: ${{ inputs.pr_number != '' && inputs.base_ref || '' }} # Runner identity contract. The ECS image uses 1001; a manually prepared # host may override via repo variables when 1001 is already taken. EXPECTED_RUNNER_UID: ${{ vars.QUERY_REGRESSION_RUNNER_UID || '1001' }} @@ -276,7 +287,7 @@ jobs: fail_closed() { printf '%s\n' \ - "PR changed or merge result unavailable; maintainer must remove and re-add the regression label after reviewing current revision. $1" >&2 + "PR changed or merge result unavailable; maintainer must re-admit the current revision by commenting /query-regression. $1" >&2 exit 1 } @@ -318,7 +329,7 @@ jobs: exit 1 } - if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then + if [[ "${PR_ADMISSION}" == "true" ]]; then for identity in EVENT_MERGE_SHA EVENT_HEAD_SHA EVENT_BASE_SHA; do value="${!identity-}" is_full_sha "${value}" || fail_closed "Missing or invalid ${identity}." @@ -392,7 +403,7 @@ jobs: run: | set -euo pipefail helper_ref="${VERIFIED_CANDIDATE_SHA}" - if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then + if [[ "${PR_ADMISSION}" == "true" ]]; then helper_ref="${VERIFIED_BASE_SHA}" fi mkdir -p "${GITHUB_WORKSPACE}/query-regression-trusted-scripts" @@ -736,16 +747,16 @@ jobs: - name: Run query regression id: run env: - CASE_PATHS: ${{ github.event_name == 'pull_request' && (github.event.label.name == 'heavy-regression' && 'heavy' || 'all') || inputs.case }} - HTTP_TIMEOUT: ${{ github.event_name == 'pull_request' && '300' || inputs.http_timeout }} - ALLOW_LARGE_FIXTURE: ${{ github.event_name == 'pull_request' && 'true' || inputs.allow_large_fixture }} + CASE_PATHS: ${{ inputs.case }} + HTTP_TIMEOUT: ${{ inputs.http_timeout }} + ALLOW_LARGE_FIXTURE: ${{ inputs.allow_large_fixture }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - CASE_NAME: ${{ github.event_name == 'pull_request' && (github.event.label.name == 'heavy-regression' && 'heavy case set' || 'default case set') || inputs.case }} + CASE_NAME: ${{ inputs.case }} BASE_BIN: ${{ github.workspace }}/query-regression-bins/base/greptime CANDIDATE_BIN: ${{ github.workspace }}/query-regression-bins/candidate/greptime FIXTURE_GENERATOR: ${{ github.workspace }}/query-regression-bins/candidate/query_perf_fixture OTELGEN_BIN: /usr/local/bin/otelgen - SUMMARY_SCRIPT: ${{ github.event_name == 'pull_request' && 'query-regression-trusted-scripts/query-regression-summary.py' || 'src/.github/scripts/query-regression-summary.py' }} + SUMMARY_SCRIPT: ${{ env.PR_ADMISSION == 'true' && 'query-regression-trusted-scripts/query-regression-summary.py' || 'src/.github/scripts/query-regression-summary.py' }} QUERY_REGRESSION_RUNNER: ${{ github.workspace }}/query-regression-bins/candidate/query_regression_runner run: >- uv run --no-project python src/.github/scripts/query-regression-run.py @@ -754,13 +765,9 @@ jobs: --summary-script "${SUMMARY_SCRIPT}" - name: Write PR metadata for trusted comment workflow - if: ${{ always() && github.event_name == 'pull_request' }} + if: ${{ always() && env.PR_ADMISSION == 'true' }} env: - PR_NUMBER: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} - BASE_REPO: ${{ github.event.pull_request.base.repo.full_name }} + HEAD_SHA: ${{ inputs.head_sha }} RUN_ID: ${{ github.run_id }} RUN_ATTEMPT: ${{ github.run_attempt }} run: uv run --no-project python query-regression-trusted-scripts/query-regression-pr-metadata.py @@ -780,9 +787,11 @@ jobs: machine-telemetry.log if-no-files-found: warn retention-days: 7 + # Re-run failed jobs uploads again in the same run; v4 rejects duplicate names. + overwrite: true - name: Upload trusted comment artifact - if: ${{ always() && github.event_name == 'pull_request' }} + if: ${{ always() && env.PR_ADMISSION == 'true' }} uses: actions/upload-artifact@v4 with: name: query-regression-comment @@ -791,6 +800,7 @@ jobs: query-regression-work/**/query-regression-report.json if-no-files-found: warn retention-days: 7 + overwrite: true - name: Report cache usage if: ${{ always() }} @@ -883,7 +893,7 @@ jobs: uses: actions/checkout@v4 with: repository: ${{ github.repository }} - ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.sha }} + ref: ${{ github.sha }} persist-credentials: false - name: Install uv diff --git a/.github/workflows/slash-command-dispatch.yml b/.github/workflows/slash-command-dispatch.yml new file mode 100644 index 0000000000..db2191c170 --- /dev/null +++ b/.github/workflows/slash-command-dispatch.yml @@ -0,0 +1,36 @@ +name: Slash Command Dispatch + +# ChatOps front door: parse `/command` on PR comments, check admin +# permission, and repository_dispatch to a per-command handler. Handlers +# own allowlists, SHA admission, and the actual work. Same-repo dispatch +# uses github.token with contents: write; GitHub starts the handler run +# for GITHUB_TOKEN-created repository_dispatch events. Do not pass the +# long-lived GH_PERSONAL_ACCESS_TOKEN into this third-party action. +# +# To add a command: list it under `commands` and add a workflow with +# `on.repository_dispatch.types: ["-command"]`. + +on: + issue_comment: + types: [created] + +permissions: + contents: write + issues: write + pull-requests: write + +jobs: + slash-command-dispatch: + name: Dispatch slash command + if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '/') }} + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Slash Command Dispatch + uses: peter-evans/slash-command-dispatch@9bdcd7914ec1b75590b790b844aa3b8eee7c683a # v5.0.2 + with: + token: ${{ github.token }} + permission: admin + issue-type: pull-request + commands: | + query-regression diff --git a/tests/perf/AGENTS.md b/tests/perf/AGENTS.md index 822b99080f..72c1912bb4 100644 --- a/tests/perf/AGENTS.md +++ b/tests/perf/AGENTS.md @@ -20,6 +20,18 @@ waits for a successful Nightly Build, then calls `query-regression.yml` with the previous vs current nightly SHAs. Keep SHA selection in `.github/scripts/query-regression-nightly-refs.py`. +- PR comment admission is two workflows: `slash-command-dispatch.yml` + (peter-evans/slash-command-dispatch) decides whether a `/command` should + run and `repository_dispatch`es payload context; `query-regression-slash.yml` + handles `/query-regression` (allowlist, dispatcher head SHA, merge SHA, + reusable call). Keep case-arg validation and dispatch-sender/head checks in + `.github/scripts/query-regression-slash.py`. The + admission job on ubuntu-latest posts a hidden HMAC-signed marker comment on + the admitted PR and uploads `query-regression-admission` as a lookup hint. + The sticky-comment workflow verifies that marker (`QUERY_REGRESSION_ADMISSION_HMAC`, + never referenced from `query-regression.yml`) before posting. There is no + PR-label trigger. To add another command, list it in the dispatcher and add + a `repository_dispatch` handler. - The case DSL is not required to keep compatibility inside this PR. When the DSL changes, update TOML cases, the outer lifecycle script, Rust helpers, and docs together. @@ -35,8 +47,7 @@ - Before pushing perf harness changes, run at least: - the Python tests in the `test-tooling` job of `.github/workflows/query-regression.yml` (ubuntu-latest, not the ECS runner). - The Checks workflow runs the same tests on ordinary PRs so they are not - gated on the `query-regression` / `heavy-regression` labels. + The Checks workflow runs the same tests on ordinary PRs. - `cargo fmt --all -- --check` - `cargo build -p cmd --bin query_perf_fixture --features dev-tools` - `cargo build -p cmd --bin query_regression_runner --features dev-tools` diff --git a/tests/perf/README.md b/tests/perf/README.md index 47495ca531..42fd8c286e 100644 --- a/tests/perf/README.md +++ b/tests/perf/README.md @@ -203,8 +203,8 @@ experiment matrix for those policy comparisons. case for issue #7913. It writes 8192 series × 20160 samples through remote-write in 1440-sample daily time chunks, flushing after each chunk before running 1d/7d/14d TQL selectors. It is not included in the default `all` case set because ingestion -cost dominates routine CI validation. Adding the `heavy-regression` PR label runs -only this case; `query-regression` runs the six routine default cases. Manual +cost dominates routine CI validation. Commenting `/query-regression heavy` runs +only this case; `/query-regression` runs the six routine default cases. Manual workflow dispatch accepts the `heavy` token to select this case. ## OTLP trace load scenario @@ -391,19 +391,43 @@ binaries. Candidate `query_perf_fixture` and `query_regression_runner` are the extra head-side helpers; `finalize-remote` uses candidate `greptime datanode parquetbench/scanbench` as the read-bench tool against each target's data directory. -The workflow runs automatically only when `query-regression` or `heavy-regression` -is added to a non-draft PR; it does not rerun on pushes, ready-for-review, or -reopen events. `query-regression` runs the six routine default cases, while -`heavy-regression` runs only the high-cardinality remote-write #7913 case. PR runs -build base/candidate once and use `--allow-large-fixture`. Manual -`workflow_dispatch` runs can pass `all`, `heavy`, one case path, or a -comma/whitespace-separated list of case paths, and can override refs. +The workflow runs when an allowlisted repository admin comments +`/query-regression` on a non-draft PR. It does not rerun on pushes, +ready-for-review, or reopen events. `/query-regression` runs the six routine +default cases; `/query-regression heavy` runs only the high-cardinality +remote-write #7913 case. PR runs build base/candidate once and +use `--allow-large-fixture`. Manual `workflow_dispatch` runs can pass `all`, +`heavy`, one case path, or a comma/whitespace-separated list of case paths, and +can override refs. + +Comment admission is two workflows. `slash-command-dispatch.yml` uses +[peter-evans/slash-command-dispatch](https://github.com/peter-evans/slash-command-dispatch) +to parse the first line of a PR comment and, for `/query-regression`, +`repository_dispatch` a payload with the comment, actor, args, and PR number. +`query-regression-slash.yml` handles that event: it requires the dispatch +sender to be `github-actions[bot]`, re-checks +`QUERY_REGRESSION_COMMENT_ALLOWLIST` (comma/whitespace-separated GitHub logins) +and repository `admin` permission, requires the current PR head to match the +head SHA peter-evans snapshotted at comment time, then snapshots +merge/head/base SHAs and calls the reusable workflow. If the head moved while +the handler was queued, admission denies and asks for another +`/query-regression`. Same-repo dispatch uses `github.token` with +`contents: write`; GitHub starts the handler for `GITHUB_TOKEN`-created +`repository_dispatch` events. The handler runs on the default branch, so it +has secrets even for fork PRs. A later head +change does not retarget an already queued run; comment `/query-regression` +again after reviewing the new revision. To add another slash command, list it +in the dispatcher and add a `repository_dispatch` handler. The main report artifact uploads only aggregate/per-target JSON reports, component logs, and `query-regression-summary.md` with seven-day retention; fixture data, SSTs, and cluster state are excluded. PR runs also upload a -separate trusted-comment artifact containing PR metadata and aggregate reports. -The workflow writes the Markdown summary to the workflow step summary and -updates a sticky PR comment through the trusted follow-up workflow. +runner comment artifact with reports. The ubuntu-latest admission job posts a +hidden HMAC-signed marker comment on the admitted PR (`QUERY_REGRESSION_ADMISSION_HMAC`) +and uploads `query-regression-admission` as a lookup hint. The sticky-comment +workflow verifies that marker before posting; the ECS job cannot forge it +because it has neither `issues` write nor the HMAC secret. The workflow writes +the Markdown summary to the workflow step summary and updates a sticky PR +comment through the trusted follow-up workflow. ## Built-in cases diff --git a/tests/perf/test_query_regression_slash.py b/tests/perf/test_query_regression_slash.py new file mode 100644 index 0000000000..bca34fa50d --- /dev/null +++ b/tests/perf/test_query_regression_slash.py @@ -0,0 +1,475 @@ +#!/usr/bin/env python3 +# Copyright 2023 Greptime Team +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Coverage for PR comment admission of query-regression.""" + +import importlib.util +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + + +SCRIPTS_DIR = Path(__file__).parents[2] / ".github/scripts" + + +def load_module(): + spec = importlib.util.spec_from_file_location( + "query_regression_slash_under_test", + SCRIPTS_DIR / "query-regression-slash.py", + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +slash = load_module() + +HEAD = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +BASE = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +MERGE = "cccccccccccccccccccccccccccccccccccccccc" + + +def pull_payload(**overrides: object) -> dict: + payload = { + "number": 42, + "state": "open", + "draft": False, + "merged": False, + "mergeable": True, + "merge_commit_sha": MERGE, + "base": { + "sha": BASE, + "repo": {"full_name": "GreptimeTeam/greptimedb"}, + }, + "head": { + "sha": HEAD, + "repo": {"full_name": "alice/greptimedb"}, + }, + } + payload.update(overrides) + return payload + + +def admit(*, body: str = "/query-regression", actor: str = "maintainer", **pull_overrides: object): + payload = pull_payload(**pull_overrides) + return slash.admit_pull( + payload, + actor=actor, + allowlist=slash.parse_allowlist("maintainer, other-admin"), + permission="admin", + command=slash.parse_command(body), + expected_repo="GreptimeTeam/greptimedb", + pr_number=str(payload.get("number") or ""), + ) + + +class ParseCaseArgsTest(unittest.TestCase): + def test_empty_args_select_all(self) -> None: + parsed = slash.parse_case_args("") + self.assertTrue(parsed.matched) + self.assertEqual(parsed.case, "all") + self.assertEqual(parsed.error, "") + + def test_heavy_and_explicit_paths(self) -> None: + self.assertEqual(slash.parse_case_args("heavy").case, "heavy") + self.assertEqual( + slash.parse_case_args("tests/perf/query_cases/sql_topk_order_by/case.toml").case, + "tests/perf/query_cases/sql_topk_order_by/case.toml", + ) + + def test_rejects_path_escape_and_all_mixed(self) -> None: + traversal = slash.parse_case_args("tests/perf/query_cases/../secret.toml") + self.assertIn("..", traversal.error) + mixed = slash.parse_case_args("all heavy") + self.assertIn("cannot be mixed", mixed.error) + + +class ParseCommandTest(unittest.TestCase): + def test_bare_command_selects_all(self) -> None: + parsed = slash.parse_command("/query-regression") + self.assertTrue(parsed.matched) + self.assertEqual(parsed.case, "all") + self.assertEqual(parsed.error, "") + + def test_heavy_and_explicit_paths(self) -> None: + heavy = slash.parse_command("/query-regression heavy") + self.assertEqual(heavy.case, "heavy") + paths = slash.parse_command( + "/query-regression tests/perf/query_cases/sql_topk_order_by/case.toml" + ) + self.assertEqual(paths.case, "tests/perf/query_cases/sql_topk_order_by/case.toml") + + def test_ignores_body_after_first_line(self) -> None: + parsed = slash.parse_command("/query-regression heavy\nplease run this") + self.assertEqual(parsed.case, "heavy") + + def test_quoted_or_unrelated_comments_do_not_match(self) -> None: + self.assertFalse(slash.parse_command("> /query-regression").matched) + self.assertFalse(slash.parse_command("please /query-regression").matched) + self.assertFalse(slash.parse_command("/query-regression-extra").matched) + + +class CommentIdentityTest(unittest.TestCase): + def comment(self, **overrides: object) -> dict: + payload = { + "user": {"login": "maintainer"}, + "body": "/query-regression heavy", + "issue_url": "https://api.github.com/repos/GreptimeTeam/greptimedb/issues/42", + } + payload.update(overrides) + return payload + + def test_reads_actor_pr_and_case_from_comment(self) -> None: + identity = slash.identity_from_comment( + self.comment(), + "GreptimeTeam/greptimedb", + ) + self.assertEqual(identity.error, "") + self.assertEqual(identity.actor, "maintainer") + self.assertEqual(identity.pr_number, 42) + self.assertEqual(identity.command.case, "heavy") + + def test_rejects_wrong_repo_and_non_command_body(self) -> None: + other_repo = slash.identity_from_comment( + self.comment(), + "other/repo", + ) + self.assertIn("not other/repo", other_repo.error) + not_command = slash.identity_from_comment( + self.comment(body="please run this"), + "GreptimeTeam/greptimedb", + ) + self.assertIn("not a query-regression command", not_command.error) + + def test_payload_mismatch_is_rejected(self) -> None: + identity = slash.identity_from_comment( + self.comment(), + "GreptimeTeam/greptimedb", + ) + self.assertEqual( + slash.payload_matches_comment( + identity, + actor="stranger", + pr_number="42", + command_args="heavy", + ), + "payload actor does not match comment author", + ) + self.assertEqual( + slash.payload_matches_comment( + identity, + actor="maintainer", + pr_number="99", + command_args="heavy", + ), + "payload PR number does not match comment", + ) + self.assertEqual( + slash.payload_matches_comment( + identity, + actor="maintainer", + pr_number="42", + command_args="all", + ), + "payload command args do not match comment", + ) + self.assertEqual( + slash.payload_matches_comment( + identity, + actor="maintainer", + pr_number="42", + command_args="heavy", + ), + "", + ) + self.assertEqual( + slash.payload_matches_comment( + identity, + actor="maintainer", + pr_number="abc", + command_args="heavy", + ), + "payload PR number is not a valid integer", + ) + + +class DispatchTrustTest(unittest.TestCase): + def test_accepts_github_actions_bot_sender(self) -> None: + self.assertEqual(slash.dispatch_sender_ok("github-actions[bot]"), "") + self.assertEqual(slash.dispatch_sender_ok("GitHub-Actions[bot]"), "") + + def test_rejects_non_actions_sender(self) -> None: + self.assertEqual( + slash.dispatch_sender_ok("alice"), + "repository_dispatch sender is not github-actions[bot]", + ) + self.assertEqual( + slash.dispatch_sender_ok(""), + "repository_dispatch sender is not github-actions[bot]", + ) + + def test_main_rejects_non_actions_sender_without_fetching(self) -> None: + def boom(*_args: object, **_kwargs: object) -> dict: + raise AssertionError("should not fetch when sender is untrusted") + + with patch.object(slash, "fetch_comment", boom): + self.assertEqual( + slash.main( + [ + "--repo", + "o/r", + "--token", + "t", + "--comment-id", + "1", + "--dispatch-sender", + "alice", + ] + ), + 0, + ) + + def test_head_must_match_dispatcher_snapshot(self) -> None: + pull = pull_payload() + self.assertEqual(slash.dispatch_head_matches(pull, HEAD), "") + self.assertEqual(slash.dispatch_head_matches(pull, HEAD.upper()), "") + self.assertIn("changed", slash.dispatch_head_matches(pull, BASE)) + self.assertIn("missing", slash.dispatch_head_matches(pull, "not-a-sha")) + self.assertIn("missing", slash.dispatch_head_matches(pull, "")) + + def test_api_failure_writes_retry_reply_and_stays_red(self) -> None: + def boom(*_args: object, **_kwargs: object) -> dict: + raise SystemExit( + "GitHub API GET /repos/o/r/issues/comments/1 failed: HTTP 502: no" + ) + + with patch.object(slash, "fetch_comment", boom): + code = slash.main( + [ + "--repo", + "o/r", + "--token", + "t", + "--comment-id", + "1", + "--dispatch-sender", + "github-actions[bot]", + "--pr-number", + "42", + ] + ) + self.assertEqual(code, 1) + + def test_write_outputs_preserves_multiline_reason_and_reply(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + output_path = os.path.join(tmp, "github_output") + with patch.dict(os.environ, {"GITHUB_OUTPUT": output_path}): + slash.write_outputs( + slash.deny( + "HTTP 502: line1\nline2", + reply="please retry\nthen comment again", + pr_number="42", + ) + ) + text = Path(output_path).read_text(encoding="utf-8") + self.assertIn("HTTP 502: line1\nline2\n", text) + self.assertIn("please retry\nthen comment again\n", text) + self.assertNotIn("reason=HTTP 502", text) + self.assertRegex(text, r"reason< None: + self.assertEqual( + slash.parse_allowlist("@Ada, bob\nCarol"), + frozenset({"ada", "bob", "carol"}), + ) + + +class ParseGithubIdTest(unittest.TestCase): + def test_accepts_positive_integers(self) -> None: + self.assertEqual(slash.parse_github_id("42"), 42) + self.assertEqual(slash.parse_github_id(" 7 "), 7) + + def test_rejects_non_numeric_and_non_positive(self) -> None: + self.assertIsNone(slash.parse_github_id("abc")) + self.assertIsNone(slash.parse_github_id("")) + self.assertIsNone(slash.parse_github_id("0")) + self.assertIsNone(slash.parse_github_id("-1")) + + def test_non_numeric_comment_id_fails_closed(self) -> None: + self.assertEqual( + slash.main(["--repo", "o/r", "--token", "t", "--comment-id", "abc"]), + 2, + ) + + +class AdmitPullTest(unittest.TestCase): + def test_allowlisted_admin_admits_merge_sha(self) -> None: + decision = admit() + self.assertFalse(decision.skip) + self.assertEqual(decision.case, "all") + self.assertEqual(decision.candidate_sha, MERGE) + self.assertEqual(decision.head_sha, HEAD) + self.assertEqual(decision.base_sha, BASE) + self.assertEqual(decision.pr_number, "42") + self.assertEqual(decision.head_repo, "alice/greptimedb") + self.assertEqual(decision.reply, "") + + def test_unknown_commenter_is_denied(self) -> None: + decision = admit(actor="stranger") + self.assertTrue(decision.skip) + self.assertIn("ALLOWLIST", decision.reason) + self.assertEqual(decision.pr_number, "42") + + def test_allowlisted_non_admin_is_denied(self) -> None: + decision = slash.admit_pull( + pull_payload(), + actor="maintainer", + allowlist=slash.parse_allowlist("maintainer"), + permission="write", + command=slash.parse_command("/query-regression"), + expected_repo="GreptimeTeam/greptimedb", + pr_number="42", + ) + self.assertTrue(decision.skip) + self.assertIn("admin", decision.reason) + self.assertEqual(decision.pr_number, "42") + + def test_empty_allowlist_fails_closed(self) -> None: + decision = slash.admit_pull( + pull_payload(), + actor="maintainer", + allowlist=frozenset(), + permission="admin", + command=slash.parse_command("/query-regression"), + expected_repo="GreptimeTeam/greptimedb", + pr_number="42", + ) + self.assertTrue(decision.skip) + self.assertIn("ALLOWLIST", decision.reason) + + def test_draft_and_conflicted_prs_are_not_admitted(self) -> None: + draft = admit(draft=True) + self.assertTrue(draft.skip) + conflicted = admit(mergeable=False) + self.assertTrue(conflicted.skip) + pending = admit(mergeable=None) + self.assertTrue(pending.skip) + + def test_missing_merge_sha_fails_closed(self) -> None: + decision = admit(merge_commit_sha="not-a-sha") + self.assertTrue(decision.skip) + + def test_deleted_fork_head_repo_fails_closed(self) -> None: + decision = admit(head={"sha": HEAD, "repo": None}) + self.assertTrue(decision.skip) + self.assertIn("head repository", decision.reason) + self.assertEqual(decision.pr_number, "42") + + +class AdmissionMarkerTest(unittest.TestCase): + def identity(self) -> dict: + return { + "run_id": 123, + "pr_number": 42, + "head_sha": HEAD, + "head_repo": "alice/greptimedb", + "base_repo": "GreptimeTeam/greptimedb", + "candidate_sha": MERGE, + "base_sha": BASE, + } + + def test_sign_and_verify_round_trip(self) -> None: + identity = self.identity() + mac = slash.sign_admission("secret", identity) + self.assertTrue(slash.verify_admission_mac("secret", identity, mac)) + self.assertFalse(slash.verify_admission_mac("other", identity, mac)) + tampered = {**identity, "pr_number": 99} + self.assertFalse(slash.verify_admission_mac("secret", tampered, mac)) + + def test_parse_ignores_surrounding_text(self) -> None: + identity = {**self.identity(), "mac": slash.sign_admission("secret", self.identity())} + parsed = slash.parse_admission_marker(f"noise\n{slash.format_admission_marker(identity)}trailing") + assert parsed is not None + self.assertEqual(parsed["pr_number"], 42) + self.assertTrue(slash.verify_admission_mac("secret", parsed, parsed["mac"])) + + def test_missing_hmac_secret_fails_closed(self) -> None: + self.assertEqual( + slash.persist_admission_identity( + admit(), + token="t", + api_url="https://api.github.com", + repo="GreptimeTeam/greptimedb", + secret="", + ), + "QUERY_REGRESSION_ADMISSION_HMAC is unset", + ) + + def test_persist_posts_signed_marker(self) -> None: + posted: dict[str, object] = {} + + def fake_request( + token: str, + api_url: str, + path: str, + *, + method: str = "GET", + payload: dict | None = None, + ) -> dict: + posted["method"] = method + posted["path"] = path + posted["payload"] = payload + return {"id": 7} + + decision = admit() + with tempfile.TemporaryDirectory() as tmp: + cwd = os.getcwd() + os.chdir(tmp) + try: + with patch.dict(os.environ, {"GITHUB_RUN_ID": "99", "GITHUB_RUN_ATTEMPT": "1"}): + with patch.object(slash, "github_request", fake_request): + error = slash.persist_admission_identity( + decision, + token="t", + api_url="https://api.github.com", + repo="GreptimeTeam/greptimedb", + secret="secret", + ) + self.assertEqual(error, "") + self.assertEqual(posted["method"], "POST") + self.assertEqual( + posted["path"], + "/repos/GreptimeTeam/greptimedb/issues/42/comments", + ) + body = (posted["payload"] or {})["body"] # type: ignore[index] + parsed = slash.parse_admission_marker(str(body)) + assert parsed is not None + self.assertEqual(parsed["run_id"], 99) + self.assertTrue(slash.verify_admission_mac("secret", parsed, parsed["mac"])) + finally: + os.chdir(cwd) + + +if __name__ == "__main__": + unittest.main()