feat(cloudfront): support for cache policies#10656
Merged
mergify[bot] merged 11 commits intomasterfrom Oct 7, 2020
Merged
Conversation
Support for the new Cache Policy concept in CloudFront, which replaces the existing `ForwardedValues` properties and specifies what parts of a request make up the cache key, and what TTLs to use. _Implementation notes:_ * To achieve the API I wanted for the managed policies, I opted to *not* have `ICachePolicy` extend `IResource`. I admit to not understanding all of the implications of this, so welcome feedback. * I also opted to exclude the `cachePolicyLastUpdatedTimestamp` attribute from both `ICachePolicy` and `CachePolicy`, as the use case for them isn't clear, and handling the property for managed/imported cache policies was awkward. * Lastly, I opted to remove completely (rather than deprecate) the `forwardedValues` property, forcing users to create (or use managed) Cache Policies to achieve the same behavior. * Related to the above, `ForwardedValues` is marked as deprecated and optional in the CloudFormation docs, but if no `CachePolicyId` or `ForwardedValues` are specified, the deployment will fail claiming `ForwardedValues` is missing. That's why I opted to default the `CachePolicyId` to "CachingOptimized". *--NOTE: This functionality is currently broken:--* The CloudFormation support for creating new Cache Policies was released, but subsequent issues have been found and it appears users have been unable to create Cache Policies. See aws-cloudformation/cloudformation-coverage-roadmap#571 Marking this as draft until the above issue is resolved. fixes #9644 BREAKING CHANGES: Distribution `forwardedValues` have been removed in favor of `cachePolicy` and the new CachePolicy construct. * **cloudfront:** Distributions now default to the "CachingOptimized" managed cache policy
skinny85
reviewed
Oct 5, 2020
Contributor
skinny85
left a comment
There was a problem hiding this comment.
Some small comments/suggestion.
Also, I think the "Breaking change" is wrong - the names of the properties that were removed are forwardQueryString and forwardQueryStringCacheKeys.
packages/@aws-cdk/aws-cloudfront/test/integ.distribution-lambda.expected.json
Outdated
Show resolved
Hide resolved
skinny85
reviewed
Oct 7, 2020
Contributor
skinny85
left a comment
There was a problem hiding this comment.
Overall looks great! Couple of minor comments.
shivlaks
reviewed
Oct 7, 2020
Contributor
shivlaks
left a comment
There was a problem hiding this comment.
looks great! some minor suggestions for your consideration 🙂
jogold
reviewed
Oct 7, 2020
njlynch
added a commit
that referenced
this pull request
Oct 7, 2020
Support for the new Origin Request Policy concept in CloudFront, which provides greater control to users over what values are forwarded to the origin from the original viewer request. _Implementation Notes:_ * *Heavily* influenced by #10656, including the same notes on not extending from `IResource` and excluding the `lastModifiedTimestamp`. * Currently marked as a DRAFT, since this PR *requires* #10656; origin request policies cannot be used without cache policies. fixes #9647
Contributor
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
shivlaks
approved these changes
Oct 7, 2020
njlynch
added a commit
that referenced
this pull request
Oct 7, 2020
Support for the new Origin Request Policy concept in CloudFront, which provides greater control to users over what values are forwarded to the origin from the original viewer request. _Implementation Notes:_ * *Heavily* influenced by #10656, including the same notes on not extending from `IResource` and excluding the `lastModifiedTimestamp`. fixes #9647
Collaborator
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Contributor
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
njlynch
added a commit
that referenced
this pull request
Oct 7, 2020
Support for the new Origin Request Policy concept in CloudFront, which provides greater control to users over what values are forwarded to the origin from the original viewer request. _Implementation Notes:_ * *Heavily* influenced by #10656, including the same notes on not extending from `IResource` and excluding the `lastModifiedTimestamp`. fixes #9647
mergify bot
pushed a commit
that referenced
this pull request
Oct 7, 2020
Support for the new Origin Request Policy concept in CloudFront, which provides greater control to users over what values are forwarded to the origin from the original viewer request. _Implementation Notes:_ * *Heavily* influenced by #10656, including the same notes on not extending from `IResource` and excluding the `lastModifiedTimestamp`. * Currently marked as a DRAFT, since this PR *requires* #10656; origin request policies cannot be used without cache policies. fixes #9647 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This was referenced Oct 12, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Support for the new Cache Policy concept in CloudFront, which replaces the
existing
ForwardedValuesproperties and specifies what parts of a requestmake up the cache key, and what TTLs to use.
Implementation notes:
ICachePolicyextendIResource. I admit to not understanding all of theimplications of this, so welcome feedback.
cachePolicyLastUpdatedTimestampattribute fromboth
ICachePolicyandCachePolicy, as the use case for them isn't clear,and handling the property for managed/imported cache policies was awkward.
forwardedValuesproperty, forcing users to create (or use managed) CachePolicies to achieve the same behavior.
ForwardedValuesis marked as deprecated and optionalin the CloudFormation docs, but if no
CachePolicyIdorForwardedValuesarespecified, the deployment will fail claiming
ForwardedValuesis missing.That's why I opted to default the
CachePolicyIdto "CachingOptimized".fixes #9644
BREAKING CHANGE: Distribution
forwardQueryStringandforwardQueryStringCacheKeyshave been removed in favor ofcachePolicyand the new CachePolicy construct.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license