[csds] Make grpc_dump_xds_configs return ClientStatusResponse

src/core/ext/xds/xds_api.cc

@@ -589,32 +589,29 @@ google_protobuf_Timestamp* EncodeTimestamp(const XdsApiContext& context,
 
 }  // namespace
 
-std::string XdsApi::AssembleClientConfig(
-    const ResourceTypeMetadataMap& resource_type_metadata_map) {
-  upb::Arena arena;
-  // Create the ClientConfig for resource metadata from XdsClient
-  auto* client_config = envoy_service_status_v3_ClientConfig_new(arena.ptr());
+envoy_service_status_v3_ClientConfig* XdsApi::AssembleClientConfig(
+    envoy_service_status_v3_ClientConfig* client_config,
+    const ResourceTypeMetadataMap& resource_type_metadata_map,
+    std::vector<std::string>* type_url_storage, upb_Arena* arena) {
   // Fill-in the node information
-  auto* node = envoy_service_status_v3_ClientConfig_mutable_node(client_config,
-                                                                 arena.ptr());
-  const XdsApiContext context = {client_, tracer_, def_pool_->ptr(),
-                                 arena.ptr()};
+  auto* node =
+      envoy_service_status_v3_ClientConfig_mutable_node(client_config, arena);
+  const XdsApiContext context = {client_, tracer_, def_pool_->ptr(), arena};
   PopulateNode(context, node_, user_agent_name_, user_agent_version_, node);
   // Dump each resource.
-  std::vector<std::string> type_url_storage;
   for (const auto& p : resource_type_metadata_map) {
     absl::string_view type_url = p.first;
     const ResourceMetadataMap& resource_metadata_map = p.second;
-    type_url_storage.emplace_back(
+    type_url_storage->emplace_back(
         absl::StrCat("type.googleapis.com/", type_url));
     for (const auto& q : resource_metadata_map) {
       absl::string_view resource_name = q.first;
       const ResourceMetadata& metadata = *q.second;
-      auto* entry =
+      envoy_service_status_v3_ClientConfig_GenericXdsConfig* entry =
           envoy_service_status_v3_ClientConfig_add_generic_xds_configs(
               client_config, context.arena);
       envoy_service_status_v3_ClientConfig_GenericXdsConfig_set_type_url(
-          entry, StdStringToUpbString(type_url_storage.back()));
+          entry, StdStringToUpbString(type_url_storage->back()));
       envoy_service_status_v3_ClientConfig_GenericXdsConfig_set_name(
           entry, StdStringToUpbString(resource_name));
       envoy_service_status_v3_ClientConfig_GenericXdsConfig_set_client_status(
@@ -628,7 +625,7 @@ std::string XdsApi::AssembleClientConfig(
             envoy_service_status_v3_ClientConfig_GenericXdsConfig_mutable_xds_config(
                 entry, context.arena);
         google_protobuf_Any_set_type_url(
-            any_field, StdStringToUpbString(type_url_storage.back()));
+            any_field, StdStringToUpbString(type_url_storage->back()));
         google_protobuf_Any_set_value(
             any_field, StdStringToUpbString(metadata.serialized_proto));
       }
@@ -649,11 +646,7 @@ std::string XdsApi::AssembleClientConfig(
       }
     }
   }
-  // Serialize the upb message to bytes
-  size_t output_length;
-  char* output = envoy_service_status_v3_ClientConfig_serialize(
-      client_config, arena.ptr(), &output_length);
-  return std::string(output, output_length);
+  return client_config;
 }
 
 }  // namespace grpc_core

src/core/ext/xds/xds_api.h

@@ -30,6 +30,7 @@
 #include "absl/status/status.h"
 #include "absl/strings/string_view.h"
 #include "envoy/admin/v3/config_dump_shared.upb.h"
+#include "envoy/service/status/v3/csds.upb.h"
 #include "upb/mem/arena.h"
 #include "upb/reflection/def.hpp"
 
@@ -176,9 +177,11 @@ class XdsApi {
                                 std::set<std::string>* cluster_names,
                                 Duration* load_reporting_interval);
 
-  // Assemble the client config proto message and return the serialized result.
-  std::string AssembleClientConfig(
-      const ResourceTypeMetadataMap& resource_type_metadata_map);
+  // Assemble and return the client config proto message.
+  envoy_service_status_v3_ClientConfig* AssembleClientConfig(
+      envoy_service_status_v3_ClientConfig* client_config,
+      const ResourceTypeMetadataMap& resource_type_metadata_map,
+      std::vector<std::string>* type_url_storage, upb_Arena* arena);
 
  private:
   XdsClient* client_;

src/core/ext/xds/xds_client.cc

@@ -2032,7 +2032,7 @@ XdsApi::ClusterLoadReportMap XdsClient::BuildLoadReportSnapshotLocked(
   return snapshot_map;
 }
 
-std::string XdsClient::DumpClientConfigBinary() {
+XdsApi::ResourceTypeMetadataMap XdsClient::BuildResourceTypeMetadataMap() {
   MutexLock lock(&mu_);
   XdsApi::ResourceTypeMetadataMap resource_type_metadata_map;
   for (const auto& a : authority_state_map_) {  // authority
@@ -2049,8 +2049,16 @@ std::string XdsClient::DumpClientConfigBinary() {
       }
     }
   }
+  return resource_type_metadata_map;
+}
+
+void XdsClient::DumpClientConfig(
+    envoy_service_status_v3_ClientConfig* client_config,
+    const XdsApi::ResourceTypeMetadataMap& resource_type_metadata_map,
+    std::vector<std::string>* strings_holder, upb_Arena* arena) {
   // Assemble config dump messages
-  return api_.AssembleClientConfig(resource_type_metadata_map);
+  api_.AssembleClientConfig(client_config, resource_type_metadata_map,
+                            strings_holder, arena);
 }
 
 }  // namespace grpc_core

src/core/ext/xds/xds_client.h

@@ -147,14 +147,19 @@ class XdsClient : public DualRefCounted<XdsClient> {
   // Resets connection backoff state.
   void ResetBackoff();
 
-  // Dumps the active xDS config in JSON format.
+  // Dumps the active xDS config.
   // Individual xDS resource is encoded as envoy.admin.v3.*ConfigDump. Returns
   // envoy.service.status.v3.ClientConfig which also includes the config
   // status (e.g., CLIENT_REQUESTED, CLIENT_ACKED, CLIENT_NACKED).
   //
   // Expected to be invoked by wrapper languages in their CSDS service
   // implementation.
-  std::string DumpClientConfigBinary();
+  void DumpClientConfig(
+      envoy_service_status_v3_ClientConfig* client_config,
+      const XdsApi::ResourceTypeMetadataMap& resource_type_metadata_map,
+      std::vector<std::string>* strings_holder, upb_Arena* arena);
+
+  XdsApi::ResourceTypeMetadataMap BuildResourceTypeMetadataMap();
 
   grpc_event_engine::experimental::EventEngine* engine() {
     return engine_.get();

src/core/ext/xds/xds_client_grpc.cc

@@ -22,12 +22,15 @@
 #include <memory>
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "absl/base/thread_annotations.h"
 #include "absl/status/status.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
+#include "envoy/service/status/v3/csds.upb.h"
+#include "upb/base/string_view.h"
 
 #include <grpc/grpc.h>
 #include <grpc/impl/channel_arg_names.h>
@@ -36,9 +39,12 @@
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 
+#include "src/core/ext/xds/upb_utils.h"
+#include "src/core/ext/xds/xds_api.h"
 #include "src/core/ext/xds/xds_bootstrap.h"
 #include "src/core/ext/xds/xds_bootstrap_grpc.h"
 #include "src/core/ext/xds/xds_channel_args.h"
+#include "src/core/ext/xds/xds_client.h"
 #include "src/core/ext/xds/xds_transport.h"
 #include "src/core/ext/xds/xds_transport_grpc.h"
 #include "src/core/lib/channel/channel_args.h"
@@ -235,9 +241,23 @@ grpc_slice grpc_dump_xds_configs(void) {
   grpc_core::ExecCtx exec_ctx;
   auto xds_client = grpc_core::GrpcXdsClient::GetOrCreate(
       grpc_core::ChannelArgs(), "grpc_dump_xds_configs()");
-  if (!xds_client.ok()) {
-    // If we aren't using xDS, just return an empty string.
-    return grpc_empty_slice();
+  upb::Arena arena;
+  // Following two containers should survive till serialization
+  std::vector<std::string> type_url_storage;
+  auto response = envoy_service_status_v3_ClientStatusResponse_new(arena.ptr());
+  // If we aren't using xDS, just return a response with empty config. This is
+  // consistent with older gRPC implementation
+  auto client_config = envoy_service_status_v3_ClientStatusResponse_add_config(
+      response, arena.ptr());
+  if (xds_client.ok()) {
+    auto metadata_map = (*xds_client)->BuildResourceTypeMetadataMap();
+    (*xds_client)
+        ->DumpClientConfig(client_config, metadata_map, &type_url_storage,
+                           arena.ptr());
   }
-  return grpc_slice_from_cpp_string((*xds_client)->DumpClientConfigBinary());
+  // Serialize the upb message to bytes
+  size_t output_length;
+  char* output = envoy_service_status_v3_ClientStatusResponse_serialize(
+      response, arena.ptr(), &output_length);
+  return grpc_slice_from_cpp_string(std::string(output, output_length));
 }

src/cpp/server/csds/csds.cc

@@ -34,19 +34,18 @@ namespace grpc {
 namespace xds {
 namespace experimental {
 
-using envoy::service::status::v3::ClientConfig;
 using envoy::service::status::v3::ClientStatusRequest;
 using envoy::service::status::v3::ClientStatusResponse;
 
 namespace {
 
-absl::StatusOr<ClientConfig> DumpClientConfig() {
-  ClientConfig client_config;
+absl::StatusOr<ClientStatusResponse> DumpClientStatusResponse() {
+  ClientStatusResponse client_config;
   grpc_slice serialized_client_config = grpc_dump_xds_configs();
   std::string bytes = StringFromCopiedSlice(serialized_client_config);
   grpc_slice_unref(serialized_client_config);
   if (!client_config.ParseFromString(bytes)) {
-    return absl::InternalError("Failed to parse ClientConfig.");
+    return absl::InternalError("Failed to parse ClientStatusResponse.");
   }
   return client_config;
 }
@@ -59,7 +58,7 @@ Status ClientStatusDiscoveryService::StreamClientStatus(
   ClientStatusRequest request;
   while (stream->Read(&request)) {
     ClientStatusResponse response;
-    absl::StatusOr<ClientConfig> s = DumpClientConfig();
+    absl::StatusOr<ClientStatusResponse> s = DumpClientStatusResponse();
     if (!s.ok()) {
       if (s.status().code() == absl::StatusCode::kUnavailable) {
         // If the xDS client is not initialized, return empty response
@@ -69,7 +68,7 @@ Status ClientStatusDiscoveryService::StreamClientStatus(
       return Status(static_cast<StatusCode>(s.status().raw_code()),
                     s.status().ToString());
     }
-    *response.add_config() = std::move(s.value());
+    response = s.value();
     stream->Write(response);
   }
   return Status::OK;
@@ -78,7 +77,7 @@ Status ClientStatusDiscoveryService::StreamClientStatus(
 Status ClientStatusDiscoveryService::FetchClientStatus(
     ServerContext* /*context*/, const ClientStatusRequest* /*request*/,
     ClientStatusResponse* response) {
-  absl::StatusOr<ClientConfig> s = DumpClientConfig();
+  absl::StatusOr<ClientStatusResponse> s = DumpClientStatusResponse();
   if (!s.ok()) {
     if (s.status().code() == absl::StatusCode::kUnavailable) {
       // If the xDS client is not initialized, return empty response
@@ -87,7 +86,7 @@ Status ClientStatusDiscoveryService::FetchClientStatus(
     return Status(static_cast<StatusCode>(s.status().raw_code()),
                   s.status().ToString());
   }
-  *response->add_config() = std::move(s.value());
+  *response = s.value();
   return Status::OK;
 }
 

src/python/grpcio_csds/grpc_csds/__init__.py

@@ -26,12 +26,7 @@ class ClientStatusDiscoveryServiceServicer(
 
     @staticmethod
     def FetchClientStatus(request, unused_context):
-        client_config = csds_pb2.ClientConfig.FromString(
-            cygrpc.dump_xds_configs()
-        )
-        response = csds_pb2.ClientStatusResponse()
-        response.config.append(client_config)
-        return response
+        return csds_pb2.ClientStatusResponse.FromString(cygrpc.dump_xds_configs())
 
     @staticmethod
     def StreamClientStatus(request_iterator, context):

test/core/xds/xds_client_fuzzer.cc

@@ -113,7 +113,7 @@ class Fuzzer {
         }
         break;
       case xds_client_fuzzer::Action::kDumpCsdsData:
-        xds_client_->DumpClientConfigBinary();
+        DumpClientConfigBinary();
         break;
       case xds_client_fuzzer::Action::kTriggerConnectionFailure:
         TriggerConnectionFailure(
@@ -225,6 +225,15 @@ class Fuzzer {
     return &bootstrap.server();
   }
 
+  void DumpClientConfigBinary() {
+    upb::Arena arena;
+    auto client_config = envoy_service_status_v3_ClientConfig_new(arena.ptr());
+    auto metadata_map = xds_client_->BuildResourceTypeMetadataMap();
+    std::vector<std::string> strings_holder;
+    xds_client_->DumpClientConfig(client_config, metadata_map, &strings_holder,
+                                  arena.ptr());
+  }
+
   void TriggerConnectionFailure(const std::string& authority,
                                 absl::Status status) {
     gpr_log(GPR_INFO, "### TriggerConnectionFailure(%s): %s", authority.c_str(),