feat(cloudfront): Custom origins and more origin properties #9137
Conversation
Adds support to the new Distribution construct for custom HTTP origins and more properties on both S3 and HTTP-based origins. fixes #9106
| @@ -59,6 +59,17 @@ CloudFront's redirect and error handling will be used. In the latter case, the O | |||
| underlying bucket. This can be used in conjunction with a bucket that is not public to require that your users access your content using CloudFront | |||
| URLs and not S3 URLs directly. | |||
|
|
|||
| #### From an HTTP endpoint | |||
|
|
|||
| Origins can also be created from other resources (e.g., load balancers, API gateways), or from any accessible HTTP server, given the domain name. | |||
There was a problem hiding this comment.
Feels like a dependency issue is awaiting here... If we want to allow something like Origin.fromRestApi it means we will need this module to depend on the APIGW module (and any other CDK module for these integrations), which can be problematic from a layer standpoint.
Normally in cases like this we introduce another module (e.g. @aws-cdk/aws-cloudfront-origins) which is the "integration" module.
There was a problem hiding this comment.
I agree a new module makes sense for integrations with ALBs, APIGW, etc.
- Do you think that module creation should happen as part of this PR? (I'm trying to be cognizant of my PR sizes.)
- Would you also recommend retroactively move the S3 integration into this new module?
Here's my take:
This PR itself isn't exposing the higher-level convenience methods for LBs, gateways, etc. It's building the basic foundation to unblock non-S3 origins for customers. As far as CloudFront is (intrinsically) concerned, there are S3 bucket origins and HTTP origins. This PR solves for the latter. As future work, a aws-cloudfront-origins module should be created and populated with a few convenience methods for the most likely sources like load balancers. S3, as a "core" element of CloudFront, should stay as-is.
There was a problem hiding this comment.
The end game (in my opinion) should be that all types of origins are defined and discoverable through -origins module. It does not really matter which ones are directly supported by CF and which ones are higher-level. From a user's perspective, all of them should be equal.
Since this is going to be a breaking change, I would encourage to create this new module and move all origin types to it earlier rather than later. Otherwise you'll risk needing to maintain the legacy.
| * This method takes in the originPath, and returns it back (if undefined) or adds/removes the '/' as appropriate. | ||
| */ | ||
| private validateOriginPath(originPath?: string): string | undefined { | ||
| if (originPath === undefined) { return undefined; } |
There was a problem hiding this comment.
What's the appropriate way to handle this? If the input is a token at this point, it can't be validated. So just take the token as-is, and skip validation, or check & throw an error?
| if (originPath === undefined) { return undefined; } | |
| if (!Token.isUnresolved(originPath) && originPath === undefined) { return undefined; } |
There was a problem hiding this comment.
Yap, don't validate if it's a token.
Whitespace change. Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
|
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). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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). |
Adds support to the new Distribution construct for custom HTTP origins and more properties on both S3 and HTTP-based origins. fixes aws#9106 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds support to the new Distribution construct for custom HTTP origins and more properties on both S3 and HTTP-based origins. fixes aws#9106 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds support to the new Distribution construct for custom HTTP origins and
more properties on both S3 and HTTP-based origins.
fixes #9106
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license