Skip to content

fix(toolkit-lib): downstream stack selection uses wrong key for nested stacks - #1835

Open
Adityaj0 wants to merge 1 commit into
aws:mainfrom
Adityaj0:fix/downstream-stack-selection-hierarchical-id
Open

fix(toolkit-lib): downstream stack selection uses wrong key for nested stacks#1835
Adityaj0 wants to merge 1 commit into
aws:mainfrom
Adityaj0:fix/downstream-stack-selection-hierarchical-id

Conversation

@Adityaj0

Copy link
Copy Markdown
Contributor

fixes #1834

Reason for this change

`includeDownstreamStacks` (`packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/stack-assembly.ts`) computes the transitive closure of stacks that depend on the selected set — this is what powers `expand: ExpandStackSelection.DOWNSTREAM`, the default expansion behavior for `cdk deploy`/`cdk destroy` when `--exclusively` is not passed.

for (const [id, stack] of allStacks) {
  if (!selectedStacks.has(id) && (stack.dependencies || []).some(dep => selectedStacks.has(dep.id))) {

`selectedStacks`/`allStacks` are both keyed by `hierarchicalId` (`manifest.displayName ?? id`) — see `indexByHierarchicalId` and `extendStacks` just above, which build these maps via `stack.hierarchicalId`. But the lookup here checks the dependency's raw `.id`, not its `.hierarchicalId`. For a stack whose `displayName` differs from its `id` (stacks inside nested assemblies/stages, e.g. CDK Pipelines), this lookup never matches, so a stack that genuinely depends on a selected stack is silently dropped from the expanded selection.

This was masked for ordinary flat apps, where `displayName` is unset and `id === hierarchicalId`.

The symmetric `includeUpstreamStacks` a few lines below already does this correctly:

for (const dependencyId of stack.dependencies.map(x => x.manifest.displayName ?? x.id)) {
  if (!selectedStacks.has(dependencyId) && allStacks.has(dependencyId)) {

Description of changes

Switches `dep.id` to `dep.hierarchicalId` (the existing getter on `CloudArtifact`, equivalent to `includeUpstreamStacks`'s inline `manifest.displayName ?? id` expression) in `includeDownstreamStacks`.

Description of how you validated changes

Added `test/api/cloud-assembly/stack-selection-expand.test.ts` with two regression tests, using `Toolkit.list()` end-to-end through a `TestCloudAssemblySource` fixture: a dependency stack with an explicit `displayName` distinct from its `stackName`/id, and a second stack that depends on it.

  • `DOWNSTREAM` expansion (selecting the dependency by its hierarchicalId, expecting the dependent stack to be pulled in) — fails on the pre-fix code (dependent stack silently missing from the result) and passes with the fix.
  • `UPSTREAM` expansion (the symmetric, already-correct case) — passes both before and after, confirming the fix doesn't change that path's behavior.

Ran the full `test/api/cloud-assembly/`, `test/actions/list.test.ts`, and the rest of `test/actions/` (deploy, diff, synth, drift, rollback, hotswap) — all pass except `bootstrap.test.ts`, which fails identically with or without this change due to a missing `bootstrap-template.yaml` asset that's only copied by a full `yarn build`, not a bare `tsc --build` compile — unrelated to this change.

Checklist

  • Unit tests added/updated
  • Integration tests added/updated (not applicable — internal stack-selection helper, fully covered by the new unit tests exercising the public `Toolkit.list()` API end-to-end)
  • No manual edits to generated files

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…d stacks

includeDownstreamStacks looked up dependencies with `selectedStacks.has(dep.id)`,
but selectedStacks/allStacks are keyed by hierarchicalId (manifest.displayName
?? id) everywhere else in this file. For a stack whose displayName differs
from its raw id - i.e. stacks inside nested assemblies/stages - this lookup
never matches, so a stack that depends on a selected stack is silently
dropped from a DOWNSTREAM-expanded selection (the default behavior for
`cdk deploy`/`cdk destroy` without `--exclusively`).

The symmetric includeUpstreamStacks already does this correctly via
`x.manifest.displayName ?? x.id`. This switches includeDownstreamStacks to
use the existing `dep.hierarchicalId` getter instead, matching that pattern.

Fixes aws#1834
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(toolkit-lib): downstream stack selection (--exclusively off / expand=DOWNSTREAM) fails to include dependent stacks with a non-trivial displayName

1 participant