proto: don't require proxying for list broker calls

proto/redpanda/core/admin/v2/broker.proto

@@ -44,9 +44,9 @@ service BrokerService {
 
 // GetBrokerRequest returns information about a single broker in the cluster
 message GetBrokerRequest {
-    // The node ID for the broker. If set to -1, the broker handling the RPC
+    // The node ID for the broker. If unset, the broker handling the RPC
     // request returns information about itself.
-    int32 node_id = 1;
+    optional int32 node_id = 1;
 }
 
 // GetBrokerResponse is the response from the GetBroker RPC.
@@ -69,9 +69,13 @@ message Broker {
     // This broker's node ID.
     int32 node_id = 1;
     // The build this broker is running.
-    BuildInfo build_info = 2;
+    //
+    // Only populated for `GetBroker` RPCs
+    optional BuildInfo build_info = 2;
     // The admin server information.
-    AdminServer admin_server = 3;
+    //
+    // Only populated for `GetBroker` RPCs
+    optional AdminServer admin_server = 3;
 }
 
 // BuildInfo contains information about the Redpanda build.

src/v/redpanda/admin/services/broker.cc

@@ -37,11 +37,13 @@ broker_service_impl::broker_service_impl(
 
 ss::future<proto::admin::get_broker_response> broker_service_impl::get_broker(
   serde::pb::rpc::context ctx, proto::admin::get_broker_request req) {
-    auto target = model::node_id(req.get_node_id());
-    if (target != -1 && target != _proxy_client.self_node_id()) {
-        co_return co_await _proxy_client
-          .make_client_for_node<proto::admin::broker_service_client>(target)
-          .get_broker(ctx, std::move(req));
+    if (req.has_node_id()) {
+        auto target = model::node_id(req.get_node_id());
+        if (target != -1 && target != _proxy_client.self_node_id()) {
+            co_return co_await _proxy_client
+              .make_client_for_node<proto::admin::broker_service_client>(target)
+              .get_broker(ctx, std::move(req));
+        }
     }
     proto::admin::get_broker_response resp;
     resp.set_broker(self_broker());
@@ -56,29 +58,31 @@ broker_service_impl::list_brokers(
     auto clients
       = _proxy_client
           .make_clients_for_other_nodes<proto::admin::broker_service_client>();
-    for (auto& [node_id, client] : clients) {
-        proto::admin::get_broker_request req;
-        req.set_node_id(node_id);
-        auto get_resp = co_await client.get_broker(ctx, std::move(req));
-        list_resp.get_brokers().push_back(std::move(get_resp.get_broker()));
+    for (auto& [node_id, _] : clients) {
+        auto& broker = list_resp.get_brokers().emplace_back();
+        broker.set_node_id(node_id());
     }
     co_return list_resp;
 }
 
 proto::admin::broker broker_service_impl::self_broker() const {
     proto::admin::broker b;
     b.set_node_id(_proxy_client.self_node_id());
-    b.get_build_info().set_version(ss::sstring(redpanda_git_version()));
-    b.get_build_info().set_build_sha(ss::sstring(redpanda_git_revision()));
+    proto::admin::build_info build_info;
+    build_info.set_version(ss::sstring(redpanda_git_version()));
+    build_info.set_build_sha(ss::sstring(redpanda_git_revision()));
+    b.set_build_info(std::move(build_info));
+    proto::admin::admin_server admin_server_info;
     for (auto& service : *_services) {
         for (auto& route : service->all_routes()) {
             proto::rpc_route r;
             r.set_name(
               fmt::format("{}.{}", route.service_name, route.method_name));
             r.set_http_route(ss::sstring(route.path));
-            b.get_admin_server().get_routes().push_back(std::move(r));
+            admin_server_info.get_routes().push_back(std::move(r));
         }
     }
+    b.set_admin_server(std::move(admin_server_info));
     return b;
 }
 

tests/rptest/clients/admin/v2.py

@@ -1,3 +1,4 @@
+import logging
 import random
 from typing import Literal, Protocol, final, Any
 
@@ -40,6 +41,9 @@
 class RedpandaServiceProto(Protocol):
     def started_nodes(self) -> list[ClusterNode]: ...
 
+    @property
+    def logger(self) -> logging.Logger: ...
+
 
 # Re-export some protobufs for convenience
 broker_pb = broker_pb2
@@ -57,8 +61,11 @@ def started_nodes(self) -> list[ClusterNode]: ...
 
 # A hacky workaround for https://github.com/connectrpc/connect-python/issues/37
 class HeaderInjectingClient:
-    def __init__(self, client, headers_to_inject: dict[str, str]):
+    def __init__(
+        self, client, logger: logging.Logger, headers_to_inject: dict[str, str]
+    ):
         self.client = client
+        self.logger = logger
         self.headers_to_inject = headers_to_inject
 
     def call_unary(
@@ -69,6 +76,7 @@ def call_unary(
         extra_headers: dict[str, str] | None = None,
         timeout_seconds: float | None = None,
     ):
+        self.logger.debug(f"making admin RPC {url}")
         if extra_headers is None:
             extra_headers = self.headers_to_inject
         else:
@@ -104,14 +112,15 @@ def __init__(
     def _make_service(self, service_clazz, node: ClusterNode | None = None):
         if not node:
             node = random.choice(self._rp.started_nodes())
+            assert node, "must have at least one started node"
         client = service_clazz(
             base_url=f"http://{node.account.hostname}:9644",
             protocol=ConnectProtocol.CONNECT_PROTOBUF
             if self._protocol == "proto"
             else ConnectProtocol.CONNECT_JSON,
         )
         client._connect_client = HeaderInjectingClient(
-            client._connect_client, self._headers.copy()
+            client._connect_client, self._rp.logger, self._headers.copy()
         )
         return client