feat(cloudfront): associate key value stores to functions #28571

kylelaker · 2024-01-04T01:54:03Z

CloudFormation now supports specifying KeyValueStoreAssocations on a AWS::CloudFront::Function. This adds that support to the CDK. Even though the field is plural (in CloudFormation and the API), a function can have one value store so the implementation here tries to provide a nicer API around that.

If a Key Value Store is specified, the function will default to js-2.0 instead of js-1.0. Behavior for existing functions is retained. Additionally, an error is thrown if 1.0 is used.

Closes #28377.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

kylelaker · 2024-01-08T22:32:50Z

@Mergifyio update

mergify · 2024-01-08T22:33:02Z

update

✅ Branch has been successfully updated

go-to-k

Thanks for this PR! I made a few comments.

packages/aws-cdk-lib/aws-cloudfront/lib/function.ts

packages/aws-cdk-lib/aws-cloudfront/test/function.test.ts

go-to-k · 2024-01-21T08:15:16Z

packages/aws-cdk-lib/aws-cloudfront/test/function.test.ts

+
+    test('no value is used in CloudFormation when unspecified in CDK', () => {
+      const stack = new Stack();
+
+      new Function(stack, 'TestFn', {
+        code: FunctionCode.fromInline('code'),
+        runtime: FunctionRuntime.JS_2_0,
+        keyValueStore: undefined,
+      });
+
+      Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Function', {
+        FunctionConfig: {
+          KeyValueStoreAssociations: Match.absent(),
+        },
+      });
+    });
+  });


I don't see much need for this, but what do you think? (unless you intend to make sure that it is not generated with an empty array, or something like that.)

Suggested change

test('no value is used in CloudFormation when unspecified in CDK', () => {

const stack = new Stack();

new Function(stack, 'TestFn', {

code: FunctionCode.fromInline('code'),

runtime: FunctionRuntime.JS_2_0,

keyValueStore: undefined,

});

Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Function', {

FunctionConfig: {

KeyValueStoreAssociations: Match.absent(),

},

});

});

});

Yeah, I think I like it being explicit that it should be left absent instead of perhaps an empty list. I agree it's perhaps covered by other cases or less critical but it does feel like an important way to preserve the correct behavior at a CloudFormation layer.

OK, I agree. Let's go ahead with this.

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

go-to-k

Looks good.

gracelu0

Thank you for contributing! Left a tiny comment but otherwise LGTM :)

...sting/framework-integ/test/aws-cloudfront/test/integ.function-key-value-store-association.ts

mergify · 2024-01-24T23:30:03Z

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-cdk-automation · 2024-01-24T23:57:32Z

AWS CodeBuild CI Report

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

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

mergify · 2024-01-24T23:57:54Z

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

CloudFormation now supports specifying [`KeyValueStoreAssocations`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-function-keyvaluestoreassociation.html) on a `AWS::CloudFront::Function`. This adds that support to the CDK. Even though the field is plural (in CloudFormation and the API), [a function can have one value store](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/kvs-with-functions-associate.html) so the implementation here tries to provide a nicer API around that. If a Key Value Store is specified, the function will default to `js-2.0` instead of `js-1.0`. Behavior for existing functions is retained. Additionally, an error is thrown if 1.0 is used. Closes aws#28377. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

CloudFormation now supports specifying [`KeyValueStoreAssocations`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-function-keyvaluestoreassociation.html) on a `AWS::CloudFront::Function`. This adds that support to the CDK. Even though the field is plural (in CloudFormation and the API), [a function can have one value store](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/kvs-with-functions-associate.html) so the implementation here tries to provide a nicer API around that. If a Key Value Store is specified, the function will default to `js-2.0` instead of `js-1.0`. Behavior for existing functions is retained. Additionally, an error is thrown if 1.0 is used. Closes #28377. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Jan 4, 2024

aws-cdk-automation requested a review from a team January 4, 2024 01:54

aws-cdk-automation previously requested changes Jan 4, 2024

View reviewed changes

kylelaker mentioned this pull request Jan 9, 2024

(cloudfront): Function name is created non-deterministically #28629

Closed

kylelaker added 4 commits January 9, 2024 22:01

feat(cloudfront): associate key value stores to functions

1d7cd79

Update README for function association

c4c136d

Fix linter error

4825740

Allow only one associated KV store

8728e41

kylelaker force-pushed the kvstore-function-assoc branch from 971bb05 to 8728e41 Compare January 10, 2024 03:01

Update README.md

3552f4a

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

pahud mentioned this pull request Jan 16, 2024

aws-cloudfront: support KeyValueStore #28377

Closed

2 tasks

go-to-k suggested changes Jan 21, 2024

View reviewed changes

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

Apply suggestions from code review

d220650

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

go-to-k approved these changes Jan 22, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 22, 2024

Merge branch 'main' into kvstore-function-assoc

3106903

gracelu0 reviewed Jan 24, 2024

View reviewed changes

...sting/framework-integ/test/aws-cloudfront/test/integ.function-key-value-store-association.ts Outdated Show resolved Hide resolved

Fix typo in integ stack name

7cb1d96

gracelu0 approved these changes Jan 24, 2024

View reviewed changes

Merge branch 'main' into kvstore-function-assoc

84bcc2f

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 24, 2024

mergify bot merged commit 5ede456 into aws:main Jan 24, 2024
12 checks passed

github-actions bot mentioned this pull request Feb 1, 2024

Monthly PR metrics report - Jan 2024 #28952

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cloudfront): associate key value stores to functions #28571

feat(cloudfront): associate key value stores to functions #28571

kylelaker commented Jan 4, 2024 •

edited

aws-cdk-automation left a comment

kylelaker commented Jan 8, 2024

mergify bot commented Jan 8, 2024

go-to-k left a comment

go-to-k Jan 21, 2024

kylelaker Jan 21, 2024

go-to-k Jan 22, 2024

go-to-k left a comment

gracelu0 left a comment

mergify bot commented Jan 24, 2024

aws-cdk-automation commented Jan 24, 2024

mergify bot commented Jan 24, 2024

feat(cloudfront): associate key value stores to functions #28571

feat(cloudfront): associate key value stores to functions #28571

Conversation

kylelaker commented Jan 4, 2024 • edited

aws-cdk-automation left a comment

Choose a reason for hiding this comment

kylelaker commented Jan 8, 2024

mergify bot commented Jan 8, 2024

✅ Branch has been successfully updated

go-to-k left a comment

Choose a reason for hiding this comment

go-to-k Jan 21, 2024

Choose a reason for hiding this comment

kylelaker Jan 21, 2024

Choose a reason for hiding this comment

go-to-k Jan 22, 2024

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

gracelu0 left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 24, 2024

aws-cdk-automation commented Jan 24, 2024

AWS CodeBuild CI Report

mergify bot commented Jan 24, 2024

kylelaker commented Jan 4, 2024 •

edited