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

docs: Grammar definition does not allow assignment or unification in query #4691

Closed
mmzeeman opened this issue May 15, 2022 · 14 comments
Closed
Labels

Comments

@mmzeeman
Copy link
Contributor

Grammar definition does not allow assignment nor unification in query.

https://www.openpolicyagent.org/docs/latest/policy-reference/#grammar

Definition for query makes it impossible to add the assignment rule to the query production rules do not include assignment operators (:=), or unification (=) which you also commonly see in the query part of the rule body.

This makes it impossible to parse packages like these if you follow to production rules in the documentation.

package play

default hello = false

hello {
    m := input.message
    m == "world"
}

Not sure what it should be though. An extra rule maybe to include assignment and unification? Is there a grammar file which is used in practice?

@mmzeeman mmzeeman added the bug label May 15, 2022
@srenatus
Copy link
Contributor

Is there a grammar file which is used in practice?

This is the root of the problem here -- that grammar is a dead weight in our docs. It doesn't get used or tested well. I'm not aware of any such grammar, however. 😞

Now to your issue, isn't unification covered by

expr-infix      = [ term "=" ] expr infix-operator expr

? But I'm quite certain we're missing a rule for :=. Perhaps this would do the trick? 👇

expr-infix      = [ term ( ":=" | "=" ) ] expr infix-operator expr

@anderseknert
Copy link
Member

Not the same, but there's at least this grammar being used in a real project, if that helps.

@mmzeeman
Copy link
Contributor Author

I don't think so. That means that you will then always have to include a infix-operator and another term in an assignment. Which is just weird.

In the old peg grammar it was defined as:

TermExpr <- negated:NotKeyword? value:LiteralExpr with:WithKeywordList? {
     return makeLiteral(negated, value, with)
 }

 LiteralExpr <- lhs:ExprTerm rest:( _ LiteralExprOperator _ ExprTerm)? {
     return makeLiteralExpr(currentLocation(c), lhs, rest)
 }

 LiteralExprOperator <- val:( ":=" / "=" ) {
     return makeInfixOperator(currentLocation(c), c.text)
 }

The expr-literal seems to be missing from the ebnf grammer in the documentation.

I was simply looking for a grammer I could use for an internal test. (Tight integration of rego into a non-go system)

@mmzeeman
Copy link
Contributor Author

mmzeeman commented May 16, 2022

Not the same, but there's at least this grammar being used in a real project, if that helps.

That grammar has an obvious error in the import rule.

(Update) Oops, sorry, no... That grammar is ok... Yesterday I made a PR for the grammar.

If this grammer is tested, maybe it is better to include it in the documentation somehow. Every newbie who is going to write a policy will look at it.

That grammar also has the := and = literal-expr rules.

@anderseknert
Copy link
Member

That grammar is used in the IDEA editor plugin for Rego, so any possible mistake (and I'm sure they exist!) should be easy to test just by writing a snippet of code in that editor with the plugin activated. Issues / PRs for that are generally dealt with quickly by @vgramer .

If this grammer is tested, maybe it is better to include it in the documentation somehow. Every newbie who is going to write a policy will look at it.

Yeah, that could be an idea (no pun intended!)... although I'm not sure that's really where most newbies go to learn about Rego 😃

That grammar also has the := and = literal-expr rules.

I'm not sure I follow :)

@mmzeeman
Copy link
Contributor Author

The ebnf grammar in opa's documentation does not have production rules for using := and = in queries.

@srenatus
Copy link
Contributor

If this grammer is tested, maybe it is better to include it in the documentation somehow. Every newbie who is going to write a policy will look at it.

💯 Not sure if everyone is actually starting with the grammar, but I'm sure some folks are. And having a grammar that's a living, breathing artifact is much preferable -- it'll also make sure that we update it as needed (e.g. by #4660).

Let's do that. Scratch the EBNF we have right now and instead link to the idea plugin grammar; or copy a section of it into OPAs docs. (We could even include railway diagrams, like generated by https://www.bottlecaps.de/rr/ui.)

@srenatus
Copy link
Contributor

@mmzeeman btw what are you up to? I'd love to know more about the project you're working in. (Please give me an excuse to write some Erlang again... 🙃)

@mmzeeman
Copy link
Contributor Author

@srenatus I'm doing a bit of research into policy languages, to see if we can integrate it into https://github.com/zotonic/zotonic.

We have a (sort of) build-in RBAC model model, but when projects increase in size we always hit a limit of what it can handle. Not speed, but complexity. In those situations you want to be able to store your access rules in git, and be able to let somebody reviewed the policy rules, without requiring them to learn Erlang first.

In the past I have done research into delegation in policy languages for distributed systems. I was first looking into XACML. On the erlang forum someone suggested I should take a look at opa.

Because acl decisions are made during rendering of templates. We check if users are allowed to access resources, of which there can be multiple on a single page, we really need something which is integrated into erlang itself. So I was looking to see if we could easily parse rego, and maybe compile the policies to beam code.

@srenatus
Copy link
Contributor

Oh that's cool. I think there are multiple ways to plug OPA into zotonic, depending on your constraints and requirements.

  1. HTTP call or opa eval shell out -- the simplest approach can go a long way
  2. Using Wasm -- opa build can create a wasm module that can be used with a minimal shim from any language that has some wasm support libraries. I would think that wasmtime should do the trick, but I haven't used it before myself.
  3. Using OPA's parser and evaluator via Go-used-as-C-library. You'd define a shim .go file, with some entrypoint that you could then call via the appropriate wrapping; a NIF or more likely a port.

I think all of these would be preferable to interpreting the parsed rego yourself. If you really want to go that route, see opa parse, it'll give your a JSON representation of the AST so you don't have to deal with the grammar. Also opa build -t plan can give you a procedural "query plan" that is the IR used when compiling OPA policies to wasm -- you could interpret that instead; it might be much more straightforward because it's simpler: it doesn't deal with object comprehensions or any such thing, just straight data lookups, functions calls.

What is best depends on how your policy files change. If you'd envision a fixed policy (not having any user-facing Rego code), then you can go with the "more static" approaches like wasm; or generating some Erlang (or Beam) code from the IR.

HTH 😄

@srenatus
Copy link
Contributor

srenatus commented Jun 8, 2022

@mmzeeman How's it going? 😃

@mmzeeman
Copy link
Contributor Author

mmzeeman commented Jun 8, 2022

Ah, we can close this issue.

FYI I've a working rego parser in erlang, looking into compiling policies to beam code, as we really can't integrate via external services or nif call. It will be too slow, but also we need also need to be able to retrieve data from the system itself. Progress is a bit slow, because I do it as a side project right now. We are bumping into the acl issue more often then we like to admit. For my master thesis I researched and implemented a policy engine for distributed object applications, so it is kind of familiar territory.

@srenatus srenatus closed this as completed Jun 8, 2022
@srenatus
Copy link
Contributor

@mmzeeman It's been a while and I just today fondly remembered Erlang for one reason or another. Do you still pursue this project? 😃

@mmzeeman
Copy link
Contributor Author

Yes, I'm still considering it. Although I'm also looking at using cue (https://cuelang.org) in a security context. It all depends a bit on priorities though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants