From 412329d834f7c260880e430b77b2ad64262fe5ca Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Sat, 24 Oct 2026 17:51:39 +1300 Subject: [PATCH 1/4] feat(converter): ship as a .zip with a free-tier kill switch - Converter is now a python3.13 arm64 .zip built by converter/Makefile (Pillow's manylinux_2_28_aarch64 wheel), so the ECR repo and its storage cost go away. CI imports the built package on arm64 to prove JP2/TIFF decoding is present. infra/bootstrap.yaml keeps only the artefact bucket. - New ConverterGuardFunction (guard/) switches the converter off by setting its reserved concurrency to 0 when the 5-minute spike alarm fires, or when month-to-date Lambda usage nears the always-free allowance, and back on hourly once usage and the alarm are clear. Every switch is emailed. - Function timeout 60 -> 120 s plus a 90 s whole-download deadline: NDHA streams some masters at under 1 MB/s, and FL73848659 (45.8 MB) timed out silently. Co-Authored-By: Claude Opus 5.5 --- .claude/rules/build-test-deploy.md | 140 ++++++--------------- .claude/rules/converter.md | 32 +++++ .github/workflows/ci-cd.yml | 39 +++--- CLAUDE.md | 41 +++---- converter/Dockerfile | 13 -- converter/Makefile | 10 ++ converter/app.py | 26 ++-- converter/local_server.py | 2 +- converter/tests/test_app.py | 22 +++- docs/ACCESS-CONTROL.md | 3 +- guard/Makefile | 5 + guard/app.py | 152 +++++++++++++++++++++++ guard/pytest.ini | 2 + guard/requirements-dev.txt | 2 + guard/tests/test_app.py | 191 +++++++++++++++++++++++++++++ infra/bootstrap.yaml | 52 +------- template.yaml | 118 +++++++++++++++--- 17 files changed, 620 insertions(+), 230 deletions(-) delete mode 100644 converter/Dockerfile create mode 100644 converter/Makefile create mode 100644 guard/Makefile create mode 100644 guard/app.py create mode 100644 guard/pytest.ini create mode 100644 guard/requirements-dev.txt create mode 100644 guard/tests/test_app.py diff --git a/.claude/rules/build-test-deploy.md b/.claude/rules/build-test-deploy.md index 24d5f9f..1e7ec01 100644 --- a/.claude/rules/build-test-deploy.md +++ b/.claude/rules/build-test-deploy.md @@ -51,9 +51,11 @@ same collections file and only one writer can hold the mutex ## SAM deploy (the real deploy path) -Stack name `nzimageapi`, region `ap-southeast-2`. Needs Docker (the Swift Lambda's Makefile -target and the converter's container image both build via Docker) — Docker commands need -the sandbox disabled here, since the Docker socket isn't sandbox-allowlisted. +Stack name `nzimageapi`, region `ap-southeast-2`. Needs Docker for the Swift Lambda's Makefile +target — Docker commands need the sandbox disabled here, since the Docker socket isn't +sandbox-allowlisted. The converter and guard are .zips built by their own Makefiles +(`converter/Makefile`, `guard/Makefile`) with no Docker; the converter's `pip install` needs +network access to PyPI. ```bash sam build @@ -75,117 +77,49 @@ Function URL wiring) — prefer `sam build && sam deploy` for anything touching ## Deploy artefact retention (cost control) -Every `sam deploy` leaves permanent artefacts behind. Left unmanaged these are the account's -only real recurring cost, and they grow with every deploy while the running stack itself -(Lambda, API Gateway, CloudWatch, SNS, SQS) sits at $0.00. +Every `sam deploy` uploads the packaged code (the Swift Lambda zip ~44 MB, the converter zip +~22 MB, the guard) and templates to S3, and versioning is on. Left unmanaged, deploy artefacts +were the account's only real recurring cost while the running stack sat at $0.00: by 2026-08-24 +there were 29 converter images in ECR (~5.5 GB) and 1.45 GB in S3. -Two things accumulate: +The converter was a container image until October 2026. ECR's free storage is a 12-month +offer this account no longer has, so even three retained images cost about $0.06 a month. It is +now a .zip, whose code storage Lambda includes for free, and the ECR repository is gone. -- **The converter's ECR repo.** Each deploy pushes a fresh ~195 MB image under a unique tag - (`jp2converterfunction--latest`), so nothing is ever overwritten. Only the one tag - referenced by the deployed `Jp2ConverterFunction` is live. -- **The SAM artefact bucket.** Each deploy uploads the Swift Lambda zip (~44 MB) plus - packaged templates, and versioning is on. +### `infra/bootstrap.yaml` owns the artefact bucket -By 2026-08-24 that had reached 29 ECR images (~5.5 GB of manifests) and 75 S3 objects -(1.45 GB) from roughly 20 deploys since June 2026 - about $0.17/month and climbing, all of -it waste. - -### `infra/bootstrap.yaml` owns both - -Both resources, and both retention policies, are declared in **`infra/bootstrap.yaml`** and -deployed as the separate `nzimageapi-bootstrap` stack: +The bucket and its retention policy are declared in **`infra/bootstrap.yaml`** and deployed as +the separate `nzimageapi-bootstrap` stack: ```bash aws cloudformation deploy --region ap-southeast-2 --stack-name nzimageapi-bootstrap --template-file infra/bootstrap.yaml ``` -**Why a separate stack and not `template.yaml`:** `sam deploy` pushes the container image to -ECR and uploads the packaged template to S3 *before* CloudFormation runs. A repo or bucket -declared in the stack that consumes them would not exist yet at push time. They have to be -created by something that runs first, hence a tiny bootstrap stack deployed on its own. - -Retention is tunable via stack parameters rather than by editing policy JSON: +**Why a separate stack and not `template.yaml`:** `sam deploy` uploads the packaged code and +template to S3 *before* CloudFormation runs, so a bucket declared in the stack that consumes it +would not exist yet at upload time. -| Parameter | Default | Effect | -| --- | --- | --- | -| `ImageRetentionCount` | `3` | ECR keeps this many images. Newest is live, the rest are rollback targets. | -| `ArtifactRetentionDays` | `60` | S3 artefacts expire after this many days. Non-current versions go at 7 days, stalled multipart uploads at 7. | +`ArtifactRetentionDays` (default `60`) sets when artefacts expire; non-current versions go at 7 +days, stalled multipart uploads at 7. A stack update failing more than 60 days after the deploy +that produced an artefact cannot roll back to it, which is well outside any realistic rollback +window here. The bucket carries `DeletionPolicy: Retain` / `UpdateReplacePolicy: Retain`. -The 60-day S3 window is deliberately generous. It does mean a stack update failing more than -60 days after the deploy that produced an artefact cannot roll back to it, which is well -outside any realistic rollback window here. +### The wiring -Both resources carry `DeletionPolicy: Retain` / `UpdateReplacePolicy: Retain`, so deleting or -replacing the bootstrap stack will not take the artefacts (or a bucket full of objects, -which would fail the delete anyway) with it. +The gitignored `samconfig.toml` points the main stack at the bucket with +`s3_bucket = "nzimageapi-sam-artifacts-686865771242-ap-southeast-2"` (which **replaces** +`resolve_s3 = true`; the two are mutually exclusive, and `resolve_s3` is what made SAM +auto-create its own unmanaged bucket). It no longer needs an `image_repositories` entry. -### The wiring in `samconfig.toml` +`.github/workflows/ci-cd.yml` cannot read `samconfig.toml`, so its deploy job reads the bucket +from the bootstrap stack's `ArtifactBucketName` output and passes it as `--s3-bucket`, failing +with a pointer to the bootstrap-deploy command if the stack or output is missing. It reads the +stack rather than a GitHub variable on purpose: a stale `CONVERTER_ECR_REPO` variable once kept CI +deploying into superseded, unmanaged stores. Never reintroduce `--resolve-s3` there. -The gitignored `samconfig.toml` points the main stack at the bootstrap stack's outputs: - -```toml -s3_bucket = "nzimageapi-sam-artifacts-686865771242-ap-southeast-2" -image_repositories = ["Jp2ConverterFunction=686865771242.dkr.ecr.ap-southeast-2.amazonaws.com/nzimageapi/jp2converter"] -``` - -`s3_bucket` **replaces** `resolve_s3 = true` - the two are mutually exclusive, and -`resolve_s3` is what made SAM auto-create its own unmanaged bucket. Because `samconfig.toml` -is gitignored, this wiring is the one part that is not in version control; re-read the two -outputs after any bootstrap-stack change: - -```bash -aws cloudformation describe-stacks --region ap-southeast-2 --stack-name nzimageapi-bootstrap --query 'Stacks[0].Outputs' -``` +### Prefer .zip over container images -### The wiring in CI - -`.github/workflows/ci-cd.yml` cannot read the gitignored `samconfig.toml`, so its deploy job -resolves the same two values itself, in a `Resolve bootstrap artefact stores` step that queries -the bootstrap stack's outputs and feeds them to `sam deploy` as `--s3-bucket` and -`--image-repositories`. The step fails the job with a pointer to the bootstrap-deploy command if -the stack is missing or either output is absent, so a misconfigured deploy never reaches the -push. - -They are read from the stack rather than from GitHub repo/environment variables on purpose. The -job previously used `--resolve-s3` plus a `CONVERTER_ECR_REPO` environment variable, and that -variable still held the *pre-migration* companion-stack repo, so every CI deploy kept pushing a -~195 MB image into the unmanaged repo and a zip into a SAM-auto-created bucket while local deploys -correctly used the bootstrap stores. Reading the stack removes the copy that can go stale. If you -add another artefact store, add its output to `infra/bootstrap.yaml` and read it in the same step. - -`CONVERTER_ECR_REPO` is no longer referenced by anything and can be deleted from the repository's -`production` environment. - -### Superseded resources (retired 2026-08-25) - -The pre-migration artefact stores are **gone**. Both were deleted on 2026-08-25, once a CI -deploy against the bootstrap stores was confirmed: - -- ECR `nzimageapi40221342/jp2converterfunctione92cbfdcrepo` and its owning SAM-generated - `nzimageapi-40221342-CompanionStack` (3 remaining images deleted first, since the stack's - `AWS::ECR::Repository` sets neither `EmptyOnDelete` nor a retain policy and would otherwise - fail to delete). -- S3 `aws-sam-cli-managed-default-samclisourcebucket-tfqwkahyri86` and its owning - `aws-sam-cli-managed-default` stack (158 object versions and delete markers purged first; - `aws s3 rm --recursive` alone is not enough on a versioned bucket, it only writes delete - markers). - -The account now holds exactly one ECR repository and one S3 bucket, both from -`infra/bootstrap.yaml`. - -The practical consequence: CloudFormation cannot roll the `nzimageapi` stack back to any -version deployed before 2026-08-25, because those templates and images no longer exist. Every -artefact from that date on lives in the bootstrap stores under their retention policies. - -They lasted as long as they did because CI kept writing to both of them until the deploy job was -pointed at the bootstrap stack (see "The wiring in CI" above), which is why their final contents -were newer than the migration date suggested. - -### When adding a new container-image Lambda - -A new `PackageType: Image` function does **not** automatically get a managed repo any more, -because `image_repositories` is now explicit. Add an `AWS::ECR::Repository` for it in -`infra/bootstrap.yaml` (copy `ConverterRepository`, including its `LifecyclePolicy`), deploy -the bootstrap stack, then add the new `=` entry to -`image_repositories`. Skipping the lifecycle policy is how this problem started. +Keep Lambdas as .zips (250 MB unzipped limit) so there is nothing in ECR to pay for. If a function +ever has to be a container image, it needs an `AWS::ECR::Repository` with a lifecycle policy in +`infra/bootstrap.yaml` (see git history before October 2026 for the old `ConverterRepository`), +plus an explicit `image_repositories` entry in `samconfig.toml` and `--image-repositories` in CI. diff --git a/.claude/rules/converter.md b/.claude/rules/converter.md index 604cb5f..99de166 100644 --- a/.claude/rules/converter.md +++ b/.claude/rules/converter.md @@ -11,6 +11,38 @@ CORS open) that downloads a master image (JP2 via OpenJPEG, TIFF via libtiff) an displayable JPEG capped at `MAX_DIM` (default 4000px). It exists because the Swift Lambda is a pure URL builder and never decodes image bytes itself. +It is a python3.13 arm64 **.zip** built by `converter/Makefile`, which installs Pillow's +`manylinux_2_28_aarch64` wheel (it bundles OpenJPEG and libtiff) on any build host. CI's deploy job +imports the built package on arm64 Python 3.13 and fails if either codec is missing. + +## Timeouts + +NDHA streams some masters to the Lambda at under 1 MB/s. The function timeout is 120 s and +`download_image` gives up after `DOWNLOAD_DEADLINE` (90 s) for the whole download, on top of the +per-socket `DOWNLOAD_TIMEOUT`, so a slow source ends as a logged `502 download failed` instead of +a silent function timeout. FL73848659 (45.8 MB, 7356×5010) is the known slow master. + +## Kill switch (`guard/`) + +The Function URL is public, so `ConverterGuardFunction` (`guard/app.py`) keeps the converter +inside Lambda's always-free allowance. It switches the converter off by setting its reserved +concurrency to 0 (every call is throttled, nothing is billed), and back on by deleting the +reservation: + +- **Spike:** `nzimageapi-converter-invocations-high` (over `ConverterSpikeThreshold`, default 100, + invocations in 5 minutes) emails `nzimageapi-cost-alarms` and, through the same topic, switches + the converter off. +- **Hourly:** month-to-date usage from CloudWatch is checked against `GuardMaxGbSeconds` (320,000 + of the 400,000 free), `GuardMaxRequests` (800,000 of 1,000,000) and + `GuardMaxConverterInvocations` (20,000, which keeps data transfer out under the free 100 GB at + up to ~4 MB a response). Over any limit, it switches off. Under all of them with the spike alarm + OK, it switches back on, which is how it recovers after a spike or at the start of a month. + +Every switch is emailed to the cost-alarm topic. While the converter is off, converter URLs fail +and the slideshow skips those images. To switch it back on by hand: +`aws lambda delete-function-concurrency --region ap-southeast-2 --function-name ` +(the next hourly run turns it off again if a limit is still reached). + ## HMAC signer/verifier contract - Swift side: `URLProcessor.signedConverterURL(for:)` (`Sources/NZImageApiLambda/Helpers/URLProcessor.swift`) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 7fd255f..97f5e86 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -28,18 +28,21 @@ jobs: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: - python-version: "3.14" # matches converter Lambda runtime + python-version: "3.13" # matches the converter and guard Lambda runtime - name: Install dev deps - run: pip install -r converter/requirements-dev.txt - - name: pytest + run: pip install -r converter/requirements-dev.txt -r guard/requirements-dev.txt + - name: pytest (converter) working-directory: converter run: pytest + - name: pytest (guard) + working-directory: guard + run: pytest deploy: name: Build & deploy (SAM) needs: [swift-test, python-test] # deploy blocked until BOTH pass if: github.event_name == 'push' && github.ref == 'refs/heads/main' - runs-on: ubuntu-24.04-arm # native arm64 Docker: fast Swift cross-compile + converter image + runs-on: ubuntu-24.04-arm # native arm64 Docker for the Swift cross-compile; arm64 Python for the converter codec check environment: production # gives an optional approval gate; scopes OIDC sub (see setup) permissions: id-token: write # required for OIDC @@ -56,13 +59,21 @@ jobs: - uses: aws-actions/setup-sam@89ddb14d60e682855e3fea4be85b3c56485de310 # v3 with: use-installer: true # arch-independent SAM CLI install + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 + with: + python-version: "3.13" # same as the Lambda runtime, so the codec check imports the built wheel - name: sam build - run: sam build # Swift via Makefile+Docker; converter via Docker image - # The artefact stores live in the separate `nzimageapi-bootstrap` stack (infra/bootstrap.yaml), - # which owns their retention lifecycle policies. Read them from that stack's outputs rather - # than from a hand-maintained repo variable: a stale variable silently sends deploys back to - # an unmanaged store, which is exactly what happened before this step existed. - - name: Resolve bootstrap artefact stores + run: sam build # Swift via Makefile+Docker; converter and guard via their Makefiles + # The converter .zip carries Pillow's manylinux aarch64 wheel. Prove the build has the two + # codecs it exists for, on the same architecture and Python as the Lambda, before deploying. + - name: Check converter codecs + run: | + PYTHONPATH=.aws-sam/build/Jp2ConverterFunction python -c "from PIL import features; assert features.check('jpg_2000'), 'OpenJPEG/JP2 NOT enabled in Pillow'; assert features.check('libtiff'), 'libtiff NOT enabled in Pillow'; print('JP2 and TIFF decoding available')" + # The artefact bucket lives in the separate `nzimageapi-bootstrap` stack (infra/bootstrap.yaml), + # which owns its retention lifecycle policy. Read it from that stack's outputs rather than + # from a hand-maintained repo variable: a stale variable silently sends deploys back to an + # unmanaged store, which is exactly what happened before this step existed. + - name: Resolve bootstrap artefact bucket id: bootstrap run: | set -euo pipefail @@ -74,22 +85,18 @@ jobs: --stack-name nzimageapi-bootstrap \ --query 'Stacks[0].Outputs' --output json) || outputs='[]' bucket=$(echo "$outputs" | jq -r '[.[]? | select(.OutputKey=="ArtifactBucketName") | .OutputValue] | first // ""') - repo=$(echo "$outputs" | jq -r '[.[]? | select(.OutputKey=="ConverterRepositoryUri") | .OutputValue] | first // ""') - if [ -z "$bucket" ] || [ "$bucket" = "null" ] || [ -z "$repo" ] || [ "$repo" = "null" ]; then - echo "::error::nzimageapi-bootstrap did not return both ArtifactBucketName and ConverterRepositoryUri. Deploy the bootstrap stack first: aws cloudformation deploy --region ap-southeast-2 --stack-name nzimageapi-bootstrap --template-file infra/bootstrap.yaml" + if [ -z "$bucket" ] || [ "$bucket" = "null" ]; then + echo "::error::nzimageapi-bootstrap did not return ArtifactBucketName. Deploy the bootstrap stack first: aws cloudformation deploy --region ap-southeast-2 --stack-name nzimageapi-bootstrap --template-file infra/bootstrap.yaml" exit 1 fi echo "bucket=$bucket" >> "$GITHUB_OUTPUT" - echo "repo=$repo" >> "$GITHUB_OUTPUT" echo "Artefact bucket: $bucket" - echo "Converter repo: $repo" - name: sam deploy run: | sam deploy \ --stack-name nzimageapi \ --region ap-southeast-2 \ --s3-bucket ${{ steps.bootstrap.outputs.bucket }} \ - --image-repositories Jp2ConverterFunction=${{ steps.bootstrap.outputs.repo }} \ --capabilities CAPABILITY_IAM \ --no-confirm-changeset \ --no-fail-on-empty-changeset \ diff --git a/CLAUDE.md b/CLAUDE.md index 8f715ef..f3b17e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,12 +12,13 @@ Lambda converts awkward master formats (JP2/TIFF) to JPEG on demand. - `Sources/NZImageApiLambda/` — the Swift HTTP API Lambda (the product). - `Sources/Testing/` — CLI tools: `CollectionTester`, `CollectionReviewer`, `CollectionLister`, `ImageResolutionChecker`, shared `LambdaTesting` lib. -- `converter/` — Python Pillow Lambda (`app.py`) that decodes JP2/TIFF masters to JPEG behind a signed Function URL. +- `converter/` — Python Pillow Lambda (`app.py`, a .zip built by `converter/Makefile`) that decodes JP2/TIFF masters to JPEG behind a signed Function URL. +- `guard/` — the converter's free-tier kill switch Lambda (`app.py`): sets the converter's concurrency to 0 on a usage spike or when month-to-date usage nears the free tier. - `collection-reviewer-web/` — Next.js 15 app for triaging DigitalNZ collections; its dev server also boots the Swift Lambda. - `Research/highres/` — durable record of the (complete, 52-collection) high-resolution sweep; `progress.json` is the authoritative machine record. - `docs/` — access-control design doc and ADRs. - `scripts/` — legacy pre-SAM build/deploy scripts (`sam` is now the real deploy path). -- `template.yaml` — the SAM stack definition for both Lambdas. +- `template.yaml` — the SAM stack definition for the Lambdas, alarms, budget and kill switch. ## Architecture essentials @@ -97,9 +98,12 @@ Secrets live in gitignored `.env`, `samconfig.toml`, and `.consumer-secrets/` changeset, get explicit confirmation, then re-run with `--no-confirm-changeset`. - The reviewer web app and the `CollectionReviewer` CLI both write the same collections file — never run both writers at once (see `collection-reviewer-web/lib/collectionsFile.ts`). -- `template.yaml` deliberately sets **no** `ReservedConcurrentExecutions` on either - function (the account's Lambda concurrency quota is already only 10, fully unreserved) — - don't describe concurrency as reserved/limited per-function. +- `template.yaml` deliberately sets **no** `ReservedConcurrentExecutions` on any + function (the account's Lambda concurrency quota is only 10 and must stay fully unreserved). + The one exception is runtime-only: `ConverterGuardFunction` sets the converter's reservation + to **0** to switch it off, and deletes it to switch it back on. A converter returning 429 / + throttling everything is usually the kill switch, not a bug: check the guard's logs and the + `nzimageapi-cost-alarms` emails. - **Never let a `URLSession` (or an Alamofire `Session`, which owns one) be deallocated in the Lambda.** On Linux, `URLSession.deinit` tears down swift-corelibs-foundation's libcurl `_MultiHandle`, whose own `deinit` re-enters curl's timer callback and takes a strong reference @@ -114,22 +118,15 @@ Secrets live in gitignored `.env`, `samconfig.toml`, and `.consumer-secrets/` `swift build`) fail DNS resolution under the command sandbox even for allowlisted hosts — same class of issue as the Docker gotcha above, but it also hits plain network calls made by a built binary, not just Docker. Run with the sandbox disabled. -- Every `sam deploy` leaves a new ~195 MB image in the converter's ECR repo and a ~44 MB - zip in the SAM artefact bucket, forever. Both stores, and their retention lifecycle - policies (ECR: keep last 3 images; S3: expire at 60 days), are declared in - [`infra/bootstrap.yaml`](infra/bootstrap.yaml) and deployed as the separate - `nzimageapi-bootstrap` stack. They are **deliberately not in `template.yaml`**: `sam deploy` - pushes the image and uploads the packaged template *before* CloudFormation runs, so a repo - or bucket in the consuming stack would not exist yet at push time. `samconfig.toml` wires - the main stack to that stack's outputs via `s3_bucket` and `image_repositories` (`s3_bucket` - replaces `resolve_s3`; they are mutually exclusive). `samconfig.toml` is gitignored, so CI - cannot read it: the deploy job in `.github/workflows/ci-cd.yml` resolves the same two values - from the bootstrap stack's outputs at deploy time and passes them as `--s3-bucket` and - `--image-repositories`. Never reintroduce `--resolve-s3` or a hand-maintained repo variable - there; a stale variable is what kept CI deploying into the superseded stores. Adding a new - container-image Lambda - means adding its repo to `infra/bootstrap.yaml` too - skipping the lifecycle policy is how - this problem started. Full detail: +- The converter is a **.zip**, not a container image, so there is no ECR repo to pay for. Its + `converter/Makefile` installs Pillow's `manylinux_2_28_aarch64` wheel with `pip --platform`, so + `sam build` gives the same artefact on macOS and in CI; CI then imports the built package on + arm64 Python 3.13 to prove JP2 and TIFF decoding are present. Every `sam deploy` still uploads + zips to the SAM artefact bucket declared in [`infra/bootstrap.yaml`](infra/bootstrap.yaml) + (separate `nzimageapi-bootstrap` stack, 60-day expiry). It is **deliberately not in + `template.yaml`**: `sam deploy` uploads before CloudFormation runs, so a bucket in the + consuming stack would not exist yet. `samconfig.toml` points at it with `s3_bucket`; CI reads + it from the bootstrap stack's outputs. Full detail: [`.claude/rules/build-test-deploy.md`](.claude/rules/build-test-deploy.md#deploy-artefact-retention-cost-control). ## Where to read more @@ -138,7 +135,7 @@ Secrets live in gitignored `.env`, `samconfig.toml`, and `.consumer-secrets/` - [`.claude/rules/build-test-deploy.md`](.claude/rules/build-test-deploy.md) — full build/test/deploy command reference and gotchas. - [`.claude/rules/converter.md`](.claude/rules/converter.md) — the Python converter Lambda: contract, allowlist, local dev. - `README.md` — end-user API reference and setup. -- `infra/bootstrap.yaml` — deploy-artefact stores (converter ECR repo, SAM artefact bucket) and their retention policies. +- `infra/bootstrap.yaml` — the SAM artefact bucket and its retention policy. - `docs/ACCESS-CONTROL.md`, `docs/adr/0001-free-per-consumer-secrets.md` — auth model and cost-control design. - `Research/highres/README.md`, `recipes.md`, `progress.json` — per-collection high-res findings. - `Sources/Testing/CollectionTester/README-CollectionTester.md` — CollectionTester options. diff --git a/converter/Dockerfile b/converter/Dockerfile deleted file mode 100644 index 426bd11..0000000 --- a/converter/Dockerfile +++ /dev/null @@ -1,13 +0,0 @@ -FROM public.ecr.aws/lambda/python:3.13 - -# Pillow's manylinux wheel bundles libopenjp2. Install it, then HARD-ASSERT at build -# time that JPEG 2000 decode is actually enabled — if it is not, the build fails here -# (a deterministic guarantee, rather than discovering it at runtime). -COPY requirements.txt . -RUN pip install --no-cache-dir -r requirements.txt -RUN python -c "from PIL import features; assert features.check('jpg_2000'), 'OpenJPEG/JP2 NOT enabled in Pillow'" -RUN python -c "from PIL import features; assert features.check('libtiff'), 'libtiff NOT enabled in Pillow'" - -COPY app.py ${LAMBDA_TASK_ROOT}/ - -CMD ["app.lambda_handler"] diff --git a/converter/Makefile b/converter/Makefile new file mode 100644 index 0000000..95371de --- /dev/null +++ b/converter/Makefile @@ -0,0 +1,10 @@ +# SAM `BuildMethod: makefile` target for the converter .zip (python3.13 on arm64). +# Installs Pillow's manylinux aarch64 wheel whatever the build host's OS or architecture, so a +# macOS build produces the same artefact as CI. Pillow 12+ only publishes manylinux_2_27/2_28 +# tags; the manylinux2014 tag in AWS's packaging guide finds no wheel. Lambda's AL2023 runtime +# has glibc 2.34, which satisfies 2_28. +build-Jp2ConverterFunction: + python3 -m pip install --quiet --no-compile --target "$(ARTIFACTS_DIR)" \ + --platform manylinux_2_28_aarch64 --implementation cp --python-version 3.13 \ + --only-binary=:all: -r requirements.txt + cp app.py "$(ARTIFACTS_DIR)/" diff --git a/converter/app.py b/converter/app.py index 63277fe..5abe68e 100644 --- a/converter/app.py +++ b/converter/app.py @@ -54,9 +54,14 @@ # Browser UA: the NDHA FL `dps_func=stream` endpoint is stateless (no cookies needed); the # Feilding download endpoint also serves anonymously for public-rights records. USER_AGENT = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36" -# Large masters (≈60 MB JP2, ≈75 MB TIFF) need headroom to download; stays well under the -# 60 s Lambda timeout once the (fast, same-region) S3/NDHA fetch completes. +# DOWNLOAD_TIMEOUT is per socket operation, so on its own it never stops a slow but steady +# download. NDHA streams some masters to the Lambda at under 1 MB/s (FL73848659, 45.8 MB, ran +# into the old 60 s function timeout with nothing logged), hence the whole-download deadline, +# set below the 120 s function timeout so a slow source fails as a logged 502 with time to spare +# for decoding the ones that make it. DOWNLOAD_TIMEOUT = int(os.environ.get("DOWNLOAD_TIMEOUT", "35")) # seconds +DOWNLOAD_DEADLINE = int(os.environ.get("DOWNLOAD_DEADLINE", "90")) # seconds, whole download +DOWNLOAD_CHUNK_BYTES = 1024 * 1024 MAX_DOWNLOAD_BYTES = 110 * 1024 * 1024 # guard on the source download (masters ~60–80 MB) # Keep the *base64* response body under the Function URL 6 MB cap. base64 inflates by @@ -105,17 +110,22 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): _OPENER = build_opener(_SafeRedirectHandler) -def download_image(url, max_bytes=MAX_DOWNLOAD_BYTES, timeout=DOWNLOAD_TIMEOUT): - """Download the source image, enforcing a byte ceiling and timeout.""" +def download_image(url, max_bytes=MAX_DOWNLOAD_BYTES, timeout=DOWNLOAD_TIMEOUT, deadline=DOWNLOAD_DEADLINE): + """Download the source image, enforcing a byte ceiling, a socket timeout and a total deadline.""" _reject_if_unsafe(url) request = Request(url, headers={"User-Agent": USER_AGENT}) + give_up_at = time.monotonic() + deadline + data = bytearray() with _OPENER.open(request, timeout=timeout) as response: # noqa: S310 (host allowlisted + SSRF-guarded) - data = response.read(max_bytes + 1) - if len(data) > max_bytes: - raise ValueError(f"source exceeds {max_bytes} byte limit") + while chunk := response.read(DOWNLOAD_CHUNK_BYTES): + data += chunk + if len(data) > max_bytes: + raise ValueError(f"source exceeds {max_bytes} byte limit") + if time.monotonic() > give_up_at: + raise ValueError(f"download still running after {deadline}s ({len(data)} bytes)") if not data: raise ValueError("empty response from source") - return data + return bytes(data) def _encode_jpeg(image, dim, quality): diff --git a/converter/local_server.py b/converter/local_server.py index 850bbfa..fb31ebc 100644 --- a/converter/local_server.py +++ b/converter/local_server.py @@ -1,6 +1,6 @@ """Local stand-in for the converter's Lambda Function URL (no AWS, dev-only). -NOT part of the deployed image (the Dockerfile only copies app.py). This faithfully +NOT part of the deployed .zip (converter/Makefile only copies app.py). This faithfully simulates a Lambda Function URL so the real app.py handler can be exercised end-to-end from the Swift Lambda without deploying: diff --git a/converter/tests/test_app.py b/converter/tests/test_app.py index 57b3ff4..2e1fdc9 100644 --- a/converter/tests/test_app.py +++ b/converter/tests/test_app.py @@ -255,10 +255,10 @@ def test_encode_jpeg_never_upscales_small_image(): class _FakeURLResponse: def __init__(self, data): - self._data = data + self._stream = BytesIO(data) def read(self, n): - return self._data[:n] + return self._stream.read(n) def __enter__(self): return self @@ -295,6 +295,24 @@ def test_download_image_returns_bytes_for_normal_response(monkeypatch): assert result == b"hello" +def test_download_image_reassembles_a_multi_chunk_body(monkeypatch): + monkeypatch.setattr(app, "DOWNLOAD_CHUNK_BYTES", 4) + _stub_download(monkeypatch, b"0123456789") + + assert app.download_image("https://example.com/x", max_bytes=100, timeout=5) == b"0123456789" + + +def test_download_image_gives_up_after_the_deadline(monkeypatch): + # A slow source that keeps sending: each chunk read takes 40 "seconds" of a 90 s deadline. + clock = iter(range(0, 1000, 40)) + monkeypatch.setattr(app.time, "monotonic", lambda: next(clock)) + monkeypatch.setattr(app, "DOWNLOAD_CHUNK_BYTES", 1) + _stub_download(monkeypatch, b"x" * 10) + + with pytest.raises(ValueError, match="after 90s"): + app.download_image("https://example.com/x", max_bytes=100, timeout=5, deadline=90) + + def test_text_response_shape(): assert app._text_response(403, "nope") == { "statusCode": 403, diff --git a/docs/ACCESS-CONTROL.md b/docs/ACCESS-CONTROL.md index 36802e6..5ac78d2 100644 --- a/docs/ACCESS-CONTROL.md +++ b/docs/ACCESS-CONTROL.md @@ -245,7 +245,8 @@ Every control below is free at this project's scale. Each caps a different failu | Stage throttling | Request **rate** at the API Gateway edge | `ThrottlingRateLimit: 10`/s, `ThrottlingBurstLimit: 20` | `template.yaml` → `ServerlessHttpApi.Properties.DefaultRouteSettings` | | Account-wide Lambda concurrency | Parallel executions across **both** functions combined | `10` (this AWS account's total quota — see below) | AWS account-level Service Quota, not this template | | CloudWatch alarm (API) | Notifies on unexpected **volume** | >200 invocations/hour → email | `NZImageApiInvocationsAlarm` | -| CloudWatch alarm (converter) | Notifies on unexpected **volume** | >100 invocations/hour → email | `Jp2ConverterInvocationsAlarm` | +| CloudWatch alarm (converter) | Notifies on, and stops, a converter **spike** | >100 invocations in 5 minutes → email + kill switch | `Jp2ConverterInvocationsAlarm` | +| Converter kill switch | Converter **spend**: sets its concurrency to 0 on a spike, or when month-to-date usage nears the free tier | 320,000 GB-s, 800,000 requests or 20,000 converter calls a month | `ConverterGuardFunction` (`guard/app.py`), see `.claude/rules/converter.md` | | Monthly budget | Notifies on any **real spend** | >$1 actual or forecasted → email | `MonthlyCostBudget` | **Reserved concurrency was planned but isn't set.** This AWS account's total Lambda concurrent- diff --git a/guard/Makefile b/guard/Makefile new file mode 100644 index 0000000..43b1570 --- /dev/null +++ b/guard/Makefile @@ -0,0 +1,5 @@ +# SAM `BuildMethod: makefile` target for the guard .zip. It has no dependencies beyond the boto3 +# bundled in the python3.13 runtime, so the build is a copy; using makefile (not SAM's +# python_pip builder) means `sam build` does not need python3.13 on the build host. +build-ConverterGuardFunction: + cp app.py "$(ARTIFACTS_DIR)/" diff --git a/guard/app.py b/guard/app.py new file mode 100644 index 0000000..e310c16 --- /dev/null +++ b/guard/app.py @@ -0,0 +1,152 @@ +"""Free-tier kill switch for the converter Lambda. + +The converter's Function URL is public, so nothing else stops a flood of (validly signed) requests +from running it past Lambda's always-free allowance. This function turns the converter off by +setting its reserved concurrency to 0: every call is then throttled before it runs, so nothing is +billed. Turning it back on deletes the reservation. + +Two triggers (template.yaml, ConverterGuardFunction): + - The spike alarm, via SNS. An ALARM notification for SPIKE_ALARM_NAME switches the converter + off straight away. Other messages on the topic (the API alarm, this function's own notices) + are ignored. + - An hourly schedule. It sums month-to-date usage from CloudWatch and switches the converter off + when any limit is reached, or back on when every limit is clear and the spike alarm is OK. + That is also how the converter comes back after a spike, or at the start of a new month. + +During a sustained flood the converter cycles: off on the alarm, back on at the next hourly run +once the alarm clears, off again. The month-to-date limits bound the total, and once one is hit the +converter stays off until the month rolls over. +""" + +import datetime +import json +import os +from dataclasses import dataclass + +import boto3 + +FUNCTION_NAME = os.environ.get("CONVERTER_FUNCTION_NAME", "") +SPIKE_ALARM_NAME = os.environ.get("SPIKE_ALARM_NAME", "") +ALERT_TOPIC_ARN = os.environ.get("ALERT_TOPIC_ARN", "") +MAX_GB_SECONDS = float(os.environ.get("MAX_GB_SECONDS", "320000")) +MAX_REQUESTS = float(os.environ.get("MAX_REQUESTS", "800000")) +MAX_CONVERTER_INVOCATIONS = float(os.environ.get("MAX_CONVERTER_INVOCATIONS", "20000")) + +_lambda = boto3.client("lambda") +_cloudwatch = boto3.client("cloudwatch") +_sns = boto3.client("sns") + + +@dataclass +class Usage: + gb_seconds: float + requests: float + converter_invocations: float + + +def limits_reached(usage): + """Names of the month-to-date limits `usage` has reached (empty when all are clear).""" + reached = [] + if usage.gb_seconds >= MAX_GB_SECONDS: + reached.append(f"compute {usage.gb_seconds:,.0f}/{MAX_GB_SECONDS:,.0f} GB-s") + if usage.requests >= MAX_REQUESTS: + reached.append(f"requests {usage.requests:,.0f}/{MAX_REQUESTS:,.0f}") + if usage.converter_invocations >= MAX_CONVERTER_INVOCATIONS: + reached.append( + f"converter calls {usage.converter_invocations:,.0f}/{MAX_CONVERTER_INVOCATIONS:,.0f}" + ) + return reached + + +def spike_alarm_fired(event): + """True when `event` is an SNS delivery of SPIKE_ALARM_NAME entering ALARM.""" + for record in event.get("Records", []): + try: + message = json.loads(record["Sns"]["Message"]) + except (KeyError, TypeError, ValueError): + continue + if message.get("AlarmName") == SPIKE_ALARM_NAME and message.get("NewStateValue") == "ALARM": + return True + return False + + +def _month_start(now): + return now.replace(day=1, hour=0, minute=0, second=0, microsecond=0) + + +def _metric_sum(metric, function_name, start, end): + response = _cloudwatch.get_metric_statistics( + Namespace="AWS/Lambda", + MetricName=metric, + Dimensions=[{"Name": "FunctionName", "Value": function_name}], + StartTime=start, + EndTime=end, + Period=86400, + Statistics=["Sum"], + ) + return sum(point["Sum"] for point in response["Datapoints"]) + + +def month_to_date_usage(now): + """Lambda usage across every function in the account since the start of the month. + + GB-seconds use each function's current memory size and the Duration metric, which excludes + init time; the margin between MAX_GB_SECONDS and the 400,000 allowance covers both. + """ + start = _month_start(now) + usage = Usage(0.0, 0.0, 0.0) + for page in _lambda.get_paginator("list_functions").paginate(): + for function in page["Functions"]: + name = function["FunctionName"] + invocations = _metric_sum("Invocations", name, start, now) + duration_ms = _metric_sum("Duration", name, start, now) + usage.requests += invocations + usage.gb_seconds += duration_ms / 1000 * function["MemorySize"] / 1024 + if name == FUNCTION_NAME: + usage.converter_invocations = invocations + return usage + + +def spike_alarm_ok(): + alarms = _cloudwatch.describe_alarms(AlarmNames=[SPIKE_ALARM_NAME])["MetricAlarms"] + return not alarms or alarms[0]["StateValue"] != "ALARM" + + +def converter_enabled(): + response = _lambda.get_function_concurrency(FunctionName=FUNCTION_NAME) + return response.get("ReservedConcurrentExecutions") != 0 + + +def set_converter_enabled(enabled, reason): + """Switch the converter on or off. Does nothing (and sends nothing) if already there.""" + if converter_enabled() == enabled: + return False + if enabled: + _lambda.delete_function_concurrency(FunctionName=FUNCTION_NAME) + else: + _lambda.put_function_concurrency(FunctionName=FUNCTION_NAME, ReservedConcurrentExecutions=0) + state = "back on" if enabled else "OFF" + print(f"converter switched {state}: {reason}") + if ALERT_TOPIC_ARN: + _sns.publish( + TopicArn=ALERT_TOPIC_ARN, + Subject=f"NZ Image converter switched {state}", + Message=f"The JP2/TIFF converter Lambda was switched {state}.\n\nReason: {reason}", + ) + return True + + +def lambda_handler(event, _context): + if "Records" in event: + if spike_alarm_fired(event): + set_converter_enabled(False, f"{SPIKE_ALARM_NAME} fired") + return {"action": "alarm"} + + usage = month_to_date_usage(datetime.datetime.now(datetime.timezone.utc)) + reached = limits_reached(usage) + print(f"month-to-date usage: {usage}") + if reached: + set_converter_enabled(False, "month-to-date free-tier limit reached: " + ", ".join(reached)) + elif spike_alarm_ok(): + set_converter_enabled(True, "usage under every limit and the spike alarm is OK") + return {"usage": usage.__dict__, "limits_reached": reached} diff --git a/guard/pytest.ini b/guard/pytest.ini new file mode 100644 index 0000000..5ee6477 --- /dev/null +++ b/guard/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +testpaths = tests diff --git a/guard/requirements-dev.txt b/guard/requirements-dev.txt new file mode 100644 index 0000000..7ae2619 --- /dev/null +++ b/guard/requirements-dev.txt @@ -0,0 +1,2 @@ +boto3 +pytest diff --git a/guard/tests/test_app.py b/guard/tests/test_app.py new file mode 100644 index 0000000..6114afe --- /dev/null +++ b/guard/tests/test_app.py @@ -0,0 +1,191 @@ +"""Tests for the converter kill switch (guard/app.py), with the AWS clients replaced by fakes.""" + +import datetime +import json +import os +import sys +from pathlib import Path + +import pytest + +os.environ.setdefault("AWS_DEFAULT_REGION", "ap-southeast-2") +os.environ["CONVERTER_FUNCTION_NAME"] = "converter" +os.environ["SPIKE_ALARM_NAME"] = "spike" +os.environ["MAX_GB_SECONDS"] = "320000" +os.environ["MAX_REQUESTS"] = "800000" +os.environ["MAX_CONVERTER_INVOCATIONS"] = "20000" +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import app # noqa: E402 + + +class FakeLambda: + def __init__(self, reserved=None, functions=()): + self.reserved = reserved + self.functions = list(functions) + self.calls = [] + + def get_function_concurrency(self, FunctionName): + return {} if self.reserved is None else {"ReservedConcurrentExecutions": self.reserved} + + def put_function_concurrency(self, FunctionName, ReservedConcurrentExecutions): + self.calls.append(("put", FunctionName, ReservedConcurrentExecutions)) + self.reserved = ReservedConcurrentExecutions + + def delete_function_concurrency(self, FunctionName): + self.calls.append(("delete", FunctionName)) + self.reserved = None + + def get_paginator(self, _name): + functions = self.functions + + class Paginator: + def paginate(self): + return [{"Functions": functions}] + + return Paginator() + + +class FakeCloudWatch: + def __init__(self, sums=None, alarm_state="OK"): + self.sums = sums or {} + self.alarm_state = alarm_state + + def get_metric_statistics(self, MetricName, Dimensions, **_kwargs): + name = Dimensions[0]["Value"] + value = self.sums.get((name, MetricName), 0) + return {"Datapoints": [{"Sum": value}] if value else []} + + def describe_alarms(self, AlarmNames): + return {"MetricAlarms": [{"AlarmName": AlarmNames[0], "StateValue": self.alarm_state}]} + + +class FakeSNS: + def __init__(self): + self.messages = [] + + def publish(self, **kwargs): + self.messages.append(kwargs) + + +@pytest.fixture +def aws(monkeypatch): + def install(lambda_client, cloudwatch=None): + sns = FakeSNS() + monkeypatch.setattr(app, "_lambda", lambda_client) + monkeypatch.setattr(app, "_cloudwatch", cloudwatch or FakeCloudWatch()) + monkeypatch.setattr(app, "_sns", sns) + monkeypatch.setattr(app, "ALERT_TOPIC_ARN", "arn:aws:sns:ap-southeast-2:1:topic") + return sns + + return install + + +def _sns_event(message): + body = message if isinstance(message, str) else json.dumps(message) + return {"Records": [{"Sns": {"Message": body}}]} + + +# --- limits --------------------------------------------------------------------------- + + +def test_no_limit_reached_under_every_limit(): + assert app.limits_reached(app.Usage(gb_seconds=1000, requests=5000, converter_invocations=300)) == [] + + +@pytest.mark.parametrize( + "usage, expected", + [ + (app.Usage(320000, 0, 0), "compute"), + (app.Usage(0, 800000, 0), "requests"), + (app.Usage(0, 0, 20000), "converter calls"), + ], +) +def test_each_limit_is_reported_once_reached(usage, expected): + reached = app.limits_reached(usage) + + assert len(reached) == 1 + assert reached[0].startswith(expected) + + +# --- alarm messages ------------------------------------------------------------------- + + +def test_spike_alarm_entering_alarm_is_recognised(): + assert app.spike_alarm_fired(_sns_event({"AlarmName": "spike", "NewStateValue": "ALARM"})) + + +@pytest.mark.parametrize( + "message", + [ + {"AlarmName": "spike", "NewStateValue": "OK"}, + {"AlarmName": "some-other-alarm", "NewStateValue": "ALARM"}, + "The JP2/TIFF converter Lambda was switched OFF.", # the guard's own notice, not JSON + ], +) +def test_other_topic_messages_are_ignored(message): + assert not app.spike_alarm_fired(_sns_event(message)) + + +# --- usage ---------------------------------------------------------------------------- + + +def test_month_to_date_usage_weights_duration_by_each_functions_memory(aws): + functions = [ + {"FunctionName": "converter", "MemorySize": 2048}, + {"FunctionName": "api", "MemorySize": 512}, + ] + sums = { + ("converter", "Invocations"): 7, + ("converter", "Duration"): 10_000, # 10 s at 2 GB = 20 GB-s + ("api", "Invocations"): 100, + ("api", "Duration"): 8_000, # 8 s at 0.5 GB = 4 GB-s + } + aws(FakeLambda(functions=functions), FakeCloudWatch(sums=sums)) + + usage = app.month_to_date_usage(datetime.datetime(2026, 10, 24, tzinfo=datetime.timezone.utc)) + + assert usage == app.Usage(gb_seconds=24.0, requests=107, converter_invocations=7) + + +# --- handler -------------------------------------------------------------------------- + + +def test_spike_alarm_switches_the_converter_off_and_says_so(aws): + lam = FakeLambda() + sns = aws(lam) + + app.lambda_handler(_sns_event({"AlarmName": "spike", "NewStateValue": "ALARM"}), None) + + assert lam.calls == [("put", "converter", 0)] + assert "switched OFF" in sns.messages[0]["Subject"] + + +def test_spike_alarm_does_nothing_when_already_off(aws): + lam = FakeLambda(reserved=0) + sns = aws(lam) + + app.lambda_handler(_sns_event({"AlarmName": "spike", "NewStateValue": "ALARM"}), None) + + assert lam.calls == [] + assert sns.messages == [] + + +def _hourly(aws, *, reserved, converter_calls, alarm_state="OK"): + lam = FakeLambda(reserved=reserved, functions=[{"FunctionName": "converter", "MemorySize": 2048}]) + cloudwatch = FakeCloudWatch(sums={("converter", "Invocations"): converter_calls}, alarm_state=alarm_state) + aws(lam, cloudwatch) + app.lambda_handler({"source": "aws.events"}, None) + return lam + + +def test_hourly_check_switches_off_when_a_limit_is_reached(aws): + assert _hourly(aws, reserved=None, converter_calls=20000).calls == [("put", "converter", 0)] + + +def test_hourly_check_switches_back_on_under_the_limits_with_the_alarm_ok(aws): + assert _hourly(aws, reserved=0, converter_calls=10).calls == [("delete", "converter")] + + +def test_hourly_check_leaves_it_off_while_the_spike_alarm_is_still_firing(aws): + assert _hourly(aws, reserved=0, converter_calls=10, alarm_state="ALARM").calls == [] diff --git a/infra/bootstrap.yaml b/infra/bootstrap.yaml index ea3aaae..93aaab6 100644 --- a/infra/bootstrap.yaml +++ b/infra/bootstrap.yaml @@ -1,13 +1,14 @@ AWSTemplateFormatVersion: "2010-09-09" Description: > - Deploy-artefact storage for the nzimageapi SAM stack: the converter's ECR - repository and the SAM artefact bucket, each with a retention lifecycle policy. + Deploy-artefact storage for the nzimageapi SAM stack: the SAM artefact bucket, + with a retention lifecycle policy. (It also held the converter's ECR repository + until the converter became a .zip in October 2026.) This is deliberately a SEPARATE stack from template.yaml, not part of it. - `sam deploy` pushes the container image to ECR and uploads the packaged template - to S3 BEFORE CloudFormation runs, so neither resource can live in the stack that - consumes them - they would not exist yet at push time. Hence a small bootstrap + `sam deploy` uploads the packaged code and template to S3 BEFORE CloudFormation + runs, so the bucket cannot live in the stack that consumes it - it would not + exist yet at upload time. Hence a small bootstrap stack, deployed once with plain `aws cloudformation deploy`, that the main stack's samconfig.toml then points at. @@ -21,15 +22,6 @@ Description: > .claude/rules/build-test-deploy.md, "Deploy artefact retention (cost control)". Parameters: - ImageRetentionCount: - Type: Number - Default: 3 - MinValue: 1 - Description: > - How many converter images to keep in ECR. The newest is the live one, so the - remainder are rollback targets. Every deploy pushes a fresh ~195 MB image under - a unique tag and never overwrites, so without this they accumulate forever. - ArtifactRetentionDays: Type: Number Default: 60 @@ -40,34 +32,6 @@ Parameters: back to it, which is far outside any realistic rollback window here. Resources: - ConverterRepository: - Type: AWS::ECR::Repository - DeletionPolicy: Retain - UpdateReplacePolicy: Retain - Properties: - RepositoryName: nzimageapi/jp2converter - ImageTagMutability: MUTABLE - ImageScanningConfiguration: - ScanOnPush: true - EncryptionConfiguration: - EncryptionType: AES256 - LifecyclePolicy: - LifecyclePolicyText: !Sub | - { - "rules": [ - { - "rulePriority": 1, - "description": "Keep only the ${ImageRetentionCount} most recent images; older ones are dead deploy artefacts", - "selection": { - "tagStatus": "any", - "countType": "imageCountMoreThan", - "countNumber": ${ImageRetentionCount} - }, - "action": { "type": "expire" } - } - ] - } - ArtifactBucket: Type: AWS::S3::Bucket DeletionPolicy: Retain @@ -114,10 +78,6 @@ Resources: "aws:SecureTransport": false Outputs: - ConverterRepositoryUri: - Description: Value for samconfig.toml image_repositories (Jp2ConverterFunction=) - Value: !Sub "${AWS::AccountId}.dkr.ecr.${AWS::Region}.amazonaws.com/${ConverterRepository}" - ArtifactBucketName: Description: Value for samconfig.toml s3_bucket (replaces resolve_s3 = true) Value: !Ref ArtifactBucket diff --git a/template.yaml b/template.yaml index 8a4eca3..8e24597 100644 --- a/template.yaml +++ b/template.yaml @@ -40,6 +40,32 @@ Parameters: Lambda (verifies them), so the public converter Function URL only accepts requests the Swift Lambda minted. Pass at deploy time via --parameter-overrides or the gitignored samconfig.toml. NEVER commit it. + ConverterSpikeThreshold: + Type: Number + Default: 100 + Description: > + Converter invocations in 5 minutes above which the spike alarm fires and + ConverterGuardFunction switches the converter off. + GuardMaxGbSeconds: + Type: Number + Default: 320000 + Description: > + Month-to-date Lambda compute (GB-seconds, all functions) at which the converter is + switched off. The always-free allowance is 400,000; the gap absorbs the guard's hourly + lag and the Duration metric not counting init time. + GuardMaxRequests: + Type: Number + Default: 800000 + Description: > + Month-to-date Lambda requests (all functions) at which the converter is switched off. + The always-free allowance is 1,000,000. + GuardMaxConverterInvocations: + Type: Number + Default: 20000 + Description: > + Month-to-date converter invocations at which it is switched off. This bounds data + transfer out, which has no CloudWatch metric: each response is at most ~4 MB, so 20,000 + stays under the 100 GB a month AWS includes for free. Resources: # --- Cost/abuse notifications: one SNS topic, emailed on any alarm or budget breach --- @@ -86,21 +112,26 @@ Resources: - SubscriptionType: EMAIL Address: !Ref AlarmEmail - # --- JP2 -> JPEG converter (container image + public Function URL) ------------------ + # --- JP2 -> JPEG converter (.zip + public Function URL) ------------------------------ + # A .zip (not a container image) so there is no ECR repo to pay for: ECR's free storage is a + # 12-month offer this account no longer has, while Lambda's .zip code storage is included. Jp2ConverterFunction: Type: AWS::Serverless::Function Properties: - PackageType: Image + CodeUri: converter/ + Handler: app.lambda_handler + Runtime: python3.13 Architectures: - arm64 MemorySize: 2048 # Lambda CPU scales with memory; headroom for large JP2 masters - Timeout: 60 # large masters: download + reduced-level decode - # No ReservedConcurrentExecutions: this account's total Lambda concurrency quota is only - # 10 (not the usual 1000 default), shared with NZImageApiFunction, and is already fully - # unreserved -- setting any reservation here would violate that floor. The account-wide - # limit itself is currently a tighter concurrency ceiling than a per-function reservation - # would be. If the quota is ever increased (see docs/ACCESS-CONTROL.md, "Cost controls"), - # reintroduce a reservation here to subdivide the larger pool between both functions. + # NDHA streams some masters to the Lambda at under 1 MB/s, so a 45 MB master can need + # ~60 s just to download. app.py stops the download at DOWNLOAD_DEADLINE (90 s), which + # leaves this much again for decode/encode. + Timeout: 120 + # No ReservedConcurrentExecutions in the template: this account's total Lambda concurrency + # quota is only 10 (not the usual 1000 default) and must stay fully unreserved. The only + # reservation ever made is 0, set at runtime by ConverterGuardFunction as a kill switch, + # which CloudFormation does not track. Environment: Variables: # Host allowlist for the master->JPEG proxy (comma-separated). NDHA for TAPUHI's @@ -118,32 +149,83 @@ Resources: AllowMethods: - GET Metadata: - Dockerfile: Dockerfile - DockerContext: ./converter - DockerTag: latest + BuildMethod: makefile # converter/Makefile: installs Pillow's aarch64 wheel on any host - # Fires if the converter -- the expensive function -- is invoked more than 100 times in - # any 1-hour window. Tune the threshold to expected legitimate traffic; the point is to - # notice a leaked/abused converter URL as a notification, not a surprise bill. + # Fires when the converter -- the expensive function -- is hammered. It emails AlarmTopic and, + # through the same topic, wakes ConverterGuardFunction, which switches the converter off. A + # 5-minute period means a flood is cut off within minutes rather than at the end of an hour. + # Normal use is about 16 conversions an hour per open slideshow tab. Jp2ConverterInvocationsAlarm: Type: AWS::CloudWatch::Alarm Properties: AlarmName: nzimageapi-converter-invocations-high - AlarmDescription: Jp2ConverterFunction invoked more than 100 times in 1 hour. + AlarmDescription: !Sub Jp2ConverterFunction invoked more than ${ConverterSpikeThreshold} times in 5 minutes. ConverterGuardFunction switches it off. Namespace: AWS/Lambda MetricName: Invocations Dimensions: - Name: FunctionName Value: !Ref Jp2ConverterFunction Statistic: Sum - Period: 3600 + Period: 300 EvaluationPeriods: 1 - Threshold: 100 + Threshold: !Ref ConverterSpikeThreshold ComparisonOperator: GreaterThanThreshold TreatMissingData: notBreaching AlarmActions: - !Ref AlarmTopic + # --- Free-tier kill switch for the converter ---------------------------------------- + # The converter's Function URL is public, so nothing else caps what it can cost. This function + # sets the converter's reserved concurrency to 0 (every call is throttled, nothing is billed) + # when the spike alarm fires, and checks month-to-date usage every hour against limits set + # below Lambda's always-free allowance. The hourly run switches the converter back on once + # usage is under the limits again (a new month) and the alarm is back to OK. See guard/app.py. + ConverterGuardFunction: + Type: AWS::Serverless::Function + Properties: + CodeUri: guard/ + Handler: app.lambda_handler + Runtime: python3.13 + Architectures: + - arm64 + MemorySize: 128 + Timeout: 30 + Environment: + Variables: + CONVERTER_FUNCTION_NAME: !Ref Jp2ConverterFunction + SPIKE_ALARM_NAME: nzimageapi-converter-invocations-high + ALERT_TOPIC_ARN: !Ref AlarmTopic + MAX_GB_SECONDS: !Ref GuardMaxGbSeconds + MAX_REQUESTS: !Ref GuardMaxRequests + MAX_CONVERTER_INVOCATIONS: !Ref GuardMaxConverterInvocations + Policies: + - Statement: + - Effect: Allow + Action: + - lambda:GetFunctionConcurrency + - lambda:PutFunctionConcurrency + - lambda:DeleteFunctionConcurrency + Resource: !GetAtt Jp2ConverterFunction.Arn + - Effect: Allow + Action: + - lambda:ListFunctions + - cloudwatch:GetMetricStatistics + - cloudwatch:DescribeAlarms + Resource: "*" + - SNSPublishMessagePolicy: + TopicName: !GetAtt AlarmTopic.TopicName + Events: + Hourly: + Type: Schedule + Properties: + Schedule: rate(1 hour) + SpikeAlarm: + Type: SNS + Properties: + Topic: !Ref AlarmTopic + Metadata: + BuildMethod: makefile + # --- Swift NZ Image API (provided.al2023 custom runtime, built via the Makefile) ------- NZImageApiFunction: Type: AWS::Serverless::Function From 7a3fcf1939076a506c60ff1d36bebf8f7cb60818 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Sat, 24 Oct 2026 17:57:04 +1300 Subject: [PATCH 2/4] fix(guard): cooldown after a spike, read1 download loop, review fixes - Hourly run only re-enables once the spike alarm has been quiet for GuardSpikeCooldownHours (6): throttled calls aren't invocations, so the alarm clears within minutes and couldn't tell a flood had stopped. - download_image reads with read1 so a trickling source can't hold one read past the deadline. - converter/Makefile fails the build if the wheel lacks OpenJPEG/libtiff. - Guard timeout 60 s, documented limits, style fixes. Co-Authored-By: Claude Opus 5.5 --- .claude/rules/build-test-deploy.md | 14 ++++++---- .claude/rules/converter.md | 26 ++++++++++++++---- .github/workflows/ci-cd.yml | 4 +-- CLAUDE.md | 4 +-- converter/Makefile | 4 ++- converter/app.py | 5 +++- converter/tests/test_app.py | 2 +- docs/ACCESS-CONTROL.md | 7 ++++- guard/Makefile | 2 +- guard/app.py | 43 +++++++++++++++++++++++------- guard/tests/test_app.py | 34 ++++++++++++++++++++--- template.yaml | 17 +++++++++--- 12 files changed, 127 insertions(+), 35 deletions(-) diff --git a/.claude/rules/build-test-deploy.md b/.claude/rules/build-test-deploy.md index 1e7ec01..2a8a504 100644 --- a/.claude/rules/build-test-deploy.md +++ b/.claude/rules/build-test-deploy.md @@ -52,9 +52,9 @@ same collections file and only one writer can hold the mutex ## SAM deploy (the real deploy path) Stack name `nzimageapi`, region `ap-southeast-2`. Needs Docker for the Swift Lambda's Makefile -target — Docker commands need the sandbox disabled here, since the Docker socket isn't +target. Docker commands need the sandbox disabled here, since the Docker socket isn't sandbox-allowlisted. The converter and guard are .zips built by their own Makefiles -(`converter/Makefile`, `guard/Makefile`) with no Docker; the converter's `pip install` needs +(`converter/Makefile`, `guard/Makefile`) with no Docker. The converter's `pip install` needs network access to PyPI. ```bash @@ -84,7 +84,11 @@ there were 29 converter images in ECR (~5.5 GB) and 1.45 GB in S3. The converter was a container image until October 2026. ECR's free storage is a 12-month offer this account no longer has, so even three retained images cost about $0.06 a month. It is -now a .zip, whose code storage Lambda includes for free, and the ECR repository is gone. +now a .zip, whose code storage Lambda includes for free. Removing `ConverterRepository` from +`infra/bootstrap.yaml` does not delete the repository (it was `DeletionPolicy: Retain`), so after +the first .zip deploy replaced the image function, the bootstrap stack was redeployed and +`nzimageapi/jp2converter` was deleted by hand with +`aws ecr delete-repository --region ap-southeast-2 --repository-name nzimageapi/jp2converter --force`. ### `infra/bootstrap.yaml` owns the artefact bucket @@ -99,7 +103,7 @@ aws cloudformation deploy --region ap-southeast-2 --stack-name nzimageapi-bootst template to S3 *before* CloudFormation runs, so a bucket declared in the stack that consumes it would not exist yet at upload time. -`ArtifactRetentionDays` (default `60`) sets when artefacts expire; non-current versions go at 7 +`ArtifactRetentionDays` (default `60`) sets when artefacts expire. Non-current versions go at 7 days, stalled multipart uploads at 7. A stack update failing more than 60 days after the deploy that produced an artefact cannot roll back to it, which is well outside any realistic rollback window here. The bucket carries `DeletionPolicy: Retain` / `UpdateReplacePolicy: Retain`. @@ -108,7 +112,7 @@ window here. The bucket carries `DeletionPolicy: Retain` / `UpdateReplacePolicy: The gitignored `samconfig.toml` points the main stack at the bucket with `s3_bucket = "nzimageapi-sam-artifacts-686865771242-ap-southeast-2"` (which **replaces** -`resolve_s3 = true`; the two are mutually exclusive, and `resolve_s3` is what made SAM +`resolve_s3 = true`. The two are mutually exclusive, and `resolve_s3` is what made SAM auto-create its own unmanaged bucket). It no longer needs an `image_repositories` entry. `.github/workflows/ci-cd.yml` cannot read `samconfig.toml`, so its deploy job reads the bucket diff --git a/.claude/rules/converter.md b/.claude/rules/converter.md index 99de166..89e2cfb 100644 --- a/.claude/rules/converter.md +++ b/.claude/rules/converter.md @@ -35,13 +35,29 @@ reservation: - **Hourly:** month-to-date usage from CloudWatch is checked against `GuardMaxGbSeconds` (320,000 of the 400,000 free), `GuardMaxRequests` (800,000 of 1,000,000) and `GuardMaxConverterInvocations` (20,000, which keeps data transfer out under the free 100 GB at - up to ~4 MB a response). Over any limit, it switches off. Under all of them with the spike alarm - OK, it switches back on, which is how it recovers after a spike or at the start of a month. + up to ~4 MB a response). Over any limit, it switches off. Under all of them, and with the spike + alarm not fired for `GuardSpikeCooldownHours` (6), it switches back on. That is how it recovers + after a spike or at the start of a month. + +The cooldown exists because throttled calls are not invocations: the spike alarm clears within +minutes of the switch-off, so its state alone can't tell that a flood has stopped. The guard reads +the alarm's history instead. A flood that outlasts the cooldown costs about one alarm period of +compute per cycle, and the month-to-date limits bound the total. Every switch is emailed to the cost-alarm topic. While the converter is off, converter URLs fail -and the slideshow skips those images. To switch it back on by hand: -`aws lambda delete-function-concurrency --region ap-southeast-2 --function-name ` -(the next hourly run turns it off again if a limit is still reached). +and the slideshow skips those images. + +Known limits, accepted for this account's size: +- Usage is read for `ap-southeast-2` only, but Lambda's free tier is account-wide. Every function + in the account is in this region today. If Lambdas are added elsewhere, lower the limits. +- The guard shares the account's 10-slot concurrency pool with the converter (nothing can be + reserved). In a flood that holds all 10, the alarm's async invocation is retried and gets in + as soon as a converter call finishes, which takes seconds to two minutes. +- The hourly run re-enables a converter that was switched off by hand. To keep it off, also + disable the guard's schedule: + `aws events disable-rule --region ap-southeast-2 --name `. + To switch it on by hand: `aws lambda delete-function-concurrency --region ap-southeast-2 + --function-name `. ## HMAC signer/verifier contract diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 97f5e86..c533af3 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -42,7 +42,7 @@ jobs: name: Build & deploy (SAM) needs: [swift-test, python-test] # deploy blocked until BOTH pass if: github.event_name == 'push' && github.ref == 'refs/heads/main' - runs-on: ubuntu-24.04-arm # native arm64 Docker for the Swift cross-compile; arm64 Python for the converter codec check + runs-on: ubuntu-24.04-arm # native arm64 Docker for the Swift cross-compile, arm64 Python for the converter codec check environment: production # gives an optional approval gate; scopes OIDC sub (see setup) permissions: id-token: write # required for OIDC @@ -63,7 +63,7 @@ jobs: with: python-version: "3.13" # same as the Lambda runtime, so the codec check imports the built wheel - name: sam build - run: sam build # Swift via Makefile+Docker; converter and guard via their Makefiles + run: sam build # Swift via Makefile+Docker, converter and guard via their Makefiles # The converter .zip carries Pillow's manylinux aarch64 wheel. Prove the build has the two # codecs it exists for, on the same architecture and Python as the Lambda, before deploying. - name: Check converter codecs diff --git a/CLAUDE.md b/CLAUDE.md index f3b17e2..8f5124f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,12 +120,12 @@ Secrets live in gitignored `.env`, `samconfig.toml`, and `.consumer-secrets/` by a built binary, not just Docker. Run with the sandbox disabled. - The converter is a **.zip**, not a container image, so there is no ECR repo to pay for. Its `converter/Makefile` installs Pillow's `manylinux_2_28_aarch64` wheel with `pip --platform`, so - `sam build` gives the same artefact on macOS and in CI; CI then imports the built package on + `sam build` gives the same artefact on macOS and in CI. CI then imports the built package on arm64 Python 3.13 to prove JP2 and TIFF decoding are present. Every `sam deploy` still uploads zips to the SAM artefact bucket declared in [`infra/bootstrap.yaml`](infra/bootstrap.yaml) (separate `nzimageapi-bootstrap` stack, 60-day expiry). It is **deliberately not in `template.yaml`**: `sam deploy` uploads before CloudFormation runs, so a bucket in the - consuming stack would not exist yet. `samconfig.toml` points at it with `s3_bucket`; CI reads + consuming stack would not exist yet. `samconfig.toml` points at it with `s3_bucket`, and CI reads it from the bootstrap stack's outputs. Full detail: [`.claude/rules/build-test-deploy.md`](.claude/rules/build-test-deploy.md#deploy-artefact-retention-cost-control). diff --git a/converter/Makefile b/converter/Makefile index 95371de..e3fe54b 100644 --- a/converter/Makefile +++ b/converter/Makefile @@ -1,10 +1,12 @@ # SAM `BuildMethod: makefile` target for the converter .zip (python3.13 on arm64). # Installs Pillow's manylinux aarch64 wheel whatever the build host's OS or architecture, so a # macOS build produces the same artefact as CI. Pillow 12+ only publishes manylinux_2_27/2_28 -# tags; the manylinux2014 tag in AWS's packaging guide finds no wheel. Lambda's AL2023 runtime +# tags, so the manylinux2014 tag in AWS's packaging guide finds no wheel. Lambda's AL2023 runtime # has glibc 2.34, which satisfies 2_28. build-Jp2ConverterFunction: python3 -m pip install --quiet --no-compile --target "$(ARTIFACTS_DIR)" \ --platform manylinux_2_28_aarch64 --implementation cp --python-version 3.13 \ --only-binary=:all: -r requirements.txt + @ls "$(ARTIFACTS_DIR)"/pillow.libs/libopenjp2-*.so* "$(ARTIFACTS_DIR)"/pillow.libs/libtiff-*.so* >/dev/null \ + || { echo "Pillow wheel is missing OpenJPEG or libtiff"; exit 1; } cp app.py "$(ARTIFACTS_DIR)/" diff --git a/converter/app.py b/converter/app.py index 5abe68e..5c4eb27 100644 --- a/converter/app.py +++ b/converter/app.py @@ -117,7 +117,10 @@ def download_image(url, max_bytes=MAX_DOWNLOAD_BYTES, timeout=DOWNLOAD_TIMEOUT, give_up_at = time.monotonic() + deadline data = bytearray() with _OPENER.open(request, timeout=timeout) as response: # noqa: S310 (host allowlisted + SSRF-guarded) - while chunk := response.read(DOWNLOAD_CHUNK_BYTES): + # read1, not read: read(n) keeps reading until it has n bytes, so a source that trickles + # just inside DOWNLOAD_TIMEOUT could hold one call past the deadline. read1 returns after + # a single socket read. + while chunk := response.read1(DOWNLOAD_CHUNK_BYTES): data += chunk if len(data) > max_bytes: raise ValueError(f"source exceeds {max_bytes} byte limit") diff --git a/converter/tests/test_app.py b/converter/tests/test_app.py index 2e1fdc9..c4cd999 100644 --- a/converter/tests/test_app.py +++ b/converter/tests/test_app.py @@ -257,7 +257,7 @@ class _FakeURLResponse: def __init__(self, data): self._stream = BytesIO(data) - def read(self, n): + def read1(self, n): return self._stream.read(n) def __enter__(self): diff --git a/docs/ACCESS-CONTROL.md b/docs/ACCESS-CONTROL.md index 5ac78d2..513c303 100644 --- a/docs/ACCESS-CONTROL.md +++ b/docs/ACCESS-CONTROL.md @@ -249,7 +249,12 @@ Every control below is free at this project's scale. Each caps a different failu | Converter kill switch | Converter **spend**: sets its concurrency to 0 on a spike, or when month-to-date usage nears the free tier | 320,000 GB-s, 800,000 requests or 20,000 converter calls a month | `ConverterGuardFunction` (`guard/app.py`), see `.claude/rules/converter.md` | | Monthly budget | Notifies on any **real spend** | >$1 actual or forecasted → email | `MonthlyCostBudget` | -**Reserved concurrency was planned but isn't set.** This AWS account's total Lambda concurrent- +**The kill switch is the only reserved concurrency, and it is set at runtime.** `ConverterGuardFunction` +sets the converter's reserved concurrency to **0** to switch it off (a reservation of 0 doesn't +lower the unreserved pool) and deletes it to switch it back on. Nothing in `template.yaml` reserves +concurrency, for the reason below. + +**Positive reserved concurrency was planned but isn't set.** This AWS account's total Lambda concurrent- executions quota is **10** (not the usual 1,000 default), and is already fully unreserved. Setting `ReservedConcurrentExecutions` on either function — even a small value — would push the account's unreserved pool below its own floor and the deploy is rejected (`InvalidRequest`: diff --git a/guard/Makefile b/guard/Makefile index 43b1570..d843ebb 100644 --- a/guard/Makefile +++ b/guard/Makefile @@ -1,5 +1,5 @@ # SAM `BuildMethod: makefile` target for the guard .zip. It has no dependencies beyond the boto3 -# bundled in the python3.13 runtime, so the build is a copy; using makefile (not SAM's +# bundled in the python3.13 runtime, so the build is a copy. Using makefile (not SAM's # python_pip builder) means `sam build` does not need python3.13 on the build host. build-ConverterGuardFunction: cp app.py "$(ARTIFACTS_DIR)/" diff --git a/guard/app.py b/guard/app.py index e310c16..5480b50 100644 --- a/guard/app.py +++ b/guard/app.py @@ -10,12 +10,19 @@ off straight away. Other messages on the topic (the API alarm, this function's own notices) are ignored. - An hourly schedule. It sums month-to-date usage from CloudWatch and switches the converter off - when any limit is reached, or back on when every limit is clear and the spike alarm is OK. - That is also how the converter comes back after a spike, or at the start of a new month. - -During a sustained flood the converter cycles: off on the alarm, back on at the next hourly run -once the alarm clears, off again. The month-to-date limits bound the total, and once one is hit the + when any limit is reached, or back on when every limit is clear and the spike alarm has not + been in ALARM for SPIKE_COOLDOWN_HOURS. That is also how the converter comes back after a + spike, or at the start of a new month. + +Throttled calls are not counted as invocations, so the spike alarm clears within minutes of the +switch-off. The cooldown (read from the alarm's history) is what stops the next hourly run from +handing a still-running flood the converter back. A flood that outlasts it costs one alarm period +of compute per cooldown, and the month-to-date limits bound the total: once one is hit the converter stays off until the month rolls over. + +Limits (by design, see .claude/rules/converter.md): usage is read for this region only, the guard +shares the account's 10-slot concurrency pool with the converter it stops (async SNS retries get it +a slot as soon as one frees), and the hourly run re-enables a converter switched off by hand. """ import datetime @@ -31,6 +38,7 @@ MAX_GB_SECONDS = float(os.environ.get("MAX_GB_SECONDS", "320000")) MAX_REQUESTS = float(os.environ.get("MAX_REQUESTS", "800000")) MAX_CONVERTER_INVOCATIONS = float(os.environ.get("MAX_CONVERTER_INVOCATIONS", "20000")) +SPIKE_COOLDOWN_HOURS = float(os.environ.get("SPIKE_COOLDOWN_HOURS", "6")) _lambda = boto3.client("lambda") _cloudwatch = boto3.client("cloudwatch") @@ -107,9 +115,21 @@ def month_to_date_usage(now): return usage -def spike_alarm_ok(): +def spike_alarm_quiet(now): + """True when the spike alarm is not in ALARM and has not entered it within the cooldown.""" alarms = _cloudwatch.describe_alarms(AlarmNames=[SPIKE_ALARM_NAME])["MetricAlarms"] - return not alarms or alarms[0]["StateValue"] != "ALARM" + if alarms and alarms[0]["StateValue"] == "ALARM": + return False + history = _cloudwatch.describe_alarm_history( + AlarmName=SPIKE_ALARM_NAME, + HistoryItemType="StateUpdate", + StartDate=now - datetime.timedelta(hours=SPIKE_COOLDOWN_HOURS), + EndDate=now, + ) + for item in history["AlarmHistoryItems"]: + if json.loads(item["HistoryData"]).get("newState", {}).get("stateValue") == "ALARM": + return False + return True def converter_enabled(): @@ -142,11 +162,14 @@ def lambda_handler(event, _context): set_converter_enabled(False, f"{SPIKE_ALARM_NAME} fired") return {"action": "alarm"} - usage = month_to_date_usage(datetime.datetime.now(datetime.timezone.utc)) + now = datetime.datetime.now(datetime.timezone.utc) + usage = month_to_date_usage(now) reached = limits_reached(usage) print(f"month-to-date usage: {usage}") if reached: set_converter_enabled(False, "month-to-date free-tier limit reached: " + ", ".join(reached)) - elif spike_alarm_ok(): - set_converter_enabled(True, "usage under every limit and the spike alarm is OK") + elif spike_alarm_quiet(now): + set_converter_enabled( + True, f"usage under every limit and no spike alarm in the last {SPIKE_COOLDOWN_HOURS:g} hours" + ) return {"usage": usage.__dict__, "limits_reached": reached} diff --git a/guard/tests/test_app.py b/guard/tests/test_app.py index 6114afe..d848d55 100644 --- a/guard/tests/test_app.py +++ b/guard/tests/test_app.py @@ -47,9 +47,10 @@ def paginate(self): class FakeCloudWatch: - def __init__(self, sums=None, alarm_state="OK"): + def __init__(self, sums=None, alarm_state="OK", history=()): self.sums = sums or {} self.alarm_state = alarm_state + self.history = list(history) def get_metric_statistics(self, MetricName, Dimensions, **_kwargs): name = Dimensions[0]["Value"] @@ -59,6 +60,15 @@ def get_metric_statistics(self, MetricName, Dimensions, **_kwargs): def describe_alarms(self, AlarmNames): return {"MetricAlarms": [{"AlarmName": AlarmNames[0], "StateValue": self.alarm_state}]} + def describe_alarm_history(self, StartDate, EndDate, **_kwargs): + return { + "AlarmHistoryItems": [ + {"HistoryData": json.dumps({"newState": {"stateValue": state}})} + for at, state in self.history + if StartDate <= at <= EndDate + ] + } + class FakeSNS: def __init__(self): @@ -171,9 +181,15 @@ def test_spike_alarm_does_nothing_when_already_off(aws): assert sns.messages == [] -def _hourly(aws, *, reserved, converter_calls, alarm_state="OK"): +def _hourly(aws, *, reserved, converter_calls, alarm_state="OK", alarm_fired_hours_ago=None): lam = FakeLambda(reserved=reserved, functions=[{"FunctionName": "converter", "MemorySize": 2048}]) - cloudwatch = FakeCloudWatch(sums={("converter", "Invocations"): converter_calls}, alarm_state=alarm_state) + history = [] + if alarm_fired_hours_ago is not None: + fired_at = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(hours=alarm_fired_hours_ago) + history = [(fired_at, "ALARM"), (fired_at + datetime.timedelta(minutes=6), "OK")] + cloudwatch = FakeCloudWatch( + sums={("converter", "Invocations"): converter_calls}, alarm_state=alarm_state, history=history + ) aws(lam, cloudwatch) app.lambda_handler({"source": "aws.events"}, None) return lam @@ -189,3 +205,15 @@ def test_hourly_check_switches_back_on_under_the_limits_with_the_alarm_ok(aws): def test_hourly_check_leaves_it_off_while_the_spike_alarm_is_still_firing(aws): assert _hourly(aws, reserved=0, converter_calls=10, alarm_state="ALARM").calls == [] + + +def test_hourly_check_waits_out_the_cooldown_after_a_spike(aws): + # The alarm cleared minutes after the switch-off (throttled calls aren't invocations), but it + # fired 1 hour ago, inside the 6-hour cooldown, so a flood may still be running. + assert _hourly(aws, reserved=0, converter_calls=10, alarm_fired_hours_ago=1).calls == [] + + +def test_hourly_check_switches_back_on_once_the_cooldown_has_passed(aws): + assert _hourly(aws, reserved=0, converter_calls=10, alarm_fired_hours_ago=7).calls == [ + ("delete", "converter") + ] diff --git a/template.yaml b/template.yaml index 8e24597..ac5f7f1 100644 --- a/template.yaml +++ b/template.yaml @@ -51,7 +51,7 @@ Parameters: Default: 320000 Description: > Month-to-date Lambda compute (GB-seconds, all functions) at which the converter is - switched off. The always-free allowance is 400,000; the gap absorbs the guard's hourly + switched off. The always-free allowance is 400,000, and the gap absorbs the guard's hourly lag and the Duration metric not counting init time. GuardMaxRequests: Type: Number @@ -67,6 +67,14 @@ Parameters: transfer out, which has no CloudWatch metric: each response is at most ~4 MB, so 20,000 stays under the 100 GB a month AWS includes for free. + GuardSpikeCooldownHours: + Type: Number + Default: 6 + Description: > + Hours after the spike alarm last fired before the hourly guard run may switch the + converter back on. Throttled calls don't count as invocations, so the alarm itself clears + within minutes and can't be used to tell that a flood has stopped. + Resources: # --- Cost/abuse notifications: one SNS topic, emailed on any alarm or budget breach --- AlarmTopic: @@ -179,7 +187,8 @@ Resources: # sets the converter's reserved concurrency to 0 (every call is throttled, nothing is billed) # when the spike alarm fires, and checks month-to-date usage every hour against limits set # below Lambda's always-free allowance. The hourly run switches the converter back on once - # usage is under the limits again (a new month) and the alarm is back to OK. See guard/app.py. + # usage is under the limits again (a new month) and the spike alarm has been quiet for + # GuardSpikeCooldownHours. See guard/app.py. ConverterGuardFunction: Type: AWS::Serverless::Function Properties: @@ -189,7 +198,7 @@ Resources: Architectures: - arm64 MemorySize: 128 - Timeout: 30 + Timeout: 60 Environment: Variables: CONVERTER_FUNCTION_NAME: !Ref Jp2ConverterFunction @@ -198,6 +207,7 @@ Resources: MAX_GB_SECONDS: !Ref GuardMaxGbSeconds MAX_REQUESTS: !Ref GuardMaxRequests MAX_CONVERTER_INVOCATIONS: !Ref GuardMaxConverterInvocations + SPIKE_COOLDOWN_HOURS: !Ref GuardSpikeCooldownHours Policies: - Statement: - Effect: Allow @@ -211,6 +221,7 @@ Resources: - lambda:ListFunctions - cloudwatch:GetMetricStatistics - cloudwatch:DescribeAlarms + - cloudwatch:DescribeAlarmHistory Resource: "*" - SNSPublishMessagePolicy: TopicName: !GetAtt AlarmTopic.TopicName From 40088d337451f4676ad175138745a22c80efdec5 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Sat, 24 Oct 2026 18:00:19 +1300 Subject: [PATCH 3/4] fix(guard): hourly run also stops a firing spike, filter the alarm subscription - Hourly run switches the converter off while the spike alarm is in ALARM, covering a failed SNS-triggered run (the alarm won't notify again). - SNS FilterPolicy so only the spike alarm entering ALARM invokes the guard. - Reference the alarm name instead of repeating it. - Converter timeout 180 s: a download can end ~125 s (90 s deadline plus one 35 s blocking read), leaving time to decode. Co-Authored-By: Claude Opus 5.5 --- .claude/rules/converter.md | 2 +- converter/app.py | 2 +- guard/app.py | 14 ++++++++++---- guard/tests/test_app.py | 7 +++++++ template.yaml | 17 +++++++++++++---- 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/.claude/rules/converter.md b/.claude/rules/converter.md index 89e2cfb..96a0966 100644 --- a/.claude/rules/converter.md +++ b/.claude/rules/converter.md @@ -17,7 +17,7 @@ imports the built package on arm64 Python 3.13 and fails if either codec is miss ## Timeouts -NDHA streams some masters to the Lambda at under 1 MB/s. The function timeout is 120 s and +NDHA streams some masters to the Lambda at under 1 MB/s. The function timeout is 180 s and `download_image` gives up after `DOWNLOAD_DEADLINE` (90 s) for the whole download, on top of the per-socket `DOWNLOAD_TIMEOUT`, so a slow source ends as a logged `502 download failed` instead of a silent function timeout. FL73848659 (45.8 MB, 7356×5010) is the known slow master. diff --git a/converter/app.py b/converter/app.py index 5c4eb27..7d335bd 100644 --- a/converter/app.py +++ b/converter/app.py @@ -58,7 +58,7 @@ # download. NDHA streams some masters to the Lambda at under 1 MB/s (FL73848659, 45.8 MB, ran # into the old 60 s function timeout with nothing logged), hence the whole-download deadline, # set below the 120 s function timeout so a slow source fails as a logged 502 with time to spare -# for decoding the ones that make it. +# for decoding the ones that make it (see the arithmetic on Timeout in template.yaml). DOWNLOAD_TIMEOUT = int(os.environ.get("DOWNLOAD_TIMEOUT", "35")) # seconds DOWNLOAD_DEADLINE = int(os.environ.get("DOWNLOAD_DEADLINE", "90")) # seconds, whole download DOWNLOAD_CHUNK_BYTES = 1024 * 1024 diff --git a/guard/app.py b/guard/app.py index 5480b50..e2dbad6 100644 --- a/guard/app.py +++ b/guard/app.py @@ -115,11 +115,13 @@ def month_to_date_usage(now): return usage -def spike_alarm_quiet(now): - """True when the spike alarm is not in ALARM and has not entered it within the cooldown.""" +def spike_alarm_firing(): alarms = _cloudwatch.describe_alarms(AlarmNames=[SPIKE_ALARM_NAME])["MetricAlarms"] - if alarms and alarms[0]["StateValue"] == "ALARM": - return False + return bool(alarms) and alarms[0]["StateValue"] == "ALARM" + + +def spike_alarm_quiet(now): + """True when the spike alarm has not entered ALARM within the cooldown.""" history = _cloudwatch.describe_alarm_history( AlarmName=SPIKE_ALARM_NAME, HistoryItemType="StateUpdate", @@ -168,6 +170,10 @@ def lambda_handler(event, _context): print(f"month-to-date usage: {usage}") if reached: set_converter_enabled(False, "month-to-date free-tier limit reached: " + ", ".join(reached)) + elif spike_alarm_firing(): + # Normally the SNS-triggered run has already done this. This covers that run failing + # (throttled past its retries, an API error): the alarm stays in ALARM and won't notify again. + set_converter_enabled(False, f"{SPIKE_ALARM_NAME} is still firing") elif spike_alarm_quiet(now): set_converter_enabled( True, f"usage under every limit and no spike alarm in the last {SPIKE_COOLDOWN_HOURS:g} hours" diff --git a/guard/tests/test_app.py b/guard/tests/test_app.py index d848d55..7c7a79b 100644 --- a/guard/tests/test_app.py +++ b/guard/tests/test_app.py @@ -207,6 +207,13 @@ def test_hourly_check_leaves_it_off_while_the_spike_alarm_is_still_firing(aws): assert _hourly(aws, reserved=0, converter_calls=10, alarm_state="ALARM").calls == [] +def test_hourly_check_switches_off_if_the_alarm_run_never_did(aws): + # The SNS-triggered run failed, so the converter is still on while the alarm sits in ALARM. + assert _hourly(aws, reserved=None, converter_calls=10, alarm_state="ALARM").calls == [ + ("put", "converter", 0) + ] + + def test_hourly_check_waits_out_the_cooldown_after_a_spike(aws): # The alarm cleared minutes after the switch-off (throttled calls aren't invocations), but it # fired 1 hour ago, inside the 6-hour cooldown, so a flood may still be running. diff --git a/template.yaml b/template.yaml index ac5f7f1..9822906 100644 --- a/template.yaml +++ b/template.yaml @@ -133,9 +133,10 @@ Resources: - arm64 MemorySize: 2048 # Lambda CPU scales with memory; headroom for large JP2 masters # NDHA streams some masters to the Lambda at under 1 MB/s, so a 45 MB master can need - # ~60 s just to download. app.py stops the download at DOWNLOAD_DEADLINE (90 s), which - # leaves this much again for decode/encode. - Timeout: 120 + # ~60 s just to download. app.py gives up once a read returns after DOWNLOAD_DEADLINE (90 s), + # and one read can block for up to DOWNLOAD_TIMEOUT (35 s), so a download ends by ~125 s. + # That leaves ~55 s for decode/encode. + Timeout: 180 # No ReservedConcurrentExecutions in the template: this account's total Lambda concurrency # quota is only 10 (not the usual 1000 default) and must stay fully unreserved. The only # reservation ever made is 0, set at runtime by ConverterGuardFunction as a kill switch, @@ -202,7 +203,7 @@ Resources: Environment: Variables: CONVERTER_FUNCTION_NAME: !Ref Jp2ConverterFunction - SPIKE_ALARM_NAME: nzimageapi-converter-invocations-high + SPIKE_ALARM_NAME: !Ref Jp2ConverterInvocationsAlarm ALERT_TOPIC_ARN: !Ref AlarmTopic MAX_GB_SECONDS: !Ref GuardMaxGbSeconds MAX_REQUESTS: !Ref GuardMaxRequests @@ -234,6 +235,14 @@ Resources: Type: SNS Properties: Topic: !Ref AlarmTopic + # Only the spike alarm entering ALARM wakes the guard. Other alarms, budget mail and + # the guard's own plain-text notices on this topic are dropped at the subscription. + FilterPolicyScope: MessageBody + FilterPolicy: + AlarmName: + - !Ref Jp2ConverterInvocationsAlarm + NewStateValue: + - ALARM Metadata: BuildMethod: makefile From 3300cd01675da5dbb6275d28330a75a1a96f4789 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Sat, 24 Oct 2026 18:05:01 +1300 Subject: [PATCH 4/4] fix(guard): wider GB-s margin, cap redirects - GuardMaxGbSeconds 250,000: about two hours of worst-case converter burn below the 400,000 free, since a flood of slow calls can stay under the spike threshold and only the hourly check catches it. - At most 3 redirects, so connection setup can't outlast the timeout. Co-Authored-By: Claude Opus 5.5 --- .claude/rules/converter.md | 4 ++-- converter/app.py | 7 ++++++- docs/ACCESS-CONTROL.md | 2 +- guard/app.py | 2 +- template.yaml | 8 +++++--- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.claude/rules/converter.md b/.claude/rules/converter.md index 96a0966..9200f7c 100644 --- a/.claude/rules/converter.md +++ b/.claude/rules/converter.md @@ -32,8 +32,8 @@ reservation: - **Spike:** `nzimageapi-converter-invocations-high` (over `ConverterSpikeThreshold`, default 100, invocations in 5 minutes) emails `nzimageapi-cost-alarms` and, through the same topic, switches the converter off. -- **Hourly:** month-to-date usage from CloudWatch is checked against `GuardMaxGbSeconds` (320,000 - of the 400,000 free), `GuardMaxRequests` (800,000 of 1,000,000) and +- **Hourly:** month-to-date usage from CloudWatch is checked against `GuardMaxGbSeconds` (250,000 + of the 400,000 free, about two hours of worst-case burn below it), `GuardMaxRequests` (800,000 of 1,000,000) and `GuardMaxConverterInvocations` (20,000, which keeps data transfer out under the free 100 GB at up to ~4 MB a response). Over any limit, it switches off. Under all of them, and with the spike alarm not fired for `GuardSpikeCooldownHours` (6), it switches back on. That is how it recovers diff --git a/converter/app.py b/converter/app.py index 7d335bd..f7458b8 100644 --- a/converter/app.py +++ b/converter/app.py @@ -57,7 +57,7 @@ # DOWNLOAD_TIMEOUT is per socket operation, so on its own it never stops a slow but steady # download. NDHA streams some masters to the Lambda at under 1 MB/s (FL73848659, 45.8 MB, ran # into the old 60 s function timeout with nothing logged), hence the whole-download deadline, -# set below the 120 s function timeout so a slow source fails as a logged 502 with time to spare +# set well inside the 180 s function timeout so a slow source fails as a logged 502 with time to spare # for decoding the ones that make it (see the arithmetic on Timeout in template.yaml). DOWNLOAD_TIMEOUT = int(os.environ.get("DOWNLOAD_TIMEOUT", "35")) # seconds DOWNLOAD_DEADLINE = int(os.environ.get("DOWNLOAD_DEADLINE", "90")) # seconds, whole download @@ -102,6 +102,11 @@ def _reject_if_unsafe(url): class _SafeRedirectHandler(HTTPRedirectHandler): """Re-validate every redirect target before following it (see _reject_if_unsafe).""" + # The real sources redirect at most once (Recollect -> presigned S3). Each hop can block for + # DOWNLOAD_TIMEOUT before the deadline applies, so urllib's default of 10 hops could outlast + # the function timeout. + max_redirections = 3 + def redirect_request(self, req, fp, code, msg, headers, newurl): _reject_if_unsafe(newurl) return super().redirect_request(req, fp, code, msg, headers, newurl) diff --git a/docs/ACCESS-CONTROL.md b/docs/ACCESS-CONTROL.md index 513c303..5ed711c 100644 --- a/docs/ACCESS-CONTROL.md +++ b/docs/ACCESS-CONTROL.md @@ -246,7 +246,7 @@ Every control below is free at this project's scale. Each caps a different failu | Account-wide Lambda concurrency | Parallel executions across **both** functions combined | `10` (this AWS account's total quota — see below) | AWS account-level Service Quota, not this template | | CloudWatch alarm (API) | Notifies on unexpected **volume** | >200 invocations/hour → email | `NZImageApiInvocationsAlarm` | | CloudWatch alarm (converter) | Notifies on, and stops, a converter **spike** | >100 invocations in 5 minutes → email + kill switch | `Jp2ConverterInvocationsAlarm` | -| Converter kill switch | Converter **spend**: sets its concurrency to 0 on a spike, or when month-to-date usage nears the free tier | 320,000 GB-s, 800,000 requests or 20,000 converter calls a month | `ConverterGuardFunction` (`guard/app.py`), see `.claude/rules/converter.md` | +| Converter kill switch | Converter **spend**: sets its concurrency to 0 on a spike, or when month-to-date usage nears the free tier | 250,000 GB-s, 800,000 requests or 20,000 converter calls a month | `ConverterGuardFunction` (`guard/app.py`), see `.claude/rules/converter.md` | | Monthly budget | Notifies on any **real spend** | >$1 actual or forecasted → email | `MonthlyCostBudget` | **The kill switch is the only reserved concurrency, and it is set at runtime.** `ConverterGuardFunction` diff --git a/guard/app.py b/guard/app.py index e2dbad6..b6a89bf 100644 --- a/guard/app.py +++ b/guard/app.py @@ -35,7 +35,7 @@ FUNCTION_NAME = os.environ.get("CONVERTER_FUNCTION_NAME", "") SPIKE_ALARM_NAME = os.environ.get("SPIKE_ALARM_NAME", "") ALERT_TOPIC_ARN = os.environ.get("ALERT_TOPIC_ARN", "") -MAX_GB_SECONDS = float(os.environ.get("MAX_GB_SECONDS", "320000")) +MAX_GB_SECONDS = float(os.environ.get("MAX_GB_SECONDS", "250000")) MAX_REQUESTS = float(os.environ.get("MAX_REQUESTS", "800000")) MAX_CONVERTER_INVOCATIONS = float(os.environ.get("MAX_CONVERTER_INVOCATIONS", "20000")) SPIKE_COOLDOWN_HOURS = float(os.environ.get("SPIKE_COOLDOWN_HOURS", "6")) diff --git a/template.yaml b/template.yaml index 9822906..2e88e8b 100644 --- a/template.yaml +++ b/template.yaml @@ -48,11 +48,13 @@ Parameters: ConverterGuardFunction switches the converter off. GuardMaxGbSeconds: Type: Number - Default: 320000 + Default: 250000 Description: > Month-to-date Lambda compute (GB-seconds, all functions) at which the converter is - switched off. The always-free allowance is 400,000, and the gap absorbs the guard's hourly - lag and the Duration metric not counting init time. + switched off. The always-free allowance is 400,000. The gap is about two hours of the + worst-case burn (10 concurrent 2 GB converters, ~72,000 GB-s an hour, which a flood of slow + calls can reach without tripping the spike alarm), to cover the hourly check, metric lag + and init time the Duration metric leaves out. GuardMaxRequests: Type: Number Default: 800000