Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Double nested stack outputs not resolved correctly #15155

Closed
MasonHM opened this issue Jun 16, 2021 · 8 comments · Fixed by #15380
Closed

core: Double nested stack outputs not resolved correctly #15155

MasonHM opened this issue Jun 16, 2021 · 8 comments · Fixed by #15380
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@MasonHM
Copy link

MasonHM commented Jun 16, 2021

When multiple stacks are nested inside of each other, accessing resources that depend on parameters doesn't seem to work correctly. I don't have a good explanation of what is happening, but I have multiple ways to fix whatever the problem is.

The CloudFormation I'm trying to import that is failing looks like:

root-stack.yaml
       |----------load-balancers.yaml
               |----------load-balancer-stack-1.yaml
               |----------load-balancer-stack-2.yaml
               |----------load-balancer-stack-3.yaml

The problem we're running into is using the TargetGroup from load-balancer-stack-1.yaml in some new CDK resources.

This is related to #1374

Reproduction Steps

bin/cdk-test.ts:

#!/usr/bin/env node
import { App, Stack } from '@aws-cdk/core';
import { CfnInclude } from '@aws-cdk/cloudformation-include';
import { CfnBucket, Bucket } from '@aws-cdk/aws-s3';

class DoubleNestedStack extends Stack {
  readonly outputBucketName : CfnBucket;

  constructor(scope: App, id: string) {
      super(scope, id);

    const parentTemplate = new CfnInclude(this, 'ParentStack', {
        templateFile: 'parent-stack.yaml',
    });
    const middleTemplate = parentTemplate.loadNestedStack('MiddleStack', {
        templateFile: 'middle-stack.yaml'
    }).includedTemplate;
    const bottomTemplate = middleTemplate.loadNestedStack('BottomStack', {
        templateFile: 'bottom-stack.yaml'
    }).includedTemplate;

    this.outputBucketName = bottomTemplate.getResource('Bucket') as CfnBucket;
  }
}

class OtherStack extends Stack {
    constructor(scope: App, id: string, bucketName: string) {
        super(scope, id);

        const newBucket = new Bucket(this, 'newBucket', {
            bucketName: bucketName
        });
    }
}

const app = new App();
const doubleNestedStack = new DoubleNestedStack(app, 'DoubleNestedStack');
const bucketName = doubleNestedStack.outputBucketName.bucketName as string;
new OtherStack(app, 'OtherStack', bucketName);

parent-stack.yaml:

AWSTemplateFormatVersion: '2010-09-09'

Resources:
  MiddleStack:
    Type: AWS::CloudFormation::Stack
    Properties:
      TemplateURL: 'https://s3.amazonaws.com/this-doesnt-matter.yaml'

middle-stack.yaml:

AWSTemplateFormatVersion: '2010-09-09'

Resources:
  BottomStack:
    Type: AWS::CloudFormation::Stack
    Properties:
      TemplateURL: 'https://s3.amazonaws.com/this-doesnt-matter.yaml'
      Parameters:
        Name: anything

bottom-stack.yaml:

AWSTemplateFormatVersion: '2010-09-09'

Parameters:
  Name:
    Type: String

Resources:
  Bucket:
    Type: AWS::S3::Bucket
    Properties:
      BucketName: !Ref Name

What did you expect to happen?

I expected cdk synth to work and my OtherStack to be able to reference the double nested stack's resource. (I realize that I'm trying to name an S3 bucket the same thing as another bucket, but that's not the problem)

[masonme@f45c89b36d13] $ cdk synth
Successfully synthesized to /Users/masonme/cdk-migration-test/cdk.out
Supply a stack id (DoubleNestedStack, OtherStack) to display its template.

What actually happened?

Failure Output:

[masonme@f45c89b36d13] $ cdk synth

/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/construct-compat.ts:83

            ^
Error: Cannot use tokens in construct ID: DoubleNestedStackParentStackMiddleStackBottomStackNestedStackBottomStackNestedStackResource43C48F46Outputs.${Token[DoubleNestedStack.ParentStack.MiddleStack.Middl...arentStackMiddleStackBottomStackName8BE86CBFRef.LogicalID.48]}
    at new Construct (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/construct-compat.ts:83:13)
    at new CfnElement (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/cfn-element.ts:57:5)
    at new CfnOutput (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/cfn-output.ts:50:5)
    at createNestedStackOutput (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/private/refs.ts:207:14)
    at resolveValue (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/private/refs.ts:86:25)
    at resolveValue (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/private/refs.ts:87:12)
    at Object.resolveReferences (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/private/refs.ts:30:24)
    at Object.prepareApp (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/private/prepare-app.ts:31:3)
    at Object.synthesize (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/private/synthesis.ts:24:3)
    at App.synth (/Users/masonme/cdk-migration-test/node_modules/@aws-cdk/core/lib/stage.ts:180:23)
Subprocess exited with error 1

cdk synth failed due to CDK using a token in the ID of an internal construct. The code for this ID seems to be here:

const outputId = `${Names.nodeUniqueId(reference.target.node)}${reference.displayName}`;

I've found multiple ways to avoid the problem:

  • Change bottom-stack.yaml's BucketName property to a string instead of a reference to a parameter
  • RemoveOtherStack or its usage of BottomStack's output

Environment

  • CDK CLI Version : 1.103.0 (build bc13a66)
  • Framework Version: 1.103.0
  • Node.js Version: v14.15.4
  • OS : OS X 10.14.6 (18G9216)
  • Language (Version): TypeScript (Version 4.1.5)

Other


This is 🐛 Bug Report

@MasonHM MasonHM added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 16, 2021
@skinny85
Copy link
Contributor

skinny85 commented Jun 16, 2021

Hey @MasonHM,

thanks for reporting the issue, and for the awesome small reproduction, as always!

Unfortunately, I'm afraid you're running into a limitation/bug of the CDK Nested Stack support, not necessarily anything with cloudformation-include.

You can see that in the following example:

import * as cdk from '@aws-cdk/core';
import * as s3  from '@aws-cdk/aws-s3';

class DoubleNestedStack2 extends cdk.Stack {
    public readonly outputBucket: s3.CfnBucket;

    constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
        super(scope, id, props);

        const middleStack = new cdk.NestedStack(this, 'MiddleStack');
        const bottomStack = new cdk.NestedStack(middleStack, 'BottomStack', {
            parameters: {
                'Name': 'anything',
            },
        });
        const cfnParameter = new cdk.CfnParameter(bottomStack, 'Name');
        const cfnBucket = new s3.CfnBucket(bottomStack, 'Bucket', {
            bucketName: cfnParameter.valueAsString,
        });
        this.outputBucket = cfnBucket;
    }
}

interface OtherStackProps extends cdk.StackProps {
    readonly bucketName: string;
}

class OtherStack extends cdk.Stack {
    constructor(scope: cdk.Construct, id: string, props: OtherStackProps) {
        super(scope, id, props);

        const newBucket = new s3.Bucket(this, 'newBucket', {
            bucketName: props.bucketName,
        });
    }
}


const app = new cdk.App();

const doubleNestedStack2 = new DoubleNestedStack2(app, 'DoubleNestedStack2');
new OtherStack(app, 'OtherStack2', {
    bucketName: doubleNestedStack2.outputBucket.bucketName!,
});

Which fails synthesis with roughly the same error as your example:

Error: Cannot use tokens in construct ID: 
DoubleNestedStack2MiddleStackBottomStackNestedStackBottomStackNestedStackResourceC3DE33B1Outputs.${Token[DoubleNestedStack2.MiddleStack.BottomStack.DoubleNestedStack2MiddleStackBottomStackName233214B2Ref.LogicalID.43]}

@skinny85
Copy link
Contributor

skinny85 commented Jun 16, 2021

I can even minimize the reproduction further:

import * as cdk from '@aws-cdk/core';
import * as s3  from '@aws-cdk/aws-s3';

class DoubleNestedStack2 extends cdk.Stack {
    public readonly outputBucket: s3.CfnBucket;

    constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
        super(scope, id, props);

        const middleStack = new cdk.NestedStack(this, 'MiddleStack');
        const bottomStack = new cdk.NestedStack(middleStack, 'BottomStack');
        this.outputBucket = new s3.CfnBucket(bottomStack, 'Bucket');
    }
}

interface OtherStackProps extends cdk.StackProps {
    readonly bucketName: string;
}

class OtherStack extends cdk.Stack {
    constructor(scope: cdk.Construct, id: string, props: OtherStackProps) {
        super(scope, id, props);

        new s3.Bucket(this, 'newBucket', {
            bucketName: props.bucketName,
        });
    }
}


const app = new cdk.App();

const doubleNestedStack2 = new DoubleNestedStack2(app, 'DoubleNestedStack2');
new OtherStack(app, 'OtherStack2', {
    bucketName: doubleNestedStack2.outputBucket.ref,
});

@skinny85 skinny85 changed the title (CfnInclude): Double nested stack outputs not resolved correctly core: Double nested stack outputs not resolved correctly Jun 16, 2021
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Jun 16, 2021
@skinny85
Copy link
Contributor

Looks like the problem is here:

const outputId = `${Names.nodeUniqueId(reference.target.node)}${reference.displayName}`;

Apparently reference.displayName can contain a Token, which is then used as the logical ID of the CfnOutput construct, which is not allowed.

@skinny85
Copy link
Contributor

Paging @eladb on this one, as he's much more proficient at Nested Stacks than I am 🙂.

@skinny85 skinny85 assigned eladb and unassigned rix0rrr Jun 16, 2021
@eladb
Copy link
Contributor

eladb commented Jun 18, 2021

I'll take a look next week

@alochbau-amzn
Copy link

Any update on this issue?

@skinny85
Copy link
Contributor

@eladb did you have a chance to look at this error?

eladb pushed a commit that referenced this issue Jul 1, 2021
When the CDK generates a CloudFormation output to communicate information from the nested stack, it uses the `reference.displayName` property to increase uniqueness. If the display name includes a token, the id of the generated output includes a token and that’s not allowed.

The fix is to do the same thing we do for parameter IDs: `resolve()` the generated output ID before using it.

Fixes #15155
@eladb eladb added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2021
@eladb eladb removed their assignment Jul 1, 2021
@mergify mergify bot closed this as completed in #15380 Jul 1, 2021
mergify bot pushed a commit that referenced this issue Jul 1, 2021
When the CDK generates a CloudFormation output to communicate information from the nested stack, it uses the `reference.displayName` property to increase uniqueness. If the display name includes a token, the id of the generated output includes a token and that’s not allowed.

The fix is to do the same thing we do for parameter IDs: `resolve()` the generated output ID before using it.

Fixes #15155


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jul 1, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…5380)

When the CDK generates a CloudFormation output to communicate information from the nested stack, it uses the `reference.displayName` property to increase uniqueness. If the display name includes a token, the id of the generated output includes a token and that’s not allowed.

The fix is to do the same thing we do for parameter IDs: `resolve()` the generated output ID before using it.

Fixes aws#15155


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants