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.

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

Using async code in Extensions #363

Open
fitzthum opened this issue Jan 24, 2025 · 18 comments
Open

Using async code in Extensions #363

fitzthum opened this issue Jan 24, 2025 · 18 comments

Comments

@fitzthum
Copy link

fitzthum commented Jan 24, 2025

Is this possible?

Async closures are not stable. I don't really want to stick a block_on thing inside the closure. I might be able to use an async function as an extension, but it would need to take some arguments other than Params and I am not sure if those can be passed in.

@anakrish
Copy link
Collaborator

@fitzthum I haven't really thought about this.
@mkulke I'm curious if you have any ideas on the best way to go about this?

@anakrish
Copy link
Collaborator

Can this be achieved by implementing the Extension trait manually?

@mkulke
Copy link

mkulke commented Jan 27, 2025

Async in traits is still tricky. there is RPITIT for a while but there are limitations with dynamic dispatch. There are macro-crates which allow to annotate traits as async, but they box the future (to make their size known) behind the curtain, which incurs allocation and potential perf hits.

also, at the moment fn's cannot be generic over async/syncv ("maybe async"), so we'd require an ExtensionAsync trait.

I think for a crate like regorus, introducing async should be done with caution, since async fn's (including their constraints) are contagious from the callee to the caller hierarchy. and since regorus as a policy evaluation engine is cpu-bound it's not a good fit for running in async event loops and costlier synchronous regorus blocks would need to be wrapped themselves in such a context.

@fitzthum can you share a bit more about the use case for an async extension?

@fitzthum
Copy link
Author

The use case is outlined here. tldr; I want to use an extension to get reference values at runtime. That could require making a remote call to the RVPS hence the async. Not sure about the best way to work around it.

@anakrish
Copy link
Collaborator

@fitzthum
Copy link
Author

Spawn blocking allows you to run sync code inside of async code. For me the extension is sync but I need to run async code. There is https://docs.rs/tokio/latest/tokio/runtime/struct.Runtime.html#method.block_on but I'm not sure it's a good idea to use that. I'm doing all of this stuff inside of a method that is already async. Creating a new runtime inside of that can create a deadlock. The control flow that actually gets me to the extension is a little bit unclear to me but it still seems like this would not be a good idea.

@mkulke
Copy link

mkulke commented Jan 28, 2025

yeah, spawn blocking would be used if a potential costly regorus invocation would be called from an async context. Is there a reason why the SNP evidence in this example has to be fetched lazily and not passed to the rego evaluation as input?

rego as a declerative language with certain constraints and guarantees (e.g. pure fn's; should always terminate) doesn't seem to mix well with imperative async/await calls during evaluation at first glance.

@fitzthum
Copy link
Author

hmmm I was hoping to rework the reference value interface so that we don't have to dump all of the reference values we have into the engine. Instead we could get them lazily and then also keep track of which reference values were actually used to evaluate the policy so we can provide a record of this in the token. This plan might have to change tho.

@mkulke
Copy link

mkulke commented Jan 28, 2025

Hmm, I think there is an expectation that a policy engine like regorus, when provided with a static input and a static policy will deterministically produce the same output. Performing (async) side effects in the evaluation will break this assumption, policies can now time out, produce different outcomes, ...

@fitzthum
Copy link
Author

Yes, this is a very reasonable stance. I guess the tension comes from the fact that OPA supports extensions in the first place.

Btw there is an interesting page about extensions here. Some of examples they have in the Go version do make network calls.

@mkulke
Copy link

mkulke commented Jan 28, 2025

interesting, they have a "Nondeterministic" annotation to make this explicit and avoid inadvertent side effects from the engine. I guess that could be reasonable approach, if the community adopted this. go also doesn't have async/sync problems, so it's less technically challenging. Would it be an option to use a sync io in such an extension?

@anakrish
Copy link
Collaborator

then also keep track of which reference values were actually used to evaluate the policy so we can provide a record of this in the token

The tracking could also be accomplished by returning the reference value from the entry point rule in addition to the actual allow/deny boolean value.

Something like

reference_value := v {
  v := # compute reference value.
}

eval_results := {
   "reference_value": reference_value,
   "result":  determine_allow_or_deny # where allow/deny is the actual policy logic
}

@anakrish
Copy link
Collaborator

anakrish commented Jan 28, 2025

One way to solve the async extension problem is to have a two-stage policy evaluation:

  1. Stage one computes the parameters needed to make the async call to obtain the reference values.
  2. The returned parameters are used to make the async call to obtain the reference values, The reference values are supplied to
    stage-two via policy input.
# stage-1 entry-point
reference_value_inputs := v {
  v := # compute inputs for reference value async call using `data` and `input`
}

# stage-2 entry-point
allow_or_deny if {
   # Expect that the reference value computed by stage-1 is available via input.
   use(input.computed_reference_value)
}

In rust,

let mut input = ...;

# Stage-1 compute input for reference values
engine.set_input(input.clone());
let reference_value_inputs = engine.eval_rule("namespace.reference_value_inputs")?;

# Make the async call to fetch the reference value.
let reference_values = async await comput reference value using reference_value_inputs;

# Stage-2 use reference value to evaluate allow/deny
# Update input with computed reference vlaue.
input.as_object_mut()?.insert(Value::from("computed_reference_value"), reference_values);
engine.set_input(input);
let allow = engine.eval_rule("namespace.allow_or_deny")?;

@fitzthum
Copy link
Author

Would it be an option to use a sync io in such an extension?

We are making a gRPC request to the RVPS, so it's hard to avoid async code. I don't think there is any way to do that with tonic. There may be another crate. I guess we could fork off a bash command to do it or use a task or something. We could also consider dropping gRPC support in the RVPS and always use the builtin one. None of these are great options.

@fitzthum
Copy link
Author

fitzthum commented Jan 28, 2025

I was envisioning a policy that looks like

executables := 3 if {
        input.snp
	input.snp.launch_measurement == get_reference_value("snp1","launch_measurement")
}

executables := 3 if {
        input.tdx
	input.tdx.launch_measurement == get_reference_value("tdx1","launch_measurement")
}

This would assign the executables claim based on whatever input claims are provided and it would keep track of which reference values were used.

I guess we could change this to

reference_values := ["snp_digest", "other_snp_thing", "etc"] if {
        input.snp
}
reference_values := ["tdx_digest", "other_tdx_thing", "etc"] if {
        input.tdx
}

executables := 3 if {
	input.snp.launch_measurement == reference.snp_digest
}

executables := 3 if {
	input.tdx.launch_measurement == reference.tdx_digest
}

Then evaluate the reference_values claim, populate the reference values accordingly, and evaluate the other claims. Not ideal but this might be the best option so far.

@anakrish
Copy link
Collaborator

anakrish commented Jan 28, 2025

That strategy should work. You might need to use input.reference in the executables rule if that is where the reference values would be installed.

@anakrish
Copy link
Collaborator

@mkulke Is there a recommended practice of how Rust crates deal with async extensions/callbacks/hooks? Should the crate become somehow async aware?

@mkulke
Copy link

mkulke commented Jan 29, 2025

not really, calls to async fns need to be awaited in an async fn, so it needs propagate up the call stack until there is an async executor (colored function problem). nesting executors (executor -> async fn -> sync fn -> executor -> async fn) will cause problems. there is an initiative to make rust generic over certain keywords, but that's a longer-term effort. For now, supporting both async and sync APIs requires a fair amount of duplication.

A popular crate that provides a blocking and async API is reqwest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants