-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
providers: add template provider #1778
Conversation
// Reload every time in case something has changed. | ||
// This should be cheap, and cache invalidation is hard. | ||
return false, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just omit Exists
, which has the same effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nevermind - didn't see that Read
was a noop. This makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @josharian - working on #1866 and coming back around to this. I think we need to rework the lifecycle a bit here. Will follow up with a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Sorry about that. Let me know if you'd like me to dive in. (Sounds from your comment like you have it under control.)
Thanks! I'll leave this here for a few days so that others who care about this (@apparentlymart?) can weigh in, can assess whether this will address their needs, and can object to the names I used (template_file, filename, vars, etc.). Meanwhile, since I didn't hear any showstopping concerns from you, I'll write some docs. |
This is a great start! 😀 Inline'd a couple of nits. Think you could take a stab at some acceptance tests? |
Absolutely. |
This looks great to me. Thanks for taking this on, @josharian! This would be very useful to us if it got merged. (Our first use-case will be to templatize the user_data for our EC2 instances from a separate file, and it looks like this implementation will work just fine for that.) |
Do directory expansion on filenames. Add basic acceptance tests. Code coverage is 72.5%. Uncovered code is uninteresting and/or impossible error cases. Note that this required adding a knob to helper/resource.TestStep to allow transient resources.
While we're here, fix a broken link.
0db2c2d
to
2da7c9a
Compare
I've addressed the review comments, added tests, and written some docs. There's a CI failure, but it looks unrelated (consul timeout, vet failure in existing code). Ready for round two. |
This is using Terraform's own string interpolation macros, isn't it? To me the idea of templates would really make most sense if templates would expose a reacher language, one with loops and logic, as means of going just a bit beyond what HCL currently offers. |
It's great to be able to do the kind of file interpolation, which is what subject of #215 state and this covers it very well. As for #642, I wished for a more extended kind of language at the time of posting. I do suppose that #642 can be achieved with an external plugin, if the core codebase isn't going to provide much beyond interpolation macros in files. |
This looks great @josharian - merging so this will make it into 0.5 release this week. 🎉 @errordeveloper - yep I think we'll definitely be talking about support for richer templating languages down the road, but this represents a great first step |
providers: add template provider
Just noticed that this didn't make it into the changelog. Want to do the honors, @phinze? Thanks for all the help, and don't worry, the barrage of PRs from me should slow soon. :) |
@josharian thanks a million for getting this of the ground! |
@josharian ^^ done 👍 |
This is great work. It would be extremely useful too if we can render resource's attributes into provisioners scripts without creating cyclic dependencies. I have my provisioners full of sed substitutions and would love to be able to render my scripts using templates instead. I believe this may require treating templates as first class citizens instead of resources. Any thoughts? |
Wow. This is fantastic. Thanks for writing this, @josharian! |
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. |
Fixes #215.
This PR is an initial pass, so that I can get early feedback. At a minimum, it lacks website/documentation updates, but I want to do that last, after the code has stabilized and the barns have been painted.
Sample
example.tf
:Sample
template.txt
:Output:
And everything shows up nicely for historical purposes in
terraform.tfstate
.