Configure CloudFront proxy for Tilegarden #629
Conversation
Sketch out the interaction between Tilegarden and a CloudFront CDN in deployment configs. In addition, adjust the TILEGARDEN_ROOT settings variable in Django so that it corresponds to an environment variable that can be propagated into Django containers. Note that the infrastructure in this commit has not been tested on staging yet, so there are likely further tweaks necessary in order to get it fully operational.
CloudFront was complaining because: * The distribution could not update properly while 'minimum_protocol_version' was unset due to hashicorp/terraform-provider-aws#407 * The distribution domain name needed 'https://' to be appended to it before the app could retrieve tiles from it Fix both of these issues. In addition, add an explicit 'jest' config block to Tilegarden's package.json in order to prevent testing errors in Babel 7.
Add a section on Tilegarden to 'deployment/README.md' documenting the necessary steps for standing up a new instance of Tilegarden with Claudia.
A previous commit introduced a superstitious Jest configuration block to src/tilegarden/package.json in an attempt to fix mysteriously failing tests. The tests are now passing without the config block, so remove it.
| @@ -157,3 +157,5 @@ variable "aws_cloudwatch_logs_policy_arn" { | |||
| variable "pfb_app_alb_ingress_cidr_block" { | |||
| type = "list" | |||
| } | |||
|
|
|||
| variable "tilegarden_api_gateway_domain_name" {} | |||
There was a problem hiding this comment.
Since Claudia creates the API Gateway resource for Tilegarden separately from Terraform, we need to specify this value as an input variable.
deployment/terraform/cdn.tf
Outdated
|
|
||
| viewer_certificate { | ||
| cloudfront_default_certificate = true | ||
| minimum_protocol_version = "TLSv1" |
There was a problem hiding this comment.
Specifying minimum_protocol_version is necessary in order to get around a Terraform bug that causes Terraform to try to recreate the distribution every time it creates a plan (see bug and related fix). Although a fix was brought into Terraform to address this, the version of Terraform running in the VM is sufficiently out of date that it doesn't include the fix.
jeancochrane
changed the title
deployment/terraform/app.tf
Outdated
| @@ -152,6 +152,7 @@ data "template_file" "pfb_app_https_ecs_task" { | |||
| batch_analysis_job_definition_name_revision = "${var.batch_analysis_job_definition_name_revision}" | |||
| batch_tilemaker_job_queue_name = "${var.batch_tilemaker_job_queue_name}" | |||
| batch_tilemaker_job_definition_name_revision = "${var.batch_tilemaker_job_definition_name_revision}" | |||
| tilegarden_root = "https://${aws_cloudfront_distribution.tilegarden.domain_name}" | |||
There was a problem hiding this comment.
I think that we should map the CloudFront distribution domain to a public DNS record that we control via an ALIAS record. Then, I think that we should thread that domain name through here.
There was a problem hiding this comment.
Good call. Does using the tiles. subdomain of the environment hostname sound reasonable?
There was a problem hiding this comment.
Yeah, I think that's been our rough convention.
Instead of accessing Tilegarden via the CloudFront default domain, use the 'tiles' subdomain of the deployed app as an alias.
| `LAMBDA_REGION`. Other optional variables can be uncommented and edited to | ||
| reflect your configuration. If you need Tilegarden to access a database, for | ||
| example, you'll likely want to set `LAMBDA_SUBNETS` and `LAMBDA_SECURITY_GROUPS` to point | ||
| to the relevant resources in your VPC. |
There was a problem hiding this comment.
How does this interact with "it's best to finish these steps before you deploy a new stack with Terraform"? Seems like there's a bit of circularity here that would require bootstrapping, i.e. the process for creating a new stack would need to be "batch, terraform, tilegarden, terraform"? (Or would it be "batch, tilegarden, terraform, tilegarden"? Or is there actually a linear way to do it?)
If you need Tilegarden to access a database
Tilegarden isn't good for much without access to a database, at least not on this project. I might rephrase this to "For example, to give Tilegarden access to the database, set..."
There was a problem hiding this comment.
This is an excellent point. It's possible you could get away with doing it linearly (Batch -> Tilegarden -> Terraform) if you used an existing VPC, but we're not doing that here, so realistically this is implying that you'll have to do Batch -> Terraform -> Tilegarden -> Terraform.
I think we could make the setup easier if we were at least explicit about this workflow, and gave the user some guidance in doing it. My expectation is that if you were to set the tilegarden_api_gateway_domain_name variable to some bogus value on the first Terraform run, the resources will build properly, but the CDN won't return tiles properly; then if you adjust the variable again after setting up Tilegarden things should wire up correctly on the second terraform apply.
I don't love this workflow, but it's the best I can think of given the constraints we're under, where the Lambda function and the CDN are interdependent but each has to be configured and managed from a separate source of truth. If you can think of a way around it, let me know!
Make explicit the expected order of resource creation in the deployment README in order to elaborate the interdependence between Terraform and Claudia resources.
|
Added a section to the docs elaborating the deployment workflow @KlaasH -- let me know if there's anything else that needs fixing! |
There was a problem hiding this comment.
👍 Looks great!
The timeout on the Lambda function is set to 3 seconds. The #LAMBDA_TIMEOUT=15 in .env.example makes it seem like 15 seconds is the default if another value isn't specified (since it says "increase this value"), but I don't see where that default would actually be set. And it looks like 3 seconds is the Lambda default. Probably we should uncomment that line in .env.example and in the staging .env. (I also wonder if bumping it to 30 would make sense, since slow tiles, especially if they're cached, are better than no tiles...)
So I guess that's a requested change, but other than that 👍
|
That change makes a lot of sense to me @KlaasH! I was noticing timeouts on certain tiles while playing with staging. I'll bump the defaults to 30 and merge this once I've confirmed that the changes are up in staging. |
The default Lambda timeout is 3 seconds. Without a tile cache, many Tilegarden requests were coming in over this limit and timing out, so set the Tilegarden timeout to 60 seconds. In addition, add PFB-specific variables to the .env.example template for Tilegarden.
Overview
This PR wires up a CloudFront proxy for Tilegarden and puts the finishing touches on the Django/Terraform config in order to get Tilegarden serving tiles on staging.
Demo
Tilegarden on staging:
Notes
Testing Instructions
Closes #594, closes #624, closes #625