From 936a091c5646e1903a59dca7040c8a50f86ced36 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:33:16 +0200 Subject: [PATCH 1/9] chore(lore-0157): activate task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branched from `docs/0156_self-service-auth-assumptions` rather than `develop`: PR #187 rewrites this task's quota-reset section — the calendar alignment of the monthly quota is unverified, so the single-date rework boundary is our design intent until 0171 #7 measures it. Building from `develop` would implement the superseded spec. Committed here instead of pushed straight to `develop` (the usual promote-task path) because PR #187 modifies the same file in `backlog/`; moving it on `develop` would leave that PR with a modify/delete conflict. --- ...FEATURE_default-key-limits-1rps-monthly-quota.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) rename lore/1-tasks/{backlog => active}/0157_FEATURE_default-key-limits-1rps-monthly-quota.md (95%) diff --git a/lore/1-tasks/backlog/0157_FEATURE_default-key-limits-1rps-monthly-quota.md b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md similarity index 95% rename from lore/1-tasks/backlog/0157_FEATURE_default-key-limits-1rps-monthly-quota.md rename to lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md index df739692..6db24b95 100644 --- a/lore/1-tasks/backlog/0157_FEATURE_default-key-limits-1rps-monthly-quota.md +++ b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md @@ -2,7 +2,7 @@ id: "0157" title: "Default key limits: 1 req/s + monthly quota, not the design doc's 100 req/s" type: FEATURE -status: backlog +status: active related_adr: ["0008", "0010"] related_tasks: ["0121", "0156", "0158", "0160", "0163", "0171"] tags: [layer-infra, priority-high, effort-medium, milestone-M3, epic-self-service-onboarding, api-gateway, usage-plan, throttling, cost] @@ -36,6 +36,17 @@ history: for gateway/stage settings. Three renames alongside the three new fields, done here because this is the task that introduces the second tier and makes `apiKeyRateLimit` actively misleading. + - date: 2026-08-10 + status: active + who: akot + note: > + Activated. Branched from `docs/0156_self-service-auth-assumptions` rather + than `develop` because [[0156]] (PR #187, in review) rewrites this task's + quota-reset section: the calendar alignment is unverified, so the + single-date property is design intent until [[0171]] #7 measures it. + Building from `develop` would implement the superseded spec. The status + move is committed on the feature branch, not pushed to `develop`, to + avoid a modify/delete conflict against PR #187 which edits this file. --- # Default key limits: 1 req/s + monthly quota From d4ad060b59359b5f1c82d16b8e03a584a6627986 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:18:31 +0200 Subject: [PATCH 2/9] feat(lore-0157)!: cut default key limits to 1 req/s + 100k/month MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deployed usage plan gave a key 100 req/s and 10 000/day — a shape designed for one hand-issued partner key, not a default for keys anybody can mint by signing in with Discord. Self-issued keys now get 1 req/s sustained, burst 5, and 100 000 requests per calendar month. One plan, renamed in place, rather than a second plan beside it. Every property of AWS::ApiGateway::UsagePlan is Update-requires-No-interruption, so the rename carries no replacement risk, and the partner key served 14 requests in 30 days — all our own verification curls — so there was no working partner to protect. The plan's logical id (ApiUsagePlanDBBE8AB1) is unchanged, so CloudFormation updates the deployed plan in place rather than creating a second one. Config fields renamed so scope comes first, then property: apiKeyRateLimit and apiKeyBurstLimit become selfServicePlanRateLimit and selfServicePlanBurstLimit, apiGatewayPartnerDailyQuota becomes selfServicePlanMonthlyQuota. apiGateway* is now reserved for gateway- and stage-level settings. CloudFormation sees only the values, so nothing in AWS is touched by the rename. Validation drops the old apiGatewayThrottleRate >= apiKeyRateLimit check (justified as letting a key reach its per-key SLA — self-service carries no SLA) for a proportionality ratio: a plan limit may be at most one tenth of the stage's default per-method limit, applied to rate and burst. At 200/400 that permits 1/5 and rejects 100. Its job is narrow — stop the design doc's 100 req/s being reinstated by typo. The plan id is published as /prices/{env}/pricing-api-free-plan-id because the backend that will issue keys (0160) lives in ComputeStack, a dependency of ApiGatewayStack, so it cannot read the plan object without closing the cycle. BREAKING CHANGE: the API key construct id changes from PartnerApiKey to SelfServiceApiKey, which changes its logical id — CloudFormation removes prices-production-partner-key and adds prices-production-selfservice-key with a new value. On deploy every holder of the old key is cut off. This is deliberate: the old key's value was exposed by get-usage-plan-keys while checking its usage, so it needed rotating regardless. --- infra/README.md | 1 + infra/envs/production.json | 6 +- infra/src/lib/stacks/api-gateway-stack.ts | 97 ++++++++++++---- infra/src/lib/stacks/compute-stack.ts | 6 +- infra/src/lib/types.ts | 130 +++++++++++++++++----- 5 files changed, 184 insertions(+), 56 deletions(-) diff --git a/infra/README.md b/infra/README.md index ac86152c..89abb8cb 100644 --- a/infra/README.md +++ b/infra/README.md @@ -147,6 +147,7 @@ namespace: | `/prices/{env}/mtls-key-secret-arn` | Secrets Manager ARN holding mTLS client key PEM | task 0052 Lambdas | | `/prices/{env}/ledger-processor-lambda-arn` | Live ingest Lambda ARN (for BE SNS subscription) | BE task 0050 / task 0038 | | `/prices/{env}/api-gateway-id` | REST API ID (for downstream domain wiring) | task 0040 | +| `/prices/{env}/pricing-api-free-plan-id` | Usage plan ID for key issuance + `GetUsage` | task 0160 | **Boundary rule:** the deploy role's IAM scope enforces this — it can `Get` both namespaces but `Put`/`Delete` only under `/prices/*`. diff --git a/infra/envs/production.json b/infra/envs/production.json index 4564a9a0..e4565bc0 100644 --- a/infra/envs/production.json +++ b/infra/envs/production.json @@ -3,9 +3,9 @@ "awsRegion": "eu-central-1", "apiGatewayThrottleRate": 200, "apiGatewayThrottleBurst": 400, - "apiGatewayPartnerDailyQuota": 10000, - "apiKeyRateLimit": 100, - "apiKeyBurstLimit": 200, + "selfServicePlanRateLimit": 1, + "selfServicePlanBurstLimit": 5, + "selfServicePlanMonthlyQuota": 100000, "apiGatewayCacheEnabled": true, "apiBaseUrl": "https://02mabge71l.execute-api.eu-central-1.amazonaws.com/production", "apiHandler": { diff --git a/infra/src/lib/stacks/api-gateway-stack.ts b/infra/src/lib/stacks/api-gateway-stack.ts index 61b5e999..a54e0348 100644 --- a/infra/src/lib/stacks/api-gateway-stack.ts +++ b/infra/src/lib/stacks/api-gateway-stack.ts @@ -55,16 +55,23 @@ const CACHE_CLUSTER_SIZE = '0.5'; * Method-level throttle for `GET /api-docs-json`. * * Every other Lambda-backed route is `apiKeyRequired: true` and therefore - * carries two limits from the usage plan: the per-key rate and the daily quota. - * This one is anonymous by design, so it has neither — its only limiter would - * otherwise be the stage-wide bucket it SHARES with paying partners. Throttling - * is evaluated before the cache, so an anonymous loop on the documentation route - * draws that bucket down and a partner inside their contracted rate starts - * seeing 429s from a route they never called. + * carries two limits from the usage plan: the per-key rate and the monthly quota. + * This one is anonymous by design, so it has neither — without an entry here it + * would fall back to the default method entry (`resourcePath: '/*'`), which is + * `apiGatewayThrottleRate` (200 req/s). That is a lot of unauthenticated traffic + * to leave available on a route nobody has to hold a key to call. + * + * What this buys is COST CONTROL ON THIS ROUTE, not protection of the others. + * Per-API per-stage limits are applied per method — each method gets its own + * bucket from that default — so an anonymous loop here cannot draw down + * `/v1/...` and cannot make a key holder see 429s on a route they never called. + * It can only exhaust its own bucket. The exposure is the bill: API Gateway + * charges per request, and with the cache off every one is also a billed Lambda + * invocation. * * Sized for what the route is: a static ~40 KB document that a reader fetches - * occasionally and that is cached for an hour at the edge. 10 req/s aggregate is - * far above any legitimate use and 5% of the stage ceiling. + * occasionally and that is cached for an hour at the edge. 10 req/s is far above + * any legitimate use and a twentieth of what the route would otherwise get. * * A local constant rather than a config key because it is a property of this * route's shape (anonymous, cached, static), not of an environment — unlike @@ -82,7 +89,8 @@ const API_DOCS_THROTTLE = { rate: 10, burst: 20 } as const; * prefix of its own, so there is no `/v1/v1` double-prefix. * * - **Auth / rate limit**: data routes set `apiKeyRequired: true`; the UsagePlan - * enforces the §2.1/§7 per-key 100 req/s (`apiKeyRateLimit`) + a daily quota. + * enforces the self-service per-key rate (`selfServicePlanRateLimit`) + a + * monthly quota (task 0157, overriding the design doc's §2.1/§7 100 req/s). * `GET /health` stays a keyless mock (cheapest liveness probe), and * `GET /api-docs-json` is a keyless proxy to the handler (task 0124 — public * documentation). @@ -247,11 +255,18 @@ export class ApiGatewayStack extends cdk.Stack { const cfnStage = this.api.deploymentStage.node .defaultChild as apigateway.CfnStage; - // Assigning `methodSettings` wholesale REPLACES the `/*/*` entry CDK + // Assigning `methodSettings` wholesale REPLACES the default entry CDK // renders from `deployOptions.throttlingRateLimit/Burst`, which would drop - // the stage-wide throttle (only the per-key usage-plan limit would remain). - // Re-declare it here so the §2.1 aggregate stage ceiling survives. - const stageWideThrottle = { + // it entirely (only the per-key usage-plan limit would remain). Re-declare + // it here so the §2.1 figure survives. + // + // Despite the name, this is NOT an aggregate ceiling across the stage: a + // per-API per-stage limit is applied per method, so this grants every method + // its OWN bucket of `apiGatewayThrottleRate`. It bounds what any one route + // can draw, not what the stage can draw in total. The only genuinely shared + // pool above the usage plan is the account-level limit (10 000 RPS / 5 000 + // burst in eu-central-1). + const defaultMethodThrottle = { resourcePath: '/*', httpMethod: '*', throttlingRateLimit: config.apiGatewayThrottleRate, @@ -279,7 +294,7 @@ export class ApiGatewayStack extends cdk.Stack { if (cacheEnabled) { cfnStage.methodSettings = [ - stageWideThrottle, + defaultMethodThrottle, apiDocsSettings, { resourcePath: '/v1/assets', @@ -329,27 +344,53 @@ export class ApiGatewayStack extends cdk.Stack { // No cache cluster, so no TTLs to declare — but the throttles still // apply, and this is the configuration in which the anonymous route is // most expensive to leave unbounded. - cfnStage.methodSettings = [stageWideThrottle, apiDocsSettings]; + cfnStage.methodSettings = [defaultMethodThrottle, apiDocsSettings]; } // --------------------------------------------------------------- - // UsagePlan + API key — per-key 100 req/s (§2.1/§7) + daily quota. + // UsagePlan + API key — self-service tier (task 0157). + // + // One plan, because a key belongs to exactly one plan per stage and + // self-service is the default (and currently only) way to hold a key. + // Higher limits are a manual, out-of-band arrangement made by hand in the + // console — see docs/runbooks/manual-api-key-tier.md. + // + // The construct id stays `UsagePlan` so this updates the deployed plan in + // place rather than creating a second one: every property of + // AWS::ApiGateway::UsagePlan, including UsagePlanName, is "no interruption". // --------------------------------------------------------------- const usagePlan = this.api.addUsagePlan('UsagePlan', { - name: `prices-${config.envName}-partner-plan`, + name: `pricing-api-free-${config.envName}`, throttle: { - rateLimit: config.apiKeyRateLimit, - burstLimit: config.apiKeyBurstLimit, + rateLimit: config.selfServicePlanRateLimit, + burstLimit: config.selfServicePlanBurstLimit, }, quota: { - limit: config.apiGatewayPartnerDailyQuota, - period: apigateway.Period.DAY, + limit: config.selfServicePlanMonthlyQuota, + period: apigateway.Period.MONTH, }, }); usagePlan.addApiStage({ stage: this.api.deploymentStage }); - const apiKey = this.api.addApiKey('PartnerApiKey', { - apiKeyName: `prices-${config.envName}-partner-key`, + // Two separate lines here can rotate this key, by two different mechanisms: + // + // 1. Changing the CONSTRUCT ID changes the logical id, so CloudFormation sees + // a removal and an unrelated addition. That is what task 0157 did + // (`PartnerApiKey` -> `SelfServiceApiKey`). + // 2. Changing `apiKeyName` alone is a Replacement — AWS::ApiGateway::ApiKey + // .Name is "update requires replacement". + // + // The distinction is bookkeeping, not safety: CloudFormation "usually creates + // the replacement resource first, changes references ... and then deletes the + // old resource", so under BOTH paths the old key stays valid for the whole + // update, dies only in the post-success cleanup phase, and survives a + // mid-update rollback untouched. + // + // What matters is the part that is the same either way: the key gets a new + // value and every holder is cut off. Deliberate here; touch neither line + // casually. + const apiKey = this.api.addApiKey('SelfServiceApiKey', { + apiKeyName: `prices-${config.envName}-selfservice-key`, }); usagePlan.addApiKey(apiKey); @@ -359,6 +400,16 @@ export class ApiGatewayStack extends cdk.Stack { description: `REST API ID for prices-${config.envName}-api`, }); + // The onboarding backend (task 0160) issues keys and reads per-key usage, + // both of which need the plan id. It lives in ComputeStack, which this stack + // depends on, so it cannot read the plan object without closing the cycle — + // same shape as the apiBaseUrl problem in task 0124. Publish via SSM instead. + new ssm.StringParameter(this, 'PricingApiFreePlanIdParam', { + parameterName: `/prices/${config.envName}/pricing-api-free-plan-id`, + stringValue: usagePlan.usagePlanId, + description: `Usage plan ID for pricing-api-free-${config.envName} (key issuance + GetUsage)`, + }); + new cdk.CfnOutput(this, 'ApiUrl', { value: this.api.url, description: `Invoke URL for prices-${config.envName}-api stage`, diff --git a/infra/src/lib/stacks/compute-stack.ts b/infra/src/lib/stacks/compute-stack.ts index c896d456..732a8df4 100644 --- a/infra/src/lib/stacks/compute-stack.ts +++ b/infra/src/lib/stacks/compute-stack.ts @@ -443,8 +443,10 @@ export class ComputeStack extends cdk.Stack { // The single axum api-handler (ADR 0008). Reads as `prices_reader` over // mTLS; reuses the same `chDomain` SSM value + secrets extension layer as // the ledger processor. No `API_KEYS` env → the in-app key gate stays - // DISARMED; per-key 100 req/s is enforced at the API Gateway usage plan - // (ADR 0008). `reservedConcurrentExecutions` is the optional SLO escape + // DISARMED; the per-key rate and monthly quota are enforced at the API + // Gateway usage plan (ADR 0008; limits set by task 0157 — + // `selfServicePlanRateLimit` / `selfServicePlanMonthlyQuota`, not the design + // doc's 100 req/s). `reservedConcurrentExecutions` is the optional SLO escape // hatch (only set when configured). API Gateway grants invoke via the // integration's resource policy (no role-cycle, unlike the SQS ESM above). const apiHandler = config.apiHandler; diff --git a/infra/src/lib/types.ts b/infra/src/lib/types.ts index 7f879d30..db9b00a1 100644 --- a/infra/src/lib/types.ts +++ b/infra/src/lib/types.ts @@ -35,19 +35,41 @@ export interface EnvironmentConfig { // API Gateway (consumed by ApiGatewayStack) - /** Stage-wide sustained requests per second before API Gateway returns 429. */ + /** + * Default per-method sustained requests/second on the stage before API Gateway + * returns 429. + * + * NOT an aggregate across the stage: a per-API per-stage limit is applied per + * method, so this value is granted to each method separately. Ten methods at + * 200 is ten independent buckets of 200, not 200 shared ten ways. + */ readonly apiGatewayThrottleRate: number; - /** Stage-wide maximum concurrent requests in a short burst above the rate. */ + /** Default per-method token-bucket capacity above the rate (same scope). */ readonly apiGatewayThrottleBurst: number; - /** Daily request quota for API-key holders (UsagePlan quota.limit). */ - readonly apiGatewayPartnerDailyQuota: number; /** - * Per-API-key sustained requests/second (UsagePlan throttle). The §2.1 / §7 - * contract is 100 req/s per key; this is the value enforced per key holder. + * Per-key sustained requests/second on the self-service usage plan. + * + * The design doc's §2.1 / §7 figure was 100 req/s, sized for a key we hand + * out deliberately. Task 0157 overrides it: a key anybody can mint by signing + * in must not be able to consume the sustained load the whole system is + * load-tested against. + */ + readonly selfServicePlanRateLimit: number; + /** + * Token-bucket capacity for the self-service plan. Refill keeps the sustained + * rate at `selfServicePlanRateLimit`; burst only lets the allowance be spent + * unevenly — enough that the quickstart's parallel example queries don't 429. */ - readonly apiKeyRateLimit: number; - /** Per-API-key burst limit (UsagePlan throttle). */ - readonly apiKeyBurstLimit: number; + readonly selfServicePlanBurstLimit: number; + /** + * Monthly request quota for the self-service plan (UsagePlan quota.limit). + * + * The operative limit a caller actually meets: at the per-second rate a key + * could produce ~2.6M requests/month, so the quota binds ~26x harder than the + * throttle. The period is in the name because a usage plan carries exactly one + * quota — encoding it makes the unit impossible to misread. + */ + readonly selfServicePlanMonthlyQuota: number; /** * Whether the API Gateway stage response cache (0.5 GB) is enabled. Per-route * TTLs are fixed in `ApiGatewayStack` per §2.1. @@ -264,8 +286,12 @@ export function validateConfig(config: EnvironmentConfig): void { `apiGatewayThrottleRate must be a positive integer, got: ${config.apiGatewayThrottleRate}`, ); } + // `< 1` for the same reason as the self-service burst check below: errors are + // accumulated, so without it an invalid rate of -5 lets a burst of -3 through + // unreported (-3 >= -5). if ( !Number.isInteger(config.apiGatewayThrottleBurst) || + config.apiGatewayThrottleBurst < 1 || config.apiGatewayThrottleBurst < config.apiGatewayThrottleRate ) { errors.push( @@ -273,24 +299,31 @@ export function validateConfig(config: EnvironmentConfig): void { ); } if ( - !Number.isInteger(config.apiGatewayPartnerDailyQuota) || - config.apiGatewayPartnerDailyQuota < 1 + !Number.isInteger(config.selfServicePlanRateLimit) || + config.selfServicePlanRateLimit < 1 ) { errors.push( - `apiGatewayPartnerDailyQuota must be a positive integer, got: ${config.apiGatewayPartnerDailyQuota}`, + `selfServicePlanRateLimit must be a positive integer, got: ${config.selfServicePlanRateLimit}`, ); } - if (!Number.isInteger(config.apiKeyRateLimit) || config.apiKeyRateLimit < 1) { + // The `< 1` test is not redundant with the rate check below it. Errors are + // accumulated, not short-circuited, so with an invalid rate of -10 a burst of + // -5 would pass `burst < rate` and go unreported until the rate was fixed. + if ( + !Number.isInteger(config.selfServicePlanBurstLimit) || + config.selfServicePlanBurstLimit < 1 || + config.selfServicePlanBurstLimit < config.selfServicePlanRateLimit + ) { errors.push( - `apiKeyRateLimit must be a positive integer, got: ${config.apiKeyRateLimit}`, + `selfServicePlanBurstLimit must be a positive integer >= selfServicePlanRateLimit (${config.selfServicePlanRateLimit}), got: ${config.selfServicePlanBurstLimit}`, ); } if ( - !Number.isInteger(config.apiKeyBurstLimit) || - config.apiKeyBurstLimit < config.apiKeyRateLimit + !Number.isInteger(config.selfServicePlanMonthlyQuota) || + config.selfServicePlanMonthlyQuota < 1 ) { errors.push( - `apiKeyBurstLimit must be a positive integer >= apiKeyRateLimit (${config.apiKeyRateLimit}), got: ${config.apiKeyBurstLimit}`, + `selfServicePlanMonthlyQuota must be a positive integer, got: ${config.selfServicePlanMonthlyQuota}`, ); } if (typeof config.apiGatewayCacheEnabled !== 'boolean') { @@ -329,17 +362,58 @@ export function validateConfig(config: EnvironmentConfig): void { } } - // The stage-wide throttle is a hard ceiling across ALL keys, so it must be at - // least the advertised per-key rate — otherwise a single key can never reach - // its SLA and compliant traffic gets spurious 429s. - if ( - Number.isInteger(config.apiGatewayThrottleRate) && - Number.isInteger(config.apiKeyRateLimit) && - config.apiGatewayThrottleRate < config.apiKeyRateLimit - ) { - errors.push( - `apiGatewayThrottleRate (${config.apiGatewayThrottleRate}) must be >= apiKeyRateLimit (${config.apiKeyRateLimit}) so a single key can reach its per-key SLA`, - ); + // A self-issued key must not be sized anywhere near what the stage hands a + // single method — that is the entire reason task 0157 exists. Cap the plan at + // one tenth of the stage default, for both rate and burst. + // + // Read the ratio for what it is: a proportionality guard, NOT a capacity + // calculation. A per-method default is not a shared pool that ten keys "fit + // under" — every method gets its own bucket, so the plan limit and the method + // default never compete for the same tokens. The number 10 is a judgement + // call, and its real job is to stop the design doc's 100 req/s being + // reinstated by typo: at a default of 200, a rate of 1 passes and 100 does + // not. A bare `<=` against the default would not catch it (200 >= 100). + // + // The only genuinely shared ceiling above the usage plan is the account limit + // (10 000 RPS / 5 000 burst in eu-central-1), which nothing here approaches. + const MAX_PLAN_SHARE_OF_STAGE_DEFAULT = 10; + const planVsStage: ReadonlyArray = + [ + [ + 'selfServicePlanRateLimit', + config.selfServicePlanRateLimit, + 'apiGatewayThrottleRate', + config.apiGatewayThrottleRate, + ], + [ + 'selfServicePlanBurstLimit', + config.selfServicePlanBurstLimit, + 'apiGatewayThrottleBurst', + config.apiGatewayThrottleBurst, + ], + ]; + // Both `>= 1` guards keep this from piling a derived error on top of a primary + // one: with a stage rate of -200 the checks above already report it, and + // "the maximum is -20" would add noise, not information. + // + // Note the implied floor this creates: since a plan limit must be >= 1, a + // stage default below MAX_PLAN_SHARE_OF_STAGE_DEFAULT makes the config + // unsatisfiable. At 200 that is nowhere near binding, but anyone dialling the + // stage throttle down has to stay at or above 10. + for (const [planField, planValue, stageField, stageValue] of planVsStage) { + if ( + Number.isInteger(stageValue) && + stageValue >= 1 && + Number.isInteger(planValue) && + planValue >= 1 && + planValue * MAX_PLAN_SHARE_OF_STAGE_DEFAULT > stageValue + ) { + errors.push( + `${planField} (${planValue}) exceeds one ${MAX_PLAN_SHARE_OF_STAGE_DEFAULT}th of ${stageField} (${stageValue}): ` + + `a self-service key must not be sized within an order of magnitude of the stage's default per-method limit, ` + + `so the maximum is ${Math.floor(stageValue / MAX_PLAN_SHARE_OF_STAGE_DEFAULT)}`, + ); + } } const api = config.apiHandler; From 84dc26776903d52d987575666d5859495bb31190 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:18:47 +0200 Subject: [PATCH 3/9] docs(lore-0157): retag 429s as monthly quota; add manual-tier runbook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seven utoipa 429 descriptions are a consumer surface outside the repo — they ship in the OpenAPI document served at GET /api-docs-json — so "Rate limit or daily quota exceeded" was a public statement the deploy makes false. Same for the auth and config doc comments that named 100 req/s as the gateway-side limit. docs/runbooks/manual-api-key-tier.md is the procedure for the tier above self-service: which plan a hand-made key attaches to, that payment is handled by bank transfer outside the product, and a registry table for what was issued. A hand-made plan never shows up in cdk diff and nobody reviews it, so recording the resource is a required step rather than an afterthought. The runbook also pins the credential-handling rule the usage check ran into: read keys with get-api-keys without --include-values, never get-usage-plan-keys, which returns the plaintext value with no flag to suppress it. The loadtest README gains the constraint 0121 now has to work around: after this change no key in the account can sustain 100 req/s, so a run against pricing-api-free-production measures our own throttle and spends a third of the month's allowance. 0121 must provision its own plan. --- .../database-schema-overview.md | 14 +- docs/prices-api-general-overview.md | 2 +- docs/runbooks/manual-api-key-tier.md | 252 ++++++++++++++++++ packages/prices-api/loadtest/README.md | 14 +- packages/prices-api/src/assets/handlers.rs | 8 +- packages/prices-api/src/auth/mod.rs | 7 +- packages/prices-api/src/backfill/handlers.rs | 2 +- packages/prices-api/src/batch/handlers.rs | 2 +- packages/prices-api/src/config.rs | 5 +- packages/prices-api/src/oracles/handlers.rs | 2 +- 10 files changed, 287 insertions(+), 21 deletions(-) create mode 100644 docs/runbooks/manual-api-key-tier.md diff --git a/docs/database-schema/database-schema-overview.md b/docs/database-schema/database-schema-overview.md index 25cdc3f7..82e003de 100644 --- a/docs/database-schema/database-schema-overview.md +++ b/docs/database-schema/database-schema-overview.md @@ -1811,13 +1811,13 @@ instance on a single Hetzner box behind Caddy:443). Prices-api joins as a second tenant via its own `prices` database, isolated by ClickHouse's native multi-tenant primitives (database, user, quota, profile). -| Metric | Value | Source | -| ---------------------------- | --------------------------------------------------------------------------- | -------------------------- | -| Prices-api storage footprint | **~3.5-6 GB/year** (realistic, retention-amortised) | Tasks 0060 + 0063 measured | -| Average per-ledger storage | **~1.9-3.7 KB/ledger** (activity-dependent, ~2× spread) | Tasks 0060 + 0063 measured | -| Strongest size lever | Retention-cap `_1h`/`_4h` → bounds DB at ~9 GB @ 10yr (vs ~43 GB unbounded) | Task 0060 measured | -| Write rate | ~1 INSERT per ledger (~12k/day per env at mainnet cadence) | §6.1 | -| Read rate | API-Gateway-throttled ≤100 req/s per key, cached at gateway | §8.2 | +| Metric | Value | Source | +| ---------------------------- | --------------------------------------------------------------------------------- | -------------------------- | +| Prices-api storage footprint | **~3.5-6 GB/year** (realistic, retention-amortised) | Tasks 0060 + 0063 measured | +| Average per-ledger storage | **~1.9-3.7 KB/ledger** (activity-dependent, ~2× spread) | Tasks 0060 + 0063 measured | +| Strongest size lever | Retention-cap `_1h`/`_4h` → bounds DB at ~9 GB @ 10yr (vs ~43 GB unbounded) | Task 0060 measured | +| Write rate | ~1 INSERT per ledger (~12k/day per env at mainnet cadence) | §6.1 | +| Read rate | ≤1 req/s per key (task 0157); ≤200 req/s per method stage-wide, cached at gateway | §8.2 | > **Sizing superseded (2026-06-19).** The original ~74 B/ledger / ~0.45 GB/yr > figure was the task-0046 _per-event estimate_. Three ground-truth backfill diff --git a/docs/prices-api-general-overview.md b/docs/prices-api-general-overview.md index 34b6b0af..d314e036 100644 --- a/docs/prices-api-general-overview.md +++ b/docs/prices-api-general-overview.md @@ -161,7 +161,7 @@ to their own infrastructure at any time if needed. | **Lambda — Asset Discovery** | Asset registry | EventBridge rate(1 hour). Detects new SEP-41 contract deployments and classic asset issuances; UPSERTs into `prices.assets` | | **Lambda — Cleanup Worker** | Data retention | EventBridge cron(02:00 UTC daily). `ALTER TABLE … DROP PARTITION` on old monthly partitions of each per-granularity OHLCV table | | **Lambda — API handlers** | Public API | Individual functions per route group. Rust / axum via `lambda_runtime`, 256–512 MB, 15s timeout. No VPC; outbound HTTPS-mTLS to Caddy:443 | -| **API Gateway** | Public API entry point | REST API, usage plans, API key auth, rate limiting (100 req/s per key), request validation. Built-in response cache (0.5 GB) with per-endpoint TTLs | +| **API Gateway** | Public API entry point | REST API, usage plans, API key auth, rate limiting (1 req/s + 100 000 req/month per self-service key — task 0157 supersedes the 100 req/s in §2.1/§7), request validation. Built-in response cache (0.5 GB) with per-endpoint TTLs | | **EventBridge Scheduler** | Scheduled triggers | Cron/rate rules for all periodic Lambda workers | | **Secrets Manager** | Credentials & mTLS material | Per-env client `{cert,key,ca}` for Caddy:443 mTLS (single JSON bundle secret per identity, named by `MTLS_SECRET_NAME`); Soroswap/Aquarius API keys; oracle contract address | | **CloudWatch + X-Ray** | Observability | API latency, error rates, ingestion lag, Lambda duration/concurrency, backfill progress; mTLS cert NotAfter alarm | diff --git a/docs/runbooks/manual-api-key-tier.md b/docs/runbooks/manual-api-key-tier.md new file mode 100644 index 00000000..b5bc4b0b --- /dev/null +++ b/docs/runbooks/manual-api-key-tier.md @@ -0,0 +1,252 @@ +# Runbook: issuing a manual higher-tier API key + +**When:** someone needs more than the self-service limits and has agreed terms with +us out of band. There is no self-serve upgrade path and no in-app billing — by +design (task 0157, epic _Self-Service Onboarding_). + +**Who:** anyone with `AdministratorAccess` on the shared AWS account +(`750702271865`, `eu-central-1`). + +--- + +## What the default tier is + +Every key issued through the portal lands on the CDK-managed plan: + +| | | +| ----- | ----------------------------- | +| Plan | `pricing-api-free-production` | +| Rate | 1 req/s sustained | +| Burst | 5 | +| Quota | 100 000 requests/month | + +That plan is defined in `infra/src/lib/stacks/api-gateway-stack.ts` and its values +come from `infra/envs/production.json`. **Do not hand-edit it in the console** — +the next deploy reverts you, and the change is invisible in review. + +## Why a manual tier is a separate plan + +A key belongs to exactly one usage plan per stage. Raising limits for one holder +therefore means a second plan, not a second set of numbers on the existing one. + +The manual plan is deliberately **not** in CDK. The epic settles this as +"a fully manual, out-of-band process for now" with "nothing to build here". +Putting it in CDK would mean carrying a resource with no holders and inventing +config fields for numbers negotiated case by case. Revisit once there is more +than one such customer. + +The trade-off is real and worth stating: a hand-made plan is **drift**. It does +not appear in `cdk diff`, nobody reviews it, and it survives only as long as +someone remembers it exists. That is why step 5 below is not optional. + +--- + +## Issue the key + +Set the negotiated limits first — these are examples, not defaults: + +```bash +export AWS_PROFILE= +export AWS_REGION=eu-central-1 +export CUSTOMER=acme # lowercase, no spaces +export RATE=20 # req/s sustained +export BURST=40 # token bucket capacity, conventionally 2x rate +export QUOTA=5000000 # requests per month +``` + +**1. Create the plan and attach the production stage.** + +```bash +API_ID=$(aws ssm get-parameter --name /prices/production/api-gateway-id \ + --query 'Parameter.Value' --output text) + +PLAN_ID=$(aws apigateway create-usage-plan \ + --name "prices-production-${CUSTOMER}-plan" \ + --description "Manual tier for ${CUSTOMER}; terms agreed out of band" \ + --api-stages "apiId=${API_ID},stage=production" \ + --throttle "rateLimit=${RATE},burstLimit=${BURST}" \ + --quota "limit=${QUOTA},period=MONTH,offset=0" \ + --tags "Project=stellar-prices-api,Environment=production,ManagedBy=manual,Customer=${CUSTOMER}" \ + --query 'id' --output text) + +echo "plan: ${PLAN_ID}" +``` + +`ManagedBy=manual` is what tells the next person this resource is not CDK's. + +**2. Create the key.** Note that `create-api-key` returns the secret `value` in +its response. `--query 'id'` filters client-side, so it never reaches your +terminal — but it did cross the wire and it lands in CLI debug output if you ever +rerun this with `--debug`. Do not. + +```bash +KEY_ID=$(aws apigateway create-api-key \ + --name "prices-production-${CUSTOMER}-key" \ + --description "Manual tier for ${CUSTOMER}" \ + --enabled \ + --tags "Project=stellar-prices-api,ManagedBy=manual,Customer=${CUSTOMER}" \ + --query 'id' --output text) + +aws apigateway create-usage-plan-key \ + --usage-plan-id "${PLAN_ID}" \ + --key-id "${KEY_ID}" \ + --key-type API_KEY +``` + +**3. Check the stage ceiling still holds.** + +The stage's default method throttle applies to every caller of a given method — +it is a per-method limit, not one pool shared across the stage — and the more +restrictive of it and the plan wins. Read the current value from +`infra/envs/production.json` (`apiGatewayThrottleRate` / `apiGatewayThrottleBurst`) +rather than trusting a number written here. + +A manual tier above that ceiling cannot be delivered by creating a plan alone — +`apiGatewayThrottleRate` has to go up too, which is a CDK change and a capacity +decision, not a runbook step. + +**4. Read the key value out and hand it over.** + +```bash +aws apigateway get-api-key --api-key "${KEY_ID}" --include-value \ + --query 'value' --output text +``` + +Send it through something that does not retain plaintext indefinitely. Do not +paste it into a ticket, a shared doc, or a chat channel with history — treat it +the way you would a password. If it does leak, rotation is step 7. + +`--include-value` is the singular form's flag and it is opt-in; the plural +`get-api-keys` takes `--include-values` and defaults to off. Use the plural +without the flag whenever you only need to find a key, not read it. + +**5. Write it down.** Add a row to the table at the bottom of this file and commit. +This file is the only record that the resource exists. + +**6. Payment** is a normal bank transfer arranged outside the product. Nothing in +the system tracks it, and nothing enforces it — if a customer stops paying, the +key has to be disabled by hand ("Suspend without destroying", below). + +--- + +## Change or revoke a manual key + +Everything below runs weeks or months after the key was issued, in a shell that +has none of step 1's variables. `PLAN_ID` and `KEY_ID` come from the registry +table at the bottom of this file — that is what it is for. If the row is missing +or stale, recover them by name: + +```bash +export CUSTOMER=acme +PLAN_ID=$(aws apigateway get-usage-plans \ + --query "items[?name=='prices-production-${CUSTOMER}-plan'].id | [0]" --output text) +KEY_ID=$(aws apigateway get-api-keys --name-query "prices-production-${CUSTOMER}-key" \ + --query 'items[0].id' --output text) +``` + +`get-api-keys` without `--include-values` does not return the secret. Then fix +the registry row. + +**Adjust limits** — safe, no key rotation: + +```bash +aws apigateway update-usage-plan --usage-plan-id "${PLAN_ID}" --patch-operations \ + op=replace,path=/throttle/rateLimit,value=30 \ + op=replace,path=/quota/limit,value=8000000 +``` + +**Suspend without destroying** (non-payment, suspected leak, investigation): + +```bash +aws apigateway update-api-key --api-key "${KEY_ID}" --patch-operations \ + op=replace,path=/enabled,value=false +``` + +Whether disabling preserves, freezes or zeroes the usage counters is +**not documented by AWS** and is tracked as task 0171 #8. Do not assume the +quota is preserved across a disable/enable cycle. + +**7. Rotate** — create a new key (step 2), attach it, confirm the customer is +using it, then delete the old one. Capture the outgoing key's id **before** you +create the replacement, or you will be picking it out of a list by name: + +```bash +OLD_KEY_ID=$(aws apigateway get-api-keys \ + --name-query "prices-production-${CUSTOMER}-key" \ + --query 'items[0].id' --output text) + +# ... run step 2 to create the new key, hand it over, confirm it is in use ... + +aws apigateway delete-api-key --api-key "${OLD_KEY_ID}" +``` + +`get-api-keys` without `--include-values` is the safe way to look a key up: the +plural form defaults to omitting the secret. Do **not** reach for +`get-usage-plan-keys` — it returns the plaintext `value` and has no flag to +suppress it, so listing keys that way puts live secrets in your shell history. + +**Quota does not carry across rotation.** A new key starts its counter at zero, +so a customer rotated mid-month gets a fresh full month's allowance. During the +overlap — old key alive, new key attached — they effectively hold two quotas, +because quota is tracked per `(usage plan, API key)`. Keep the overlap short. + +**Wind down** — detach the stage first, then delete the key and the plan: + +```bash +API_ID=$(aws ssm get-parameter --name /prices/production/api-gateway-id \ + --query 'Parameter.Value' --output text) + +aws apigateway update-usage-plan --usage-plan-id "${PLAN_ID}" \ + --patch-operations op=remove,path=/apiStages,value="${API_ID}:production" + +aws apigateway delete-api-key --api-key "${KEY_ID}" +aws apigateway delete-usage-plan --usage-plan-id "${PLAN_ID}" +``` + +The detach is not optional. A plan that still has an API stage attached cannot be +deleted — API Gateway returns `BadRequestException: Cannot delete Usage Plan +because there are API Stages associated with it`. Step 1 always attaches a stage, +so every plan created by this runbook hits it. The `value` format is +`apiId:stageName`, colon-separated. + +This is **not** in the `DeleteUsagePlan` API docs, which list only generic errors — +which is why it is easy to write the obvious two-line teardown and have it fail. + +Then delete the row from the table below. + +--- + +## Things that will bite you + +- **Never attach a manual key to `pricing-api-free-production`.** It would + silently inherit 1 req/s and a 100 000/month quota. A key belongs to exactly + one usage plan **per stage** — so on this API there is no "also attach it to + the bigger plan". (A key may sit in up to 10 plans overall, across different + stages; that does not help here.) +- **Quotas are best-effort.** AWS: _"Usage plan throttling and quotas are not hard + limits… Don't rely on usage plan quotas or throttling to control costs."_ For a + tier large enough to matter financially, back it with AWS Budgets. +- **A cached response still costs a request.** This half is firm: API Gateway + charges per call received, cache hit or not — the cache is billed separately by + the hour and nowhere described as reducing call charges. Whether the _quota_ is + also decremented before the cache lookup is our inference, not documented; AWS's + throttling order lists usage plan, stage, account and Regional limits and never + mentions the cache. See task 0171. +- **The monthly reset schedule is all but undocumented.** The only statement in + AWS's docs is an example caption — _"creates a usage plan that resets at the + beginning of the month"_ — with no timezone, no instant, and nothing on whether + `MONTH` is calendar-aligned or runs from plan creation. Note also that + `QuotaSettings.offset` is _"the number of requests subtracted from the given + limit in the initial time period"_ — a request count, not a way to shift the + reset day, so it cannot be used to force alignment. Unverified; task 0171 #7. + Do not promise a customer a specific reset date until it is measured. + +--- + +## Issued manual keys + +Keep this current. One row per plan; delete the row when the plan is deleted. + +| Customer | Plan name | Plan ID | Key ID | Limits | Issued | Issued by | +| ------------ | --------- | ------- | ------ | ------ | ------ | --------- | +| _(none yet)_ | | | | | | | diff --git a/packages/prices-api/loadtest/README.md b/packages/prices-api/loadtest/README.md index 9f5d5042..c2e8d74c 100644 --- a/packages/prices-api/loadtest/README.md +++ b/packages/prices-api/loadtest/README.md @@ -14,13 +14,25 @@ real Lambda concurrency + cross-cloud mTLS to ClickHouse all in play): ```sh k6 run packages/prices-api/loadtest/price_load.js \ -e BASE_URL=https://.execute-api..amazonaws.com/production \ - -e API_KEY= \ + -e API_KEY= \ -e ASSET=native ``` Deploy-gated — requires the stack deployed (Phase 4 CDK) and a price row written by the ingest/current-prices path. +> **The key matters, and no key that exists today will do.** Task 0157 caps the +> CDK-managed plan (`pricing-api-free-production`) at **1 req/s with a 100 000/month +> quota**. Running this script against a key on that plan does not measure the +> system — it measures our own throttle, and reports a configuration artefact as +> an SLO result. +> +> A 100 req/s run for 5 minutes is 30 000 requests, so even a generous monthly +> quota is a real constraint if the test is repeated. Provision a dedicated +> throttle-and-quota-headroom plan for the run (see +> `docs/runbooks/manual-api-key-tier.md`), and state in the report which plan the +> key was on. Tracked in task 0121. + ## Approximate run — local server Exercises the handler + ClickHouse path only (no gateway cache, no Lambda cold diff --git a/packages/prices-api/src/assets/handlers.rs b/packages/prices-api/src/assets/handlers.rs index 7377c7b8..44b8d094 100644 --- a/packages/prices-api/src/assets/handlers.rs +++ b/packages/prices-api/src/assets/handlers.rs @@ -43,7 +43,7 @@ const MAX_LIMIT: u32 = 200; (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), (status = 404, description = "No current price for the asset", body = ErrorEnvelope), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), ) )] @@ -80,7 +80,7 @@ pub async fn get_price(State(state): State, Path(raw): Path) - (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), (status = 404, description = "Unknown asset", body = ErrorEnvelope), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), ) )] @@ -154,7 +154,7 @@ pub struct ListParams { (status = 400, description = "Invalid query parameter", body = ErrorEnvelope), (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), ) )] @@ -271,7 +271,7 @@ pub struct OhlcvParams { (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), (status = 404, description = "Unknown asset", body = ErrorEnvelope), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), (status = 503, description = "The requested `base_currency` quote asset is not \ tracked (`quote_unavailable`)", body = ErrorEnvelope), diff --git a/packages/prices-api/src/auth/mod.rs b/packages/prices-api/src/auth/mod.rs index d568a70a..3547f8ff 100644 --- a/packages/prices-api/src/auth/mod.rs +++ b/packages/prices-api/src/auth/mod.rs @@ -2,9 +2,10 @@ //! //! Ported from BE's `api/src/auth`, trimmed to the API-key path (no JWT / //! Turnstile — those are BE's free-tier concern). Keys are compared in constant -//! time. The per-key 100 req/s throttle lives at the API Gateway usage-plan; -//! this gate only enforces *presence of a valid key*, as defense-in-depth and -//! for local/non-gateway runs. +//! time. The per-key rate limit and monthly quota live at the API Gateway +//! usage-plan (sized by task 0157, not the design doc's 100 req/s); this gate +//! only enforces *presence of a valid key*, as defense-in-depth and for +//! local/non-gateway runs. //! //! The gate is **armed only when keys are configured** (`API_KEYS` non-empty), //! so unconfigured local/dev and the early load test run open. `/health` and diff --git a/packages/prices-api/src/backfill/handlers.rs b/packages/prices-api/src/backfill/handlers.rs index 612cdb76..9d76d583 100644 --- a/packages/prices-api/src/backfill/handlers.rs +++ b/packages/prices-api/src/backfill/handlers.rs @@ -19,7 +19,7 @@ use crate::state::AppState; (status = 200, description = "Backfill progress", body = BackfillStatus), (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), ) )] diff --git a/packages/prices-api/src/batch/handlers.rs b/packages/prices-api/src/batch/handlers.rs index c4b502e3..9ddf1a9a 100644 --- a/packages/prices-api/src/batch/handlers.rs +++ b/packages/prices-api/src/batch/handlers.rs @@ -30,7 +30,7 @@ use crate::state::AppState; body = ErrorEnvelope), (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), ) )] diff --git a/packages/prices-api/src/config.rs b/packages/prices-api/src/config.rs index 39ef1a26..246b1ed0 100644 --- a/packages/prices-api/src/config.rs +++ b/packages/prices-api/src/config.rs @@ -16,8 +16,9 @@ pub struct AppConfig { /// Accepted `X-API-Key` values, parsed from comma-separated `API_KEYS`. /// When empty the in-app auth gate is **disarmed** (open) — so local/dev and /// the early Phase 2 load test work before keys are provisioned. The - /// per-key 100 req/s throttle is enforced at the API Gateway usage-plan - /// regardless (ADR 0008). Mirrors BE's deploy-dark gating. + /// per-key rate limit and monthly quota are enforced at the API Gateway + /// usage-plan regardless (ADR 0008; sized by task 0157 — not the design + /// doc's 100 req/s). Mirrors BE's deploy-dark gating. pub api_keys: Vec, } diff --git a/packages/prices-api/src/oracles/handlers.rs b/packages/prices-api/src/oracles/handlers.rs index 98bc702c..07bd3cbe 100644 --- a/packages/prices-api/src/oracles/handlers.rs +++ b/packages/prices-api/src/oracles/handlers.rs @@ -30,7 +30,7 @@ use crate::state::AppState; (status = 401, description = "Missing or invalid `x-api-key`", body = ErrorEnvelope), (status = 403, description = "API key missing, invalid, or not authorized for this API"), (status = 404, description = "Unknown asset", body = ErrorEnvelope), - (status = 429, description = "Rate limit or daily quota exceeded (API Gateway usage plan)"), + (status = 429, description = "Per-key rate limit or monthly quota exceeded (API Gateway usage plan)"), (status = 500, description = "Query or upstream failure (`db_error`)", body = ErrorEnvelope), ) )] From 22abb57fe8a43f3bff509add20775d817461eb6a Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:19:01 +0200 Subject: [PATCH 4/9] docs(lore-0157): record the one-plan outcome and its corrections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The task shipped past its own Summary and the record now says so. Three things the plan asserted turned out wrong and are corrected in place rather than quietly dropped: - Two plans became one, renamed. UsagePlanName is No-interruption, so the replacement risk that justified leaving partner-plan alone did not exist, and nobody was using the partner key anyway. - The stage throttle is per-method, not an aggregate stage pool (DD 5b). Three pieces of reasoning built on the aggregate reading had to be rewritten, and the epic's "10 keys saturate the 1000 req/s global burst" premise joins db.t4g.micro and the $900 figure as premises that do not map onto this deployment. The conclusion is unaffected. - The key rotates via the construct-id change (remove + add), not via ApiKey.Name replacement — corrected against the synthesized template. Also separated two claims that were previously asserted as one fact: a cached response still costs the caller a request is documented for billing, but that the quota decrements before the cache lookup is our inference — AWS's documented throttling order never mentions the cache. 0121 and 0047 pick up the consequence: no key in the account can sustain 100 req/s after this, so the load test has to provision its own plan and say which plan the key was on. --- ...E_default-key-limits-1rps-monthly-quota.md | 322 ++++++++++++++---- ...ghput-verification-on-shared-hetzner-ch.md | 11 +- .../0121_TEST_api-load-test-100rps-report.md | 16 +- 3 files changed, 271 insertions(+), 78 deletions(-) diff --git a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md index 6db24b95..75956718 100644 --- a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md +++ b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md @@ -18,7 +18,8 @@ history: note: > Epic AC 5, verbatim as the task title. Independently shippable ahead of the portal — the limits must exist before any key can be self-issued - against them. Quota fixed at 100 000/month (top of the epic's range) and + against them. Quota fixed at 100 000/month (our choice; the epic's + "50,000–100,000" is an e.g., not a bound) and burst at 5; both confirmed by Adam on 2026-08-06. - date: 2026-08-07 status: backlog @@ -47,6 +48,26 @@ history: Building from `develop` would implement the superseded spec. The status move is committed on the feature branch, not pushed to `develop`, to avoid a modify/delete conflict against PR #187 which edits this file. + - date: 2026-08-10 + status: active + who: akot + note: > + Implemented as ONE usage plan renamed in place, not two — the specified + two-plan design rested on two claims that did not survive checking: + `UsagePlanName` is "no interruption" (no replacement risk), and the partner + key served 14 requests in 30 days, all our own verification curls. The epic + never asked for a second plan. Full reasoning in Design Decisions #3-#6. + Changed: `types.ts`, `api-gateway-stack.ts`, `envs/production.json`, + `infra/README.md` SSM contract, plus a new + `docs/runbooks/manual-api-key-tier.md`. Three config fields renamed to + `selfServicePlan*`; six were planned, the one-plan outcome halved it. + Key rotated in the same change (construct-id change → remove + add, not + `ApiKey.Name` replacement) after its value was exposed by + `get-usage-plan-keys` during the usage check. + Verified by `tsc`, `cdk synth` and `cdk diff` against the live stack: plan + updates in place under logical id `ApiUsagePlanDBBE8AB1`, key removed and + re-added under a new logical id, SSM parameter added. Not yet deployed — + four ACs stay open until then. --- # Default key limits: 1 req/s + monthly quota @@ -63,6 +84,17 @@ The epic is explicit that this is a **rate-limit-only change**: it does not affect the `403`-without-key behaviour or the usage-plan mechanism already agreed. +**As built, this task goes past that framing — deliberately, and the Summary +should not be read as covering it.** Besides the limits it renames the plan to +`pricing-api-free-production`, renames three config fields, publishes a new SSM +parameter, and **rotates the production API key**. The rotation is not epic +compliance: it is scope this task added after the old key's value was exposed by +`get-usage-plan-keys` during the usage check (Design Decision 5). Its +consequence is the one thing in here that touches someone outside the repo — on +deploy, every holder of `prices-production-partner-key` is cut off. Usage over +the preceding 30 days was 14 requests across 4 days, all our own verification +curls, which is why that is acceptable rather than a blocker. + ## Context `api-gateway-stack.ts` creates one `UsagePlan` (`prices--partner-plan`) @@ -93,7 +125,9 @@ conclusion stands on the remaining arguments. - **Default free-tier limit for self-issued keys: 1 req/s per key** (60 req/min) — roughly 2× CoinGecko's free registered tier. - **A monthly request quota alongside the per-second throttle.** The epic gives - a range of 50 000–100 000 calls/month; take **100 000** — ~3 300/day, ~6.7× + "(e.g. 50,000–100,000 calls/month)" — an **illustration, not a range to pick + from**, so the number below is our decision rather than epic compliance, and + nothing in the epic sets a floor of 50 000. Take **100 000** — ~3 300/day, ~6.7× CoinMarketCap's free tier, and a round number the dashboard ([[0160]]) can render as "used / 100 000 this month" without explanation. A per-second limit alone does not stop a key idling at 1 req/s from producing 2.6M @@ -105,19 +139,30 @@ conclusion stands on the remaining arguments. **Follows from the epic, but not stated in it** -- **Two usage plans on the same stage.** The epic requires 1 req/s for - self-issued keys *and* a hand-issued higher tier; a single plan cannot hold - both. So add `prices--selfservice-plan` (1 rps, 100 000/month) and leave - `prices--partner-plan` exactly as it is — same logical id, same name, - same limits — as the manual tier. Renaming it risks a CloudFormation resource - replacement that would invalidate a working partner key. A key belongs to one +- ~~**Two usage plans on the same stage.**~~ **Superseded 2026-08-10 — see + Design Decisions below.** The original reasoning was: the epic requires 1 req/s + for self-issued keys *and* a hand-issued higher tier, a single plan cannot hold + both, so add `prices--selfservice-plan` and leave `prices--partner-plan` + untouched as the manual tier — because renaming it "risks a CloudFormation + resource replacement that would invalidate a working partner key". + + **Both halves of that turned out to be wrong.** `UsagePlanName` is + *Update requires: No interruption* (every property of + `AWS::ApiGateway::UsagePlan` is), so there is no replacement risk; and the + partner key served 14 requests in 30 days — all our own verification curls — so + there was no working partner to invalidate. The epic never asks for a second + plan either; it calls the manual tier "nothing to build here". + + **Implemented instead: one plan, renamed in place.** A key still belongs to one plan per stage, so `(usagePlanId, apiKeyId)` stays unambiguous for `GetUsage`. - **Burst 5.** The epic fixes the sustained rate and says nothing about burst. At burst 1 the quickstart page ([[0163]]) firing two example queries in parallel 429s on a key we issued ourselves, which contradicts epic AC 3. Token-bucket refill keeps the sustained rate at exactly 1 req/s; burst only - allows the allowance to be spent unevenly. 1:5 is the same shape as the - existing 100:200 partner plan and 200:400 stage throttle. + allows the allowance to be spent unevenly. 1:5 is a wider ratio than the + 1:2 the plan carried before (100:200) and than the stage throttle's 200:400 — + deliberately so, because at a sustained rate of 1 a 1:2 bucket refills too + slowly to absorb even a pair of parallel requests. - **Config naming — settled 2026-08-07. Scope first, then property:** `apiGateway*` for anything belonging to the gateway or the stage, `Plan*` for anything belonging to a specific usage plan. @@ -126,43 +171,61 @@ conclusion stands on the remaining arguments. | --- | --- | | `apiGatewayThrottleRate` / `apiGatewayThrottleBurst` | unchanged — stage-level, not a plan | | `apiGatewayCacheEnabled`, `apiBaseUrl` | unchanged | - | `apiKeyRateLimit` | `partnerPlanRateLimit` | - | `apiKeyBurstLimit` | `partnerPlanBurstLimit` | - | `apiGatewayPartnerDailyQuota` | `partnerPlanDailyQuota` | - | — | `selfServicePlanRateLimit` (1) | - | — | `selfServicePlanBurstLimit` (5) | - | — | `selfServicePlanMonthlyQuota` (100000) | - - Three renames, three additions. The reason to do it here rather than later: - `apiKeyRateLimit` reads as a global property of API keys, and the moment a - second plan exists that is simply false — it is one plan's rate. Two fields of - the same kind should look like a pair, and the rule for a third tier should - need no discussion. - - The quota period stays in the name (`DailyQuota` vs `MonthlyQuota`) on - purpose: a usage plan carries exactly one quota and these two plans use - different periods, so encoding it makes the two impossible to misread. + | `apiKeyRateLimit` | `selfServicePlanRateLimit` (1) | + | `apiKeyBurstLimit` | `selfServicePlanBurstLimit` (5) | + | `apiGatewayPartnerDailyQuota` | `selfServicePlanMonthlyQuota` (100000) | + + Three renames, no additions — the one-plan outcome collapsed the six fields + this table originally listed into three. The reason to rename here rather than + later: `apiKeyRateLimit` reads as a global property of API keys, when it is one + plan's rate. + + The quota period stays in the name (`MonthlyQuota`) on purpose: a usage plan + carries exactly one quota, so encoding the unit makes it impossible to misread. - **Renaming a config key is not renaming a resource.** CloudFormation sees only the *values*; the key exists solely for our own typed config, so `tsc` flags - every missed usage and no resource is touched. This is the opposite of the - usage plan's `name` property, which must **not** change — that one risks a - replacement that would invalidate the working partner key. Keep the two - straight. - - Blast radius, counted: `types.ts` (14 occurrences, declarations plus - validations), `api-gateway-stack.ts` (4), `envs/production.json` (3). `cicd.json` - does not carry these fields and there are no consumers outside the repo. - [[0121]] mentions the old names in prose and should be updated with them; + every missed usage and no resource is touched. + + Renaming the *plan* turned out to be equally safe — `UsagePlanName` is + *No interruption* — so the caution originally recorded here was unnecessary. + + `AWS::ApiGateway::ApiKey.Name` *is* replacement, but **that is not the + mechanism that rotated the key here** *(corrected 2026-08-11 from the synthesized + template)*. The construct id changed too (`PartnerApiKey` → + `SelfServiceApiKey`), which changes the logical id — so CloudFormation sees + `ApiPartnerApiKey8034B29D` removed and `ApiSelfServiceApiKey90815276` added, + two unrelated resources, and `Name`'s replacement semantics never come into + play. The outcome is identical (new value, old key destroyed); the CFN verb is + remove+add, not Replacement. Both paths delete the old resource only in the + post-success cleanup phase, so neither opens a window where a valid key is + rejected. + + **Blast radius — corrected 2026-08-11.** The list below was written about the + *config-field rename* and then read as the whole change's footprint, and the + claim "no consumers outside the repo" is what let seven published OpenAPI + descriptions go unnoticed. The full set actually touched: + `types.ts`, `api-gateway-stack.ts`, `compute-stack.ts`, `envs/production.json`, + `infra/README.md`, the new `docs/runbooks/manual-api-key-tier.md`, + `docs/prices-api-general-overview.md`, + `docs/database-schema/database-schema-overview.md`, + `packages/prices-api/loadtest/README.md`, [[0121]], and in Rust: + seven `utoipa` 429 descriptions (`assets/`, `batch/`, `backfill/`, + `oracles/handlers.rs`) plus `config.rs` and `auth/mod.rs`. The 429 descriptions + **are** a consumer outside the repo — they ship in the OpenAPI document served + at `GET /api-docs-json`. + + Original note, scoped to the config rename: `types.ts`, `api-gateway-stack.ts`, + `envs/production.json`, plus the SSM contract table in `infra/README.md`. + `cicd.json` does not carry these fields and there are no consumers outside the + repo. [[0121]] mentions the old names in prose and should be updated with them; archived tasks keep the old names as historical record. -- Validation follows the existing pattern (positive integers, burst ≥ rate), with - the cross-checks moved onto the new names: - `apiGatewayThrottleRate >= partnerPlanRateLimit`, and a new - `selfServicePlanRateLimit <= partnerPlanRateLimit` — cheap, and it catches the - "100 instead of 1" typo this whole task exists to prevent. That last check also - encodes "self-service is never faster than the manual tier", which is intended - but will fail synth if the partner tier is ever lowered. +- Validation follows the existing pattern (positive integers, burst ≥ rate). + The originally-planned cross-check `selfServicePlanRateLimit <= + partnerPlanRateLimit` does not exist in the one-plan outcome — and would not + have caught the "100 instead of 1" typo anyway (100 ≤ 100 passes). Replaced by + a check derived from the epic's own argument; see Design Decisions. - Quota period `apigateway.Period.MONTH`. Note that **a usage plan carries exactly one quota** — no daily sub-cap alongside the monthly one — so a key at full rate spends the month's allowance in ~28 hours and then waits for the @@ -200,40 +263,163 @@ conclusion stands on the remaining arguments. ## Acceptance Criteria -- [ ] Self-service usage plan in CDK: 1 req/s sustained, burst 5, 100 000 +- [x] Self-service usage plan in CDK: 1 req/s sustained, burst 5, 100 000 requests/month - [ ] A key on that plan is throttled at 1 req/s and its monthly quota - decrements; `403`-without-key behaviour unchanged + decrements; `403`-without-key behaviour unchanged *(needs deploy)* - [ ] The quickstart's example queries run without hitting the burst limit -- [ ] Partner plan and key unchanged and still working — verified against the - deploy diff, not assumed -- [ ] Self-service usage plan id readable by [[0160]] without closing the - Compute → ApiGateway cycle -- [ ] New config validated at synth like the existing key fields -- [ ] Config fields renamed to the `Plan*` scheme, `apiGateway*` reserved - for gateway- and stage-level settings; synth produces the same template it - did before the rename, proving no resource depends on a key name -- [ ] Manual higher-tier runbook written + *(needs deploy; [[0163]] not written yet)* +- [ ] ~~Partner plan and key unchanged~~ — **superseded, not met.** This AC + assumed the two-plan design. As built the plan is renamed and re-limited in + place and the key rotates; both deliberate, both verified against the live + `cdk diff` rather than assumed. Nothing has changed in AWS yet — the + rotation happens *on deploy*, and at that moment every holder of + `prices-production-partner-key` is cut off. +- [x] Self-service usage plan id readable by [[0160]] without closing the + Compute → ApiGateway cycle — published as + `/prices/{env}/pricing-api-free-plan-id`, recorded in the `infra/README.md` + SSM contract table +- [x] New config validated at synth like the existing key fields +- [x] Config fields renamed; `apiGateway*` reserved for gateway- and stage-level + settings. ~~Synth produces the same template as before the rename~~ — moot + under the one-plan outcome: the template *must* change. What was verified + instead is that the usage plan's logical id (`ApiUsagePlanDBBE8AB1`) is + unchanged, so CloudFormation updates the deployed plan in place rather than + creating a second one. +- [x] Manual higher-tier runbook written — `docs/runbooks/manual-api-key-tier.md` - [ ] Epic AC 5 satisfied: default limits are 1 req/s + monthly quota, not the - design doc's 100 req/s + design doc's 100 req/s *(code done; closes on deploy)* +- [ ] `docs/scf/milestone-1-evidence.md:795` states 100 req/s as a delivered + property — must be reconciled with the new limit before this task closes + +## Design Decisions + +### From Plan + +1. **1 req/s, burst 5, 100 000/month.** Rate straight from the epic; quota at the + the top of the epic's illustrative "e.g. 50,000–100,000" — our choice, not a + bound the epic set, and [[0010]] Alternative 6 treats it as a live cost lever + (~$0.19/key-month at 50k, ~$0.09 at 25k), so it is revisitable downward + without contradicting anything; burst 5 so the quickstart's parallel + example queries do not 429. All three confirmed by Adam 2026-08-06. + +2. **Plan id published via SSM** rather than read from the plan object — + `ComputeStack` is a dependency of `ApiGatewayStack`, so [[0160]] cannot reach + the construct without closing the cycle. Same shape as [[0124]]'s `apiBaseUrl`. + +### Emerged + +3. **One usage plan, renamed in place — not two plans.** The task specified adding + a second plan and leaving `partner-plan` untouched. Three findings overturned + that: + + - **The plan was never a tier decision.** It was created 2026-05-21 by task + 0011, a CDK bootstrap task, described there as "Usage plan + API key *pattern + wired*". Its numbers arrived in 0040, copied from the design doc's §2.1/§7. + The epic postdates it by two and a half months (2026-08-06). + - **The epic does not ask for a second plan.** It says "rate-limit-only change + … does not affect the usage-plan mechanism already agreed", and calls the + manual tier "nothing to build here — explicitly a future problem". + - **Nobody was using the partner key.** `GetUsage` over 30 days: **14 requests + across 4 days**, max 4/day — the shape of our own verification curls. + + Had the usage been non-trivial, the two-plan design would have stood: silently + dropping a live holder from 100 to 1 req/s is not a side effect to accept. + +4. **`UsagePlanName` is not a replacement risk.** The task asserted renaming the + plan "risks a CloudFormation resource replacement". Every property of + `AWS::ApiGateway::UsagePlan` — `ApiStages`, `Description`, `Quota`, `Tags`, + `Throttle`, `UsagePlanName` — is *Update requires: No interruption*. Confirmed + in practice: `cdk diff` shows the plan updated in place under its original + logical id. + +5. **Key rotated as part of the same change** — through CDK rather than through + manual CLI steps that would drift from the stack. Mechanically it rotates via + the construct-id change (remove + add), not via `ApiKey.Name` replacement; see + the correction in Implementation. Prompted by the old key's + value being exposed while checking its usage — it was read with + `get-usage-plan-keys`, which returns the plaintext `value` with no way to + suppress it. Use `get-api-keys` without `--include-values` for that question. + +5b. **The stage throttle is per-method, not an aggregate stage pool** *(found + 2026-08-11, after the code was written)*. AWS: *"Per-API, per-stage throttling + limits are applied at the API method level for a stage."* So + `apiGatewayThrottleRate: 200` grants **each method its own** 200 req/s bucket; + there is no stage-wide pool that callers draw down together. The only genuinely + shared ceiling above the usage plan is the account limit — 10 000 RPS / 5 000 + burst in `eu-central-1`. + + Three things in this task were built on the aggregate reading and had to be + rewritten: the justification for the `/api-docs-json` method throttle (an + anonymous loop there **cannot** 429 a key holder on `/v1/...` — the throttle + earns its place by bounding that route's own cost instead), the + `apiGatewayThrottleRate` doc comment, and the derivation under DD #6. The + epic's "10 keys saturate the 1000 req/s global burst ceiling" premise + (Context, above) does not survive this either — it should be read as a third + rejected premise alongside the `db.t4g.micro` and $900 ones. The epic's + conclusion is unaffected: 100 req/s per self-issued key is still wrong, just + not for that reason. + +6. **Validation is a proportionality ratio, not a capacity calculation** *(recast + 2026-08-11 — see 5b; the original text below described a stage "ceiling" that + keys "fit under", which per-method semantics do not support)*. The planned + check (`selfServicePlanRateLimit <= partnerPlanRateLimit`) has no counterpart + with one plan, and would not have caught the "100 instead of 1" typo it was + meant to catch (100 ≤ 100 passes). Replaced with: a plan limit may be at most + **one tenth of the stage's default per-method limit**, applied to rate *and* + burst. At 200/400 that permits 1/5 and rejects 100. + + The ratio is a judgement call, not arithmetic derived from anything. Its real + job is narrow and worth stating plainly: stop the design doc's 100 req/s being + reinstated by typo. A bare `<=` against the stage value would not — 200 ≥ 100 + passes. The earlier framing ("10 keys must fit under the ceiling") was + retired by 5b; it also transplanted the epic's arithmetic, which was against + the 1000 req/s global burst, not against `apiGatewayThrottleRate`. + + Three consequences, all intended: the pre-existing + `apiGatewayThrottleRate >= apiKeyRateLimit` check (justified as "so a single + key can reach its per-key SLA") was dropped, since self-service carries no SLA; + the plan limits are now coupled to the stage defaults, so a future tier above + one tenth of them fails synth by design; and because a plan limit must be ≥ 1, + the coupling gives `apiGatewayThrottleRate` an implied floor of 10 — not + binding at 200, but a constraint on anyone dialling the stage throttle down. + +7. **`docs/runbooks/manual-api-key-tier.md` states the drift trade-off + explicitly.** A hand-made plan does not appear in `cdk diff` and nobody reviews + it, so the runbook carries a registry table and makes recording the resource a + required step rather than an afterthought. ## Notes -- **Sequencing with [[0121]] — and the partner key does not solve it either.** - The 100 req/s load test must not run against a self-service key, or it just - measures our own throttle. But the manual-tier key carries a **10 000/day - quota**, which at 100 req/s is exhausted in **100 seconds** — so a sustained - test hits `429` from the quota rather than the throttle, and reports the same - configuration artefact by a different route. Either give the load test its own - quota-free plan, raise the partner quota for the duration, or cap the run - below 100 seconds and say so in the report. Belongs in [[0121]]; recorded here - because it surfaced while sizing this task. -- Throttle and quota are evaluated *before* the response cache, so a cached - response still counts against the caller's quota. Worth stating in the - quickstart ([[0163]]) — it is the first thing a partner asks. -- The stage throttle (200 rps / 400 burst) stays the population-level ceiling; +- **Sequencing with [[0121]] — the one-plan outcome makes this sharper, not + softer.** *(Rewritten 2026-08-11; the original version reasoned about a + manual-tier key with a 10 000/day quota. Neither exists — the two-plan design + was dropped and the daily quota became monthly.)* After this task there is + exactly one CDK-managed plan, at 1 req/s / 100 000 per month, and **no key in + the account can sustain 100 req/s**. So the load test cannot simply pick a + different existing key: a run against `pricing-api-free-production` measures our + own throttle, and at 30 000 requests per 5-minute run it also spends nearly a + third of the month's allowance. 0121 must provision a plan for the run — + `docs/runbooks/manual-api-key-tier.md` is the procedure — and state in the + report which plan the key was on. Applied to [[0121]] and to + `packages/prices-api/loadtest/README.md` on 2026-08-11. +- **A cached response still costs the caller a request** — worth stating in the + quickstart ([[0163]]), it is the first thing a partner asks. Two halves, with + different standing *(separated 2026-08-11; this bullet previously asserted both + as fact)*: the **billing** half is documented — API Gateway charges per call + received, hit or miss, and the cache is billed separately by the hour. That the + **quota** decrements before the cache lookup is **our inference** — AWS's + documented throttling order names the usage plan, stage, account and Regional + limits and never mentions the cache. Say it in [[0163]] with that split, the + way the runbook now does; do not present the ordering as AWS behaviour. +- ~~The stage throttle (200 rps / 400 burst) stays the population-level ceiling; the usage plan is the per-key one, and the more restrictive of the two - applies. It takes ~200 flat-out self-service keys to reach that ceiling. + applies. It takes ~200 flat-out self-service keys to reach that ceiling.~~ + **Wrong — struck 2026-08-11, see Design Decision 5b.** The stage value is a + *per-method default*, not a population pool, so no number of keys collectively + "reaches" it. What survives: the more restrictive of stage-method and plan + applies, and the real population ceiling is the account limit (10 000 RPS / + 5 000 burst in `eu-central-1`). - Verification is manual at deploy time: `infra/` has no unit tests today, so there is nowhere to assert the plan's shape in CI without building that first. Out of scope here. diff --git a/lore/1-tasks/backlog/0047_RESEARCH_cross-tenant-throughput-verification-on-shared-hetzner-ch.md b/lore/1-tasks/backlog/0047_RESEARCH_cross-tenant-throughput-verification-on-shared-hetzner-ch.md index f9e0a713..23f05504 100644 --- a/lore/1-tasks/backlog/0047_RESEARCH_cross-tenant-throughput-verification-on-shared-hetzner-ch.md +++ b/lore/1-tasks/backlog/0047_RESEARCH_cross-tenant-throughput-verification-on-shared-hetzner-ch.md @@ -103,9 +103,12 @@ the architecture". Specifically: - **Writes:** ~6 Lambdas × 3 envs, batched HTTP `INSERT` into `prices.*` tables, peak ~10-15 req/s per env (per the brief §3.2 ask 5). - - **Reads:** API gateway → handler Lambdas → CH at ~100 req/s - per key (per overview §2.1), aggregating to roughly comparable - peak per env. + - **Reads:** API gateway → handler Lambdas → CH. [[0157]] caps a + self-service key at **1 req/s** (overview §2.1's 100 req/s per key + is superseded), so the per-key figure no longer sizes anything — + what reaches ClickHouse is bounded by the stage's 200 req/s + per-method default across all callers, less whatever the gateway + response cache absorbs. - CH is a single instance on a single physical host with bounded CPU / RAM / IOPS / `max_concurrent_queries`. Caddy is a single process with bounded `max_keepalive_conns`. @@ -144,7 +147,7 @@ Build a write-load and read-load model: | Oracle Fetcher (0039) | EventBridge 5min | 1 INSERT per 5min | INSERT into `oracle_prices` (also driven by REFLECTOR events via Ledger Processor) | | Asset Discovery (0039) | EventBridge 1hr | 1 small INSERT per hour | UPSERT into `assets` | | Cleanup Worker (0039) | EventBridge daily | 1 ALTER TABLE per day | DROP PARTITION (cheap) | -| API read handlers (0040) | API Gateway | up to ~100 req/s per key | SELECT from `price_ohlcv_*` / `current_prices` / `oracle_prices` | +| API read handlers (0040) | API Gateway | ≤1 req/s per self-service key ([[0157]]); population bound is the stage's 200 req/s per-method default, minus gateway cache hits | SELECT from `price_ohlcv_*` / `current_prices` / `oracle_prices` | Aggregate per env, then × 3 envs (dev/staging/prod) where prod dominates. diff --git a/lore/1-tasks/backlog/0121_TEST_api-load-test-100rps-report.md b/lore/1-tasks/backlog/0121_TEST_api-load-test-100rps-report.md index 82a49c1e..e9f1a202 100644 --- a/lore/1-tasks/backlog/0121_TEST_api-load-test-100rps-report.md +++ b/lore/1-tasks/backlog/0121_TEST_api-load-test-100rps-report.md @@ -76,18 +76,22 @@ was opened to answer. the test measures latency, not 429s. - Instrument: p50/p90/p95/p99, error rate by status class, `X-Cache` hit ratio, Lambda duration + cold starts, ClickHouse query time, and API Gateway 4xx/5xx. -- Use a dedicated API key with its own usage plan so the test cannot exhaust the - partner key's daily quota (`apiGatewayPartnerDailyQuota: 10000` — note that - 100 req/s × 5 min = **30,000 requests**, which *exceeds* that quota; a - separate key or a raised quota is mandatory, not optional). +- Use a dedicated API key with its own usage plan. **Updated by [[0157]]:** there + is now exactly one CDK-managed plan, `pricing-api-free-production`, at **1 req/s + with a 100 000/month quota**. Both limits make it unusable here — the rate alone + means the run measures our own throttle rather than the system, and + 100 req/s × 5 min = **30,000 requests** would spend nearly a third of the + month's allowance per run. A separate plan sized for the run is mandatory, not + optional; `docs/runbooks/manual-api-key-tier.md` is the procedure. - Publish a report: test plan, environment, raw numbers, graphs, and an explicit statement of whether the AC passed. ## Acceptance Criteria - [ ] k6 (or Locust) script committed to the repo and runnable from the README -- [ ] Dedicated load-test API key / usage plan provisioned — the partner key's - 10,000/day quota is **not** used and is not exhausted by the run +- [ ] Dedicated load-test API key / usage plan provisioned, sized above 100 req/s + — the run does **not** use a key on `pricing-api-free-production` (1 req/s, + 100 000/month), which would measure our own throttle rather than the system - [ ] 100 req/s sustained for 5 minutes on `GET /assets/{id}/price` completes - [ ] p95 < 200ms and error rate < 0.1% on the 20-asset spread scenario - [ ] Percentiles reported separately for cache hits and cache misses From 308c1ad145c430016708e69bb4d11bd95d26b601 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:44:14 +0200 Subject: [PATCH 5/9] refactor(lore-0157)!: name the plan family pricing-api-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Plan*` naming rule requires `` to be the plan's actual name, and it was not: config fields, the key construct and the key name said `selfService*` while everything AWS sees said `pricing-api-free`. One plan carried two names split down the middle. "Self-service" is how a key is obtained, not what the plan is called; it stays in prose only. Also enforces the floor the plan/stage ratio implies. It was documented in a comment and therefore surfaced as an instruction nobody could follow: clamping apiGatewayThrottleRate to 5 to shed load failed synth with "the maximum is 0" against a field that must be >= 1. The floor is now asserted directly against the stage field that actually has to change, so exactly one error fires below 10. Deploy impact is unchanged from d4ad060 — one remove plus one add, so the production key still rotates and every holder is cut off. Only the logical id on the add side differs (ApiPricingApiFreeApiKey04E49C4B). The usage plan's logical id (ApiUsagePlanDBBE8AB1) is untouched, so the deployed plan is updated in place rather than duplicated. BREAKING CHANGE: `prices-production-partner-key` is destroyed on deploy and replaced by `pricing-api-free-production-key` with a new value. --- infra/envs/production.json | 6 +- infra/src/lib/stacks/api-gateway-stack.ts | 23 +++--- infra/src/lib/stacks/compute-stack.ts | 2 +- infra/src/lib/types.ts | 85 +++++++++++++++-------- 4 files changed, 74 insertions(+), 42 deletions(-) diff --git a/infra/envs/production.json b/infra/envs/production.json index e4565bc0..5e04d538 100644 --- a/infra/envs/production.json +++ b/infra/envs/production.json @@ -3,9 +3,9 @@ "awsRegion": "eu-central-1", "apiGatewayThrottleRate": 200, "apiGatewayThrottleBurst": 400, - "selfServicePlanRateLimit": 1, - "selfServicePlanBurstLimit": 5, - "selfServicePlanMonthlyQuota": 100000, + "pricingApiFreePlanRateLimit": 1, + "pricingApiFreePlanBurstLimit": 5, + "pricingApiFreePlanMonthlyQuota": 100000, "apiGatewayCacheEnabled": true, "apiBaseUrl": "https://02mabge71l.execute-api.eu-central-1.amazonaws.com/production", "apiHandler": { diff --git a/infra/src/lib/stacks/api-gateway-stack.ts b/infra/src/lib/stacks/api-gateway-stack.ts index a54e0348..938848f6 100644 --- a/infra/src/lib/stacks/api-gateway-stack.ts +++ b/infra/src/lib/stacks/api-gateway-stack.ts @@ -89,7 +89,7 @@ const API_DOCS_THROTTLE = { rate: 10, burst: 20 } as const; * prefix of its own, so there is no `/v1/v1` double-prefix. * * - **Auth / rate limit**: data routes set `apiKeyRequired: true`; the UsagePlan - * enforces the self-service per-key rate (`selfServicePlanRateLimit`) + a + * enforces the self-service per-key rate (`pricingApiFreePlanRateLimit`) + a * monthly quota (task 0157, overriding the design doc's §2.1/§7 100 req/s). * `GET /health` stays a keyless mock (cheapest liveness probe), and * `GET /api-docs-json` is a keyless proxy to the handler (task 0124 — public @@ -348,7 +348,7 @@ export class ApiGatewayStack extends cdk.Stack { } // --------------------------------------------------------------- - // UsagePlan + API key — self-service tier (task 0157). + // UsagePlan + API key — the `pricing-api-free` tier (task 0157). // // One plan, because a key belongs to exactly one plan per stage and // self-service is the default (and currently only) way to hold a key. @@ -362,11 +362,11 @@ export class ApiGatewayStack extends cdk.Stack { const usagePlan = this.api.addUsagePlan('UsagePlan', { name: `pricing-api-free-${config.envName}`, throttle: { - rateLimit: config.selfServicePlanRateLimit, - burstLimit: config.selfServicePlanBurstLimit, + rateLimit: config.pricingApiFreePlanRateLimit, + burstLimit: config.pricingApiFreePlanBurstLimit, }, quota: { - limit: config.selfServicePlanMonthlyQuota, + limit: config.pricingApiFreePlanMonthlyQuota, period: apigateway.Period.MONTH, }, }); @@ -376,7 +376,7 @@ export class ApiGatewayStack extends cdk.Stack { // // 1. Changing the CONSTRUCT ID changes the logical id, so CloudFormation sees // a removal and an unrelated addition. That is what task 0157 did - // (`PartnerApiKey` -> `SelfServiceApiKey`). + // (`PartnerApiKey` -> `PricingApiFreeApiKey`). // 2. Changing `apiKeyName` alone is a Replacement — AWS::ApiGateway::ApiKey // .Name is "update requires replacement". // @@ -389,8 +389,15 @@ export class ApiGatewayStack extends cdk.Stack { // What matters is the part that is the same either way: the key gets a new // value and every holder is cut off. Deliberate here; touch neither line // casually. - const apiKey = this.api.addApiKey('SelfServiceApiKey', { - apiKeyName: `prices-${config.envName}-selfservice-key`, + // + // This is the ONLY key on the plan that CloudFormation manages, and it is + // ours — the one verification curls authenticate with. Task 0160 mints a key + // per Discord user onto the same plan via the SDK at runtime; those never + // appear in this template and no deploy can touch them. So "a key on + // pricing-api-free" is not the same thing as "this key", and only this one + // has no owning row in 0158's registry. + const apiKey = this.api.addApiKey('PricingApiFreeApiKey', { + apiKeyName: `pricing-api-free-${config.envName}-key`, }); usagePlan.addApiKey(apiKey); diff --git a/infra/src/lib/stacks/compute-stack.ts b/infra/src/lib/stacks/compute-stack.ts index 732a8df4..024f1115 100644 --- a/infra/src/lib/stacks/compute-stack.ts +++ b/infra/src/lib/stacks/compute-stack.ts @@ -445,7 +445,7 @@ export class ComputeStack extends cdk.Stack { // the ledger processor. No `API_KEYS` env → the in-app key gate stays // DISARMED; the per-key rate and monthly quota are enforced at the API // Gateway usage plan (ADR 0008; limits set by task 0157 — - // `selfServicePlanRateLimit` / `selfServicePlanMonthlyQuota`, not the design + // `pricingApiFreePlanRateLimit` / `pricingApiFreePlanMonthlyQuota`, not the design // doc's 100 req/s). `reservedConcurrentExecutions` is the optional SLO escape // hatch (only set when configured). API Gateway grants invoke via the // integration's resource policy (no role-cycle, unlike the SQS ESM above). diff --git a/infra/src/lib/types.ts b/infra/src/lib/types.ts index db9b00a1..7bc15432 100644 --- a/infra/src/lib/types.ts +++ b/infra/src/lib/types.ts @@ -47,29 +47,29 @@ export interface EnvironmentConfig { /** Default per-method token-bucket capacity above the rate (same scope). */ readonly apiGatewayThrottleBurst: number; /** - * Per-key sustained requests/second on the self-service usage plan. + * Per-key sustained requests/second on the `pricing-api-free` usage plan. * * The design doc's §2.1 / §7 figure was 100 req/s, sized for a key we hand * out deliberately. Task 0157 overrides it: a key anybody can mint by signing * in must not be able to consume the sustained load the whole system is * load-tested against. */ - readonly selfServicePlanRateLimit: number; + readonly pricingApiFreePlanRateLimit: number; /** - * Token-bucket capacity for the self-service plan. Refill keeps the sustained - * rate at `selfServicePlanRateLimit`; burst only lets the allowance be spent + * Token-bucket capacity for the `pricing-api-free` plan. Refill keeps the sustained + * rate at `pricingApiFreePlanRateLimit`; burst only lets the allowance be spent * unevenly — enough that the quickstart's parallel example queries don't 429. */ - readonly selfServicePlanBurstLimit: number; + readonly pricingApiFreePlanBurstLimit: number; /** - * Monthly request quota for the self-service plan (UsagePlan quota.limit). + * Monthly request quota for the `pricing-api-free` plan (UsagePlan quota.limit). * * The operative limit a caller actually meets: at the per-second rate a key * could produce ~2.6M requests/month, so the quota binds ~26x harder than the * throttle. The period is in the name because a usage plan carries exactly one * quota — encoding it makes the unit impossible to misread. */ - readonly selfServicePlanMonthlyQuota: number; + readonly pricingApiFreePlanMonthlyQuota: number; /** * Whether the API Gateway stage response cache (0.5 GB) is enabled. Per-route * TTLs are fixed in `ApiGatewayStack` per §2.1. @@ -299,31 +299,31 @@ export function validateConfig(config: EnvironmentConfig): void { ); } if ( - !Number.isInteger(config.selfServicePlanRateLimit) || - config.selfServicePlanRateLimit < 1 + !Number.isInteger(config.pricingApiFreePlanRateLimit) || + config.pricingApiFreePlanRateLimit < 1 ) { errors.push( - `selfServicePlanRateLimit must be a positive integer, got: ${config.selfServicePlanRateLimit}`, + `pricingApiFreePlanRateLimit must be a positive integer, got: ${config.pricingApiFreePlanRateLimit}`, ); } // The `< 1` test is not redundant with the rate check below it. Errors are // accumulated, not short-circuited, so with an invalid rate of -10 a burst of // -5 would pass `burst < rate` and go unreported until the rate was fixed. if ( - !Number.isInteger(config.selfServicePlanBurstLimit) || - config.selfServicePlanBurstLimit < 1 || - config.selfServicePlanBurstLimit < config.selfServicePlanRateLimit + !Number.isInteger(config.pricingApiFreePlanBurstLimit) || + config.pricingApiFreePlanBurstLimit < 1 || + config.pricingApiFreePlanBurstLimit < config.pricingApiFreePlanRateLimit ) { errors.push( - `selfServicePlanBurstLimit must be a positive integer >= selfServicePlanRateLimit (${config.selfServicePlanRateLimit}), got: ${config.selfServicePlanBurstLimit}`, + `pricingApiFreePlanBurstLimit must be a positive integer >= pricingApiFreePlanRateLimit (${config.pricingApiFreePlanRateLimit}), got: ${config.pricingApiFreePlanBurstLimit}`, ); } if ( - !Number.isInteger(config.selfServicePlanMonthlyQuota) || - config.selfServicePlanMonthlyQuota < 1 + !Number.isInteger(config.pricingApiFreePlanMonthlyQuota) || + config.pricingApiFreePlanMonthlyQuota < 1 ) { errors.push( - `selfServicePlanMonthlyQuota must be a positive integer, got: ${config.selfServicePlanMonthlyQuota}`, + `pricingApiFreePlanMonthlyQuota must be a positive integer, got: ${config.pricingApiFreePlanMonthlyQuota}`, ); } if (typeof config.apiGatewayCacheEnabled !== 'boolean') { @@ -377,33 +377,58 @@ export function validateConfig(config: EnvironmentConfig): void { // The only genuinely shared ceiling above the usage plan is the account limit // (10 000 RPS / 5 000 burst in eu-central-1), which nothing here approaches. const MAX_PLAN_SHARE_OF_STAGE_DEFAULT = 10; + + // The floor the ratio implies, stated as its own rule rather than left to be + // discovered through an instruction that cannot be followed. A plan limit must + // be >= 1, so a stage default below MAX_PLAN_SHARE_OF_STAGE_DEFAULT admits no + // legal plan limit at all: the ratio check would report "the maximum is 0" + // while the checks above reject anything under 1. Fail here instead, naming + // the field that is actually wrong. At 200 this is nowhere near binding, but + // an operator clamping the stage throttle to shed load has to stay at or + // above 10 — and lower the plan limits with it, not instead of it. + const stageFloors: ReadonlyArray = [ + ['apiGatewayThrottleRate', config.apiGatewayThrottleRate], + ['apiGatewayThrottleBurst', config.apiGatewayThrottleBurst], + ]; + for (const [stageField, stageValue] of stageFloors) { + if ( + Number.isInteger(stageValue) && + stageValue >= 1 && + stageValue < MAX_PLAN_SHARE_OF_STAGE_DEFAULT + ) { + errors.push( + `${stageField} (${stageValue}) must be at least ${MAX_PLAN_SHARE_OF_STAGE_DEFAULT}: ` + + `a usage-plan limit may be at most one ${MAX_PLAN_SHARE_OF_STAGE_DEFAULT}th of it and must itself be >= 1, ` + + `so a lower stage default leaves no satisfiable plan limit`, + ); + } + } + const planVsStage: ReadonlyArray = [ [ - 'selfServicePlanRateLimit', - config.selfServicePlanRateLimit, + 'pricingApiFreePlanRateLimit', + config.pricingApiFreePlanRateLimit, 'apiGatewayThrottleRate', config.apiGatewayThrottleRate, ], [ - 'selfServicePlanBurstLimit', - config.selfServicePlanBurstLimit, + 'pricingApiFreePlanBurstLimit', + config.pricingApiFreePlanBurstLimit, 'apiGatewayThrottleBurst', config.apiGatewayThrottleBurst, ], ]; - // Both `>= 1` guards keep this from piling a derived error on top of a primary - // one: with a stage rate of -200 the checks above already report it, and - // "the maximum is -20" would add noise, not information. - // - // Note the implied floor this creates: since a plan limit must be >= 1, a - // stage default below MAX_PLAN_SHARE_OF_STAGE_DEFAULT makes the config - // unsatisfiable. At 200 that is nowhere near binding, but anyone dialling the - // stage throttle down has to stay at or above 10. + // Both guards keep this from piling a derived error on top of a primary one: + // with a stage rate of -200 the checks above already report it, and "the + // maximum is -20" would add noise, not information. The stage-side guard is + // the floor check rather than `>= 1` for the same reason — below 10 the floor + // check above has already named the problem, and reporting "the maximum is 0" + // alongside it would only tell the operator to do something impossible. for (const [planField, planValue, stageField, stageValue] of planVsStage) { if ( Number.isInteger(stageValue) && - stageValue >= 1 && + stageValue >= MAX_PLAN_SHARE_OF_STAGE_DEFAULT && Number.isInteger(planValue) && planValue >= 1 && planValue * MAX_PLAN_SHARE_OF_STAGE_DEFAULT > stageValue From 831ce5b7a35d3fd2756558da6f0c56ea57614d33 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:44:34 +0200 Subject: [PATCH 6/9] docs(lore-0157): harden the manual-tier runbook and finish the 429 sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runbook, from review of PR 195. Nothing may resolve a key by taking the first match: key names are not unique (0158) and `--name-query` has no documented matching semantics at all (0156), so `items[0]` indexed into an undefined set. Mid-rotation that could disable the new key while the old one kept serving. Lookups now list candidates client-side and make the operator choose, key names carry the issuing instant so two never collide, and the registry is one row per key with the name as a column. The instant is to the second, not the day — the likeliest rotation is a leak rotated within the hour, which is exactly where a day suffix fails. "Change or revoke" runs in a fresh shell and re-exported only CUSTOMER, so against another default profile every lookup silently targeted the wrong account. It re-exports the profile and region, and guards the one remaining path where a failure could pass itself off as a value: with `--output text` an empty result prints the literal "None", which `-z` does not catch, so the guard unsets it and the failure lands on the command that needs it. Docs sweep. The row "Request throttling (100/s per API key, 1000/s global burst)" survived in both §6 tables; the database-schema one was cited as the source by a row this task had already rewritten to 1 req/s, so that file contradicted itself. Both now drop the global burst figure, which per-method throttle semantics retired. The tranche load-test criteria keep their 100 req/s target — it comes from the RFP and lowering it is a scope decision, not a docs sweep — but note that no key can sustain it any more, so the run needs a plan built for it. milestone-1-evidence keeps its 100 req/s paragraph with a dated superseded note: it records what the milestone was evidenced against, and what it is offered as evidence of is unaffected. 0171 gains the runbook alongside 0158 and 0160 as a consumer of the `nameQuery` measurement. --- .../database-schema-overview.md | 2 +- docs/prices-api-general-overview.md | 11 +- docs/runbooks/manual-api-key-tier.md | 119 +++++++++-- docs/scf/milestone-1-evidence.md | 8 + ...E_default-key-limits-1rps-monthly-quota.md | 184 +++++++++++++++++- ...60_FEATURE_onboarding-backend-endpoints.md | 4 +- ...undocumented-discord-and-aws-behaviours.md | 9 +- 7 files changed, 308 insertions(+), 29 deletions(-) diff --git a/docs/database-schema/database-schema-overview.md b/docs/database-schema/database-schema-overview.md index 82e003de..ea1c5bdf 100644 --- a/docs/database-schema/database-schema-overview.md +++ b/docs/database-schema/database-schema-overview.md @@ -1797,7 +1797,7 @@ asset's inception date, `GET /assets/{asset_identifier}/ohlcv` includes a | Layer | Strategy | | ------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **API Gateway caching** | Built-in response cache (0.5 GB). Per-endpoint TTLs: `/assets` list 60s, `/ohlcv` 60s, `/price` 15s, `/backfill/status` 30s. Cache key includes query params. `POST /prices/batch` uncached | -| **API Gateway throttling** | Request throttling (100/s per API key, 1000/s global burst) | +| **API Gateway throttling** | Request throttling (1 req/s sustained, burst 5, 100 000 req/month per self-service key — task 0157; 200 req/s per method stage-wide) | | **Lambda** | Rust binary with `lambda_runtime`. Sub-millisecond cold starts. Stateless, auto-scales to concurrency limit. No VPC, so no ENI provisioning latency on cold start | | **ClickHouse client (`clickhouse` crate)** | Warm connection pool reused across Lambda invocations to amortise mTLS handshake (~80-130 ms cross-cloud RTT to Caddy). Per-request payloads batched per-ledger so a typical invocation issues 1–2 INSERTs, not one per trade | | **Sort key + partitioning** | Per-granularity tables sorted by `(asset_id, quote_asset_id, source, timestamp)`; monthly partitions on `timestamp`. Partition pruning + sort-key skip eliminate irrelevant months and assets on hot reads | diff --git a/docs/prices-api-general-overview.md b/docs/prices-api-general-overview.md index d314e036..968e8389 100644 --- a/docs/prices-api-general-overview.md +++ b/docs/prices-api-general-overview.md @@ -161,7 +161,7 @@ to their own infrastructure at any time if needed. | **Lambda — Asset Discovery** | Asset registry | EventBridge rate(1 hour). Detects new SEP-41 contract deployments and classic asset issuances; UPSERTs into `prices.assets` | | **Lambda — Cleanup Worker** | Data retention | EventBridge cron(02:00 UTC daily). `ALTER TABLE … DROP PARTITION` on old monthly partitions of each per-granularity OHLCV table | | **Lambda — API handlers** | Public API | Individual functions per route group. Rust / axum via `lambda_runtime`, 256–512 MB, 15s timeout. No VPC; outbound HTTPS-mTLS to Caddy:443 | -| **API Gateway** | Public API entry point | REST API, usage plans, API key auth, rate limiting (1 req/s + 100 000 req/month per self-service key — task 0157 supersedes the 100 req/s in §2.1/§7), request validation. Built-in response cache (0.5 GB) with per-endpoint TTLs | +| **API Gateway** | Public API entry point | REST API, usage plans, API key auth, rate limiting (1 req/s sustained, burst 5, 100 000 req/month per self-service key — task 0157), request validation. Built-in response cache (0.5 GB) with per-endpoint TTLs | | **EventBridge Scheduler** | Scheduled triggers | Cron/rate rules for all periodic Lambda workers | | **Secrets Manager** | Credentials & mTLS material | Per-env client `{cert,key,ca}` for Caddy:443 mTLS (single JSON bundle secret per identity, named by `MTLS_SECRET_NAME`); Soroswap/Aquarius API keys; oracle contract address | | **CloudWatch + X-Ray** | Observability | API latency, error rates, ingestion lag, Lambda duration/concurrency, backfill progress; mTLS cert NotAfter alarm | @@ -1093,7 +1093,7 @@ CLI progress via direct SQL on the local workstation ClickHouse. | Layer | Strategy | | ---------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **API Gateway caching** | Built-in response cache (0.5 GB). Per-endpoint TTLs: `/assets` list 60s, `/ohlcv` 60s, `/price` 15s, `/backfill/status` 30s. Cache key includes query params. POST `/prices/batch` uncached | -| **API Gateway throttling** | Request throttling (100/s per API key, 1000/s global burst) | +| **API Gateway throttling** | Request throttling (1 req/s sustained, burst 5, 100 000 req/month per self-service key — task 0157; 200 req/s per method stage-wide) | | **Lambda** | Rust binary with `lambda_runtime`. Sub-millisecond cold starts. Stateless, auto-scales to concurrency limit. No VPC, so no ENI provisioning latency on cold start | | **Database client (`clickhouse` crate)** | Warm connection pool reused across Lambda invocations to amortise mTLS handshake (~80-130 ms cross-cloud RTT to Caddy). Per-request payloads batched per-ledger so a typical invocation issues 1–2 INSERTs, not one per trade | | **Sort key & partitioning** | Per-granularity tables sorted by `(asset_id, quote_asset_id, source, timestamp)`; monthly partitions on `timestamp`. Partition pruning + sort-key skip eliminate irrelevant months and assets on hot reads | @@ -1275,6 +1275,10 @@ for the full local-CLI metrics. 1. All 7 endpoint groups return correct, schema-valid responses for at least 20 major assets 2. Load test (k6 or Locust, script provided): 100 req/s sustained for 5 minutes on `GET /assets/{id}/price` → p95 latency <200ms, error rate <0.1% + — the target is unchanged, but since task 0157 no key in the account can sustain + it: the default plan is 1 req/s (§6). The run needs a usage plan created for it, + per `docs/runbooks/manual-api-key-tier.md`; the report must state which plan the + key was on (task 0121) 3. Cache confirmed: consecutive identical requests within TTL window return `X-Cache: Hit` header 4. VWAP calculation verifiable against raw `price_ohlcv` rows for at least 3 assets 5. `GET /backfill/status` shows `earliest_data_available` ≤ 2022-01-01 @@ -1332,7 +1336,8 @@ post-delivery monitoring. 2. OpenAPI spec passes `openapi-validator` lint with no errors; Swagger UI deployed 3. Onboarding portal accessible; self-service API key request flow functional 4. Integration test suite: all tests pass on CI (GitHub Actions link provided) -5. Load test report: p95 <100ms at 100 req/s confirmed +5. Load test report: p95 <100ms at 100 req/s confirmed — same caveat as Tranche 2 + AC 2: a purpose-built usage plan is required, and the report names it 6. Security checklist signed off: no wildcard IAM, ClickHouse endpoint reachable only via mTLS through Caddy:443, mTLS cert + key in Secrets Manager (not env vars), all inputs validated diff --git a/docs/runbooks/manual-api-key-tier.md b/docs/runbooks/manual-api-key-tier.md index b5bc4b0b..cdf9f127 100644 --- a/docs/runbooks/manual-api-key-tier.md +++ b/docs/runbooks/manual-api-key-tier.md @@ -80,8 +80,10 @@ terminal — but it did cross the wire and it lands in CLI debug output if you e rerun this with `--debug`. Do not. ```bash +KEY_NAME="prices-production-${CUSTOMER}-key-$(date -u +%Y%m%dT%H%M%SZ)" + KEY_ID=$(aws apigateway create-api-key \ - --name "prices-production-${CUSTOMER}-key" \ + --name "${KEY_NAME}" \ --description "Manual tier for ${CUSTOMER}" \ --enabled \ --tags "Project=stellar-prices-api,ManagedBy=manual,Customer=${CUSTOMER}" \ @@ -91,8 +93,30 @@ aws apigateway create-usage-plan-key \ --usage-plan-id "${PLAN_ID}" \ --key-id "${KEY_ID}" \ --key-type API_KEY + +echo "key name: ${KEY_NAME}" +echo "key id: ${KEY_ID}" ``` +**The timestamp in the name is load-bearing, not decoration.** Rotation (step 7) +runs step 2 again while the old key is still alive, and AWS does not enforce unique +key names — so without it the customer would briefly hold two keys called the same +thing, and every later lookup by name would have two right answers. With it, each +key names the instant it was issued and stays distinguishable for the rest of its +life. Record the full name in the registry table, not just the id. + +It is to the second (`20260812T142317Z`), not to the day, and that is deliberate. +The likeliest rotation is not a scheduled one — it is a leak, rotated the same +hour, and rotated again an hour later because the first replacement went to the +wrong inbox. A day-granular suffix collides in exactly that case, which is the one +the suffix exists for. + +The prefix `prices-production-${CUSTOMER}-` is also what keeps hand-made keys out +of the way of the self-service ones: task 0160 issues those as +`discord--key` and looks them up by exact name. **Never give a manual key +a `discord-` name**, and never give a self-service user a customer slug that +collides with one here. + **3. Check the stage ceiling still holds.** The stage's default method throttle applies to every caller of a given method — @@ -137,16 +161,68 @@ table at the bottom of this file — that is what it is for. If the row is missi or stale, recover them by name: ```bash +export AWS_PROFILE= +export AWS_REGION=eu-central-1 export CUSTOMER=acme + PLAN_ID=$(aws apigateway get-usage-plans \ --query "items[?name=='prices-production-${CUSTOMER}-plan'].id | [0]" --output text) -KEY_ID=$(aws apigateway get-api-keys --name-query "prices-production-${CUSTOMER}-key" \ - --query 'items[0].id' --output text) + +# `--output text` prints the literal string "None" for an empty result, not an +# empty string — so `[ -z "$PLAN_ID" ]` does NOT catch it, and an unguarded +# "None" flows into `update-usage-plan --usage-plan-id None` below. Unset it so +# the failure lands on the command that needs it, not three snippets later. +[ "${PLAN_ID}" = "None" ] && { + echo "no plan for ${CUSTOMER} — wrong AWS_PROFILE/AWS_REGION, or the plan is gone" + unset PLAN_ID +} + +# Keys: list every candidate and choose by hand. Do NOT collapse this to +# `items[0].id` — see below for why there can legitimately be more than one. +aws apigateway get-api-keys \ + --query "items[?starts_with(name, 'prices-production-${CUSTOMER}-key')].[id,name,createdDate]" \ + --output table + +export KEY_ID= ``` `get-api-keys` without `--include-values` does not return the secret. Then fix the registry row. +The two `export`s at the top are not ceremony: this section runs in a fresh +shell, and against a different default profile every lookup here quietly targets +the wrong account. Both failures then say the same thing in different ways — the +plan resolves to `None`, and the key table comes back empty — which is why the +guard names the profile rather than the plan. Nothing mutates in the wrong +account either way (`--usage-plan-id None` is rejected), but the error you get +from AWS points nowhere near the actual cause. + +The guard warns and unsets rather than exiting, because these snippets are pasted +into an interactive shell where `exit` would close your terminal. Unsetting is the +part that matters: a warning scrolls past — the table below it draws the eye — and +`PLAN_ID` would otherwise still hold `"None"` for every snippet after this one. +Unset, the next command that needs it fails on an empty argument, at the point of +use, saying so. + +Two reasons this lists rather than picks, both of which have bitten elsewhere in +this project: + +- **A key name is not unique.** AWS enforces uniqueness only on key _values_; + `name` is optional and duplicable. During a rotation (step 7) the customer + deliberately holds two keys at once, so a lookup by name has two right answers + and no way to rank them. Taking the first would suspend or delete whichever one + the API happened to list first — including, in the worst case, disabling the + _new_ key while the old one keeps serving. +- **`--name-query` has no documented matching semantics**, which is why it does + not appear above at all. AWS's entire description of it is _"The name of queried + API keys."_ — not documented as exact, as a prefix, or as anything else (task + 0156, 2026-08-10; measured by task 0171). `starts_with` is JMESPath, evaluated + client-side, and does exactly what it says. Task 0160 reaches the same + conclusion for the automated issuance path and comments it so nobody deletes it + as redundant; the same applies here. If you re-add `--name-query` as a + server-side prefilter, keep the client-side filter — it is the only part that + decides. + **Adjust limits** — safe, no key rotation: ```bash @@ -168,18 +244,26 @@ quota is preserved across a disable/enable cycle. **7. Rotate** — create a new key (step 2), attach it, confirm the customer is using it, then delete the old one. Capture the outgoing key's id **before** you -create the replacement, or you will be picking it out of a list by name: +create the replacement: once both exist, a lookup by name has two answers. ```bash -OLD_KEY_ID=$(aws apigateway get-api-keys \ - --name-query "prices-production-${CUSTOMER}-key" \ - --query 'items[0].id' --output text) +# From the registry row. If it is missing or stale, list candidates with the +# snippet above and choose the older one by createdDate — never items[0]. +OLD_KEY_ID= -# ... run step 2 to create the new key, hand it over, confirm it is in use ... +# ... run step 2 to create the new key; it carries the issuing instant, so the +# two never share a name — even rotating twice within the hour. Hand it +# over, confirm it is in use ... aws apigateway delete-api-key --api-key "${OLD_KEY_ID}" ``` +Update the registry row in the same sitting as step 2, not after the customer +confirms. The window in which two keys exist is exactly the window in which +somebody might have to suspend one of them in a hurry — a leak, a missed payment +— and a registry that still names only the old key is worse than useless then, +because it looks authoritative. + `get-api-keys` without `--include-values` is the safe way to look a key up: the plural form defaults to omitting the secret. Do **not** reach for `get-usage-plan-keys` — it returns the plaintext `value` and has no flag to @@ -223,6 +307,12 @@ Then delete the row from the table below. one usage plan **per stage** — so on this API there is no "also attach it to the bigger plan". (A key may sit in up to 10 plans overall, across different stages; that does not help here.) +- **Nothing stops you creating two keys with the same name.** Only key _values_ + are unique in API Gateway; `name` is optional and duplicable, and there is no + documented way to ask "which of these is the current one". That is why step 2 + timestamps the name, why the registry carries the name as well as the id, and + why every lookup in this file lists candidates instead of taking the first. If + you find yourself typing `items[0]`, stop. - **Quotas are best-effort.** AWS: _"Usage plan throttling and quotas are not hard limits… Don't rely on usage plan quotas or throttling to control costs."_ For a tier large enough to matter financially, back it with AWS Budgets. @@ -245,8 +335,13 @@ Then delete the row from the table below. ## Issued manual keys -Keep this current. One row per plan; delete the row when the plan is deleted. +Keep this current. One row per **key**, so a customer mid-rotation has two; +delete a row when its key is deleted, and the last one when the plan goes. + +| Customer | Plan name | Plan ID | Key name | Key ID | Limits | Issued | Issued by | +| ------------ | --------- | ------- | -------- | ------ | ------ | ------ | --------- | +| _(none yet)_ | | | | | | | | -| Customer | Plan name | Plan ID | Key ID | Limits | Issued | Issued by | -| ------------ | --------- | ------- | ------ | ------ | ------ | --------- | -| _(none yet)_ | | | | | | | +**Key name** is a column because key names are not unique and now carry the issuing +instant — during a rotation two rows may share a customer, and the name is what +tells them apart. Keep both rows until the old key is deleted. diff --git a/docs/scf/milestone-1-evidence.md b/docs/scf/milestone-1-evidence.md index 6884f94c..96a2269d 100644 --- a/docs/scf/milestone-1-evidence.md +++ b/docs/scf/milestone-1-evidence.md @@ -796,6 +796,14 @@ The usage plan `prices-production-partner-plan` throttles each key at 100 req/s (200 burst) with a 10,000/day quota; the stage caches this route for 60 s. +> **Superseded by task 0157 (2026-08-12).** That plan is renamed +> `pricing-api-free-production` and re-limited to 1 req/s sustained, burst 5 and +> 100 000 requests/month; the change is in CDK and takes effect on the next +> production deploy. The paragraph above is left as written because it records +> the configuration this milestone was actually evidenced against. What it is +> offered as evidence of — that the gateway, not the application, enforces the +> key requirement — is unchanged by the new limits. + _A precise statement about auth, because overclaiming here would be easy:_ the API Gateway key requirement is the live enforcement layer. The application also contains a constant-time `X-API-Key` gate, but it is diff --git a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md index 75956718..c3d93659 100644 --- a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md +++ b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md @@ -60,7 +60,7 @@ history: Changed: `types.ts`, `api-gateway-stack.ts`, `envs/production.json`, `infra/README.md` SSM contract, plus a new `docs/runbooks/manual-api-key-tier.md`. Three config fields renamed to - `selfServicePlan*`; six were planned, the one-plan outcome halved it. + `pricingApiFreePlan*`; six were planned, the one-plan outcome halved it. Key rotated in the same change (construct-id change → remove + add, not `ApiKey.Name` replacement) after its value was exposed by `get-usage-plan-keys` during the usage check. @@ -171,15 +171,50 @@ conclusion stands on the remaining arguments. | --- | --- | | `apiGatewayThrottleRate` / `apiGatewayThrottleBurst` | unchanged — stage-level, not a plan | | `apiGatewayCacheEnabled`, `apiBaseUrl` | unchanged | - | `apiKeyRateLimit` | `selfServicePlanRateLimit` (1) | - | `apiKeyBurstLimit` | `selfServicePlanBurstLimit` (5) | - | `apiGatewayPartnerDailyQuota` | `selfServicePlanMonthlyQuota` (100000) | + | `apiKeyRateLimit` | `pricingApiFreePlanRateLimit` (1) | + | `apiKeyBurstLimit` | `pricingApiFreePlanBurstLimit` (5) | + | `apiGatewayPartnerDailyQuota` | `pricingApiFreePlanMonthlyQuota` (100000) | Three renames, no additions — the one-plan outcome collapsed the six fields this table originally listed into three. The reason to rename here rather than later: `apiKeyRateLimit` reads as a global property of API keys, when it is one plan's rate. + **Corrected 2026-08-12, from review of [[PR 195]].** The right-hand column read + `selfServicePlan*` until Adam noticed it did not match the plan. The rule says + `Plan*`, and `` has to be the plan's **actual name** or the rule + buys nothing — the fields were named after "self-service", which is how you + *obtain* a key, while the plan they configure is called `pricing-api-free`. One + plan therefore carried two names, split down the middle: `pricing-api-free` on + everything AWS sees (`UsagePlanName`, the SSM parameter and its construct id), + `selfService*` on everything the repo sees (config fields, key construct, key + name). + + Worth recording that the rename that *was* reasoned about got a table, a date + and a rationale, while `pricing-api-free` itself arrived with none of the three + — no epic mention, no Design Decision, no row here. It is now the settled + family name by Adam's decision (2026-08-12); "self-service" stays in prose for + the concept — a key anybody can mint by signing in — and never as a resource or + field name. Everything the plan owns now reads the same way: + + | | | + | --- | --- | + | Usage plan | `pricing-api-free-` (construct `UsagePlan`, logical id unchanged) | + | SSM parameter | `/prices//pricing-api-free-plan-id` (construct `PricingApiFreePlanIdParam`) | + | Config fields | `pricingApiFreePlanRateLimit` / `…BurstLimit` / `…MonthlyQuota` | + | API key | `pricing-api-free--key` (construct `PricingApiFreeApiKey`) | + + The key was the one real decision in that list rather than a mechanical + follow-through, because it is the only entry whose name could reasonably have + come from its *function* instead of its plan — it is ours, the one verification + curls use, and after [[0160]] it will be one of many keys on this plan and the + only one CloudFormation manages or that has no owning row in [[0158]]'s + registry. Adam chose the family name; that distinction is therefore carried by + a comment at the construct instead, so nobody later goes looking for its + registry row. Free to do here either way: this task already destroys and + recreates that key, so the second rename rides along on a rotation that was + already paid for. + The quota period stays in the name (`MonthlyQuota`) on purpose: a usage plan carries exactly one quota, so encoding the unit makes it impossible to misread. @@ -193,10 +228,13 @@ conclusion stands on the remaining arguments. `AWS::ApiGateway::ApiKey.Name` *is* replacement, but **that is not the mechanism that rotated the key here** *(corrected 2026-08-11 from the synthesized template)*. The construct id changed too (`PartnerApiKey` → - `SelfServiceApiKey`), which changes the logical id — so CloudFormation sees - `ApiPartnerApiKey8034B29D` removed and `ApiSelfServiceApiKey90815276` added, + `PricingApiFreeApiKey`), which changes the logical id — so CloudFormation sees + `ApiPartnerApiKey8034B29D` removed and `ApiPricingApiFreeApiKey04E49C4B` added, two unrelated resources, and `Name`'s replacement semantics never come into - play. The outcome is identical (new value, old key destroyed); the CFN verb is + play. *(The construct id was `SelfServiceApiKey` until 2026-08-12; the naming + correction below changed which logical id appears on the right-hand side, not + what CloudFormation does — it is one remove + one add either way, so the + deploy impact is unchanged.)* The outcome is identical (new value, old key destroyed); the CFN verb is remove+add, not Replacement. Both paths delete the old resource only in the post-success cleanup phase, so neither opens a window where a valid key is rejected. @@ -289,8 +327,27 @@ conclusion stands on the remaining arguments. - [x] Manual higher-tier runbook written — `docs/runbooks/manual-api-key-tier.md` - [ ] Epic AC 5 satisfied: default limits are 1 req/s + monthly quota, not the design doc's 100 req/s *(code done; closes on deploy)* -- [ ] `docs/scf/milestone-1-evidence.md:795` states 100 req/s as a delivered - property — must be reconciled with the new limit before this task closes +- [x] `docs/scf/milestone-1-evidence.md:795` states 100 req/s as a delivered + property — reconciled 2026-08-12 with a dated *Superseded* note rather than + a rewrite. The paragraph records the configuration the milestone was + evidenced against; editing the number in place would turn a grant-facing + record of what was true into a statement of what is true now. What the + paragraph is offered as evidence of — gateway-enforced key auth — is + untouched by this task. +- [x] The §6 performance tables state the new per-key limit. **Found in review, + 2026-08-12** ([[PR 195]]): the 429 sweep was described as complete, but the + row `Request throttling (100/s per API key, 1000/s global burst)` survived + in *two* places — `docs/prices-api-general-overview.md:1096` and + `docs/database-schema/database-schema-overview.md:1800`. The second is the + worse of the two: this task had already rewritten that file's "Read rate" + row to `≤1 req/s per key`, and that row cites **§8.2 as its source** — the + very table still saying 100/s. The doc contradicted itself in a file the + task had edited. Both rows now also drop the "1000/s global burst" figure, + which Design Decision 5b retired. + The pointer at `prices-api-general-overview.md:164` is removed rather than + redirected: it named §2.1/§7, but line 164 *is* in §2.1 (so it pointed at + itself) and §7 states no numeric limit. With §6 corrected there is no + longer a section for it to point at. ## Design Decisions @@ -384,11 +441,120 @@ conclusion stands on the remaining arguments. the coupling gives `apiGatewayThrottleRate` an implied floor of 10 — not binding at 200, but a constraint on anyone dialling the stage throttle down. + **That third consequence is now a check of its own** *(2026-08-12, from review + of [[PR 195]])*. Leaving the floor implicit meant it surfaced as an + instruction nobody could follow: an operator clamping `apiGatewayThrottleRate` + to 5 to shed load during an incident got `pricingApiFreePlanRateLimit (1) … + so the maximum is 0`, while the positive-integer check rejects anything below + 1 — synth failing with two rules that contradict each other and no way out. + The ratio was never the thing that was wrong there; the stage value was. So + the floor is asserted directly (`apiGatewayThrottleRate`/`Burst` ≥ 10, naming + why), and the ratio loop's stage-side guard is raised from `>= 1` to the same + floor. Below 10 exactly one error now fires, against the field that actually + has to change. The noise-avoidance rationale is unchanged — this is the same + principle the `>= 1` guards already encoded, applied to a case that had been + documented in a comment instead of enforced. + 7. **`docs/runbooks/manual-api-key-tier.md` states the drift trade-off explicitly.** A hand-made plan does not appear in `cdk diff` and nobody reviews it, so the runbook carries a registry table and makes recording the resource a required step rather than an afterthought. +8. **Nothing in the runbook may resolve a key by taking the first match** + *(2026-08-12, from review of [[PR 195]])*. The recovery snippet read + `get-api-keys --name-query … --query 'items[0].id'`, and rotation re-ran the + issue step under the *same* key name — so during the overlap the customer holds + two keys called the same thing and `items[0]` picks one of them for reasons + nobody can state. The failure that matters is not cosmetic: a customer stops + paying mid-rotation, the operator recovers `KEY_ID` by name and runs "Suspend + without destroying", and disables the **new** key while the old one keeps + serving. + + Two facts the repo had already established, in tasks nobody thought to + re-read while writing this runbook: + + - **[[0158]]: an API key name is not unique.** AWS enforces uniqueness on key + *values* only; `name` is optional and duplicable, and AWS will not maintain + the invariant for us. + - **[[0156]]: `--name-query` has no documented matching semantics.** The whole + of AWS's description is *"The name of queried API keys."* — not exact, not a + prefix, nothing. The review that found this finding called it "a prefix + match, which widens it further"; that is the claim 0156 retired on + 2026-08-10, and its correction makes the finding *stronger* rather than + weaker — the set `items[0]` indexes into is not merely wider than expected, + it is undefined. + + The fix is not new invention: [[0160]] hit the identical problem on the + automated path and settled on client-side exact matching, commented so it is + not later deleted as redundant. The runbook was already doing that for the + *plan* (`items[?name=='…']`) two lines above the key lookup that wasn't — so + the change extends an idiom already in the file. Three parts: lookups list + candidates with `starts_with` + `createdDate` and make the operator choose; + step 2 timestamps the key name so two keys never collide in the first place; + the registry gains a **Key name** column and becomes one row per key rather + than per plan. + + The suffix was rejected earlier in the review discussion on the grounds + that it "widens the prefix match" — wrong order of operations, and worth + recording because the reasoning rested on the same undocumented behaviour it + was trying to avoid. Once lookups match client-side, the suffix is safe: + `name=='…-key'` does not match `…-key-20260812T142317Z`, and the explicit + `starts_with` shows both with their timestamps. + + **The suffix is to the second, not to the day** *(corrected 2026-08-12, later + the same review)*. It was written `date -u +%Y%m%d` first, and both this entry + and the runbook then claimed two keys "never" collide — which day-granularity + does not deliver. The case it fails is not a corner: the likeliest rotation is + not a scheduled one but a leak, rotated within the hour, and rotated again + when the first replacement goes to the wrong inbox. That is the same day, so a + day suffix reproduces exactly the duplicate-name state the suffix exists to + prevent — and it would do so at the moment the operator is least able to + reason carefully. `%Y%m%dT%H%M%SZ` costs one format string and makes the + absolute claim true. + + Also written down while here, as cheap insurance rather than a finding: the + `discord-` prefix is reserved for [[0160]]'s keys and manual keys must never + use it. [[0164]] already tests prefix collisions *within* the self-service + namespace (snowflakes are 17–19 digits, so one user id can prefix another); + nothing tested across the two namespaces because nobody had declared them + disjoint. + +9. **The runbook's second half re-exports the AWS profile, and guards the one + remaining `None`** *(2026-08-12, same review)*. "Change or revoke" states in its + own first sentence that it runs in a shell with none of step 1's variables, then + re-exported only `CUSTOMER` — so against a different default profile every + lookup in it quietly targets the wrong account. + + Worth recording *why this was fixed second*. Taken on its own the finding is + low: nothing mutates in the wrong account, because `--usage-plan-id None` is + rejected. It was deferred once on that basis. What changed is Design Decision + 8: replacing the key lookup with a printed table removed one of the two silent + paths outright — a wrong profile now shows up as an empty table, which an + operator sees. That left `PLAN_ID` as the only place a failure could still pass + itself off as a value, so the guard stopped being a patch over a snippet that + hid failures and became the smaller job of bringing one path up to the standard + the other already met. + + The AWS CLI detail is the reusable part, and it is verified rather than + asserted: `--output text` prints the literal four-character string `None` for an + empty result, so `[ -z "$VAR" ]` does **not** catch it. Expect it again in + [[0160]]'s operational procedures. The guard warns rather than exits — the + snippets are pasted into an interactive shell, where `return` is invalid and + `exit` would close the operator's terminal. + + **It also unsets the variable** *(added 2026-08-12, later the same review)*. + Warning alone left `PLAN_ID="None"` live in the shell, and a warning is exactly + what gets scrolled past — the printed key table immediately below it is what + the operator is looking at. Unsetting keeps the interactive-shell constraint + above intact while moving the failure to the point of use: the next command + that needs a plan id fails on an empty argument instead of asking AWS about a + plan named `None` several snippets later. This is the same principle as + Design Decision 8 — a failure must not be able to present itself as a value. + + Also corrected here: the note explaining the new lookup still described + `--name-query` as "a cheap server-side prefilter", which the rewritten snippet + no longer uses at all. + ## Notes - **Sequencing with [[0121]] — the one-plan outcome makes this sharper, not diff --git a/lore/1-tasks/backlog/0160_FEATURE_onboarding-backend-endpoints.md b/lore/1-tasks/backlog/0160_FEATURE_onboarding-backend-endpoints.md index fdb3ff14..331f5baf 100644 --- a/lore/1-tasks/backlog/0160_FEATURE_onboarding-backend-endpoints.md +++ b/lore/1-tasks/backlog/0160_FEATURE_onboarding-backend-endpoints.md @@ -123,9 +123,9 @@ period, one quota. response would hand one user another user's key. Set `cachingEnabled: false` on every portal method and `Cache-Control: no-store` on every response. - **IAM scoped to the resources it needs**: `apigateway:POST` on `/apikeys` - and on `/usageplans/{selfServicePlanId}/keys`, `GET` on **`/apikeys`** (the + and on `/usageplans/{pricingApiFreePlanId}/keys`, `GET` on **`/apikeys`** (the collection — the reconciler lists it), on `/apikeys/{id}` and on - `/usageplans/{selfServicePlanId}/usage`, `DELETE` on `/apikeys/{id}`. Nothing + `/usageplans/{pricingApiFreePlanId}/usage`, `DELETE` on `/apikeys/{id}`. Nothing wildcard — Tranche 3 AC 6 audits exactly this. The collection-level `GET` is not optional: without it every issue path and the reveal-path recovery below fail at runtime with `AccessDenied`, because diff --git a/lore/1-tasks/backlog/0171_RESEARCH_settle-undocumented-discord-and-aws-behaviours.md b/lore/1-tasks/backlog/0171_RESEARCH_settle-undocumented-discord-and-aws-behaviours.md index e1e4995e..a655b84f 100644 --- a/lore/1-tasks/backlog/0171_RESEARCH_settle-undocumented-discord-and-aws-behaviours.md +++ b/lore/1-tasks/backlog/0171_RESEARCH_settle-undocumented-discord-and-aws-behaviours.md @@ -89,6 +89,10 @@ nobody has written yet — the cheapest possible moment to check them. **Action:** measure it, and either way keep the client-side exact-match filter — it is load-bearing, not defence in depth. Update the reasoning in both tasks so nobody later "simplifies" the filter away. + `docs/runbooks/manual-api-key-tier.md` reasons about this too ([[0157]]): it + dropped `--name-query` for client-side `starts_with` on the same grounds, and + explicitly invites re-adding it as a server-side prefilter. Whatever is + measured here decides whether that invitation stands. 7. **The monthly quota reset instant and timezone.** **This is a correction.** [[0157]]/[[0158]]/[[0160]] state "1st of the month, @@ -120,8 +124,9 @@ nobody has written yet — the cheapest possible moment to check them. - [ ] Items 1-4 measured against `stellar_test` and a screening-off scratch guild, results written down with the date - [ ] Consent screen captured with and without `guilds.members.read` -- [ ] `nameQuery` semantics measured; [[0158]] and [[0160]] updated so the - exact-match filter is documented as load-bearing rather than redundant +- [ ] `nameQuery` semantics measured; [[0158]], [[0160]] and + `docs/runbooks/manual-api-key-tier.md` updated so the client-side filter is + documented as load-bearing rather than redundant - [ ] Quota rollover instant measured; [[0157]]/[[0158]]/[[0160]] corrected to stop presenting "00:00 UTC on the 1st" as AWS-documented behaviour - [ ] `enabled=false` effect on usage counters measured and recorded against From 30c501c47596ed5221279ccc2e3c0181668a6532 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:47:07 +0200 Subject: [PATCH 7/9] docs(lore-0157): close the last blast-radius question on the key rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GetUsage` says nothing about a holder who has not called in 30 days, and two SCF documents hand a reviewer a curl with `x-api-key` and tell them to run it. Had the key's value gone out with them, the rotation would break a grant deliverable on deploy. It never did — the docs carry `$KEY` as a placeholder the reader exports themselves. Recorded because the question is re-askable from those documents alone and the answer is not derivable from them. --- ..._FEATURE_default-key-limits-1rps-monthly-quota.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md index c3d93659..77cc985d 100644 --- a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md +++ b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md @@ -95,6 +95,18 @@ deploy, every holder of `prices-production-partner-key` is cut off. Usage over the preceding 30 days was 14 requests across 4 days, all our own verification curls, which is why that is acceptable rather than a blocker. +**One holder that argument cannot see, checked separately** *(2026-08-12, from +review of [[PR 195]])*. `GetUsage` proves nothing about a holder who has not +called in 30 days, and this repo has two documents — `docs/scf/milestone-1-evidence.md` +and `docs/scf/milestone-1-video-scenario.md` — that hand an SCF reviewer a curl +with `x-api-key` and tell them to run it. If the key's value had ever gone out +with them, the reviewer would be exactly such a holder and the rotation would +break a grant deliverable on deploy. **It never did** — confirmed by Adam +2026-08-12; the evidence docs carry `$KEY` as a placeholder the reader exports +themselves, never a value. So the blast radius is what the usage figure says it +is. Worth writing down because the question is re-askable from the docs alone, +and the answer is not derivable from them. + ## Context `api-gateway-stack.ts` creates one `UsagePlan` (`prices--partner-plan`) From 51e86f5b1f801268e5cac1e942c798a6974c3b36 Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:38:56 +0200 Subject: [PATCH 8/9] docs(lore-0157): fix the contradictory throttle comment The block said dropping the default method entry would leave "only the per-key usage-plan limit", then ten lines later correctly named the account-level limit sitting above it. Found in review of PR 195. Both throttles are now named in the first sentence, so the paragraph agrees with itself. --- infra/src/lib/stacks/api-gateway-stack.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/infra/src/lib/stacks/api-gateway-stack.ts b/infra/src/lib/stacks/api-gateway-stack.ts index 938848f6..e3a23a44 100644 --- a/infra/src/lib/stacks/api-gateway-stack.ts +++ b/infra/src/lib/stacks/api-gateway-stack.ts @@ -257,8 +257,9 @@ export class ApiGatewayStack extends cdk.Stack { // Assigning `methodSettings` wholesale REPLACES the default entry CDK // renders from `deployOptions.throttlingRateLimit/Burst`, which would drop - // it entirely (only the per-key usage-plan limit would remain). Re-declare - // it here so the §2.1 figure survives. + // it entirely — leaving the per-key usage-plan limit, and above it the + // account-level limit, as the only throttles on every method. Re-declare it + // here so the §2.1 figure survives. // // Despite the name, this is NOT an aggregate ceiling across the stage: a // per-API per-stage limit is applied per method, so this grants every method From a3e8c56431c6bb46856baeb75fbf1c098c9fa57a Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:39:00 +0200 Subject: [PATCH 9/9] docs(lore-0157): warn that broadening is silent on deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--require-approval broadening` prompts on IAM and security-group widening, not on deletion or replacement — so the deploy that rotates the production API key destroys it without asking. That hazard was documented only in the PR description and this task file: one disappears on merge, the other is archived. The construct comment warns whoever edits those two lines, which is not the person this catches — they are deploying an unrelated stack. The note goes above `deploy-production` instead, and asks for `make diff-production` read for removals. Found in review of PR 195. --- infra/Makefile | 9 +++++++++ ...RE_default-key-limits-1rps-monthly-quota.md | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/infra/Makefile b/infra/Makefile index b75d4325..e5733ff5 100644 --- a/infra/Makefile +++ b/infra/Makefile @@ -54,6 +54,15 @@ flush-production-cache: --stage-name production @echo "production stage cache flushed" +# `--require-approval broadening` prompts on IAM/security-group widening ONLY. It +# does not prompt on resource deletion or replacement, so a deploy can destroy a +# live resource without asking — and an API key destroyed this way takes its value +# with it, cutting off every holder the moment the deploy succeeds. Task 0157 +# rotated `prices-production-partner-key` exactly this way, deliberately. +# +# So `make diff-production` before any production deploy, and read it for removals +# rather than skimming it for additions. This applies to every deploy target below +# too; it lives here because this is the one that deploys all stacks at once. deploy-production: build npx cdk --app "$(PRODUCTION_APP)" deploy --all --require-approval broadening $(MAKE) flush-production-cache diff --git a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md index 77cc985d..e76578a7 100644 --- a/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md +++ b/lore/1-tasks/active/0157_FEATURE_default-key-limits-1rps-monthly-quota.md @@ -567,6 +567,24 @@ conclusion stands on the remaining arguments. `--name-query` as "a cheap server-side prefilter", which the rewritten snippet no longer uses at all. +10. **The rotation hazard is written into `infra/Makefile`, because that is the + only place that outlives this task** *(2026-08-12, from review of [[PR 195]])*. + `make deploy-production` runs `cdk deploy --all --require-approval broadening`, + which prompts on IAM and security-group widening and **not** on deletion or + replacement — so the deploy that rotates this key destroys it without asking. + + The hazard itself was well documented, in the PR description and in this file. + Neither survives: one disappears on merge, the other is archived. The comment + at the `PricingApiFreeApiKey` construct warns whoever edits *those two lines*, + which is not the person this catches — that person is running a routine deploy + of an unrelated stack and never opens `api-gateway-stack.ts`. + + So the note sits above `deploy-production` and asks for `make diff-production` + read for removals rather than skimmed for additions. Deliberately not phrased + around this key: after deploy the rotation is done, and what remains true is + the general one — an approval mode that is silent on deletion means the diff + is the only place a destroyed resource ever announces itself. + ## Notes - **Sequencing with [[0121]] — the one-plan outcome makes this sharper, not