StyraInc / regal

Regal is a linter and language server for Rego, bringing your policy development experience to the next level!
https://docs.styra.com/regal
Apache License 2.0
259 stars 35 forks source link

Make inline eval in VS Code aware of project-roots. #1192

Open Graloth opened 1 week ago

Graloth commented 1 week ago

Currently when using the inline eval in VS code, it includes the whole workspace in its evaluation.

This causes issues for those that have many different project roots that do not import or use each other.

Example workspace:

.regal/
  - config.yaml
environment-a/
 - service-a/
     - policy.rego
     - policy_test.rego
 - service-b/
     - policy.rego
     - policy_test.rego
environment-b/
 - service-a/
     - policy.rego
     - policy_test.rego
 - service-b/
     - policy.rego
     - policy_test.rego

If any of the policies have rules defined with the same name (yet still different package namespace), eval will fail with the "multiple default rules found" error.

However, in our case, the two services will never use or access eachother, making the error simply prevent us from utilising the VS Code extension to it's full extent.

This also goes for code coverage (failing for same reason).

Suggestions:

  1. Make the inline eval respect the project root based on the file it's checking
    • Preferably this should do so for every file if it's told to eval the entire workspace, so it doesn't complain about errors due to comparisons made across project roots.
    • In the workspace example above, it would mean that even if it's run for the whole workspace, if any of the services are defined as project-roots, they would not cause an error when evaluated as it would not look outside its root.
  2. Allow regal config.yaml to define a "root" config to inherit from.
    • We can then still keep the config defined in the root, but allow overwriting in the project-root config.
    • This way we can simple add a .regal dir with a config.yaml in each of our services in the example above, which then should make them automatically be treated as a project root.
anderseknert commented 1 week ago

If any of the policies have rules defined with the same name (yet still different package namespace), eval will fail with the "multiple default rules found" error.

That shouldn't happen, as the full path (package + rule) is used to determine conflicts. So you should definitely be able to have a default allow := false in package foo.bar and another in package bar.baz. I'm guessing there's some overlap in paths between the environments?

Anyway, I think the first suggestions would be the most straightforward, as we already have the plumbing in place (project root config options). It won't however solve something like "3 different project roots but one common lib that they all depend on". But since those are not your requirements, we can deal with that later.

Graloth commented 1 week ago

Ah, you are indeed correct, some of our more ancient policies I can see have invalid package namespaces, which then makes sense they cause problems 😁

As the the config/root inheritance part, I will create a separate issue for that, so they can be prioritised and broken down as separate tasks.

anderseknert commented 1 week ago

Feel free to create a feature request for that too! The topic of multiple/nested configurations have been discussed before, and while there are some advantages to that, there might also be a risk of things becoming less obvious if configuration is sourced from multiple locations. So at least for the near future, I think we'll want to see how far we can get with project roots as a way of defining multiple projects in a single repo.

Graloth commented 1 week ago

Maybe even support wildcards in project roots config list? That way it could match multiple directories as their own roots without explicitly listing all of them in the config (useful for large codebases)

anderseknert commented 1 week ago

Yeah, I guess that makes sense. So something like this:

project:
  roots:
    - projects/*

Would mean "every directory in projects is a project root"?

Assuming the projects are built as bundles, one option that works today already is putting a bundle .manifest file in the root directory of each bundle/project. Regal will register those directories as roots too. That saves you from updating the configuration, but it obviously instead requires having a .manifest file in each bundle. That's a good practice anyway, but of course, not all projects are built as bundles.