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

pipelines: Runtime error when a Stage has multiple Stacks with the same stack name #30960

Closed
Gum-Christopher-bah opened this issue Jul 26, 2024 · 7 comments · Fixed by #31328
Closed
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@Gum-Christopher-bah
Copy link

Gum-Christopher-bah commented Jul 26, 2024

Describe the bug

This is a duplicate of issue #30449 which should not have been closed.

To provide additional context, if you have an app that you are deploying without pipelines, this behavior works, because the logical ids are different. Using waves only solves the case where you have two stacks with no other dependencies, or dependencies that are in the same region. If you have stacks which consume variables or exports from other stacks cross region by passing crossRegionReferences: true it is not possible to split these stacks into separate stages, because it is not possible to consume variables from stage to stage. This causes working cdk apps to be unable to be adopted by cdk pipelines.

Expected Behavior

App is able to synth and deploy

Current Behavior

Stacks with the same name but different IDs in the same stage cause a runtime error:

Reproduction Steps

See duplicate issue

Possible Solution

Add an explicit override parameter that can be used to bypass this check (which I'm sure does save a lot of headaches if code gets mistakenly copy pasted)

Additional Information/Context

No response

CDK CLI Version

2.150.0 (build 3f93027)

Framework Version

No response

Node.js Version

v18.20.4

OS

Mac

Language

TypeScript

Language Version

No response

Other information

No response

@Gum-Christopher-bah Gum-Christopher-bah added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 26, 2024
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Jul 26, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 29, 2024
@khushail
Copy link
Contributor

khushail commented Jul 31, 2024

Hi @Gum-Christopher-bah , thanks for reporting this.

I tried to repro the issue with this code (Ref - #30449)

Code -
Deployment stage

import aws_cdk as cdk
from constructs import Construct
from aws_cdk import Stack, Environment

class DeploymentStage(cdk.Stage):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        Stack(
            self,
            "StackEnv1",
            stack_name="my-test-stack",
            env=Environment(
                account="111111111111",
                region="us-east-1",
            ),
        )
        Stack(
            self,
            "StackEnv2",
            stack_name="my-test-stack",
            env=Environment(
                account="333333333333",
                region="us-east-2",
            ),
        )

Pipeline stack

from aws_cdk import (
    # Duration,
    Environment,
    Stack,
    pipelines
)
from constructs import Construct
from pipeline_issue.deployment_stage import DeploymentStage

class PipelineIssueStack(Stack):

    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        pipeline = pipelines.CodePipeline(
            self,
            "Pipeline",
            cross_account_keys=True,
            synth=pipelines.ShellStep("Synth",
                input=pipelines.CodePipelineSource.git_hub("khushai/test01", "main"),
                commands=["npm ci", "npm run build", "npx cdk synth"],
            ),
        )

        pipeline.add_stage(DeploymentStage(self, "Production"))

app.py

#!/usr/bin/env python3
import os

import aws_cdk as cdk

from pipeline_issue.pipeline_issue_stack import PipelineIssueStack


app = cdk.App()
PipelineIssueStack(app, "PipelineIssueStack",
    env=cdk.Environment(account='333333333333', region='us-east-1'),
    )

app.synth()

Error

.venv) khushail@a07817b31bf3 pipelineIssue % cdk synth
jsii.errors.JavaScriptError: 
  @jsii/kernel.RuntimeError: Error: Node with duplicate id: my-test-stack
      at Kernel._Kernel_ensureSync (/private/var/folders/94/v50y280d7gvbzhqqst22gj800000gr/T/tmpxbe56ox1/lib/program.js:10502:23)
      at Kernel.invoke (/private/var/folders/94/v50y280d7gvbzhqqst22gj800000gr/T/tmpxbe56ox1/lib/program.js:9866:102)
      at KernelHost.processRequest (/private/var/folders/94/v50y280d7gvbzhqqst22gj800000gr/T/tmpxbe56ox1/lib/program.js:11707:36)
      at KernelHost.run (/private/var/folders/94/v50y280d7gvbzhqqst22gj800000gr/T/tmpxbe56ox1/lib/program.js:11667:22)
      at Immediate._onImmediate (/private/var/folders/94/v50y280d7gvbzhqqst22gj800000gr/T/tmpxbe56ox1/lib/program.js:11668:46)
      at process.processImmediate (node:internal/timers:471:21)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/khushail/pipelineIssue/app.py", line 14, in <module>
    app.synth()
  File "/Users/khushail/pipelineIssue/.venv/lib/python3.12/site-packages/aws_cdk/__init__.py", line 21371, in synth
    return typing.cast(_CloudAssembly_c693643e, jsii.invoke(self, "synth", [options]))
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/khushail/pipelineIssue/.venv/lib/python3.12/site-packages/jsii/_kernel/__init__.py", line 149, in wrapped
    return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/khushail/pipelineIssue/.venv/lib/python3.12/site-packages/jsii/_kernel/__init__.py", line 399, in invoke
    response = self.provider.invoke(
               ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/khushail/pipelineIssue/.venv/lib/python3.12/site-packages/jsii/_kernel/providers/process.py", line 380, in invoke
    return self._process.send(request, InvokeResponse)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/khushail/pipelineIssue/.venv/lib/python3.12/site-packages/jsii/_kernel/providers/process.py", line 342, in send
    raise RuntimeError(resp.error) from JavaScriptError(resp.stack)
RuntimeError: Error: Node with duplicate id: my-test-stack

AFAIK, the resource Physical names have some constraints when being used across regions or cross-envoironment references, and should not produce any issue here since we are using the same stack in different regions.
Though I am still investigating this one, marking it as P2 for now. Will consult with my team on this.

@khushail khushail added the effort/medium Medium work item – several days of effort label Jul 31, 2024
@Gum-Christopher-bah
Copy link
Author

Thanks @khushail for taking another look at this. @jsii/kernel.RuntimeError: Error: Node with duplicate id: my-test-stack Is the exact error I'm seeing using typescript as well. It's interesting because if you look at the cdk output, these stacks actually do have unique assets/template files.

@pahud
Copy link
Contributor

pahud commented Aug 12, 2024

Hi @Gum-Christopher-bah

As @khushail is on leave I am taking over this issue. Looks like you are deploying two stacks with exactly the same stack name but in different regions. Is it correct?

@pahud
Copy link
Contributor

pahud commented Aug 12, 2024

I can reproduce this using TypeScript as well. The workaround is to leave stackName undefined but it should allow two stacks for different account/region using the same stackName.

export class DeploymentStage extends Stage {
  constructor(scope: Construct, id: string) {
    super(scope, id);
    new Stack(this, 'StackEnv1', {
      // stackName: 'my-test-stack',
      env: {
        account: DEFAULT_ACCOUNT_ENV,
        region: 'us-east-1'
      }
    });

    new Stack(this, 'StackEnv2', {
      // stackName: 'my-test-stack',
      env: {
        account: DEFAULT_ACCOUNT_ENV,
        region: 'us-west-2'
      }
    });
  }
}

I am making it a p1 as this does not have any workaround.

@pahud pahud added p1 and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 labels Aug 12, 2024
@Gum-Christopher-bah
Copy link
Author

Hello @pahud, appreciate the second set of eyes here.

Looks like you are deploying two stacks with exactly the same stack name but in different regions. Is it correct?

Yes, this is correct.

I'm actually unable to omit the stack name from my stacks, because the stacks are already deployed unfortunately. Passing the stackname explicitly was the method I found best to deal with pipelines stack adoptions, because changing the scope from the app to a stage in the pipeline would always generate new stack names and thus new resources which was not desirable. My flow has been: Create stacks in app, deploy, validate, add stacks as is to pipeline. Which has worked well in the past when I named the stacks like MyStack_east and MyStack_west, but has backfired now that I've named them the same.

Copy link

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.

1 similar comment
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants