fix(core): cross-stack references to NestedStack list values produces invalid outputs #32575

brandondahler · 2024-12-18T15:18:38Z

Issue

Reason for this change

Referencing a list attribute of a resource defined within a NestedStack synthesizes successfully but the nested stack will fail deployment with the error:

Template format error: Every Value member must be a string.

This prevents deploying resources into a NestedStack instance if a reference to one of that resource's list attribute exists within a cross-stack context. For example, deploying a InterfaceVpcEndpoint instance in a nested stack and attempting to reference its vpcEndpointDnsEntries property within a different stack will cause this error.

See new integration test at packages/@aws-cdk-testing/framework-integ/test/core/test/integ.nested-stack-references.ts for minimal reproduction.

Description of changes

A similar strategy to exportStringListValue is used to serialized the reference's values into a string and expose that value as the output from the nested stack. The reference to the serialized value is then made exportable as normally needed to hoist it to the top-level parent stack. The final reference that imports the value is then re-written to also deserialize the imported string back to the original list.

The return types of some internal methods were modified to handle the fact that core/lib/private/refs.ts's getExportable no longer necessarily returns a Reference. This was needed because an exportable may now be a value derived from a reference instead of only a direct reference.

Describe any new or updated permissions being added

N/A

Description of how you validated changes

Added unit test to verify low-level operation
Added integration test to verify high-level behavior and deploy-ability
- Executed integration test within a personal account to verify success

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

nmussy

Mostly nits about coverage, LGTM overall though 👍

packages/@aws-cdk-testing/framework-integ/test/core/test/integ.nested-stack-references.ts

packages/aws-cdk-lib/core/lib/private/refs.ts

packages/aws-cdk-lib/core/test/stack.test.ts

packages/@aws-cdk-testing/framework-integ/test/core/test/integ.nested-stack-references.ts

… invalid outputs Referencing a list attribute of a resource defined within a NestedStack synthesizes successfully but the nested stack will fail deployment with the error: ``` Template format error: Every Value member must be a string. ``` This prevents deploying resources such into NestedStack instances if a reference to that resource's list attribute needs to be referenced in a cross-stack context. For example, deploying a `InterfaceVpcEndpoint` instance in a nested stack and attempting to reference its `vpcEndpointDnsEntries` property within a different stack will cause this error. To fix this issue, a similar strategy to `exportStringListValue` is used to join the reference's values into a string and expose that value as the output from the nested stack. The reference to the serialized value is then re-exported as normally needed to hoist it to the top-level parent stack. The final reference that imports the value is then re-written to also deserialize the imported string back to the original list. fixes aws#27233 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

nmussy

LGTM, thanks for the changes

packages/aws-cdk-lib/core/lib/private/refs.ts

kaizencc

sorry for the super late review @brandondahler. my main concern is the change between Reference and Intrinsic. Let's figure that out, and then I'm happy to send this one into main!

kaizencc · 2025-02-19T15:49:22Z

packages/aws-cdk-lib/core/lib/private/refs.ts

@@ -321,17 +334,23 @@ function createNestedStackOutput(producer: Stack, reference: Reference): CfnRefe
 *
 * Will create Outputs along the chain of Nested Stacks, and return the final `{ Fn::GetAtt }`.
 */
-export function referenceNestedStackValueInParent(reference: Reference, targetStack: Stack) {
+export function referenceNestedStackValueInParent(reference: Reference, targetStack: Stack): Intrinsic {


why are we returning an Intrinsic here? it should be a Reference right.

We have to use Intrinsic here because we're reversing the tokenized list via Fn.split (line 354). We're having to use Fn.split because we're having to do a join when creating the output within the nested stack since the output cannot be a list.

Prior to this update, the values returned from here were only able to reference strings and therefore were just outer stack Reference's to the nested stack's resource attribute. After this update, the values returned from here may either reference a string or a list. In the list case, it is an implementation detail of this method that we're having to re-split the Reference on the nested stack's resource out to its list representation again (and therefore are returning an Intrinsic instead).

Ultimately the choice from a type perspective was either returning an Intrinsic or an IResolvable, either would be correct per the type hierarchies involved.

kaizencc · 2025-02-19T15:50:06Z

packages/aws-cdk-lib/core/lib/private/refs.ts

@@ -260,7 +269,7 @@ function generateExportName(importStack: Stack, reference: Reference, id: string
  return prefix + localPart.slice(Math.max(0, localPart.length - maxLength + prefix.length));
 }

-export function getExportable(stack: Stack, reference: Reference): Reference {
+export function getExportable(stack: Stack, reference: Reference): Intrinsic {


i understand that this is a private function but I am concerned about the downgrade from returning a Reference to an Intrinsic. i want to learn more about why thats something you've deemed necessary.

This ultimately changed because referenceNestedStackValueInParent changed, see that comment for more details. TL;DR Fn.split returns an Intrinsic (not a Reference) when untokenized.

kaizencc

Thanks @brandondahler!

mergify · 2025-02-24T18:50:06Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2025-02-24T18:50:11Z

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

mergify · 2025-02-24T18:54:37Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

brandondahler · 2025-02-24T22:38:37Z

@Mergifyio requeue

mergify · 2025-02-24T22:39:12Z

requeue

❌ Command disallowed due to command restrictions in the Mergify configuration.

sender-permission >= write

aws-cdk-automation · 2025-02-25T16:29:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 8968e4a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-02-25T16:30:11Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-02-25T16:30:30Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

brandondahler requested a review from a team as a code owner December 18, 2024 15:18

github-actions bot added bug This issue is a bug. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Dec 18, 2024

aws-cdk-automation requested a review from a team December 18, 2024 15:18

brandondahler changed the title ~~fix(core): cross-stack references to NestedStack list values produces invalid outputs (#27233)~~ fix(core): cross-stack references to NestedStack list values produces invalid outputs Dec 18, 2024

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 18, 2024

nmussy suggested changes Dec 19, 2024

View reviewed changes

brandondahler force-pushed the features/NestedStackListReference branch from 8219bff to 28721e3 Compare December 20, 2024 00:51

Merge branch 'main' into features/NestedStackListReference

9dfbc83

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 20, 2024

nmussy mentioned this pull request Dec 20, 2024

integ-tests: assertion chaining do not fail as expected #32604

Open

1 task

nmussy approved these changes Dec 20, 2024

View reviewed changes

github-actions bot mentioned this pull request Jan 1, 2025

Monthly PR metrics report #32703

Closed

Merge branch 'main' into features/NestedStackListReference

0883740

kaizencc reviewed Feb 17, 2025

View reviewed changes

packages/aws-cdk-lib/core/lib/private/refs.ts Outdated Show resolved Hide resolved

Update packages/aws-cdk-lib/core/lib/private/refs.ts

4804135

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 17, 2025

kaizencc reviewed Feb 19, 2025

View reviewed changes

samson-keung added the skip-abstractions-board signal to automated workflow to skip adding to project board label Feb 19, 2025

kaizencc approved these changes Feb 24, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 24, 2025

Merge branch 'main' into features/NestedStackListReference

8968e4a

kaizencc approved these changes Feb 25, 2025

View reviewed changes

mergify bot merged commit f9252ab into aws:main Feb 25, 2025
10 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2025

brandondahler deleted the features/NestedStackListReference branch February 25, 2025 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): cross-stack references to NestedStack list values produces invalid outputs #32575

fix(core): cross-stack references to NestedStack list values produces invalid outputs #32575

brandondahler commented Dec 18, 2024

nmussy left a comment

nmussy left a comment

kaizencc left a comment

kaizencc Feb 19, 2025

brandondahler Feb 20, 2025 •

edited

Loading

kaizencc Feb 19, 2025

brandondahler Feb 20, 2025

kaizencc left a comment

mergify bot commented Feb 24, 2025

mergify bot commented Feb 24, 2025

mergify bot commented Feb 24, 2025

brandondahler commented Feb 24, 2025

mergify bot commented Feb 24, 2025

aws-cdk-automation commented Feb 25, 2025

mergify bot commented Feb 25, 2025

github-actions bot commented Feb 25, 2025

fix(core): cross-stack references to NestedStack list values produces invalid outputs #32575

fix(core): cross-stack references to NestedStack list values produces invalid outputs #32575

Conversation

brandondahler commented Dec 18, 2024

Issue

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

nmussy left a comment

Choose a reason for hiding this comment

nmussy left a comment

Choose a reason for hiding this comment

kaizencc left a comment

Choose a reason for hiding this comment

kaizencc Feb 19, 2025

Choose a reason for hiding this comment

brandondahler Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

kaizencc Feb 19, 2025

Choose a reason for hiding this comment

brandondahler Feb 20, 2025

Choose a reason for hiding this comment

kaizencc left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 24, 2025

mergify bot commented Feb 24, 2025

mergify bot commented Feb 24, 2025

brandondahler commented Feb 24, 2025

mergify bot commented Feb 24, 2025

❌ Command disallowed due to command restrictions in the Mergify configuration.

aws-cdk-automation commented Feb 25, 2025

AWS CodeBuild CI Report

mergify bot commented Feb 25, 2025

github-actions bot commented Feb 25, 2025

brandondahler Feb 20, 2025 •

edited

Loading