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

aws-cdk-lib: cross-stage references with PhysicalName.GENERATE_IF_NEEDED #26571

Open
2 tasks done
braska opened this issue Jul 29, 2023 · 5 comments
Open
2 tasks done
Labels
@aws-cdk/core Related to core CDK functionality effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@braska
Copy link

braska commented Jul 29, 2023

Describe the feature

I suggest to allow passing resorces between stages if resource name is marked with PhysicalName.GENERATE_IF_NEEDED.

Use Case

I have a CDK Pipeline with multiple deployment stage running in a wave. 1 stage = 1 region, but same account. I also want to have 1 stage that deploying after all these stages. In my case, I want to deploy Cloudwatch dashboard that will show graph widget for the same lambda deployed to many regions (I want to use addLeftMetric, so graph widget will show 1 line per region on a single widget).

Proposed Solution

It could be simple as this: 0e86b2e

The only concern I have:
Looking at this code I can say that PhysicalName.GENERATE_IF_NEEDED can generate same names for different stages. It could be an issue if for stages deployed to the same account and region it will generage exactly the same resource names.

It could be solved by including this into name generator:

const stage = Stage.of(resource);
if (stage) {
  sha256.update(stage.stageName);
}

But it is a breaking change (it will generate new names for people who was using PhysicalName.GENERATE_IF_NEEDED before this change).

Other Information

I currently found a way to build a single dashboard using resources deployed by multiple stages. It looks like this: https://gist.github.com/braska/6f622fdac92d8c66d9d69300d480fbed

The only reason why it is possible - dashboard deployed to the different region where non of other stages deployed. You can see env.region for stages in pipeline.ts. It works because of this condition CDK will generate static name.

But I still want to deploy dashboard into us-east-1 and it is not possible with current implementation.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.88.0

Environment details (OS name and version, etc.)

(irrelevant)

@braska braska added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 29, 2023
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Jul 29, 2023
@pahud
Copy link
Contributor

pahud commented Jul 31, 2023

I am not sure if this is a good idea as this introduces breaking change but having cross-stage reference is a good feature request.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 31, 2023
@braska
Copy link
Author

braska commented Jul 31, 2023

@pahud thanks for the feedback. I agree. What could be an alternative?

I have 2 ideas:

  • Add boolean crossStage parameter here and here and here. It can be passed by Resource from here. Based on this parameter physical name generator will optionally include stage part into hash.
  • Introduce a new marker: PhysicalName.GENERATE. It will work almost the same way as PhysicalName.GENERATE_IF_NEEDED, but it will always generate name in synth time without any conditions. But I don't like that PhysicalName.GENERATE will work for cross-stage references and PhysicalName.GENERATE_IF_NEEDED will not. These names don't imply this difference.

@pahud
Copy link
Contributor

pahud commented Aug 1, 2023

We probably need more feedback from more people but if your proposed solution would not introduce any breaking changes, we always welcome community PRs for that. Also if you need to discuss the design with the core team SDE in the PR planning stage, feel free to schedule an office hour slot with the team. Stay tuned on this thread or the contributing channel on cdk.dev.

@braska
Copy link
Author

braska commented Aug 2, 2023

Thanks @pahud. I will book a slot for an office hour event as soon it will be announced.

@rik-iso
Copy link

rik-iso commented Dec 5, 2023

FWIW I found using ._enableCrossEnvironment() on the resources in question (while also adding GENERATE_IF_NEEDED) seems to force CDK to generate a physical name even in cross-stage cases. The fact this method is prefixed _ suggests to me I should not be doing this though :)

@moelasmar moelasmar added @aws-cdk/core Related to core CDK functionality and removed aws-cdk-lib Related to the aws-cdk-lib package labels Aug 28, 2024
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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

4 participants