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

gRPC server pseudo-node in Rust Oak Runtime #751

Closed
ipetr0v opened this issue Mar 23, 2020 · 8 comments · Fixed by #772
Closed

gRPC server pseudo-node in Rust Oak Runtime #751

ipetr0v opened this issue Mar 23, 2020 · 8 comments · Fixed by #772
Assignees

Comments

@ipetr0v
Copy link
Contributor

ipetr0v commented Mar 23, 2020

Currently Rust Oak Runtime doesn't support TLS gRPC credentials that is supported by the C++ version:

grpc::Status CreateApplication(const oak::ApplicationConfiguration& application_configuration,
std::shared_ptr<grpc::ServerCredentials> grpc_credentials);

Necessary, since we are moving towards using a Rust Oak Loader binary in the future #749.

@ipetr0v ipetr0v changed the title TLS credentials in Rust Oak Runtime gRPC server node in Rust Oak Runtime Mar 24, 2020
@ipetr0v
Copy link
Contributor Author

ipetr0v commented Mar 24, 2020

TLS credentials are not implemented because Rust Oak Runtime does not have a gRPC server pseudo-node for receiving messages from clients yet.

Before Split gRPC (#627) is implemented we could have a gRPC server pseudo-node implemented as a Tonic server (related to #695) that encapsulates client requests in a generic Protobuf message:

message GrpcRequest {
string method_name = 1;
google.protobuf.Any req_msg = 2;
bool last = 3;
}

and passes it to the initial Oak node, specified in the config:
initial_node_config_name: "app"
initial_entrypoint_name: "oak_main"

@ipetr0v ipetr0v self-assigned this Mar 24, 2020
@ipetr0v ipetr0v changed the title gRPC server node in Rust Oak Runtime gRPC server pseudo-node in Rust Oak Runtime Mar 24, 2020
@ipetr0v
Copy link
Contributor Author

ipetr0v commented Mar 24, 2020

@conradgrobler

The C++ implementation of the gRPC pseudo-node uses the grpc::AsyncGenericService, that accepts all gRPC calls and does not implement any particular Protobuf schema:

// Register an async service.
node->module_service_ = absl::make_unique<grpc::AsyncGenericService>();
builder.RegisterAsyncGenericService(node->module_service_.get());

Have you seen a similar service for Tonic?
IIUC Tonic requires some schema in order to start server:

service HelloWorld {
rpc SayHello(HelloRequest) returns (HelloResponse);
rpc LotsOfReplies(HelloRequest) returns (stream HelloResponse);
rpc LotsOfGreetings(stream HelloRequest) returns (HelloResponse);
rpc BidiHello(stream HelloRequest) returns (stream HelloResponse);
}

Server::builder()
.tls_config(ServerTlsConfig::new().identity(identity))
.add_service(HelloWorldServer::new(handler))
.serve(address)
.await?;

@conradgrobler
Copy link
Collaborator

conradgrobler commented Mar 24, 2020

The approach I was considering was to create a new generic service (see #695 (comment)). The service would need to implement a call method that can receive a generic Request object.

This service would look similar to some of the code generated by Tonic for a proto file (eg https://github.com/project-oak/oak/pull/647/files#diff-6116dfd8bdebbd1ea4776bec4c89ecccR65)

Initially, we can test using only a unary service and later figure out how to handle client streaming or bidirectional streaming services in a generic way.

This service could then be added using add_service

@ipetr0v
Copy link
Contributor Author

ipetr0v commented Mar 25, 2020

In order to be able to use add_service, the service should implement a tonic::transport::server::NamedService trait:
https://github.com/project-oak/oak/pull/647/files#diff-6116dfd8bdebbd1ea4776bec4c89ecccR229-R231

impl<T: HelloWorld> tonic::transport::NamedService for HelloWorldServer<T> {
    const NAME: &'static str = "oak.examples.hello_world.HelloWorld";
}

And it requires to set a service name (e.g. "oak.examples.hello_world.HelloWorld").

Is it enough to just set an empty string as a service name? Because it looks like upon receiving a message from a client Tonic searches for a corresponding service based on its name.

@conradgrobler
Copy link
Collaborator

Looking at the Tonic code, it seems that the routing just checks that the request URI starts with / followed by the service name.
https://github.com/hyperium/tonic/blob/master/tonic/src/transport/server/mod.rs#L328

I would think that an empty string would work, although it feels like it might be brittle if the logic changes in future. For initial testing it should be OK, but it might be better in the long term to use hyper directly, rather than Tonic, as we are not currently relying on any real gRPC functionality in the gRPC pseudo node. The code could look very similar to the hyper web API example:
https://github.com/hyperium/hyper/blob/master/examples/web_api.rs#L100

@tiziano88
Copy link
Collaborator

Yes, my suggestion is to aim directly for a Hyper service, and not involve Tonic at all, if possible.

@conradgrobler
Copy link
Collaborator

Tonic provides a nice mechanism for configuring TLS. When using Hyper directly, it would need code similar to the following:
https://users.rust-lang.org/t/use-hyper-server-with-tls/18535

tiziano88 added a commit to tiziano88/oak that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Ref project-oak#603 project-oak#751
tiziano88 added a commit to tiziano88/oak that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Add a simple test for the Wasm node.

Ref project-oak#603 project-oak#751
@blaxill blaxill mentioned this issue Mar 25, 2020
19 tasks
tiziano88 added a commit to tiziano88/oak that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Add a simple test for the Wasm node.

Ref project-oak#603 project-oak#751
tiziano88 added a commit to tiziano88/oak that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Add a simple test for the Wasm node.

Ref project-oak#603 project-oak#751
tiziano88 added a commit to tiziano88/oak that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Add a simple test for the Wasm node.

Ref project-oak#603 project-oak#751
tiziano88 added a commit to tiziano88/oak that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Add a simple test for the Wasm node.

Ref project-oak#603 project-oak#751
tiziano88 added a commit that referenced this issue Mar 25, 2020
Have Log and Wasm nodes implement it.

Add a simple test for the Wasm node.

Ref #603 #751
@ipetr0v
Copy link
Contributor Author

ipetr0v commented Mar 28, 2020

Since Hyper requires the function that creates and starts a server to be async:
https://github.com/hyperium/hyper/blob/97ed0478a871f45555ee1985ae228a0adc27c35a/examples/hello.rs#L18-L23

Do we need to consider making node_create function async as well?

pub fn node_create(
&self,
_node_id: NodeId,
module_name: &str,
entrypoint: &str,
label: &oak_abi::label::Label,
reader: Handle,
) -> Result<(), OakStatus> {

But I think this may require rewriting a lot of internal Runtime code in async/await.

ipetr0v added a commit that referenced this issue Apr 7, 2020
This change adds a Rust gRPC server pseudo-node.

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

Successfully merging a pull request may close this issue.

3 participants