Skip to content

grpc/xds: Add feature-gated xDS proto codegen and the generated .rs code#2723

Open
gu0keno0 wants to merge 15 commits into
grpc:masterfrom
gu0keno0:xds-protoc
Open

grpc/xds: Add feature-gated xDS proto codegen and the generated .rs code#2723
gu0keno0 wants to merge 15 commits into
grpc:masterfrom
gu0keno0:xds-protoc

Conversation

@gu0keno0

@gu0keno0 gu0keno0 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Add xds protos, codegen and generated .rs files

Motivation

The usual places for the generated code of xDS protobuf is https://github.com/envoyproxy, e.g. https://github.com/envoyproxy/go-control-plane. However, currently there's no official https://github.com/envoyproxy/rust-control-plane.

For gRPC, we need to use the Google protobuf codegen to generate the xDS structs, therefore created this PR to do the job.

Solution

  1. Add import script to download relevant protobuf sources from the SOT locations. This is mirroring grpc-java
  2. Add codegen logic to the existing codegen crate to generate .rs files for the downloaded protobuf sources. By default, the Rust protoc codegen library assumes all types are in a flattened namespace in a single crate. This does not work for us because it will cause name collisions in multiple xDS protobuf source files. Therefore, the codegen generates a aggregation .rs file to re-export the protobuf types as a module, which also provides namespace isolations and organizes the generated protobuf in the same hierarchy as the original protobuf sources.
  3. Gate xds feature in both codegen and grpc crates.

Test Plan

  1. cargo build + CI
  2. I also created a local test to verify that the generated types work as expected
#[cfg(test)]
mod tests {
    //! Demonstrates building the four core xDS resource messages
    //! (LDS/RDS/CDS/EDS) from the generated types and round-tripping them
    //! through the protobuf wire format.

    // `.serialize()` and `T::parse(&bytes)` come from the protobuf message
    // traits; the prelude brings them into scope under `_` aliases.
    use protobuf::prelude::*;

    use super::generated::envoy::config::{
        cluster::v3::cluster::{cluster::DiscoveryType, Cluster},
        endpoint::v3::endpoint::ClusterLoadAssignment,
        listener::v3::listener::Listener,
        route::v3::route::RouteConfiguration,
    };

    /// Encodes then decodes a message, exercising the protobuf wire round-trip.
    fn round_trip<M: protobuf::Message>(msg: &M) -> M {
        M::parse(&msg.serialize().expect("serialize")).expect("parse")
    }

    #[test]
    fn lds_listener_round_trip() {
        let mut listener = Listener::new();
        listener.set_name("grpc-listener");

        let decoded = round_trip(&listener);
        assert_eq!(decoded.name().as_bytes(), b"grpc-listener");
    }

    #[test]
    fn rds_route_configuration_round_trip() {
        let mut route_config = RouteConfiguration::new();
        route_config.set_name("grpc-routes");

        let decoded = round_trip(&route_config);
        assert_eq!(decoded.name().as_bytes(), b"grpc-routes");
    }

    #[test]
    fn cds_cluster_round_trip() {
        let mut cluster = Cluster::new();
        cluster.set_name("grpc-cluster");
        // A CDS cluster whose endpoints are resolved via EDS.
        cluster.set_type(DiscoveryType::Eds);

        let decoded = round_trip(&cluster);
        assert_eq!(decoded.name().as_bytes(), b"grpc-cluster");
        assert_eq!(decoded.r#type(), DiscoveryType::Eds);
    }

    #[test]
    fn eds_cluster_load_assignment_round_trip() {
        let mut endpoints = ClusterLoadAssignment::new();
        // EDS keys its endpoints by the owning cluster's name (the CDS -> EDS link).
        endpoints.set_cluster_name("grpc-cluster");

        let decoded = round_trip(&endpoints);
        assert_eq!(decoded.cluster_name().as_bytes(), b"grpc-cluster");
    }
}
  1. Soon we'll add meaningful handlings of these generated structs when we implement the xDS dependency manager.

@gu0keno0 gu0keno0 requested review from dfawley and ejona86 July 7, 2026 14:07
@gu0keno0 gu0keno0 marked this pull request as ready for review July 7, 2026 15:56
@gu0keno0 gu0keno0 requested a review from YutaoMa July 7, 2026 15:56
@ejona86

ejona86 commented Jul 9, 2026

Copy link
Copy Markdown
Member

I've been poking at this today on my computer, trying to understand what protobuf is doing and trying to understand our options.

Some notes that don't impact the heart of the PR too much:

  1. I don't think we want to be using codegen/ for this. I think we'd want to use a build.rs instead. We could use GRPC_RUST_REGENERATE_PROTO=1 so it isn't done on every build, but... that's invalidated by the next point
  2. I don't think we want to check in this much code. I think we want to regenerate it for right now. Since xds is not generally enabled that is not a problem for users today. Once we get further then we'll want a separate crate in a separate repo, so it could be committed then, but not to this repo

I had thought (2) is what we said would be best during our meeting. @dfawley can confirm. (I'll be on vacation tomorrow.)

At this point I think I see why you've made the various choices you did. I think I agree that is at least close to optimal right now. But I think we'd also all agree this is ridiculous. I want to think more about what we can suggest to protobuf to make the situation better. I feel we'll be doing everyone a disservice if we don't figure out a better approach before protobuf stabilizes.

(I don't yet see a reason protoc is run per-file instead of per-directory/proto-package. I'm right now assuming both ways would work and you just chose something and went with it.)

@gu0keno0

gu0keno0 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for looking. Answer the questions inline.

I've been poking at this today on my computer, trying to understand what protobuf is doing and trying to understand our options.

Some notes that don't impact the heart of the PR too much:

  1. I don't think we want to be using codegen/ for this. I think we'd want to use a build.rs instead. We could use GRPC_RUST_REGENERATE_PROTO=1 so it isn't done on every build, but... that's invalidated by the next point
  1. I don't think we want to check in this much code. I think we want to regenerate it for right now. Since xds is not generally enabled that is not a problem for users today. Once we get further then we'll want a separate crate in a separate repo, so it could be committed then, but not to this repo

I had thought (2) is what we said would be best during our meeting. @dfawley can confirm. (I'll be on vacation tomorrow.)

In #2728 @dfawley is suggesting to put the generated code into a separate crate grpc-xds. I think build.rs makes sense. For Envoy go-control-plaine and java-control-plane, they have to check in generated code because they don't have build.rs . For Rust it's not a problem.

At this point I think I see why you've made the various choices you did. I think I agree that is at least close to optimal right now. But I think we'd also all agree this is ridiculous. I want to think more about what we can suggest to protobuf to make the situation better. I feel we'll be doing everyone a disservice if we don't figure out a better approach before protobuf stabilizes.

(I don't yet see a reason protoc is run per-file instead of per-directory/proto-package. I'm right now assuming both ways would work and you just chose something and went with it.)

The per-file approach is because there are naming conflicts. If we feed all of them directly into protoc, the generated code will have naming conflicts under the same flattened namespace (5000+ errors), because in generated code, the proto types are re-exported by a statement like "pub use internal_do_not_use_:: re-export." and there are the name conflicts such as "Policy" and "Filter". The per-file approach creates a module wrapper for each file, and therefore eliminates the name collisions.

I think this shenanigan will go away if Protobuf natively support organizing the generated code by protobuf package modules (similar to this PR but natively supported by protoc).

@ejona86

ejona86 commented Jul 10, 2026

Copy link
Copy Markdown
Member

The per-file approach is because there are naming conflicts

I was contrasting per-file with per-directory/package. There shouldn't be name conflicts within a proto-package.

@dfawley

dfawley commented Jul 10, 2026

Copy link
Copy Markdown
Member

I feel we'll be doing everyone a disservice if we don't figure out a better approach before protobuf stabilizes.

cc @esrauchg 😄

@esrauchg

esrauchg commented Jul 10, 2026

Copy link
Copy Markdown

Protobuf Rust maintainer here: I'm definitely happy to get the "wtf is this" feedback spelled out and we can definitely consider adjustments.

I have a very firm grasp of how this does/should work in blazel, but I'm not so clear as to how it should work outside of that context and so am open to feedback.

Just to clarify, the intended model of protoc (language agonstic) is that you should be running it once per conceptual library/package granularity, and your library/package granularity shouldn't be so large that you have a bunch of shortname collisions within it: it suggests something is conceptually smelly at best if you have two top-level messages with the same name in the same library/package that are only disambiguated by the package name spelled at the top of the file.

Theres an oddity which is that there's 3 different concepts of: file paths, object namespacing, and the library boundaries. In Java for example the file paths and namespaces are coupled, but library boundary is M:N compared to that (*except under JPMS, which has somewhat shallow uptake in the ecosystem). In C++ directories:namespaces are M:N and namespaces:libraries are also M:N.

Rust and Go are special here because your imports name the package. In blazel model, this is resolved by the target name as written in the BUILD file being used as the effective namespace. With fine-grained rust_library/proto_library (its one crate per .proto file effective) this namespacing is already very tight, and so also adding in mods of the .proto package would be really bad on ergonomics. Its technically true that you can make a proto_library() that doesn't work in Rust here from name collisions, but it would require you to make a proto_library() which has 2 .proto files with different proto-packages and the same top level name, which is basically a "own breakage" at that point.

Go in non-blazel has a solution of go_package, where a single protoc invocation actually can end up emitting N different Go Packages. This is basically a historical oddity and debatably regretted: really a single protoc invocation shouldn't ever be emitting N conceptual 'libraries'. But we're not reopening GoProto design here, its basically fine even if this quirk is a bit outside of "what should a single protoc invocation do" ethos.

Crates.io Cargo crates are basically coarse-grained instead of the fine-grained ones in blazel, I fully understand that you wouldn't really want 1-crate-per-.proto-file in that environment. I still did mostly expect that you would likely have 1 crate per conceptual package and conceptual package would map back to the package at the top of the .proto file. But broadly I expect you shouldn't have 1 crate with 100 .proto files which have 10 different packages declared at the top of them either:, middle ground is "at least 1 crate per .proto package".

In general you also could run protoc N times and then put post-hoc put those into corresponding mods, as the example crate chooses a mod here:
https://docs.rs/crate/protobuf-example/latest/source/src/main.rs#3

Hope that context helps, and I'm happy to entertain improvements here. Thanks!

@gu0keno0

Copy link
Copy Markdown
Contributor Author

@esrauchg : thanks for the context and discussion. I was able to read and understand the rational of crate-per-package design as I was working on this PR. I'd like to suggest that the main issue here is maintenance overhead. The xDS protos contains ~67 proto packages, if we make all of them crates, then potentially we will add the 67 crates to workspace, and will need to manage their publications. While technically this is still doable, I just feel it's probably not the ideal approach because we just need the generated code for grpc-xds to work, the publication of the crates, or even the creation of the 67 crates themselves, are just not the essential bit of gRPC xDS. The xDS proto is also moderately complex, many other projects (primarily, data model libs) has a more complicated protobuf setup.

This is why I was considering the alternative approach: codegen the .rs files and wrap each file/package with their own rust module. Therefore, the namespace problem is solved by the Rust modules, and we only need to publish grpc-xds crate (I'll update the PR for doing that), which only uses the protos as pub(crate) dependencies. The proto themselves do not even need to be published because users do not need to see them.

Maybe there are better alternatives, glad to learn about them.

@gu0keno0

gu0keno0 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@ejona86 @dfawley I've done the following clean-ups:

  1. create new crate grpc-xds, make it unpublished for now for CI to pass, soon we can decide when and how to publish it
  2. moved the downloaded proto files into grpc-xds, use build.rs to build the generated code; removed all generated codde.
  3. make code-gen per-proto-package instead of per-file. I still need to keep the mod.rs generations to give namespace isolation for the proto modules.

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

Successfully merging this pull request may close these issues.

4 participants