Update CloudFormation template. #435
Conversation
|
|
||
| PublicHostname: | ||
| Description: 'The public custom domain name for your CloudFront distribution' | ||
| Description: 'Optional. Replace *.cloudfront.net in TileJSON with a custom hostname (e.g. example.com). See docs on how this value is cached.' |
There was a problem hiding this comment.
See CloudFront function below for explanation on how this works.
The end result here is that the PublicHostname is optional - if you leave it blank the TileJSON will be filled in with the *.cloudfront.net domain name.
| - logs:CreateLogStream | ||
| - logs:PutLogEvents | ||
| Resource: '*' | ||
| - PolicyName: S3AccessPolicy | ||
| Resource: !Sub "arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/lambda/${AWS::StackName}:log-stream:*" |
There was a problem hiding this comment.
Scope the resource more closely instead of using *
| # ########################################################################## | ||
| # # CloudFront::Distribution # | ||
| # ########################################################################## | ||
| ViewerRequestCloudFrontFunction: |
There was a problem hiding this comment.
This is necessary in the case where PublicHostname parameter is left blank.
By default, the Lambda has no way to know what the CloudFront distribution's domain name is.
One way to do this is to set PUBLIC_HOSTNAME to reference the CloudFront resource in the CF template, but this introduces a circular dependency between the Lambda and the Distribution.
Some hacks to remove circular dependencies like listed in https://theburningmonk.com/2022/05/how-to-work-around-cloudformation-circular-dependencies/ such as creating the Lambda function ARN by hand don't work; the Lambda Function URL checks on provisioning the referenced function exists.
So instead, what we do is we check for the PublicHostname parameter, if blank we install a CloudFront Function that simply attaches the distribution domain name as the x-distribution-domain-name header that the Lambda can then read from.
There was a problem hiding this comment.
(The code for reading x-distribution-domain-name is not yet in this PR)
There was a problem hiding this comment.
The downside of adding CF Functions is that it costs $0.1 per million requests that go into CloudFront (very little)
| ViewerProtocolPolicy: redirect-to-https | ||
| CachePolicyId: !Ref CachePolicyId | ||
| ResponseHeadersPolicyId: !Ref ResponseHeadersPolicyId | ||
| OriginRequestPolicyId: b689b0a8-53d0-40ab-baf2-68738e2966ac |
There was a problem hiding this comment.
This is hardcoded to AllViewerExceptHostHeader as described in the docs:
read x-distribution-name header in TileJSON response if PUBLIC_HOSTNAME is not set.
|
Here is the full output YAML with inlined Lambda code, 494 lines after minification: https://gist.github.com/bdon/ce3180498561fb0904b7d7ac6df441de |
serverless/aws/src/index.ts
Outdated
| headers["Content-Type"] = "application/json"; | ||
|
|
||
| const t = tileJSON( | ||
| header, | ||
| await p.getMetadata(), | ||
| process.env.PUBLIC_HOSTNAME, | ||
| process.env.PUBLIC_HOSTNAME || event.headers['x-distribution-domain-name'] || "", |
There was a problem hiding this comment.
Prior to this change, this function was returning a 501 if no public hostname could be resolved. Here, you're returning an empty string.
Besides notifying a user their setup may not be configured correctly, are there any other reason to avoid returning an empty string?
There was a problem hiding this comment.
It should probably go back to giving a 501 if both the PUBLIC_HOSTNAME and the distribution domain name header are both blank.
serverless/aws/package.json
Outdated
| }, | ||
| "private": true, | ||
| "scripts": { | ||
| "tsc": "tsc --noEmit --watch", | ||
| "build": "esbuild src/index.ts --target=es2020 --outfile=dist/index.mjs --format=esm --bundle --platform=node --target=node18 --external:@aws-sdk/client-s3 --external:@aws-sdk/node-http-handler --banner:js=//$(git describe --always) && cd dist && zip lambda_function.zip index.mjs", | ||
| "build": "esbuild src/index.ts --target=es2020 --minify --outfile=dist/index.js --format=cjs --bundle --platform=node --target=node20 --external:@aws-sdk/client-s3 --external:@aws-sdk/node-http-handler --banner:js=//sha:$(git describe --always)", |
There was a problem hiding this comment.
Any thoughts on including the template as part of the distribution for PMTiles?
It would be great to be able load the compiled yaml directly in JS ('import { pmtilesCFN } from "pmtiles"'). This could be used alongside CfnInclude to embed this infra in existing CDK stacks. Otherwise, clients will need to manually copy the template.
There was a problem hiding this comment.
do you mean as part of the pmtiles NPM package, that can be included as a AWS CDK dependency?
Originally I kept the Cloudflare and AWS specific code out of the main JS package to reduce versioning churn and dependencies, but since those are stabilizing we can consider this in a next major version.
There was a problem hiding this comment.
Yup, any NPM package can be used in typescript based CDK projects.
|
Build output URL: https://pmtiles.io/cloudformation-stack.yaml |
Updates to make the CF template as one-click as possible.
Not yet inlining the Lambda zip, that will come in a future commit.
Comments inline
@Honricris @charliemcgrady