opa check should ignore non-Rego files

[anderseknert]

Running opa check --strict on a couple of policy libraries in the wild, and I'm seeing all sorts of errors related to data files in the scanned directories (commonly, merge errors). It's not clear to me why a command that describes its own role as:

Check Rego source files for parse and compilation errors.

Would need to attempt merging data files found in directories it traverses. I'm currently deploying something like:

--ignore "*.yaml" --ignore "*.yml" --ignore "*.json"

as a workaround, but I doubt most users would come to the conclusion that's what's needed in order to solve the quite opaque "merge error" (with no further explanation given) failure.

It is imperative that this command is easy to run regardless of project structure, so unless there are good reasons for parsing data files as part of this process, we shouldn't. If there are good reasons for doing that, we should explain those in the output of opa check --help. Given that the command seems to do exactly what I asked for with the --ignore flags provided, I doubt that's the case though.

[ashutosh-narkar]

In bundle mode, we read all files as seen here. We could apply some auto filters maybe to only read the Rego files.

[anderseknert]

Thanks Ash. FWIW, I did not run with the --bundle flag, just on a directory directly. I guess it's fine if opa check --bundle verifies the bundle as a whole, but if so it must be made clear what the flag means. Without the bundle flag, the command should just read Rego files, right?

[ashutosh-narkar]

Without the bundle flag, the command should just read Rego files, right?

Currently it reads both data and policy files. You mentioned that you saw errors merging data files. Did that happen w/o the bundle flag. If yes, did bundle mode resolve it?

[anderseknert]

Yes, it happened without the bundle flag, and bundle mode did not resolve it. A lot of projects in the infra space keeps test data files like resouce manifests in each policy directory, so you'll see lots of directories containing things like:

policy.rego
policy_test.rego
testdata/
  pod1.yaml
  pod2.yaml
  pod3.yaml

They then have some custom script that'll run tests in each directory separately. I have suggested in the past that OPA would better support this natively to avoid having users glue together their tests with bash scripts. But leaving that aside, there's no reason opa check would need to try and merge these various YAML or JSON files if the purpose of the command is to check only Rego files like it says it does, or at least I can't see one.

[ashutosh-narkar]

But leaving that aside, there's no reason opa check would need to try and merge these various YAML or JSON files if the purpose of the command is to check only Rego files like it says it does, or at least I can't see one.

Seems that way to me as well. Applying some auto filters seems like one way to resolve this.

[tjons]

I'll work on this