~/definitionOfDone.dev/blog/fitness-functions-for-infrastructure.md

8 min read

The Deploy-Time Bug You Can Catch at Pull-Request Time

There's a bug that only exists in estates with nested infrastructure stacks, and everyone who runs them has met it.

TL;DR

  • Nested infrastructure stacks fail in one characteristic way: a child template grows a required parameter and the parent never passes it. Nothing catches it until the deploy — which in CloudFormation means during the rollback.
  • It's entirely statically detectable. A couple of hundred lines of script, run on every pull request, walks the stack tree and checks that every required input is supplied and every reference resolves.
  • Write these checks at the scale of the problem. No framework, no plugin system, whatever language your CI already has. The correct level of rigour is set by the cost of the check's failure modes, not by the standards of the code it inspects.
  • Make them block merges. A finding that doesn't block is a finding that gets ignored.
  • Write down what the check cannot see, in the file, next to the check. The danger of automation isn't that it misses things — it's that people stop looking at what it covers, and the gaps become invisible.
  • Other invariants worth this treatment: every queue has a redrive policy, every visibility timeout exceeds its consumer's timeout, no template contains a literal account ID, every route is covered by an authorizer or listed as a deliberate exception.

There's a bug that only exists in estates with nested infrastructure stacks, and everyone who runs them has met it.

A child template grows a new required parameter. The parent isn't updated to pass it. Nothing complains — not the editor, not the build, not the packaging step. Review doesn't catch it either, because comparing a parameter list in one file against a template you don't have open is not a thing humans are good at.

Then the deploy runs, works through half the tree, and fails. And because this is CloudFormation, "fails" means it now rolls back — which on a large estate is the slowest possible way to discover a two-line mistake. You get to watch it happen, in order, for several minutes, knowing exactly what it's doing.

The frustrating part is that everything needed to catch it was in the repository the whole time.

The check

Two invariants, both statically checkable, both worth about the same amount:

Every required input is supplied. Read the parameters a template declares. Filter to those with no default value — those are the required ones. Check each appears in what the parent passes.

The "no default" filter is doing real work here, and it's worth being deliberate about it in your own templates. A parameter with a default is optional by definition. A parameter without one is part of the contract. That distinction turns a stylistic choice into a machine-readable statement of intent, which is a nice property to get for free.

Every reference resolves. Walk the template's resources and globals, collect everything referenced by an intrinsic function, and check each one against the union of that template's own parameters and its own logical resource IDs. A !Ref to something the template can't see is an error.

Then recurse. For each nested stack resource, take the parameter block the parent passes as the child's supplied inputs, and run both checks against the child's template. All the way down.

mermaidsnippet
flowchart TD
    CFG[Deployment config<br/>supplied parameter keys] --> R[Root template]
    R --> C1{params without defaults<br/>all supplied?}
    R --> C2{every Ref / GetAtt / Sub<br/>target resolvable?}
    R --> F[find nested stack resources]
    F -->|parent's Parameters block<br/>becomes child's supplied keys| CH[Child template]
    CH --> C1
    CH --> C2
    CH --> F

The recursion is where the value is. The parent-to-child parameter block is a contract, and this script is the only thing in the pipeline that reads both sides of it at once.

Why I'd call this a fitness function and not a linter

A linter checks conformance to a style. An architectural fitness function checks that a structural property you actually depend on still holds, and it earns its place by being tied to a failure you've personally experienced.

That distinction tells you when to write one. Not "we should have more checks" — that way lies a CI pipeline nobody reads. Write one when you can name a failure that satisfies all three of:

  1. It has bitten you.
  2. It's expensive to discover late.
  3. It's statically detectable.

Most "we should validate this" ideas fail the third. The nested-stack parameter bug passes all three comfortably, which is why it's the best first one to write.

The economics are lopsided in the way good tooling usually is. A few seconds per pull request against a failed rollout, a locked stack, and someone reading CloudFormation events trying to work out which of a hundred parameters was wrong.

Build it badly on purpose

Here's the part I'd push back on if someone told me they were waiting until they had time to do it properly.

CloudFormation's YAML short tags — !Ref, !GetAtt, !Sub — will be rejected by a standard YAML parser, because they're custom tags needing a schema. The correct fix is to define that schema and hand it to the parser.

You don't have to. You can read the file as text, replace the tags with plain markers, parse the result, and walk the object graph looking for your markers.

One thing to get right, because it's the difference between a script that runs and one that dies on your second template: don't enumerate the tags. There are around twenty, and a substitution covering !Ref, !GetAtt and !Sub throws a parse error the moment it meets !If, !Equals, !Join, !Select, !FindInMap, !ImportValue or !Base64. Substitute the shape instead:

jssnippet
const text = readFileSync(path, 'utf8')
  .replace(/!([A-Za-z]\w*)/g, '__TAG_$1__');
const template = yaml.load(text);

Every intrinsic now survives as a plain string carrying its own name, so finding them is a prefix test on a recursive walk:

jssnippet
function* tags(node) {
  if (typeof node === 'string') {
    const m = node.match(/^__TAG_(\w+)__\s*(.*)$/);
    if (m) yield { fn: m[1], arg: m[2] };
  } else if (Array.isArray(node)) {
    for (const child of node) yield* tags(child);
  } else if (node && typeof node === 'object') {
    for (const child of Object.values(node)) yield* tags(child);
  }
}

!Ref and the single-argument forms carry their target in arg. !GetAtt and !Sub need one case each to pull the logical ID out of the string or the array that follows. That's the whole reference check.

This is crude. It misfires on any exclamation mark followed by a word — !Important in a description becomes a tag. It's the sort of thing that gets a comment in code review.

It's also completely fine, and I'd write it. The script has one job, runs against templates you control, and the cost of being wrong is a false positive — someone reads the message, sees it's spurious, moves on. Thirty seconds.

The correct level of rigour for a tool is set by the cost of its failure modes, not by the standards of the code it inspects. The real alternative to the crude version isn't the elegant version. It's not having the check at all, which is what happens when "do it properly" means a week nobody can schedule.

Document the blind spot — it's the most valuable part

Every static check has a hole. Conditionals are the usual one, because resolving them statically means evaluating them, which means knowing parameter values, which means modelling the tool's own evaluation semantics. Reasonable to skip.

But skipping it has a consequence, and the consequence needs writing down.

If your shared layer, your authorizer, or your environment switch is wired through a conditional expression, then that check cannot see your highest-blast-radius parameter. It'll pass a template where the single value affecting hundreds of functions is wrong. And because the check is green, nobody looks.

That's the actual danger of automated checks. Not that they miss things — everything misses things — but that coverage creates the impression of coverage, and the uncovered area becomes invisible precisely because something appears to be watching it.

So put a comment at the top of the file: what it checks, what it deliberately doesn't, and what that omission means for this estate specifically. "Does not evaluate conditionals" is a fact. "Does not evaluate conditionals, which means shared layer wiring is unverified" is information.

Same applies when you add a new deployment path. A check written for the architecture that hurt won't automatically cover the architecture you're migrating to. That gap should be a decision, not a default.

Other invariants worth the same treatment

Once you have the pattern, the list writes itself. All of these are a morning's work and all of them otherwise fail at three on a Friday afternoon:

  • Every SQS queue has a redrive policy. A queue without one loses poison messages silently, and a FIFO queue without one blocks its message group forever.
  • Every queue's visibility timeout exceeds its consumer's function timeout — comfortably, not by one second. Equal values mean a function running to its wall clock can have its message redelivered while the first invocation is still working.
  • No template contains a literal account ID. Pins your estate to one account and turns a disaster-recovery exercise into a find-and-replace.
  • Every API route is covered by an authorizer, or named in an explicit exception list. The exceptions are the interesting part — an unauthenticated endpoint somebody chose is a position, an unauthenticated endpoint nobody noticed is a finding.
  • Every Lambda has a log retention policy. The default is "forever", which is a bill.
  • No function's timeout exceeds its API Gateway integration timeout. Otherwise the gateway gives up first and your error handling never runs.

Pick the two that map to incidents you've actually had. Ignore the rest until they earn a place.

How to build one

Small. A few hundred lines, in whatever your CI already runs. The moment it needs its own configuration file you've overbuilt it.

Blocking. If it isn't worth failing a build over, it isn't worth writing. Advisory checks are noise with a status icon.

Runnable by hand. One idiom worth copying: make the file work as both a module and a CLI, so the same code CI imports is what a developer runs locally to debug a failure. It's the difference between a tool people fix and a tool people route around.

Honest. Comment the gaps.

The deployment failure you can catch with a static check is the cheapest bug in your system. It's also the one most teams keep paying for, because the fix looks too unglamorous to put on a board.

newer Stop Hand-Writing Deployment Configuration older A Lambda Layer Is Not a Library