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

feature(builtins): add a new builtin function to render templated strings #6411

Merged

Conversation

RDVasavada
Copy link
Contributor

@RDVasavada RDVasavada commented Nov 14, 2023

Why the changes in this PR are needed?

closes: #6371

Adds a builtin function for template rendering.

What are the changes in this PR?

Adds a new builtin function that accepts a templated string and a mapping of vars to inject, and returns the rendered string with the vars injected.

Notes to assist PR review:

For this builtin function, I've set the missingkey option to error, so that if the template contains a variable that is not present in the given variable mapping, the function will not attempt to render the template and return an error. This was done to prevent silent failures from occurring.

Copy link

netlify bot commented Nov 14, 2023

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit dbf6871
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/655691a409b1d80008e4ecac
😎 Deploy Preview https://deploy-preview-6411--openpolicyagent.netlify.app/docs/edge/policy-reference
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@RDVasavada RDVasavada changed the title Feature/template rendering feature(builtins): add a new builtin function to render templated strings Nov 14, 2023
@RDVasavada RDVasavada marked this pull request as ready for review November 14, 2023 17:13
Copy link
Member

@ashutosh-narkar ashutosh-narkar left a comment

Choose a reason for hiding this comment

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

@RDVasavada thanks for the contribution. Few comments inline.

ast/builtins.go Outdated Show resolved Hide resolved
ast/builtins.go Outdated Show resolved Hide resolved
ast/builtins.go Outdated Show resolved Hide resolved
@RDVasavada
Copy link
Contributor Author

RDVasavada commented Nov 15, 2023

Thanks for your feedback @ashutosh-narkar. I have addressed your comments -- let me know if you have any additional feedback!

ast/builtins.go Outdated Show resolved Hide resolved
ast/builtins.go Outdated Show resolved Hide resolved
ast/builtins.go Outdated Show resolved Hide resolved
ast/builtins.go Outdated Show resolved Hide resolved
Copy link
Member

@ashutosh-narkar ashutosh-narkar left a comment

Choose a reason for hiding this comment

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

LGTM

@ashutosh-narkar
Copy link
Member

@RDVasavada can you please squash your commits and we can get this in. Thanks!

@ashutosh-narkar
Copy link
Member

Check this link if you need pointers to craft a good commit message.

builtin_metadata.json Outdated Show resolved Hide resolved
@RDVasavada
Copy link
Contributor Author

Added a unit test for non-string template var inputs and added back the template docs link as requested by @johanfylling, please let me know if there's any other feedback before I squash, thanks!

Copy link
Member

@ashutosh-narkar ashutosh-narkar left a comment

Choose a reason for hiding this comment

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

LGTM.

@RDVasavada RDVasavada force-pushed the feature/template-rendering branch 2 times, most recently from 8185718 to 76f6d7c Compare November 16, 2023 22:54
@RDVasavada
Copy link
Contributor Author

@ashutosh-narkar rebased and squashed

… render templated strings

This adds support for rendering of templated strings utilizing Golang's text/template library.
For a given templated string and key/value mapping of template var inputs, this builtin will
inject the values into the template where they are referenced by key.

Fixes open-policy-agent#6371
Signed-off-by: Rohan Vasavada <[email protected]>
@ashutosh-narkar ashutosh-narkar merged commit d46bc9d into open-policy-agent:main Nov 17, 2023
24 checks passed
@RDVasavada RDVasavada deleted the feature/template-rendering branch November 17, 2023 17:24
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.

Add a builtin function to render JSON templates
3 participants