Skip to content

Use the TF_DATA_DIR env var for temporary files #36863

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lattwood
Copy link

@lattwood lattwood commented Apr 8, 2025

This (effectively) prevents Terraform from crossing filesystem boundaries when installing providers and cloudplugins.

I am proposing this change, because of a recent experience with running a large number of terraform init statements (and I mean a lot). We do our checkouts on ephemeral storage for performance reasons, but we were shocked to discover that Terraform was still writing to the boot drive after making that change. This uses TF_DATA_DIR for the temporary files if it's set. If it isn't set, it will evaluate as "", and here's what happens, right from the godoc-

func CreateTemp(dir, pattern string) (*File, error)
... (omitted for brevity)
If dir is the empty string, CreateTemp uses the default directory for temporary files, as returned by TempDir.

and then for TempDir-

TempDir returns the default directory to use for temporary files.
On Unix systems, it returns $TMPDIR if non-empty, else /tmp. On Windows, it uses GetTempPath, returning the first non-empty value from %TMP%, %TEMP%, %USERPROFILE%, or the Windows directory. On Plan 9, it returns /tmp.
The directory is neither guaranteed to exist nor have accessible permissions.

(I guess this means there's a bug, too, since I didn't see any checks for if it exists or has accessible permissions)

Target Release

1.13.x

CHANGELOG entry

Ehhh, not sure what to go for with this.

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

This (effectively) prevents Terraform from crossing filesystem boundaries when
installing providers and cloudplugins.
@lattwood lattwood requested a review from a team as a code owner April 8, 2025 16:00
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @lattwood
Have you considered setting $TMPDIR or equivalent?

Is there a reason that wouldn't/didn't work for you?

@lattwood
Copy link
Author

lattwood commented Apr 8, 2025

@radeksimko yes, we have, and that's the plan to work around this.

What nobody would be expecting is Terraform continuing to beat the expletive out of /tmp on root.

I was surprised and caught offguard by this. I wouldn't be surprised if others end up having the same/similar experience, so I decided the least I could do is fire off a PR to make the behaviour adhere to the principle of least surprise.

Plus, I had to read the source code to find out the behaviour, I'd like to save others from having to do the same.

@radeksimko
Copy link
Member

I appreciate the frustration but I'm afraid I don't understand how the mentioned surprising behaviour is addressed by introducing a new environment variable.

It sounds like the surprise comes more from init having the (undocumented) side effect that plugins (archives) are written to a location on disk before unpacking rather than the location specifically being the default OS temp directory. Please do correct me if I'm wrong.

Would you be open to instead documenting $TMPDIR as an existing variable in the appropriate pages of documentation and/or linking to os.TempDir from there? For example, we could mention it at https://developer.hashicorp.com/terraform/cli/commands/init

@crw crw added the enhancement label Apr 8, 2025
@lattwood
Copy link
Author

lattwood commented Apr 8, 2025

oh, I didn't add a new environment variable-

https://developer.hashicorp.com/terraform/cli/config/environment-variables#tf_data_dir

I took advantage of an existing one that's used for relocating .terraform but I could change the PR to be documentation only.

@radeksimko
Copy link
Member

I see, I did not realise this was an existing variable. Thanks for clarifying that.

One side effect of moving files of "temporary nature" outside of $TMPDIR is that the OS will no longer be responsible for cleaning up files in that location - that applies to both finished and unfinished files. Some Linux distributions perform cleanup on boot, some do it periodically but there is a clear expectation it will happen. We could technically introduce that cleanup logic into Terraform of course but this PR isn't adding it currently.

Going back to the original motivation though

We do our checkouts on ephemeral storage for performance reasons, but we were shocked to discover that Terraform was still writing to the boot drive after making that change.

Maybe I'm missing something here - is there a reason why the OS' $TMPDIR is located on your boot drive, rather than the ephemeral drive? $TMPDIR isn't a concept unique to Terraform, it is used by many other programs, presumably including the OS' own packaging system and you will face the same problem when running these other programs - perhaps at a different scale, but the nature would be the same.

If your intention is to have immutable filesystem at apply time, you could run init as part of provisioning your filesystem/OS, or use the terraform providers mirror command to pre-install providers to a dedicated location. Would that help?

Regardless of where the archives get stored, I still agree that we could clarify in the docs that the archives do get written somewhere as part of init and where that is.

@radeksimko
Copy link
Member

Just one extra thought - while we do currently rely entirely on the OS in terms of the cleanup we could remove the archives proactively from $TMPDIR right after they are unpacked - I think that would be a relatively straight forward improvement that could reduce the amount of data stored there at any time without introducing any side effects.

Would that help?

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

Successfully merging this pull request may close these issues.

None yet

3 participants