opa icon indicating copy to clipboard operation
opa copied to clipboard

Parameterized function name caused `undefined function`

Open onelittlenightmusic opened this issue 4 years ago • 6 comments

[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

onelittlenightmusic avatar Nov 17 '21 05:11 onelittlenightmusic

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.

srenatus avatar Nov 17 '21 10:11 srenatus

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"
}

tsandall avatar Nov 18 '21 01:11 tsandall

@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 to data.main.test(vars) <-- Supported
    • Dynamic: data.main[vars](vars2) <-- Not supported
  • 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.main reference in package main implementation might be recursion check failure. I will try different package name like package lib to avoid apparent recursion risk. It might be just a workaround though..

onelittlenightmusic avatar Nov 18 '21 06:11 onelittlenightmusic

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.

anderseknert avatar Nov 18 '21 11:11 anderseknert

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.

onelittlenightmusic avatar Nov 19 '21 07:11 onelittlenightmusic

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.

anderseknert avatar Apr 24 '25 13:04 anderseknert