feat(jsonnet): add support for importing all files in a directory by partcyborg · Pull Request #1374 · grafana/tanka

partcyborg · 2025-03-04T23:17:57Z

Add native function importFiles which reads and evaluates all jsonnet files in a directory.

Returns an object of with key/value pairs of <file name>: <evaluated object>

Directory path is found using a calledFrom option, similar to helmTemplate.

Includes support for:

Choosing files by extension using the extension option (default is .libsonnet)
Excluding files using the exclude option, which takes an array of file names to skip

Fixes #1375

partcyborg · 2025-03-04T23:24:26Z

If accepted, I will add support for this function to jsonnet-libs

Additionally, I noticed that the TestColordiff test is failing locally, but I am not sure if that is related to my local system or caused by something else.

dsotirakis

@partcyborg 👋
Thanks for your contribution! Would you also be able to add some tests please, for our better understanding and ease of reviewing?

meant to comment

partcyborg · 2025-03-10T23:27:56Z

@partcyborg 👋 Thanks for your contribution! Would you also be able to add some tests please, for our better understanding and ease of reviewing?

Hey @dsotirakis, thank you for your reply. I have added a unit test for the importFiles native function

Update: I noticed i was missing a test for the exclude functionality, so i updated the test to ensure that is working too

Add native function `importFiles` which reads and evaluates all jsonnet files in a directory. Returns an object of with key/value pairs of `<file name>: <evaluated object>` Directory path is found using a `calledFrom` option, similar to `helmTemplate`. Includes support for: - Choosing files by extension using the `extension` option (default is `.libsonnet`) - Excluding files using the `exclude` option, which takes an array of file names to skip

zerok

Thank you for proposing this 🙂 I've left you a comment also in the issue with an idea for a different approach 🙂

zerok · 2025-03-14T07:51:37Z

 }
+
+func TestImportFiles(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "importFilesTest")


suggestion: Please use T.TempDir here. With that you don't need to do the os.RemoveAll manually later on.

Replaced this with a call to T.TempDir()

zerok · 2025-03-14T08:04:39Z

+	importDir := filepath.Join(tempDir, importDirName)
+	err = os.Mkdir(importDir, 0750)
+	assert.Nil(t, err)
+	importFiles := []string{"test1.libsonnet", "test2.libsonnet"}


suggestion: I think it would make the test easier to read if the files were explicitly filled with certain content and the result was compared to an "expected" JSON string 🙂

I have refactored the test to

Use static jsonnet content in the imported and excluded files

Compare the result in json format with the expected json

Please let me know if this is what you were thinking.

zerok · 2025-03-14T08:07:16Z

+}
+
+// importFiles imports and evaluates all matching jsonnet files in the given relative directory
+func importFiles(vm *jsonnet.VM) *jsonnet.NativeFunction {


todo: Please add documentation about this new function to https://github.com/grafana/tanka/blob/main/docs/src/content/docs/jsonnet/native.md 🙂

@zerok I am happy to do this, but held off because I don't see documents for helmTemplate here either. My original plan was to add jsonnet library code similar to the helm functions in tanka-util and document those.

That being said, I am happy to also add documentation for the raw native function here, but I would like to get some clarity on whether this will be accepted before I do the work to add it.

Duologic · 2025-03-14T17:11:09Z

This breaks the hermitic nature of jsonnet, I'd vouch heavily against doing this in Tanka as we'd be creating a parallel ecosystem that is different from upstream.

See upstream discussion here: https://groups.google.com/g/jsonnet/c/JdvlXDAdvq0

Duologic

adding a request changes as I would love to have a more in depth discusssion about this, see comments in #1375

partcyborg · 2025-03-19T19:23:07Z

This breaks the hermitic nature of jsonnet, I'd vouch heavily against doing this in Tanka as we'd be creating a parallel ecosystem that is different from upstream.

See upstream discussion here: https://groups.google.com/g/jsonnet/c/JdvlXDAdvq0

While I see your point, I don't think has the impact you think it does for a few reasons:

The jsonnet files rendered by importFiles are each evaluated in their own jsonnet context, with the result being a json object. This means there is no late binding between the primary VM, nor between individual files. My plan is to make this clear in the documentation and that the primary use case for this is for generating configs for inline environments.
The hermetic nature of jsonnet is already broken in Tanka via the helmTemplate native function, as this loads content from an arbitrary number of files in an arbitrary directory

Duologic · 2025-03-26T08:13:57Z

Why not render the JSON beforehand then? Seems like this could be handled by the tooling pipeline rather than the jsonnet code.

The fact that we break hermiticity in another feature does not warrant that we should do it regardless. These features have different tradeoffs.

partcyborg requested a review from a team as a code owner March 4, 2025 23:17

partcyborg mentioned this pull request Mar 4, 2025

Add support for importing all jsonnet files in a directory #1375

Open

partcyborg force-pushed the import-files branch 2 times, most recently from d0afe89 to a73c5d1 Compare March 5, 2025 00:45

dsotirakis previously approved these changes Mar 10, 2025

View reviewed changes

partcyborg force-pushed the import-files branch 5 times, most recently from 4c46530 to 0f7d83d Compare March 12, 2025 19:34

add unit test

af47706

partcyborg force-pushed the import-files branch from 0f7d83d to af47706 Compare March 12, 2025 19:36

zerok reviewed Mar 14, 2025

View reviewed changes

Duologic requested changes Mar 14, 2025

View reviewed changes

partcyborg added 2 commits March 19, 2025 12:49

require calledFrom to be inside Tankas current working directory

692ef56

refactor TestImportFiles to compare rendered json with static data

dfba8d4

Conversation

partcyborg commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

partcyborg commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsotirakis left a comment

Choose a reason for hiding this comment

Uh oh!

partcyborg commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zerok left a comment

Choose a reason for hiding this comment

Uh oh!

zerok Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

partcyborg Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zerok Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

partcyborg Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

zerok Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

partcyborg Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

Duologic commented Mar 14, 2025

Uh oh!

Duologic left a comment

Choose a reason for hiding this comment

Uh oh!

partcyborg commented Mar 19, 2025

Uh oh!

Duologic commented Mar 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

partcyborg commented Mar 4, 2025 •

edited

Loading

partcyborg commented Mar 4, 2025 •

edited

Loading

partcyborg commented Mar 10, 2025 •

edited

Loading