containerd / rust-extensions

Rust crates to extend containerd
https://containerd.io
Apache License 2.0
184 stars 73 forks source link

Enable type names for Protocol Buffer message types to support `Any` decoding. #262

Closed tobz closed 7 months ago

tobz commented 7 months ago

Context

When using something like EventsClient::subscribe, the Envelope value returned provides the event data (if any) behind prost_types::Any.

This value can be fallibly decoded to the underlying type (such as containerd_client::events::ContainerCreate for a container create event) by using Any::to_msg<M>, but only if M implements Name.

Currently, based on how tonic_build is used to do the codegen, the Name trait is not implemented for any of the generated message types, making decoding much harder to do.

Solution

This PR is a simple change that simply provides additional configuration in build.rs, enabling the generation of Name implementations for message types. Builder::compile_with_config simply re-applies any of its own configuration on top of the configuration value given, so we're essentially just starting with a base of "enable type Name generation" and whatever else comes after (via tonic_build::configure()...) gets applied on top.

jsturtevant commented 7 months ago

could you add an example in https://github.com/containerd/rust-extensions/tree/main/crates/client/examples that demonstrates this? It would also ensure we have a way to validate it.

mxpv commented 7 months ago

It would also ensure we have a way to validate it.

I think a unit test with decoding should do it ?

Generally the PR is SGTM.

tobz commented 7 months ago

I pushed both a unit test and an example of listening for and decoding event payloads.

Admittedly, I have not yet tested the example.

tobz commented 7 months ago

Alright, I ended up testing this and it seems to work, but with one caveat.

You'll see in the example that I had to futz with the type URL field for the Any payloads. It appears that containerd generates non-compliant type URLs by not emitting a leading slash, so that a payload for the ContainerCreate event has a type URL of containerd.events.ContainerCreate, when prost generates the type URL as /containerd.events.ContainerCreate.

The Protocol Buffers definition for Any states that there must be at least one forward slash in the type URL, which seems to be what prost is following and adhering to... but practically, it also seems pointless to have if there's no actual leading URL, given how all of this stuff was based on the premise of querying some schema/type registry endpoint and handling it all automagically... so I can understand why containerd just omitted the leading slash.

Either way, the functionality does work, so long as we adjust the type URL to make prost happy. Hopefully that's sufficient.

Example output:

toby@consigliera:~/src/rust-extensions$ cargo run --example container_events
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
     Running `target/debug/examples/container_events`
container created: id=0f550a4e94ae12a8881cf017e14e3b505ea62b022f21876613490f99ce9549e6 payload=ContainerCreate { id: "0f550a4e94ae12a8881cf017e14e3b505ea62b022f21876613490f99ce9549e6", image: "", runtime: Some(Runtime { name: "io.containerd.runc.v2", options: Some(Any { type_url: "containerd.runc.v1.Options", value: [50, 4, 114, 117, 110, 99, 58, 28, 47, 118, 97, 114, 47, 114, 117, 110, 47, 100, 111, 99, 107, 101, 114, 47, 114, 117, 110, 116, 105, 109, 101, 45, 114, 117, 110, 99, 72, 1] }) }) }
container deleted: id=0f550a4e94ae12a8881cf017e14e3b505ea62b022f21876613490f99ce9549e6 payload=ContainerDelete { id: "0f550a4e94ae12a8881cf017e14e3b505ea62b022f21876613490f99ce9549e6" }