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

Added templatestring function similar to templatefile #1223

Merged
merged 22 commits into from Feb 28, 2024

Conversation

sanskruti-shahu
Copy link
Contributor

@sanskruti-shahu sanskruti-shahu commented Feb 1, 2024

Objective

The objective of this issue is to address a limitation in Terraform, where the templatefile() function requires a file to be present on disk, thus restricting the dynamism of template usage. The goal is to introduce a new function named templatestring() to enable template usage directly from a string, eliminating the need for a file and enhancing flexibility in generating dynamic templates.

Changes

  • Implemented the MakeTemplateStringFunc() function inside string.go, containing the execution logic for the templatestring() function.
  • Created a new file, render_template.go, to house the common renderTemplate() function used by both templatestring() and templatefile().
  • Added comprehensive tests in string_test.go to validate the implementation of the templatestring() function and renderTemplate() function in render_template_test.go.
  • Created templatestring.mdx, a documentation file outlining the usage of the templatestring() function, to be displayed on the opentofu website.
  • Updated language-nav-data.json and description.go files to include information about the templatestring() function.
  • Updated CHANGELOG.md to reflect the addition of the templatestring() function.

Resolves #301

Target Release

1.7.0

Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
@sanskruti-shahu sanskruti-shahu requested a review from a team as a code owner February 1, 2024 20:29
Copy link

github-actions bot commented Feb 1, 2024

Reminder for the PR assignee: If this is a user-visible change, please update the changelog as part of the PR.

Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
@janosdebugs janosdebugs requested review from janosdebugs and removed request for a team February 1, 2024 21:02
Copy link
Member

@Yantrio Yantrio left a comment

Choose a reason for hiding this comment

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

Hi @sanskruti-shahu , thanks for submitting this. I've left some feedback in comments.

internal/lang/funcs/filesystem.go Outdated Show resolved Hide resolved
internal/lang/funcs/filesystem_test.go Outdated Show resolved Hide resolved
internal/lang/funcs/filesystem_test.go Outdated Show resolved Hide resolved
internal/lang/funcs/filesystem_test.go Outdated Show resolved Hide resolved
@sanskruti-shahu
Copy link
Contributor Author

@Yantrio I'll work upon the changes that you requested.

internal/lang/funcs/filesystem_test.go Outdated Show resolved Hide resolved
internal/lang/funcs/string.go Outdated Show resolved Hide resolved
internal/lang/funcs/string.go Outdated Show resolved Hide resolved
website/docs/language/functions/templatestring.mdx Outdated Show resolved Hide resolved
website/docs/language/functions/templatestring.mdx Outdated Show resolved Hide resolved
website/docs/language/functions/templatestring.mdx Outdated Show resolved Hide resolved
website/docs/language/functions/templatestring.mdx Outdated Show resolved Hide resolved
website/docs/language/functions/templatestring.mdx Outdated Show resolved Hide resolved
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
internal/lang/funcs/filesystem_test.go Outdated Show resolved Hide resolved
internal/lang/funcs/string.go Outdated Show resolved Hide resolved
internal/lang/funcs/string_test.go Show resolved Hide resolved
internal/lang/functions.go Outdated Show resolved Hide resolved
sanskruti-shahu and others added 3 commits February 9, 2024 15:53
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
Copy link
Member

@Yantrio Yantrio left a comment

Choose a reason for hiding this comment

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

I've discussed this with the core team, and we believe that in situations where you pass in either

  • A sensitive template
  • OR any senstive variables

The resultant templated string should be sensitive.

This is in line with other functionality in OpenTofu.

Would you mind implementing it this way please instead of having it error? Thanks!

Please reach out with any questions you have before beginning any other implementations to avoid re-doing some work!

internal/lang/funcs/string.go Outdated Show resolved Hide resolved
internal/lang/funcs/string.go Outdated Show resolved Hide resolved
internal/lang/funcs/string.go Outdated Show resolved Hide resolved
Signed-off-by: Sanskruti Shahu <76054960+sanskruti-shahu@users.noreply.github.com>
@sanskruti-shahu
Copy link
Contributor Author

Hey @Yantrio , I've replied to your comments. Could you please check it once? Also, could you review the code changes again to let me know if there are any further changes needed?

@Yantrio
Copy link
Member

Yantrio commented Feb 20, 2024

Hi @sanskruti-shahu , we discussed it in one of the conversations that the sensitive value should be propogated throughout instead of erroring.

Would you mind implementing this please? I do not see this in the current code.

@sanskruti-shahu
Copy link
Contributor Author

Hey @Yantrio , I've implemented the changes you mentioned. Could you please review them?

Copy link
Member

@Yantrio Yantrio left a comment

Choose a reason for hiding this comment

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

Apart from the one tiny tiny nitpick, everything is looking good to me. I've requested that @janosdebugs also looks so that I have a second set of eyes on this, just to be safe.

I've been running things locally and i can confirm the sensitive flag is propagated properly as per the tests.

internal/lang/funcs/string.go Outdated Show resolved Hide resolved
Copy link
Contributor

@janosdebugs janosdebugs left a comment

Choose a reason for hiding this comment

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

I'm sorry to jump in this late into the PR, but there's a large chunk of copy-pasted code and there's no good reason to maintain two copies of the code. Everything else, including the tests is fine, but this should not be merged as is. Please do not copy-paste this much code.

internal/lang/funcs/string.go Outdated Show resolved Hide resolved
Co-authored-by: James Humphries <James@james-humphries.co.uk>
Signed-off-by: Sanskruti Shahu <76054960+sanskruti-shahu@users.noreply.github.com>
@sanskruti-shahu
Copy link
Contributor Author

@janosdebugs , I have implemented the changes you mentioned. Could you please review them?

@cam72cam
Copy link
Contributor

Two small changes @sanskruti-shahu requested, but it's looking good !

Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
cam72cam
cam72cam previously approved these changes Feb 22, 2024
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
Copy link
Contributor

@janosdebugs janosdebugs left a comment

Choose a reason for hiding this comment

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

Thanks @sanskruti-shahu great job! Tiny nitpick, but I'll approve since these don't change much.

internal/lang/funcs/render_template.go Outdated Show resolved Hide resolved
internal/lang/funcs/string.go Outdated Show resolved Hide resolved
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
@cam72cam cam72cam merged commit 835dcb8 into opentofu:main Feb 28, 2024
8 checks passed
IgnorantSapient pushed a commit to IgnorantSapient/opentofu that referenced this pull request Apr 1, 2024
Signed-off-by: sanskruti-shahu <sanskruti.shahu@harness.io>
Signed-off-by: Sanskruti Shahu <76054960+sanskruti-shahu@users.noreply.github.com>
Co-authored-by: James Humphries <James@james-humphries.co.uk>
Signed-off-by: Ashwin Annamalai <4549937+IgnorantSapient@users.noreply.github.com>
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.

Render templates from strings e.g. templatestring function similar to templatefile
4 participants