open-policy-agent / conftest

Write tests against structured configuration data using the Open Policy Agent Rego query language
https://conftest.dev
Other
2.86k stars 304 forks source link

Change output format to include message type and absolute file path #42

Closed brendanjryan closed 5 years ago

brendanjryan commented 5 years ago

The current command lint output of conftest is difficult to parse by CI or other command line tooling - mostly due to the fact that it does use show absolute file paths or distinguish between errors and warnings aside from color.

I propose changing the "output" to be of the form <msg_type>: <file> - <msg> (still colorized)

For example:

WARNING: /Users/brendanjryan/projects/conftest/example/test.yaml - nginx-deployment should not be configured to live in the default namespace
FAILURE: /Users/brendanjryan/projects/conftest/example/test.yaml - nginx-deployment must not include any Horizontal Pod AutoScalers

Reference of existing format:

conftest test deployment.yaml
deployment.yaml
   Containers must not run as root
   Deployments are not allowed

Is this something we are interested in supporting? While this may break existing integrations - it will make future ones much easier to write.

garethr commented 5 years ago

I'm happy to break backwards compatibility at this early stage, and had been thinking about changes to the output as well. So definitely happy to see a PR for the proposed change here.

A few other thoughts:

Worth noting errors as well are currently output differently. eg:

FATA[0000] Problem building rego compiler: 1 error occurred: kubernetes.rego:7: rego_unsafe_var_error: var iput is unsaf

It might be nice to normalize the output for those as well, though could separate as well.

brendanjryan commented 5 years ago

Good points, and thank you for the links!

I think the overarching theme here is that there is a lot of interest around different output formats, some which are easy to add (json etc..) while others are a little tricker (JUnit XML..). I have some free time over the next few days so I can take a first pass at breaking out some of this code / building a framework that others can extend as well. I'll likely start small with only supporting the existing text format and json - but will look at TAP and JUnit as well!

garethr commented 5 years ago

I just used https://github.com/gotestyourself/gotestsum in CircleCI to get a JUnit XML output for go test and it has some sample code for generating the relevant XML: https://github.com/gotestyourself/gotestsum/blob/master/internal/junitxml/report.go. Noting here for reference.

garethr commented 5 years ago

Thinking a bit more about the UI for this, and looking at other examples.

Very similar to what you proposed, one approach would definitely seem to be:

FAIL test.yaml - nginx-deployment should not be configured to live in the default namespace
FAIL test.yaml - nginx-deployment must not include any Horizontal Pod AutoScalers

Keeping the prefix the same length would be nice, so maybe using FAIL, WARN or PASS.

For color, trying this out I think this might be better if only the prefix (ie. FAIL above) is in color. I think this improves the readability without scarifying the scan-ability.

Another very similar approach, which is slightly shorter and very slightly less explicit, would be to use the unicode characters.

✗ test.yaml - nginx-deployment should not be configured to live in the default namespace
✗ test.yaml - nginx-deployment must not include any Horizontal Pod AutoScalers

We could use for PASS, for WARN and for FAIL.

One other thing here that might be out of scope of the first implementation. At the moment Conftest only shows failures and warnings in output. Would it be useful to also show passing rules? Either behind a flag or otherwise.

garethr commented 5 years ago

After merging #55 I took a run at altering the output as per the above. See #58

garethr commented 5 years ago

I'll close this issue out now. The UI is much improved, and we have both JSON and TAP (PR: #68) output that are both machine readable. The default output is also much easier to parse line-by-line.