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

lang/funcs: Filesystem functions hint about dynamically-generated files #25361

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

apparentlymart
Copy link
Contributor

The functions that interact with the filesystem are, by design, restricted to reading files that are distributed as a static part of the configuration, and cannot be used to interact with files that are generated dynamically by resources in the configuration.

However, new users have often yet developed a correct mental model of how Terraform execution works and are confused by the terse error messages these functions return. As an interim step to help some of those users, this just adds some more commentary to the error message which gives a vague, generic directive to look to attributes of the resource that is generating the file, which should (if it's designed well) export attributes that allow the resulting file to be used effectively with common patterns, such as checksums of the content of the generated file.

This error message is coming from a helper function that is used from file, templatefile, and all of the functions that can produce hash values from the contents of a file, so all of them will now return this extended error message if they encounter a "file not found" condition.

The new error message is vague and not particularly attractive due to the limitations of the context where it's being returned from, but I'm accepting that here in the interest of keeping this change simple, so we can give a hint about a case that seems to frequently generate new-user questions.

This is related to #25162, but since that issue is mainly just a placeholder for "something should be done about this" we'll need to see if this issue is a sufficient "something" by observing if the volume of questions about this changes some time after we release this change.

Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

Just needs a test fixture update, but otherwise LGTM

The functions that interact with the filesystem are, by design, restricted
to reading files that are distributed as a static part of the
configuration, and cannot be used to interact with files that are
generated dynamically by resources in the configuration.

However, new users have often yet developed a correct mental model of how
Terraform execution works and are confused by the terse error messages
these functions return. As an interim step to help some of those users,
this just adds some more commentary to the error message which gives a
vague, generic directive to look to attributes of the resource that is
generating the file, which should (if it's designed well) export
attributes that allow the resulting file to be used effectively with
common patterns, such as checksums of the content of the generated file.

The error message here is not particularly attractive due to the
limitations of the context where it's being returned from, but I'm
accepting that here in the interest of keeping this change simple, so we
can give a hint about a case that seems to frequently generate new-user
questions. We may iterate further on the presentation of this message
later.
@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #25361 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
lang/funcs/filesystem.go 88.17% <100.00%> (+0.06%) ⬆️
dag/marshal.go 53.33% <0.00%> (-1.12%) ⬇️
terraform/evaluate.go 53.60% <0.00%> (+0.45%) ⬆️
backend/remote/backend_common.go 55.25% <0.00%> (+0.67%) ⬆️
terraform/node_resource_plan.go 93.44% <0.00%> (+1.63%) ⬆️

@apparentlymart
Copy link
Contributor Author

Oh, whoops! I guess I missed that file when I was committing. Fixed now, and merging.

@apparentlymart apparentlymart merged commit 8f77bd3 into master Jun 24, 2020
@apparentlymart apparentlymart deleted the f-file-func-errs branch June 24, 2020 16:02
@ghost
Copy link

ghost commented Jul 25, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants