Open mykter opened 3 years ago
@mykter I think this has to do with 22
not being in quotes. Once the quotes are added, the result fails:
test_services_not_denied {
deny["Cannot expose one of the following ports on a LoadBalancer [22]"] with input as {"kind": "Service", "metadata": { "name": "sample" }, "spec": { "type": "LoadBalancer", "ports": [{ "port": "22" }]}}
}
The test does indeed need to be updated
(just in case it got lost in the discussion of the test: the important part of this report is the incorrect behaviour of verify --data
)
I added #536 to hopefully clarify the behavior. Looks like the docs on the website showcase the current behavior: https://www.conftest.dev/options/#-data
At the moment the folder itself doesn't come into play, it's more on the structure of the data. If this differs from OPA behavior it would probably be worth looking into. I personally haven't used the --bundle
flag from the OPA binary that much.
Do you have an example OPA and Conftest command that differs so I can compare the behavior? Everytime I try and add multiple bundles I'm seeing a root conflict error.
Here's a fairly minimal example of the difference in behaviour of opa's bundles vs conftest's --data.
$ cat a/dir/data.json
{"v":1}
$ cat a/p.rego
package main
import data.dir
deny[msg] {
not dir.v == 1
msg := "couldn't load data via dir"
}
import data.v
deny[msg] {
not v == 1
msg := "couldn't load data via v"
}
$ conftest test --policy=a --data=a <(echo 1)
FAIL - /dev/fd/63 - main - couldn't load data via dir
2 tests, 1 passed, 0 warnings, 1 failure, 0 exceptions
$ opa -b a eval data.main.deny
{
"result": [
{
"expressions": [
{
"value": [
"couldn't load data via v"
],
"text": "data.main.deny",
"location": {
"row": 1,
"col": 1
}
}
]
}
]
}
To represent what you're trying to do in Conftest, it would be: conftest test a/dir/data.json -p a
Typically you'll pass in the configuration that you're trying to test (data.json) and tell conftest the policy directory. With opa eval
you're telling OPA to evaluate data.main.deny
which conftest does by default.
Ah sorry I wasn't clear, the data.json is meant to be an example of static data that the policy refers to, as opposed to the input document. I just didn't refer to "input" or have an input doc to conftest for simplicity. The intent was to show that in conftest the first rule matches and the second doesn't, with opa it's the other way round.
Yeah I see, the --data
for Conftest will take the data files passed in and make them accessible based on their keys. So in this case, {"v":1}
would be accessible from data.v
not data.dir
.
If the desire is to have it indexed based on the directory, I'd need to think through it some more as I haven't seen many questions or issues come up about conftest's data parameter.
@anderseknert are you familiar with OPA and using bundles? There does appear to be a difference with how Conftest and OPA handle external data, but I'm not familiar with them enough to know if the difference is warranted or if making these behaviors consistent is something we should do.
Yeah, OPA will load any JSON/YAML data (obviously not policies as those have a package defined) on the (relative) path under the which it was read. The only difference I know of in behavior as far as --bundle
is concerned here is that only files named data.json/yaml
will be considered in that case.
Whether it makes sense to preserve this behavior for conftest verify
you're probably better equipped to tell, but I helped (or well, half-failed to help) another user with this exact issue on Slack the other day, so either the behavior should change or the docs should.
Thanks! The documentation has been updated to reflect reality. I'll play around with --data and --bundle and see if it makes sense to change. The fear being that I imagine it would be a breaking change, and want to make sure we're actually solving a problem and not a symptom of poor docs 😅
Yeah, I guess a sensible middle ground could be to keep current behavoir while introducing the --bundle
flag for conftest too, and have that act exactly as it does for OPA? There are already plans/WIP to push more for bundle based policy and data packaging in the OPA docs and so on, so it would make it easier for user to jump seamlessly between the two tools.
Yeah, I guess a sensible middle ground could be to keep current behavoir while introducing the
--bundle
flag for conftest too, and have that act exactly as it does for OPA? There are already plans/WIP to push more for bundle based policy and data packaging in the OPA docs and so on, so it would make it easier for user to jump seamlessly between the two tools.
That's honestly not a bad idea either. Love it.
Then come v1.0.0 we could consider removing --data
if --bundle
does everything we want it to.
@mykter @anderseknert I just noticed that OPA also has a --data
flag. Using the above example, if you use OPA's --data
flag the example is the same as Conftest. i.e.
echo {} | conftest test --policy=a --data=a -
opa -d a/dir eval data.main.deny # -b also works here as well.. whats the difference between bundle and data?
So that behavior seems consistent. The dir is slightly different, because Conftest traverses all children.
That said, if the above finding is correct, this issue then is more about Conftest supporting --bundle
as well as --data
I've just ran into this and am perplexed. I've spent a bunch of time wrapping my head around how opa
loads data files based on the relative path passed and where the file was found walking towards it. Now that my head is wrapped around it, I came to find that conftest just ignores that and loads them directly in data. The main problem being is that in order to validate/develop in opa, and then move to conftest for usage, I have to rewrite my imports to account for each tool loading differently.
I have a policy directory that looks like
├── policy
│  ├── escalation
│  │  ├── infra
│  │  │  ├── escalate_changes.rego
│  │  │  ├── escalate_changes_mock.json
│  │  │  └── test_escalate_changes.rego
│  │  └── security
│  ├── standard
│  │  ├── aws_iam.rego
│  │  ├── aws_iam_test.rego
│  │  ├── aws_security_groups.rego
│  │  └── aws_security_groups_test.rego
│  └── tools.rego
opa test policy
gives escalate_changes_mock.json
the path of data.escalation.infra
where conftest verify -d policy
assigns the data directly in data
This means the tests that use the data mock have to be changed depending on if the person is working currently in opa or conftest
#import data.escalation.infra.escalate_changes_mock as mock
# conftest
import data.escalate_changes_mock as mock
is now at the top of my test file to accommodate for this. If I remember I can also force opa to shorten the path but then the command becomes quite hard to remember opa test policy/tools.rego policy/escalation/infra
and doesn't run the all of the tests unless each folder is explicitly passed
Any advice on this? We would prefer this path-based data structure instead of playing around with our data and merging it into a single file.
Reusing the bundle loader from OPA should not be too complicated, I think. If you want to give it a shot :)
The docs say that data in a folder "exceptions/a.json" will be exposed as "data.exceptions".
The example, which is the tested behaviour, shows that it is actually available directly under "data".
I think the test logic is inverted - it is checking that running
conftest test
gives an error, but it should be checking that it doesn't:Am I right in thinking that
opa test --bundle <path>
should be equivalent toconftest verify --policy <path> --data <path>
? My tests are working using this opa command at any rate!