aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.5k stars 3.85k forks source link

(pipelines): UpdatePipeline - SelfMutate action injects stack name instead of the stack id #19350

Open rehos opened 2 years ago

rehos commented 2 years ago

What is the problem?

I ran into an issue where the UpdatePipeline - SelfMutate action failed. In my situation the Stack Id is different from the Stack Name. Looking at the generated CodeBuild commands I saw that it injected the stackName instead of the stackId. I believe the CDK cli requires a stack id to select the correct stack.

This line is wrong I think: See https://github.com/aws/aws-cdk/blob/3382e997067f7e9f98ba98f618402b48bea838b9/packages/%40aws-cdk/pipelines/lib/codepipeline/codepipeline.ts#L619

https://github.com/aws/aws-cdk/blob/3382e997067f7e9f98ba98f618402b48bea838b9/packages/%40aws-cdk/pipelines/lib/codepipeline/codepipeline.ts#L628

Reproduction Steps

Create a pipeline in a stack where the stack id is different from the stack name. Change the pipeline so it has to do a self update

What did you expect to happen?

The pipeline should update after a change

What actually happened?

The UpdatePipeline - SelfMutate action fails because it cannot find the stack in the assembly

CDK CLI Version

2.15.0

Framework Version

No response

Node.js Version

14

OS

Amazon Linux

Language

Typescript

Language Version

No response

Other information

No response

corymhall commented 2 years ago

@rehos can you provide a reproducible example where this issue occurs? I'm not able to reproduce this. If I do something like:

const stack = new Stack(app, 'PipelineStack', { stackName: 'someothername' });
new CodePipeline(stack, 'Pipeline', { ... });

It generates the mutate action with the correct PipelineStack identifier.

rehos commented 2 years ago

@corymhall thank you for getting back on this issue. I have tested it with the latest version of the CDK and I'm not able to reproduce this anymore because pipelineStack.node.path is now always set. So the following code doesn't hit the faulty assignment of pipelineStack.stackName which is the stack name instead of the stack id.

See https://github.com/aws/aws-cdk/blob/3382e997067f7e9f98ba98f618402b48bea838b9/packages/%40aws-cdk/pipelines/lib/codepipeline/codepipeline.ts#L619

I think the correct code should look something like this:

const pipelineStackIdentifier = pipelineStack.node.path ?? pipelineStack.node.id ; // or something else that returns the stack id
corymhall commented 2 years ago

@rehos thanks for double checking and confirming!

I'm going to downgrade this to a p2. Given the amount of work there is to do and the relative priority of this issue, the CDK team is unlikely to address it. That does not mean the issue will never be fixed! If someone from the community submits a PR to fix this issue, and the PR is small and straightforward enough, and meets the quality bars to be reviewed and merged with little effort we will accept that PR. PRs that do not build or need complex or multiple rounds of reviews are unlikely to be merged and will be closed to keep our backlog manageable.