Skip to content

Keep distinct Fn::GetStackOutput values distinct - #259

Open
owevertonguedes wants to merge 1 commit into
aws-cloudformation:mainfrom
owevertonguedes:fix/247-get-stack-output-identity
Open

Keep distinct Fn::GetStackOutput values distinct#259
owevertonguedes wants to merge 1 commit into
aws-cloudformation:mainfrom
owevertonguedes:fix/247-get-stack-output-identity

Conversation

@owevertonguedes

@owevertonguedes owevertonguedes commented Aug 2, 2026

Copy link
Copy Markdown

Closes #247

The cause

The GetStackOutput arm in src/template-model/src/resolver.rs resolved its arguments only to keep current_path in step, discarded the results with let _ = ..., and returned a constant ResolvedValue::Dynamic { reason: "cross-stack output" }. StackName, Region and OutputName never reached the resolved value, so every call in a template compared equal and W9007 saw an array of identical entries.

The change

The arm now mirrors the ImportValue arm directly above it and folds the arguments that identify which output is read into the resolved value. Three details are deliberate:

  • Only arguments that resolve to a concrete string contribute. When none does, the reason stays the plain "cross-stack output" constant, the same fallback ImportValue uses for a non-concrete argument.
  • The parts are emitted in a fixed key order, not template order, so the same call written with its arguments in a different order stays equal. resolve_get_stack_output_distinct_sources_differ covers this.
  • Each value is quoted, so a separator that appears inside a value cannot read as a field boundary.

Measured, before and after

Both engines, on the template from the issue and on the two boundary cases below, counting W9007 diagnostics at --level debug:

template main rego main cel this branch rego this branch cel
the issue's three distinct outputs 1 1 0 0
two reads distinguished only by a Ref OutputName 1 1 1 1
same output, one entry naming Region and one omitting it 1 1 0 0

Two scope decisions, both separable

RoleArn is excluded from the identity. The issue names StackName + Region + OutputName, and I kept to that. Two calls that differ only in RoleArn still name the same output of the same stack, so treating them as distinct would suppress a duplicate W9007 should report. resolve_get_stack_output_distinct_sources_differ pins that decision, so if you want RoleArn in, it is a one-line change plus that assertion.

I kept ResolvedValue::Dynamic rather than switching to TypedDynamic. The issue suggests considering TypedDynamic with PARAM_TYPE_STRING for parity with ImportValue. I did not, because describe_resolution in src/schema-validator/src/validate.rs renders TypedDynamic as format!("parameter '{}' (type {})", name, typ), so the diagnostic's resolutionSource would read parameter 'cross-stack output: StackName="VpcStack", ...' (type String), which is wrong for a cross-stack read. Happy to make the switch if you would rather change that rendering too.

Two limitations I did not fix

Both are visible in the table above and I would rather state them than have you find them.

  1. An identifying argument that is itself deploy-time cannot contribute. Two reads whose OutputName comes from different Refs still collapse and still fire W9007. This is not a regression, main fires there too, and it is the same behaviour ImportValue has with a non-concrete argument. Closing it would mean putting a symbolic description of an unresolved value into a user-visible string, which felt like a separate decision rather than part of this fix.

  2. An omitted Region no longer matches an explicit one. On main these two entries collapsed and W9007 fired, which happened to be right when the deployment region is the one written out. On this branch they stay distinct and W9007 is silent. I left it that way on purpose: the engine cannot know the deployment region, so it cannot show that the two entries read the same output. If you would rather have the absent Region normalised to the assumed region, that is a small follow-up, but it puts a region the template never wrote into resolutionSource for every such call.

Tests

  • src/resources/templates/gh-issues/issue-247.json holds the issue's own snippet, following the issue-52.json precedent.
  • issue_247_no_w9007_on_distinct_getstackoutput, issue_247_each_identifying_argument_distinguishes_on_its_own (the three rows of the probe table in the issue, StackName, Region and OutputName each on its own), and issue_247_w9007_still_fires_on_repeated_getstackoutput as the positive boundary.
  • resolve_get_stack_output_carries_source_identity and resolve_get_stack_output_distinct_sources_differ at the resolver level, matching the pair that already exists for ImportValue.

Reverting the production arm to the constant reason fails both resolver tests and two of the three integration tests. issue_247_w9007_still_fires_on_repeated_getstackoutput passes either way by construction, exactly as its issue_52 counterpart does, since the old constant reason also made identical calls compare equal.

Checks

cargo test --workspace passes: 44 test binaries, 1801 tests, 0 failures. cargo fmt --all and cargo clippy --locked --all-targets --workspace -- -D warnings are clean, and git diff --check is empty. The golden was regenerated with cargo run --release -p resources --example generate_golden, which reports 595 templates and Engine parity verified: rego == cel on all 595 templates. Its only content change outside the new fixture is one resolutionSource string that now carries the stack, region and output name.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Every Fn::GetStackOutput resolved to the same constant
ResolvedValue::Dynamic { reason: "cross-stack output" }, so an array
holding several reads of different stack outputs looked like a list of
identical values and tripped the duplicate-values warning.

Carry the arguments that identify which output is read (StackName,
Region, OutputName) into the resolved value, the way the ImportValue arm
already folds in the export name. Only arguments that resolve to a
concrete string contribute; when none does, the reason stays the plain
constant. The parts are emitted in a fixed key order so the same call
written with its arguments in a different order stays equal, and each
value is quoted so a separator inside a value cannot read as a field
boundary.

RoleArn is left out on purpose: two calls differing only there still
name the same output of the same stack, so they are the same value and
should still be reported as a duplicate.
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.

W9007: distinct Fn::GetStackOutput values are treated as duplicates

1 participant