Incoherent result of opa compile response
Closed this issue · 8 comments
Short description
We use OPA to a subject until a certain date. We use the time.now_ns() to determine whether there is access.
We have a 3 layer deep data structure. We use the compile query in two ways: one way that works is if we have the second layer given and the third layer as unknown, the values of the third layer are correctly returned according to the current date and the date in the data.
However when we have the second layer as unknown, the compile query returns all the entries that have an entry of the third layer 'under them', not accounting the date parameter. It just returns the result and it gives the date until it is valid. I assumed the date would be evaluated and the 'expired' values to not be returned.
Steps To Reproduce
I have a rego:
package nurse.nurseLink
import future.keywords
has_nurse_link := x if {
end_date := data.nurseData[input.tenantId].nurseLink[input.practitionerRoleResourceId][input.patientResourceId].endDate
due_date := time.parse_rfc3339_ns(sprintf("%vT23:59:59Z", [end_date]))
now := time.now_ns()
x := now <= due_date
}
and data
{
"62229a16-777f-433a-9d15-8c5a4315f92d": {
"nurseLink": {
"9b49d1c5-0f68-4f40-9aa5-3b97d0e54c03": {
"0f1ab1de-2c1c-494b-b5f0-5d5bcb60a41e": {
"endDate": "2023-01-01"
}
}
}
}
}
When I execute the compile query:
{
"query": "data.nurse.nurseLink.has_nurse_link == true",
"input": {
"tenantId": "62229a16-777f-433a-9d15-8c5a4315f92d",
"patientResourceId": "0f1ab1de-2c1c-494b-b5f0-5d5bcb60a41e"
},
"unknowns": [
"input.practitionerRoleResourceId"
]
}
Expected behavior
I expect that the result is an empty json, because the has_nurse_link would never return true
Actual behaviour
A result containing the "9b49d1c5-0f68-4f40-9aa5-3b97d0e54c03" is returned with a date query as well:
{
"result": {
"queries": [
[
{
"index": 0,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "eq"
}
]
},
{
"type": "string",
"value": "9b49d1c5-0f68-4f40-9aa5-3b97d0e54c03"
},
{
"type": "ref",
"value": [
{
"type": "var",
"value": "input"
},
{
"type": "string",
"value": "practitionerRoleResourceId"
}
]
}
]
},
{
"index": 1,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "lte"
}
]
},
{
"type": "call",
"value": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "time"
},
{
"type": "string",
"value": "now_ns"
}
]
}
]
},
{
"type": "number",
"value": 1672617599000000000
},
{
"type": "boolean",
"value": true
}
]
}
]
]
}
}
time.now_ns is not evaluated when doing PE. It's assumed that users want the time as if the actual eval, not the partial one.
time.now_ns is not evaluated when doing PE. It's assumed that users want the time as if the actual eval, not the partial one.
Isn't that a bit weird? The partial evaluation is at a given time, not at a potential time in the future/past. That's why the now_ns() function exists right?
The only solution then to solve this is to have the now_ns() as an input instead of the built-in function?
PE is often used for optimization. In that case, it does make sense, doesn't it? When building an optimized bundle, would you want the time at that then bundle is built to matter for the later policy eval..? 🤔
PE is often used for optimization. In that case, it does make sense, doesn't it? When building an optimized bundle, would you want the time at that then bundle is built to matter for the later policy eval..? 🤔
No, not at all. Didn't know the time.now_ns() worked like that. I thought it would just refer to a method call when doing the evaluation/partial evaluation of the rego. But it inserts the now_ns() at the time of bundling then?
What you see in the response,
{
"index": 1,
"terms": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "lte"
}
]
},
{
"type": "call",
"value": [
{
"type": "ref",
"value": [
{
"type": "var",
"value": "time"
},
{
"type": "string",
"value": "now_ns"
}
]
}
]
},
{
"type": "number",
"value": 1672617599000000000
},
{
"type": "boolean",
"value": true
}
]
}
]is, in source code form, and simplified,
time.now.ns() <= 1672617599000000000so, the RHS is coming from the input that's declared as known at PE-time. The LHS is the call to time.now_ns() that PE never evaluates.
There's a short list of builtins that PE will never touch: https://github.com/open-policy-agent/opa/blob/main/ast/builtins.go#L308
It's ugly that those are hard-coded, since you have presented a use case where you actually do want it to be evaluated. So, I think it would make for a good enhancement to allow for controlling those builtins for the Compile API.
Please note that the linked thing is a package-level variable. So if you're calling OPA through from a golang lib, you could change that list.
What you see in the response,
{ "index": 1, "terms": [ { "type": "ref", "value": [ { "type": "var", "value": "lte" } ] }, { "type": "call", "value": [ { "type": "ref", "value": [ { "type": "var", "value": "time" }, { "type": "string", "value": "now_ns" } ] } ] }, { "type": "number", "value": 1672617599000000000 }, { "type": "boolean", "value": true } ] } ]is, in source code form, and simplified,
time.now.ns() <= 1672617599000000000so, the RHS is coming from the input that's declared as known at PE-time. The LHS is the call to
time.now_ns()that PE never evaluates.There's a short list of builtins that PE will never touch: https://github.com/open-policy-agent/opa/blob/main/ast/builtins.go#L308
It's ugly that those are hard-coded, since you have presented a use case where you actually do want it to be evaluated. So, I think it would make for a good enhancement to allow for controlling those builtins for the Compile API.
Please note that the linked thing is a package-level variable. So if you're calling OPA through from a golang lib, you could change that list.
We are using the opa as a standalone with rest calls to retrieve the information, so that won't work for us. I will rewrite the policy for now, but is it possible to do a feature request for this?
☝️ Please take a look at the issue, and please comment if it does (not) cover your requirements.
☝️ Please take a look at the issue, and please comment if it does (not) cover your requirements.
yes that seems about right what I requested. Thanks for the quick help.