-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create cloudformation script #1
Conversation
9476a3a
to
b41ddd9
Compare
Right now it is missing CloudFront "error pages" configuration. We need to decide how we want to handle the following errors:
|
b41ddd9
to
cf8c245
Compare
Despite CloudFront AND S3 allowing a number of symbols in their bucket directory names (cloudfront: origin path).. I tried the following combination for a base path: /a-zA-Z0-9_.;:!@$&()=+-/a-zA-Z0-9_.;:!@$&()=+- Even though it worked for the cloudformation script (with a successful deployment) and maintained the value... it doesn't serve the page. I believe I'll tighten up the OriginPath regex unless anyone has any objections. The resulting url: /a-zA-Z0-9_.%253B%253A*!%2540%2524%2526()%253D%252B-/a-zA-Z0-9_.%253B%253A*!%2540%2524%2526()%253D%252B-/ |
That fix did the trick. The following OriginPath works just fine: /a-zA-Z0-9_.!()-/a-zA-Z0-9_.!()- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theneverstill Nice work!
I left some comments that I think we could apply before merging it. Or we can handle them as separate PRs.
For now I think it is enough for a first version.
S3Bucket: | ||
Type: AWS::S3::Bucket | ||
Properties: | ||
BucketName: !Join ["", [!Ref "Subdomain", ., !Ref "Domain"]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and in other places I think we could use !Sub
instead of !Join
, like
BucketName: !Sub "${Subdomain}.${Domain}"
Enabled: true | ||
HttpVersion: http2 | ||
IPV6Enabled: false | ||
PriceClass: PriceClass_All |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PriceClass
should be a parameter --> https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/PriceClass.html
From https://docs.aws.amazon.com/cloudfront/latest/APIReference/API_UpdateDistribution.html:
Valid Values: PriceClass_100 | PriceClass_200 | PriceClass_All
DistributionConfig: | ||
Aliases: | ||
- !Join ["", [!Ref "Subdomain", ., !Ref "Domain"]] | ||
DefaultRootObject: index.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default root object should be also a parameter with the default value set to index.html
.
ViewerCertificate: | ||
AcmCertificateArn: !Ref AcmCertificateArn | ||
MinimumProtocolVersion: TLSv1.2_2018 | ||
SslSupportMethod: sni-only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for now, but we should see if we want to configure it in another more flexible way.
No description provided.