diff --git a/docker/job-started-hook.sh b/docker/job-started-hook.sh index e056bee..8b7d8bc 100644 --- a/docker/job-started-hook.sh +++ b/docker/job-started-hook.sh @@ -43,13 +43,24 @@ while [ "$attempt" -le "$assignment_max_attempts" ]; do if [ "$status" = '200' ]; then exit 0 fi - if [ "$status" != '202' ]; then - printf '%s\n' "::error title=Cloudflare runner cache assignment::The Worker returned HTTP $status while waiting for GitHub's runner assignment." - exit 1 - fi + case "$status" in + 202) ;; + # Transient conditions, not verdicts. The assignment record is written by + # the Worker and read back through Cloudflare's edge, so a container can + # poll before its authorization is visible and see 401. 000 is curl's + # output for a connection failure. Keep polling inside the existing + # bounded window instead of failing the job on the first sample. + 000|401|408|429|500|502|503|504) ;; + *) + printf '%s\n' "::error title=Cloudflare runner cache assignment::The Worker returned HTTP $status while waiting for GitHub's runner assignment." + exit 1 + ;; + esac attempt=$((attempt + 1)) sleep "$assignment_poll_seconds" done -printf '%s\n' "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within ${assignment_max_attempts} seconds." +# Still fail closed. The job must not run with a cache capability whose +# assignment the Worker never confirmed. +printf '%s\n' "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within ${assignment_max_attempts} seconds (last Worker status: ${status})." exit 1 diff --git a/tests/job-started-hook.test.ts b/tests/job-started-hook.test.ts index 997bcea..863634f 100644 --- a/tests/job-started-hook.test.ts +++ b/tests/job-started-hook.test.ts @@ -67,7 +67,7 @@ describe("runner cache assignment hook", () => { }, 10_000); it("fails closed on an unexpected Worker status and never prints the runner capability", async () => { - const server = createServer((_request, response) => response.writeHead(401).end()); + const server = createServer((_request, response) => response.writeHead(403).end()); const port = await listen(server); const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); const configurationPath = join(directory, "cache-assignment"); @@ -78,7 +78,7 @@ describe("runner cache assignment hook", () => { await expect(runHook(configurationPath)).resolves.toEqual({ code: 1, stdout: - "::error title=Cloudflare runner cache assignment::The Worker returned HTTP 401 while waiting for GitHub's runner assignment.\n", + "::error title=Cloudflare runner cache assignment::The Worker returned HTTP 403 while waiting for GitHub's runner assignment.\n", stderr: "", }); await expect(readFile(configurationPath)).rejects.toMatchObject({ code: "ENOENT" }); @@ -110,7 +110,7 @@ describe("runner cache assignment hook", () => { ).resolves.toEqual({ code: 1, stdout: - "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within 2 seconds.\n", + "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within 2 seconds (last Worker status: 202).\n", stderr: "", }); expect(attempts).toBe(2); @@ -121,6 +121,64 @@ describe("runner cache assignment hook", () => { } }); + it("keeps polling when the assignment authorization is not yet visible", async () => { + // The Worker writes the assignment record and the container reads it back + // through Cloudflare's edge, so the first samples can 401 before the + // authorization propagates. That is a race, not a verdict. + let attempts = 0; + const server = createServer((_request, response) => { + attempts += 1; + response.writeHead(attempts < 3 ? 401 : 200).end(); + }); + const port = await listen(server); + const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); + const configurationPath = join(directory, "cache-assignment"); + await writeFile(configurationPath, `http://127.0.0.1:${port}/v1/runner-cache\nBearer runner-capability\n`, { + mode: 0o600, + }); + + try { + await expect( + runHook(configurationPath, { + CF_RUNNER_CACHE_ASSIGNMENT_MAX_ATTEMPTS: "5", + CF_RUNNER_CACHE_ASSIGNMENT_POLL_SECONDS: "0", + }), + ).resolves.toEqual({ code: 0, stdout: "", stderr: "" }); + expect(attempts).toBe(3); + await expect(readFile(configurationPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await close(server); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("still fails closed when the authorization never becomes visible", async () => { + const server = createServer((_request, response) => response.writeHead(401).end()); + const port = await listen(server); + const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); + const configurationPath = join(directory, "cache-assignment"); + const capability = "Bearer capability-that-must-not-leak"; + await writeFile(configurationPath, `http://127.0.0.1:${port}/v1/runner-cache\n${capability}\n`, { mode: 0o600 }); + + try { + const result = await runHook(configurationPath, { + CF_RUNNER_CACHE_ASSIGNMENT_MAX_ATTEMPTS: "2", + CF_RUNNER_CACHE_ASSIGNMENT_POLL_SECONDS: "0", + }); + expect(result).toEqual({ + code: 1, + stdout: + "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within 2 seconds (last Worker status: 401).\n", + stderr: "", + }); + expect(result.stdout).not.toContain(capability); + await expect(readFile(configurationPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await close(server); + await rm(directory, { recursive: true, force: true }); + } + }); + it("does not start a job when the one-time cache configuration is malformed", async () => { const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); const configurationPath = join(directory, "cache-assignment");