diff --git a/.changeset/complete-openapi.md b/.changeset/complete-openapi.md new file mode 100644 index 0000000..e810b0e --- /dev/null +++ b/.changeset/complete-openapi.md @@ -0,0 +1,5 @@ +--- +'@flakemetry/contracts': patch +--- + +`/openapi.json` now describes the whole API rather than the read half. It was generated from the read-route table, so ingestion, the quality gate, artifact presigning and quarantine were invisible to anything generating a client — with nothing in the document to say a client was seeing a fraction of it. Request bodies are now included, generated from the same zod schemas the endpoints validate against. diff --git a/apps/api/package.json b/apps/api/package.json index d48f89e..4b2582c 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -36,12 +36,14 @@ "@flakemetry/contracts": "workspace:*", "@flakemetry/db": "workspace:*", "@flakemetry/queries": "workspace:*", + "@flakemetry/storage": "workspace:*", "@opentelemetry/api": "^1.9.0", "@opentelemetry/exporter-metrics-otlp-http": "^0.221.0", "@opentelemetry/resources": "^2.10.0", "@opentelemetry/sdk-metrics": "^2.10.0", "@trpc/server": "^11.0.0", "fastify": "^5.11.0", - "@flakemetry/storage": "workspace:*" + "zod": "^3.25.76", + "zod-to-json-schema": "^3.24.1" } } diff --git a/apps/api/src/__tests__/api-surface.test.ts b/apps/api/src/__tests__/api-surface.test.ts index cdf9cd1..2cb15f9 100644 --- a/apps/api/src/__tests__/api-surface.test.ts +++ b/apps/api/src/__tests__/api-surface.test.ts @@ -7,7 +7,7 @@ import type { PrismaClient } from '@flakemetry/db' import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { buildApp } from '../app' -import { READ_ROUTES } from '../rest' +import { openApiDocument, READ_ROUTES } from '../rest' import { appRouter } from '../trpc/router' const app = buildApp({ prisma: {} as unknown as PrismaClient, store: null }) @@ -69,3 +69,57 @@ describe('documented API surface', () => { expect(documented).toEqual(exposed) }) }) + +describe('the machine-readable description covers the whole surface', () => { + const document = openApiDocument('0.0.0') as { + paths: Record> + } + + it('describes every documented endpoint, not only the readable half', () => { + // It used to be generated from READ_ROUTES, so ingestion, the quality gate and + // quarantine were invisible to anything generating a client — with nothing in the + // document to say a client was seeing a fraction of the API. + const missing = REST_ENDPOINTS.filter((endpoint) => { + const path = endpoint.path.replace(/:(\w+)/g, '{$1}') + return !document.paths[path]?.[endpoint.method.toLowerCase()] + }).map((endpoint) => `${endpoint.method} ${endpoint.path}`) + + expect(missing).toEqual([]) + }) + + it('describes the body of every endpoint that takes one', () => { + for (const endpoint of REST_ENDPOINTS.filter((candidate) => candidate.request)) { + const path = endpoint.path.replace(/:(\w+)/g, '{$1}') + const operation = document.paths[path]?.[endpoint.method.toLowerCase()] + expect(operation?.requestBody, `${endpoint.path} has no requestBody`).toBeDefined() + } + }) + + it('leaves the unauthenticated endpoints unauthenticated', () => { + const health = document.paths['/health']?.get + const ingest = document.paths['/v1/ingest']?.post + + // A document that demands a token for the liveness probe teaches people to ignore its + // security blocks entirely. + expect(health?.security).toEqual([]) + expect(ingest?.security).not.toEqual([]) + }) + + it('declares every path parameter it names', () => { + const undeclared: string[] = [] + for (const [path, operations] of Object.entries(document.paths)) { + const named = [...path.matchAll(/\{(\w+)\}/g)].map((match) => match[1]) + for (const [method, operation] of Object.entries(operations)) { + const params = ((operation as { parameters?: { name: string; in: string }[] }).parameters ?? + []) as { name: string; in: string }[] + for (const name of named) { + if (!params.some((param) => param.name === name && param.in === 'path')) { + undeclared.push(`${method.toUpperCase()} ${path} → ${name}`) + } + } + } + } + // A path with an undeclared parameter is a URL no generated client can build. + expect(undeclared).toEqual([]) + }) +}) diff --git a/apps/api/src/__tests__/rest.test.ts b/apps/api/src/__tests__/rest.test.ts index b3dc1c7..6200693 100644 --- a/apps/api/src/__tests__/rest.test.ts +++ b/apps/api/src/__tests__/rest.test.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'node:crypto' +import { REST_ENDPOINTS } from '@flakemetry/contracts' import { generateToken, hashToken, PrismaClient } from '@flakemetry/db' import type { FastifyInstance } from 'fastify' import { afterAll, beforeEach, describe, expect, it } from 'vitest' @@ -27,10 +28,23 @@ describe('openapi document', () => { it('describes every route that is registered', () => { const document = openApiDocument('0.1.0') as { paths: Record } - // Generated from the same table that registers the routes, so a spec that disagrees with - // the implementation is not expressible. - expect(Object.keys(document.paths)).toHaveLength(READ_ROUTES.length) + // Generated from REST_ENDPOINTS — the same table the human reference is built from — + // so a spec that disagrees with the implementation is not expressible. It covers the + // whole surface, not only the read routes, which is why the count is checked against + // the documented endpoints rather than against READ_ROUTES. + const documented = new Set( + REST_ENDPOINTS.map((endpoint) => endpoint.path.replace(/:(\w+)/g, '{$1}')), + ) + expect(Object.keys(document.paths).sort()).toEqual([...documented].sort()) expect(document.paths['/v1/runs/{runId}']).toBeDefined() + expect(document.paths['/v1/ingest']).toBeDefined() + }) + + it('still registers every read route it describes', () => { + expect(READ_ROUTES.length).toBeGreaterThan(0) + for (const route of READ_ROUTES) { + expect(REST_ENDPOINTS.some((endpoint) => endpoint.path === route.path)).toBe(true) + } }) }) diff --git a/apps/api/src/rest.ts b/apps/api/src/rest.ts index 2857202..3060b18 100644 --- a/apps/api/src/rest.ts +++ b/apps/api/src/rest.ts @@ -3,6 +3,9 @@ import { createGzip } from 'node:zlib' import { flakyBoardInputSchema, + REST_ENDPOINTS, + type RestAuth, + type RestEndpoint, runsListInputSchema, testGetInputSchema, testHealthInputSchema, @@ -22,6 +25,7 @@ import { } from '@flakemetry/queries' import { type ObjectStore, projectArtifactPrefix } from '@flakemetry/storage' import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify' +import { zodToJsonSchema } from 'zod-to-json-schema' import { type AuthenticatedProject, authenticateProject, hasScope } from './auth' import type { RateLimiter } from './rate-limit' @@ -218,40 +222,98 @@ export const READ_ROUTES: ReadRoute[] = [ }, ] +const SCOPE_NOTE: Record = { + none: null, + 'ingest-token': 'Needs a token carrying the "ingest" scope.', + 'read-token': 'Needs a token carrying the "read" scope.', + 'quarantine-token': 'Needs a token carrying the "quarantine" scope.', + 'any-token': 'Needs a project token carrying either the "ingest" or the "read" scope.', +} + +const LEADING_STATUS = /^`(\d{3})`/ + +const successStatus = (response: string): number => { + const match = LEADING_STATUS.exec(response.trim()) + return match ? Number(match[1]) : 200 +} + +const readRouteByPath = new Map(READ_ROUTES.map((route) => [route.path, route])) + +const parametersFor = (endpoint: RestEndpoint): unknown[] => { + const route = readRouteByPath.get(endpoint.path) + const declared = [ + ...(route?.params ?? []).map((param) => ({ ...param, in: 'path', required: true })), + ...(route?.query ?? []).map((param) => ({ ...param, in: 'query', required: false })), + ] + if (declared.length > 0) { + return declared.map((param) => ({ + name: param.name, + in: param.in, + required: param.required, + description: param.description, + schema: { type: 'string' }, + })) + } + + // Paths carrying a segment that no route table describes still have to declare it, or + // the document is not a valid description of a URL a client can build. + return [...endpoint.path.matchAll(/:(\w+)/g)].map((match) => ({ + name: match[1], + in: 'path', + required: true, + schema: { type: 'string' }, + })) +} + +/** + * Generated from `REST_ENDPOINTS` — the same table the human reference is built from — so + * the machine-readable description covers the whole surface rather than the read half. + * It previously came from `READ_ROUTES`, which meant ingestion, the quality gate and + * quarantine were invisible to anything generating a client, with nothing to say so. + */ export const openApiDocument = (version: string): Record => { - const paths: Record = {} + const paths: Record> = {} + + for (const endpoint of REST_ENDPOINTS) { + const path = endpoint.path.replace(/:(\w+)/g, '{$1}') + const route = readRouteByPath.get(endpoint.path) + const scopeNote = SCOPE_NOTE[endpoint.auth] + + const responses: Record = { + [successStatus(endpoint.response)]: { + description: endpoint.response, + ...(route?.produces ? { content: { [route.produces]: {} } } : {}), + }, + } + if (endpoint.auth !== 'none') { + responses[401] = { description: 'Missing or invalid token' } + responses[403] = { description: 'The token does not carry the required scope' } + responses[429] = { description: 'Rate limited' } + } - for (const route of READ_ROUTES) { - const path = route.path.replace(/:(\w+)/g, '{$1}') paths[path] = { - get: { - summary: route.summary, - security: [{ bearerAuth: [] }], - parameters: [ - ...(route.params ?? []).map((param) => ({ - name: param.name, - in: 'path', - required: true, - description: param.description, - schema: { type: 'string' }, - })), - ...(route.query ?? []).map((param) => ({ - name: param.name, - in: 'query', - required: false, - description: param.description, - schema: { type: 'string' }, - })), - ], - responses: { - 200: route.produces - ? { description: 'Success', content: { [route.produces]: {} } } - : { description: 'Success' }, - 401: { description: 'Missing or invalid token' }, - 403: { description: 'The token does not carry the read scope' }, - 404: { description: 'Not found' }, - 429: { description: 'Rate limited' }, - }, + ...paths[path], + [endpoint.method.toLowerCase()]: { + summary: endpoint.summary, + ...(scopeNote ? { description: scopeNote } : {}), + security: endpoint.auth === 'none' ? [] : [{ bearerAuth: [] }], + parameters: parametersFor(endpoint), + ...(endpoint.request + ? { + requestBody: { + required: true, + content: { + 'application/json': { + // Inlined rather than named: a `name` puts the schema under + // `definitions` and leaves a $ref pointing where OpenAPI does not + // look, so a generated client cannot resolve it. + schema: zodToJsonSchema(endpoint.request.schema, { $refStrategy: 'none' }), + }, + }, + }, + } + : {}), + responses, }, } } @@ -259,17 +321,16 @@ export const openApiDocument = (version: string): Record => { return { openapi: '3.1.0', info: { - title: 'Flakemetry read API', + title: 'Flakemetry API', version, description: - 'Read-only access to a single project. Authorise with a token carrying the "read" scope; an ingest token will not do, so a credential handed to a script cannot forge test data.', + 'Ingestion, read and quarantine for a single project. Scopes are separate on purpose: a credential handed to a script cannot forge test data, one that reads cannot silence a failing test, and the one in every CI job carries neither.', }, components: { securitySchemes: { bearerAuth: { type: 'http', scheme: 'bearer', description: 'A project token' }, }, }, - security: [{ bearerAuth: [] }], paths, } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bfa4009..ccadee6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -70,6 +70,12 @@ importers: fastify: specifier: ^5.11.0 version: 5.11.2 + zod: + specifier: ^3.25.76 + version: 3.25.76 + zod-to-json-schema: + specifier: ^3.24.1 + version: 3.25.2(zod@3.25.76) devDependencies: '@flakemetry/eslint-config': specifier: workspace:*