Terraform-aws-eks: 12.0.0 broke userdata_template_file in worker_groups

[dpiddockcmp]

I have issues

I'm submitting a...

• [x] bug report
• [ ] feature request
• [ ] support request - read the FAQ first!
• [ ] kudos, thank you, warm fuzzy

What is the current behavior?

worker_groups key userdata_template_file used to contain the content of a template file. #854 accidentally changed this to being a filename

As reported here: https://github.com/terraform-aws-modules/terraform-aws-eks/commit/b183b972539fa9d39dc55c453d68a234fdb683b1#r39182495

If this is a bug, how to reproduce? Please include a code sample if relevant.

Try to set worker_groups["userdata_template_file"] to the contents of a userdata file. It'll explode trying to find a file of that name

What's the expected behavior?

Input passed as raw data to be used as the userdata template.

Are you able to fix this problem and submit a PR? Link here if you have already.

Later

Environment details

• Affected module version: 12.0.0
• OS: All
• Terraform version: 0.12.24

Any other relevant info

My bad 😞

[mbarrien]

Copying parts of the contents of the issue I had almost written up:

What is the current behavior?

Error: Error in function call

  on .terraform/modules/eks-cluster.cluster/terraform-aws-eks-12.0.0/local.tf line 194, in locals:
 194:   launch_template_userdata = [for worker in var.worker_groups_launch_template : templatefile(
...
Call to function "templatefile" failed: failed to read Content-Type:
multipart/mixed; boundary="MIMEBOUNDARY"
MIME-Version: 1.0

where the Content-Type: ... was the contents of our template file

What's the expected behavior?

The user data is passed in and handled as a template already

Are you able to fix this problem and submit a PR? Link here if you have already.

In theory this is fixable in one of 4 ways:
1) Revert #854 and parts of #865, say we forever will use template_file data source.
2) Keep as is, say that you've change the type of userdata_template_file to be the filename instead of the contents of the template. This breaks ALL use cases where the user is generating a template file dynamically, since according to Terraform docs at https://www.terraform.io/docs/configuration/functions/templatefile.html: This function can be used only with files that already exist on disk at the beginning of a Terraform run.
3) Reintroduce template_file for this use case but just when the user overrides with userdata_template_file. (This is kind of the worst of both worlds, using both templatefile() function and template_file data source.)
4) Add a new field userdata_file/change the semantics of userdata_template_file to require that the user pre-render the file with their own template substitutions outside the module (this would work for my org, since we actually use a fixed userdata that doesn't need any of the template substitutions here.)

It's unclear which direction to go, although 1 or a combination of 2 and 4 would probably be sufficient.

Any other relevant info

Given that 12.0.0 also fixes the incompatibility with Terraform AWS Provider 2.61.0 in #868, this is pretty urgent to fix since now we can upgrade neither the provider nor this module in our production cluster until this is fixed.

[dpiddockcmp]

I don't see a nice solution to this.

Terraform doesn't have a straight up template() function where you pass it a template and a vars block. I guess they assume you either want to pull stuff from a file or can use here docs. Or are happy with the template provider.

Either we revert #854 and go back to the obnoxious behaviour of the template_file data source or change how some use cases worked 😞

[barryib]

Oups. I don't see a nice solution either.

Keep as is, say that you've change the type of userdata_template_file to be the filename instead of the contents of the template. This breaks ALL use cases where the user is generating a template file dynamically, since according to Terraform docs at https://www.terraform.io/docs/configuration/functions/templatefile.html: This function can be used only with files that already exist on disk at the beginning of a Terraform run.

FWIW, after some tests, I figured out that for templatefile function the file should only exist, but if those variables come from outputs of another resource, then Terraform must wait for that resource to apply before reading the file. Terraform magic again 🤷‍♂️

Here is my test:

In main.tf

locals {
  template = templatefile(local_file.template.filename, {
    a = "test",
  })
}

resource "local_file" "template" {
  content  = "foo!"
  filename = "${path.module}/template.tpl"
}

output "a" {
  value = local.template
}
````

### In `template.tpl`
```hcl
# Fake template

Results

 ~ terraform apply
local_file.template: Refreshing state... [id=4bf3e335199107182c6f7638efaad377acc7f452]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # local_file.template will be created
  + resource "local_file" "template" {
      + content              = "foo!"
      + directory_permission = "0777"
      + file_permission      = "0777"
      + filename             = "./template.tpl"
      + id                   = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

local_file.template: Creating...
local_file.template: Creation complete after 0s [id=4bf3e335199107182c6f7638efaad377acc7f452]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Outputs:

a = foo!

That means, we can probably follow the option 2 and write a small doc for users who want to dynamically generate their template. They must create an empty file in their module codebase before the running terraform.

@dpiddockcmp @mbarrien can you confirm these tests ?

[dpiddockcmp]

That's really not a safe path. People seem to struggle to read instructions.

It will cause updates each time the plan is run on a clean environment, like a CI system. So for those users wanting to pass in their own template the situation will be even worse.

Updating the comment in locals.tf to say that userdata_template_file now means a file path is a bit rude and stops dynamic generation of the template.

Unfortunately I think we'll have to revert my changes. #883

Maybe I'll file a wishlist item against Terraform. See if they can add a generic template() function.

[mbarrien]

2 by itself won't work for us; we dynamically generate the contents, and can't point to a single file on disk. (We reuse this userdata generating mechanism for use cases outside of EKS.)

If we add a #4, by creating a new variable called userdata_raw_contents alongside the now-repurposed userdata_template_file we might be able to get away with it. That said, I looked closer and it does appear that my org's current method is relying on this double-templating within the context of EKS; either we need the full revert, or we'd need to be able to pre-calculate the variables that the terraform-aws-eks module would provide to the template so that we can apply that externally ourselves.

[barryib]

@dpiddockcmp agree that it's an ugly hack.

Maybe I'll file a wishlist item against Terraform. See if they can add a generic template() function.

To me this is the way to go. I just opened a PR https://github.com/hashicorp/terraform/pull/24978 to add the template function. Let's see where it'll go.

I'm not happy with reverting https://github.com/terraform-aws-modules/terraform-aws-eks/pull/854. It adds consistency during plan/apply phases and avoid lot of known after apply issues. Is there any way to improve the "obnoxious random updates" without removing the template_file data source ?

[dpiddockcmp]

It looks like the coalescelist trick can solve the issue of updating launch templates when the eks_cluster gets modified. I've pushed an update to #883. Appears to solve my original issue and hopefully makes enough other people happy. But we still have the template provider involved

[dpiddockcmp]

The revert of this change has now been released in v12.1.0