gatekeeper icon indicating copy to clipboard operation
gatekeeper copied to clipboard

rego.v1 syntax

Open sergii-auctane opened this issue 1 year ago • 29 comments
trafficstars

Hi, sorry if it's a wrong template, i wasn't sure if it's a bug or a feature request. As the OPA which runs inside of the gatekeeper container supports rego.v1 syntax i consider this more like a bug, as the only limiting factor from support rego.v1 is rule-schema at first look.

But feel free to change the label, if you think it's a feature request.

What steps did you take and what happened: I create a ConstraintTemplate which contains import rego.v1 line.

import rego.v1

violation[{"msg": msg}] if {
...
}

and get error in ConstraintTemplate status

    - errors:
      - code: ingest_error
        message: 'Could not ingest Rego: invalid ConstraintTemplate: invalid rego:
          invalid module: missing required rules: [violation]'

What did you expect to happen: I expect it would parse the rule as it supports rego.v1 syntax as if i remove if from violation[{"msg": msg}] if { i get another error, which can exist in v1 only:

    - errors:
      - code: ingest_error
        message: |-
          Could not ingest Rego: invalid ConstraintTemplate: 2 errors occurred:
          template:6: rego_parse_error: `if` keyword is required before rule body
          template:6: rego_parse_error: `contains` keyword is required for partial set rules

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

Environment:

  • Gatekeeper version: 3.17.1
  • Kubernetes version: (use kubectl version):
Client Version: v1.30.0
Server Version: v1.29.7-eks-a18cd3a

sergii-auctane avatar Oct 08 '24 16:10 sergii-auctane

@srenatus

Does using v1 syntax change the rule name somehow? Here is how Gatekeeper analyzes Rego's rules for presence (via Rego's parsing library):

https://github.com/open-policy-agent/frameworks/blob/e84361fed75850acbbc0293f0b47df0f5ad6176c/constraint/pkg/client/drivers/rego/driver.go#L439-L462

maxsmythe avatar Oct 09 '24 03:10 maxsmythe

Yeah that code will need updating, I believe, but not just for rego.v1. With our without that import, you can have a rule like

foo[x].bar[y].baz { ... }

and it's not clear what its name would be. You should try using .Ref() instead.

srenatus avatar Oct 09 '24 06:10 srenatus

Thanks!

It looks like Ref is a list of Term... is that correct?

https://github.com/open-policy-agent/opa/blob/78a73025233189adf1573b483dcc4742b7673944/ast/term.go#L880

What does that look like for a more complex reference like what you cited above? I.e. how are brackets handled?

Is it possible to have a tree-like structure to these?

e.g.

foo[bar[x]].baz[y] {}?

maxsmythe avatar Oct 16 '24 00:10 maxsmythe

Ref is []*Term, yeah, but the possible ref-heads of rules are more limited. I can't find a location right now, but it should be safe to assume that it's only strings and vars. @johanfylling do you know where we enforce that?

srenatus avatar Oct 16 '24 08:10 srenatus

I don't think we do much more enforcement than requiring it to be a valid ref. Any terms containing other things than scalar values and vars - like refs and calls - will be moved into the body and replaced with vars by the compiler. So a compiled rule head's ref should only contain strings and vars, yes; but one that has only been through the parser, I'd not expect to be so clean.


violation is supposed to be a partial rule? I.e. it's building a set?

violation[{"msg": msg}] if {
...
}

is semantically equivalent to

violation[{"msg": msg}] := true if {
...
}

and will create an object at violation with key {"msg": msg} and value true if the rule validates.

To create a set-building partial rule in v1 you need to use the contains keyword:

violation contains {"msg": msg} if {
...
}

The rule name will only be available for rules where a name can be inferred; which excludes rules with multiple ref terms in the head. Non-ref-head complete rules (violation if {...}) and non-ref-head partial rules (violation contains x if {...}, violation[x] := y {...}) should be assigned a name I think, though.


Tree structures can be constructed through rules with multiple variables in the rule head's ref.

foo[bar[x]].baz[y] if {
...
}

is indeed a valid rule head.

johanfylling avatar Oct 16 '24 13:10 johanfylling

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 20 '24 01:12 stale[bot]

can you share a valid example or is this completely unusable ?

grosser avatar Dec 22 '24 22:12 grosser

tried this since it is valid v1 and in strict

violation contains {"msg": msg} if {

but still got "validation.gatekeeper.sh" denied the request: missing ConstraintTemplate: template "foo" not found

grosser avatar Dec 22 '24 23:12 grosser

tried this since it is valid v1 and in strict

violation contains {"msg": msg} if {

Is Gator 3.18 ready to evaluate the rego v1 syntax? Not sure if I'm doing wrong but migrating any code with functions like violation[{"msg": msg}] { results in errors:

  adding template: invalid ConstraintTemplate: 1 error occurred: template:3: rego_parse_error: unexpected : token: expected \n or ; or }
        violation contains {"msg": msg, "details": {}} if {
                                 ^

v0 minimal example

# cat test.rego
package k8sallowedrepos

violation[{"msg": msg}] {
  container := input.review.object.spec.containers[_]
  not allowed_image(container.image)
  msg := sprintf("container <%v> has an invalid image repo <%v>.", [container.name, container.image])
}

allowed_image(image) {
  strings.any_prefix_match(image, input.parameters.repos)
}
# opa check --strict test.rego --v0-compatible

migrated to v1

# opa fmt --v0-v1 test.rego
package k8sallowedrepos

import rego.v1

violation contains {"msg": msg} if {
        container := input.review.object.spec.containers[_]
        not allowed_image(container.image)
        msg := sprintf("container <%v> has an invalid image repo <%v>.", [container.name, container.image])
}

allowed_image(image) if {
        strings.any_prefix_match(image, input.parameters.repos)
}

This seems to be valid for opa but not for gator.

tberreis avatar Dec 23 '24 17:12 tberreis

where do you see these errors, I tried applying the template and it did not get rejected, is it in opa logs ?

grosser avatar Dec 23 '24 18:12 grosser

where do you see these errors, I tried applying the template and it did not get rejected, is it in opa logs ?

When running gator verify ./test/...

tberreis avatar Dec 23 '24 18:12 tberreis

ah missed that, we are using just opa test to run our tests

there seems to also be an issue where the actual update of the constrainttemplate is not validated but things then fail internally and we run into kubernetes saying the constraint does not exist

grosser avatar Dec 23 '24 20:12 grosser

I found the parse errors by tailing controller manager logs and editing the constrainttemplate, when using violation contains {"msg": msg} if { the only remaining error is

invalid ConstraintTemplate: invalid import: bad import: \"rego.v1\"",

grosser avatar Jan 02 '25 02:01 grosser

@grosser Same issue here. I've never been able to use import rego.v1 or any rego.v1 related syntax in a gatekeeper constraint template. I somehow assumed that was by design until I stumbled upon this GitHub issue 😅

netops2devops avatar Jan 15 '25 01:01 netops2devops

Folks, are there specific rego v1 features you need for your policies or is rego v0 sufficient?

ritazh avatar Jan 22 '25 22:01 ritazh

mostly want to use the new syntax to be consistent and on latest, no real new features I'm after

grosser avatar Jan 22 '25 23:01 grosser

I don't think it's so much about features per se as it is having a consistent experience when working with Rego for Gatekeeper policies vs Rego for other projects. Also, the OPA docs are all updated to demonstrate modern Rego, and new users turning to those docs to learn the language shouldn't have to first deal with the fact that the Rego in front of them looks very different compared to the Rego they see in the docs.

Since most users are moving to v1 and the transition is normally quite effortless (opa fmt), we should definitely support it here. And given that OPA 1.0+ (and tools like Regal) supports running v0 policies too, people can still use their v0 policies if they don't see an immediate need to migrate, or perhaps need more time doing so.

But as for features, I'm sure we'll find a few of them improve things as we work on migrating existing policies. I'll work with @charlieegan3 on helping you all out with that, and we can perhaps discuss the details further in a PR we submit? 🙂

anderseknert avatar Jan 23 '25 10:01 anderseknert

Thanks @anderseknert! Yes a PR would be great. The concern is having to support both rego v0 and v1 adds complexity in the code, user experience, and maintenance/support. We want to make sure there is enough user interest and benefits before jumping into that work.

ritazh avatar Jan 23 '25 15:01 ritazh

Understood! And we'll be happy to help support you in that effort 🙂

The complexity of the user experience is IMHO made worse by only supporting a syntax of Rego that people outside of this project haven't really been using for a long time, and which isn't featured in any recent documentation, blogs, examples, etc.

Oh well, we'll get to work on a few PR's to get the ball rolling 😃

anderseknert avatar Jan 23 '25 15:01 anderseknert

I have a PoC PR open here showing an update that parses input in both v0 and v1. This would be transparent to users and would allow them to use either version.

https://github.com/open-policy-agent/frameworks/pull/517

I am a little unsure what is required for regorewriter here, so any input on that part is appreciated. As is any pointers / ideas about the changes in the PR generally. I will pick it up next week.

charlieegan3 avatar Jan 23 '25 16:01 charlieegan3

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 24 '25 22:03 stale[bot]

It is torturing to code in OPA v.068 although I am pretty confident that OPA 1.x is still full of crazy syntax to drive devs and LLM nuts.

tom10271 avatar Mar 25 '25 10:03 tom10271

Hey @tom10271 🙂 we have put a lot of work into the language server and linter, Regal. It's not perfect but it's better than LLMs that much is true! 😅

On another topic, I think we're getting close with https://github.com/open-policy-agent/frameworks/pull/517 which gets us a big chunk closer to having this in GK.

charlieegan3 avatar Mar 25 '25 16:03 charlieegan3

Hello,

Is there any estimate date or version when rego v1 will be officially supported?

csapinoso avatar Mar 27 '25 18:03 csapinoso

We merged the PR https://github.com/open-policy-agent/frameworks/pull/517 @charlieegan3 mentioned and worked on. And brought the changes into Gatekeeper.

There is a release candidate out - https://github.com/open-policy-agent/gatekeeper/releases/tag/v3.19.0-rc.1 with OPA v1 support. Here is how you can enable v1 syntax - https://github.com/open-policy-agent/frameworks/pull/517#discussion_r1983826280.

Try it out and let us know! while we are working on docs.

JaydipGabani avatar Mar 27 '25 21:03 JaydipGabani

It would be great if we can have the command to install the RC in our K8s cluster? And Helm or kubectl applicable yml files? Thanks

tom10271 avatar Mar 28 '25 01:03 tom10271

v1 versions of all the bundled policies available in this PR: https://github.com/open-policy-agent/gatekeeper-library/pull/639 I hope we can get them integrated as an option at some point :)

anderseknert avatar Apr 07 '25 11:04 anderseknert

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 06 '25 15:06 stale[bot]

@tom10271 you should be able to use RC charts to install RC in k8s https://artifacthub.io/packages/helm/gatekeeper/gatekeeper/3.19.0-rc.2. Let me know if you are facing any trouble.

JaydipGabani avatar Jun 06 '25 16:06 JaydipGabani

I updated helm chart to 3.19.2

updated constraint template to opt-in to rego v1 by explicitly adding:

  targets:
    - target: admission.k8s.gatekeeper.sh
      code:
      - engine: Rego
        source:
          version: v1
          rego: |

Updated rego policies to v1 syntax, and get this error:

2025-07-15T16:03:56.982Z	error	Reconciler error	{"controller": "constrainttemplate-controller", "object": {"name":"k8srequiredresources"}, "namespace": "", "name": "k8srequiredresources", "reconcileID": "acdf5dc4-748e-4bb3-ba06-6a28140f0145", "error": "invalid ConstraintTemplate: invalid import: bad import: \"rego.v1\""}

if i remove import it seems to work fine.

In the docs, it says no need to use import rego.v1, but no need doesn't mean must not use, so I would like to stick to this. My rego policies are stored in individual rego files, and I use the VS Code Linter plugin. Do I have to remove import rego.v1 every time now after editing the policies ? It also breaks the CI part which includes linting.

sergii-auctane avatar Jul 15 '25 16:07 sergii-auctane