Feature: support ruletype definitions as rego + metadata#6558
Feature: support ruletype definitions as rego + metadata#6558evankanderson wants to merge 6 commits into
Conversation
|
Coverage is 60.772% — evankanderson:rego-style-rules into mindersec:main. No base build found for |
evankanderson
force-pushed
the
rego-style-rules
branch
from
8a62d58 to
227667c
Compare
| if proj != "" { | ||
| if r.Context == nil { | ||
| r.Context = &minderv1.Context{} | ||
| for _, resource := range resources { |
There was a problem hiding this comment.
Could we return an error when an explicitly provided file contains no resources? ReadFromPath returns an empty slice on EOF, so an empty YAML or Rego file currently makes ruletype create/apply exit successfully without doing anything. That feels especially confusing for automation.
There was a problem hiding this comment.
Yes, good point!
Added a test for this case as well.
| # eval: | ||
| # rego: | ||
| # type: 'deny-by-default' | ||
| package minder |
There was a problem hiding this comment.
This second package minder makes the fixture invalid Rego; opa fmt --fail reports unexpected package here. Could we move the metadata before the original package declaration and keep only one package statement? The duplicate fixture under cmd/cli/app/fixture has the same problem.
There was a problem hiding this comment.
Yeah, it turns out that the metadata comment after the package makes the annotation technically associated with the import line rather than the package line.
I ran opa check and opa fmt on all the sample rego files.
| "repo": input.properties["github/repo_name"], | ||
| "pr": input.properties["github/pr_number"], | ||
| }) | ||
| comment contains "badword" |
There was a problem hiding this comment.
This fixture still fails opa check. With import rego.v1, the rule on line 30 needs the v1 contains ... if form, and line 36 should use contains(comment, "badword"). Could we make the fixture valid so the successful conversion test represents a rule the server can actually accept?
There was a problem hiding this comment.
Yes on making the fixture valid. I don't really want the client to actually check the Rego with an embedded toolchain, because that may end up blocking valid future rules if the server is upgraded, but the user is using an old client. (As someone who has multiple gardens of tools on different machines, I often end up with out-of-date tools somewhere.)
evankanderson
force-pushed
the
rego-style-rules
branch
from
1b0abd8 to
b2f0f3d
Compare
|
It's good to go I think |
4 participants
Summary
I've talked about this for several months, but some of the recent Rego v1 migrations made me pull the trigger.
This allows Rego ruletypes to be defined as standard
.regofiles (so thatopa fmt, etc will work on them), and includes the Minder Ruletype metadata via the OPA metadata convention.Testing
Added integration tests for the various Minder commands.
Still TODO: documentation for the different file formats supported by Minder (DataSources, Profiles, and two filetypes for RuleTypes).