feat(core): support adding additional cache key to cdk context values #31586

luxaritas · 2024-09-27T18:20:29Z

Issue # (if applicable)

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

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

…etImage

…-cache

…g provider props

luxaritas · 2024-09-27T18:22:05Z

This PR continues from #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

…-cache

aws-cdk-automation · 2024-10-21T00:12:16Z

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.

aws-cdk-automation · 2024-10-21T14:20:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 29a5a95
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

rix0rrr

I like it, thanks!

I've got some small requests for changes but other than that this seems good to go.

rix0rrr · 2025-04-16T08:11:05Z

packages/aws-cdk-lib/aws-ec2/lib/security-group.ts

@@ -369,22 +369,22 @@ export class SecurityGroup extends SecurityGroupBase {
   *
   * @deprecated Use `fromLookupById()` instead
   */
-  public static fromLookup(scope: Construct, id: string, securityGroupId: string) {
-    return this.fromLookupAttributes(scope, id, { securityGroupId });
+  public static fromLookup(scope: Construct, id: string, securityGroupId: string, additionalCacheKey?: string) {


I'd prefer not to have optional parameters as positionals, because this ultimately leads to callFunction(3, undefined, undefined, true, undefined, false) 😉.

If you want to add it here, create an Options struct for it. But more pragmatically: it's already available in the fromLookupAttributes method, which is good enough. No need to add it here as well then.

rix0rrr · 2025-04-16T08:11:26Z

packages/aws-cdk-lib/aws-ec2/lib/security-group.ts

  }

  /**
   * Look up a security group by id.
   */
-  public static fromLookupById(scope: Construct, id: string, securityGroupId: string) {
-    return this.fromLookupAttributes(scope, id, { securityGroupId });
+  public static fromLookupById(scope: Construct, id: string, securityGroupId: string, additionalCacheKey?: string) {


Same argument here.

rix0rrr · 2025-04-16T08:11:35Z

packages/aws-cdk-lib/aws-ec2/lib/security-group.ts

  }

  /**
   * Look up a security group by name.
   */
-  public static fromLookupByName(scope: Construct, id: string, securityGroupName: string, vpc: IVpc) {
-    return this.fromLookupAttributes(scope, id, { securityGroupName, vpc });
+  public static fromLookupByName(scope: Construct, id: string, securityGroupName: string, vpc: IVpc, additionalCacheKey?: string) {


Same argument hre.

rix0rrr · 2025-04-16T08:12:45Z

packages/aws-cdk-lib/aws-ecs/lib/amis.ts

@@ -152,13 +160,14 @@ export class EcsOptimizedAmi implements ec2.IMachineImage {
      + (this.windowsVersion ? 'image_id' : 'recommended/image_id');

    this.cachedInContext = props?.cachedInContext ?? false;
+    this.additionalCacheKey = props?.additionalCacheKey;


We should probably add some validation to say that additionalCacheKey only makes sense if cachedInContext==true ?

rix0rrr · 2025-04-16T08:12:50Z

packages/aws-cdk-lib/aws-ecs/lib/amis.ts

@@ -280,14 +297,15 @@ export class EcsOptimizedImage implements ec2.IMachineImage {
      + (this.hwType === AmiHardwareType.NEURON ? 'inf/' : '')
      + (this.windowsVersion ? 'image_id' : 'recommended/image_id');

-    this.cachedInContext = props?.cachedInContext ?? false;
+    this.cachedInContext = props.cachedInContext ?? false;
+    this.additionalCacheKey = props.additionalCacheKey;


We should probably add some validation to say that additionalCacheKey only makes sense if cachedInContext==true ?

rix0rrr · 2025-04-16T08:12:58Z

packages/aws-cdk-lib/aws-ecs/lib/amis.ts

@@ -395,13 +422,14 @@ export class BottleRocketImage implements ec2.IMachineImage {
    this.amiParameterName = `/aws/service/bottlerocket/${this.variant}/${this.architecture}/latest/image_id`;

    this.cachedInContext = props.cachedInContext ?? false;
+    this.additionalCacheKey = props.additionalCacheKey;


We should probably add some validation to say that additionalCacheKey only makes sense if cachedInContext==true ?

rix0rrr · 2025-04-16T08:14:38Z

packages/aws-cdk-lib/aws-ssm/lib/parameter.ts

@@ -579,12 +579,16 @@ export class StringParameter extends ParameterBase implements IStringParameter {
   * and the ContextProvider will be told NOT to raise an error on synthesis
   * if the SSM Parameter is not found in the account at synth time.
   */
-  public static valueFromLookup(scope: Construct, parameterName: string, defaultValue?: string): string {
+
+  public static valueFromLookup(scope: Construct, parameterName: string, defaultValue?: string, additionalCacheKey?: string): string {


defaultValue is borderline (should probably have been in an options struct already), but I'd really like the additionalCacheKey to be part of an options struct.

luxaritas added 30 commits September 6, 2023 18:16

feat(core): Support linking cdk context values to scope

ec846d2

linkContextToScope should be passed to LookupMachineImage props not g…

00cc494

…etImage

Fix missing passthroughs of linkContextToScope

015bad1

Add integration test for cached machine image lookup

d8d27f9

Fix default in AmazonLinuxImageSsmParameterBase

ea2d380

Update snapshot

edbccad

Use explicit cache key instead of opting in to scope key

285fa90

Fix unit tests

bf8c6c1

Remove dead imports

4608dba

Integ structure

925bd48

Streamline info in README

f5324ac

Correct comment

bc36cab

Simplify references from props

bf503e8

No need to spread

b783667

Revert format change

0b39809

Revert function signature change

0e6cfb5

Remove more spreads, add to interfaces

7dc9501

Fix unit test

f7bcd97

update schema

8c508ab

Merge remote-tracking branch 'upstream/main' into scope-local-context…

c093baf

…-cache

Docs improvements

8397e44

Regen schema from main

4cdc802

update-schema

1b6955c

Merge remote-tracking branch 'upstream/main' into scope-local-context…

ae5638c

…-cache

update-schema after merge

1507ae8

Merge branch 'main' into scope-local-context-cache

1485a80

Merge branch 'main' into scope-local-context-cache

3473a97

Merge remote-tracking branch 'upstream/main' into scope-local-context…

5ba3bc8

…-cache

Move handling of additional cache key into getKey instead of extendin…

2593af6

…g provider props

Remove outdated change

d35b71f

aws-cdk-automation requested a review from a team September 27, 2024 18:20

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Sep 27, 2024

luxaritas added 2 commits September 27, 2024 14:37

Docs update

9cb435b

Test fixes

cd2774e

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 27, 2024

Merge remote-tracking branch 'upstream/main' into scope-local-context…

68061ae

…-cache

luxaritas added 2 commits October 20, 2024 21:56

Merge branch 'main' into scope-local-context-cache

9217a01

Lint fix

29a5a95

samson-keung added the skip-abstractions-board signal to automated workflow to skip adding to project board label Feb 19, 2025

rix0rrr self-assigned this Apr 16, 2025

rix0rrr requested changes Apr 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): support adding additional cache key to cdk context values #31586

feat(core): support adding additional cache key to cdk context values #31586

luxaritas commented Sep 27, 2024

luxaritas commented Sep 27, 2024 •

edited

Loading

aws-cdk-automation commented Oct 21, 2024

aws-cdk-automation commented Oct 21, 2024

rix0rrr left a comment

rix0rrr Apr 16, 2025

rix0rrr Apr 16, 2025

rix0rrr Apr 16, 2025

rix0rrr Apr 16, 2025

rix0rrr Apr 16, 2025

rix0rrr Apr 16, 2025

rix0rrr Apr 16, 2025

feat(core): support adding additional cache key to cdk context values #31586

Are you sure you want to change the base?

feat(core): support adding additional cache key to cdk context values #31586

Conversation

luxaritas commented Sep 27, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

luxaritas commented Sep 27, 2024 • edited Loading

aws-cdk-automation commented Oct 21, 2024

aws-cdk-automation commented Oct 21, 2024

AWS CodeBuild CI Report

rix0rrr left a comment

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

rix0rrr Apr 16, 2025

Choose a reason for hiding this comment

luxaritas commented Sep 27, 2024 •

edited

Loading