Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rego.v1 syntax #3577

Open
sergii-auctane opened this issue Oct 8, 2024 · 13 comments
Open

rego.v1 syntax #3577

sergii-auctane opened this issue Oct 8, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@sergii-auctane
Copy link

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 sergii-auctane added the bug Something isn't working label Oct 8, 2024
@maxsmythe
Copy link
Contributor

maxsmythe commented Oct 9, 2024

@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

@srenatus
Copy link
Contributor

srenatus commented Oct 9, 2024

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.

@maxsmythe
Copy link
Contributor

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] {}?

@srenatus
Copy link
Contributor

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?

@johanfylling
Copy link

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.

Copy link

stale bot commented Dec 20, 2024

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 stale bot added the stale label Dec 20, 2024
@grosser
Copy link
Contributor

grosser commented Dec 22, 2024

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

@grosser
Copy link
Contributor

grosser commented Dec 22, 2024

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

@stale stale bot removed the stale label Dec 22, 2024
@tberreis
Copy link
Contributor

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.

@grosser
Copy link
Contributor

grosser commented Dec 23, 2024

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

@tberreis
Copy link
Contributor

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/...

@grosser
Copy link
Contributor

grosser commented Dec 23, 2024

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
Copy link
Contributor

grosser commented Jan 2, 2025

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\"",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants