diff --git a/.github/workflows/phpunit-feature.yml b/.github/workflows/phpunit-feature.yml index 3929f9396..184511f00 100644 --- a/.github/workflows/phpunit-feature.yml +++ b/.github/workflows/phpunit-feature.yml @@ -86,7 +86,7 @@ jobs: run: node --test tests/Unit/PatchCohortMaintenanceTest.mjs - name: Run the required checks for merge queue groups - run: node --test tests/Unit/MergeGroupTriggersTest.mjs + run: node --test tests/Unit/MergeGroupTriggersTest.mjs tests/Unit/MergeQueueSonarTest.mjs - name: Report one Shared Server fixtures result on every pull request run: node --test tests/Unit/ServerParityAggregateTest.mjs tests/Unit/ServerParityPartsTest.mjs diff --git a/.github/workflows/queue-sonar.yml b/.github/workflows/queue-sonar.yml new file mode 100644 index 000000000..fd24c0f44 --- /dev/null +++ b/.github/workflows/queue-sonar.yml @@ -0,0 +1,65 @@ +name: Merge queue Sonar + +# workflow_run uses the default-branch controller, never candidate workflow source. +on: + workflow_run: + workflows: [PHPUnit Feature] + types: [completed] + +permissions: + contents: read + +concurrency: + group: queue-sonar-${{ github.event.workflow_run.head_sha }} + cancel-in-progress: false + +jobs: + queue-analysis: + name: SonarQube merge queue analysis + if: >- + github.event_name == 'workflow_run' && + github.event.workflow_run.event == 'merge_group' && + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Check out trusted scanner controls + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ github.workflow_sha }} + path: .sonar-control + persist-credentials: false + + - name: Validate the merge queue source identity + id: candidate + run: node .sonar-control/scripts/ci/merge-queue-sonar.mjs validate + + - name: Check out the synthetic commit as analysis data + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ steps.candidate.outputs.sha }} + path: .sonar-candidate + fetch-depth: 0 + persist-credentials: false + submodules: false + lfs: false + + - name: Apply trusted scanner configuration + run: node .sonar-control/scripts/ci/merge-queue-sonar.mjs prepare + + # No candidate command, dependency installation or scanner configuration + # executes with this token. Only the pinned static scanner receives it. + # The required Sonar App check gates this SHA. Upload the analysis and + # let GitHub wait for that App's quality-gate result. + - uses: SonarSource/sonarqube-scan-action@d209202bc7d53ff1cc128f7f907dac145c9d6ae9 # v8.3.0 + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + with: + projectBaseDir: .sonar-candidate + args: >- + -Dsonar.branch.name=${{ steps.candidate.outputs.branch }} + -Dsonar.scm.revision=${{ steps.candidate.outputs.sha }} + -Dsonar.scm.exclusions.disabled=true + -Dsonar.rust.clippy.enabled=false + -Dsonar.working.directory=${{ runner.temp }}/queue-sonar-work + -Dsonar.qualitygate.wait=false diff --git a/scripts/ci/merge-queue-sonar.mjs b/scripts/ci/merge-queue-sonar.mjs new file mode 100644 index 000000000..500525e3e --- /dev/null +++ b/scripts/ci/merge-queue-sonar.mjs @@ -0,0 +1,57 @@ +#!/usr/bin/env node + +import {execFileSync} from 'node:child_process'; +import {appendFileSync, copyFileSync, lstatSync, readFileSync, readdirSync} from 'node:fs'; +import {resolve} from 'node:path'; +import {fileURLToPath} from 'node:url'; + +export function queueIdentity(event, repositoryId, repositoryName) { + const run = event.workflow_run; + if (!/^[1-9]\d*$/.test(String(repositoryId)) || repositoryName !== 'durable-workflow/server' || + event.repository?.id !== Number(repositoryId) || event.repository?.full_name !== repositoryName || + run?.repository?.id !== Number(repositoryId) || run?.head_repository?.id !== Number(repositoryId) || + run?.name !== 'PHPUnit Feature' || run?.path !== '.github/workflows/phpunit-feature.yml' || + run?.event !== 'merge_group' || run?.status !== 'completed' || run?.conclusion !== 'success') { + throw new Error('Expected a successful merge_group PHPUnit Feature run from this repository'); + } + if (typeof run.head_sha !== 'string' || !/^[0-9a-f]{40}$/.test(run.head_sha) || /^0+$/.test(run.head_sha) || + typeof run.head_branch !== 'string' || !/^gh-readonly-queue\/main\/pr-[1-9]\d*-[0-9a-f]{40}$/.test(run.head_branch)) { + throw new Error('Invalid merge queue commit or branch identity'); + } + return {sha: run.head_sha, branch: run.head_branch}; +} + +function regularSource(directory) { + for (const entry of readdirSync(directory, {withFileTypes: true})) { + if (entry.name === '.git') continue; + const path = resolve(directory, entry.name); + if (entry.isDirectory()) regularSource(path); + else if (!entry.isFile()) throw new Error('Analysis data must contain only regular files and directories'); + } +} + +export function prepareSource(candidateDirectory, trustedConfiguration, sha) { + const candidate = resolve(candidateDirectory); + if (!lstatSync(candidate).isDirectory()) throw new Error('Candidate must be a directory'); + const head = execFileSync('/usr/bin/git', ['-C', candidate, 'rev-parse', 'HEAD'], {encoding: 'utf8'}).trim(); + if (head !== sha) throw new Error('Checked-out candidate differs from the merge queue SHA'); + const entries = execFileSync('/usr/bin/git', ['-C', candidate, 'ls-files', '--stage', '-z'], {encoding: 'utf8'}).split('\0'); + if (entries.some(entry => entry.startsWith('120000 ') || entry.startsWith('160000 '))) { + throw new Error('Merge queue analysis does not follow symlinks or submodules'); + } + regularSource(candidate); + // Candidate scanner properties never control the credentialed analysis. + copyFileSync(trustedConfiguration, resolve(candidate, 'sonar-project.properties')); +} + +if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + const event = JSON.parse(readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8')); + const identity = queueIdentity(event, process.env.GITHUB_REPOSITORY_ID, process.env.GITHUB_REPOSITORY); + if (process.argv[2] === 'validate') { + appendFileSync(process.env.GITHUB_OUTPUT, `sha=${identity.sha}\nbranch=${identity.branch}\n`); + } else if (process.argv[2] === 'prepare') { + prepareSource('.sonar-candidate', '.sonar-control/sonar-project.properties', identity.sha); + } else { + throw new Error('Expected validate or prepare'); + } +} diff --git a/scripts/conformance/server-parity/lock-pressure-middleware.php b/scripts/conformance/server-parity/lock-pressure-middleware.php index 1ced25c8d..100f86901 100644 --- a/scripts/conformance/server-parity/lock-pressure-middleware.php +++ b/scripts/conformance/server-parity/lock-pressure-middleware.php @@ -15,7 +15,7 @@ /** * The published PHP SQLite reference shares its file with its maintenance loop - * and may refuse a request with the documented 503 backend_lock_pressure reply + * and may defer a request with the documented backend_lock_pressure reply * (see lockPressureOperationRetryDelay and lockPressurePollRetryDelay for the * exact conditions). This Guzzle middleware retries only that refusal, below the SDK transport, so * the transport keeps every capability the SDK checks (bounded requests, @@ -30,11 +30,12 @@ final class LockPressurePollRetries { /** @var list */ public static array $retries = []; + private static bool $reporting = false; public static function middleware(): callable { - if (!self::$reporting) { + if (! self::$reporting) { self::$reporting = true; register_shutdown_function(static function (): void { if (self::$retries !== []) { @@ -51,18 +52,23 @@ public static function middleware(): callable while (true) { try { $response = $handler($request, $options)->wait(); - } catch (\Throwable $error) { + } catch (Throwable $error) { return Create::rejectionFor($error); } // A request body that cannot be replayed exactly is never retried. - if (!$response instanceof ResponseInterface || $response->getStatusCode() !== 503 - || (!$request->getBody()->isSeekable() && $request->getBody()->getSize() !== 0)) { + if (! $response instanceof ResponseInterface + || (! $request->getBody()->isSeekable() && $request->getBody()->getSize() !== 0)) { + return Create::promiseFor($response); + } + $status = $response->getStatusCode(); + if ($status !== 503 && ! ($status >= 200 && $status < 300 && $request->getMethod() === 'POST' + && str_ends_with($request->getUri()->getPath(), '/heartbeat'))) { return Create::promiseFor($response); } $text = (string) $response->getBody(); $response = $response->withBody(Utils::streamFor($text)); $decoded = json_decode($text, true); - $delay = lockPressureOperationRetryDelay($request->getMethod(), (string) $request->getUri(), 503, + $delay = lockPressureOperationRetryDelay($request->getMethod(), (string) $request->getUri(), $status, is_array($decoded) ? $decoded : null, $count, $waited); if ($delay === null || ($budget > 0 && microtime(true) - $started + $delay >= $budget)) { return Create::promiseFor($response); diff --git a/scripts/conformance/server-parity/lock-pressure-retry.php b/scripts/conformance/server-parity/lock-pressure-retry.php index 112299d78..625c36ff0 100644 --- a/scripts/conformance/server-parity/lock-pressure-retry.php +++ b/scripts/conformance/server-parity/lock-pressure-retry.php @@ -49,6 +49,9 @@ function lockPressurePollRetryDelay( * task and attempt identifiers (`outcome: deferred`), so repeating the request * cannot record a second outcome. Polls keep lockPressurePollRetryDelay's * stricter check that no task was handed out. + * Published heartbeat endpoints also defer without recording progress in a + * 2xx envelope, to keep older workers alive. Only their explicit negative + * recording/renewal acknowledgement is eligible for the same bounded retry. * * Returns the delay in seconds before the next attempt, or null to return the * refusal to the caller unchanged. @@ -68,7 +71,15 @@ function lockPressureOperationRetryDelay( return lockPressurePollRetryDelay($method, $uri, $status, $response, $retriesSoFar, $waitedSeconds, $maxRetries, $maxWaitSeconds); } - if ($status !== 503 || ! is_array($response) || ($response['reason'] ?? null) !== 'backend_lock_pressure' + $deferredField = match (true) { + preg_match('~/worker/activity-tasks/[^/]+/heartbeat$~', $path) === 1 => 'heartbeat_recorded', + preg_match('~/worker/workflow-tasks/[^/]+/heartbeat$~', $path) === 1 => 'renewed', + str_ends_with($path, '/worker/heartbeat') => 'acknowledged', + default => null, + }; + $deferredHeartbeat = $method === 'POST' && $status >= 200 && $status < 300 + && $deferredField !== null && is_array($response) && ($response[$deferredField] ?? null) === false; + if (($status !== 503 && ! $deferredHeartbeat) || ! is_array($response) || ($response['reason'] ?? null) !== 'backend_lock_pressure' || ($response['retryable'] ?? null) !== true || $retriesSoFar >= $maxRetries) { return null; } diff --git a/tests/Unit/MergeGroupTriggersTest.mjs b/tests/Unit/MergeGroupTriggersTest.mjs index 0b196a7df..7fe4c1866 100644 --- a/tests/Unit/MergeGroupTriggersTest.mjs +++ b/tests/Unit/MergeGroupTriggersTest.mjs @@ -1,7 +1,7 @@ // The Actions checks a GitHub merge queue waits for must run on merge_group. -// Source-only: enabling the queue is a separate repository setting. Its Sonar -// gate is a separate prerequisite (Server #725); the terminal Shared Server -// fixtures result is covered by ServerParityAggregateTest.mjs. +// Queue activation is a separate repository setting. Sonar scans synthetic +// commits through default-branch workflow_run controls; the terminal Shared +// Server fixtures result is covered by ServerParityAggregateTest.mjs. import test from 'node:test'; import assert from 'node:assert/strict'; import {readFileSync} from 'node:fs'; @@ -24,7 +24,7 @@ for (const name of ['phpunit-feature.yml', 'server-parity.yml', 'public-boundary // The Sonar job receives SONAR_TOKEN and its fork guard is always true for a // merge group, so candidate source must not reach it through the queue. -test('SonarQube does not run for merge queue groups', () => { +test('candidate-controlled merge_group workflows never receive the Sonar token', () => { assert.doesNotMatch(triggers(workflow('sonar.yml')), /merge_group/); }); diff --git a/tests/Unit/MergeQueueSonarTest.mjs b/tests/Unit/MergeQueueSonarTest.mjs new file mode 100644 index 000000000..a958be424 --- /dev/null +++ b/tests/Unit/MergeQueueSonarTest.mjs @@ -0,0 +1,101 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {execFileSync} from 'node:child_process'; +import {mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import {prepareSource, queueIdentity} from '../../scripts/ci/merge-queue-sonar.mjs'; + +const repository = {id: 123, full_name: 'durable-workflow/server'}; +const valid = () => ({repository, workflow_run: { + repository, head_repository: repository, name: 'PHPUnit Feature', + path: '.github/workflows/phpunit-feature.yml', event: 'merge_group', + status: 'completed', conclusion: 'success', head_sha: 'a'.repeat(40), + head_branch: `gh-readonly-queue/main/pr-42-${'b'.repeat(40)}`, +}}); +const identity = event => queueIdentity(event, '123', repository.full_name); + +test('accepts the successful same-repository synthetic identity', () => { + assert.deepEqual(identity(valid()), {sha: 'a'.repeat(40), branch: valid().workflow_run.head_branch}); +}); + +for (const [key, value] of [ + ['event', 'pull_request'], ['event', 'workflow_dispatch'], ['conclusion', 'failure'], + ['conclusion', 'skipped'], ['status', 'in_progress'], ['name', 'Other workflow'], + ['path', '.github/workflows/other.yml'], ['head_repository', {id: 456}], + ['repository', {id: 456}], ['head_sha', '0'.repeat(40)], ['head_sha', 'main'], + ['head_sha', `${'a'.repeat(40)}\n`], ['head_sha', `${'a'.repeat(40)}\r`], ['head_branch', 'main'], + ['head_branch', `gh-readonly-queue/main/pr-42-${'b'.repeat(40)}\n`], + ['head_branch', `gh-readonly-queue/other/pr-42-${'b'.repeat(40)}`], + ['head_branch', `gh-readonly-queue/main/pr-42-${'b'.repeat(40)}\n-Dsonar.host.url=https://example.invalid`], +]) { + test(`refuses ${key}=${JSON.stringify(value)}`, () => { + const event = valid(); + event.workflow_run[key] = value; + assert.throws(() => identity(event)); + }); +} + +test('does not trust a matching name from a different repository ID', () => { + assert.throws(() => queueIdentity(valid(), '456', repository.full_name)); +}); + +function source(t) { + const root = mkdtempSync(join(tmpdir(), 'merge-queue-sonar-')); + t.after(() => rmSync(root, {recursive: true, force: true})); + execFileSync('git', ['init', '-q', root]); + writeFileSync(join(root, 'sonar-project.properties'), 'sonar.host.url=https://example.invalid\n'); + execFileSync('git', ['-C', root, 'add', '.']); + execFileSync('git', ['-C', root, '-c', 'user.name=Test', '-c', 'user.email=test@example.invalid', 'commit', '-qm', 'fixture']); + const sha = execFileSync('git', ['-C', root, 'rev-parse', 'HEAD'], {encoding: 'utf8'}).trim(); + const trusted = join(root, '.git', 'trusted.properties'); + writeFileSync(trusted, 'sonar.projectKey=durable-workflow_server\n'); + return {root, sha, trusted}; +} + +test('replaces candidate scanner settings using trusted configuration', t => { + const {root, sha, trusted} = source(t); + prepareSource(root, trusted, sha); + assert.equal(readFileSync(join(root, 'sonar-project.properties'), 'utf8'), readFileSync(trusted, 'utf8')); +}); + +test('refuses a checkout other than the reported synthetic commit', t => { + const {root, trusted} = source(t); + assert.throws(() => prepareSource(root, trusted, 'f'.repeat(40)), /differs/); +}); + +test('refuses tracked and untracked symlinks before overwriting configuration', t => { + const {root, sha, trusted} = source(t); + symlinkSync(trusted, join(root, 'escape')); + assert.throws(() => prepareSource(root, trusted, sha), /regular files/); + execFileSync('git', ['-C', root, 'add', 'escape']); + assert.throws(() => prepareSource(root, trusted, sha), /symlinks or submodules/); + assert.equal(readFileSync(join(root, 'sonar-project.properties'), 'utf8'), 'sonar.host.url=https://example.invalid\n'); +}); + +test('refuses submodule entries even though checkout does not populate them', t => { + const {root, sha, trusted} = source(t); + execFileSync('git', ['-C', root, 'update-index', '--add', '--cacheinfo', `160000,${sha},module`]); + assert.throws(() => prepareSource(root, trusted, sha), /symlinks or submodules/); +}); + +test('queue scanning uses trusted controls and a separate candidate without running its commands', () => { + const text = readFileSync('.github/workflows/queue-sonar.yml', 'utf8'); + const queue = text.split(' queue-analysis:\n')[1]; + assert.ok(text.includes('workflows: [PHPUnit Feature]')); + assert.ok(!text.includes(' pull_request:')); + assert.ok(!text.includes(' push:')); + assert.ok(queue.includes("github.event.workflow_run.event == 'merge_group'")); + assert.ok(queue.includes("github.event.workflow_run.conclusion == 'success'")); + assert.ok(queue.includes('ref: ${{ github.workflow_sha }}')); + assert.ok(queue.includes('ref: ${{ steps.candidate.outputs.sha }}')); + assert.deepEqual(queue.match(/^ *run:.*$/gm)?.map(line => line.trim()), [ + 'run: node .sonar-control/scripts/ci/merge-queue-sonar.mjs validate', + 'run: node .sonar-control/scripts/ci/merge-queue-sonar.mjs prepare', + ]); + assert.equal((queue.match(/SONAR_TOKEN: /g) ?? []).length, 1); + assert.ok(queue.includes('-Dsonar.scm.revision=${{ steps.candidate.outputs.sha }}')); + assert.ok(queue.includes('-Dsonar.rust.clippy.enabled=false')); + assert.ok(queue.includes('-Dsonar.qualitygate.wait=false')); + assert.ok(queue.includes('-Dsonar.working.directory=${{ runner.temp }}/queue-sonar-work')); +}); diff --git a/tests/Unit/ServerParityLockPressureRetryTest.php b/tests/Unit/ServerParityLockPressureRetryTest.php index 849e1b310..81d1fff33 100644 --- a/tests/Unit/ServerParityLockPressureRetryTest.php +++ b/tests/Unit/ServerParityLockPressureRetryTest.php @@ -4,9 +4,14 @@ namespace Tests\Unit; +use GuzzleHttp\Client; +use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\HandlerStack; +use GuzzleHttp\Middleware; +use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; -require_once dirname(__DIR__, 2).'/scripts/conformance/server-parity/lock-pressure-retry.php'; +require_once dirname(__DIR__, 2).'/scripts/conformance/server-parity/lock-pressure-middleware.php'; final class ServerParityLockPressureRetryTest extends TestCase { @@ -80,4 +85,42 @@ public function test_retries_stop_at_the_attempt_and_wait_budgets(): void self::assertNull(\lockPressurePollRetryDelay('POST', self::POLL, 503, $this->refusal(), 2, 29.5)); self::assertSame(1.0, \lockPressurePollRetryDelay('POST', self::POLL, 503, $this->refusal(), 14, 29.0)); } + + public function test_only_explicitly_deferred_published_heartbeat_shapes_are_retried(): void + { + $base = 'http://parity-server:8080/api/worker/'; + foreach (['activity-tasks/task/heartbeat' => 'heartbeat_recorded', + 'workflow-tasks/task/heartbeat' => 'renewed', 'heartbeat' => 'acknowledged'] as $path => $field) { + $body = ['reason' => 'backend_lock_pressure', 'retryable' => true, $field => false, 'retry_after_seconds' => 0.1]; + self::assertSame(0.1, \lockPressureOperationRetryDelay('POST', $base.$path, 200, $body, 0, 0.0)); + foreach ([[$field => true], [$field => null], [$field => 'false'], ['retryable' => false], + ['reason' => 'attempt_closed']] as $change) { + self::assertNull(\lockPressureOperationRetryDelay('POST', $base.$path, 200, array_replace($body, $change), 0, 0.0)); + } + foreach ([['GET', $base.$path, 200], ['POST', $base.$path, 409], ['POST', $base.$path, 500], + ['POST', $base.'activity-tasks/task/complete', 200], ['POST', $base.'sessions/session/heartbeat', 200]] as [$method, $uri, $status]) { + self::assertNull(\lockPressureOperationRetryDelay($method, $uri, $status, $body, 0, 0.0)); + } + self::assertNull(\lockPressureOperationRetryDelay('POST', $base.$path, 200, $body, 15, 0.0)); + self::assertNull(\lockPressureOperationRetryDelay('POST', $base.$path, 200, $body, 0, 30.0)); + } + } + + public function test_middleware_replays_the_same_heartbeat_after_a_deferred_200(): void + { + $deferred = ['reason' => 'backend_lock_pressure', 'retryable' => true, 'heartbeat_recorded' => false, 'retry_after_seconds' => 0.1]; + $terminal = ['reason' => 'attempt_closed', 'heartbeat_recorded' => false]; + $mock = new MockHandler([new Response(200, [], json_encode($deferred)), new Response(200, [], json_encode($terminal))]); + $stack = HandlerStack::create($mock); + $requests = []; + $stack->push(Middleware::history($requests)); + $stack->push(\LockPressurePollRetries::middleware()); + $response = (new Client(['handler' => $stack]))->post('http://parity-server:8080/api/worker/activity-tasks/task/heartbeat', [ + 'body' => '{"lease_owner":"worker","progress":1}', 'timeout' => 2, + ]); + self::assertSame($terminal, json_decode((string) $response->getBody(), true)); + self::assertCount(1, $requests, 'The recorder sees the final response only.'); + self::assertSame(0, $mock->count(), 'Both scripted responses were consumed.'); + self::assertSame('{"lease_owner":"worker","progress":1}', (string) $mock->getLastRequest()->getBody()); + } }