Create Fastly service for static.crates.io #155

jdno · 2022-11-21T16:20:17Z

This is a first pass at a Fastly service for static.crates.io (tracking issue rust-lang/infra-team#62).

terraform/crates-io/impl/fastly-static.tf

jdno · 2022-11-23T14:18:39Z

terraform/crates-io/compute-static/src/main.rs

+    let status_code = response.get_status().as_u16();
+
+    if status_code >= 500 {
+        warn!(


The setup for logging is not complete yet, and we are not streaming logs. So this warning will not show up anywhere.

jdno · 2022-11-28T11:04:03Z

Just a heads-up that I'll be setting up terragrunt for the crates-io module, and then I will rebase this PR.

jdno · 2022-12-13T16:00:46Z

I've rebased the pull request on a very recent commit in master and moved the resources into the new terragrunt structure.

jdno · 2022-12-13T16:06:14Z

The following things still need to be done:

Set up weighted routing for DNS records
- Rename the domain name of the CloudFront distribution (e.g. cloudfront-${var.static_domain_name})
- Add a variable to configure the weights
- Create a DNS record for var.static_domain_name for each CDN
Set TTL explicitly
Set up log streaming
- Figure out where to send the logs
- Configure the log destination
Add some tests (if it's possible to unit test the function)

My preference would be to merge this pull request and then work through these items in separate branches. This pull request is long enough as it is, and it'll be easier to review the changes one-by-one.

jdno · 2022-12-14T12:31:40Z

This feels like a good place to stop and merge.

The configuration only sets up Fastly right now, which will make it possible to deploy it to production as well and test there without touching any existing resources.

All changes beyond this point will interact with the CloudFront distribution or with DNS, which we probably want to review and test separately.

rylev · 2022-12-14T13:34:21Z

terragrunt/modules/crates-io/fastly-static.tf

@@ -0,0 +1,123 @@
+# The Fastly service must be deployed in two steps, since some resources depend


I'm surprised by this - isn't terraform supposed to be able to handle dependencies like this?

Yes and no.

The problem here is that the newly created resources are used as keys in a for_each statement:

Error: Invalid for_each argument on fastly-index.tf line 74, in resource "aws_route53_record" "index_tls_validation": 74: for_each = { 75: for challenge in fastly_tls_subscription.index.managed_dns_challenges : 76: trimprefix(challenge.record_name, "_acme-challenge.") => challenge 77: } ├──────────────── │ fastly_tls_subscription.index.managed_dns_challenges is a set of object, known only after apply The "for_each" map includes keys derived from resource attributes that cannot be determined until apply, and so Terraform cannot determine the full set of keys that will identify the instances of this resource. When working with unknown values in for_each, it's better to define the map keys statically in your configuration and place apply-time results only in the map values. Alternatively, you could use the -target planning option to first apply only the resources that the for_each value depends on, and then apply a second time to fully converge.

The same problem exists when rewriting this block to use count:

Error: Invalid count argument on fastly-index.tf line 72, in resource "aws_route53_record" "index_tls_validation": 72: count = length(fastly_tls_subscription.index.managed_dns_challenges) The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the count depends on.

The only potential workaround I've found is splitting the configuration into two modules, but those would still need to be applied after each other. We can do that with dependencies in Terragrunt, but that would mean we need a state for the fastly_tls_subscription and a state for the validation.

crates-io-staging ├── crates-io ├── fastly-tls-subscription └── fastly-tls-validation

One could then apply all changes by running terragrunt run-all apply from the crates-io-staging directory, which will resolve the dependencies and apply the changes in the right order.

Thinking more about this, I kinda like this approach. It makes the dependency explicit in the tooling, and not in the documentation. What do you think?

I've put together a quick proof-of-concept here: jdno@28bb026 Haven't tried actually running it, though.

Without too much experience here, I can't have a strong opinion, but the suggestion feels better than commenting out code.

I'm a bit confused by this, how's AWS validation working then? It also generates the DNS records dynamically.

Here is the relevant part of a new aws_acm_certificate in a Terraform plan:

resource "aws_acm_certificate" "cert" { domain_validation_options = [ { domain_name = "test.jdno.dev" resource_record_name = (known after apply) resource_record_type = (known after apply) resource_record_value = (known after apply) }, ]

And here is the fastly_tls_subscription:

resource "fastly_tls_subscription" "subscription" { managed_dns_challenges = (known after apply)

The reason why AWS works is that the number of domain_validation_options with their respective domain_name, i.e. the key for the for_each statement, is known when planning.

We can create a feature request to get this added to terraform-provider-fastly, but in the meantime we are stuck with either running the initial apply twice or using dependencies.

Opinions, @pietroalbini and @rylev?

I've created a feature request for the Fastly provider: fastly/terraform-provider-fastly#627

Ooh ok, your explaination on the issue (unfortunately) makes a lot of sense. Ugh. I'd go with the two applies for now. Also, can you flag the issue with our Fastly contact to see if it can be prioritized on their end?

I forwarded both issues to our contact and will post an update in the team meeting when I know more about the priorities.

pietroalbini · 2022-12-16T12:06:59Z

terragrunt/modules/crates-io/compute-static/Cargo.lock

@@ -0,0 +1,394 @@
+# This file is automatically @generated by Cargo.


Is this lockfile actually used? The dependencies should be included in the one at the root of the repo due to the workspace.

It wasn't until I added the external data source. But since Terragrunt copies the Rust crate into a temporary build environment, we cannot provide a stable path to include the crate in the workspace.

pietroalbini · 2022-12-16T12:09:27Z

terragrunt/modules/crates-io/compute-static/README.md

+Build the function:
+
+```shell
+cd compute-static
+fastly compute build
+```
+
+Export an API token for Fastly and then run Terraform:
+
+```shell
+export FASTLY_API_KEY=""
+terraform plan
+```


I'm worried that you have to remember to run fastly compute build to get the right configuration, and if you don't changes will be silently not applied. Can we use the external data source to invoke the compilation as part of planning?

Done. I had to add a shell scripts that runs fastly compute build and returns a valid JSON object, though.

pietroalbini · 2022-12-16T12:18:15Z

Set up log streaming

Figure out where to send the logs

Configure the log destination

Note that we do not collect logs from CloudFront either as of today.

jdno · 2022-12-19T11:53:46Z

Log streaming might be interesting for debugging our functions. Not so much for request logs.

The `managed_dns_challenge` attribute has been deprecated by Fastly. It's replacement is only known after the `fastly_tls_subscription` has been applied, which requires us to deploy the Fastly service in two steps.

The CNAME for the Fastly service cannot be accessed through a Terraform resource. As a hack, we can get it from the ACME HTTP challenge.

The Fastly function is now built whenever `terraform plan` is run. This This ensures that the function is always up to date, and prevents users from accidentally uploading a stale version of the WASM module. This is implemented using an external data source [^1] inside Terraform. The data source calls a script that compiles the function and returns the path to the artifact. [^1]: https://registry.terraform.io/providers/hashicorp/external/latest/docs/data-sources/data_source

An outdated Cargo.lock file got pushed to the default branch when rebasing pull request rust-lang#155.

jdno commented Nov 21, 2022

View reviewed changes

terraform/crates-io/impl/fastly-static.tf Outdated Show resolved Hide resolved

jdno commented Nov 21, 2022

View reviewed changes

terraform/crates-io/impl/fastly-static.tf Outdated Show resolved Hide resolved

jdno requested a review from pietroalbini November 21, 2022 16:38

jdno mentioned this pull request Nov 23, 2022

Migrate static.crates.io to Fastly rust-lang/infra-team#62

Closed

27 tasks

jdno commented Nov 23, 2022

View reviewed changes

jdno force-pushed the fastly-static-staging branch from e7fc60a to 768191c Compare December 12, 2022 13:23

jdno force-pushed the fastly-static-staging branch from db93010 to a74163e Compare December 14, 2022 12:27

jdno marked this pull request as ready for review December 14, 2022 12:29

rylev reviewed Dec 14, 2022

View reviewed changes

pietroalbini requested changes Dec 16, 2022

View reviewed changes

pietroalbini approved these changes Dec 21, 2022

View reviewed changes

jdno added 9 commits December 21, 2022 12:20

Create Fastly service for static.crates.io

5d0de69

Switch to compute service

1dccc14

Create a DNS record for Fastly service

ba62c0b

Update documentation for edge function

6e6a15d

Replace use of deprecated attribute

9dc90d1

The `managed_dns_challenge` attribute has been deprecated by Fastly. It's replacement is only known after the `fastly_tls_subscription` has been applied, which requires us to deploy the Fastly service in two steps.

Get CNAME for Fastly from Terraform

da7cb34

The CNAME for the Fastly service cannot be accessed through a Terraform resource. As a hack, we can get it from the ACME HTTP challenge.

Add comment about deploying the Fastly service

d69699c

Delete obsolete Cargo.lock file

986d062

jdno force-pushed the fastly-static-staging branch from a84c0c7 to 509df69 Compare December 21, 2022 11:24

Fix formatting of Terraform file

8bad725

jdno merged commit 5757d44 into rust-lang:master Dec 21, 2022

jdno deleted the fastly-static-staging branch December 21, 2022 11:33

jdno added a commit to jdno/rust-simpleinfra that referenced this pull request Dec 21, 2022

Update Cargo.lock

b8d44f4

An outdated Cargo.lock file got pushed to the default branch when rebasing pull request rust-lang#155.

jdno mentioned this pull request Dec 21, 2022

Update Cargo.lock #177

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Fastly service for static.crates.io #155

Create Fastly service for static.crates.io #155

jdno commented Nov 21, 2022

jdno Nov 23, 2022

jdno commented Nov 28, 2022

jdno commented Dec 13, 2022

jdno commented Dec 13, 2022

jdno commented Dec 14, 2022

rylev Dec 14, 2022

jdno Dec 14, 2022

jdno Dec 14, 2022

rylev Dec 14, 2022

pietroalbini Dec 16, 2022

jdno Dec 19, 2022

jdno Dec 20, 2022

pietroalbini Dec 21, 2022

jdno Dec 21, 2022

pietroalbini Dec 16, 2022

jdno Dec 19, 2022

pietroalbini Dec 16, 2022

jdno Dec 19, 2022

pietroalbini commented Dec 16, 2022

jdno commented Dec 19, 2022

		@@ -0,0 +1,123 @@
		# The Fastly service must be deployed in two steps, since some resources depend

		@@ -0,0 +1,394 @@
		# This file is automatically @generated by Cargo.

Create Fastly service for static.crates.io #155

Create Fastly service for static.crates.io #155

Conversation

jdno commented Nov 21, 2022

Choose a reason for hiding this comment

jdno commented Nov 28, 2022

jdno commented Dec 13, 2022

jdno commented Dec 13, 2022

jdno commented Dec 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietroalbini commented Dec 16, 2022

jdno commented Dec 19, 2022