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.

Sign up for GitHub

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

Jump to bottom

Custom Built-in functions do not propagate to PartialResult #1792

Closed
kenfdev opened this issue Sep 26, 2019 · 0 comments · Fixed by #1967
Closed

Custom Built-in functions do not propagate to PartialResult #1792

kenfdev opened this issue Sep 26, 2019 · 0 comments · Fixed by #1967
Assignees
@tsandall
Labels
bug

Comments

@kenfdev
Copy link
Contributor

kenfdev commented Sep 26, 2019

Expected Behavior

  1. Register a custom built-in for a Rego object
  2. Create a PartialResult from the Rego object
  3. Eval with the PartialResult
  4. Receive a ResultSet

Actual Behavior

In step 3 of the Expected Behavior, an error occurs saying eval_internal_error: unsupported built-in.

Steps to Reproduce the Problem

You can clone my sample repo here and checkout commit f2a44459aa4d20f61f084431bfe65c7950faa24d.

go run main.go

And error will occur.

Additional Info

If you re-assign the custom built-in when executing PartialResult#Rego(), the error goes away. You can see the diff here.

As discussed in the Slack thread, this occurs because custom built-ins don't propagate to PartialResults.

Related Slack thread:
https://openpolicyagent.slack.com/archives/C1H19LW4F/p1569483116206300
@tsandall tsandall added the bug label Sep 26, 2019
@tsandall tsandall self-assigned this Sep 26, 2019
patrick-east added a commit to patrick-east/opa that referenced this issue Dec 18, 2019
@patrick-east
rego: Propagate custom builtins to PartialResult
8cda284 
We would previously lose any builtin functions provided to the Rego
object after partially evaluating it. Upon creating a new Rego
instance from the PartialResult you would have to re-declare them
all again which breaks the simple case of just getting a new Rego
and evaluating. We should, at a minimum, make sure the Rego produced
by the PartialResult is able to be evaluated.

This change just passes along the declarations to the new Rego
instance. Everything is done with internal bits in the rego package
so we could adjust this later on if we want to.

Fixes: open-policy-agent#1792
Signed-off-by: Patrick East <east.patrick@gmail.com>
@patrick-east patrick-east mentioned this issue Dec 18, 2019
Merged
patrick-east added a commit that referenced this issue Jan 15, 2020
@patrick-east
rego: Propagate custom builtins to PartialResult
a5ed67a 
We would previously lose any builtin functions provided to the Rego
object after partially evaluating it. Upon creating a new Rego
instance from the PartialResult you would have to re-declare them
all again which breaks the simple case of just getting a new Rego
and evaluating. We should, at a minimum, make sure the Rego produced
by the PartialResult is able to be evaluated.

This change just passes along the declarations to the new Rego
instance. Everything is done with internal bits in the rego package
so we could adjust this later on if we want to.

Fixes: #1792
Signed-off-by: Patrick East <east.patrick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsandall tsandall

Labels
bug
Projects
Open Policy Agent
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rego: Propagate custom builtins to PartialResult patrick-east/opa
2 participants
@tsandall @kenfdev