fix(ci): deploy against the bootstrap artefact stores, not the superseded ones - #49
Merged
Merged
Conversation
…eded 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 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Commit 3fa8f68 put the deploy-artefact stores under IaC in the separate
nzimageapi-bootstrapstack, and wired local deploys to them viasamconfig.toml. CI was never wired up:sam deploystill passed--resolve-s3, so it reused the SAM-auto-createdaws-sam-cli-managed-default-...bucket.--image-repositoriescame from theCONVERTER_ECR_REPOenvironment variable (production, set 2026-07-09), which still held the pre-migration companion-stack reponzimageapi40221342/jp2converterfunctione92cbfdcrepo.So every CI deploy kept pushing a ~195 MB image into the unmanaged repo and a zip into the unmanaged bucket, which is the exact cost problem 3fa8f68 set out to solve. Confirmed against the account: the most recent push to the old repo is from today's CI deploy, while the bootstrap repo has only the one image from the local deploy that created it.
The fix
samconfig.tomlis gitignored, so CI cannot read the wiring. A newResolve bootstrap artefact storesstep reads the two values from the bootstrap stack's outputs at deploy time and passes them tosam deployas--s3-bucketand--image-repositories.Reading the stack rather than adding a second GitHub variable is the point of the change: a hand-maintained copy going stale is what caused this in the first place.
If the stack is missing or either output is absent, the step fails the job with a pointer to the bootstrap-deploy command, so a misconfigured deploy never reaches the push.
|| outputs='[]'andjq '.[]?'keep the missing-stack and empty-outputs cases on that guarded path instead of aborting on a rawValidationErrorunderset -e.Verification
The step body was simulated against a stubbed
aws:awsexits 255)nullGITHUB_OUTPUTcorrectlyAgainst the live stack it returns
nzimageapi-sam-artifacts-...and.../nzimageapi/jp2converter. YAML parses. The deploy role carries PowerUserAccess, socloudformation:DescribeStacksis permitted, andubuntu-24.04-armpreinstalls both the AWS CLI and jq.The real proof is the deploy this PR triggers on merge: its artefacts should land in the bootstrap bucket and repo, and nothing new should appear in the superseded ones.
Follow-ups (agreed, not in this PR)
CONVERTER_ECR_REPOenvironment variable.aws-sam-cli-managed-defaultbucket once a CI deploy against the bootstrap stores is confirmed.🤖 Generated with Claude Code