Structured output about conftest HOT 6 CLOSED

Just fyi, as a workaround while waiting for #100 to be merged I've been using the following pattern:

package my.conftest

import my.structured_rules as rules

warn[msg] {
  rules.violation[v]
  v.level = "warn"
  msg = v.msg
}

warn[msg] {
  rules.violation[v]
  v.level = "deadline"
  time.now_ns() < time.parse_rfc3339_ns(v.deadline)
  msg = sprintf("%s (Will become an error after %s)", [v.msg, v.deadline])
}

deny[msg] {
  rules.violation[v]
  v.level = "deadline"
  time.now_ns() >= time.parse_rfc3339_ns(v.deadline)
  msg = v.msg
}

deny[msg] {
  rules.violation[v]
  v.level = "deny"
  msg = v.msg
}

Now I can already write structure errors in the my.structured_rules namespace as violation. Note that here I also used the error level as a property of the object as I found I often got code duplication when writing deny and warn rules separately (especially when including other namespaces). Also added the deadline level allowing you to specify a violation to start off as a warning and automatically become an error when a certain date passes.

An example structured ruleset could be something like:

package my.structured_errors

violation[{
  "msg": msg,
  "level": "deadline",
  "deadline": "2020-01-01T00:00:00.000Z",
  "code": "FooNotEqualToBarError"
}] {
  input.foo != "bar"
  msg := sprintf("Your foo should always be 'bar', instead found '%s'", [input.foo])
}

Which will (currently) yield the warning:

Your foo should always be 'bar', instead found 'baz' (Will become an error after 2020-01-01T00:00:00.000Z)

while in two months it will be an error (without the suffix warning it will become an error of course). This allows us to introduce new rules without immediately breaking all the builds.

Also with the code it is nicer to write tests as you don't have to inspect the message itself as I mentioned earlier:

test_foo_required {
  violation[v] with input as {"foo": "baz"}
  trace(json.marshal(v)) # Helps with debugging
  v.code == "FooNotEqualToBarError"
}

from conftest.

I think this sounds like a nice area to explore.

Backwards compatibility is a valid concern, but also happy to break it if needed at this stage of the project.

Would be interesting to consider how we surface those details in the output.

It's probably worth proposing a specific design and having some discussion about it I think, it might be worth posting something in Slack (#conftest on the OPA Slack) and seeing what folks think.

from conftest.

I dove into the code a bit to try and understand how things are working, and it looks like there might be a simpler (backwards compatible-ish) way. Currently, conftest just takes the OPA policy result and type-asserts it to a string, changing that to a type switch and testing for a map[string]interface{} in addition to string could take care of both formats, even intermixed.

That still leaves two questions:

1. Should an object instead of a string passed as a policy violation adhere to some kind of schema, and if so, what kind?

2. How should the different output formats handle structured data?

With regards to the first question, I think it's probably best to have a very loose schema by default, perhaps just require a field called msg and leaving anything else up to the policy writer. A future improvement could be to have a policy enforcing the schema so you could ensure all your results adhere to your domain's requirements (If only we had some kind of policy enforcement engine 😛 )

As for the output, that's a little trickier, I think neither the stdout or tap formats really inherently support structured output (TAP does have directives, but it'd be tricky to map arbitrary structured data to it).

For json output it would be simpler, instead of

{
    "filename": "-",
    "Warnings": [],
    "Failures": [
        "Your foo has to bar the ban"
    ]
}

you'd get something like:

{
    "filename": "-",
    "Warnings": [],
    "Failures": [
        {
            "msg": "Your foo has to bar the ban",
            "details": {
                "code": "FOOBARBANERROR",
                "url": "https://lmgtfy.com/?q=how+to+bar+the+ban+with+foo"
            }
        }
    ]
}

strings and objects should be able to mix in the warnings and failures arrays, while a bit ugly, could just be the incentive for people to make their policies consistent, or if more compatibility is needed either a separate, second output format could be added where the old format always yields a string (obj["msg"] for structured violations) and a second format that casts non-structured violations to a simple single-field object.

from conftest.

This is something i've been wanting for a very long time -- thank you for writing this up!

Overall I think the proposed implementation looks good. Perhaps we could put this feature behind a "-details" flag so that it doesn't break current tooling out of the box?

Currently, conftest just takes the OPA policy result and type-asserts it to a string, changing that to a type switch and testing for a map[string]interface{} in addition to string could take care of both formats, even intermixed.

This sounds like a good path forward 👍

from conftest.

@brendanjryan I've added your suggestion to the PR I just pushed, thanks!

from conftest.

Released in v0.17.0

from conftest.