[FR] when building a brokerpak, package files needed by Terraform code

[mogul]

Is your feature request related to a problem? Please describe.
There's a local solr-crd directory containing a Helm chart that I want my Terraform to use. Here's the Terraform in question:

resource "helm_release" "solrcloud" {
  name = local.cloud_name
  chart = "./solr-crd"
  namespace = data.kubernetes_namespace.namespace.id
  cleanup_on_fail = true
  atomic = true
  wait = true
  timeout = 600
}

However, when the CSB builds the brokerpak, the referenced directory is not included. As a result, when I try to use the broker, I see:

2020/10/13 19:55:10 Last operation for "ex3065862978-" was "failed": Error: path "./solr-crd" not found  on brokertemplate/definition.tf line 62, in resource "helm_release" "solrcloud":  62: resource "helm_release" "solrcloud" { exit status 1

Describe the solution you'd like

The manifest.yml should include fields for specifying additional files to be included alongside the Terraform providers and service definition YAML.

(Alternatively, if Terraform provides a way, the broker could infer the needed files by inspecting the provided HCL.)

Describe alternatives you've considered

I can work around this particular problem by having the helm_release resource refer to a .tgz of the Helm chart hosted on GitHub.

However, this won't be the case for other Terraform code, eg when using a local file as a template; see below.

Additional Context

Priority

High - It's impossible to fully take advantage of Terraform and certain services cannot be brokered without this feature. For example, I'm about to start brokering AWS EKS. In my Terraform code, I have:

resource "helm_release" "prometheus" {
  count   = local.env == "default" ? 1 : 0
  name    = "prometheus"
  chart   = "stable/prometheus-operator"
  version = "8.13.11"
  namespace = "monitoring"

  values    = [
    templatefile("./charts/prometheus/values.yaml", { grafana_pwd = var.GRAFANA_PWD, base_domain = local.base_domain })
  ]
  provisioner "local-exec" {
    command = "helm --kubeconfig kubeconfig_${module.eks.cluster_id} test -n ${self.namespace} ${self.name}"
  }

  depends_on = [
    module.eks.cluster_id
  ]
}

Here we see that using templatefile(), a workhorse in lots of Terraform deployments, will not be possible with CSB.

Priority Context

It prevents brokering any but the most trivial Terraform deployments.

Platform

N/A

Applicable Services

Anything that wants to use a resource other than HCL code.

[omerbensaadon]

Hey Bret, the team is currently focused on releasing CSB for AWS and a lot of our attention is going to nurturing Beta engagements.

Unfortunately, that means this isn't going to jump to the top of the queue anytime soon. The team recognizes that this is an important feature for contributors.

Any chance you would consider a contribution here? We will get to this eventually, just maybe not in the timeframe you need given the priority context...

[mogul]

I said:

It's impossible to fully take advantage of Terraform and certain services cannot be brokered without this feature.

Researching further, I think I can work around the specific lack of templatefile() with a heredoc template in the case above.

Any chance you would consider a contribution here?

I'm not much of a Go programmer, but if I end up totally stuck I'll look into it.

[omerbensaadon]

I'm not much of a Go programmer, but if I end up totally stuck I'll look into it.

No better time than the present to learn a new programming language 😂

[dmachard]

Hi all,
I just pushed a pull request to add support for local files #231 . It's my first contribution, so perhaps I am not compliant with your best practices.
Denis

[pivotal-marcela-campo]

Hi @mogul @dmachard, we were looking at this issue and PR supplied and got to the conclusion that this limitation is because up to now the broker merges all tf files in one big file to store it in the database. If we were to just grab whatever is in the brokerpak directory structure, this problem would go away without the need of specifying additional files.

We have done work to update the HCL that is stored in the database on each execution. This is currently behind a feature flag. Our thinking is once we remove that feature flag there would be no need to store HCL in the database anymore. We could then just simplify the broker and unpak the brokerpak as provided into a temp directory, which would include any extra files provided.

Does that make sense? Thoughts?

[mogul]

A little context: When you use the Terraform Kubernetes provider (as we do) Hashicorp strongly recommends that you create the IaaS resources it refers to in a separate module, and if possible, a separate apply operation. We use this practice when iterating on the Terraform code used by our brokerpak... The code is divided into module directories that we can iterate on and apply either independently or as sub-modules for a single apply operation.

However, because the brokerpak mashes all the .tf files referenced in the service manifest together, the module structure is not preserved when the brokerpak runs apply, and the CSB does not provide a facility to run successive applies on a series of targets.

Since that's the case, we are extremely careful to specify only a subset of the HCL files across our module directories for inclusion, and we also include a couple extra "glue" files that aren't in them so that the files can all apply together. We have been lucky that we are not running into the provider interpolation issues described in the Hashicorp documentation, but we have been nervous about this for a while.

This has been a long way to say: If you make the change described (eg just name a directory), that will certainly solve the problem described in this issue. However, it will mean that we need to move the bulk of our HCL code out into modules maintained elsewhere. It's something we've thought about doing that may be overdue, but it does stray from the idea that brokerpaks are self-contained. (I guess we use modules by others now in any case; we're just not iterating there.)

[mogul]

However, because the brokerpak mashes all the .tf files referenced in the service manifest together, the module structure is not preserved when the brokerpak runs apply, and the CSB does not provide a facility to run successive applies on a series of targets.

Since that's the case, we are extremely careful to specify only a subset of the HCL files across our module directories for inclusion, and we also include a couple extra "glue" files that aren't in them so that the files can all apply together. We have been lucky that we are not running into the provider interpolation issues described in the Hashicorp documentation, but we have been nervous about this for a while.

We do test our Terraform in isolation with all the files mashed together as they will be when the brokerpak includes them... Note the symlinking instructions here.

[mogul]

Actually thinking about this a little more: Maybe just specifying a directory would be fine if it includes all the subdirectories beneath it... That way our module structure would be preserved and we'd get the increased safety we're after, and we can drop all the complexity we have currently to simulate the existing behavior!

So, if this path simplifies the CSB brokerpak handling and keeps the on-disk hydrated workspace closer to what we intend, I say go for it.

[mogul]

If we were to just grab whatever is in the brokerpak directory structure, this problem would go away without the need of specifying additional files.

A request if you do this: Give us a way to reliably ignore certain files akin to .gitignore or .cfignore. That way things like .terraform subdirectories or .tfvar files with credentials used during iteration and testing won't accidentally get included in the built brokerpak.

[pivotal-marcela-campo]

Thanks for the feedback @mogul. This would be our preferred path once we are able to remove the feature flag.

We are not sure when we will be able to confidently remove it as our testing is mainly on updating from the previous version of the brokerpak, but it is definitely in our roadmap as part of the terraform upgrades track as well.

[mogul]

We've gotten to the point where it is pretty unwieldy to develop without module boundaries being preserved in the brokerpak, so I'm now eagerly awaiting this change!

[mogul]

Am I right in thinking that this PR implements this functionality?

[pivotal-marcela-campo]

Oh, I am sorry @mogul we gave you false hopes. That PR is to allow our test framework to create an environment with all it needs. In this case we do have some binaries that we include in our brokerpaks that are referenced from the tf templates that use them as local-exec provisioners.
We are still working on the terraform upgrade functionality, after that we will reevaluate not storing the workspace in the database and that would enable us to just accept any files. Cannot yet establish a timeline of when we will be able to tackle that, but will definitely keep this feature request updated.

[mogul]

I'm about to pick up work on our most complex brokerpak again and I'm wondering if preserving module boundaries/directory layout is anywhere near the top of your backlog...? It would make this work much easier.

[pivotal-marcela-campo]

Hi @mogul, unfortunately it is not. At the moment we don't have any estimate on when we will be able to tackle this request or at least pave the way to it.

Thanks
Marcela

[nouseforaname]

FWIW:

if you need arbitratry text files, you can just create them within TF files. E.g. your requirement for templatefile() can be replaced by using:

❯ cat template.tf
data "template_file" "init" {
  template =  <<EOF
test_data_template_var=$${template_var}
test_data_global_var=${var.global}

EOF
  vars = {
    template_var = "a_value"
  }
}
output "example" {
  value = data.template_file.init.rendered
}

variable "global" {
  default = "global"
}

❯ terraform apply
data.template_file.init: Reading...
data.template_file.init: Read complete after 0s [id=6751cdb0621aff3f26ef044138480119eecf1ee91c9078728e33f7d3e606cfa8]

Changes to Outputs:
  + example = <<-EOT
        test_data_template_var=a_value
        test_data_global_var=global

    EOT

You can apply this plan to save these new output values to the Terraform state, without changing any real infrastructure.

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


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

Outputs:

example = <<EOT
test_data_template_var=a_value
test_data_global_var=global