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

provider/template: Add a 'dir' resource to template entire directories #13652

Merged
merged 4 commits into from
Apr 25, 2017
Merged

provider/template: Add a 'dir' resource to template entire directories #13652

merged 4 commits into from
Apr 25, 2017

Conversation

Quentin-M
Copy link
Contributor

When TerraForm is used to configure and deploy infrastructure
applications that require dozens templated files, such as Kubernetes, it
becomes extremely burdensome to template them individually: each of them
requires a data source block as well as an upload/export (file
provisioner, AWS S3, ...).

Instead, this commit introduces a mean to template an entire folder of
files (recursively), that can then be treated as a whole by any provider
or provisioner that support directory inputs (such as the
file provisioner, the archive provider, ...).

This does not intend to make TerraForm a full-fledged templating system
as the templating grammar and capabilities are left unchanged. This only
aims at improving the user-experience of the existing templating
provider by significantly reducing the overhead when several files are
to be generated - without forcing the users to rely on external tools
when these templates stay simple and that their generation in TerraForm
is justified.

/cc @sym3tri @philips @apparentlymart

@apparentlymart
Copy link
Contributor

Hi @Quentin-M! Thanks for submitting this.

This is an interesting idea. Again I don't have time today to dig in fully but I'll mark myself as reviewer on it to remind me to look soon.

One bit of initial feedback is on the name itself. Elsewhere in Terraform we've tended to use the term "dir" to talk about directories, vs. "folder". So perhaps we could call this template_dir instead to align terminology.

I assume you based this on the archive_file data source since I see some remnants in there from my quick skim. Note that the functions should conventionally be called resource... rather than dataSource... here because that's what this is, and I think there are some remnant functions left over here that aren't used anymore.

@Quentin-M Quentin-M changed the title provider/template: Add a 'folder' resource to template entire folders provider/template: Add a 'dir' resource to template entire directories Apr 14, 2017
@Quentin-M
Copy link
Contributor Author

Hey @apparentlymart,

Thank you for the extremely prompt answer!

  • I just replaced every occurrences of folder to dir (or directory) in the code, documentation, and provider name. Does it look better?
  • Regarding to the dataSource..., great catch! I started writing this as a data source and apparently forgot to rename these functions.
  • I imagine that the reason you think this is based off archive_file is that you encountered the tarDir function? This is used to easily and deterministically generate the hash of a given folder. The hashes of both the input and destination folders are generated to determine whether we need to re-template anything. Both hashes are then used as the ID of the resource.

@Quentin-M
Copy link
Contributor Author

I'd like to follow-up on this PR. Would this provider be of interest for the HashiCorp project and its community?

@apparentlymart
Copy link
Contributor

Hi again @Quentin-M. Sorry for the delay here... I'm taking a deeper look at this now.

Quentin-M and others added 4 commits April 25, 2017 10:48
When TerraForm is used to configure and deploy infrastructure
applications that require dozens templated files, such as Kubernetes, it
becomes extremely burdensome to template them individually: each of them
requires a data source block as well as an upload/export (file
provisioner, AWS S3, ...).

Instead, this commit introduces a mean to template an entire folder of
files (recursively), that can then be treated as a whole by any provider
or provisioner that support directory inputs (such as the
file provisioner, the archive provider, ...).

This does not intend to make TerraForm a full-fledged templating system
as the templating grammar and capabilities are left unchanged. This only
aims at improving the user-experience of the existing templating
provider by significantly reducing the overhead when several files are
to be generated - without forcing the users to rely on external tools
when these templates stay simple and that their generation in TerraForm
is justified.
When an error is passed, the FileInfo can be nil, which was previously
causing a crash on trying to evaluate f.IsDir(). By checking for an error
first we avoid this crash.
Previously we were letting it get implicitly created as part of making
the structure for copying in each file, but that isn't sufficient if the
source directory is empty.

By explicitly creating the directory first we ensure that it will complete
successfully even in the case of an empty directory.
@Quentin-M
Copy link
Contributor Author

Quentin-M commented Apr 25, 2017

@apparentlymart Oh wow, thank you so much for your changes! I'd be happy to help in any way I can!

@apparentlymart apparentlymart merged commit 2871641 into hashicorp:master Apr 25, 2017
@Quentin-M Quentin-M deleted the template_folder branch April 25, 2017 18:15
@apparentlymart
Copy link
Contributor

Hi @Quentin-M! I just took care of a couple edge cases I found during testing. Seemed more efficient to just tweak in place rather than having another round-trip of change/review!

I've just merged it with the little tweaks you saw and some elaborated documentation. Thanks again!

@Quentin-M
Copy link
Contributor Author

Thanks again for your prompt help on both providers, and for your hard work on TerraForm itself of course! Truly appreciate it! Slick open-source experience. 💯

I wish you to have a great week.

@ghost
Copy link

ghost commented Apr 13, 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 Apr 13, 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.

3 participants