Feat/prsd 1544 maintenance page #184
Conversation
…policy to allow cloudfront access
| @@ -0,0 +1,75 @@ | |||
| # tfsec:ignore:aws-s3-enable-bucket-logging | |||
There was a problem hiding this comment.
I've left logging off for now - do we think we need it?
There was a problem hiding this comment.
I'd be tempted to use it - but it's not a big issue I don't think.
| } | ||
| } | ||
|
|
||
| # KMS encryption is not supported for s3 buckets configured as a static website endpoint |
There was a problem hiding this comment.
This is what Copilot tells me, it wasn't entirely clear from the documentation if you could get around it with Cloudfront but hopefully this is enough?
This is the default and what we use on the log buckets (s3_bucket/main.tf)
There was a problem hiding this comment.
I think there's a bit of complexity around website endpoints vs REST endpoints for s3 that we'll need to resolve, which affects how we configure the origin, whether the bucket needs to be public, and how we handle redirects in the bucket. Happy to chat through that in a bit more detail.
| domain_name = aws_s3_bucket.maintenance_page_bucket.website_endpoint | ||
| origin_id = local.maintenance_origin_id | ||
|
|
||
| custom_origin_config { |
There was a problem hiding this comment.
So I think this comes down to whether we're using a website endpoint or a REST endpoint for the s3 bucket. If we use a website endpoint then we can't use OAC or OAI according to the AWS docs, and we need to make the bucket public (but can still restrict using IAM I think).
Alternatively we can use a REST endpoint. The terraform docs (https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudfront_distribution#origin-arguments) suggest we don't want to use this block for s3 origins in that case - looks like EPB used s3_origin_config instead with a origin_access_identity which they then use in the IAM policy for the bucket.
Note that if we do that I think we need to use error pages in cloudfront to do redirects rather than setting up redirects on the bucket itself, as they're not supported for REST endpoints.
| origin_ssl_protocols = ["TLSv1.2"] | ||
| } | ||
|
|
||
| custom_header { |
There was a problem hiding this comment.
If the bucket isn't public we don't need the custom header, as we haven't got a way to check it without a load balancer or WAF web ACL between Cloudfront and the bucket, and requests that don't come from within AWS will be blocked by the IAM policy anyway (see other comment about using an OAI to restrict that)
| @@ -0,0 +1,75 @@ | |||
| # tfsec:ignore:aws-s3-enable-bucket-logging | |||
There was a problem hiding this comment.
I'd be tempted to use it - but it's not a big issue I don't think.
# Conflicts: # terraform/production/main.tf # terraform/test/main.tf
JasminConterioSW
force-pushed
the
feat/prsd-1544-maintenance-page
branch
from
7072e2f to
f28a00a
Compare
JasminConterioSW
force-pushed
the
feat/prsd-1544-maintenance-page
branch
from
f28a00a to
7fb9671
Compare
Ticket number
PRSD-1544
Goal of change
Summary of the problem the PR is trying to solve - usually a 1 sentence summary of the ticket
Description of main change(s)
First pass added
maintenance_mode_onvariablecustom_headerto match the main origin - do we need this? Does it work with http only?ordered_cache_policyto the distribution - assume this is checked before using default. Set thepath_patternto "/maintenance" or "*" depending on the value ofmaintenance_mode_onto choose which traffic to send to the second origin.viewer_protocol_policytoredirect-to-https- I'm hoping this is ok for accessing Cloudfront even though the s3 bucket has to be http onlyAnything you'd like to highlight to the reviewer?
Note this is not tested at all, I wanted to check if it looks sensibly before deploying to integration.
Based on blog posts https://dev.to/chinmay13/implementing-aws-cloudfront-with-multiple-origin-cache-behavior-using-terraform-4i51 and https://medium.com/@donnyspi/maintenance-pages-made-easy-fc795f70167a
Checklist
Delete any that are not applicable, and add explanation below for any that are applicable but haven't been done
Any special release instructions (e.g. set environment variables) have been added as checklist items to a draft PR (merging
mainintotest) for the next release.Make sure to include details such as the name of the variable, where it needs to be set (e.g. AWS Secrets Manager or Parameter Store), and what it should be set to (this might be a location in keeper where a secret value can be found)