tflint-ruleset-opa icon indicating copy to clipboard operation
tflint-ruleset-opa copied to clipboard

Add support for multiple policy directories

Open edubxb opened this issue 3 weeks ago • 6 comments

Motivation

At work, we are using rego for validating our terraform code.

Our approach is:

  1. Use tflint + the opa plugin for validating the HCL code with rego policies.
  2. Use conftest for validating the terraform plan output with rego policies.

The rego policies are different for both cases, but some conventions, values, and constants are shared between tflint and conftest policies. So, to avoid duplication, we created some "libraries" to share between both tools.

We have the following structure:

policies/
├── lib/
│   └── common.rego
├── conftest/
│   ├── policy1.rego
│   ├── policy2.rego
└── tflint/
    ├── policy1.rego
    └── policy2.rego

conftest allows using multiple paths for policies, but tflint doesn't, this PR implements this feature for tflint.

Please, don't hesitate to ask if you have any questions or need more information about the changes or if the approach used is not the best one.

edubxb avatar Nov 06 '25 00:11 edubxb

Initial notes in no particular order:

  • See failing test

Okay.

  • Link to documentation for other OPA tools to demonstrate that accepting multiple directories is a standard/common pattern

Will do.

  • This should be made backward compatible even if this the intended interface

    • Deprecate existing attributes/vars and warn on usage
    • Error if both deprecated and new keys are mixed

Regarding these points, the config property and the env var can be the same (basically, keep their names in singular, not plural), and add support for using a comma-separated list instead of a single value.

This way, there's no need to deprecate anything or generating a breaking change.

WDYT?

As long as you can address those this seems reasonable

Thanks for your comments!

edubxb avatar Nov 06 '25 17:11 edubxb

Regarding other tools supporting loading policies from different directories, the "official ones" from Open Policy Agent, allow it:

  • conftest→ https://www.conftest.dev/options/#-policy
  • opa (cli) → https://www.openpolicyagent.org/docs/cli#eval

edubxb avatar Nov 07 '25 14:11 edubxb

Yes, agree that singularizing is good for the env var, since you need to parse the list out of a string regardless. For the config file, having a structured serialized in a string is messy. For that, I don't see any alternative to a second plural attribute that's mutually exclusive with the singular one. It's not essential to deprecate the singular one either.

bendrucker avatar Nov 11 '25 00:11 bendrucker

Yes, agree that singularizing is good for the env var, since you need to parse the list out of a string regardless.

Agree.

For the config file, having a structured serialized in a string is messy. For that, I don't see any alternative to a second plural attribute that's mutually exclusive with the singular one. It's not essential to deprecate the singular one either.

I think having two (similar but exclusive) properties could be confusing, but the deprecation could be managed in future releases.

edubxb avatar Nov 11 '25 12:11 edubxb

What does conftest do for its config files? Its docs show:

# You can override the directory in which to store and look for policies
policy = "tests"

Since conftest is written in Go, I have to assume they are also trying to decode the TOML into a struct.

bendrucker avatar Nov 11 '25 16:11 bendrucker

Seems like the answer is that it uses Viper which handles coercing a single string into []string. HCL2 won't do that so you'd need to use an intermediate dynamic type. If you want to take a stab at it go for it otherwise I can try to pick this up and finish it.

bendrucker avatar Nov 11 '25 17:11 bendrucker

I've been a little busy, so if you want to take over this, feel free! Thanks, @bendrucker.

edubxb avatar Nov 19 '25 18:11 edubxb