Merge remote-tracking branch 'upstream/master' into clang_format_fix

CMakeLists.txt

@@ -29994,6 +29994,7 @@ target_include_directories(test_core_security_credentials_test
 target_link_libraries(test_core_security_credentials_test
   ${_gRPC_ALLTARGETS_LIBRARIES}
   gtest
+  absl::check
   grpc_test_util
 )
 

bazel/grpc_build_system.bzl

@@ -117,6 +117,7 @@ def _update_visibility(visibility):
         "iomgr_internal_errqueue": PRIVATE,
         "iomgr_buffer_list": PRIVATE,
         "json_reader_legacy": PRIVATE,
+        "otel_plugin": PRIVATE,
         "public": PUBLIC,
         "ref_counted_ptr": PRIVATE,
         "tcp_tracer": PRIVATE,

build_autogenerated.yaml

@@ -19037,6 +19037,7 @@ targets:
   - test/core/util/tracer_util.cc
   deps:
   - gtest
+  - absl/log:check
   - grpc_test_util
 - name: test_core_security_ssl_credentials_test
   gtest: true

src/core/lib/transport/handshaker_factory.h

@@ -58,6 +58,22 @@ class HandshakerFactory {
     // Handshakers that are responsible for post connect security handshakes.
     // Applicable for both Client and Server handshakers.
     kSecurityHandshakers,
+    // TEMPORARY HACK -- DO NOT USE
+    // Currently, handshakers that need to hijack the endpoint's fd and
+    // exit early (which generally run at priority kSecurityHandshakers)
+    // need to call grpc_tcp_destroy_and_release_fd(), which asserts
+    // that the endpoint is an iomgr endpoint.  If another handshaker
+    // has wrapped the endpoint before then, this assertion fails.  So
+    // for now, we introduce a new priority here for handshakers that
+    // need to wrap the endpoint, to make sure that they run after
+    // handshakers that hijack the fd and exit early.
+    // TODO(hork): As part of migrating to the EventEngine endpoint API,
+    // remove this priority.  In the EE API, handshakers that want to
+    // hijack the fd will do so via the query interface, so we can just
+    // have any wrapper endpoints forward query interfaces to the wrapped
+    // endpoint, so that it's not a problem if the endpoint is wrapped
+    // before a handshaker needs to hijack the fd.
+    kTemporaryHackDoNotUseEndpointWrappingHandshakers,
   };
 
   virtual void AddHandshakers(const ChannelArgs& args,

src/core/resolver/.clang-format

@@ -5,9 +5,6 @@ DerivePointerAlignment: false
 PointerAlignment: Left
 IncludeBlocks: Regroup
 IncludeCategories:
-  # port_platform.h is before almost everything
-  - Regex: '^<grpc/(support|impl/codegen)/port_platform.h>'
-    Priority: -100
   # ruby.h is even more first if it's included
   - Regex: '^<ruby/ruby.h>'
     Priority: -200

src/cpp/ext/otel/BUILD

@@ -57,7 +57,7 @@ grpc_cc_library(
         "otel/api",
     ],
     language = "c++",
-    visibility = ["//:__subpackages__"],
+    visibility = ["@grpc:otel_plugin"],
     deps = [
         "//:call_tracer",
         "//:config",

test/core/security/BUILD

@@ -95,7 +95,10 @@ grpc_cc_test(
 grpc_cc_test(
     name = "credentials_test",
     srcs = ["credentials_test.cc"],
-    external_deps = ["gtest"],
+    external_deps = [
+        "absl/log:check",
+        "gtest",
+    ],
     language = "C++",
     deps = [
         "//:gpr",

test/core/transport/test_suite/test.h

@@ -130,9 +130,9 @@ PromiseSpawner SpawnerForContext(
 template <typename Arg>
 using NextSpawner = absl::AnyInvocable<void(Arg)>;
 
-template <typename R>
+template <typename R, typename P>
 Promise<Empty> WrapPromiseAndNext(std::shared_ptr<ActionState> action_state,
-                                  Promise<R> promise, NextSpawner<R> next) {
+                                  P promise, NextSpawner<R> next) {
   return Promise<Empty>(OnCancel(
       [action_state, promise = std::move(promise),
        next = std::move(next)]() mutable -> Poll<Empty> {
@@ -172,8 +172,7 @@ NextSpawner<Arg> WrapFollowUps(NameAndLocation loc,
     action_state->Set(ActionState::kNotStarted);
     spawner(name,
             WrapPromiseAndNext(std::move(action_state),
-                               Promise<Result>(factory.Make(std::move(arg))),
-                               std::move(next)));
+                               factory.Make(std::move(arg)), std::move(next)));
   };
 }
 
@@ -191,9 +190,9 @@ void StartSeq(NameAndLocation loc, ActionStateFactory action_state_factory,
       [spawner, first = Factory(std::move(first)), next = std::move(next),
        action_state = std::move(action_state), name = loc.name()]() mutable {
         action_state->Set(ActionState::kNotStarted);
+        auto promise = first.Make();
         spawner(name, WrapPromiseAndNext(std::move(action_state),
-                                         Promise<Result>(first.Make()),
-                                         std::move(next)));
+                                         std::move(promise), std::move(next)));
         return Empty{};
       });
 }

test/cpp/util/grpc_tool.cc

@@ -25,9 +25,12 @@
 #include <sstream>
 #include <string>
 #include <thread>
+#include <utility>
 
 #include "absl/flags/flag.h"
-#include "absl/memory/memory.h"
+#include "absl/strings/numbers.h"
+#include "absl/strings/str_split.h"
+#include "absl/strings/string_view.h"
 
 #include <grpc/grpc.h>
 #include <grpc/support/port_platform.h>
@@ -82,6 +85,9 @@ ABSL_FLAG(
     int, max_recv_msg_size, 0,
     "Specify the max receive message size in bytes for all RPCs. -1 indicates "
     "unlimited. The default value of 0 means to use the gRPC default.");
+ABSL_FLAG(std::string, channel_args, "",
+          "Comma-separated list of key=value gRPC ChannelArgs to apply "
+          "(a=b,c=d,...). Values may be integers or strings.");
 
 namespace grpc {
 namespace testing {
@@ -244,6 +250,20 @@ std::shared_ptr<grpc::Channel> CreateCliChannel(
   // See |GRPC_ARG_MAX_METADATA_SIZE| in |grpc_types.h|.
   // Set to large enough size (10M) that should work for most use cases.
   args.SetInt(GRPC_ARG_MAX_METADATA_SIZE, 10 * 1024 * 1024);
+
+  // Extend channel args according to flag --channel_args.
+  const auto flag = absl::GetFlag(FLAGS_channel_args);
+  for (absl::string_view arg :
+       absl::StrSplit(flag, ',', absl::SkipWhitespace())) {
+    std::pair<std::string, std::string> kv =
+        absl::StrSplit(arg, absl::MaxSplits('=', 1), absl::SkipWhitespace());
+    int ival;
+    if (absl::SimpleAtoi(kv.second, &ival)) {
+      args.SetInt(kv.first, ival);
+    } else if (!kv.second.empty()) {
+      args.SetString(kv.first, kv.second);
+    }
+  }
   return grpc::CreateCustomChannel(server_address, cred.GetCredentials(), args);
 }
 

test/cpp/util/grpc_tool_test.cc

@@ -133,6 +133,7 @@ ABSL_DECLARE_FLAG(std::string, proto_path);
 ABSL_DECLARE_FLAG(std::string, default_service_config);
 ABSL_DECLARE_FLAG(double, timeout);
 ABSL_DECLARE_FLAG(int, max_recv_msg_size);
+ABSL_DECLARE_FLAG(std::string, channel_args);
 
 namespace grpc {
 namespace testing {
@@ -1299,11 +1300,11 @@ TEST_F(GrpcToolTest, CallMaxRecvMessageSizeSmall) {
 
   // Set max_recv_msg_size to 4 which is not enough.
   absl::SetFlag(&FLAGS_max_recv_msg_size, 4);
-
-  // This should fail.
   EXPECT_FALSE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
                                     std::bind(PrintStream, &output_stream,
                                               std::placeholders::_1)));
+  absl::SetFlag(&FLAGS_max_recv_msg_size, 0);
+
   // No output expected.
   EXPECT_TRUE(0 == output_stream.tellp());
   ShutdownServer();
@@ -1320,10 +1321,11 @@ TEST_F(GrpcToolTest, CallMaxRecvMessageSizeEnough) {
 
   // Set max_recv_msg_size to a large enough number.
   absl::SetFlag(&FLAGS_max_recv_msg_size, 1024 * 1024);
-
   EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
                                    std::bind(PrintStream, &output_stream,
                                              std::placeholders::_1)));
+  absl::SetFlag(&FLAGS_max_recv_msg_size, 0);
+
   // Expected output: "message: \"Hello\""
   EXPECT_TRUE(nullptr !=
               strstr(output_stream.str().c_str(), "message: \"Hello\""));
@@ -1341,10 +1343,10 @@ TEST_F(GrpcToolTest, CallMaxRecvMessageSizeDefault) {
 
   // Set max_recv_msg_size to gRPC default, which should suffice.
   absl::SetFlag(&FLAGS_max_recv_msg_size, 0);
-
   EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
                                    std::bind(PrintStream, &output_stream,
                                              std::placeholders::_1)));
+
   // Expected output: "message: \"Hello\""
   EXPECT_TRUE(nullptr !=
               strstr(output_stream.str().c_str(), "message: \"Hello\""));
@@ -1362,16 +1364,46 @@ TEST_F(GrpcToolTest, CallMaxRecvMessageSizeUnlimited) {
 
   // Set max_recv_msg_size to unlimited (-1), which should work.
   absl::SetFlag(&FLAGS_max_recv_msg_size, -1);
-
   EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
                                    std::bind(PrintStream, &output_stream,
                                              std::placeholders::_1)));
+  absl::SetFlag(&FLAGS_max_recv_msg_size, 0);
+
   // Expected output: "message: \"Hello\""
   EXPECT_TRUE(nullptr !=
               strstr(output_stream.str().c_str(), "message: \"Hello\""));
   ShutdownServer();
 }
 
+// This duplicates CallMaxRecvMessageSizeSmall, but using --channel_args.
+TEST_F(GrpcToolTest, CallWithChannelArgs) {
+  std::stringstream output_stream;
+  const std::string server_address = SetUpServer();
+  // Test input "grpc_cli call localhost:10000 Echo "message: 'Hello'
+  // --channel_args=grpc.max_receive_message_length=4"
+  const char* argv[] = {"grpc_cli", "call", server_address.c_str(),
+                        "grpc.testing.EchoTestService.Echo",
+                        "message: 'Hello'"};
+
+  // Set max receive size to 4 bytes which is not enough.
+  absl::SetFlag(&FLAGS_channel_args,
+                "x=y,grpc.max_receive_message_length=4,z=1");
+  EXPECT_FALSE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
+                                    std::bind(PrintStream, &output_stream,
+                                              std::placeholders::_1)));
+  absl::SetFlag(&FLAGS_channel_args, "");
+
+  // Then try again with large enough size, which should succeed.
+  absl::SetFlag(&FLAGS_channel_args,
+                ",grpc.max_receive_message_length=1000000,");
+  EXPECT_TRUE(0 == GrpcToolMainLib(ArraySize(argv), argv, TestCliCredentials(),
+                                   std::bind(PrintStream, &output_stream,
+                                             std::placeholders::_1)));
+  absl::SetFlag(&FLAGS_channel_args, "");
+
+  ShutdownServer();
+}
+
 TEST_F(GrpcToolTest, ListCommandOverrideSslHostName) {
   const std::string server_address = SetUpServer(true);