search

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.61k stars 3.91k forks source link

(aws-cdk:cli): Cross-stack dependencies should be handled more gracefully #27420

Open ajhool opened 1 year ago

ajhool commented 1 year ago

Describe the feature

Summary: cross-stack dependencies cause a lot of headaches when designing multi-stack apps and when removing dependent resources

The main issue discussing the problem and current solution is here: https://github.com/aws/aws-cdk/issues/3414

The current solution of hardcoding the export value still feels unnecessarily complicated.

I believe that the main problem is that CDK is too aggressive in deleting the exportedValue from the SourceStack when a developer removes the dependency in DependentStack. What's the rush? Why can't the SourceStack continue to export the exportedValue even if no downstream resources depend on it. In a subsequent deployment, if synth realizes that an autogenerated exportedValue is no longer used, it can be safely deleted.

While I recognize that there is a solution for this, it still feels like a workaround rather than the library working as expected. In CICD pipelines where the deployments are more rigid, it's awkward for developers to keep track of this and prepare PRs specifically to address this problem before merging the real PRs with their desired updates.

I labeled this a feature request because there is a workaround, but I view this behavior as a bug.

Use Case

Using a simple example where one stack creates an S3 bucket and another lambda stack depends on the S3 bucket (eg. the lambda uses the S3 bucket's name as an env variable)

Current Behavior

const bucketStack = new BucketStack(this, 'bucket', {});
const lambdaStack = LambdaStack(this, 'lambdas', { bucket: bucketStack.bucket });

If I want to delete the lambdaStack or delete the lambda function then I need to first update the bucketStack with a hardcoded exportValue(bucket.name). In this simple example that's straightforward, but in a more typical case to a developer it makes no sense why deleting a lambda function (maybe one lambda of 100s) in a dependent stack (maybe 1 stack of 20) requires thinking about the BucketStack or taking action on it.

Desired behavior Suggested workflow where developers intuitively work with StackB's dependency on StackA:

Day 1

  1. Deploy BucketStack
  2. Deploy LambdaStack

Day 2

  1. LambdaStack - remove the lambda function in the code. Shows a cdk diff (removal of lambda function).
  2. BucketStack - Shows no diff and a deploy would say "No changes" .
  3. LambdaStack - Deploy LambdaStack. Nothing happens to BucketStack
  4. BucketStack - Diff BucketStack - Shows a diff (removal of ExportValue S3 ARN).

Day 2 or 3 or 74

  1. Somebody pushes code and CICD detects that BucketStack has a diff (removal of ExportValue S3 ARN). CICD deploys BucketStack to remove the OutputValue. This might be surprising to developers because they see a cdk diff for BucketStack but don't see any relevant code changes in BucketStack or LambdaStack. The developers know that this is a quirk of cross-stack dependency management in CDK.

Proposed Solution

Requirements:

  1. My guess is that currently CDK computes the exportedValues for each stack every synth. That would still occur, but the results would be cached somewhere (cdk.context.json??).

  2. If all dependent resources for an exportedValue are deleted (eg. LambdaStack is deleted), a flag could be set on that exportedValue metadata in the cache saying "this exportedValue is no longer neeeded".

  3. Either a command line flag could be exposed to --deleteUnusedExportedValues so that the developer determines when to delete any stale exported values OR maybe the metadata could have a hash key and when cdk synth is executed some logic on the hash key could determine that all stale exported values can be safely deleted

Other Information

No response

Acknowledgements

CDK version used

2.99.1

Environment details (OS name and version, etc.)

Ubuntu

peterwoodworth commented 1 year ago

It sounds like your proposed solution is to only remove an export if it's detected that the export is not currently being used by any other stacks in your app after deployment? This would take a great engineering effort to work around the current design

If I want to delete the lambdaStack or delete the lambda function then I need to first update the bucketStack with a hardcoded exportValue(bucket.name)

I'm not finding this behavior on the latest version, when I followed these steps the CDK deleted the Lambda Stack first. Not to say that cross-stack dependencies don't have their issues at times regarding imports/exports, but I'm not sure your proposed solution is a direction we would go

github-actions[bot] commented 1 year ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

ajhool commented 1 year ago

@peterwoodworth

Here's a reproduction using the latest CDK version: https://github.com/ajhool/cdk-dependency-issue

The cdk source code contains a message detailing this issue's behavior and explaining why exportValue is the current workaround: https://github.com/aws/aws-cdk/blob/06bea3159e137a8e85e362795ef2cf7e219e381c/packages/aws-cdk-lib/core/lib/stack.ts#L1156

I do run into this problem frequently and consider it to be the most annoying thing about the CDK deployment and CICD experience.

It sounds like your proposed solution is to only remove an export if it's detected that the export is not currently being used by any other stacks in your app after deployment? This would take a great engineering effort to work around the current design

I don't think that is what I'm suggesting. I'm not suggesting to run commands after deployment.

I'm suggesting that when cdk deploy would ordinarily delete one of these autogenerated exported values, it should instead write a "deleteExportedValueXYZ" to a cache. I'm not familiar with the CDK deployment process but I am assuming that there is some basic caching/metadata/context mechanism that persists across cdk deploy commands (eg. I think there is metadata in the cloudformation stacks and maybe cdk.context.json is a cache of some sort?). On a subsequent deployment, CDK could read the cache and delete any exported values that are no longer in use (by this point, the LambdaStack would already have deployed and BucketStack would be able to delete the exported value).

If there isn't a caching mechanism or a simple way to do that then I'm not suggesting a major CDK rewrite for this feature.

Another approach could be to notify the user of the list of "exportValues" that are autogenerated so that it's easier for the user to add that list into the SourceStack. Sometimes when I pass through an entire interface (eg. I pass the entire cross-stack IVpc through to a consuming resource, it isn't obvious what underlying exportValues are used by the consuming resource. In that scenario, if I want to remove the consuming resource (eg. a Fargate Task that depends on a VPC and subnets), I would need to find all of the cross-stack references, add them to the NetworkStack as exportValue()s, deploy the NetworkStack, and then deploy the Fargate stack again -- just to delete a Fargate Task that depends on the cross-stack VPC from the NetworkStack.

Another approach would be for CDK to simply never delete those exportedValues unless the user sets a flag to delete them during deploy -- that might need a cache too