From 9f213947c097d033e7081311193438d29cd99749 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Mon, 24 Aug 2026 21:55:45 +1200 Subject: [PATCH] fix(ci): deploy against the bootstrap artefact stores, not the superseded ones Commit 3fa8f68 put the deploy-artefact stores under IaC in the separate `nzimageapi-bootstrap` stack and wired local deploys to them through `samconfig.toml`. CI was never wired up: the deploy job still passed `--resolve-s3`, and its `CONVERTER_ECR_REPO` environment variable still held the pre-migration companion-stack repo. Every CI deploy therefore pushed a ~195 MB image into the unmanaged ECR repo and a zip into a SAM-auto-created bucket, which is the cost problem 3fa8f68 set out to solve. `samconfig.toml` is gitignored, so CI cannot read the wiring. The deploy job now resolves the same two values from the bootstrap stack's outputs at deploy time and passes them as `--s3-bucket` and `--image-repositories`. Reading the stack rather than adding a second GitHub variable is the point: a hand-maintained copy going stale is exactly what caused this. 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. `|| outputs='[]'` and `jq '.[]?'` keep the missing-stack and empty-outputs cases on that path instead of aborting on a raw ValidationError under `set -e`. Verified by simulating the step body against a stubbed `aws`: missing stack, null outputs, and one-key-missing all fail through the guard; both-present resolves and writes the step outputs. Against the live stack it returns the bootstrap bucket and repo. `CONVERTER_ECR_REPO` is now unreferenced and can be deleted from the repository's production environment. Co-Authored-By: Claude Opus 5 --- .claude/rules/build-test-deploy.md | 23 +++++++++++++++++++++++ .github/workflows/ci-cd.yml | 29 +++++++++++++++++++++++++++-- CLAUDE.md | 7 ++++++- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/.claude/rules/build-test-deploy.md b/.claude/rules/build-test-deploy.md index 5466a83..2c39299 100644 --- a/.claude/rules/build-test-deploy.md +++ b/.claude/rules/build-test-deploy.md @@ -138,6 +138,25 @@ outputs after any bootstrap-stack change: aws cloudformation describe-stacks --region ap-southeast-2 --stack-name nzimageapi-bootstrap --query 'Stacks[0].Outputs' ``` +### 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 The pre-migration artefact stores still exist and still hold their history: @@ -151,6 +170,10 @@ Both had the same lifecycle policies applied imperatively on 2026-08-24, so they their own rather than sitting there forever. Leave them until a deploy against the new repo and bucket is confirmed working and you are past wanting to roll back to an old image. +Note that CI kept writing to both of them until the deploy job was pointed at the bootstrap stack +(see "The wiring in CI" above), so their newest contents are more recent than the migration date +suggests. + ### When adding a new container-image Lambda A new `PackageType: Image` function does **not** automatically get a managed repo any more, diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index ad83994..7fd255f 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -58,13 +58,38 @@ jobs: use-installer: true # arch-independent SAM CLI install - 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 + id: bootstrap + run: | + set -euo pipefail + # `|| outputs='[]'` so a missing stack reaches the guard below (which explains the fix) + # instead of aborting on a raw ValidationError under `set -e`. The AWS error itself is + # still printed to the log. `.[]?` does the same for a stack that has no outputs at all. + outputs=$(aws cloudformation describe-stacks \ + --region ap-southeast-2 \ + --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" + 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 \ - --resolve-s3 \ - --image-repositories Jp2ConverterFunction=${{ vars.CONVERTER_ECR_REPO }} \ + --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 52454ce..8f715ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,7 +122,12 @@ Secrets live in gitignored `.env`, `samconfig.toml`, and `.consumer-secrets/` 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). Adding a new container-image Lambda + 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: [`.claude/rules/build-test-deploy.md`](.claude/rules/build-test-deploy.md#deploy-artefact-retention-cost-control).