Support newline-delimited JSON and fix YAML multi-doc vs. sequence #1168
Conversation
neiser
force-pushed
the
support-ndjson-as-multidoc
branch
4 times, most recently
from
d9156c4 to
24908ea
Compare
This removes the clumsy cast to []any in engine by changing the Parser interface. Also, use the official go-yaml library from the YAML foundation, and give Parser an io.Reader which is more memory efficient for some implementations Signed-off-by: Andreas Grub <[email protected]>
Signed-off-by: Andreas Grub <[email protected]>
neiser
force-pushed
the
support-ndjson-as-multidoc
branch
from
24908ea to
c833538
Compare
|
Adding a |
@jalseth Thank you for looking at this PR! As managing multiple PRs on top of each other is some effort, would you also be fine if I split up the first commit of this PR in somewhat smaller parts? I think I can create the following sequence of commits which you can review one by one, I hope:
Would that be separated enough? I fear that the first two steps (touching |
|
@neiser Thanks for the detailed plan, and sorry for the delay in getting back to you. After doing some testing, I think your proposal makes sense. Although JSON and some other formats only allow a single object per file, others allow multiple, and in that world the best way to handle that is always return a list even if some formats will always have a single item in the list. A couple of notes:
|
|
@jalseth I agree with your proposal and I'm glad that you see the benefit of my changes to the |
2 participants
First of all, sorry for this unsolicited PR!
The changes here fix an inconsistency when handling YAML multi documents, such as
vs. a single YAML document which is a sequence on top-level (resulting in a config which can accidentally be cast to
[]any):Due to an unfortunate cast to
[]anyin theenginepackage, this situation was handled incorrectly and it was impossible to run Rego validations on those fundamentally different structures correctly, as both were handled as the same config inputs to Rego policy"a"and"b". With the change, the inputs stay for the multi-doc (first case), but changes to the correct single input["a", "b"]for the single-document (second case).Also, newline-delimited JSON (
ndjsonextension) can be seen as some multi-document JSON, and I've adjusted the existing JSON parser similarly to the YAML parser such that multiple documents in one file are parsed as[]anywithlength>1.Such
ndjsoninput is convenient whenconftesting logs from sources. I've added an example for this.In fact, there's not even an
\nas delimiter needed, as we just continue parsing documents until we encounter EOF.This required some larger changes across all parsers, and I hope you see the benefit of this change.
Most importantly, the code in
engineis streamlined and does not cast the parsed configurations, leading to the initially described confusion with YAML.I've also took the chance and simplified the
formatcommand, which now consistently uses two spaces instead of tab for separation (this was previously depending on whether--combinewas used or not).I could also offer to make the tests a bit more beautiful using the famous
testifypackage. Would you accept/merge this as well?Let me know what you think!