Support newline-delimited JSON and fix YAML multi-doc vs. sequence

[neiser]

First of all, sorry for this unsolicited PR!

The changes here fix an inconsistency when handling YAML multi documents, such as

a
---
b

vs. a single YAML document which is a sequence on top-level (resulting in a config which can accidentally be cast to []any):

- a
- b

Due to an unfortunate cast to []any in the engine package, 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 (ndjson extension) 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 []any with length>1.
Such ndjson input is convenient when conftesting logs from sources. I've added an example for this.
In fact, there's not even an \n as 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 engine is 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 format command, which now consistently uses two spaces instead of tab for separation (this was previously depending on whether --combine was used or not).

I could also offer to make the tests a bit more beautiful using the famous testify package. Would you accept/merge this as well?

Let me know what you think!

[jalseth]

Adding a ndjson parser seems reasonable to me, but this is a huge PR that touches many parts of the code base. From a quick skim, I don't see why the changes are needed to the existing parsers. Please split into smaller PRs with each addressing one specific change to the functionality of the code.

[neiser]

Please split into smaller PRs with each addressing one specific change to the functionality of the code.

@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:

1. Change Parser interface to return []any and remove the cast in the engine package, fixing the multi-doc vs. sequence confusion
2. Change Parser interface to take an io.Reader
3. Simplify and align format command output
4. Simpler YAML parsing using io.Reader, using the yaml/go-yaml lib
5. Simpler JSON parsing code using the io.Reader approach with accidental support for ndjson and using a library to strip the UTF-8 BOM

Would that be separated enough? I fear that the first two steps (touching Parser interface) are still quite a large commits, but the changes are rather repeating for each existing parser. I might even be simpler for me to combine step 1 und 2 into one commit. Would that be fine?

[jalseth]

@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:

• We should deprioritize step 3 as it is unrelated to the parser changes. It's fine to still send a PR, but can be reviewed separately from this series of changes.
• I'd prefer not taking a dependency on a library for the UTF BOM check as it's a single if statement that will not change over time.

[neiser]

@jalseth I agree with your proposal and I'm glad that you see the benefit of my changes to the Parser interface.
Concerning the UTF BOM check: The library I've included is actually capable of detecting more BOMs than just the one that was previously removed. Also, do you think one should introduce another package where I could put a wrapper type around io.Reader, such as TrimLeftReader which is able to strip the BOM bytes in the beginning (if present)?

[jalseth]

I reviewed the BOM library and am OK with taking a dependency on it. It has no transitive dependencies and doesn't have churn in the repo. Let's discuss additional changes after the core of the proposal has been implemented.

[neiser]

@jalseth Thanks for looking at the library. I hope I can work on it in about two weeks.