Feture request: script environment variables

[jamesarosen]

Terraform Version

$ terraform -v
Terraform v0.12.20
+ provider.cloudflare v2.3.0

Affected Resource(s)

• cloudflare_worker_script

Terraform Configuration Files

I'd like to define some firewall data in Terraform that also gets used in one of my scripts:

locals {
  good_bot_uas = "Good Bot",
  good_bot_ips = [
    "1.2.3.4",
    "2.3.4.5",
  ]
}

resource "cloudflare_filter" "example-com-good-bot" {
  zone_id    = cloudflare_zone.example-com.id
  expression = "(http.user_agent contains \"${local.goot_bot_ua}\" and ip.src in {${join(" " local.good_bot_ips)}})"
}

resource "cloudflare_worker_script" "example-com" {
  name    = "production"
  content = file("build/worker/index.js")

  # can't do this:
  environment {
    RELEASE = "production",
    GOOD_BOT_UA = local.good_bot_ua,
    GOOD_BOT_IPS = join("," local.good_bot_ips),
  }
}

Unfortunately, cloudflare_worker_script doesn't support environment variables. The only solution I can come up with is to store the good_bot info in a separate JSON file, then read it into cloudflare_filter.example-com-good-bot and compile it into the source of cloudflare_worker_script.example-com.

[jacobbednarz]

Workers doesn't currently expose the environment options as a publicly documented resource from what I can see.

You're best bet is to raise a ticket with Cloudflare to ask about it and then we can add it in cloudflare/cloudflare-go for Terraform to consume.

[jamesarosen]

Their API does support it, though you're right that it's not publicly documented. I'll ask them to do so.

GET /api/v4/accounts/<AccountID>/workers/scripts/<WorkerName>/bindings returns something of the form

{
  "result": [
    {
      "name": "MY_ENV_VAR",
      "text": "my value",
      "type": "plain_text"
    },
    {
      "name": "MY_SECRET_ENV_VAR",
      "type": "secret_text"
    }
  ]
}

That same data structure goes into the bindings properties in PUT /api/v4/accounts/<AccountID>/workers/scripts/<WorkerName>

[jacobbednarz]

Yep, we don't implement it if it's not publicly documented as it's stability isn't guaranteed. If you can get that on the team's radar, I'm happy to 👍 a PR for it or take a pass at it if no one else does.

[benwillkommen]

I left a comment on their blog explaining the situation and asking to update their docs.

They have links to Workers Secret API docs in the source code for Wrangler (which also uses the public, undocumented endpoint), but the docs don't actually exist yet 😂.

Hopefully they get updated soon. 🤞

[jacobbednarz]

If you haven’t already, I’d recommend opening a support ticket. I don’t know how often the blog comments are read but I do know the right teams always see support tickets raised by customers.

[jamesarosen]

I have opened a support ticket. This is actually a pretty severe bug, not just a feature-request. Whenever I apply changes for my scripts, this provider deletes all (unencrypted) environment variables for that script.

That's because the API for updating the script expects you to pass back the environment variables. Unfortunately, that also means we must manage all our (unencrypted) environment variables in Terraform. They cannot be disconnected from the worker script.

(Note: I'm not blaming the maintainers of this Terraform provider. Cloudflare made a breaking API change without changing the major version. They broke the SEMVER contract.)

[jacobbednarz]

Cheers for opening that support ticket, hopefully it gets some eyes on the issue.

As it stands, I'm afraid there is very little we can do until there is a publicly documented endpoint otherwise we're building on unstable endpoints which may change from underneath us at any time.

If the maintainers of Wrangler (Cloudflare themselves) are comfortable using undocumented endpoints, would that be a better fit for your needs here? That looks like it currently supports the things you're after.

[kentonv]

Hi @jamesarosen and all,

The bindings endpoint is documented here:

https://developers.cloudflare.com/workers/tooling/api/bindings/

However, secrets (a brand-new feature) are not covered yet. We are in the process of getting that fixed.

Note that the API does not require re-uploading the secrets every time. The API allows you to specify that the old value of a binding should be kept as-is, by specifying the binding as "type": "inherit" on upload. This causes the type and value to be copied from the previous version of the script. This is particularly important to use with secrets since you cannot download the previous value of the secret. Again, apologies that we haven't gotten the documentation for this up yet, we're working on it.

Can you clarify what you mean about this being a breaking change? Does the breakage only occur when using the new secrets feature, or are you observing the problem even when managing scripts that don't use the new feature at all?

EDIT: Oops, my information was slightly incorrect here. It turns out that specifically for secret bindings, if you don't specify the binding on upload, we automatically assume it should be inherited. But all other binding types must be specified explicitly. It sounds like perhaps this thread is not about secret bindings, but regular plaintext environment variables? However, those are also a new feature introduced at the same time as secrets.

EDIT 2: Also note that the concept of bindings is not new. For example, KV namespace bindings have existed for some time, and have always had the behavior that you must specify the binding at every upload. I guess if the Terraform provider doesn't query the bindings endpoint then it must have the effect of deleting KV bindings on updates, too?

[jamesarosen]

It sounds like perhaps this thread is not about secret bindings, but regular plaintext environment variables?

Exactly.

I guess if the Terraform provider doesn't query the bindings endpoint then it must have the effect of deleting KV bindings on updates, too?

That's not really how Terraform is intended to work. The Terraform configuration is meant to be the source of truth. Whenever the developer runs terrafrom apply, Terraform makes all the changes necessary to the systems (e.g. Cloudfalre).

Also note that the concept of bindings is not new. For example, KV namespace bindings have existed for some time, and have always had the behavior that you must specify the binding at every upload.

KV-bindings are implemented in the Cloudflare Terraform provider as the kv_namespace_binding object of cloudflare_worker_script.

Perhaps we need a new more generic binding object in Terraform:

resource "cloudflare_worker_script" "my_script" {
  name = "script_1"
  content = file("script.js")

  # a binding with type="kv_namesapce" is equivalent to kv_namespace_binding,
  # which is still supported for backwards-compatibility
  binding {
    name = "MY_KV"
    # required & allowed if and only if type="kv_namesapce"
    namespace_id = cloudflare_workers_kv_namespace.my_namespace.id
    type = "kv_namespace"
  }

  binding {
    name = "FOO"
    text = "bar"
    type = "plain_text"
  }

  binding {
    name = "MY_API_KEY"
    text = var.MY_API_KEY
    type = "secret"
  }
}

[kentonv]

@jamesarosen I'm still trying to understand if there was actually a breaking API change here that we need to fix. We try very hard not to make backwards-incompatible changes so if we did break something I want to go back and fix it and make sure we don't do that again in the future.

But from what I can tell here, it sounds like the problem is that Terraform is deleting environment variables from an existing workers deployment? As you mention, Terraform is intended to be the source of truth. Since the Terraform provider doesn't support environment variables yet, isn't it expected that they'd be deleted when deploying via Terraform? Or is there something else going on here that I'm not understanding?

[jamesarosen]

Terraform is supposed to be the source of truth for things it knows about.

For example, Terraform knows about global zone settings like waf: "on". So my configuration might look like

resource "cloudflare_zone_settings_override" "example-com" {
  settings: {
    waf = "on"
  }
}

If Cloudflare introduces a new feature, "QUIC support," Terraform won't know about it. It won't appear in my config and the Cloudflare provider won't send any information about it to the API. I can still manage it in the UI (or by manual API calls). Let's say I turn QUIC on in the UI then run terraform apply. What happens? There are two possibilities:

1. PATCH-style. The API for global zone settings assumes that if you don't provide a value, you don't mean to change it. In this case, Terraform sends { "waf": "on" } and nothing happens to the QUIC setting. If I've turned it on in the UI, it's still on.
2. PUT-style. The API for global zone settings assumes you specify all the values. If you don't specify one, you get "off" (or perhaps the default). In this case, Terraform sends { "waf": "on" }, which disables QUIC.

Bindings are PUT-style. If Terraform doesn't know about something, it gets deleted. That means that if I want to use Terraform, I can't use environment variables until Terraform supports them.

Is this a breaking API change? I'm not sure. It depends on whether you think I should be able to use Terraform for feature A and the UI for (seemingly unrelated) feature B.

[jacobbednarz]

Thanks for dropping by @kentonv to clarify 🤗 There are a few moving parts here but I think we have enough information here now to formulate a path forward.

• Update cloudflare/cloudflare-go worker API to support environment and secret variables.
• Pull the update into Terraform
• Look to consolidate (or expand to individual blocks) the bindings configuration of Workers. I think the single generic approach is probably better going forward as then if a new binding type is added, we just need to allow it as a value and we’re good to go.

How does this sound?

[kentonv]

It depends on whether you think I should be able to use Terraform for feature A and the UI for (seemingly unrelated) feature B.

Ah. I guess I'd argue that worker scripts and their bindings are pretty closely related -- indeed, our internal data model ties these together very closely. But I can also understand how someone might think of them as separate, and expect that manipulating one doesn't affect the other. I'm sorry this led to issues for you.

@jacobbednarz Sounds reasonable to me, but there's one tricky thing with secrets: The person running Terraform shouldn't be required to have a copy of the secret locally, since this significantly increases the risk of leaking the secret. Terraform either needs to understand how to send the binding as "type": "inherit", or maybe it should not send the binding at all and rely on the fact that secrets are automatically inherited. I guess an interesting question is how, exactly, a Terraform user should go about uploading the secret the first time... do they use Terraform for that or do they use Cloudflare's UI / other tooling?

[jacobbednarz]

We try to follow the internal modelling as much as possible to avoid issues like this, unfortunately, I think because the initial implementations separated them, that has been the assumption going forward despite the product moving in another direction.

With regards to the secrets store, we have a couple of options and which approach we take will depend on the way the API presents them to clients (concealed vs in the clear). We’ve got some spots in the provider that set the value but never update it in place because it’s not stored but that isn’t always the best approach. On the Terraform side of things, we also draw the assumption that you’re responsible for your state file. If you don’t protect the information and values in it, that is a shortcoming on your behalf since some of the sensitive resources, outside of Cloudflare, require being managed.

In terms of the workflow, Terraform needs to be the source of truth otherwise it can’t manage the state properly so adding the values would be done in Terraform unless the resource was already created in the UI and imported.

[sergiitk]

Here's some more official docs on secrets and text bindings:

• https://developers.cloudflare.com/workers/tooling/api/scripts/#add-a-plain-text-binding
• https://developers.cloudflare.com/workers/tooling/api/scripts/#add-a-secret-text-binding

[sergiitk]

Here's my 2 cents: currently I'm unable to update a worker script containing environment variables using terraform. On terraform apply I'm getting:

Error: error updating worker script: error from makeRequest: HTTP status 400: 
...
 "message": "Uncaught ReferenceError: VAR_NAME is not defined

Where VAR_NAME is one of environment variables I use in the script.

As some folks mentioned above, metadata block with bindings must be supplied. Currently terraform only does this for KV bindings.

[jamesarosen]

One other thing to consider: Terraform generally takes the stance that "if it's not represented in .tf, Terraform shouldn't modify it." That implies that if I set an environment variable in the UI and then run terraform apply, it shouldn't unset the variable. That is, this provider should read out the variables currently set from the Cloudflare API, then overlay the ones in .tf files on top of that, and finally write the result back.

[sergiitk]

In case someone needs it, my temporary solution is to deploy workers with a provisioner.

locals {
  worker_filepath = "${abspath(path.root)}/../../workers/dist/my-worker.js"
}

resource "cloudflare_worker_script" "my_worker" {
  # To preserve environment variables configuration, ensure worker scripts are never destroyed
  lifecycle {
    prevent_destroy = true
    # Ignore changes due to https://github.com/terraform-providers/terraform-provider-cloudflare/issues/615
    ignore_changes = [
      content,
    ]
  }

  name = "my-worker"
  content = file(local.worker_filepath)
}

# This is a temporary workaround to manually deploy worker script content due to
# https://github.com/terraform-providers/terraform-provider-cloudflare/issues/615
resource "null_resource" "my_worker_content" {
  triggers = {
    # Trigger when worker script content has changed.
    my_worker_hash = filemd5(local.worker_filepath)
  }

  provisioner "local-exec" {
    # Deploy the worker by manually uploading worker content to Cloudflare API via curl.
    command = "${abspath(path.root)}/../upload_worker.sh ${cloudflare_worker_script.my_worker.name}"
  }
}

Upload script:

#!/bin/sh

set -eo pipefail

WORKER_NAME="${1?'The name of the worker is required'}"
BASEDIR=$(dirname "$0")

curl_out=`curl -sS --compressed --request PUT \
  --header "Authorization: Bearer $CLOUDFLARE_API_TOKEN" \
  --header "Content-Type: application/javascript" \
  --data-binary "@${BASEDIR}/../workers/dist/my-worker.js" \
  "https://api.cloudflare.com/client/v4/accounts/$CLOUDFLARE_ACCOUNT_ID/workers/scripts/$WORKER_NAME"`
curl_exit_code=$?

if [[ $curl_exit_code -ne 0 ]]; then
  exit $curl_exit_code
fi

# Avoid shell built-in echo as some of them don't preserve \n chars and break json output
printf "%s" "$curl_out"
printf "%s" "$curl_out" | grep -v '"script":' | grep -o '"success": true'

[jacobbednarz]

#465 is taking a pass at implementing this in cloudflare/cloudflare-go. Once that lands, we can pull it downstream into the resource.