Compile API and negation #2045
Comments
|
Just as a side-note -- have you tried a simple not-equal there? |
|
@srenatus Yes and it works fine. The fact is those are just dummy tests. |
|
Ok cool. Sorry for the noise. :)
…On Wed, 29 Jan 2020, 20:21 Antonio Pantano, ***@***.***> wrote:
@srenatus <https://github.com/srenatus> Yes and it works fine. The fact
is those are just dummy tests.
I'm trying to translate the query results in a database query and I was
just trying to translate the not operand.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2045?email_source=notifications&email_token=AAGUR3U7S3GBDJBSD7E2SSTRAHJMTA5CNFSM4KNI2KX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKINKOI#issuecomment-579917113>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGUR3TVZSEHCKFGHC7PWQLRAHJMTANCNFSM4KNI2KXQ>
.
|
|
@srenatus Hey no noise at all :-) . Thank you for your interest. |
|
Sorry, another side-note: Did you by any chance mean Probably also irrelevant for the question, so please bear with me once more 😄 |
|
Playing around here a little on the REPL, with your code (using and running note that However, I'm not sure if this helps you any further. I tend to appreciate the REPL when it comes to partial eval, if only for pretty-printed queries. I also don't understand (yet) why the Compile API would give you something different. 🤔 |
|
Hmm interesting, running the same ...which closely matches the REPL output I've posted above. Not sure what's going on here. We might be using different versions? 💭 |
|
Ok posted too fast once more. You're probably setting |
|
@srenatus thanks for following up. Regarding the negation those are the data I'm using for my tests: policy.rego input: I'm running |
|
Opa version is 0.16.2 |
|
@srenatus However I guess the way you've added the |
|
Thanks for sharing more details 👍 With that, I was able to reproduce what you had initially posted. Running the same stuff using ( So, what does this suggest for how to proceed? I'm not certain, it's been a while since I've looked at the data filtering examples. I'd start looking for insights here, https://github.com/open-policy-agent/contrib/blob/master/data_filter_example/data_filter_example/opa.py#L39, if you haven't already. |
|
Gave it a quick go, but I just ran out of time. With your |
|
Thank you so much @srenatus. I don't know if we have a different opa version but i'm getting $_term_1_21 and not the underscore as in you example. |
|
Adding to this, I've tried using The rendered query then becomes and the python translation fails like this: which I'd take to mirror my confusion on how to translate that query 1 into SQL. 💭 |
|
Yes. That's a problem of the python translator program but the query returned by opa is correct. I guess it's because the array type is not handled in the python code. I'm using code written by myself in typescript and I've added the case of sets and arrays(as well as concat, startsWith, endsWith and others.. all working fine). |
|
Unfortunately I don't know Go language to understand the code deeply but I was looking at this line here Line 3466 in 12ff9c0 Is it correct the way the ast is parsed for the not keyword ? |
|
This looks relevant but it was fixed some time ago: #1814 |
|
ℹ️ FWIW, the compile API returns Also, this still does seem like a bug: what is |
|
It should be |
|
@srenatus basically it should be the same as explained here https://www.openpolicyagent.org/docs/latest/policy-language/#negation |
Yup, I'm not sure about the semantics of Also DB-wise, I'm curious (genuinely!) how you're translating query 1. Assuming the Since in rego, these are implicitly AND'ed together, this would roughly mean "allow is true if"
Ah, the IMHO perhaps contradictory requirements in query 1 come from those lines (cut out): (Please note that I'm not trying to be annoying on purpose, but I'm kinda interested in this case now 😅 Even more so since you've taken your own route with a TS implementation.) |
|
The idea is to generate a custom tree from the opa results that allows one to build queries for different database engines. For now I'm getting something like this from the code I wrote: this would allow (hopefully) each database adapters to build their own specific query. As a result, with a basic sqlAdapter i'm working on now, this allows me to generate this: I've found a couple of example with node JS over the Internet where I've got the idea, so I decided to go on my own trying to make it database agnostic. But there is that In a couple of projects I'm working on it is required to give permission to access some resources not only based on the user role and permissions (known as RBAC) but based on attributes (ABAC), something like users can access if their IP is x or is location is from y. |
|
@ovidius72 sorry for the delayed response. To answer your original question, yes you can partially evaluate policies that use negation. If a negated expression contains an unknown value, it will saved. If the negated expression depends on unknowns, transitively, (e.g., support you wrote Can you share the parameters you used to generate the partial eval result in the original issue description? When I run partial eval with the input from the comment above the output is better: Output $ opa eval -d simple.rego -p -u data.keypads -i input.json.1 'data.foo.allowed[x]' -f source
# Query 1
x = data.keypads[_]
x.group = {"C", "D", "E"}
not x.name = "example"
xPolicy package foo
allowed[keypad] { #Company C allows only members from C, D and E
re_match("C", input.subject.group)
keypad := data.keypads[_]
g := {"C", "D", "E"}
keypad.group == g
not keypad.name == "example"
}Input {
"action": {
"type": "READ"
},
"env": {
"domain": "D",
"ip": 123,
"strings": [
"str1",
"str2"
]
},
"method": "GET",
"path": [
"keypads"
],
"subject": {
"group": "C",
"isMale": false,
"location": "ROME",
"role": "not_admin",
"user": "Jack"
}
} |
|
Thank you @tsandall You are asking to run for the Here are my results: OR |
|
I'm seeing the same behaviour. Adding a simple allow rule reproduces the problems: $ opa eval -d simple.rego -p -u data.keypads -i input.json.1 'data.foo.allow=true' -f source# Query 1
data.keypads[_].group = {"C", "D", "E"}
not _.name = "example"There are two problems with this output. The first issue is that However, the second issue is that the {
"partial": {
"queries": [
[
{
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "ref",
"value": [
{
"type": "var",
"value": "data"
},
{
"type": "string",
"value": "keypads"
},
{
"type": "var",
"value": "$02"
},
{
"type": "string",
"value": "group"
}
]
},
{
"type": "set",
"value": [
{
"type": "string",
"value": "C"
},
{
"type": "string",
"value": "D"
},
{
"type": "string",
"value": "E"
}
]
}
]
},
{
"index": 1,
"negated": true,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "ref",
"value": [
{
"type": "var",
"value": "$_term_1_21"
},
{
"type": "string",
"value": "name"
}
]
},
{
"type": "string",
"value": "example"
}
]
}
]
]
}
}simple.rego: package foo
allow {
input.action.type == "READ"
input.path == ["keypads"]
allowed[keypad]
}
allowed[keypad] { #Company C allows only members from C, D and E
re_match("C", input.subject.group)
keypad := data.keypads[_]
g := {"C", "D", "E"}
keypad.group == g
not keypad.name == "example"
}input.json.1: {
"action": {
"type": "READ"
},
"env": {
"domain": "D",
"ip": 123,
"strings": [
"str1",
"str2"
]
},
"method": "GET",
"path": [
"keypads"
],
"subject": {
"group": "C",
"isMale": false,
"location": "ROME",
"role": "not_admin",
"user": "Jack"
}
} |
In copy propogation we ran into issues where an expression was negated and required a variable in it to show up in a non-negated expression to be safe. We had the binding for the var and could have created it but there were two issues: 1) We didn't resolve the variable binding for refs in negated expressions. This left the "wrong" thing in the result. 2) Once we had the unified representation for the value in the result we needed to ensure that not only was the binding contained in the result somewhere, but it needed to appear in a non-negated expression. The copy propagation code will then add an equality expression for bindings that are left over and that are not safely contained in the result. However, we don't always need the full equality expression. If the key isn't found anywhere in the result we don't need the lhs of the equality. We are essentially adding a variable that will be ignored when all we care is that the rhs is defined. So now we will only generate an expression for the rhs in those cases. Fixes: open-policy-agent#2045 Signed-off-by: Patrick East <east.patrick@gmail.com>
In copy propagation we ran into issues where an expression was negated and required a variable in it to show up in a non-negated expression to be safe. What we do now is expose some of the safety helpers from the ast package and check if any of the removed expressions provided safety to any vars left in the result (that are otherwise unsafe). If they do we will re-add the expression. This isn't done until after computing the initial result to ensure that we get the minimal set of expressions. If we attempt to check eagerly, before we kill/remove any, we might let one remain only to later add an expression to the result that makes it no longer used. The current pattern of removing as aggressively as possible and then taking another pass over the removed expressions to re-add seems to be the most optimal approach. Fixes: open-policy-agent#2045 Signed-off-by: Patrick East <east.patrick@gmail.com>
|
I'm sorry to reopen a very old issue. However, a couple of days ago I started to work again on the project from where I found that issue. I haven't had the chance to check it at that time. Even with the latest part of the compile api response. thanks. |
Can the negation be used with the compile APIs for checking the value of an unknown variable ?
returns the following result:
The name of the unknown value is "$_term_1_21" in the second query with the negation. In the first query the unknown value has been replaced with the value from the payload.
As for the documentation
For safety, a variable appearing in a negated expression must also appear in another non-negated equality expression in the rule.. Is this the reason it is not correctly evaluated ?The text was updated successfully, but these errors were encountered: