From d9903aa44efad4e8493c4f9acb2cb1790cc4bfd9 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 8 Mar 2021 17:11:58 -0800 Subject: [PATCH] Format Channelz Address.TcpIpAddress.address as packed bytes (#25629) * Move address packing to Core * Format * Use absl::Base64Escape * Update src/core/lib/channel/channelz.cc Co-authored-by: Yash Tibrewal Co-authored-by: Yash Tibrewal --- src/core/lib/channel/channelz.cc | 8 ++++++-- src/core/lib/iomgr/sockaddr_utils.cc | 19 +++++++++++++++++++ src/core/lib/iomgr/sockaddr_utils.h | 3 +++ .../tests/channelz/_channelz_servicer_test.py | 10 ++++++++++ test/cpp/end2end/channelz_service_test.cc | 14 ++++++++++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 6f1869c9eb0..22279b1c531 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -19,6 +19,8 @@ #include #include "src/core/lib/channel/channelz.h" +#include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/sockaddr_utils.h" #include "absl/strings/escaping.h" #include "absl/strings/strip.h" @@ -434,12 +436,14 @@ void PopulateSocketAddressJson(Json::Object* json, const char* name, if (!port.empty()) { port_num = atoi(port.data()); } - char* b64_host = grpc_base64_encode(host.data(), host.size(), false, false); + grpc_resolved_address resolved_host; + grpc_string_to_sockaddr(&resolved_host, host.c_str(), port_num); + std::string packed_host = grpc_sockaddr_get_packed_host(&resolved_host); + std::string b64_host = absl::Base64Escape(packed_host); data["tcpip_address"] = Json::Object{ {"port", port_num}, {"ip_address", b64_host}, }; - gpr_free(b64_host); } else if (uri.ok() && uri->scheme() == "unix") { data["uds_address"] = Json::Object{ {"filename", uri->path()}, diff --git a/src/core/lib/iomgr/sockaddr_utils.cc b/src/core/lib/iomgr/sockaddr_utils.cc index 692893c0f39..333358abd48 100644 --- a/src/core/lib/iomgr/sockaddr_utils.cc +++ b/src/core/lib/iomgr/sockaddr_utils.cc @@ -294,3 +294,22 @@ int grpc_sockaddr_set_port(grpc_resolved_address* resolved_addr, int port) { return 0; } } + +std::string grpc_sockaddr_get_packed_host( + const grpc_resolved_address* resolved_addr) { + const grpc_sockaddr* addr = + reinterpret_cast(resolved_addr->addr); + if (addr->sa_family == GRPC_AF_INET) { + const grpc_sockaddr_in* addr4 = + reinterpret_cast(addr); + const char* addr_bytes = reinterpret_cast(&addr4->sin_addr); + return std::string(addr_bytes, 4); + } else if (addr->sa_family == GRPC_AF_INET6) { + const grpc_sockaddr_in6* addr6 = + reinterpret_cast(addr); + const char* addr_bytes = reinterpret_cast(&addr6->sin6_addr); + return std::string(addr_bytes, 16); + } else { + GPR_ASSERT(false); + } +} diff --git a/src/core/lib/iomgr/sockaddr_utils.h b/src/core/lib/iomgr/sockaddr_utils.h index ba91e56d008..e5297c63e53 100644 --- a/src/core/lib/iomgr/sockaddr_utils.h +++ b/src/core/lib/iomgr/sockaddr_utils.h @@ -77,4 +77,7 @@ const char* grpc_sockaddr_get_uri_scheme(const grpc_resolved_address* addr); int grpc_sockaddr_get_family(const grpc_resolved_address* resolved_addr); +std::string grpc_sockaddr_get_packed_host( + const grpc_resolved_address* resolved_addr); + #endif /* GRPC_CORE_LIB_IOMGR_SOCKADDR_UTILS_H */ diff --git a/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py b/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py index 784307ae005..a5008970293 100644 --- a/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py +++ b/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py @@ -350,6 +350,15 @@ class ChannelzServicerTest(unittest.TestCase): self.assertEqual(gsc_resp.subchannel.data.calls_succeeded, gs_resp.socket.data.messages_received) + if gs_resp.socket.remote.HasField("tcpip_address"): + address = gs_resp.socket.remote.tcpip_address.ip_address + self.assertTrue( + len(address) == 4 or len(address) == 16, address) + if gs_resp.socket.local.HasField("tcpip_address"): + address = gs_resp.socket.local.tcpip_address.ip_address + self.assertTrue( + len(address) == 4 or len(address) == 16, address) + def test_streaming_rpc(self): self._pairs = _generate_channel_server_pairs(1) # In C++, the argument for _send_successful_stream_stream is message length. @@ -413,6 +422,7 @@ class ChannelzServicerTest(unittest.TestCase): gs_resp = self._channelz_stub.GetSocket( channelz_pb2.GetSocketRequest( socket_id=gss_resp.server[0].listen_socket[0].socket_id)) + # If the RPC call failed, it will raise a grpc.RpcError # So, if there is no exception raised, considered pass diff --git a/test/cpp/end2end/channelz_service_test.cc b/test/cpp/end2end/channelz_service_test.cc index a9e901ee674..53f0bf58306 100644 --- a/test/cpp/end2end/channelz_service_test.cc +++ b/test/cpp/end2end/channelz_service_test.cc @@ -47,6 +47,7 @@ #include +using grpc::channelz::v1::Address; using grpc::channelz::v1::GetChannelRequest; using grpc::channelz::v1::GetChannelResponse; using grpc::channelz::v1::GetServerRequest; @@ -66,6 +67,14 @@ namespace grpc { namespace testing { namespace { +static bool ValidateAddress(const Address& address) { + if (address.address_case() != Address::kTcpipAddress) { + return true; + } + return address.tcpip_address().ip_address().size() == 4 || + address.tcpip_address().ip_address().size() == 16; +} + // Proxy service supports N backends. Sends RPC to backend dictated by // request->backend_channel_idx(). class Proxy : public ::grpc::testing::EchoTestService::Service { @@ -787,6 +796,8 @@ TEST_P(ChannelzServerTest, GetServerSocketsTest) { s = channelz_stub_->GetSocket(&get_socket_context, get_socket_request, &get_socket_response); EXPECT_TRUE(s.ok()) << "s.error_message() = " << s.error_message(); + EXPECT_TRUE(ValidateAddress(get_socket_response.socket().remote())); + EXPECT_TRUE(ValidateAddress(get_socket_response.socket().local())); switch (GetParam()) { case CredentialsType::kInsecure: EXPECT_FALSE(get_socket_response.socket().has_security()); @@ -898,6 +909,9 @@ TEST_P(ChannelzServerTest, GetServerListenSocketsTest) { s = channelz_stub_->GetSocket(&get_socket_context_1, get_socket_request, &get_socket_response); EXPECT_TRUE(s.ok()) << "s.error_message() = " << s.error_message(); + + EXPECT_TRUE(ValidateAddress(get_socket_response.socket().remote())); + EXPECT_TRUE(ValidateAddress(get_socket_response.socket().local())); if (listen_socket_size == 2) { get_socket_request.set_socket_id( get_server_response.server(0).listen_socket(1).socket_id());