tflint-ruleset-opa
tflint-ruleset-opa copied to clipboard
Add support for multiple policy directories
Motivation
At work, we are using rego for validating our terraform code.
Our approach is:
- Use tflint + the opa plugin for validating the HCL code with rego policies.
- 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.
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!
Regarding other tools supporting loading policies from different directories, the "official ones" from Open Policy Agent, allow it:
conftest→ https://www.conftest.dev/options/#-policyopa(cli) → https://www.openpolicyagent.org/docs/cli#eval
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.
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.
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.
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.
I've been a little busy, so if you want to take over this, feel free! Thanks, @bendrucker.