cloudflare / terraform-provider-cloudflare

Cloudflare Terraform Provider
https://registry.terraform.io/providers/cloudflare/cloudflare
Mozilla Public License 2.0
778 stars 599 forks source link

Support for `cloudflare_worker_script` resources being module syntax #1417

Closed mekanoe closed 2 years ago

mekanoe commented 2 years ago

Current Terraform and Cloudflare provider version

$ terraform -v
Terraform v1.1.4
on linux_amd64
+ provider registry.terraform.io/cloudflare/cloudflare v3.8.0

Description

Similar to the proposed upstream change (cloudflare/cloudflare-go#794), cloudflare_worker_script resource needs to be aware of Worker module syntax. The quick tl;dr: Workers only accepts module-based workers when uploaded with Content-Type: application/javascript+module header.

It seems most worth it to add a module boolean parameter, and pass through through to cloudflare-go.

Attempting to upload a valid worker without this sort of change results in a response:

{
  "code": 10021,
  "message": "Uncaught SyntaxError: Unexpected token 'export'\n  at line 1\n"
}

Use cases

Uploading workers scripts based on ES Modules, documented here: https://developers.cloudflare.com/workers/learning/migrating-to-module-workers

Potential Terraform configuration

resource "cloudflare_worker_script" "hello" {
  name    = "hello"
  content = "export default { async fetch() { return new Response('hello-world') } }"
  module  = true
}

References

sodabrew commented 2 years ago

My $0.02 suggestion:

Instead of:

resource "cloudflare_worker_script" "hello" {
  name    = "hello"
  content = "export default { async fetch() { return new Response('hello-world') } }"
  module  = true   # If false, then type worker is implied
}

How about:

resource "cloudflare_worker_script" "hello" {
  name    = "hello"
  content = "export default { async fetch() { return new Response('hello-world') } }"
  type    = "module" # default is "worker"
}
moritonal commented 2 years ago

Currently I have a fairy hacky start of a solution where I have to use npx wrangler publish --outdir dist --dry-run to build the compiled script, then use content = file("dist/index.js") in Terraform, only to get Uncaught SyntaxError: Unexpected token 'export' which actually means that the provider didn't add a header during upload.

What I've learnt is that this only fails when you've outputted a moduled Worker. If instead you build your worker using the old addEventListener method as shown here, then it works fine.

js0cha commented 2 years ago

Hello ,

I am currently blocked by this. I am implementing the service worker bindings like instructed in the official docs but it fails in my terraform (Unexpected token 'export'). Do we have some trick to bypass this stupid error ? I am not using wrangler

dotansimha commented 2 years ago

Same here. Seems like the actual change the needs to be done is in the Content-Type.

Sending a regular worker code (event handler)

image

Content-Type is set to application/javascript

Sending a module-based worker

image

Content-Type is set to application/javascript+module

dotansimha commented 2 years ago

It seems like https://github.com/cloudflare/cloudflare-go/issues/794 fixed this issue, and we might be able to use it here soon?

sodabrew commented 2 years ago

@dotansimha it's halfway there -- uploading a module script works but downloading it returns a raw multipart/form-data body, which would put the work onto the caller to parse in order to compare the terraform state and determine whether the resource has changed. See https://github.com/cloudflare/cloudflare-go/pull/1040 for my personal first effort at a solution, but there isn't an official direction on this yet, nor on how to handle a multiscript bundle.

sodabrew commented 2 years ago

https://github.com/cloudflare/cloudflare-go/pull/1040 is landed! With cloudflare-go 0.49.0, it will be possible to both push a module-format worker script and also download one, enabling the terraform state refresh and comparison steps.

Multiple-script workers are still an open issue that isn't fully explored, but I probably won't be pushing through solving that.

github-actions[bot] commented 2 years ago

This functionality has been released in v3.23.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!