aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

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

Open braska opened this issue 2 years ago • 5 comments

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: https://github.com/aws/aws-cdk/commit/0e86b2e24fe434743d762e2818091eeb345d2947

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

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

CDK version used

2.88.0

Environment details (OS name and version, etc.)

(irrelevant)

braska avatar Jul 29 '23 23:07 braska

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 avatar Jul 31 '23 16:07 pahud

@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.

braska avatar Jul 31 '23 17:07 braska

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.

pahud avatar Aug 01 '23 18:08 pahud

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

braska avatar Aug 02 '23 05:08 braska

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 :)

rik-iso avatar Dec 05 '23 20:12 rik-iso