From 691199ab80cc3aaaac66e9014988a272cd2934bc Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 9 Jun 2022 13:51:39 -0700 Subject: [PATCH] xDS Istop Interop: Add differentiation on the protocol being used (#29965) * xDS Istop Interop: Add differentiation on the protocol being used * Fixes * clang-tidy --- test/cpp/interop/istio_echo_server.cc | 103 +++++++++++++++++----- test/cpp/interop/istio_echo_server_lib.cc | 42 +++++---- 2 files changed, 106 insertions(+), 39 deletions(-) diff --git a/test/cpp/interop/istio_echo_server.cc b/test/cpp/interop/istio_echo_server.cc index 40deed78997..70b54763451 100644 --- a/test/cpp/interop/istio_echo_server.cc +++ b/test/cpp/interop/istio_echo_server.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -40,9 +41,11 @@ #include #include #include +#include #include "src/core/lib/channel/status_util.h" #include "src/core/lib/gpr/env.h" +#include "src/core/lib/gprpp/host_port.h" #include "src/core/lib/iomgr/gethostname.h" #include "src/proto/grpc/testing/istio_echo.pb.h" #include "test/core/util/test_config.h" @@ -52,6 +55,13 @@ // A list of ports to listen on, for gRPC traffic. ABSL_FLAG(std::vector, grpc, std::vector({"7070"}), "GRPC ports"); +ABSL_FLAG(std::vector, tls, std::vector({}), + "Ports that are using TLS. These must be defined as http/grpc/tcp."); +ABSL_FLAG(std::vector, xds_grpc_server, + std::vector({}), + "Ports that should rely on XDS configuration to serve"); +ABSL_FLAG(std::string, crt, "", "gRPC TLS server-side certificate"); +ABSL_FLAG(std::string, key, "", "gRPC TLS server-side key"); // The following flags must be defined, but are not used for now. Some may be // necessary for certain tests. @@ -59,32 +69,26 @@ ABSL_FLAG(std::vector, port, std::vector({"8080"}), "HTTP/1.1 ports"); ABSL_FLAG(std::vector, tcp, std::vector({"9090"}), "TCP ports"); -ABSL_FLAG(std::vector, tls, std::vector({""}), - "Ports that are using TLS. These must be defined as http/grpc/tcp."); -ABSL_FLAG(std::vector, bind_ip, std::vector({""}), +ABSL_FLAG(std::vector, bind_ip, std::vector({}), "Ports that are bound to INSTANCE_IP rather than wildcard IP."); ABSL_FLAG(std::vector, bind_localhost, - std::vector({""}), + std::vector({}), "Ports that are bound to localhost rather than wildcard IP."); -ABSL_FLAG(std::vector, server_first, - std::vector({""}), +ABSL_FLAG(std::vector, server_first, std::vector({}), "Ports that are server first. These must be defined as tcp."); -ABSL_FLAG(std::vector, xds_grpc_server, - std::vector({""}), - "Ports that should rely on XDS configuration to serve"); ABSL_FLAG(std::string, metrics, "", "Metrics port"); ABSL_FLAG(std::string, uds, "", "HTTP server on unix domain socket"); ABSL_FLAG(std::string, cluster, "", "Cluster where this server is deployed"); -ABSL_FLAG(std::string, crt, "", "gRPC TLS server-side certificate"); -ABSL_FLAG(std::string, key, "", "gRPC TLS server-side key"); ABSL_FLAG(std::string, istio_version, "", "Istio sidecar version"); ABSL_FLAG(std::string, disable_alpn, "", "disable ALPN negotiation"); namespace grpc { namespace testing { namespace { -/*std::vector>*/ -void RunServer(std::vector ports) { + +void RunServer(std::vector grpc_ports, std::set xds_ports, + std::set tls_ports) { + // Get hostname std::string hostname; char* hostname_p = grpc_gethostname(); if (hostname_p == nullptr) { @@ -95,20 +99,49 @@ void RunServer(std::vector ports) { } EchoTestServiceImpl echo_test_service(hostname); ServerBuilder builder; + XdsServerBuilder xds_builder; + bool has_xds_listeners = false; builder.RegisterService(&echo_test_service); - for (int port : ports) { - std::ostringstream server_address; - server_address << "0.0.0.0:" << port; - builder.AddListeningPort(server_address.str(), - grpc::InsecureServerCredentials()); - gpr_log(GPR_DEBUG, "Server listening on %s", server_address.str().c_str()); + // Create Credentials for Tls Servers - + // 1. Uses FileWatcherCertificateProvider with a refresh interval of 600 + // seconds. (Number decided based on gRPC defaults. + // 2. Do not ask for client certificates. (Not yet sure what is needed right + // now.) + experimental::TlsServerCredentialsOptions options( + std::make_shared( + absl::GetFlag(FLAGS_key), absl::GetFlag(FLAGS_crt), 600)); + options.set_cert_request_type(GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE); + options.watch_identity_key_cert_pairs(); + options.set_check_call_host(false); + auto tls_creds = TlsServerCredentials(options); + // Add ports to the builders + for (int port : grpc_ports) { + auto server_address = grpc_core::JoinHostPort("0.0.0.0", port); + if (xds_ports.find(port) != xds_ports.end()) { + xds_builder.AddListeningPort( + server_address, XdsServerCredentials(InsecureServerCredentials())); + gpr_log(GPR_INFO, "Server listening on %s over xds", + server_address.c_str()); + has_xds_listeners = true; + } else if (tls_ports.find(port) != tls_ports.end()) { + builder.AddListeningPort(server_address, tls_creds); + gpr_log(GPR_INFO, "Server listening on %s over tls", + server_address.c_str()); + } else { + builder.AddListeningPort(server_address, InsecureServerCredentials()); + gpr_log(GPR_INFO, "Server listening on %s over insecure", + server_address.c_str()); + } + } + // Enable the default health check service, probably not needed though. + grpc::EnableDefaultHealthCheckService(true); + std::unique_ptr xds_server; + if (has_xds_listeners) { + xds_server = xds_builder.BuildAndStart(); } - // 3333 is the magic port that the istio testing for k8s health checks. And // it only needs TCP. So also make the gRPC server to listen on 3333. - std::ostringstream server_address_3333; - server_address_3333 << "0.0.0.0:" << 3333; - builder.AddListeningPort(server_address_3333.str(), + builder.AddListeningPort(grpc_core::JoinHostPort("0.0.0.0", 3333), grpc::InsecureServerCredentials()); std::unique_ptr server(builder.BuildAndStart()); server->Wait(); @@ -124,6 +157,7 @@ int main(int argc, char** argv) { // "--grpc=8080,9090". // 2. replace '-' to '_'. So "--istio-version=123" becomes // "--istio_version=123". + // 3. remove --version since that is specially interpretted by absl std::map> argv_dict; for (int i = 0; i < argc; i++) { std::string arg(argv[i]); @@ -165,6 +199,27 @@ int main(int argc, char** argv) { int grpc_port = std::stoi(p); grpc_ports.push_back(grpc_port); } - grpc::testing::RunServer(grpc_ports); + // Create a map of which ports are supposed to use xds + std::set xds_ports; + for (const auto& p : absl::GetFlag(FLAGS_xds_grpc_server)) { + int port = 0; + if (!absl::SimpleAtoi(p, &port)) { + gpr_log(GPR_ERROR, "SimpleAtoi Failure: %s", p.c_str()); + return 1; + } + xds_ports.insert(port); + } + // Create a map of which ports are supposed to use tls + std::set tls_ports; + for (const auto& p : absl::GetFlag(FLAGS_tls)) { + int port = 0; + if (!absl::SimpleAtoi(p, &port)) { + gpr_log(GPR_ERROR, "SimpleAtoi Failure: %s", p.c_str()); + return 1; + } + tls_ports.insert(port); + } + // Start the servers + grpc::testing::RunServer(grpc_ports, xds_ports, tls_ports); return 0; } diff --git a/test/cpp/interop/istio_echo_server_lib.cc b/test/cpp/interop/istio_echo_server_lib.cc index e572076d408..5a279443336 100644 --- a/test/cpp/interop/istio_echo_server_lib.cc +++ b/test/cpp/interop/istio_echo_server_lib.cc @@ -97,9 +97,7 @@ Status EchoTestServiceImpl::Echo(ServerContext* context, absl::StrAppend(&s, kStatusCodeField, "=", std::to_string(200), "\n"); absl::StrAppend(&s, kHostnameField, "=", this->hostname_, "\n"); absl::StrAppend(&s, "Echo=", request->message(), "\n"); - gpr_log(GPR_ERROR, "here"); response->set_message(s); - gpr_log(GPR_ERROR, "here"); gpr_log(GPR_INFO, "Echo response:\n%s", s.c_str()); return Status::OK; } @@ -109,20 +107,34 @@ Status EchoTestServiceImpl::ForwardEcho(ServerContext* /*context*/, ForwardEchoResponse* response) { std::string raw_url = request->url(); size_t colon = raw_url.find_first_of(':'); - if (colon != std::string::npos) { - std::string scheme = raw_url.substr(0, colon); - if (scheme != "grpc") { - gpr_log(GPR_ERROR, "Protocol %s not supported", scheme.c_str()); - return Status( - StatusCode::UNIMPLEMENTED, - absl::StrFormat("Protocol %s not supported", scheme.c_str())); - } + std::string scheme; + if (colon == std::string::npos) { + return Status( + StatusCode::INVALID_ARGUMENT, + absl::StrFormat("No protocol configured for url %s", raw_url)); + } + scheme = raw_url.substr(0, colon); + std::shared_ptr channel; + if (scheme == "xds") { + // We can optionally add support for TLS creds, but we are primarily + // concerned with proxyless-grpc here. + gpr_log(GPR_INFO, "Creating channel to %s using xDS Creds", + raw_url.c_str()); + channel = + CreateChannel(raw_url, XdsCredentials(InsecureChannelCredentials())); + } else if (scheme == "grpc") { + // We don't really want to test this but the istio test infrastructure needs + // this to be supported. If we ever decide to add support for this properly, + // we would need to add support for TLS creds here. + absl::string_view address = absl::StripPrefix(raw_url, "grpc://"); + gpr_log(GPR_INFO, "Creating channel to %s", std::string(address).c_str()); + channel = CreateChannel(std::string(address), InsecureChannelCredentials()); + } else { + std::string status_msg = + absl::StrFormat("Protocol %s not supported", scheme); + gpr_log(GPR_ERROR, "Protocol %s not supported", status_msg.c_str()); + return Status(StatusCode::UNIMPLEMENTED, status_msg); } - // May need to use xds security if urlScheme is "xds" - absl::string_view address = absl::StripPrefix(raw_url, "grpc://"); - gpr_log(GPR_INFO, "Creating channel to %s", std::string(address).c_str()); - auto channel = - CreateChannel(std::string(address), InsecureChannelCredentials()); auto stub = EchoTestService::NewStub(channel); auto count = request->count() == 0 ? 1 : request->count(); // Calculate the amount of time to sleep after each call.