Enhance CDK support with nested stack handling #295

flostadler · 2024-12-16T18:18:34Z

This change adds support for Nested Stacks to pulumi-cdk.
In CDK, a nested stack is a CDK stack that you create inside another stack, known as the parent stack. Users create nested stacks using the NestedStack construct.
By using nested stacks, users can organize resources across multiple stacks. Nested stacks also offer a way around the AWS CloudFormation 500-resource limit for stacks. A nested stack counts as only one resource in the stack that contains it. However, it can contain up to 500 resources, including additional nested stacks.
While Pulumi doesn't have those limitations, many existing CDK constructs use nested stacks to get around the CloudFormation limitations. The EKS cluster construct is one of the most prominent ones that require nested stacks.

In detail, this change:

enhances the AssemblyManifestReader to support reading nested stacks.
introduces a new unique StackAddress for elements in the CloudFormation templates. It combines the stack path (tree path of the stack) and the stack-local logical ID in order to form an assembly wide unique address. This is necessary in order to represent a hierarchy of stacks in the same pulumi program
adds a new NestedStackConstruct component. It is used to namespace the resources of a NestedStack in order to not run into any issues with duplicate logical IDs. It achieves this by including the stack path in the pulumi resource type. The types of parent resources are included in URNs, meaning all resources parented under the nested stack have their own unique namespace.
updates the artifact converter and intrinsics to handle cross stack references (Parameters & Outputs) for nested stacks

Integration tests and an example are part of #298 which is part of this PR stack.

Review Advice

I'd recommend first reviewing the changes to the code reading the cloud assembly (src/assembly/manifest.ts), then the GraphBuilder (src/graph.ts) and lastly the app converter and intrinsics (src/converters/app-converter.ts & src/converters/intrinsics.ts). The other changes are in support for those

Fixes #80

- Introduced `StackAddress` type to uniquely identify resources across nested stacks. - Updated `CloudFormationParameterWithId` to use `stackAddress` instead of `id`. - Added `NestedStackTemplate` interface and `isNestedStackTemplate` function for better nested stack management. - Modified various functions and interfaces to accommodate stack paths and addresses, improving resource mapping and retrieval. - Created `StackMap` class for efficient management of resources across stacks. - Updated tests to reflect changes in resource identification and nested stack handling. This update improves the overall structure and usability of arbitrary stacks in the cloud assembly.

flostadler · 2024-12-17T14:11:34Z

This change is part of the following stack:

Enhance CDK support with nested stack handling #295 ◀
- Add integration test and example for using CDK Nested Stacks with Pulumi #298

_{Change managed by git-spice.}

flostadler · 2024-12-17T16:54:48Z

src/converters/app-converter.ts

@@ -367,10 +448,39 @@ export class StackConverter extends ArtifactConverter implements intrinsics.Intr
    /** @internal */
    asOutputValue<T>(v: T): T {


Cutting a follow up ticket for handling cases where multiple nested stacks have a resource with the same logical ID. The users could inform the converter about the right stack by passing it in as an optional argument.

corymhall

This is awesome! My only minor comment is around adding some unit tests to the intrinsics and graph tests.

corymhall · 2024-12-17T16:52:49Z

tests/converters/app-converter.test.ts

+
+        // resources of the nested stack should be mapped
+        // this tests that properties are correctly passed to the nested stack
+        const nestedBucket = converter.resources.get({ stackPath: `${manifest.id}/nesty`, id: 'bucket43879C71'})?.resource as native.s3.Bucket;


Looks like NestedStack resources still have the cdk random suffix (no way around it). More reason not to use nested stacks!

tests/graph.test.ts

tests/converters/intrinsics.test.ts

flostadler · 2024-12-18T13:26:52Z

@corymhall I think this is ready for another look

corymhall

🚀

…umi (#298) This PR adds an integration test and example for CDK Nested stacks. Stacked on top of #295 Related to #80

t0yv0 · 2024-12-18T18:50:19Z

src/assembly/stack.ts

+     * @param logicalId - The logicalId of the nested stack
+     * @returns The path to the nested stack wrapper node in the construct tree
+     */
+    public static getNestedStackPath(nestedStackResourcePath: string, logicalId: string): string {


Nit: type aliaseses instead of string for these things can be helpful to clarify at type system level what's what.

t0yv0 · 2024-12-18T18:51:35Z

src/cfn.ts

+    readonly Metadata?: { [key: string]: any };
+}
+
+export interface CloudFormationOutput {


What does this represent and what can we expect under Value: any?

t0yv0 · 2024-12-18T18:52:11Z

src/converters/app-converter.ts

@@ -95,9 +103,11 @@ export class AppConverter {
 * StackConverter converts all of the resources in a CDK stack to Pulumi resources
 */
 export class StackConverter extends ArtifactConverter implements intrinsics.IntrinsicContext {
-    readonly parameters = new Map<CloudFormationParameterLogicalId, any>();
-    readonly resources = new Map<string, Mapping<pulumi.Resource>>();
+    readonly parameters = new StackMap<any>();


t0yv0 · 2024-12-18T18:55:04Z

src/converters/app-converter.ts

@@ -367,10 +448,44 @@ export class StackConverter extends ArtifactConverter implements intrinsics.Intr
    /** @internal */
    asOutputValue<T>(v: T): T {
        const value = this.cdkStack.resolve(v);
-        return this.processIntrinsics(value) as T;
+        try {
+            return this.processIntrinsics(value, this.stack.constructTree.path) as T;


Can this eventually become a single-pass processIntrinsics?

That is can processIntrinsics absorb this.graph.nestedStackNodes parameter and just not fail but to the right thing.

t0yv0 · 2024-12-18T18:56:47Z

src/converters/intrinsics.ts

@@ -85,22 +96,22 @@ export interface IntrinsicContext {
     *
     * See https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/conditions-section-structure.html
     */
-    findCondition(conditionName: string): Expression | undefined;
+    findCondition(stackAddress: StackAddress): Expression | undefined;


t0yv0 · 2024-12-18T18:57:48Z

src/assembly/types.ts

+ * StackAddress uniquely identifies a resource in a cloud assembly
+ * across several (nested) stacks.
+ */
+export type StackAddress = {


Perhaps ResourceAddress? StackAddress makes it sound like it identifies a Stack. Doc comments on props are also nice, with examples.

t0yv0 · 2024-12-18T18:59:05Z

Very nice. Sorry for late review with some questions. Nothing blocking.

pulumi-bot · 2024-12-23T20:13:51Z

This PR has been shipped in release v1.4.0.

flostadler self-assigned this Dec 16, 2024

flostadler force-pushed the flostadler/nested-stacks branch from d48ac5e to 2caf7b8 Compare December 17, 2024 14:09

flostadler force-pushed the flostadler/nested-stacks branch from 2caf7b8 to 674ff01 Compare December 17, 2024 14:11

flostadler mentioned this pull request Dec 17, 2024

Add integration test and example for using CDK Nested Stacks with Pulumi #298

Merged

flostadler changed the title ~~Nested Stack Support~~ Enhance CDK support with nested stack handling Dec 17, 2024

flostadler requested review from corymhall, t0yv0 and a team December 17, 2024 14:26

flostadler marked this pull request as ready for review December 17, 2024 14:26

flostadler commented Dec 17, 2024

View reviewed changes

corymhall requested changes Dec 17, 2024

View reviewed changes

flostadler added 3 commits December 17, 2024 19:25

Add tests for asOutputValue

158dc33

Add unit tests for graph

e7d4e70

Add tests for intrinsics to make sure they're picking the right stack

1a72004

corymhall approved these changes Dec 18, 2024

View reviewed changes

flostadler merged commit fa10f4a into main Dec 18, 2024
13 checks passed

flostadler deleted the flostadler/nested-stacks branch December 18, 2024 14:13

flostadler added a commit that referenced this pull request Dec 18, 2024

Add integration test and example for using CDK Nested Stacks with Pul…

569f57a

…umi (#298) This PR adds an integration test and example for CDK Nested stacks. Stacked on top of #295 Related to #80

t0yv0 reviewed Dec 18, 2024

View reviewed changes

pulumi-bot mentioned this pull request Dec 23, 2024

Support NestedStacks (cannot create eks.Cluster) #80

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance CDK support with nested stack handling #295

Enhance CDK support with nested stack handling #295

flostadler commented Dec 16, 2024 •

edited

Loading

flostadler commented Dec 17, 2024 •

edited

Loading

flostadler Dec 17, 2024

corymhall left a comment

corymhall Dec 17, 2024

flostadler commented Dec 18, 2024

corymhall left a comment

t0yv0 Dec 18, 2024

t0yv0 Dec 18, 2024

t0yv0 Dec 18, 2024

t0yv0 Dec 18, 2024

t0yv0 Dec 18, 2024

t0yv0 Dec 18, 2024

t0yv0 commented Dec 18, 2024

pulumi-bot commented Dec 23, 2024

		@@ -367,10 +448,39 @@ export class StackConverter extends ArtifactConverter implements intrinsics.Intr
		/** @internal */
		asOutputValue<T>(v: T): T {

Enhance CDK support with nested stack handling #295

Enhance CDK support with nested stack handling #295

Conversation

flostadler commented Dec 16, 2024 • edited Loading

Review Advice

flostadler commented Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

corymhall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flostadler commented Dec 18, 2024

corymhall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t0yv0 commented Dec 18, 2024

pulumi-bot commented Dec 23, 2024

flostadler commented Dec 16, 2024 •

edited

Loading

flostadler commented Dec 17, 2024 •

edited

Loading