core: incorrect asset bundling skip for assets in stacks inside stages inside stacks

[plumdog]

Describe the bug

Eg when you have:

• PipelineStack
  • DeploymentStage
    • DeploymentStack
      • SomeLambda

If I try to deploy just the DeploymentStack with npm run -- cdk deploy -e pipeline-stack/deployment-stage/deployment-stack, the asset for SomeLambda is not bundled. Instead, I just get the whole repo, and Lambda complains about the zip being too big.

I think this might be because of this line here: https://github.com/aws/aws-cdk/blob/v1-main/packages/@aws-cdk/core/lib/stack.ts#L1179

Perhaps that .replace should instead act on the last occurrence in the string, not the first. This is half right. this.stackName is deployment-stage-deployment-stack, but bundlingStacks from cxapi.BUNDLING_STACKS is pipeline-stack/deployment-stage/deployment-stack. So to get the correct behaviour need to munge pattern as follows:

• replace the last / with a -
• then remove everything up-to-and-including any remaining /

But this feels - in general - unsafe. Why are cxapi.BUNDLING_STACKS and this.stackName in different formats and what is a valid way of comparing them?

The above would match with some crude debugging I bodged into my project where I encountered this, and with the comment there "bundlingStacks is of the form Stage/Stack, convert it to Stage-Stack before comparing to stack name": if I override bundlingRequired in my stack and print out what this.node.tryGetContext(cxapi.BUNDLING_STACKS) ?? ['*'] is, I get ['pipeline-stack/deployment-stage/deployment-stack'], but this.stackName is 'deployment-stage-deployment-stack'.

So when the first / is replaced and the minimatch comparison occurs, the match fails, because it was the first / that was replaced. If it was the last, then it would match. (false, see above)

Expected Behavior

The asset for the stack I am deployed to be bundled.

Current Behavior

The project root is zipped as the asset.

Reproduction Steps

I'm in the process of trying to write a failing test, but am fighting buildup atm.

The test I'm trying to write looks just like the test added in cda6601 but with an extra layer of nesting, like Stack1/Stage/Stack2.

Possible Solution

In Stack.bundlingRequired, replace the last / in pattern, not the first.

Additional Information/Context

I think this is related to #15346

CDK CLI Version

2.35.0

Framework Version

No response

Node.js Version

16

OS

Linux

Language

Typescript

Language Version

No response

Other information

My workaround is to put:

public override get bundlingRequired() {
    return true;
}

in the Stack that contains the asset. This results in the expected behaviour.

[dragonlobster]

i have the same problem how to fix this?

[plumdog]

@dragonlobster

I found that putting

public override get bundlingRequired() {
    return true;
}

in the stack class that directly contains the Lambda resulted in the correct behaviour, though will mean bundling sometimes happens unnecessarily.

[peterwoodworth]

Could you clarify what error message you're getting (it's an error on deploy that the zipfile is too big for the lambda?), plus could you share reproduction steps (are you using just a lambda.Function?)? This pattern (stack with assets inside stage inside stack) is common and I've never come across any issues before - checking my cdk.out directory seems to give me about what I'm expecting with a directory just for the stage, and inside that directory there's a stack template and an asset just for the lambda

[plumdog]

@peterwoodworth the behaviour only occurs when CDK is trying to work out which stacks need bundling and which don't, which I think is only when deploying with the -e switch to select a specific stack. So I think it requires all of the following to occur at once:

• At least 3 layers of stacks or stages
• A construct in such a stack that requires bundling. Certainly NodejsFunction is one of these, but I imagine there are others.
• Only a subset of stacks are being acted on. Eg, -e is used to deploy such a stack.

When those three things occur:

• The construct in the stack still needs bundling
• But cxapi.BUNDLING_STACKS is not ['*'], it is the stack's logical path within the CDK app
• The .replace('/', '-') transformation on the stack's logical path doesn't fudge to match with the stack's name, which only works in the 2 layers case because, I believe, a stack within a stage defaults to be ${stageName}-${stackId} per https://github.com/aws/aws-cdk/blob/v1.171.0/packages/@aws-cdk/core/lib/stack.ts#L1139-L1143

Thinking about it further, I think it will also fail in the 2 layers case (so just Stage -> Stack -> NodejsFunction) if the stack's name is not the default.

I'll try to provide a way of replicating or a failing test.

[peterwoodworth]

Thanks for the further explanation, it's a big help 🙂

[plumdog]

Extra realisation that I'll check when I can: I think the cause is that cxapi.BUNDLING_STACKS refers to stacks by their path within the CDK App, but the comparison is with this.stackName.

These just happen to often be the same. A stack name defaults to the id, which is also the path when there's no stages. And a stack in a stage defaults to stage-stack, which is why the .replace sometimes works.

Assuming I'm on the right track, I think I can write a few failing tests, and I think the fix will be to compare with the stack's logical path instead of this.stackName.

[kristiandreheraxis]

Is there any progress on this? Not being able to use the --exclusively flag is quite annoying when working on a project with many stacks.

[TheRealAmazonKendra]

Looks like the fix for this has been merged. Closing.

[github-actions]

⚠️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.