fugue / fregot

Fugue Rego Toolkit
Apache License 2.0
234 stars 11 forks source link

Fregot REPL Not Recognizing Rego Errors #289

Closed layro01 closed 2 years ago

layro01 commented 2 years ago

I'm running through the Interactively Debugging the Rego Policy Language with Fregot tutorial and the REPL isn't entering error mode when the JSON error is encountered on line 10.

Steps to reproduce:

(1) Installed Fregot v0.13.4 (fregot-v0.13.4-linux-x86_64.tar.gz). (2) Followed steps in tutorial down to Oh No, an Error!. No error is reported, just an empty return value. (3) Run fregot repl demo.rego --watch and continue to follow the steps. Output is shown below.

F u g u e   R E G O   T o o l k i t
fregot v0.13.4 repl - use :help for usage info
repl% :load demo.rego
Loading demo.rego...
Loaded package fregot.examples.demo
fregot.examples.demo% :input repl_demo_input.json
fregot.examples.demo% :break deny
Set breakpoint at fregot.examples.demo.deny
fregot.examples.demo% deny
21|     ami = amis[ami]
        ^^^^^^^^^^^^^^^
fregot.examples.demo(debug)% :step
10|     ami = input.resource_changes.change.after.ami
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
fregot.examples.demo(debug)% :step
(debug) finished     <--- DID NOT ENTER ERROR MODE
fregot.examples.demo%

Additionally, evaluating input.resource_changes.change does not return the expected index type error:

fregot.examples.demo(debug)% input.resource_changes.change
{}     <--- DID NOT RETURN EXPECTED evalRefArg: cannot index array with a string ERROR
fregot.examples.demo(debug)%

Any ideas?

ameliafugue commented 2 years ago

Hi @layro01,

We were able to reproduce your issue and we are working to resolve it.

jaspervdj-luminal commented 2 years ago

Hey @layro01,

Thanks for the detailed report! OPA Rego ignores these sort of errors and just fails the query instead. In order to better comply to the OPA standard, we updated that in fregot, so indexing an array with a string does no longer throw an error.

As a workaround for the purpose of this tutorial, you can use the following code instead.

For the initial "broken" version in the tutorial, use this definition for amis[ami]:

# All AMIs in the input
amis[ami] {
    ami = input.resource_changes[_].change.after
    startswith(ami, "ami-")
}

This forgets to grab the ".ami" field, and the startswith will throw an error since it is not being called on a string.

For further down in the tutorial, you can use this "fixed" version:

# All AMIs in the input
amis[ami] {
    ami = input.resource_changes[_].change.after.ami
    startswith(ami, "ami-")
}

While the messages you get from fregot will be slightly different due to the new code, I think it preserves the goal of the demo, i.e. stepping through code. We'll work on updating the examples.

Please let us know if you have any other questions!

layro01 commented 2 years ago

Thank you for the confirmation - I'm still trying to wrap my head around OPA treating unexpected schema elements as policy failures rather than syntax errors (I almost wish there was a way to configure this behavior - it would even make writing Rego tests clearer).

I've validated that the workaround works correctly - should this make it's way into the tutorial?

jaspervdj-luminal commented 2 years ago

@layro01 Thanks for verifying! Yeah, unfortunately Rego takes a little effort to learn, but I think the payoff is worth it. Let us know if you have any other questions :-)

We've now updated the tutorial and examples, so I'm going to close this issue.