Skip to content

fix(cloudformation): unique generated names, and replacement that can roll back - #335

Merged
Neaox merged 3 commits into
mainfrom
fix/cfn-unnamed-resource-collisions
Jul 28, 2026
Merged

fix(cloudformation): unique generated names, and replacement that can roll back#335
Neaox merged 3 commits into
mainfrom
fix/cfn-unnamed-resource-collisions

Conversation

@Neaox

@Neaox Neaox commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Found during alpha.26 release smoke testing, when a real cdk deploy failed. Two coupled defects.

1. Unnamed resources collided

lambda CreateFunction: HTTP 409:
{"__type":"ResourceConflictException","message":"Function already exist: OvercastSmokeCsc-Function"}

Physical names came from the stack alone:

funcName = fmt.Sprintf("%s-Function", rCtx.StackName)

So every unnamed resource of a type in a stack got the same name and the second collided with the first. CDK almost never sets FunctionName — it lets CloudFormation generate a physical ID — so a stack with two Lambda functions could not deploy at all. Very likely the cause of the long-standing cdk/cdk-lifecycle/Deploy compat failure.

Fourteen handlers shared the fallback, so this was never Lambda-only: queues, tables, topics, buckets, roles, policies, instance profiles, secrets, log groups, log streams, SSM parameters, layer versions and event buses all collided the same way. Two unnamed queues resolved to the identical URL.

Names now follow CloudFormation's own shape — {StackName}-{LogicalID}-{RANDOM} — via a single resolveContext.generatedName(), capped per service (generatedNameWithin) since adding the logical ID made them longer. Truncation keeps the unique suffix, because that is what guarantees uniqueness.

2. A failed replacement destroyed the resource

Replacement deleted the old resource before creating the new one, and rollbackUpdate only ever deleted resources — it had no path to restore one. So a replacement whose create failed left the resource gone entirely, with nothing to roll back to.

The test for exactly that scenario, run against the old code:

--- FAIL: TestUpdateStack_failedReplacementKeepsTheOriginalResource
    expected queue "replace-rollback-v1" to exist; queues=[]

Both the original and the replacement gone. Real CloudFormation creates the replacement first and deletes the original only after the whole update commits, which is what makes rollback possible.

Now:

  • Replacement creates first and returns the old physical ID to the caller.
  • Old resources are deleted in a cleanup phase that runs only once every resource updated successfully.
  • On failure, rollback deletes the replacement and keeps the original.

This is what the random name component is for: the two must coexist for the window between create and cleanup.

3. Fallout caught while doing the above

Both IAM policy handlers compared a freshly generated name against the stored one to detect a rename. With a random suffix that differs every time, which would have forced a replacement on every update of an unnamed policy. An unnamed resource now keeps the name it was generated with — CloudFormation does not rename one on update.

Verification

Failing-first, all four new tests fail on the old code with the symptoms above.

Real cdk deploy of a stack with two unnamed functions and two unnamed queues — the exact case that failed:

✅  OvercastUnnamed
OvercastUnnamed.AlphaName  = OvercastUnnamed-Alpha5E55F45A-52LDPKUAF4Z8
OvercastUnnamed.BetaName   = OvercastUnnamed-BetaC60672CA-HU8OICQ1P7AR
OvercastUnnamed.FirstUrl   = .../OvercastUnnamed-First8D4707F1-S1EHJD3YC0K6
OvercastUnnamed.SecondUrl  = .../OvercastUnnamed-Second394350F9-ZXDCINM9X1SU

Both functions invoke and return their own payloads ({"from":"alpha"} / {"from":"beta"}), so they are genuinely separate resources.

go vet ./...        # clean
go test ./...       # all pass

Two existing tests hardcoded the old generated names and now derive them from the stack output instead, since a generated name is unpredictable by design.

Note for reviewers

iamPolicyHandler builds its physical ID as {StackName}-{policyName}, so an unnamed policy's ID now reads {Stack}-{Stack}-{Logical}-{RANDOM}. It is self-consistent — Update strips the prefix to recover the name — and I left the format alone rather than change a stored-state shape in this PR, but it is worth tidying.

Neaox added 3 commits July 28, 2026 13:18
… roll back

Two coupled defects in how CloudFormation handles resources a template does
not name.

Names came from the stack alone — {StackName}-Function — so every unnamed
resource of a type in a stack got the same name and the second collided with
the first: "ResourceConflictException: Function already exist". CDK almost
never names its resources, leaving CloudFormation to generate one, so a stack
with two Lambda functions could not deploy at all. Fourteen handlers shared
the fallback, so queues, tables, topics, buckets, roles, policies, instance
profiles, secrets, log groups, log streams, SSM parameters, layer versions and
event buses were all affected. Names now follow CloudFormation's own shape,
{StackName}-{LogicalID}-{RANDOM}, capped to each service's limit with the
unique suffix preserved, since adding the logical ID made them longer.

Replacement deleted the old resource before creating its replacement. If the
update then failed anywhere, the original was already gone and rollback had
nothing to restore: a test of exactly that leaves ListQueues returning [] on
the old code — the update destroyed the resource outright. Replacement now
creates first and hands the original back for a cleanup phase that runs only
after the whole update succeeds. A failure deletes the replacement and keeps
the original, which is what makes the update recoverable and what the random
name component exists for — the two have to coexist.

Both IAM policy handlers compared a freshly generated name against the stored
one to detect a rename, which with a random suffix would differ every time and
force a replacement on every update. An unnamed resource now keeps the name it
was generated with.
Checked the new replacement path against the CloudFormation docs. The ordering
matches — "CloudFormation usually creates the replacement resource first ...
and then deletes the old resource" — but the phase that does the deleting is
observable on AWS and Overcast was not reporting it: an update went straight
from UPDATE_IN_PROGRESS to UPDATE_COMPLETE.

Adds UPDATE_COMPLETE_CLEANUP_IN_PROGRESS and
UPDATE_ROLLBACK_COMPLETE_CLEANUP_IN_PROGRESS, emitted while removing what the
update superseded — resources dropped from the template and the originals that
replacements replaced. This is the state a stack visibly sits in when a
leftover resource will not delete, which is the whole reason AWS surfaces it
separately rather than folding it into the in-progress status.

changeSetExecStatus already maps unknown statuses to EXECUTE_IN_PROGRESS, which
is correct for both.
…ements

Two defects found while checking the retain path against AWS.

A rollback rebuilt the stack's resource list from the caller's `existing` map,
which the update empties as it processes each resource. By the time rollback
ran it held only what the update had not reached, so every resource already
handled was dropped: after a failed update the stack reported zero resources
while those resources still existed, and the next update would try to create
them on top of themselves. Rollback now restores from an untouched snapshot
taken before the update starts.

UpdateReplacePolicy=Retain conflated two separate facts. Retain is about the
*original* — do not delete it during cleanup — but the resource was still
replaced, and rollback has to remove the replacement the failed update created
regardless. Returning "not replaced" for retained resources meant rollback left
the replacement behind, so both it and the retained original survived with
nothing recording which the stack owned. updateResource now returns a
resourceUpdateOutcome carrying the two facts separately.

Both are pinned by tests that fail on the previous behaviour: the first with
resources=map[], the second with the replacement still listed alongside the
retained original.
@Neaox
Neaox merged commit 1e6320c into main Jul 28, 2026
39 checks passed
@Neaox
Neaox deleted the fix/cfn-unnamed-resource-collisions branch July 28, 2026 01:50
Neaox added a commit that referenced this pull request Jul 28, 2026
…oint

Two endpoint bugs (#318, #353) hid behind the same thing: the AWS CLI is not a
representative client. botocore honours AWS_ENDPOINT_URL, while the JS SDK
derives the SQS endpoint from the QueueUrl and .NET/Java v1 use the queue URL
as the request URI. A queue URL minted on an origin the caller cannot dial
therefore passes the CLI and fails a real SDK. Passing --endpoint-url and a 1:1
port mapping mask it further.

That is not something a coded test can carry, so it goes in a guide agents can
find: docs/dev/manual-testing.md, linked from AGENTS.md and tests/AGENTS.md.

It leads with the invariant everything else follows from -- inside a container
localhost is that container, never Overcast -- then the CLI caveat, the
deployment and client shapes worth covering, the split-horizon /etc/hosts
rewriting and why bare localhost cannot be hijacked, the reserved ports rule,
and the CDK specifics that caught #335 and #353.

Also records in the handover doc that flipping OVERCAST_HOSTNAME precedence for
path-style URLs was tried and reverted: with a split-horizon hostname the
configured name is better than the caller's raw address, because it resolves
from both sides of the container boundary.
Neaox added a commit that referenced this pull request Jul 28, 2026
…#353)

* fix(cloudformation): re-mint stack output URLs on the caller's origin

A stack output naming one of Overcast's own path-style URLs was returned with
the origin it was built from at provisioning time. Provisioning runs through
internal requests, so resource handlers have no caller to derive an origin
from and use cfg.ExternalBaseURL(); that value was then handed unchanged to
every later caller, including ones that reached Overcast somewhere else.

With the API port remapped -- docker run -p 4600:4566, which is what
scripts/run-test-instance.sh does -- a stack's queue-URL output named 4566
while the same server's GetQueueUrl returned 4600. The output was undialable:
AWS SDKs resolve the SQS endpoint from the QueueUrl rather than from client
configuration, so a JS SDK client reading the stack output got the queue URL's
origin substituted for its endpoint and failed against a closed port. The AWS
CLI hid it by honouring AWS_ENDPOINT_URL, and passing --endpoint-url hides it
too, so it only shows up with a default-configured SDK client.

#351 already re-mints host-routed outputs on the caller's origin. This extends
that to path-style URLs whose origin is recognisably Overcast's own -- the
configured external base, or loopback on its listen port -- and leaves
everything else alone, the same restraint the host-routed branch shows. ARNs,
ECR URIs and third-party endpoints are untouched.

Verified against a container published on 4600: the output moved from
localhost:4566 to localhost:4600, matching GetQueueUrl, and the host-side JS
SDK case that previously failed now succeeds.

docs/plans/cfn-output-url-origin.md records the investigation, including why
the AWS CLI cannot reproduce it.

* docs: manual testing guide, and why the AWS CLI cannot verify an endpoint

Two endpoint bugs (#318, #353) hid behind the same thing: the AWS CLI is not a
representative client. botocore honours AWS_ENDPOINT_URL, while the JS SDK
derives the SQS endpoint from the QueueUrl and .NET/Java v1 use the queue URL
as the request URI. A queue URL minted on an origin the caller cannot dial
therefore passes the CLI and fails a real SDK. Passing --endpoint-url and a 1:1
port mapping mask it further.

That is not something a coded test can carry, so it goes in a guide agents can
find: docs/dev/manual-testing.md, linked from AGENTS.md and tests/AGENTS.md.

It leads with the invariant everything else follows from -- inside a container
localhost is that container, never Overcast -- then the CLI caveat, the
deployment and client shapes worth covering, the split-horizon /etc/hosts
rewriting and why bare localhost cannot be hijacked, the reserved ports rule,
and the CDK specifics that caught #335 and #353.

Also records in the handover doc that flipping OVERCAST_HOSTNAME precedence for
path-style URLs was tried and reverted: with a split-horizon hostname the
configured name is better than the caller's raw address, because it resolves
from both sides of the container boundary.

* docs: drop the manual-testing pointer from tests/AGENTS.md

That file is about writing and maintaining coded tests; how to smoke test by
hand is a different concern and already reachable from the root AGENTS.md.

* docs: record that wildcard hostnames do not resolve inside containers

/etc/hosts is an exact-match table, so the split-horizon entries Overcast
injects cover only the apex names. A subdomain — virtual-hosted S3, an
execute-api invoke URL — misses the table and falls through to public DNS,
which answers 127.0.0.1: the container itself. The lookup succeeds, so it
presents as a connection failure rather than a resolution one.

Enumeration cannot fix it. The names are created after the container starts
and ExtraHosts is create-time only. Measured cost rules file size out as the
constraint (100 entries adds ~9us per lookup), so the failure is coverage,
not speed.

Records the mechanism that does work, verified with a throwaway resolver:
Docker's embedded DNS forwards unanswered names to --dns upstreams while
keeping 127.0.0.11 in resolv.conf, so container-name discovery is unaffected.

Not an alpha.26 regression; the server-side host routing already exists and
is simply unreachable from function code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant