CLI: opa eval -S/--strict

[srenatus]

The compiler's strict mode is only exposed via opa check --strict <file.rego>. It would be nice if opa eval also had a flag like this.

[tsandall]

We should only apply the strict mode checks to the Rego modules that get compiled, i.e., do not enable strict mode on the user-supplied query.

[Parsifal-M]

Hey guys 👋

Just curious as I've been playing around with this a bit but getting a little stuck, how would you go about approach this? Maybe at a high level? 😅

[srenatus]

Heya. I'm assuming that adding the CLI flag isn't the problem 😄

So, having skimmed the code of cmd/eval.go, it seems we don't have any way to access the compiler -- it is used through the rego package. That package wraps the compiler, and the module parsing and all that.

So we would have to add a field to its config methods, like Strict(yes bool) here, and plumb that together with the compiler's WithStrict(bool), maybe in rego.New().

Does that help?

[Parsifal-M]

Heya. I'm assuming that adding the CLI flag isn't the problem smile

So, having skimmed the code of cmd/eval.go, it seems we don't have any way to access the compiler -- it is used through the rego package. That package wraps the compiler, and the module parsing and all that.

So we would have to add a field to its config methods, like Strict(yes bool) here, and plumb that together with the compiler's WithStrict(bool), maybe in rego.New().

Does that help?

Definitely helps thanks! Now I see I was getting stuck here:

So, having skimmed the code of cmd/eval.go, it seems we don't have any way to access the compiler -- it is used through the rego package. That package wraps the compiler, and the module parsing and all that.

Glad I asked the question, I will keep hacking away then!

Thanks again 👍

[srenatus]

To be quite honest, I didn't foresee that extra hoop to jump through. 😅

[Parsifal-M]

Hey @srenatus 👋

I think I can work on this one now if you'd like to assign it to me.

I've made some good progress and just need to figure out the testing part and what needs to be modified there.

Is that ok?

Thanks!

[Parsifal-M]

Just small question on this, running opa eval -S policy.rego doesn't seem to work that well... however running for example opa eval -S -d policy.rego data.app.rbac.allow will return something like:

{
  "errors": [
    {
      "message": "import must not shadow import input.attributes.request.http as http_request",
      "code": "rego_compile_error",
      "location": {
        "file": "policy.rego",
        "row": 7,
        "col": 1
      }
    }
  ]
}

Could be I'm implementing it incorrectly 🤔 just wondering what you guys think should be the functionality here, should it be just running opa eval -S policy.rego should give the same output as running opa check -S policy.rego ?

Which would just be 1 error occurred: policy.rego:7: rego_compile_error: import must not shadow import input.attributes.request.http as http_request

Curious to hear your thoughts on this 👍

[anderseknert]

opa eval should require a query, so the current behavior is IMHO the right one. Adding a strict flag should simply allow the strictness check baked into the eval command, but not have it's semantics changed.

[Parsifal-M]

Hey! 👋

Just wondering @srenatus, how you would like to see this tested? As its kind of new behavior what should I consider when making tests?

Any tips you can give on this? 😄

Thanks!

[srenatus]

There are no e2e tests for executables and their outputs. So it would be nice if there was a test cases added in cmd/eval_test.go, and it would test the eval method, like this test

[Parsifal-M]

Hey! @srenatus 👋

I will push what I have tomorrow/Monday for this, I have the basic groundwork done (but need to do some tidying up) for the testing but might need some guidance on doing the testing properly, hope that is okay, think it may be easier if you can see what I have done... 😅

Thanks!

[Parsifal-M]

Hey @srenatus 👋, team,

While working on this, I did notice the following and just wondered if it's intended or not, let's say in one policy we have multiple issues that strict would pick up on, currently, it only returns the "last" e.g:

opa eval -S -d example2.rego "data.x.foo"

Policy:

package x

import future.keywords.if

import future.keywords.if

data if {

foo = 2

}

Outputs:

{
  "errors": [
    {
      "message": "rules must not shadow data (use a different rule name)",
      "code": "rego_compile_error",
      "location": {
        "file": "example2.rego",
        "row": 7,
        "col": 1
      }
    }
  ]
}

I double-checked with opa check -S example2.rego

And get (not sure if intended):

1 error occurred: example2.rego:7: rego_compile_error: rules must not shadow data (use a different rule name)

This is a bit of a head-scratcher for me 😅 but I think I am getting somewhere, was just curious about the above.

Thanks! 👍