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

fixed: invalid json after injecting object or array in values template #492

Conversation

akselleirv
Copy link
Collaborator

fixes issue #491

It removes quotes around objects and arrays which happens when an object or array is injected in a template inside the values field.

// template after tmpl.Execute
// `{ "my_object":"${{ .my_object }}" }` ---> `{ "my_object":"{"this":"object"}" }`
// We therefore need to remove unnecessary strings making it valid JSON again
type replacement struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

text replacement like this sounds like sub-optimal for me.
let's discuss a better way to handle this conversion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I definitely agree that this is not the best solution. I was also thinking of using a function inside the template, for example:

...
values:
  my_object: ${{ .my_object | toObject }}
  my_array: ${{ .my_array | toArray }}

Would this be an acceptable solution? This is more explicit at least, but could be tiresome as one might think that this should work out of the box.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you write some unit test cases to cover these cases, also for primitive values like strings itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can do that, but for which approach? The string replacement or the function based approach?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The correctness of the replacement I would say.
I would expect that the result JSON gets parsed correctly for

  • primitive values (string, number, boolean)
  • object
  • array
  • map

@akselleirv akselleirv closed this Jan 31, 2023
@akselleirv akselleirv deleted the fix-invalid-json-tmpl-inject-on-obj-or-array branch January 31, 2023 06:33
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.

2 participants