Repository navigation
feat(lambda): stamp the deployed version, and gate that every deploy does (#654) - #768
Merged
Merged
Conversation
…does #654's remaining scope. The self-hosted reaper already stamped spawn:version (pkg/reaperdeploy.Tags, read back by `spawn reaper status` via #670). The spore.host-OPERATED lambdas exposed nothing: no env var, no tag beyond SAM's own, nothing in their source. Verified again before starting rather than trusting the four-day-old comment. Nothing synchronises the CLI with the control plane, so skew is the NORMAL state — defensible, since they are independently deployed. What was not defensible is that it was invisible: answering "is the deployed reaper current?" meant comparing LastModified against `git log -- lambda/<name>/`. That is archaeology, I did it by hand four times in one session, and it is wrong in both directions — a redeploy with no source change moves LastModified, and a source change never deployed does not. Six deploy mechanisms now stamp spawn:version, readable in ONE API call without invoking the function: - three SAM templates take a Version parameter and set the tag there, NOT via `aws lambda tag-resource`. A tag CloudFormation does not declare shows up as stack drift, and the pipeline-orchestrator stack was just brought to IN_SYNC (#754). - three shell deploys call tag-resource, which is correct for them precisely because they are not CFN-managed. The value is `git describe --tags --always --dirty`, so a deploy from an uncommitted tree stamps 0.126.1-1-gabc-dirty and SAYS it was built from uncommitted code instead of claiming a release. ## The gate, and what it found scripts/lambda-version-census.sh, two halves mirroring lambda-runtime-census.sh and verify-pins.sh. The offline half gates every PR: a new lambda cannot arrive unreadable. It checks TWO forms, because a SAM template declares the tag in YAML and a shell deploy passes it to tag-resource — grepping only templates is what left the script-deployed ones out. It earned its keep immediately, finding a mechanism I had missed: scripts/deploy-custom-dns.sh deploys spawn-dns-updater, but its own name says "custom-dns", so enumerating by lambda directory never sees it. The networked half was WRONG first and the first run proved it. It listed five expected function names; the account has sixteen, including `scheduler-handler` with no spawn- prefix at all, and prism-bot / spore-bot / spore-rest-api. A hardcoded list reports a clean bill of health for everything it forgot — the same lesson lambda-runtime-census.sh records about github-oauth-bridge. It now enumerates. Live state it surfaced, previously unavailable at a glance: SIX functions last deployed in February 2026 (alert-evaluator, both autoscale orchestrators, cost-history-collector, both dashboard-websocket functions). An unstamped function reports `?`, not a mismatch — the distinction cmd/reaper_skew.go already draws between "unknown" and "different". Guessing there would be the same error as claiming skew against a dev build. ## Gates confirmed to fail on revert — after being fixed Both matchers initially PASSED a deliberate revert, because `grep -q spawn:version` also matches the string in the parameter's own Description and in the script's echo line. Tightened to require the assignment (`spawn:version: !Ref Version`) and a --tags value (`--tags.*spawn:version=`); both reverts now fail. The ERE also had to move off `[^\n]*`, which excludes a literal `n` and so could never match "spawn:version". All three templates pass `sam validate --lint`. ci.yml parses. Root module, docs-drift, changelog and semgrep clean. Not done here: moving the operated reaper off CloudFormation to the pkg/reaperdeploy Go path, which the issue prefers. That is a much larger change and the version stamp is what made the skew visible.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Oct 9, 2026
scttfrdmn
added a commit
that referenced
this pull request
Oct 11, 2026
… the 15 defects that found (#799) (#800) The issue said lambda/*/deploy.sh was ungated. It was worse: four of the gates had each hardcoded the SAME TWO filenames, so even scripts/deploy-custom-dns.sh — in the directory being scanned — was unchecked. 2 of 8 deploying scripts were actually covered. Discovery is now by what a script DOES (aws lambda create-function, cloudformation deploy, ...) anywhere in the tree, and fails when it finds nothing rather than passing vacuously. Name- and location-based filters have both already failed here: #136's lambda/*-scoped module loop, #790's unprefixed scheduler-handler, and scripts/setup-schedules-dynamodb.sh, which sits in scripts/ and escaped a `deploy-*.sh` glob by its name. ## 15 defects across 6 scripts no account assertion 6 scripts no spawn:version stamp 4 scripts no log retention 4 scripts swallowed CFN failures 1 script The swallowing one is the worst: setup-schedules-dynamodb.sh masked both update-stack and wait failures behind `|| true`, then printed "✅ Stack updated successfully" unconditionally. A failed deploy reported a green tick. It now matches the one error that genuinely means success — "No updates are to be performed" — and aborts on anything else. The missing version stamps have a measured consequence: the live spawn-dashboard-api carries spawn:managed and spawn:purpose but NO version, so #768's "skew is readable in one API call" was false for it, and the census could not say so because it only globbed scripts/deploy-*.sh. Stamping now happens on every deploy, not only on create — which is how a function deployed since February stayed untagged. ## Two places I did NOT "fix" working code Widening a gate surfaces false positives as well as defects, and a gate is not evidence that working code is wrong. - The region gate flagged setup-schedules-dynamodb.sh for taking AWS_REGION. But it documents that, offers --region, echoes "Region: $REGION" before acting, and three regional spawn-schedules buckets exist — it is deliberately multi-region. The gate's hazard is a function SILENTLY created where nothing invokes it, so it is now scoped to Lambda deploys rather than breaking a documented feature. - Two of four `|| true`s are correct: a get-role inside a propagation poll (which is the FIX for the bug TestNoBareSleepAfterIAMMutation guards) and an idempotent remove-permission. Rather than weaken the gate to its most awkward case, those carry an explicit `# swallow-ok: <reason>`, following the nosemgrep convention already used here — honoured on the offending line or the one immediately above, which my own first attempt got wrong by writing a two-line comment. ## The account gate did not catch its own revert Renaming EXPECTED_ACCOUNT= to something else still PASSED, because the script's own error message ("expected $EXPECTED_ACCOUNT") keeps the substring alive. Fifth time in this repo a gate has matched its own prose. It now requires the ASSIGNMENT and the COMPARISON, because either alone asserts nothing: an assignment never compared is dead, and a comparison against an unset variable is always false. Both halves verified against separate compiling reverts, with the right diagnostic each time. And the first verification run reported `(cached)` — Go's test cache cannot see files a test reads at runtime, so that pass meant nothing. The #792 lesson arriving in my own checking. Re-run with -count=1 is when the gate actually failed. ## Also fixed, from deploying it for real earlier lambda/dashboard-api/deploy.sh called update-function-configuration immediately after update-function-code, which Lambda rejects while the first is in flight: exit 254 with the code updated and the configuration not, which looks like total failure. It now waits. Cleanup moved to a trap so it runs on the error paths too, where `set -e` had been leaving 27 MB of artifacts behind. ## Verification scripts module -count=1, census offline half, make check (all 10 nested modules), semgrep 0 findings. Every script `bash -n` clean. Note for a follow-up, deliberately not done here: `set -euo pipefail` is still gated on one script only. Widening it fails several others, and the fix is not mechanical — `set -u` turns an unset variable from an empty string into an abort, which can break a script that works today. That needs per-script verification against a real deploy.
This was referenced Oct 11, 2026
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.
Closes #654's remaining scope.
What was left
spawn reaper deployalready stampedspawn:versionfor the self-hostedreaper, and #670 added the comparison to
spawn reaper status. Thespore.host-operated lambdas exposed nothing — no env var, no tag beyond
SAM's own, nothing in their source. I re-verified that against the live account
before starting rather than trusting the four-day-old comment.
Why it matters, stated precisely
Nothing synchronises the CLI with the control plane, so skew is the normal
state. That is defensible — they are independently deployed. What was not is
that it was invisible: answering "is the deployed reaper current?" meant
comparing a function's
LastModifiedagainstgit log -- lambda/<name>/.That is archaeology, I did it by hand four times in one session, and it is
wrong in both directions — a redeploy carrying no source change moves
LastModified, and a source change never deployed does not.The change
Six deploy mechanisms now stamp
spawn:version, readable in one API callwithout invoking the function:
Versionparameter →TagsIN_SYNC(#754)aws lambda tag-resourceThe value is
git describe --tags --always --dirty, so a deploy from anuncommitted tree stamps
0.126.1-1-gabc-dirtyand says it was built fromuncommitted code instead of claiming a release.
The gate found things on its first run
scripts/lambda-version-census.sh, two halves mirroringlambda-runtime-census.shandverify-pins.sh. The offline half gates every PR,checking two forms — YAML tag and
tag-resourceargument — because greppingonly templates is what left the script-deployed ones out.
A mechanism I had missed:
scripts/deploy-custom-dns.shdeploysspawn-dns-updater, but its own name says custom-dns, so enumerating by lambdadirectory never sees it.
My networked half was wrong, and its first run proved it. It listed five
expected names. The account has sixteen functions — including
scheduler-handlerwith nospawn-prefix at all, plusprism-bot,spore-bot,spore-rest-api. A hardcoded list reports a clean bill of healthfor everything it forgot, which is the lesson
lambda-runtime-census.shalreadyrecords about
github-oauth-bridge. It now enumerates.Live state it surfaced, previously unavailable at a glance — six functions
last deployed in February 2026:
alert-evaluator, both autoscaleorchestrators,
cost-history-collector, and both dashboard-websocket functions.An unstamped function reports
?, not a mismatch — the distinctioncmd/reaper_skew.goalready draws between unknown and different.Gates confirmed to fail on revert — after being fixed
Worth stating plainly: both matchers initially passed a deliberate revert.
grep -q spawn:versionalso matches the string in the parameter's ownDescriptionand in the script's ownecho, so the gate could never have fired.Tightened to require the actual assignment (
spawn:version: !Ref Version) and a--tagsvalue (--tags.*spawn:version=); both reverts now fail correctly.The ERE also had to move off
[^\n]*— grep treats thenas a literal, so thatclass excludes the
ninspawn:versionand could never match.All three templates pass
sam validate --lint;ci.ymlparses; root module,docs-drift, changelog and
semgrep --config=auto --errorclean.Deliberately not here
Moving the operated reaper off CloudFormation onto the
pkg/reaperdeployGopath, which #654 prefers. That is a much larger change, and the version stamp is
the part that makes the skew visible — which was the actual complaint.
Also not here: redeploying anything. Every function keeps its current code;
the tags appear on each one's next deploy, which is why the census reports
?rather than failing today.