opa
opa copied to clipboard
Parameterized function name caused `undefined function`
[Updated 11/17/2021] This was originally submitted as issue, but actually enhancement request. Dynamic function call is not supported yet at the time of report.
This issue is about calling a function. Functions can be called like test(var). In another case, it would be useful that we can call it like data.main["test"](var), especially when the function name depends on packages or other aspects.
When I have this test_pass.rego file,
package main
allow {
data.main["test"]("hello")
}
test(name) = {
name == "hello"
}
I can call the function like this successfully. That is good.
$ opa eval -d ./test_pass.rego 'data.main.allow' --format raw
true
Expected Behavior
I tried parameterization of the function name. When I have this test_fail.rego file,
package main
allow {
field := "test"
data.main[field]("hello")
}
test(name) = {
name == "hello"
}
I would expect that this command succeeds.
$ opa eval -d ./test_fail.rego 'data.main.allow' --format raw
true
Actual Behavior
This command fails like this.
$ opa eval -d ./test_fail.rego 'data.main.allow' --format raw
1 error occurred: ./main2.rego:6: rego_type_error: undefined function data.main[__local0__]
Where should I start to fix this error? I might be going wrong way. I would appreciate any advice. Thank you.
Steps to Reproduce the Problem
- OPA version: 0.34.2
- Tested environment: MacOS 12.0.1
Thanks for compiling this report. I can see how it's misleading that
data.main["test"]("hello")
works and
x := "test"
data.main[x]("hello")
does not.
The analogy, however, is on the surface only: data.main["test"]("hello") is a different way to write data.main.test("hello").
Once a variable comes into the ref (= the function name part), the recursion checker has something to say: it's not clever enough that data.main[x]("hello") could not be referencing data.main.allow("hello"), so it would be rejected on those grounds, too.
I think what you'd do here is to switch to rules (not functions), and either have them reference data and input themselves, or use with to pass information; then apply the standard policy composition patterns... 🤔 LMK if you need assistance with that.
It would be nice to support vars in function names. A few thoughts...
If we support dynamic calls then I'd naturally want to support iteration there as well:
some x
y := 7
data.foo[x](y)
If we wanted to support vars in function names we'd have to update both the compiler and the evaluator:
- Type checker would need to tolerate vars in function calls (recursion checks can still be done)
- Safety check would need to update to include outputs from the iteration case
- Evaluator needs to update to plug function call names
- Evaluator needs to update to support iteration for vars in function calls (i.e., evalTree needs to be run for calls)
Dealing with built-in functions would be harder though we could leave that out initially.
As a side note, we shouldn't include rewritten vars in errors, so I've filed #4031 to track that bug.
In the short term, rules need to be used. If inputs must be passed then with can be used. For example:
x := "test"; data.main[x] with input as "hello"
Then define "test" as a complete rule:
package main
test {
input == "hello"
}
@srenatus @tsandall Thank you very much for the clarification and your insightful thoughts! Thanks to your explanation, now I understood the following things.
- Dynamic function call is not supported yet by compiler. So, this is not an issue but enhancement request.
- Static:
data.main["test"](vars)is corresponding todata.main.test(vars)<-- Supported - Dynamic:
data.main[vars](vars2)<-- Not supported
- Static:
- Dynamic function call is tough thing. When I would start to contribute to this enhancement, I have to understand type checker, safety check...
- Iteration use case that @tsandall mentioned is similar to my original use case.
- I understand that consistency of output type is important...
- I appreciate new issue creation #4031
- As short-term solution, rules that both you guys suggested looks very interesting. I will try it and let you know workaround for my case.
- Recursion risk that @srenatus explained was understandable. It should be avoided.
- My guess is that
data.mainreference inpackage mainimplementation might be recursion check failure. I will try different package name likepackage libto avoid apparent recursion risk. It might be just a workaround though..
Possible duplicate of https://github.com/open-policy-agent/opa/issues/3167
Which is fine of course :) We might want to close one of them though.
To make clear the context, I explain my original use case. Very similar to iteration use case.
In short, I would like to call arbitrary function defined in JSON.
I have the following JSON data.
{
"user1": { "need_admission_from": ["user2", "user3"] },
"user2": { "need_admission_from": ["user3"],
"need_to_inform": ["user1"]},
"user3": { "other_requirements": ....}
}
I would like to write functions for each field that objects like "userX" have ("need_admission_from" etc)
need_admission_from(obj, children) = rtn {
...
}
need_to_inform(obj, children) = rtn {
...
}
If an object "userX" has field "a", then I want to call function "a".
some user
a := fields[_]
output := data.main[a](user, children)
# modify output to use for final logic
Using this approach, JSON field name (=function name) can be flexible. Then, the policies are going to be extensible.
That is my original motivation. I guess that for we might be able to use not only function but also rules my use case. I guess #3167 is also from similar motivation. Unlike this issue, #3167 says package name should be flexible.
Just saw a different error message about this, which was... interesting :) and perhaps a case to remember when this is fixed one day:
package p
for.foo.f(_) := 1
values contains value if {
x := "foo"
value := for[x].f(x)
}
1 error occurred: policy.rego:7: rego_compile_error: called function data.p.for[x].f shadowed
The difference being that it's not the function name that's dynamic, but a part of the ref which is used to invoke it.