StyraInc / regal

Regal is a linter for Rego, with the goal of making your Rego magnificent!
https://docs.styra.com/regal
Apache License 2.0
237 stars 30 forks source link

Make linter rules of strict compile time checks? #2

Open anderseknert opened 1 year ago

anderseknert commented 1 year ago

Since we'll be depending on OPA anyway, it would be useful to include all of the strictness checks from OPA but in the form of linter checks. These would naturally not be described in Rego, but would be displayed as other linter violations without requiring the use of another command (opa check, opa eval, ...) and would have links back to our documentation portal like the other linter rules do.

anderseknert commented 1 year ago

This is "blocked" by https://github.com/open-policy-agent/opa/issues/5815

Most of these should be easy to implement using the AST alone, so will probably go with that rather than relying on the compiler. First one already submitted: https://github.com/StyraInc/regal/pull/76

anderseknert commented 1 year ago

Current status:

Rather than having a "strict" category, I think we can place these in the existing categories, with references pointing to strict mode in the OPA docs.

anderseknert commented 8 months ago

Given how most of the strict checks will be made default in OPA 1.0, I don't think there's any point in having them included in Regal at this point. The two rules that will remain checked only in strict mode are "unused imports" and "unused local assignment". While I would like to have both of those included as Regal linter checks, the API for the OPA compiler wrt strictness checks currently leaves a lot to be desired (as previously mentioned).

I doubt that these two rules have dependencies among them (i.e. that unused local assignment depends on the unused imports check) so perhaps they could be made independent checks exposed via some API. But until that day, this is on hold.

anderseknert commented 2 weeks ago

Reading this blog helped me understand that making compilation mandatory / opt-out would mean Regal would fail on many policies we're currently able to lint.

Finally, regarding the linter, although a syntax check command called opa check is provided as standard, there are issues with the Rego code executed with Conftest provided by OPA , such as errors occurring when using Conftest's own parse_config function because the syntax cannot be understood.

The reason opa check fails here is of course the custom built-in the command is unaware of. regal lint won't care about that though as long as we're able to parse the file. Both commands allow providing capabilities to help with this, but that's not likely an option new users will be aware of.

This makes me think compilation should be opt-in for regal lint. The language server OTOH should probably make it opt-out, as these are errors most would likely want to have surfaced in their editor without having to run external commands, "check on save", or whatnot.

Either way, in order for us to provide compilation even as an optional feature, we'll most likely need to look into configuration options for defining multiple bundle roots, rather than assuming that we can treat the whole workspace as a bundle.