Skip to content

(PUP-10139) Add find_template function #7840

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

Merged
merged 1 commit into from
Dec 2, 2019
Merged

(PUP-10139) Add find_template function #7840

merged 1 commit into from
Dec 2, 2019

Conversation

binford2k
Copy link
Contributor

This makes it easier users to render templates on the agent, meaning that
they can use secret values resolved by deferred functions directly with
e.g. Vault. That means that the master no longer needs keys to the kingdom.

@binford2k binford2k requested a review from a team November 13, 2019 19:39
Copy link
Contributor

@hlindberg hlindberg left a comment

Choose a reason for hiding this comment

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

Thanks Ben, that adds the missing find function to puppet core...

#
# This function can also accept:
#
# * An absolute String path, which will check for the existence of a template from anywhere on disk.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to clarify where on disk this is searched. The master or the agent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@puppetcla
Copy link

CLA signed by all contributors.

@melissa
Copy link
Contributor

melissa commented Nov 16, 2019

jenkins please test this on centos6-64

@jtappa
Copy link
Contributor

jtappa commented Nov 25, 2019

@puppetlabs/tech-pubs This is a new function with all new description, would someone mind taking a look?

Copy link
Contributor

@jbondpdx jbondpdx left a comment

Choose a reason for hiding this comment

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

Looks good, overall! I've suggested a few changes for spelling/typos and style.

# will search for the file `<MODULES DIRECTORY>/mymod/templates/secret.conf.epp`.)
#
# The primary use case is for agent-side template rendering with late-bound variables
# resolved, e.g. from secret stores inaccessable to the master, such as
Copy link
Contributor

Choose a reason for hiding this comment

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

replace "e.g." with "such as"

Copy link
Contributor

Choose a reason for hiding this comment

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

also "inaccessable" should be "inaccessible"

# Finds an existing template from a module and returns its path.
#
# The argument to this function should be a String as a `<MODULE NAME>/<TEMPLATE>`
# reference, which will search for `<TEMPLATE>` relative to a module's `templates`
Copy link
Contributor

Choose a reason for hiding this comment

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

replace "which will search" with "which searches"

@@ -0,0 +1,63 @@
# Finds an existing template from a module and returns its path.
#
# The argument to this function should be a String as a `<MODULE NAME>/<TEMPLATE>`
Copy link
Contributor

Choose a reason for hiding this comment

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

Change "The argument to this function should be" to "This function accepts an argument that is"

# The primary use case is for agent-side template rendering with late-bound variables
# resolved, e.g. from secret stores inaccessable to the master, such as
#
# ````
Copy link
Contributor

Choose a reason for hiding this comment

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

extra backtick

#
# This function can also accept:
#
# * An absolute String path, which will check for the existence of a template from anywhere on disk.
Copy link
Contributor

Choose a reason for hiding this comment

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

change "which will check" to "which checks"

# This function can also accept:
#
# * An absolute String path, which will check for the existence of a template from anywhere on disk.
# * Multiple String arguments, which will return the path of the **first** template
Copy link
Contributor

Choose a reason for hiding this comment

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

change "which will return" to "which returns"

#
# * An absolute String path, which will check for the existence of a template from anywhere on disk.
# * Multiple String arguments, which will return the path of the **first** template
# found, skipping non existing files.
Copy link
Contributor

Choose a reason for hiding this comment

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

"non existing" should be "nonexistent"

# * An absolute String path, which will check for the existence of a template from anywhere on disk.
# * Multiple String arguments, which will return the path of the **first** template
# found, skipping non existing files.
# * An array of string paths, which will return the path of the **first** template
Copy link
Contributor

Choose a reason for hiding this comment

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

change "which will return" to "which returns"

# * Multiple String arguments, which will return the path of the **first** template
# found, skipping non existing files.
# * An array of string paths, which will return the path of the **first** template
# found from the given paths in the array, skipping non existing files.
Copy link
Contributor

Choose a reason for hiding this comment

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

"non existing" should be "nonexistent"

# * An array of string paths, which will return the path of the **first** template
# found from the given paths in the array, skipping non existing files.
#
# The function returns `undef` if none of the given paths were found
Copy link
Contributor

Choose a reason for hiding this comment

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

missing period at the end of the sentence.

@binford2k
Copy link
Contributor Author

Thanks @jbondpdx. I'll file a separate PR to port these back to the find_file() function which was the original source.

@jbondpdx
Copy link
Contributor

jbondpdx commented Nov 25, 2019 via email

This makes it easier users to render templates on the agent, meaning that
they can use secret values resolved by deferred functions directly with
e.g. Vault. That means that the master no longer needs keys to the kingdom.
@jtappa
Copy link
Contributor

jtappa commented Dec 2, 2019

After @jbondpdx approval this is probably good to merge 👍

Copy link
Contributor

@jbondpdx jbondpdx left a comment

Choose a reason for hiding this comment

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

lgtm

@joshcooper joshcooper merged commit 1e71172 into puppetlabs:master Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants