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:cli): Cross-stack dependencies should be handled more gracefully #27420

Open
2 tasks
ajhool opened this issue Oct 5, 2023 · 3 comments
Open
2 tasks
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p3

Comments

@ajhool
Copy link

ajhool commented Oct 5, 2023

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

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

CDK version used

2.99.1

Environment details (OS name and version, etc.)

Ubuntu

@ajhool ajhool added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 5, 2023
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Oct 5, 2023
@peterwoodworth
Copy link
Contributor

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

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. effort/large Large work item – several weeks of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 5, 2023
@github-actions
Copy link

github-actions bot commented Oct 8, 2023

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.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Oct 8, 2023
@ajhool
Copy link
Author

ajhool commented Oct 8, 2023

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

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

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Oct 8, 2023
@kellertk kellertk added the p3 label Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p3
Projects
None yet
Development

No branches or pull requests

3 participants