open-policy-agent / conftest

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

Rules returning a boolean (no msg) #864

Open zregvart opened 1 year ago

zregvart commented 1 year ago

Hello :wave:

Are Rego rules without the msg, e.g. ones returning a boolean supported?

Given this Rego rule:

package example

violation[msg] {
    msg := "should emit violation"
}

violation[msg] {
    false

    msg := "should not emit violation"
}

# expecting a violation
violation_no_msg = true

violation_no_msg_succeed {
    false # should not fail
}

deny[msg] {
    msg := "should emit deny"
}

deny[msg] {
    false

    msg := "should not emit deny"
}

# expecting a deny
deny_no_msg = true

deny_no_msg_succeed {
    false # should not fail
}

warn[msg] {
    msg := "should emit warning"
}

warn[msg] {
    false
    msg := "should not emit warning"
}

# expecting a warning
warn_no_msg = true

warn_no_msg_succeed {
    false # should not warn
}

When I run conftest test input.json --all-namespaces I get:

WARN - input.json - example - should emit warning
FAIL - input.json - example - should emit violation
FAIL - input.json - example - should emit deny

12 tests, 9 passed, 1 warning, 2 failures, 0 exceptions

I was expecting that the *_no_msg rules that matched would also appear in the output. Perhaps something like:

WARN - input.json - example - should emit warning
WARN - input.json - example - warn_no_msg
FAIL - input.json - example - should emit violation
FAIL - input.json - example - deny_no_msg
FAIL - input.json - example - should emit deny
FAIL - input.json - example - violation_no_msg

12 tests, 6 passed, 2 warning, 4 failures, 0 exceptions

I'm looking at the comment here, and I'm not certain that what is stated there is correct, the *_no_msg rules above did return true but were counted towards successes.

Perhaps the rules returning a boolean true could be considered, for example using something like:

diff --git a/policy/engine.go b/policy/engine.go
index b84c91c..89ca921 100644
--- a/policy/engine.go
+++ b/policy/engine.go
@@ -475,6 +475,9 @@ func (e *Engine) query(ctx context.Context, input interface{}, query string) (ou
            if _, ok := expression.Value.([]interface{}); ok {
                expressionValues = expression.Value.([]interface{})
            }
+           if _, ok := expression.Value.(bool); ok {
+               expressionValues = []interface{}{expression.Text}
+           }
            if len(expressionValues) == 0 {
                results = append(results, output.Result{})
                continue
boranx commented 11 months ago

Hi

Apparently, as you mentioned, it considers the result only for the forms deny[msg] or violation[{"msg": msg}] What would be the use cases of having rules returning just a boolean? As far as I can imagine, the following would always fail no matter how the body is evaluated

deny_no_msg2 = true {
    false # OR not.input.data.field
}

If it's a need, we'd welcome a PR addressing the issue with the changes you pointed out above

lcarva commented 11 months ago

@boranx, the example you provided does not cause a failure. conftest reports it as passed.

I don't have a use case for using a rule that returns just a boolean, but it is quite surprising that it always passes. It is certainly a pitfall for users.