aws-cdk
aws-cdk copied to clipboard
feat(core): support adding additional cache key to cdk context values
Issue # (if applicable)
Closes #26982
Reason for this change
The ContextProvider mechanism and various "lookup" functions of a number of constructs support caching resolved values in the cdk.context.json. The context keys are constructed from the parameters of the lookup, which for lookup functions means whenever a resource with the same parameters is resolved, it is resolved as the same value across the entire app. However when a value may change over time, the user may wish to use the latest value when creating creating a new reference to the construct, effectively tying the cached context value to the scope - this patch enables this.
The primary use case is looking up an AMI parameter for a "stateful" EC2 instance. Currently if you specify cachedInContext, any future images created would use the same cached AMI, and updating the value would require updating all usages of the image across the entire app.
Description of changes
Adds an additionalCacheKey parameter/property to multiple areas of the CDK where lookups can be cached
Description of how you validated changes
Unit + integration tests
Checklist
- [x] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
This PR continues from https://github.com/aws/aws-cdk/pull/28766, which I let go stale stale while waiting for feedback from a maintainer
CC @TheRealAmazonKendra who was having some discussions about this
I still have to make some docs changes based on a couple rounds of previous reviews, but wanted to kick off a CI from merging in main and making related tweaks first
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
This PR 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.
This is on my radar to look at soon
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. Note that PRs with failing linting check or builds are not reviewed, please ensure your build is passing
To prevent automatic closure:
- Resume work on the PR
- OR request an exemption by adding a comment containing 'Exemption Request' with justification e.x "Exemption Request:
" - OR request clarification by adding a comment containing 'Clarification Request' with a question e.x "Clarification Request:
"
This PR will automatically close in 14 days if no action is taken.
Something else I've just discovered: ApplicationListener.fromLookup, ApplicationLoadBalancer.fromLookup, and NetworkLoadBalancer.fromLookup all call SecurityGroup.fromLookupById. Should we pass through the additionalContextKey from these to the SecurityGroup lookups?
Though that said, Im having some second thoughts about this cache customization for lookups that aren't tied to an SSM parameter (eg, looking up a VPC, LB, or SG). At the very least, it shouldn't be possible for an ID lookup to return something different over time, right? I'm less sure about a situation like looking up a resource by name/alias, as it could theoretically be referring to a different resource based on when you cache it, but it still feels a little gross as it's not "intended" to change over time in the same way, though it gets even fuzzier once you start talking about things like tag lookups (and there is some difference across different constructs, eg VPC lookups don't discriminate between mutially exclusive parameters like SG lookups do)
Though that said, Im having some second thoughts about this cache customization for lookups that aren't tied to an SSM parameter (eg, looking up a VPC, LB, or SG). At the very least, it shouldn't be possible for an ID lookup to return something different over time, right?
That is true. At least a Vpc can get looked up by isDefaultVpc: true, or by tags, or by name, so it's not necessarily only an id. An LB can get looked up by tags, and an SG by name--which theoretically could point to a different SG over time.
I think the question is how likely it is that the underlying "looked-up" parameter is going to change, and how likely it is that you'd want all different versions over time being used within one application simultaneously.
The use case for AMIs you identified is a good one... I'm struggling to think of others.
The right solution is probably to implement this only for AMIs, and nothing else (both using the AMI provider and the SSM provider).
This PR 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.
Sorry for the delay - planning on looking at this tomorrow and ripping out the non-AMI/SSM stuff
Thanks!
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).
This pull request has been removed from the queue for the following reason: pull request branch update failed.
The pull request can't be updated.
You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).
@rix0rrr Automerge failed because of workflow updates, so manually pulled in main
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 45e0641cbbae19b945ee49b2b43e40a433b09672
- Result: SUCCEEDED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).
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.