Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: create temporary files #17008

Open
natefaerber opened this issue Dec 29, 2017 · 11 comments
Open

Feature Request: create temporary files #17008

natefaerber opened this issue Dec 29, 2017 · 11 comments

Comments

@natefaerber
Copy link

I recently came upon hashicorp/terraform-provider-consul#5 that led me to imagine a new function that "wraps a text block in a file". This basically amounts to a tempfile function where terraform manages ephemeral temporary files for use in a single run. The idea is that I could pass a variable or attribute to a resource argument that normally expects a filename using this tempfile function.

The example I provided in hashicorp/terraform-provider-consul#5 is:

provider "consul" {
  address = "${aws_instance.master.public_ip}:8080"
  datacenter = "${coalesce(var.cluster_name, var.subnet_id)}"
  scheme = "https"

  # Use of templates is a nasty hack to force dependency ordering
  ca_file = "${tempfile(tls_self_signed_cert.consul_ca.cert_pem)}"
  cert_file = "${tempfile(tls_locally_signed_cert.consul_node.cert_pem)}"
  key_file = "${tempfile(tls_private_key.consul_node.cert_pem)}"
}

tempfile returns the path of the temporary file that Terraform will clean out at the end of its run.

This avoids the use of local_file to manage the file locally and puts the burden of managing temporary files on Terraform instead of the user. Just a thought but I wanted to follow-up my comment with this feature request to gauge the likelihood of this happening.

Regards.

@apparentlymart
Copy link
Contributor

Hi @natefaerber! Thanks for this suggestion.

We've generally tried to avoid the need for temporary files by passing direct values rather than file paths where possible. I assume that isn't true here due to limitations of the underlying Consul client library, which perhaps expects to read keys from files rather than in-memory strings. (I didn't actually check, so this assumption may be wrong.)

In an ideal world I'd prefer to add to the consul provider new settings ca_cert_pem, cert_pem and key_pem that just take the strings directly, taking external files out of the equation altogether. This is more consistent with our usual approach and avoids the risk of leaking potentially-sensitive data in the temporary directory in cases where cleanup is not possible for some reason.

What do you think? Is there another reason why passing this data through temporary files is desirable, or would passing them directly in memory meet your needs here?

@natefaerber
Copy link
Author

Only reason I suggested temporary files is because I looked at the code and Condul client can't accept a string so I was trying to avoid changes to two projects. Also, I made an assumption that this wasn't the only case where an underlying client library would force a provider to pass a file rather than in-memory strings.

By the way, I could totally do the work for adding these settings. I have many outstanding pull-requests on the consul provider. How do I get someone with authority to review them? Also, I made many comments to the issues in that project that could allow closing the tickets. Sorry to hijack the discussion.

@BernhardBln
Copy link

I'd need this for the archive_file data source - when zipping a js file for an AWS lambda.

I'd love if it would be possible to use that without output_path, and then just refer to data.archive_file.lambdazipped.zippedcontent, but that's not possible...

Now the zip file ends up in my source folder :-/

@whyman
Copy link

whyman commented Aug 3, 2018

👍

@apparentlymart
Copy link
Contributor

Hi @natefaerber! Sorry for the silence here, and on your issues in the Consul provider.

The Consul provider is now maintained by the Consul team themselves, to allow it to better keep up with new Consul features. Looking over there I see that in the mean time your PRs have been merged; I expect the delay you saw was prior to the transition of responsibility to the Consul team, where the Consul provider was unfortunately competing for attention with many of the other providers that our provider development team looks after.

As for the new settings we were talking about: I assume it would first need a change to the Consul client library, but at least coordinating such a thing should be easier with the same team looking after both codebases. That'd be a couple changes over in the Consul repository and the Consul provider repository rather than here, I assume.

As for temporary files in general: I think a function isn't the right way to implement that because we'd have no way to actually clean it up afterwards (functions don't have a "lifecycle" to implement a cleanup step in) but we'll use this issue as a marker for the general use-case of creating temporary files, or perhaps alternatively for creating temporary directories that you could then write files into in other ways (provisioners, etc). This will probably require a new concept in Terraform altogether, of something that lives only for a single run, which might also generalize to include the use-case described in #8367 but not sure yet about that.

We won't be able to look into this right now due to attention being elsewhere, but I'll mark it as needing further design effort and we'll return to it when we're able.

(In the mean time, if you want to "vote" for this to influence priority, please do so by leaving a 👍 reaction on the original comment rather than leaving new comments, since we can report on which issues have the most thumbs-up reactions while comments just create noise for anyone following the issue.)

@apparentlymart
Copy link
Contributor

Ahh, I recently opened #21308 and I knew when I was writing it that there was another issue for this out there somewhere but I couldn't find it at the time.

For the moment I'm just going to connect the two with a link, because there are lots of votes on this one but there's a recent design proposal on the other one. We're not planning to take any immediate action on this due to priorities being elsewhere, but once we do I expect we'll close one of these issues to consolidate. For now, my issue #21308 can be thought of as a specific technical proposal to meet the use-case described in this issue.

@apparentlymart apparentlymart changed the title Feature Request: Add tempfile function Feature Request: create temporary files Jun 5, 2019
@jemc
Copy link

jemc commented Aug 23, 2019

My use case is a bit different from the OP, so it may add some justification.

Because Terraform's Kubernetes provider doesn't support arbitrary manifests, the most common approach in the community seems to be hacking it in with a local-exec provider, like this:

locals {
  my_kubernetes_yaml = templatefile("my_kubernetes.yaml", {
    # ...
  })
}

resource "null_resource" "kubectl_apply_my_kubernetes_yaml" {
  triggers = {
    my_kubernetes_yaml = "${local.my_kubernetes_yaml}"
  }

  provisioner "local-exec" {
    command = "kubectl apply -f - <<'EOF'\n${local.my_kubernetes_yaml}\nEOF"
  }
}

The issue is that for a certain application (that expects a very large config file in a configmap manifest), the string local.my_kubernetes_yaml ends up huge (141301 bytes), which results in a shell error that the argument list to the shell is too long:

fork/exec /bin/sh: argument list too long.

Because the large config file in question needs to be one contiguous file, there is no way to split it up among multiple invocations. In such a situation, it seems I am forced into using a temporary file because I have no way to get the manifest into the kubectl apply invocation.

@hterik
Copy link

hterik commented Sep 15, 2021

Similar use case here with https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_share_file. It only allows local file via the source argument. But i need the content of the file to be populated via other terraform variables through templatefile.

@apparentlymart
Copy link
Contributor

Hi all! Thanks for adding these additional use-cases.

I want to reinforce my earlier comment that whereever possible we prefer to pass data between resources entirely in memory and not create temporary files on disk. If the current design of a provider prevents that from happening then the first step would be to work with the relevant provider team to understand if it would be possible to change the provider's design to better suit that ideal.

If there are specific technical constraints that prevent a provider from accepting data in memory -- either as a replacement for reading files from disk, or a design where both are allowed using different arguments as we see for aws_s3_bucket_object, then that could be a good motivating use case for creating temporary files, but we'd still always consider that to be a last resort after exhausting the possibility of passing data in memory in the usual way.

So far it's typically been possible to change providers to accept data directly via arguments and so consequently we've not prioritized this feature. It's likely that we will investigate this topic further eventually, but other work is taking priority for the moment.


For Kubernetes in particular I think the process I described above is currently playing out. Recent versions of the hashicorp/kubernetes provider have the kubernetes_manifest resource type which takes a raw manifest directly as a data structure inside Terraform, without any provider-imposed assumptions about what shape it will have.

My understanding is that this feature is currently in beta at the time I'm writing this, so I understand that the problem is not fully solved, but the work is in progress and if you're able to give it a try I expect the Kubernetes provider team would like to hear feedback to help it progress to a stable release.

@hterik
Copy link

hterik commented Sep 16, 2021

See #21308 (comment) for a workaround using local_file, which seems to work fine.

@brian-fogg-candid-health
Copy link

brian-fogg-candid-health commented Aug 11, 2022

I would also love this feature. Using google_storage_bucket_object resource with a content string, the resource considers this sensitive, so I can't review changes in the plan. Can hack around this with a local_file and pass to the source attribute, but then this will show up in every run and pollute the plan output and local directory. tempfile() Would be a great feature ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants