Add support for multiple policy directories

[edubxb]

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.

[bendrucker]

Initial notes in no particular order:

• See failing test
• Link to documentation for other OPA tools to demonstrate that accepting multiple directories is a standard/common pattern
• 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

As long as you can address those this seems reasonable

[bendrucker]

Avoid unnecessary length checks before loops. You can just declare var dirs []string in the function scope. range c.PolicyDirs, appending in the loop, and then if len(dirs) after that, return.

Same thing for env.

strings.Split(os.Getenv("TFLINT_OPA_POLICY_DIRS"), ",")

Just range over that, if the env is "", you're ranging over an empty slice. And again return dirs if len(dirs) > 0 after that loop too.

[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]

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

[bendrucker]

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.

[edubxb]

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.

[bendrucker]

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]

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.

[edubxb]

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