From c60ba7e77fdabe407cdbeee227088d191aaa7674 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 11 Jun 2020 12:04:46 -0700 Subject: [PATCH 01/34] Add back source-only ruby packages for master --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 3b23d0b1b40..a488eb03f0b 100755 --- a/Rakefile +++ b/Rakefile @@ -142,7 +142,7 @@ task 'gem:native' do gem update --system --no-document && \ bundle && \ - rake native:#{plat} pkg/#{spec.full_name}-#{plat}.gem \ + rake native:#{plat} pkg/#{spec.full_name}-#{plat}.gem pkg/#{spec.full_name}.gem \ RUBY_CC_VERSION=2.7.0:2.6.0:2.5.0:2.4.0:2.3.0 \ V=#{verbose} \ GRPC_CONFIG=#{grpc_config} From 08e117ae806225fd704ab4221c1014b5fa495482 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 17 Jun 2020 16:12:59 -0700 Subject: [PATCH 02/34] Improve build slightly --- setup.py | 1 + src/python/grpcio/_parallel_compile_patch.py | 5 ++- src/python/grpcio/commands.py | 45 +++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 807d005899d..3f27ebd4c14 100644 --- a/setup.py +++ b/setup.py @@ -383,6 +383,7 @@ COMMAND_CLASS = { 'build_py': commands.BuildPy, 'build_ext': commands.BuildExt, 'gather': commands.Gather, + 'clean': commands.Clean, } # Ensure that package data is copied over before any commands have been run: diff --git a/src/python/grpcio/_parallel_compile_patch.py b/src/python/grpcio/_parallel_compile_patch.py index b34aa17fd0b..e4d50c38311 100644 --- a/src/python/grpcio/_parallel_compile_patch.py +++ b/src/python/grpcio/_parallel_compile_patch.py @@ -22,7 +22,10 @@ import os try: BUILD_EXT_COMPILER_JOBS = int( - os.environ.get('GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS', '1')) + os.environ['GRPC_PYTHON_BUILD_EXT_COMPILER_JOBS']) +except KeyError: + import multiprocessing + BUILD_EXT_COMPILER_JOBS = multiprocessing.cpu_count() except ValueError: BUILD_EXT_COMPILER_JOBS = 1 diff --git a/src/python/grpcio/commands.py b/src/python/grpcio/commands.py index b9106826b5d..6d7a286632d 100644 --- a/src/python/grpcio/commands.py +++ b/src/python/grpcio/commands.py @@ -13,6 +13,8 @@ # limitations under the License. """Provides distutils command classes for the GRPC Python setup process.""" +from __future__ import print_function + import distutils import glob import os @@ -218,7 +220,10 @@ class BuildExt(build_ext.build_ext): if platform.system() != 'Windows': return False # TODO(lidiz) Remove the generated a.out for success tests. - cc_test = subprocess.Popen(['cc', '-x', 'c', '-std=c++11', '-'], + cc_test = subprocess.Popen([ + distutils.ccompiler.executable_filename, '-x', 'c', + '-std=c++11', '-' + ], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -290,3 +295,41 @@ class Gather(setuptools.Command): self.distribution.install_requires) if self.test and self.distribution.tests_require: self.distribution.fetch_build_eggs(self.distribution.tests_require) + + +class Clean(setuptools.Command): + """Command to clean build artifacts.""" + + description = 'Clean build artifacts.' + user_options = [] + + _FILE_PATTERNS = [ + 'python_build', + 'src/python/grpcio/__pycache__/', + 'src/python/grpcio/grpc/_cython/cygrpc.cpp', + 'src/python/grpcio/grpc/_cython/*.so', + 'src/python/grpcio/grpcio.egg-info/', + ] + _CURRENT_DIRECTORY = os.path.normpath( + os.path.join(os.path.dirname(os.path.realpath(__file__)), "../../..")) + + def initialize_options(self): + pass + + def finalize_options(self): + pass + + def run(self): + for path_spec in self._FILE_PATTERNS: + this_glob = os.path.normpath( + os.path.join(Clean._CURRENT_DIRECTORY, path_spec)) + abs_paths = glob.glob(this_glob) + for path in abs_paths: + if not str(path).startswith(Clean._CURRENT_DIRECTORY): + raise ValueError( + "Cowardly refusing to delete {}.".format(path)) + print("Removing {}".format(os.path.relpath(path))) + if os.path.isfile(path): + os.remove(str(path)) + else: + shutil.rmtree(str(path)) From f22e247183876cffbb3768eb73199fd31eb277fd Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 17 Jun 2020 22:00:15 -0700 Subject: [PATCH 03/34] Add script to install all Python modules --- tools/distrib/install_all_python_modules.sh | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100755 tools/distrib/install_all_python_modules.sh diff --git a/tools/distrib/install_all_python_modules.sh b/tools/distrib/install_all_python_modules.sh new file mode 100755 index 00000000000..fd036cb9a5e --- /dev/null +++ b/tools/distrib/install_all_python_modules.sh @@ -0,0 +1,25 @@ +#!/bin/bash + +echo "It's recommended that you run this script from a virtual environment." + +set -e + +BASEDIR=$(dirname "$0") +BASEDIR=$(realpath "$BASEDIR")/../.. + +(cd "$BASEDIR"; + pip install cython; + python setup.py install; + pushd tools/distrib/python/grpcio_tools; + ../make_grpcio_tools.py + GRPC_PYTHON_BUILD_WITH_CYTHON=1 pip install . + popd; + pushd src/python; + for PACKAGE in ./grpcio_*; do + pushd "${PACKAGE}"; + python setup.py preprocess; + python setup.py install; + popd; + done + popd; +) From edcf2100503e4aa658df1f57b37973c05cb4370b Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 17 Jun 2020 22:00:54 -0700 Subject: [PATCH 04/34] Use proper compiler --- src/python/grpcio/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio/commands.py b/src/python/grpcio/commands.py index 6d7a286632d..61332995393 100644 --- a/src/python/grpcio/commands.py +++ b/src/python/grpcio/commands.py @@ -221,7 +221,7 @@ class BuildExt(build_ext.build_ext): return False # TODO(lidiz) Remove the generated a.out for success tests. cc_test = subprocess.Popen([ - distutils.ccompiler.executable_filename, '-x', 'c', + self.compiler.compiler[0], '-x', 'c', '-std=c++11', '-' ], stdin=subprocess.PIPE, From af222241a7f7adb8ab49ea4a027c7464cab3e25e Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Thu, 18 Jun 2020 10:40:31 -0700 Subject: [PATCH 05/34] Give up on getting compiler executable from distutils --- src/python/grpcio/commands.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/python/grpcio/commands.py b/src/python/grpcio/commands.py index 61332995393..dc441a2efd6 100644 --- a/src/python/grpcio/commands.py +++ b/src/python/grpcio/commands.py @@ -220,10 +220,7 @@ class BuildExt(build_ext.build_ext): if platform.system() != 'Windows': return False # TODO(lidiz) Remove the generated a.out for success tests. - cc_test = subprocess.Popen([ - self.compiler.compiler[0], '-x', 'c', - '-std=c++11', '-' - ], + cc_test = subprocess.Popen(['cc', '-x', 'c', '-std=c++11', '-'], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) From 98bb4b612448303eadd2282746303e08e6665fbb Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 19 Jun 2020 09:54:04 -0700 Subject: [PATCH 06/34] Copyright header --- tools/distrib/install_all_python_modules.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tools/distrib/install_all_python_modules.sh b/tools/distrib/install_all_python_modules.sh index fd036cb9a5e..51a11341bb6 100755 --- a/tools/distrib/install_all_python_modules.sh +++ b/tools/distrib/install_all_python_modules.sh @@ -1,4 +1,17 @@ #!/bin/bash +# Copyright 2020 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. echo "It's recommended that you run this script from a virtual environment." From 4fcf9d01d204bd03e9f4b364130a18796cc8fd6e Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 19 Jun 2020 15:31:55 -0700 Subject: [PATCH 07/34] Address review comments --- src/python/grpcio/commands.py | 4 ++-- tools/distrib/install_all_python_modules.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/python/grpcio/commands.py b/src/python/grpcio/commands.py index dc441a2efd6..fb678eb2d53 100644 --- a/src/python/grpcio/commands.py +++ b/src/python/grpcio/commands.py @@ -300,13 +300,13 @@ class Clean(setuptools.Command): description = 'Clean build artifacts.' user_options = [] - _FILE_PATTERNS = [ + _FILE_PATTERNS = ( 'python_build', 'src/python/grpcio/__pycache__/', 'src/python/grpcio/grpc/_cython/cygrpc.cpp', 'src/python/grpcio/grpc/_cython/*.so', 'src/python/grpcio/grpcio.egg-info/', - ] + ) _CURRENT_DIRECTORY = os.path.normpath( os.path.join(os.path.dirname(os.path.realpath(__file__)), "../../..")) diff --git a/tools/distrib/install_all_python_modules.sh b/tools/distrib/install_all_python_modules.sh index 51a11341bb6..8189a0ffc3b 100755 --- a/tools/distrib/install_all_python_modules.sh +++ b/tools/distrib/install_all_python_modules.sh @@ -21,7 +21,7 @@ BASEDIR=$(dirname "$0") BASEDIR=$(realpath "$BASEDIR")/../.. (cd "$BASEDIR"; - pip install cython; + pip install --upgrade cython; python setup.py install; pushd tools/distrib/python/grpcio_tools; ../make_grpcio_tools.py From 5be0b22dc0ac2dc9864b057d5096ec9725b81313 Mon Sep 17 00:00:00 2001 From: Patrice Chalin Date: Tue, 16 Jun 2020 09:08:18 -0400 Subject: [PATCH 08/34] Update links to grpc.io guides in header files --- include/grpcpp/grpcpp.h | 2 +- include/grpcpp/impl/codegen/client_context_impl.h | 2 +- include/grpcpp/security/credentials_impl.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/grpcpp/grpcpp.h b/include/grpcpp/grpcpp.h index aa8a24203bc..923def19ace 100644 --- a/include/grpcpp/grpcpp.h +++ b/include/grpcpp/grpcpp.h @@ -21,7 +21,7 @@ /// The gRPC C++ API mainly consists of the following classes: ///
/// - grpc::Channel, which represents the connection to an endpoint. See [the -/// gRPC Concepts page](https://grpc.io/docs/guides/concepts.html) for more +/// gRPC Concepts page](https://grpc.io/docs/what-is-grpc/core-concepts) for more /// details. Channels are created by the factory function grpc::CreateChannel. /// /// - grpc::CompletionQueue, the producer-consumer queue used for all diff --git a/include/grpcpp/impl/codegen/client_context_impl.h b/include/grpcpp/impl/codegen/client_context_impl.h index b8e90722b25..f44b1f764bb 100644 --- a/include/grpcpp/impl/codegen/client_context_impl.h +++ b/include/grpcpp/impl/codegen/client_context_impl.h @@ -319,7 +319,7 @@ class ClientContext { /// /// It is legal to call this only before initial metadata is sent. /// - /// \see https://grpc.io/docs/guides/auth.html + /// \see https://grpc.io/docs/guides/auth void set_credentials( const std::shared_ptr& creds); diff --git a/include/grpcpp/security/credentials_impl.h b/include/grpcpp/security/credentials_impl.h index aed58283722..2148cf8d32b 100644 --- a/include/grpcpp/security/credentials_impl.h +++ b/include/grpcpp/security/credentials_impl.h @@ -62,7 +62,7 @@ std::shared_ptr CreateCustomChannelWithInterceptors( /// It can make various assertions, e.g., about the client’s identity, role /// for all the calls on that channel. /// -/// \see https://grpc.io/docs/guides/auth.html +/// \see https://grpc.io/docs/guides/auth class ChannelCredentials : private grpc::GrpcLibraryCodegen { public: ChannelCredentials(); @@ -107,7 +107,7 @@ class ChannelCredentials : private grpc::GrpcLibraryCodegen { /// A call credentials object encapsulates the state needed by a client to /// authenticate with a server for a given call on a channel. /// -/// \see https://grpc.io/docs/guides/auth.html +/// \see https://grpc.io/docs/guides/auth class CallCredentials : private grpc::GrpcLibraryCodegen { public: CallCredentials(); From a5c9b22cc9867262787c209a1897e9284264b988 Mon Sep 17 00:00:00 2001 From: Patrice Chalin Date: Mon, 22 Jun 2020 11:39:03 -0400 Subject: [PATCH 09/34] Fix the grpcpp.h formatting. --- include/grpcpp/grpcpp.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/grpcpp/grpcpp.h b/include/grpcpp/grpcpp.h index 923def19ace..41dc09581cd 100644 --- a/include/grpcpp/grpcpp.h +++ b/include/grpcpp/grpcpp.h @@ -21,8 +21,9 @@ /// The gRPC C++ API mainly consists of the following classes: ///
/// - grpc::Channel, which represents the connection to an endpoint. See [the -/// gRPC Concepts page](https://grpc.io/docs/what-is-grpc/core-concepts) for more -/// details. Channels are created by the factory function grpc::CreateChannel. +/// gRPC Concepts page](https://grpc.io/docs/what-is-grpc/core-concepts) for +/// more details. Channels are created by the factory function +/// grpc::CreateChannel. /// /// - grpc::CompletionQueue, the producer-consumer queue used for all /// asynchronous communication with the gRPC runtime. From 39103342cf4fc2d3694ee7618d668581439c08b0 Mon Sep 17 00:00:00 2001 From: Srini Polavarapu Date: Mon, 22 Jun 2020 18:55:24 -0700 Subject: [PATCH 10/34] v1.30.0 interop tests --- tools/interop_matrix/client_matrix.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/interop_matrix/client_matrix.py b/tools/interop_matrix/client_matrix.py index e38c1c77ee9..a37cf9d189f 100644 --- a/tools/interop_matrix/client_matrix.py +++ b/tools/interop_matrix/client_matrix.py @@ -105,6 +105,7 @@ LANG_RELEASE_MATRIX = { ('v1.25.0', ReleaseInfo()), ('v1.26.0', ReleaseInfo()), ('v1.27.3', ReleaseInfo()), + ('v1.30.0', ReleaseInfo()), ]), 'go': OrderedDict([ @@ -276,6 +277,7 @@ LANG_RELEASE_MATRIX = { ('v1.25.0', ReleaseInfo(runtimes=['python'])), ('v1.26.0', ReleaseInfo(runtimes=['python'])), ('v1.27.3', ReleaseInfo(runtimes=['python'])), + ('v1.30.0', ReleaseInfo(runtimes=['python'])), ]), 'node': OrderedDict([ @@ -334,6 +336,7 @@ LANG_RELEASE_MATRIX = { # go ahead and upload the docker image for new releases. ('v1.26.0', ReleaseInfo()), ('v1.27.3', ReleaseInfo()), + ('v1.30.0', ReleaseInfo()), ]), 'php': OrderedDict([ @@ -365,6 +368,7 @@ LANG_RELEASE_MATRIX = { ('v1.25.0', ReleaseInfo()), ('v1.26.0', ReleaseInfo()), ('v1.27.3', ReleaseInfo()), + ('v1.30.0', ReleaseInfo()), ]), 'csharp': OrderedDict([ @@ -401,5 +405,6 @@ LANG_RELEASE_MATRIX = { ('v1.25.0', ReleaseInfo()), ('v1.26.0', ReleaseInfo()), ('v1.27.3', ReleaseInfo()), + ('v1.30.0', ReleaseInfo()), ]), } From e98eaa5052a0f2b0299591dc05760805df04656c Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 23 Jun 2020 12:19:07 -0700 Subject: [PATCH 11/34] Revert "Revert "Add message-size check before message decompression"" --- BUILD | 2 + BUILD.gn | 1 + CMakeLists.txt | 2 + Makefile | 2 + build_autogenerated.yaml | 2 + config.m4 | 1 + config.w32 | 1 + gRPC-Core.podspec | 1 + grpc.gemspec | 1 + grpc.gyp | 2 + package.xml | 1 + .../service_config_channel_arg_filter.cc | 142 ++++++++ .../ext/filters/http/http_filters_plugin.cc | 3 +- .../message_decompress_filter.cc | 92 +++-- .../message_decompress_filter.h | 4 +- .../message_size/message_size_filter.cc | 106 +++--- .../message_size/message_size_filter.h | 6 + .../plugin_registry/grpc_plugin_registry.cc | 4 + .../grpc_unsecure_plugin_registry.cc | 4 + src/python/grpcio/grpc_core_dependencies.py | 1 + test/core/end2end/tests/max_message_length.cc | 332 ++++++++++++++++++ test/cpp/microbenchmarks/bm_call_create.cc | 3 +- tools/doxygen/Doxyfile.c++.internal | 1 + tools/doxygen/Doxyfile.core.internal | 1 + 24 files changed, 622 insertions(+), 93 deletions(-) create mode 100644 src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc diff --git a/BUILD b/BUILD index 13d3d6883d8..32bb6512ec4 100644 --- a/BUILD +++ b/BUILD @@ -1045,6 +1045,7 @@ grpc_cc_library( "src/core/ext/filters/client_channel/retry_throttle.cc", "src/core/ext/filters/client_channel/server_address.cc", "src/core/ext/filters/client_channel/service_config.cc", + "src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc", "src/core/ext/filters/client_channel/service_config_parser.cc", "src/core/ext/filters/client_channel/subchannel.cc", "src/core/ext/filters/client_channel/subchannel_pool_interface.cc", @@ -1186,6 +1187,7 @@ grpc_cc_library( language = "c++", deps = [ "grpc_base", + "grpc_message_size_filter", ], ) diff --git a/BUILD.gn b/BUILD.gn index 0b72deafa94..f2e173ad9c5 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -295,6 +295,7 @@ config("grpc_config") { "src/core/ext/filters/client_channel/service_config.cc", "src/core/ext/filters/client_channel/service_config.h", "src/core/ext/filters/client_channel/service_config_call_data.h", + "src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc", "src/core/ext/filters/client_channel/service_config_parser.cc", "src/core/ext/filters/client_channel/service_config_parser.h", "src/core/ext/filters/client_channel/subchannel.cc", diff --git a/CMakeLists.txt b/CMakeLists.txt index 59387045c7d..3fa1c7bb8c1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1376,6 +1376,7 @@ add_library(grpc src/core/ext/filters/client_channel/retry_throttle.cc src/core/ext/filters/client_channel/server_address.cc src/core/ext/filters/client_channel/service_config.cc + src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc src/core/ext/filters/client_channel/service_config_parser.cc src/core/ext/filters/client_channel/subchannel.cc src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -2048,6 +2049,7 @@ add_library(grpc_unsecure src/core/ext/filters/client_channel/retry_throttle.cc src/core/ext/filters/client_channel/server_address.cc src/core/ext/filters/client_channel/service_config.cc + src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc src/core/ext/filters/client_channel/service_config_parser.cc src/core/ext/filters/client_channel/subchannel.cc src/core/ext/filters/client_channel/subchannel_pool_interface.cc diff --git a/Makefile b/Makefile index 0f1e72c2eaa..675c7613d50 100644 --- a/Makefile +++ b/Makefile @@ -3679,6 +3679,7 @@ LIBGRPC_SRC = \ src/core/ext/filters/client_channel/retry_throttle.cc \ src/core/ext/filters/client_channel/server_address.cc \ src/core/ext/filters/client_channel/service_config.cc \ + src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/subchannel.cc \ src/core/ext/filters/client_channel/subchannel_pool_interface.cc \ @@ -4325,6 +4326,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/ext/filters/client_channel/retry_throttle.cc \ src/core/ext/filters/client_channel/server_address.cc \ src/core/ext/filters/client_channel/service_config.cc \ + src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/subchannel.cc \ src/core/ext/filters/client_channel/subchannel_pool_interface.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 3ac9f3c74b9..fac33c197a2 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -795,6 +795,7 @@ libs: - src/core/ext/filters/client_channel/retry_throttle.cc - src/core/ext/filters/client_channel/server_address.cc - src/core/ext/filters/client_channel/service_config.cc + - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc - src/core/ext/filters/client_channel/service_config_parser.cc - src/core/ext/filters/client_channel/subchannel.cc - src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -1656,6 +1657,7 @@ libs: - src/core/ext/filters/client_channel/retry_throttle.cc - src/core/ext/filters/client_channel/server_address.cc - src/core/ext/filters/client_channel/service_config.cc + - src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc - src/core/ext/filters/client_channel/service_config_parser.cc - src/core/ext/filters/client_channel/subchannel.cc - src/core/ext/filters/client_channel/subchannel_pool_interface.cc diff --git a/config.m4 b/config.m4 index 5ee9f1c8d58..a6bf9a21b6c 100644 --- a/config.m4 +++ b/config.m4 @@ -93,6 +93,7 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/filters/client_channel/retry_throttle.cc \ src/core/ext/filters/client_channel/server_address.cc \ src/core/ext/filters/client_channel/service_config.cc \ + src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/subchannel.cc \ src/core/ext/filters/client_channel/subchannel_pool_interface.cc \ diff --git a/config.w32 b/config.w32 index 1c533a33302..03cf0684684 100644 --- a/config.w32 +++ b/config.w32 @@ -62,6 +62,7 @@ if (PHP_GRPC != "no") { "src\\core\\ext\\filters\\client_channel\\retry_throttle.cc " + "src\\core\\ext\\filters\\client_channel\\server_address.cc " + "src\\core\\ext\\filters\\client_channel\\service_config.cc " + + "src\\core\\ext\\filters\\client_channel\\service_config_channel_arg_filter.cc " + "src\\core\\ext\\filters\\client_channel\\service_config_parser.cc " + "src\\core\\ext\\filters\\client_channel\\subchannel.cc " + "src\\core\\ext\\filters\\client_channel\\subchannel_pool_interface.cc " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index aaf5609a9c3..43a47a09fa7 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -279,6 +279,7 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/service_config.cc', 'src/core/ext/filters/client_channel/service_config.h', 'src/core/ext/filters/client_channel/service_config_call_data.h', + 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/service_config_parser.h', 'src/core/ext/filters/client_channel/subchannel.cc', diff --git a/grpc.gemspec b/grpc.gemspec index 881f55a81c2..72ec55f0c86 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -201,6 +201,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/client_channel/service_config.cc ) s.files += %w( src/core/ext/filters/client_channel/service_config.h ) s.files += %w( src/core/ext/filters/client_channel/service_config_call_data.h ) + s.files += %w( src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc ) s.files += %w( src/core/ext/filters/client_channel/service_config_parser.cc ) s.files += %w( src/core/ext/filters/client_channel/service_config_parser.h ) s.files += %w( src/core/ext/filters/client_channel/subchannel.cc ) diff --git a/grpc.gyp b/grpc.gyp index e8bed76e891..587e6ba8d94 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -488,6 +488,7 @@ 'src/core/ext/filters/client_channel/retry_throttle.cc', 'src/core/ext/filters/client_channel/server_address.cc', 'src/core/ext/filters/client_channel/service_config.cc', + 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/subchannel.cc', 'src/core/ext/filters/client_channel/subchannel_pool_interface.cc', @@ -996,6 +997,7 @@ 'src/core/ext/filters/client_channel/retry_throttle.cc', 'src/core/ext/filters/client_channel/server_address.cc', 'src/core/ext/filters/client_channel/service_config.cc', + 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/subchannel.cc', 'src/core/ext/filters/client_channel/subchannel_pool_interface.cc', diff --git a/package.xml b/package.xml index 262c66a6ac3..7553fe27739 100644 --- a/package.xml +++ b/package.xml @@ -181,6 +181,7 @@ + diff --git a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc new file mode 100644 index 00000000000..6b8e6373af7 --- /dev/null +++ b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc @@ -0,0 +1,142 @@ +// +// Copyright 2020 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// This filter reads GRPC_ARG_SERVICE_CONFIG and populates ServiceConfigCallData +// in the call context per call for direct channels. + +#include + +#include "src/core/ext/filters/client_channel/service_config_call_data.h" +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/channel/channel_stack_builder.h" +#include "src/core/lib/surface/channel_init.h" + +namespace grpc_core { + +namespace { + +class ServiceConfigChannelArgChannelData { + public: + explicit ServiceConfigChannelArgChannelData( + const grpc_channel_element_args* args) { + const char* service_config_str = grpc_channel_args_find_string( + args->channel_args, GRPC_ARG_SERVICE_CONFIG); + if (service_config_str != nullptr) { + grpc_error* service_config_error = GRPC_ERROR_NONE; + auto service_config = + ServiceConfig::Create(service_config_str, &service_config_error); + if (service_config_error == GRPC_ERROR_NONE) { + service_config_ = std::move(service_config); + } else { + gpr_log(GPR_ERROR, "%s", grpc_error_string(service_config_error)); + } + GRPC_ERROR_UNREF(service_config_error); + } + } + + RefCountedPtr service_config() const { + return service_config_; + } + + private: + RefCountedPtr service_config_; +}; + +class ServiceConfigChannelArgCallData { + public: + ServiceConfigChannelArgCallData(grpc_call_element* elem, + const grpc_call_element_args* args) { + ServiceConfigChannelArgChannelData* chand = + static_cast(elem->channel_data); + RefCountedPtr service_config = chand->service_config(); + if (service_config != nullptr) { + GPR_DEBUG_ASSERT(args->context != nullptr); + const auto* method_params_vector = + service_config->GetMethodParsedConfigVector(args->path); + args->arena->New( + std::move(service_config), method_params_vector, args->context); + } + } +}; + +grpc_error* ServiceConfigChannelArgInitCallElem( + grpc_call_element* elem, const grpc_call_element_args* args) { + ServiceConfigChannelArgCallData* calld = + static_cast(elem->call_data); + new (calld) ServiceConfigChannelArgCallData(elem, args); + return GRPC_ERROR_NONE; +} + +void ServiceConfigChannelArgDestroyCallElem( + grpc_call_element* elem, const grpc_call_final_info* /* final_info */, + grpc_closure* /* then_schedule_closure */) { + ServiceConfigChannelArgCallData* calld = + static_cast(elem->call_data); + calld->~ServiceConfigChannelArgCallData(); +} + +grpc_error* ServiceConfigChannelArgInitChannelElem( + grpc_channel_element* elem, grpc_channel_element_args* args) { + ServiceConfigChannelArgChannelData* chand = + static_cast(elem->channel_data); + new (chand) ServiceConfigChannelArgChannelData(args); + return GRPC_ERROR_NONE; +} + +void ServiceConfigChannelArgDestroyChannelElem(grpc_channel_element* elem) { + ServiceConfigChannelArgChannelData* chand = + static_cast(elem->channel_data); + chand->~ServiceConfigChannelArgChannelData(); +} + +const grpc_channel_filter ServiceConfigChannelArgFilter = { + grpc_call_next_op, + grpc_channel_next_op, + sizeof(ServiceConfigChannelArgCallData), + ServiceConfigChannelArgInitCallElem, + grpc_call_stack_ignore_set_pollset_or_pollset_set, + ServiceConfigChannelArgDestroyCallElem, + sizeof(ServiceConfigChannelArgChannelData), + ServiceConfigChannelArgInitChannelElem, + ServiceConfigChannelArgDestroyChannelElem, + grpc_channel_next_get_info, + "service_config_channel_arg"}; + +bool maybe_add_service_config_channel_arg_filter( + grpc_channel_stack_builder* builder, void* /* arg */) { + const grpc_channel_args* channel_args = + grpc_channel_stack_builder_get_channel_arguments(builder); + if (grpc_channel_args_want_minimal_stack(channel_args) || + grpc_channel_args_find_string(channel_args, GRPC_ARG_SERVICE_CONFIG) == + nullptr) { + return true; + } + return grpc_channel_stack_builder_prepend_filter( + builder, &ServiceConfigChannelArgFilter, nullptr, nullptr); +} + +} // namespace + +} // namespace grpc_core + +void grpc_service_config_channel_arg_filter_init(void) { + grpc_channel_init_register_stage( + GRPC_CLIENT_DIRECT_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, + grpc_core::maybe_add_service_config_channel_arg_filter, nullptr); +} + +void grpc_service_config_channel_arg_filter_shutdown(void) {} diff --git a/src/core/ext/filters/http/http_filters_plugin.cc b/src/core/ext/filters/http/http_filters_plugin.cc index 2fedc7fe3d7..637dc3030f2 100644 --- a/src/core/ext/filters/http/http_filters_plugin.cc +++ b/src/core/ext/filters/http/http_filters_plugin.cc @@ -38,7 +38,8 @@ static optional_filter compress_filter = { &grpc_message_compress_filter, GRPC_ARG_ENABLE_PER_MESSAGE_COMPRESSION}; static optional_filter decompress_filter = { - &grpc_message_decompress_filter, GRPC_ARG_ENABLE_PER_MESSAGE_DECOMPRESSION}; + &grpc_core::MessageDecompressFilter, + GRPC_ARG_ENABLE_PER_MESSAGE_DECOMPRESSION}; static bool is_building_http_like_transport( grpc_channel_stack_builder* builder) { diff --git a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc index d12f4013bb2..c16ea66e7a3 100644 --- a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc +++ b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc @@ -18,6 +18,8 @@ #include +#include "src/core/ext/filters/http/message_compress/message_decompress_filter.h" + #include #include @@ -27,7 +29,8 @@ #include #include -#include "src/core/ext/filters/http/message_compress/message_decompress_filter.h" +#include "absl/strings/str_format.h" +#include "src/core/ext/filters/message_size/message_size_filter.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/compression/algorithm_metadata.h" #include "src/core/lib/compression/compression_args.h" @@ -37,14 +40,25 @@ #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" +namespace grpc_core { namespace { -class ChannelData {}; +class ChannelData { + public: + explicit ChannelData(const grpc_channel_element_args* args) + : max_recv_size_(GetMaxRecvSizeFromChannelArgs(args->channel_args)) {} + + int max_recv_size() const { return max_recv_size_; } + + private: + int max_recv_size_; +}; class CallData { public: - explicit CallData(const grpc_call_element_args& args) - : call_combiner_(args.call_combiner) { + CallData(const grpc_call_element_args& args, const ChannelData* chand) + : call_combiner_(args.call_combiner), + max_recv_message_length_(chand->max_recv_size()) { // Initialize state for recv_initial_metadata_ready callback GRPC_CLOSURE_INIT(&on_recv_initial_metadata_ready_, OnRecvInitialMetadataReady, this, @@ -59,6 +73,13 @@ class CallData { GRPC_CLOSURE_INIT(&on_recv_trailing_metadata_ready_, OnRecvTrailingMetadataReady, this, grpc_schedule_on_exec_ctx); + const MessageSizeParsedConfig* limits = + MessageSizeParsedConfig::GetFromCallContext(args.context); + if (limits != nullptr && limits->limits().max_recv_size >= 0 && + (limits->limits().max_recv_size < max_recv_message_length_ || + max_recv_message_length_ < 0)) { + max_recv_message_length_ = limits->limits().max_recv_size; + } } ~CallData() { grpc_slice_buffer_destroy_internal(&recv_slices_); } @@ -82,7 +103,7 @@ class CallData { void MaybeResumeOnRecvTrailingMetadataReady(); static void OnRecvTrailingMetadataReady(void* arg, grpc_error* error); - grpc_core::CallCombiner* call_combiner_; + CallCombiner* call_combiner_; // Overall error for the call grpc_error* error_ = GRPC_ERROR_NONE; // Fields for handling recv_initial_metadata_ready callback @@ -91,17 +112,18 @@ class CallData { grpc_metadata_batch* recv_initial_metadata_ = nullptr; // Fields for handling recv_message_ready callback bool seen_recv_message_ready_ = false; + int max_recv_message_length_; grpc_message_compression_algorithm algorithm_ = GRPC_MESSAGE_COMPRESS_NONE; grpc_closure on_recv_message_ready_; grpc_closure* original_recv_message_ready_ = nullptr; grpc_closure on_recv_message_next_done_; - grpc_core::OrphanablePtr* recv_message_ = nullptr; + OrphanablePtr* recv_message_ = nullptr; // recv_slices_ holds the slices read from the original recv_message stream. // It is initialized during construction and reset when a new stream is // created using it. grpc_slice_buffer recv_slices_; - std::aligned_storage::type + std::aligned_storage::type recv_replacement_stream_; // Fields for handling recv_trailing_metadata_ready callback bool seen_recv_trailing_metadata_ready_ = false; @@ -139,7 +161,7 @@ void CallData::OnRecvInitialMetadataReady(void* arg, grpc_error* error) { calld->MaybeResumeOnRecvTrailingMetadataReady(); grpc_closure* closure = calld->original_recv_initial_metadata_ready_; calld->original_recv_initial_metadata_ready_ = nullptr; - grpc_core::Closure::Run(DEBUG_LOCATION, closure, GRPC_ERROR_REF(error)); + Closure::Run(DEBUG_LOCATION, closure, GRPC_ERROR_REF(error)); } void CallData::MaybeResumeOnRecvMessageReady() { @@ -170,6 +192,19 @@ void CallData::OnRecvMessageReady(void* arg, grpc_error* error) { 0) { return calld->ContinueRecvMessageReadyCallback(GRPC_ERROR_NONE); } + if (calld->max_recv_message_length_ >= 0 && + (*calld->recv_message_)->length() > + static_cast(calld->max_recv_message_length_)) { + std::string message_string = absl::StrFormat( + "Received message larger than max (%u vs. %d)", + (*calld->recv_message_)->length(), calld->max_recv_message_length_); + GPR_DEBUG_ASSERT(calld->error_ == GRPC_ERROR_NONE); + calld->error_ = grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_COPIED_STRING(message_string.c_str()), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_RESOURCE_EXHAUSTED); + return calld->ContinueRecvMessageReadyCallback( + GRPC_ERROR_REF(calld->error_)); + } grpc_slice_buffer_destroy_internal(&calld->recv_slices_); grpc_slice_buffer_init(&calld->recv_slices_); return calld->ContinueReadingRecvMessage(); @@ -241,9 +276,9 @@ void CallData::FinishRecvMessage() { // Initializing recv_replacement_stream_ with decompressed_slices removes // all the slices from decompressed_slices leaving it empty. new (&recv_replacement_stream_) - grpc_core::SliceBufferByteStream(&decompressed_slices, recv_flags); - recv_message_->reset(reinterpret_cast( - &recv_replacement_stream_)); + SliceBufferByteStream(&decompressed_slices, recv_flags); + recv_message_->reset( + reinterpret_cast(&recv_replacement_stream_)); recv_message_ = nullptr; } ContinueRecvMessageReadyCallback(GRPC_ERROR_REF(error_)); @@ -254,7 +289,7 @@ void CallData::ContinueRecvMessageReadyCallback(grpc_error* error) { // The surface will clean up the receiving stream if there is an error. grpc_closure* closure = original_recv_message_ready_; original_recv_message_ready_ = nullptr; - grpc_core::Closure::Run(DEBUG_LOCATION, closure, error); + Closure::Run(DEBUG_LOCATION, closure, error); } void CallData::MaybeResumeOnRecvTrailingMetadataReady() { @@ -283,7 +318,7 @@ void CallData::OnRecvTrailingMetadataReady(void* arg, grpc_error* error) { calld->error_ = GRPC_ERROR_NONE; grpc_closure* closure = calld->original_recv_trailing_metadata_ready_; calld->original_recv_trailing_metadata_ready_ = nullptr; - grpc_core::Closure::Run(DEBUG_LOCATION, closure, error); + Closure::Run(DEBUG_LOCATION, closure, error); } void CallData::DecompressStartTransportStreamOpBatch( @@ -322,37 +357,44 @@ void DecompressStartTransportStreamOpBatch( calld->DecompressStartTransportStreamOpBatch(elem, batch); } -static grpc_error* DecompressInitCallElem(grpc_call_element* elem, - const grpc_call_element_args* args) { - new (elem->call_data) CallData(*args); +grpc_error* DecompressInitCallElem(grpc_call_element* elem, + const grpc_call_element_args* args) { + ChannelData* chand = static_cast(elem->channel_data); + new (elem->call_data) CallData(*args, chand); return GRPC_ERROR_NONE; } -static void DecompressDestroyCallElem( - grpc_call_element* elem, const grpc_call_final_info* /*final_info*/, - grpc_closure* /*ignored*/) { +void DecompressDestroyCallElem(grpc_call_element* elem, + const grpc_call_final_info* /*final_info*/, + grpc_closure* /*ignored*/) { CallData* calld = static_cast(elem->call_data); calld->~CallData(); } -static grpc_error* DecompressInitChannelElem( - grpc_channel_element* /*elem*/, grpc_channel_element_args* /*args*/) { +grpc_error* DecompressInitChannelElem(grpc_channel_element* elem, + grpc_channel_element_args* args) { + ChannelData* chand = static_cast(elem->channel_data); + new (chand) ChannelData(args); return GRPC_ERROR_NONE; } -void DecompressDestroyChannelElem(grpc_channel_element* /*elem*/) {} +void DecompressDestroyChannelElem(grpc_channel_element* elem) { + ChannelData* chand = static_cast(elem->channel_data); + chand->~ChannelData(); +} } // namespace -const grpc_channel_filter grpc_message_decompress_filter = { +const grpc_channel_filter MessageDecompressFilter = { DecompressStartTransportStreamOpBatch, grpc_channel_next_op, sizeof(CallData), DecompressInitCallElem, grpc_call_stack_ignore_set_pollset_or_pollset_set, DecompressDestroyCallElem, - 0, // sizeof(ChannelData) + sizeof(ChannelData), DecompressInitChannelElem, DecompressDestroyChannelElem, grpc_channel_next_get_info, "message_decompress"}; +} // namespace grpc_core diff --git a/src/core/ext/filters/http/message_compress/message_decompress_filter.h b/src/core/ext/filters/http/message_compress/message_decompress_filter.h index 7d567bf08a2..f19a4ca0cbd 100644 --- a/src/core/ext/filters/http/message_compress/message_decompress_filter.h +++ b/src/core/ext/filters/http/message_compress/message_decompress_filter.h @@ -23,7 +23,9 @@ #include "src/core/lib/channel/channel_stack.h" -extern const grpc_channel_filter grpc_message_decompress_filter; +namespace grpc_core { +extern const grpc_channel_filter MessageDecompressFilter; +} // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_HTTP_MESSAGE_COMPRESS_MESSAGE_DECOMPRESS_FILTER_H \ */ diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index d2ef5477636..89fdab6fae8 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -45,6 +45,25 @@ namespace { size_t g_message_size_parser_index; } // namespace +// +// MessageSizeParsedConfig +// + +const MessageSizeParsedConfig* MessageSizeParsedConfig::GetFromCallContext( + const grpc_call_context_element* context) { + if (context == nullptr) return nullptr; + auto* svc_cfg_call_data = static_cast( + context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); + if (svc_cfg_call_data == nullptr) return nullptr; + return static_cast( + svc_cfg_call_data->GetMethodParsedConfig( + MessageSizeParser::ParserIndex())); +} + +// +// MessageSizeParser +// + std::unique_ptr MessageSizeParser::ParsePerMethodParams(const Json& json, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); @@ -97,12 +116,26 @@ void MessageSizeParser::Register() { } size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } + +int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args) { + if (grpc_channel_args_want_minimal_stack(args)) return -1; + return grpc_channel_args_find_integer( + args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, + {GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH, -1, INT_MAX}); +} + +int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args) { + if (grpc_channel_args_want_minimal_stack(args)) return -1; + return grpc_channel_args_find_integer( + args, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, + {GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH, -1, INT_MAX}); +} + } // namespace grpc_core namespace { struct channel_data { grpc_core::MessageSizeParsedConfig::message_size_limits limits; - grpc_core::RefCountedPtr svc_cfg; }; struct call_data { @@ -118,24 +151,8 @@ struct call_data { // Note: Per-method config is only available on the client, so we // apply the max request size to the send limit and the max response // size to the receive limit. - const grpc_core::MessageSizeParsedConfig* limits = nullptr; - grpc_core::ServiceConfigCallData* svc_cfg_call_data = nullptr; - if (args.context != nullptr) { - svc_cfg_call_data = static_cast( - args.context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); - } - if (svc_cfg_call_data != nullptr) { - limits = static_cast( - svc_cfg_call_data->GetMethodParsedConfig( - grpc_core::MessageSizeParser::ParserIndex())); - } else if (chand.svc_cfg != nullptr) { - const auto* objs_vector = - chand.svc_cfg->GetMethodParsedConfigVector(args.path); - if (objs_vector != nullptr) { - limits = static_cast( - (*objs_vector)[grpc_core::MessageSizeParser::ParserIndex()].get()); - } - } + const grpc_core::MessageSizeParsedConfig* limits = + grpc_core::MessageSizeParsedConfig::GetFromCallContext(args.context); if (limits != nullptr) { if (limits->limits().max_send_size >= 0 && (limits->limits().max_send_size < this->limits.max_send_size || @@ -288,35 +305,11 @@ static void message_size_destroy_call_elem( calld->~call_data(); } -static int default_size(const grpc_channel_args* args, - int without_minimal_stack) { - if (grpc_channel_args_want_minimal_stack(args)) { - return -1; - } - return without_minimal_stack; -} - grpc_core::MessageSizeParsedConfig::message_size_limits get_message_size_limits( const grpc_channel_args* channel_args) { grpc_core::MessageSizeParsedConfig::message_size_limits lim; - lim.max_send_size = - default_size(channel_args, GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH); - lim.max_recv_size = - default_size(channel_args, GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH); - for (size_t i = 0; i < channel_args->num_args; ++i) { - if (strcmp(channel_args->args[i].key, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH) == - 0) { - const grpc_integer_options options = {lim.max_send_size, -1, INT_MAX}; - lim.max_send_size = - grpc_channel_arg_get_integer(&channel_args->args[i], options); - } - if (strcmp(channel_args->args[i].key, - GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH) == 0) { - const grpc_integer_options options = {lim.max_recv_size, -1, INT_MAX}; - lim.max_recv_size = - grpc_channel_arg_get_integer(&channel_args->args[i], options); - } - } + lim.max_send_size = grpc_core::GetMaxSendSizeFromChannelArgs(channel_args); + lim.max_recv_size = grpc_core::GetMaxRecvSizeFromChannelArgs(channel_args); return lim; } @@ -327,26 +320,6 @@ static grpc_error* message_size_init_channel_elem( channel_data* chand = static_cast(elem->channel_data); new (chand) channel_data(); chand->limits = get_message_size_limits(args->channel_args); - // TODO(yashykt): We only need to read GRPC_ARG_SERVICE_CONFIG in the case of - // direct channels. (Service config is otherwise stored in the call_context by - // client_channel filter.) If we ever need a second filter that also needs to - // parse GRPC_ARG_SERVICE_CONFIG, we should refactor this code and add a - // separate filter that reads GRPC_ARG_SERVICE_CONFIG and saves the parsed - // config in the call_context. - const grpc_arg* channel_arg = - grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVICE_CONFIG); - const char* service_config_str = grpc_channel_arg_get_string(channel_arg); - if (service_config_str != nullptr) { - grpc_error* service_config_error = GRPC_ERROR_NONE; - auto svc_cfg = grpc_core::ServiceConfig::Create(service_config_str, - &service_config_error); - if (service_config_error == GRPC_ERROR_NONE) { - chand->svc_cfg = std::move(svc_cfg); - } else { - gpr_log(GPR_ERROR, "%s", grpc_error_string(service_config_error)); - } - GRPC_ERROR_UNREF(service_config_error); - } return GRPC_ERROR_NONE; } @@ -387,6 +360,9 @@ static bool maybe_add_message_size_filter(grpc_channel_stack_builder* builder, void* /*arg*/) { const grpc_channel_args* channel_args = grpc_channel_stack_builder_get_channel_arguments(builder); + if (grpc_channel_args_want_minimal_stack(channel_args)) { + return true; + } bool enable = false; grpc_core::MessageSizeParsedConfig::message_size_limits lim = get_message_size_limits(channel_args); diff --git a/src/core/ext/filters/message_size/message_size_filter.h b/src/core/ext/filters/message_size/message_size_filter.h index 132d7b2af0f..ea0dd0266d0 100644 --- a/src/core/ext/filters/message_size/message_size_filter.h +++ b/src/core/ext/filters/message_size/message_size_filter.h @@ -40,6 +40,9 @@ class MessageSizeParsedConfig : public ServiceConfigParser::ParsedConfig { const message_size_limits& limits() const { return limits_; } + static const MessageSizeParsedConfig* GetFromCallContext( + const grpc_call_context_element* context); + private: message_size_limits limits_; }; @@ -54,6 +57,9 @@ class MessageSizeParser : public ServiceConfigParser::Parser { static size_t ParserIndex(); }; +int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args); +int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args); + } // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_MESSAGE_SIZE_MESSAGE_SIZE_FILTER_H */ diff --git a/src/core/plugin_registry/grpc_plugin_registry.cc b/src/core/plugin_registry/grpc_plugin_registry.cc index f1c3cbf4036..ef8b10df647 100644 --- a/src/core/plugin_registry/grpc_plugin_registry.cc +++ b/src/core/plugin_registry/grpc_plugin_registry.cc @@ -64,6 +64,8 @@ void grpc_max_age_filter_init(void); void grpc_max_age_filter_shutdown(void); void grpc_message_size_filter_init(void); void grpc_message_size_filter_shutdown(void); +void grpc_service_config_channel_arg_filter_init(void); +void grpc_service_config_channel_arg_filter_shutdown(void); void grpc_client_authority_filter_init(void); void grpc_client_authority_filter_shutdown(void); void grpc_workaround_cronet_compression_filter_init(void); @@ -114,6 +116,8 @@ void grpc_register_built_in_plugins(void) { grpc_max_age_filter_shutdown); grpc_register_plugin(grpc_message_size_filter_init, grpc_message_size_filter_shutdown); + grpc_register_plugin(grpc_service_config_channel_arg_filter_init, + grpc_service_config_channel_arg_filter_shutdown); grpc_register_plugin(grpc_client_authority_filter_init, grpc_client_authority_filter_shutdown); grpc_register_plugin(grpc_workaround_cronet_compression_filter_init, diff --git a/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc b/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc index de53f173294..525fa108d81 100644 --- a/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc +++ b/src/core/plugin_registry/grpc_unsecure_plugin_registry.cc @@ -64,6 +64,8 @@ void grpc_max_age_filter_init(void); void grpc_max_age_filter_shutdown(void); void grpc_message_size_filter_init(void); void grpc_message_size_filter_shutdown(void); +void grpc_service_config_channel_arg_filter_init(void); +void grpc_service_config_channel_arg_filter_shutdown(void); void grpc_client_authority_filter_init(void); void grpc_client_authority_filter_shutdown(void); void grpc_workaround_cronet_compression_filter_init(void); @@ -114,6 +116,8 @@ void grpc_register_built_in_plugins(void) { grpc_max_age_filter_shutdown); grpc_register_plugin(grpc_message_size_filter_init, grpc_message_size_filter_shutdown); + grpc_register_plugin(grpc_service_config_channel_arg_filter_init, + grpc_service_config_channel_arg_filter_shutdown); grpc_register_plugin(grpc_client_authority_filter_init, grpc_client_authority_filter_shutdown); grpc_register_plugin(grpc_workaround_cronet_compression_filter_init, diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index a25f10239f6..7689ddedeba 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -71,6 +71,7 @@ CORE_SOURCE_FILES = [ 'src/core/ext/filters/client_channel/retry_throttle.cc', 'src/core/ext/filters/client_channel/server_address.cc', 'src/core/ext/filters/client_channel/service_config.cc', + 'src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc', 'src/core/ext/filters/client_channel/service_config_parser.cc', 'src/core/ext/filters/client_channel/subchannel.cc', 'src/core/ext/filters/client_channel/subchannel_pool_interface.cc', diff --git a/test/core/end2end/tests/max_message_length.cc b/test/core/end2end/tests/max_message_length.cc index 40e752a3d63..256cc982940 100644 --- a/test/core/end2end/tests/max_message_length.cc +++ b/test/core/end2end/tests/max_message_length.cc @@ -29,6 +29,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/transport/metadata.h" #include "test/core/end2end/cq_verifier.h" @@ -466,6 +467,328 @@ static void test_max_message_length_on_response(grpc_end2end_test_config config, grpc_byte_buffer_destroy(response_payload); grpc_byte_buffer_destroy(recv_payload); + grpc_call_unref(c); + if (s != nullptr) grpc_call_unref(s); + cq_verifier_destroy(cqv); + end_test(&f); + config.tear_down_data(&f); +} + +static grpc_metadata gzip_compression_override() { + grpc_metadata gzip_compression_override; + gzip_compression_override.key = GRPC_MDSTR_GRPC_INTERNAL_ENCODING_REQUEST; + gzip_compression_override.value = grpc_slice_from_static_string("gzip"); + memset(&gzip_compression_override.internal_data, 0, + sizeof(gzip_compression_override.internal_data)); + return gzip_compression_override; +} + +// Test receive message limit with compressed request larger than the limit +static void test_max_receive_message_length_on_compressed_request( + grpc_end2end_test_config config, bool minimal_stack) { + gpr_log(GPR_INFO, + "test max receive message length on compressed request with " + "minimal_stack=%d", + minimal_stack); + grpc_end2end_test_fixture f; + grpc_call* c = nullptr; + grpc_call* s = nullptr; + cq_verifier* cqv; + grpc_op ops[6]; + grpc_op* op; + grpc_slice request_payload_slice = grpc_slice_malloc(1024); + memset(GRPC_SLICE_START_PTR(request_payload_slice), 'a', 1024); + grpc_byte_buffer* request_payload = + grpc_raw_byte_buffer_create(&request_payload_slice, 1); + grpc_byte_buffer* recv_payload = nullptr; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + grpc_slice details, status_details; + int was_cancelled = 2; + + // Set limit via channel args. + grpc_arg arg[2]; + arg[0] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH), 5); + arg[1] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_MINIMAL_STACK), minimal_stack); + grpc_channel_args* server_args = + grpc_channel_args_copy_and_add(nullptr, arg, 2); + + f = begin_test(config, "test_max_request_message_length", nullptr, + server_args); + { + grpc_core::ExecCtx exec_ctx; + grpc_channel_args_destroy(server_args); + } + cqv = cq_verifier_create(f.cq); + c = grpc_channel_create_call(f.client, nullptr, GRPC_PROPAGATE_DEFAULTS, f.cq, + grpc_slice_from_static_string("/service/method"), + nullptr, gpr_inf_future(GPR_CLOCK_REALTIME), + nullptr); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + grpc_metadata compression_md = gzip_compression_override(); + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 1; + op->data.send_initial_metadata.metadata = &compression_md; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message.send_message = request_payload; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->flags = 0; + op->reserved = nullptr; + op++; + error = grpc_call_start_batch(c, ops, static_cast(op - ops), tag(1), + nullptr); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message.recv_message = &recv_payload; + op->flags = 0; + op->reserved = nullptr; + op++; + if (minimal_stack) { + /* Expect the RPC to proceed normally for a minimal stack */ + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_OK; + status_details = grpc_slice_from_static_string("xyz"); + op->data.send_status_from_server.status_details = &status_details; + op->flags = 0; + op->reserved = nullptr; + op++; + } + error = grpc_call_start_batch(s, ops, static_cast(op - ops), tag(102), + nullptr); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/service/method")); + if (minimal_stack) { + /* We do not perform message size checks for minimal stack. */ + GPR_ASSERT(status == GRPC_STATUS_OK); + } else { + GPR_ASSERT(was_cancelled == 1); + GPR_ASSERT(status == GRPC_STATUS_RESOURCE_EXHAUSTED); + GPR_ASSERT(grpc_slice_str_cmp( + details, "Received message larger than max (29 vs. 5)") == + 0); + } + grpc_slice_unref(details); + grpc_slice_unref(request_payload_slice); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + grpc_byte_buffer_destroy(request_payload); + grpc_byte_buffer_destroy(recv_payload); + grpc_call_unref(c); + if (s != nullptr) grpc_call_unref(s); + cq_verifier_destroy(cqv); + + end_test(&f); + config.tear_down_data(&f); +} + +// Test receive message limit with compressed response larger than the limit. +static void test_max_receive_message_length_on_compressed_response( + grpc_end2end_test_config config, bool minimal_stack) { + gpr_log(GPR_INFO, + "testing max receive message length on compressed response with " + "minimal_stack=%d", + minimal_stack); + grpc_end2end_test_fixture f; + grpc_call* c = nullptr; + grpc_call* s = nullptr; + cq_verifier* cqv; + grpc_op ops[6]; + grpc_op* op; + grpc_slice response_payload_slice = grpc_slice_malloc(1024); + memset(GRPC_SLICE_START_PTR(response_payload_slice), 'a', 1024); + grpc_byte_buffer* response_payload = + grpc_raw_byte_buffer_create(&response_payload_slice, 1); + grpc_byte_buffer* recv_payload = nullptr; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + grpc_slice details; + int was_cancelled = 2; + + // Set limit via channel args. + grpc_arg arg[2]; + arg[0] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH), 5); + arg[1] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_MINIMAL_STACK), minimal_stack); + grpc_channel_args* client_args = + grpc_channel_args_copy_and_add(nullptr, arg, 2); + + f = begin_test(config, "test_max_response_message_length", client_args, + nullptr); + { + grpc_core::ExecCtx exec_ctx; + grpc_channel_args_destroy(client_args); + } + cqv = cq_verifier_create(f.cq); + + c = grpc_channel_create_call(f.client, nullptr, GRPC_PROPAGATE_DEFAULTS, f.cq, + grpc_slice_from_static_string("/service/method"), + nullptr, gpr_inf_future(GPR_CLOCK_REALTIME), + nullptr); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message.recv_message = &recv_payload; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->flags = 0; + op->reserved = nullptr; + op++; + error = grpc_call_start_batch(c, ops, static_cast(op - ops), tag(1), + nullptr); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + grpc_metadata compression_md = gzip_compression_override(); + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 1; + op->data.send_initial_metadata.metadata = &compression_md; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message.send_message = response_payload; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_OK; + grpc_slice status_details = grpc_slice_from_static_string("xyz"); + op->data.send_status_from_server.status_details = &status_details; + op->flags = 0; + op->reserved = nullptr; + op++; + error = grpc_call_start_batch(s, ops, static_cast(op - ops), tag(102), + nullptr); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/service/method")); + if (minimal_stack) { + /* We do not perform message size checks for minimal stack. */ + GPR_ASSERT(status == GRPC_STATUS_OK); + } else { + GPR_ASSERT(status == GRPC_STATUS_RESOURCE_EXHAUSTED); + GPR_ASSERT(grpc_slice_str_cmp( + details, "Received message larger than max (29 vs. 5)") == + 0); + } + grpc_slice_unref(details); + grpc_slice_unref(response_payload_slice); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + grpc_byte_buffer_destroy(response_payload); + grpc_byte_buffer_destroy(recv_payload); + grpc_call_unref(c); if (s != nullptr) grpc_call_unref(s); @@ -500,6 +823,15 @@ void max_message_length(grpc_end2end_test_config config) { test_max_message_length_on_response(config, false /* send_limit */, true /* use_service_config */, true /* use_string_json_value */); + /* The following tests are not useful for inproc transport and do not work + * with our simple proxy. */ + if (strcmp(config.name, "inproc") != 0 && + (config.feature_mask & FEATURE_MASK_SUPPORTS_REQUEST_PROXYING) == 0) { + test_max_receive_message_length_on_compressed_request(config, false); + test_max_receive_message_length_on_compressed_request(config, true); + test_max_receive_message_length_on_compressed_response(config, false); + test_max_receive_message_length_on_compressed_response(config, true); + } } void max_message_length_pre_init(void) {} diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index 803ad38f1c5..b4a70272ea6 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -529,9 +529,10 @@ static void BM_IsolatedFilter(benchmark::State& state) { grpc_call_final_info final_info; TestOp test_op_data; const int kArenaSize = 4096; + grpc_call_context_element context[GRPC_CONTEXT_COUNT] = {}; grpc_call_element_args call_args{call_stack, nullptr, - nullptr, + context, method, start_time, deadline, diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 7d56259977e..036af6fcc6b 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1164,6 +1164,7 @@ src/core/ext/filters/client_channel/server_address.h \ src/core/ext/filters/client_channel/service_config.cc \ src/core/ext/filters/client_channel/service_config.h \ src/core/ext/filters/client_channel/service_config_call_data.h \ +src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/service_config_parser.h \ src/core/ext/filters/client_channel/subchannel.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index a4443ebd080..d33ceb61599 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -965,6 +965,7 @@ src/core/ext/filters/client_channel/server_address.h \ src/core/ext/filters/client_channel/service_config.cc \ src/core/ext/filters/client_channel/service_config.h \ src/core/ext/filters/client_channel/service_config_call_data.h \ +src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc \ src/core/ext/filters/client_channel/service_config_parser.cc \ src/core/ext/filters/client_channel/service_config_parser.h \ src/core/ext/filters/client_channel/subchannel.cc \ From bdc83185d37419337df51a2d65e258d0648a6bd9 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 23 Jun 2020 13:59:13 -0700 Subject: [PATCH 12/34] Incrementally remove docker images as we test --- .../run_interop_matrix_tests.py | 117 ++++++++---------- 1 file changed, 52 insertions(+), 65 deletions(-) diff --git a/tools/interop_matrix/run_interop_matrix_tests.py b/tools/interop_matrix/run_interop_matrix_tests.py index 2e11a715029..a68b703f1fd 100755 --- a/tools/interop_matrix/run_interop_matrix_tests.py +++ b/tools/interop_matrix/run_interop_matrix_tests.py @@ -203,69 +203,37 @@ def _generate_test_case_jobspecs(lang, runtime, release, suite_name): return job_spec_list -def _pull_images_for_lang(lang, images): - """Pull all images for given lang from container registry.""" - jobset.message('START', - 'Downloading images for language "%s"' % lang, - do_newline=True) - download_specs = [] - for release, image in images: - # Pull the image and warm it up. - # First time we use an image with "docker run", it takes time to unpack - # the image and later this delay would fail our test cases. - cmdline = [ - 'time gcloud docker -- pull %s && time docker run --rm=true %s /bin/true' - % (image, image) - ] - spec = jobset.JobSpec(cmdline=cmdline, - shortname='pull_image_%s' % (image), - timeout_seconds=_PULL_IMAGE_TIMEOUT_SECONDS, - shell=True, - flake_retries=2) - download_specs.append(spec) - # too many image downloads at once tend to get stuck - max_pull_jobs = min(args.jobs, _MAX_PARALLEL_DOWNLOADS) - num_failures, resultset = jobset.run(download_specs, +# TODO: Return a spec and then parallelize. +def _pull_image_for_lang(lang, image, release): + """Pull an image for a given language form the image registry.""" + cmdline = [ + 'time gcloud docker -- pull %s && time docker run --rm=true %s /bin/true' + % (image, image) + ] + spec = jobset.JobSpec(cmdline=cmdline, + shortname='pull_image_{}'.format(image), + timeout_seconds=_PULL_IMAGE_TIMEOUT_SECONDS, + shell=True, + # TODO: Pull out to constant. + flake_retries=2) + num_failures, resultset = jobset.run([spec], newline_on_success=True, - maxjobs=max_pull_jobs) - if num_failures: - jobset.message('FAILED', - 'Failed to download some images', - do_newline=True) - return False - else: - jobset.message('SUCCESS', - 'All images downloaded successfully.', - do_newline=True) - return True - + maxjobs=1) + return not num_failures -def _run_tests_for_lang(lang, runtime, images, xml_report_tree): - """Find and run all test cases for a language. - - images is a list of (, ) tuple. - """ - skip_tests = False - if not _pull_images_for_lang(lang, images): - jobset.message( - 'FAILED', - 'Image download failed. Skipping tests for language "%s"' % lang, - do_newline=True) - skip_tests = True +def _test_release(lang, runtime, release, image, xml_report_tree, skip_tests): total_num_failures = 0 - for release, image in images: - suite_name = '%s__%s_%s' % (lang, runtime, release) - job_spec_list = _generate_test_case_jobspecs(lang, runtime, release, - suite_name) - - if not job_spec_list: - jobset.message('FAILED', - 'No test cases were found.', - do_newline=True) - total_num_failures += 1 - continue + suite_name = '%s__%s_%s' % (lang, runtime, release) + job_spec_list = _generate_test_case_jobspecs(lang, runtime, release, + suite_name) + if not job_spec_list: + jobset.message('FAILED', + 'No test cases were found.', + do_newline=True) + total_num_failures += 1 + else: num_failures, resultset = jobset.run(job_spec_list, newline_on_success=True, add_env={'docker_image': image}, @@ -275,22 +243,41 @@ def _run_tests_for_lang(lang, runtime, images, xml_report_tree): upload_test_results.upload_interop_results_to_bq( resultset, args.bq_result_table) if skip_tests: - jobset.message('FAILED', 'Tests were skipped', do_newline=True) + jobset.message('FAILED', 'Tests were skipped', + do_newline=True) total_num_failures += 1 - elif num_failures: - jobset.message('FAILED', 'Some tests failed', do_newline=True) + if num_failures: total_num_failures += num_failures - else: - jobset.message('SUCCESS', 'All tests passed', do_newline=True) report_utils.append_junit_xml_results(xml_report_tree, resultset, 'grpc_interop_matrix', suite_name, str(uuid.uuid4())) + return total_num_failures + - # cleanup all downloaded docker images - for _, image in images: +def _run_tests_for_lang(lang, runtime, images, xml_report_tree): + """Find and run all test cases for a language. + + images is a list of (, ) tuple. + """ + skip_tests = False + total_num_failures = 0 + + # TODO: Do more intelligent chunking. + for release, image in images: + if not skip_tests and not _pull_image_for_lang(lang, image, release): + jobset.message( + 'FAILED', + 'Image download failed. Skipping tests for language "%s"' % lang, + do_newline=True) + skip_tests = True + total_num_failures += _test_release(lang, runtime, release, image, xml_report_tree, skip_tests) if not args.keep: _cleanup_docker_image(image) + if not total_num_failures: + jobset.message('SUCCESS', 'All {} tests passed'.format(lang), do_newline=True) + else: + jobset.message('FAILED', 'Some {} tests failed'.format(lang), do_newline=True) return total_num_failures From 1010d3a6199a02c966e762488cea46b1227ef498 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 23 Jun 2020 14:30:30 -0700 Subject: [PATCH 13/34] Batch download jobs --- .../run_interop_matrix_tests.py | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) mode change 100755 => 100644 tools/interop_matrix/run_interop_matrix_tests.py diff --git a/tools/interop_matrix/run_interop_matrix_tests.py b/tools/interop_matrix/run_interop_matrix_tests.py old mode 100755 new mode 100644 index a68b703f1fd..d6a1489a3eb --- a/tools/interop_matrix/run_interop_matrix_tests.py +++ b/tools/interop_matrix/run_interop_matrix_tests.py @@ -203,23 +203,17 @@ def _generate_test_case_jobspecs(lang, runtime, release, suite_name): return job_spec_list -# TODO: Return a spec and then parallelize. def _pull_image_for_lang(lang, image, release): """Pull an image for a given language form the image registry.""" cmdline = [ 'time gcloud docker -- pull %s && time docker run --rm=true %s /bin/true' % (image, image) ] - spec = jobset.JobSpec(cmdline=cmdline, + return jobset.JobSpec(cmdline=cmdline, shortname='pull_image_{}'.format(image), timeout_seconds=_PULL_IMAGE_TIMEOUT_SECONDS, shell=True, - # TODO: Pull out to constant. flake_retries=2) - num_failures, resultset = jobset.run([spec], - newline_on_success=True, - maxjobs=1) - return not num_failures def _test_release(lang, runtime, release, image, xml_report_tree, skip_tests): @@ -263,17 +257,35 @@ def _run_tests_for_lang(lang, runtime, images, xml_report_tree): skip_tests = False total_num_failures = 0 - # TODO: Do more intelligent chunking. - for release, image in images: - if not skip_tests and not _pull_image_for_lang(lang, image, release): + max_pull_jobs = min(args.jobs, _MAX_PARALLEL_DOWNLOADS) + max_chunk_size = max_pull_jobs + chunk_count = (len(images) + max_chunk_size) // max_chunk_size + + for chunk_index in range(chunk_count): + chunk_start = chunk_index * max_chunk_size + chunk_size = min(max_chunk_size, len(images) - chunk_start) + chunk_end = chunk_start + chunk_size + pull_specs = [] + if not skip_tests: + for release, image in images[chunk_start:chunk_end]: + pull_specs.append(_pull_image_for_lang(lang, image, release)) + + # NOTE(rbellevi): We batch docker pull operations to maximize + # parallelism, without letting the disk usage grow unbounded. + pull_failures, _ = jobset.run(pull_specs, + newline_on_success=True, + maxjobs=max_pull_jobs) + if pull_failures: jobset.message( 'FAILED', 'Image download failed. Skipping tests for language "%s"' % lang, do_newline=True) skip_tests = True - total_num_failures += _test_release(lang, runtime, release, image, xml_report_tree, skip_tests) + for release, image in images[chunk_start:chunk_end]: + total_num_failures += _test_release(lang, runtime, release, image, xml_report_tree, skip_tests) if not args.keep: - _cleanup_docker_image(image) + for _, image in images[chunk_start:chunk_end]: + _cleanup_docker_image(image) if not total_num_failures: jobset.message('SUCCESS', 'All {} tests passed'.format(lang), do_newline=True) else: From 09007ebccff359ad9a0bd84b913c55df5ff023bc Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 23 Jun 2020 14:31:55 -0700 Subject: [PATCH 14/34] Yapf --- .../run_interop_matrix_tests.py | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tools/interop_matrix/run_interop_matrix_tests.py b/tools/interop_matrix/run_interop_matrix_tests.py index d6a1489a3eb..be92458ea62 100644 --- a/tools/interop_matrix/run_interop_matrix_tests.py +++ b/tools/interop_matrix/run_interop_matrix_tests.py @@ -210,10 +210,10 @@ def _pull_image_for_lang(lang, image, release): % (image, image) ] return jobset.JobSpec(cmdline=cmdline, - shortname='pull_image_{}'.format(image), - timeout_seconds=_PULL_IMAGE_TIMEOUT_SECONDS, - shell=True, - flake_retries=2) + shortname='pull_image_{}'.format(image), + timeout_seconds=_PULL_IMAGE_TIMEOUT_SECONDS, + shell=True, + flake_retries=2) def _test_release(lang, runtime, release, image, xml_report_tree, skip_tests): @@ -223,9 +223,7 @@ def _test_release(lang, runtime, release, image, xml_report_tree, skip_tests): suite_name) if not job_spec_list: - jobset.message('FAILED', - 'No test cases were found.', - do_newline=True) + jobset.message('FAILED', 'No test cases were found.', do_newline=True) total_num_failures += 1 else: num_failures, resultset = jobset.run(job_spec_list, @@ -237,8 +235,7 @@ def _test_release(lang, runtime, release, image, xml_report_tree, skip_tests): upload_test_results.upload_interop_results_to_bq( resultset, args.bq_result_table) if skip_tests: - jobset.message('FAILED', 'Tests were skipped', - do_newline=True) + jobset.message('FAILED', 'Tests were skipped', do_newline=True) total_num_failures += 1 if num_failures: total_num_failures += num_failures @@ -278,18 +275,24 @@ def _run_tests_for_lang(lang, runtime, images, xml_report_tree): if pull_failures: jobset.message( 'FAILED', - 'Image download failed. Skipping tests for language "%s"' % lang, + 'Image download failed. Skipping tests for language "%s"' % + lang, do_newline=True) skip_tests = True for release, image in images[chunk_start:chunk_end]: - total_num_failures += _test_release(lang, runtime, release, image, xml_report_tree, skip_tests) + total_num_failures += _test_release(lang, runtime, release, image, + xml_report_tree, skip_tests) if not args.keep: for _, image in images[chunk_start:chunk_end]: _cleanup_docker_image(image) if not total_num_failures: - jobset.message('SUCCESS', 'All {} tests passed'.format(lang), do_newline=True) + jobset.message('SUCCESS', + 'All {} tests passed'.format(lang), + do_newline=True) else: - jobset.message('FAILED', 'Some {} tests failed'.format(lang), do_newline=True) + jobset.message('FAILED', + 'Some {} tests failed'.format(lang), + do_newline=True) return total_num_failures From 0fe7738330724b9139c24b068b64dc8875442615 Mon Sep 17 00:00:00 2001 From: Sanjay Pujare Date: Tue, 23 Jun 2020 14:37:21 -0700 Subject: [PATCH 15/34] Add grpc-java v1.30.2 to interop test client matrix --- tools/interop_matrix/client_matrix.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/interop_matrix/client_matrix.py b/tools/interop_matrix/client_matrix.py index e38c1c77ee9..b85ea3ba20c 100644 --- a/tools/interop_matrix/client_matrix.py +++ b/tools/interop_matrix/client_matrix.py @@ -221,6 +221,7 @@ LANG_RELEASE_MATRIX = { ('v1.29.0', ReleaseInfo()), ('v1.30.0', ReleaseInfo()), ('v1.30.1', ReleaseInfo()), + ('v1.30.2', ReleaseInfo()), ]), 'python': OrderedDict([ From 06b1a9b0d3f83bdbaafe846a69af16602883ad69 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Tue, 23 Jun 2020 15:01:07 -0700 Subject: [PATCH 16/34] Add manylinux2014 docker images --- .../Dockerfile | 28 +++++++++++++++++++ .../Dockerfile | 28 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile create mode 100644 tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile diff --git a/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile b/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile new file mode 100644 index 00000000000..5e8fed0a0f2 --- /dev/null +++ b/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile @@ -0,0 +1,28 @@ +# Copyright 2020 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Docker file for building gRPC manylinux Python artifacts. + +FROM quay.io/pypa/manylinux2014_x86_64 + +# Update the package manager +RUN yum update -y +RUN yum install -y curl-devel expat-devel gettext-devel openssl-devel zlib-devel + +################################### +# Install Python build requirements +RUN /opt/python/cp35-cp35m/bin/pip install --upgrade cython +RUN /opt/python/cp36-cp36m/bin/pip install --upgrade cython +RUN /opt/python/cp37-cp37m/bin/pip install --upgrade cython +RUN /opt/python/cp38-cp38/bin/pip install --upgrade cython diff --git a/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile b/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile new file mode 100644 index 00000000000..6695f642f74 --- /dev/null +++ b/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile @@ -0,0 +1,28 @@ +# Copyright 2020 The gRPC Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Docker file for building gRPC manylinux Python artifacts. + +FROM quay.io/pypa/manylinux2014_i686 + +# Update the package manager +RUN yum update -y +RUN yum install -y curl-devel expat-devel gettext-devel openssl-devel zlib-devel + +################################### +# Install Python build requirements +RUN /opt/python/cp35-cp35m/bin/pip install --upgrade cython +RUN /opt/python/cp36-cp36m/bin/pip install --upgrade cython +RUN /opt/python/cp37-cp37m/bin/pip install --upgrade cython +RUN /opt/python/cp38-cp38/bin/pip install --upgrade cython From 445f6814ebcb22dbecb75e16f90d7c6018dc0f03 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Tue, 23 Jun 2020 15:09:04 -0700 Subject: [PATCH 17/34] Add manylinux2014 artifacts --- tools/run_tests/artifacts/artifact_targets.py | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tools/run_tests/artifacts/artifact_targets.py b/tools/run_tests/artifacts/artifact_targets.py index dbc37e6f57c..c7e3d81fb44 100644 --- a/tools/run_tests/artifacts/artifact_targets.py +++ b/tools/run_tests/artifacts/artifact_targets.py @@ -350,8 +350,20 @@ def targets(): CSharpExtArtifact('linux', 'android', arch_abi='armeabi-v7a'), CSharpExtArtifact('linux', 'android', arch_abi='x86'), CSharpExtArtifact('macos', 'ios'), - # TODO(https://github.com/grpc/grpc/issues/20283) - # Add manylinux2010_x86 targets once this issue is resolved. + PythonArtifact('manylinux2014', 'x64', 'cp35-cp35m'), + PythonArtifact('manylinux2014', 'x64', 'cp36-cp36m'), + PythonArtifact('manylinux2014', 'x64', 'cp37-cp37m'), + PythonArtifact('manylinux2014', 'x64', 'cp38-cp38'), + PythonArtifact('manylinux2014', 'x86', 'cp35-cp35m'), + PythonArtifact('manylinux2014', 'x86', 'cp36-cp36m'), + PythonArtifact('manylinux2014', 'x86', 'cp37-cp37m'), + PythonArtifact('manylinux2014', 'x86', 'cp38-cp38'), + PythonArtifact('manylinux2010', 'x64', 'cp27-cp27m'), + PythonArtifact('manylinux2010', 'x64', 'cp27-cp27mu'), + PythonArtifact('manylinux2010', 'x64', 'cp35-cp35m'), + PythonArtifact('manylinux2010', 'x64', 'cp36-cp36m'), + PythonArtifact('manylinux2010', 'x64', 'cp37-cp37m'), + PythonArtifact('manylinux2010', 'x64', 'cp38-cp38'), PythonArtifact('manylinux2010', 'x86', 'cp27-cp27m'), PythonArtifact('manylinux2010', 'x86', 'cp27-cp27mu'), PythonArtifact('manylinux2010', 'x86', 'cp35-cp35m'), @@ -364,12 +376,6 @@ def targets(): PythonArtifact('linux_extra', 'armv6', '2.7'), PythonArtifact('linux_extra', 'armv6', '3.5'), PythonArtifact('linux_extra', 'armv6', '3.6'), - PythonArtifact('manylinux2010', 'x64', 'cp27-cp27m'), - PythonArtifact('manylinux2010', 'x64', 'cp27-cp27mu'), - PythonArtifact('manylinux2010', 'x64', 'cp35-cp35m'), - PythonArtifact('manylinux2010', 'x64', 'cp36-cp36m'), - PythonArtifact('manylinux2010', 'x64', 'cp37-cp37m'), - PythonArtifact('manylinux2010', 'x64', 'cp38-cp38'), PythonArtifact('macos', 'x64', 'python2.7'), PythonArtifact('macos', 'x64', 'python3.5'), PythonArtifact('macos', 'x64', 'python3.6'), From a6e49e084708ff5a96e8e57a37029bda5af8412f Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 23 Jun 2020 18:01:36 -0700 Subject: [PATCH 18/34] Use old ordering logic for message size limit source --- .../message_size/message_size_filter.cc | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index 89fdab6fae8..5579fbd620c 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -117,17 +117,38 @@ void MessageSizeParser::Register() { size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } +namespace { +const grpc_arg* ChannelArgsFindLast(const grpc_channel_args* args, + const char* name) { + grpc_arg* arg = nullptr; + if (args != nullptr) { + for (size_t i = 0; i < args->num_args; ++i) { + if (strcmp(args->args[i].key, name) == 0) { + arg = &args->args[i]; + } + } + } + return arg; +} +} // namespace + int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args) { if (grpc_channel_args_want_minimal_stack(args)) return -1; - return grpc_channel_args_find_integer( - args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, + // We are not using grpc_channel_args_find_integer here because of ordering + // issues. The logic below maintains existing behavior by choosing the last + // matching channel argument instead of the first. + return grpc_channel_arg_get_integer( + ChannelArgsFindLast(args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH), {GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH, -1, INT_MAX}); } int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args) { if (grpc_channel_args_want_minimal_stack(args)) return -1; - return grpc_channel_args_find_integer( - args, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, + // We are not using grpc_channel_args_find_integer here because of ordering + // issues. The logic below maintains existing behavior by choosing the last + // matching channel argument instead of the first. + return grpc_channel_arg_get_integer( + ChannelArgsFindLast(args, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH), {GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH, -1, INT_MAX}); } From e8f17c5f9610451400cdbf68155aa586665ff740 Mon Sep 17 00:00:00 2001 From: jiangtaoli2016 Date: Tue, 23 Jun 2020 22:43:41 -0700 Subject: [PATCH 19/34] Fix SPIFFE ID check --- .../security/security_connector/ssl_utils.cc | 27 ++++++++------- test/core/security/security_connector_test.cc | 34 ++++++++++++++----- test/core/tsi/ssl_transport_security_test.cc | 1 + 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index 664f3a18e61..3aa12a8f1d8 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -257,7 +257,8 @@ grpc_core::RefCountedPtr grpc_ssl_peer_to_auth_context( transport_security_type); const char* spiffe_data = nullptr; size_t spiffe_length = 0; - int spiffe_id_count = 0; + int uri_count = 0; + bool has_spiffe_id = false; for (i = 0; i < peer->property_count; i++) { const tsi_peer_property* prop = &peer->properties[i]; if (prop->name == nullptr) continue; @@ -290,11 +291,12 @@ grpc_core::RefCountedPtr grpc_ssl_peer_to_auth_context( ctx.get(), GRPC_TRANSPORT_SECURITY_LEVEL_PROPERTY_NAME, prop->value.data, prop->value.length); } else if (strcmp(prop->name, TSI_X509_URI_PEER_PROPERTY) == 0) { + uri_count++; absl::string_view spiffe_id(prop->value.data, prop->value.length); if (IsSpiffeId(spiffe_id)) { spiffe_data = prop->value.data; spiffe_length = prop->value.length; - spiffe_id_count += 1; + has_spiffe_id = true; } } } @@ -302,16 +304,17 @@ grpc_core::RefCountedPtr grpc_ssl_peer_to_auth_context( GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( ctx.get(), peer_identity_property_name) == 1); } - // SPIFFE ID should be unique. If we find more than one SPIFFE IDs, we log - // the error without returning the error. - if (spiffe_id_count > 1) { - gpr_log(GPR_INFO, "Invalid SPIFFE ID: SPIFFE ID should be unique."); - } - if (spiffe_id_count == 1) { - GPR_ASSERT(spiffe_length > 0); - GPR_ASSERT(spiffe_data != nullptr); - grpc_auth_context_add_property(ctx.get(), GRPC_PEER_SPIFFE_ID_PROPERTY_NAME, - spiffe_data, spiffe_length); + // A valid SPIFFE certificate can only have exact one URI SAN field. + if (has_spiffe_id) { + if (uri_count == 1) { + GPR_ASSERT(spiffe_length > 0); + GPR_ASSERT(spiffe_data != nullptr); + grpc_auth_context_add_property(ctx.get(), + GRPC_PEER_SPIFFE_ID_PROPERTY_NAME, + spiffe_data, spiffe_length); + } else { + gpr_log(GPR_INFO, "Invalid SPIFFE ID: multiple URI SANs."); + } } return ctx; } diff --git a/test/core/security/security_connector_test.cc b/test/core/security/security_connector_test.cc index 8f249bec97b..c6b760be18a 100644 --- a/test/core/security/security_connector_test.cc +++ b/test/core/security/security_connector_test.cc @@ -472,16 +472,13 @@ static void test_spiffe_id_peer_to_auth_context(void) { GPR_ASSERT(check_spiffe_id(invalid_ctx.get(), nullptr, false)); tsi_peer_destruct(&invalid_peer); invalid_ctx.reset(DEBUG_LOCATION, "test"); - // A valid SPIFFE ID with other URI fields should be plumbed. + // A valid SPIFFE ID should be plumbed. tsi_peer valid_peer; - std::vector valid_spiffe_id = {"spiffe://foo.bar.com/wl", - "https://xyz"}; - GPR_ASSERT(tsi_construct_peer(valid_spiffe_id.size(), &valid_peer) == TSI_OK); - for (i = 0; i < valid_spiffe_id.size(); i++) { - GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( - TSI_X509_URI_PEER_PROPERTY, valid_spiffe_id[i].c_str(), - &valid_peer.properties[i]) == TSI_OK); - } + std::string valid_spiffe_id = "spiffe://foo.bar.com/wl"; + GPR_ASSERT(tsi_construct_peer(1, &valid_peer) == TSI_OK); + GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( + TSI_X509_URI_PEER_PROPERTY, valid_spiffe_id.c_str(), + &valid_peer.properties[0]) == TSI_OK); grpc_core::RefCountedPtr valid_ctx = grpc_ssl_peer_to_auth_context(&valid_peer, GRPC_SSL_TRANSPORT_SECURITY_TYPE); @@ -507,6 +504,25 @@ static void test_spiffe_id_peer_to_auth_context(void) { GPR_ASSERT(check_spiffe_id(multiple_ctx.get(), nullptr, false)); tsi_peer_destruct(&multiple_peer); multiple_ctx.reset(DEBUG_LOCATION, "test"); + // A valid SPIFFE certificate should only has one URI SAN field. + // SPIFFE ID should not be plumbed if there are multiple URIs. + tsi_peer multiple_uri_peer; + std::vector multiple_uri = {"spiffe://foo.bar.com/wl", + "https://xyz", "ssh://foo.bar.com/"}; + GPR_ASSERT(tsi_construct_peer(multiple_uri.size(), &multiple_uri_peer) == + TSI_OK); + for (i = 0; i < multiple_spiffe_id.size(); i++) { + GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( + TSI_X509_URI_PEER_PROPERTY, multiple_uri[i].c_str(), + &multiple_uri_peer.properties[i]) == TSI_OK); + } + grpc_core::RefCountedPtr multiple_uri_ctx = + grpc_ssl_peer_to_auth_context(&multiple_uri_peer, + GRPC_SSL_TRANSPORT_SECURITY_TYPE); + GPR_ASSERT(multiple_uri_ctx != nullptr); + GPR_ASSERT(check_spiffe_id(multiple_uri_ctx.get(), nullptr, false)); + tsi_peer_destruct(&multiple_uri_peer); + multiple_uri_ctx.reset(DEBUG_LOCATION, "test"); } static const char* roots_for_override_api = "roots for override api"; diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 8c4e736fc8b..26d4d477050 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -895,6 +895,7 @@ void ssl_tsi_test_extract_x509_subject_names() { GPR_ASSERT(check_subject_alt_name(&peer, "foo.test.domain.com") == 1); GPR_ASSERT(check_subject_alt_name(&peer, "bar.test.domain.com") == 1); // Check URI + // Note that a valid SPIFFE certificate should only have one URI. GPR_ASSERT(check_subject_alt_name(&peer, "spiffe://foo.com/bar/baz") == 1); GPR_ASSERT( check_subject_alt_name(&peer, "https://foo.test.domain.com/test") == 1); From f3fff54db9336b73424eb0934e6ecfef0f1fe65c Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 24 Jun 2020 10:00:40 -0700 Subject: [PATCH 20/34] Revert "Use old ordering logic for message size limit source" This reverts commit a6e49e084708ff5a96e8e57a37029bda5af8412f. --- .../message_size/message_size_filter.cc | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index 5579fbd620c..89fdab6fae8 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -117,38 +117,17 @@ void MessageSizeParser::Register() { size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } -namespace { -const grpc_arg* ChannelArgsFindLast(const grpc_channel_args* args, - const char* name) { - grpc_arg* arg = nullptr; - if (args != nullptr) { - for (size_t i = 0; i < args->num_args; ++i) { - if (strcmp(args->args[i].key, name) == 0) { - arg = &args->args[i]; - } - } - } - return arg; -} -} // namespace - int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args) { if (grpc_channel_args_want_minimal_stack(args)) return -1; - // We are not using grpc_channel_args_find_integer here because of ordering - // issues. The logic below maintains existing behavior by choosing the last - // matching channel argument instead of the first. - return grpc_channel_arg_get_integer( - ChannelArgsFindLast(args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH), + return grpc_channel_args_find_integer( + args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, {GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH, -1, INT_MAX}); } int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args) { if (grpc_channel_args_want_minimal_stack(args)) return -1; - // We are not using grpc_channel_args_find_integer here because of ordering - // issues. The logic below maintains existing behavior by choosing the last - // matching channel argument instead of the first. - return grpc_channel_arg_get_integer( - ChannelArgsFindLast(args, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH), + return grpc_channel_args_find_integer( + args, GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, {GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH, -1, INT_MAX}); } From 557e2ef5b1382a125a720305aff710b34a271425 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 24 Jun 2020 10:24:17 -0700 Subject: [PATCH 21/34] Prefer SetMax*MessageSize value over ServerBuilderOption --- src/cpp/server/server_builder.cc | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 0acb486149d..6461c2fcffa 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -219,24 +219,13 @@ ServerBuilder& ServerBuilder::AddListeningPort( std::unique_ptr ServerBuilder::BuildAndStart() { grpc::ChannelArguments args; - + if (max_receive_message_size_ >= -1) { + args.SetInt(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, max_receive_message_size_); + } for (const auto& option : options_) { option->UpdateArguments(&args); option->UpdatePlugins(&plugins_); } - if (max_receive_message_size_ >= -1) { - grpc_channel_args c_args = args.c_channel_args(); - const grpc_arg* arg = - grpc_channel_args_find(&c_args, GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH); - // Some option has set max_receive_message_length and it is also set - // directly on the ServerBuilder. - if (arg != nullptr) { - gpr_log( - GPR_ERROR, - "gRPC ServerBuilder receives multiple max_receive_message_length"); - } - args.SetInt(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, max_receive_message_size_); - } // The default message size is -1 (max), so no need to explicitly set it for // -1. if (max_send_message_size_ >= 0) { From 1a17a14ceb731250204c26b5bd291759679fcb36 Mon Sep 17 00:00:00 2001 From: Sanjay Pujare Date: Wed, 24 Jun 2020 10:38:32 -0700 Subject: [PATCH 22/34] Remove 1.30.0 and 1.30.1 from the matrix --- tools/interop_matrix/client_matrix.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/interop_matrix/client_matrix.py b/tools/interop_matrix/client_matrix.py index b85ea3ba20c..cfd5bb7fd09 100644 --- a/tools/interop_matrix/client_matrix.py +++ b/tools/interop_matrix/client_matrix.py @@ -219,8 +219,6 @@ LANG_RELEASE_MATRIX = { ('v1.27.2', ReleaseInfo()), ('v1.28.1', ReleaseInfo()), ('v1.29.0', ReleaseInfo()), - ('v1.30.0', ReleaseInfo()), - ('v1.30.1', ReleaseInfo()), ('v1.30.2', ReleaseInfo()), ]), 'python': From fc059ded05247987b22160b35f8f2f24e9157870 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 24 Jun 2020 11:13:17 -0700 Subject: [PATCH 23/34] Make the same change for max send message size --- src/cpp/server/server_builder.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 6461c2fcffa..a2403fe996d 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -222,16 +222,13 @@ std::unique_ptr ServerBuilder::BuildAndStart() { if (max_receive_message_size_ >= -1) { args.SetInt(GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH, max_receive_message_size_); } + if (max_send_message_size_ >= -1) { + args.SetInt(GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, max_send_message_size_); + } for (const auto& option : options_) { option->UpdateArguments(&args); option->UpdatePlugins(&plugins_); } - // The default message size is -1 (max), so no need to explicitly set it for - // -1. - if (max_send_message_size_ >= 0) { - args.SetInt(GRPC_ARG_MAX_SEND_MESSAGE_LENGTH, max_send_message_size_); - } - args.SetInt(GRPC_COMPRESSION_CHANNEL_ENABLED_ALGORITHMS_BITSET, enabled_compression_algorithms_bitset_); if (maybe_default_compression_level_.is_set) { From f69b8f831c7ad3fd01090e1c6ed8d2b81b5a5136 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 24 Jun 2020 11:49:54 -0700 Subject: [PATCH 24/34] Fix executable bit --- tools/interop_matrix/run_interop_matrix_tests.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tools/interop_matrix/run_interop_matrix_tests.py diff --git a/tools/interop_matrix/run_interop_matrix_tests.py b/tools/interop_matrix/run_interop_matrix_tests.py old mode 100644 new mode 100755 From 53d734a0dc867f9e11577beda6c2101ae681a205 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Wed, 24 Jun 2020 20:03:58 +0100 Subject: [PATCH 25/34] Fixed typo --- src/php/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/php/README.md b/src/php/README.md index 0862ceac446..1704c814ad8 100644 --- a/src/php/README.md +++ b/src/php/README.md @@ -96,7 +96,7 @@ composer package as well. Add this to your project's `composer.json` file. ```json "require": { - "grpc/grpc": "~v1.30.0" + "grpc/grpc": "~1.30.0" } ``` From 837c4ce589924640bbc395128ba07cbeffdd5b5c Mon Sep 17 00:00:00 2001 From: jiangtaoli2016 Date: Wed, 24 Jun 2020 10:54:58 -0700 Subject: [PATCH 26/34] Fix ALTS shutdown crash on envoy --- .../alts/handshaker/alts_handshaker_client.cc | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/core/tsi/alts/handshaker/alts_handshaker_client.cc b/src/core/tsi/alts/handshaker/alts_handshaker_client.cc index 61927276195..067f46c46d6 100644 --- a/src/core/tsi/alts/handshaker/alts_handshaker_client.cc +++ b/src/core/tsi/alts/handshaker/alts_handshaker_client.cc @@ -661,11 +661,18 @@ static void handshaker_client_destruct(alts_handshaker_client* c) { // TODO(apolcyn): we could remove this indirection and call // grpc_call_unref inline if there was an internal variant of // grpc_call_unref that didn't need to flush an ExecCtx. - grpc_core::ExecCtx::Run( - DEBUG_LOCATION, - GRPC_CLOSURE_CREATE(handshaker_call_unref, client->call, - grpc_schedule_on_exec_ctx), - GRPC_ERROR_NONE); + if (grpc_core::ExecCtx::Get() == nullptr) { + // Unref handshaker call if there is no exec_ctx, e.g., in the case of + // Envoy ALTS transport socket. + grpc_call_unref(client->call); + } else { + // Using existing exec_ctx to unref handshaker call. + grpc_core::ExecCtx::Run( + DEBUG_LOCATION, + GRPC_CLOSURE_CREATE(handshaker_call_unref, client->call, + grpc_schedule_on_exec_ctx), + GRPC_ERROR_NONE); + } } } From 7af03152ab6d3578524889018d888725e1542302 Mon Sep 17 00:00:00 2001 From: yihuaz Date: Wed, 24 Jun 2020 18:29:03 -0700 Subject: [PATCH 27/34] handle the error in alts_tsi_handshaker_result_create --- src/core/tsi/alts/handshaker/alts_handshaker_client.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/tsi/alts/handshaker/alts_handshaker_client.cc b/src/core/tsi/alts/handshaker/alts_handshaker_client.cc index 28f65dd32fc..e2f04f7310d 100644 --- a/src/core/tsi/alts/handshaker/alts_handshaker_client.cc +++ b/src/core/tsi/alts/handshaker/alts_handshaker_client.cc @@ -263,7 +263,13 @@ void alts_handshaker_client_handle_response(alts_handshaker_client* c, } tsi_handshaker_result* result = nullptr; if (is_handshake_finished_properly(resp)) { - alts_tsi_handshaker_result_create(resp, client->is_client, &result); + tsi_result status = + alts_tsi_handshaker_result_create(resp, client->is_client, &result); + if (status != TSI_OK) { + gpr_log(GPR_ERROR, "alts_tsi_handshaker_result_create() failed"); + handle_response_done(client, status, nullptr, 0, nullptr); + return; + } alts_tsi_handshaker_result_set_unused_bytes( result, &client->recv_bytes, grpc_gcp_HandshakerResp_bytes_consumed(resp)); From 34b210b44acac90dac88f4dc381d20f9aca8c07f Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Thu, 25 Jun 2020 16:29:10 -0700 Subject: [PATCH 28/34] Use the wrapper version of bazel in the script --- src/abseil-cpp/preprocessed_builds.yaml.gen.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/abseil-cpp/preprocessed_builds.yaml.gen.py b/src/abseil-cpp/preprocessed_builds.yaml.gen.py index 82bb15ef395..6e765e6f610 100755 --- a/src/abseil-cpp/preprocessed_builds.yaml.gen.py +++ b/src/abseil-cpp/preprocessed_builds.yaml.gen.py @@ -69,8 +69,11 @@ def parse_bazel_rule(elem, package): def read_bazel_build(package): """Runs bazel query on given package file and returns all cc rules.""" + # Use a wrapper version of bazel in gRPC not to use system-wide bazel + # to avoid bazel conflict when running on Kokoro. + BAZEL_BIN = "../../tools/bazel" result = subprocess.check_output( - ["bazel", "query", package + ":all", "--output", "xml"]) + [BAZEL_BIN, "query", package + ":all", "--output", "xml"]) root = ET.fromstring(result) return [ parse_bazel_rule(elem, package) From 93d947b364184c2c363516668c9f972651a10cd2 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Thu, 25 Jun 2020 19:52:47 -0700 Subject: [PATCH 29/34] Removed sort_keys=True in yaml.gen.py --- src/abseil-cpp/preprocessed_builds.yaml.gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/abseil-cpp/preprocessed_builds.yaml.gen.py b/src/abseil-cpp/preprocessed_builds.yaml.gen.py index 6e765e6f610..599cc98de7f 100755 --- a/src/abseil-cpp/preprocessed_builds.yaml.gen.py +++ b/src/abseil-cpp/preprocessed_builds.yaml.gen.py @@ -206,7 +206,7 @@ def main(): builds = generate_builds("absl") os.chdir(previous_dir) with open(OUTPUT_PATH, 'w') as outfile: - outfile.write(yaml.dump(builds, indent=2, sort_keys=True)) + outfile.write(yaml.dump(builds, indent=2)) if __name__ == "__main__": From c33c67edf63197436da888b95672f001e2e78425 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Thu, 25 Jun 2020 20:35:00 -0700 Subject: [PATCH 30/34] Add Python 3.9 support --- .../grpc_artifact_python_manylinux2010_x64/Dockerfile | 2 ++ .../grpc_artifact_python_manylinux2010_x86/Dockerfile | 2 ++ .../grpc_artifact_python_manylinux2014_x64/Dockerfile | 2 ++ .../grpc_artifact_python_manylinux2014_x86/Dockerfile | 2 ++ 4 files changed, 8 insertions(+) diff --git a/tools/dockerfile/grpc_artifact_python_manylinux2010_x64/Dockerfile b/tools/dockerfile/grpc_artifact_python_manylinux2010_x64/Dockerfile index 390ab453136..287ed1c2725 100644 --- a/tools/dockerfile/grpc_artifact_python_manylinux2010_x64/Dockerfile +++ b/tools/dockerfile/grpc_artifact_python_manylinux2010_x64/Dockerfile @@ -13,6 +13,7 @@ # limitations under the License. # Docker file for building gRPC manylinux Python artifacts. +# Updated: 2020-06-25 FROM quay.io/pypa/manylinux2010_x86_64 @@ -28,3 +29,4 @@ RUN /opt/python/cp35-cp35m/bin/pip install --upgrade cython RUN /opt/python/cp36-cp36m/bin/pip install --upgrade cython RUN /opt/python/cp37-cp37m/bin/pip install --upgrade cython RUN /opt/python/cp38-cp38/bin/pip install --upgrade cython +RUN /opt/python/cp39-cp39/bin/pip install --upgrade cython diff --git a/tools/dockerfile/grpc_artifact_python_manylinux2010_x86/Dockerfile b/tools/dockerfile/grpc_artifact_python_manylinux2010_x86/Dockerfile index 6e5e43315e7..240165a0e42 100644 --- a/tools/dockerfile/grpc_artifact_python_manylinux2010_x86/Dockerfile +++ b/tools/dockerfile/grpc_artifact_python_manylinux2010_x86/Dockerfile @@ -13,6 +13,7 @@ # limitations under the License. # Docker file for building gRPC manylinux Python artifacts. +# Updated: 2020-06-25 FROM quay.io/pypa/manylinux2010_i686 @@ -28,3 +29,4 @@ RUN /opt/python/cp35-cp35m/bin/pip install --upgrade cython RUN /opt/python/cp36-cp36m/bin/pip install --upgrade cython RUN /opt/python/cp37-cp37m/bin/pip install --upgrade cython RUN /opt/python/cp38-cp38/bin/pip install --upgrade cython +RUN /opt/python/cp39-cp39/bin/pip install --upgrade cython diff --git a/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile b/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile index 5e8fed0a0f2..ef90555174d 100644 --- a/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile +++ b/tools/dockerfile/grpc_artifact_python_manylinux2014_x64/Dockerfile @@ -13,6 +13,7 @@ # limitations under the License. # Docker file for building gRPC manylinux Python artifacts. +# Updated: 2020-06-25 FROM quay.io/pypa/manylinux2014_x86_64 @@ -26,3 +27,4 @@ RUN /opt/python/cp35-cp35m/bin/pip install --upgrade cython RUN /opt/python/cp36-cp36m/bin/pip install --upgrade cython RUN /opt/python/cp37-cp37m/bin/pip install --upgrade cython RUN /opt/python/cp38-cp38/bin/pip install --upgrade cython +RUN /opt/python/cp39-cp39/bin/pip install --upgrade cython diff --git a/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile b/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile index 6695f642f74..ff23414ae4e 100644 --- a/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile +++ b/tools/dockerfile/grpc_artifact_python_manylinux2014_x86/Dockerfile @@ -13,6 +13,7 @@ # limitations under the License. # Docker file for building gRPC manylinux Python artifacts. +# Updated: 2020-06-25 FROM quay.io/pypa/manylinux2014_i686 @@ -26,3 +27,4 @@ RUN /opt/python/cp35-cp35m/bin/pip install --upgrade cython RUN /opt/python/cp36-cp36m/bin/pip install --upgrade cython RUN /opt/python/cp37-cp37m/bin/pip install --upgrade cython RUN /opt/python/cp38-cp38/bin/pip install --upgrade cython +RUN /opt/python/cp39-cp39/bin/pip install --upgrade cython From 1df2cb376049c52ceda8f22dc1c55a35139b7706 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Fri, 26 Jun 2020 11:26:55 -0700 Subject: [PATCH 31/34] Dump details around ruby call creds user callback invocation when debug logs are enabled --- src/ruby/ext/grpc/rb_call_credentials.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/ruby/ext/grpc/rb_call_credentials.c b/src/ruby/ext/grpc/rb_call_credentials.c index 61170cc73b2..a38145e129d 100644 --- a/src/ruby/ext/grpc/rb_call_credentials.c +++ b/src/ruby/ext/grpc/rb_call_credentials.c @@ -56,6 +56,28 @@ typedef struct callback_params { static VALUE grpc_rb_call_credentials_callback(VALUE callback_args) { VALUE result = rb_hash_new(); + if (gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { + VALUE callback_args_as_str = + rb_funcall(callback_args, rb_intern("to_s"), 0); + VALUE callback_source_info = rb_funcall(rb_ary_entry(callback_args, 0), + rb_intern("source_location"), 0); + if (callback_source_info != Qnil) { + VALUE source_filename = rb_ary_entry(callback_source_info, 0); + VALUE source_line_number = rb_funcall( + rb_ary_entry(callback_source_info, 1), rb_intern("to_s"), 0); + gpr_log(GPR_DEBUG, + "GRPC_RUBY: grpc_rb_call_credentials invoking user callback " + "(source_filename:%s line_number:%s) with arguments:%s", + StringValueCStr(source_filename), + StringValueCStr(source_line_number), + StringValueCStr(callback_args_as_str)); + } else { + gpr_log(GPR_DEBUG, + "GRPC_RUBY: grpc_rb_call_credentials invoking user callback " + "(failed to get source filename ane line) with arguments:%s", + StringValueCStr(callback_args_as_str)); + } + } VALUE metadata = rb_funcall(rb_ary_entry(callback_args, 0), rb_intern("call"), 1, rb_ary_entry(callback_args, 1)); rb_hash_aset(result, rb_str_new2("metadata"), metadata); From b88c0a22c743a6d09a35d1754f1e125f03a76792 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 25 Jul 2019 11:46:32 -0700 Subject: [PATCH 32/34] Fix use-after-free in ruby call creds --- grpc.def | 2 + include/grpc/grpc_security.h | 8 + .../lib/security/transport/auth_filters.h | 5 - .../call_credentials_timeout_driver.rb | 153 ++++++++++++++++++ src/ruby/end2end/end2end_common.rb | 5 +- src/ruby/ext/grpc/rb_call_credentials.c | 5 +- src/ruby/ext/grpc/rb_grpc_imports.generated.c | 4 + src/ruby/ext/grpc/rb_grpc_imports.generated.h | 6 + src/ruby/spec/support/services.rb | 14 +- .../core/surface/public_headers_must_be_c89.c | 2 + .../helper_scripts/run_ruby_end2end_tests.sh | 1 + 11 files changed, 193 insertions(+), 12 deletions(-) create mode 100755 src/ruby/end2end/call_credentials_timeout_driver.rb diff --git a/grpc.def b/grpc.def index 2f753b29939..ad0dca2f0d4 100644 --- a/grpc.def +++ b/grpc.def @@ -112,6 +112,8 @@ EXPORTS grpc_access_token_credentials_create grpc_google_iam_credentials_create grpc_sts_credentials_create + grpc_auth_metadata_context_copy + grpc_auth_metadata_context_reset grpc_metadata_credentials_create_from_plugin grpc_secure_channel_create grpc_server_credentials_release diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index e910b86596b..99635ea8807 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -390,6 +390,14 @@ typedef struct { void* reserved; } grpc_auth_metadata_context; +/** Performs a deep copy from \a from to \a to. **/ +GRPCAPI void grpc_auth_metadata_context_copy(grpc_auth_metadata_context* from, + grpc_auth_metadata_context* to); + +/** Releases internal resources held by \a context. **/ +GRPCAPI void grpc_auth_metadata_context_reset( + grpc_auth_metadata_context* context); + /** Maximum number of metadata entries returnable by a credentials plugin via a synchronous return. */ #define GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX 4 diff --git a/src/core/lib/security/transport/auth_filters.h b/src/core/lib/security/transport/auth_filters.h index 5e73c21cc44..ae31b1d578b 100644 --- a/src/core/lib/security/transport/auth_filters.h +++ b/src/core/lib/security/transport/auth_filters.h @@ -32,9 +32,4 @@ void grpc_auth_metadata_context_build( const grpc_slice& call_method, grpc_auth_context* auth_context, grpc_auth_metadata_context* auth_md_context); -void grpc_auth_metadata_context_copy(grpc_auth_metadata_context* from, - grpc_auth_metadata_context* to); - -void grpc_auth_metadata_context_reset(grpc_auth_metadata_context* context); - #endif /* GRPC_CORE_LIB_SECURITY_TRANSPORT_AUTH_FILTERS_H */ diff --git a/src/ruby/end2end/call_credentials_timeout_driver.rb b/src/ruby/end2end/call_credentials_timeout_driver.rb new file mode 100755 index 00000000000..7618b14b974 --- /dev/null +++ b/src/ruby/end2end/call_credentials_timeout_driver.rb @@ -0,0 +1,153 @@ +#!/usr/bin/env ruby +# +# Copyright 2016 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +this_dir = File.expand_path(File.dirname(__FILE__)) +protos_lib_dir = File.join(this_dir, 'lib') +grpc_lib_dir = File.join(File.dirname(this_dir), 'lib') +$LOAD_PATH.unshift(grpc_lib_dir) unless $LOAD_PATH.include?(grpc_lib_dir) +$LOAD_PATH.unshift(protos_lib_dir) unless $LOAD_PATH.include?(protos_lib_dir) +$LOAD_PATH.unshift(this_dir) unless $LOAD_PATH.include?(this_dir) + +require 'grpc' +require 'end2end_common' + +def create_channel_creds + test_root = File.join(File.dirname(__FILE__), '..', 'spec', 'testdata') + files = ['ca.pem', 'client.key', 'client.pem'] + creds = files.map { |f| File.open(File.join(test_root, f)).read } + GRPC::Core::ChannelCredentials.new(creds[0], creds[1], creds[2]) +end + +def client_cert + test_root = File.join(File.dirname(__FILE__), '..', 'spec', 'testdata') + cert = File.open(File.join(test_root, 'client.pem')).read + fail unless cert.is_a?(String) + cert +end + +def create_server_creds + test_root = File.join(File.dirname(__FILE__), '..', 'spec', 'testdata') + GRPC.logger.info("test root: #{test_root}") + files = ['ca.pem', 'server1.key', 'server1.pem'] + creds = files.map { |f| File.open(File.join(test_root, f)).read } + GRPC::Core::ServerCredentials.new( + creds[0], + [{ private_key: creds[1], cert_chain: creds[2] }], + true) # force client auth +end + +# Useful to update a value within a do block +class MutableValue + attr_accessor :value + + def initialize(value) + @value = value + end +end + +# rubocop:disable Metrics/AbcSize +# rubocop:disable Metrics/MethodLength +def main + server_runner = ServerRunner.new(EchoServerImpl) + server_runner.server_creds = create_server_creds + server_port = server_runner.run + channel_args = { + GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.fr' + } + token_fetch_attempts = MutableValue.new(0) + token_fetch_attempts_mu = Mutex.new + jwt_aud_uri_extraction_success_count = MutableValue.new(0) + jwt_aud_uri_extraction_success_count_mu = Mutex.new + expected_jwt_aud_uri = 'https://foo.test.google.fr/echo.EchoServer' + jwt_aud_uri_failure_values = [] + times_out_first_time_auth_proc = proc do |args| + # We check the value of jwt_aud_uri not necessarily as a test for + # the correctness of jwt_aud_uri w.r.t. its expected semantics, but + # more for as an indirect way to check for memory corruption. + jwt_aud_uri_extraction_success_count_mu.synchronize do + if args[:jwt_aud_uri] == expected_jwt_aud_uri + jwt_aud_uri_extraction_success_count.value += 1 + else + jwt_aud_uri_failure_values << args[:jwt_aud_uri] + end + end + token_fetch_attempts_mu.synchronize do + old_val = token_fetch_attempts.value + token_fetch_attempts.value += 1 + if old_val.zero? + STDERR.puts 'call creds plugin sleeping for 4 seconds' + sleep 4 + STDERR.puts 'call creds plugin done with 4 second sleep' + raise 'test exception thrown purposely from call creds plugin' + end + end + { 'authorization' => 'fake_val' }.merge(args) + end + channel_creds = create_channel_creds.compose( + GRPC::Core::CallCredentials.new(times_out_first_time_auth_proc)) + stub = Echo::EchoServer::Stub.new("localhost:#{server_port}", + channel_creds, + channel_args: channel_args) + STDERR.puts 'perform a first few RPCs to try to get things into a bad state...' + threads = [] + got_at_least_one_failure = MutableValue.new(false) + 2000.times do + threads << Thread.new do + begin + # 2 seconds is chosen as deadline here because it is less than the 4 second + # sleep that the first call creds user callback does. The idea here is that + # a lot of RPCs will be made concurrently all with 2 second deadlines, and they + # will all queue up onto the call creds user callback thread, and will all + # have to wait for the first 4 second sleep to finish. When the deadlines + # of the associated calls fire ~2 seconds in, some of their C-core data + # will have ownership dropped, and they will hit the user-after-free in + # https://github.com/grpc/grpc/issues/19195 if this isn't handled correctly. + stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 2) + rescue GRPC::BadStatus + got_at_least_one_failure.value = true + # We don't care if these RPCs succeed or fail. The purpose of these + # RPCs is just to try to induce a specific use-after-free bug, and to get + # the call credentials callback thread into a bad state. + end + end + end + threads.each(&:join) + unless got_at_least_one_failure.value + fail 'expected at least one of the initial RPCs to fail' + end + # Expect three more RPCs to succeed + STDERR.puts 'now perform another RPC and expect OK...' + stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + STDERR.puts 'now perform another RPC and expect OK...' + stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + STDERR.puts 'now perform another RPC and expect OK...' + stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + jwt_aud_uri_extraction_success_count_mu.synchronize do + if jwt_aud_uri_extraction_success_count.value != 2003 + fail "Expected to get jwt_aud_uri:#{expected_jwt_aud_uri} passed to call creds +user callback 2003 times, but it was only passed to the call creds user callback +#{jwt_aud_uri_extraction_success_count.value} times. This suggests that either: +a) the expected jwt_aud_uri value is incorrect +b) there is some corruption of the jwt_aud_uri argument +Here are are the values of the jwt_aud_uri parameter that were passed to the call +creds user callback that did not match #{expected_jwt_aud_uri}: +#{jwt_aud_uri_failure_values}" + end + end + server_runner.stop +end + +main diff --git a/src/ruby/end2end/end2end_common.rb b/src/ruby/end2end/end2end_common.rb index ffbaa1986d0..1bc17066735 100755 --- a/src/ruby/end2end/end2end_common.rb +++ b/src/ruby/end2end/end2end_common.rb @@ -43,14 +43,17 @@ end # ServerRunner starts an "echo server" that test clients can make calls to class ServerRunner + attr_accessor :server_creds + def initialize(service_impl, rpc_server_args: {}) @service_impl = service_impl @rpc_server_args = rpc_server_args + @server_creds = :this_port_is_insecure end def run @srv = new_rpc_server_for_testing(@rpc_server_args) - port = @srv.add_http2_port('0.0.0.0:0', :this_port_is_insecure) + port = @srv.add_http2_port('0.0.0.0:0', @server_creds) @srv.handle(@service_impl) @thd = Thread.new do diff --git a/src/ruby/ext/grpc/rb_call_credentials.c b/src/ruby/ext/grpc/rb_call_credentials.c index 61170cc73b2..a63d5dd5aa3 100644 --- a/src/ruby/ext/grpc/rb_call_credentials.c +++ b/src/ruby/ext/grpc/rb_call_credentials.c @@ -109,6 +109,7 @@ static void grpc_rb_call_credentials_callback_with_gil(void* param) { params->callback(params->user_data, md_ary.metadata, md_ary.count, status, error_details); grpc_rb_metadata_array_destroy_including_entries(&md_ary); + grpc_auth_metadata_context_reset(¶ms->context); gpr_free(params); } @@ -118,9 +119,9 @@ static int grpc_rb_call_credentials_plugin_get_metadata( grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], size_t* num_creds_md, grpc_status_code* status, const char** error_details) { - callback_params* params = gpr_malloc(sizeof(callback_params)); + callback_params* params = gpr_zalloc(sizeof(callback_params)); params->get_metadata = (VALUE)state; - params->context = context; + grpc_auth_metadata_context_copy(&context, ¶ms->context); params->user_data = user_data; params->callback = cb; diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 4e50a5c3ea7..37b864d5be4 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -135,6 +135,8 @@ grpc_google_refresh_token_credentials_create_type grpc_google_refresh_token_cred grpc_access_token_credentials_create_type grpc_access_token_credentials_create_import; grpc_google_iam_credentials_create_type grpc_google_iam_credentials_create_import; grpc_sts_credentials_create_type grpc_sts_credentials_create_import; +grpc_auth_metadata_context_copy_type grpc_auth_metadata_context_copy_import; +grpc_auth_metadata_context_reset_type grpc_auth_metadata_context_reset_import; grpc_metadata_credentials_create_from_plugin_type grpc_metadata_credentials_create_from_plugin_import; grpc_secure_channel_create_type grpc_secure_channel_create_import; grpc_server_credentials_release_type grpc_server_credentials_release_import; @@ -407,6 +409,8 @@ void grpc_rb_load_imports(HMODULE library) { grpc_access_token_credentials_create_import = (grpc_access_token_credentials_create_type) GetProcAddress(library, "grpc_access_token_credentials_create"); grpc_google_iam_credentials_create_import = (grpc_google_iam_credentials_create_type) GetProcAddress(library, "grpc_google_iam_credentials_create"); grpc_sts_credentials_create_import = (grpc_sts_credentials_create_type) GetProcAddress(library, "grpc_sts_credentials_create"); + grpc_auth_metadata_context_copy_import = (grpc_auth_metadata_context_copy_type) GetProcAddress(library, "grpc_auth_metadata_context_copy"); + grpc_auth_metadata_context_reset_import = (grpc_auth_metadata_context_reset_type) GetProcAddress(library, "grpc_auth_metadata_context_reset"); grpc_metadata_credentials_create_from_plugin_import = (grpc_metadata_credentials_create_from_plugin_type) GetProcAddress(library, "grpc_metadata_credentials_create_from_plugin"); grpc_secure_channel_create_import = (grpc_secure_channel_create_type) GetProcAddress(library, "grpc_secure_channel_create"); grpc_server_credentials_release_import = (grpc_server_credentials_release_type) GetProcAddress(library, "grpc_server_credentials_release"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index cf351a50a19..260d673a668 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -380,6 +380,12 @@ extern grpc_google_iam_credentials_create_type grpc_google_iam_credentials_creat typedef grpc_call_credentials*(*grpc_sts_credentials_create_type)(const grpc_sts_credentials_options* options, void* reserved); extern grpc_sts_credentials_create_type grpc_sts_credentials_create_import; #define grpc_sts_credentials_create grpc_sts_credentials_create_import +typedef void(*grpc_auth_metadata_context_copy_type)(grpc_auth_metadata_context* from, grpc_auth_metadata_context* to); +extern grpc_auth_metadata_context_copy_type grpc_auth_metadata_context_copy_import; +#define grpc_auth_metadata_context_copy grpc_auth_metadata_context_copy_import +typedef void(*grpc_auth_metadata_context_reset_type)(grpc_auth_metadata_context* context); +extern grpc_auth_metadata_context_reset_type grpc_auth_metadata_context_reset_import; +#define grpc_auth_metadata_context_reset grpc_auth_metadata_context_reset_import typedef grpc_call_credentials*(*grpc_metadata_credentials_create_from_plugin_type)(grpc_metadata_credentials_plugin plugin, grpc_security_level min_security_level, void* reserved); extern grpc_metadata_credentials_create_from_plugin_type grpc_metadata_credentials_create_from_plugin_import; #define grpc_metadata_credentials_create_from_plugin grpc_metadata_credentials_create_from_plugin_import diff --git a/src/ruby/spec/support/services.rb b/src/ruby/spec/support/services.rb index 438459dfd79..a5d8e7c187b 100644 --- a/src/ruby/spec/support/services.rb +++ b/src/ruby/spec/support/services.rb @@ -17,12 +17,18 @@ require 'spec_helper' # A test message class EchoMsg - def self.marshal(_o) - '' + attr_reader :msg + + def initialize(msg: '') + @msg = msg end - def self.unmarshal(_o) - EchoMsg.new + def self.marshal(o) + o.msg + end + + def self.unmarshal(msg) + EchoMsg.new(msg: msg) end end diff --git a/test/core/surface/public_headers_must_be_c89.c b/test/core/surface/public_headers_must_be_c89.c index b6ba715e247..1b119317969 100644 --- a/test/core/surface/public_headers_must_be_c89.c +++ b/test/core/surface/public_headers_must_be_c89.c @@ -179,6 +179,8 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_access_token_credentials_create); printf("%lx", (unsigned long) grpc_google_iam_credentials_create); printf("%lx", (unsigned long) grpc_sts_credentials_create); + printf("%lx", (unsigned long) grpc_auth_metadata_context_copy); + printf("%lx", (unsigned long) grpc_auth_metadata_context_reset); printf("%lx", (unsigned long) grpc_metadata_credentials_create_from_plugin); printf("%lx", (unsigned long) grpc_secure_channel_create); printf("%lx", (unsigned long) grpc_server_credentials_release); diff --git a/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh b/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh index fc0759fc836..304814c2553 100755 --- a/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh +++ b/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh @@ -35,4 +35,5 @@ time ruby src/ruby/end2end/graceful_sig_stop_driver.rb || EXIT_CODE=1 time ruby src/ruby/end2end/errors_load_before_grpc_lib.rb || EXIT_CODE=1 time ruby src/ruby/end2end/logger_load_before_grpc_lib.rb || EXIT_CODE=1 time ruby src/ruby/end2end/status_codes_load_before_grpc_lib.rb || EXIT_CODE=1 +time ruby src/ruby/end2end/call_credentials_timeout_driver.rb || EXIT_CODE=1 exit $EXIT_CODE From e2889fe265ecd89b1f911759b3ff17f5fbabb246 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Sat, 27 Jun 2020 01:00:40 -0700 Subject: [PATCH 33/34] Added missing deps_linkage --- tools/buildgen/extract_metadata_from_bazel_xml.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/buildgen/extract_metadata_from_bazel_xml.py b/tools/buildgen/extract_metadata_from_bazel_xml.py index 720ce54f6c1..9d0dc2e7c26 100755 --- a/tools/buildgen/extract_metadata_from_bazel_xml.py +++ b/tools/buildgen/extract_metadata_from_bazel_xml.py @@ -589,6 +589,7 @@ _BUILD_EXTRA_METADATA = { 'build': 'all', 'baselib': True, 'secure': True, + 'deps_linkage': 'static', 'dll': True, 'generate_plugin_registry': True }, @@ -622,6 +623,7 @@ _BUILD_EXTRA_METADATA = { 'grpc_csharp_ext': { 'language': 'c', 'build': 'all', + 'deps_linkage': 'static', 'dll': 'only' }, 'grpc_unsecure': { @@ -629,6 +631,7 @@ _BUILD_EXTRA_METADATA = { 'build': 'all', 'baselib': True, 'secure': False, + 'deps_linkage': 'static', 'dll': True, 'generate_plugin_registry': True }, From 70e4e4f3e77aadd6d45d56a775e6eb827a6760f7 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Sat, 27 Jun 2020 01:02:19 -0700 Subject: [PATCH 34/34] Regenerated projects --- Makefile | 30 +++++++++++++++--------------- build_autogenerated.yaml | 3 +++ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index ebae573af33..328235869c4 100644 --- a/Makefile +++ b/Makefile @@ -4150,18 +4150,18 @@ endif ifeq ($(SYSTEM),MINGW32) -$(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/gpr$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/address_sorting$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/upb$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(OPENSSL_DEP) +$(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_DEP) $(E) "[LD] Linking $@" $(Q) mkdir -p `dirname $@` - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,--output-def=$(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).def -Wl,--out-implib=$(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE)-dll.a -o $(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_OBJS) $(OPENSSL_MERGE_LIBS) $(LDLIBS_SECURE) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgpr$(SHARED_VERSION_CORE)-dll -laddress_sorting$(SHARED_VERSION_CORE)-dll -lupb$(SHARED_VERSION_CORE)-dll + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,--output-def=$(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).def -Wl,--out-implib=$(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE)-dll.a -o $(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_MERGE_LIBS) $(LDLIBS_SECURE) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) else -$(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgpr.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libaddress_sorting.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libupb.$(SHARED_EXT_CORE) $(OPENSSL_DEP) +$(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_DEP) $(E) "[LD] Linking $@" $(Q) mkdir -p `dirname $@` ifeq ($(SYSTEM),Darwin) - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -install_name $(SHARED_PREFIX)grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) -dynamiclib -o $(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_OBJS) $(OPENSSL_MERGE_LIBS) $(LDLIBS_SECURE) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgpr -laddress_sorting -lupb + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -install_name $(SHARED_PREFIX)grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) -dynamiclib -o $(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_MERGE_LIBS) $(LDLIBS_SECURE) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) else - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,-soname,libgrpc.so.11 -o $(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_OBJS) $(OPENSSL_MERGE_LIBS) $(LDLIBS_SECURE) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgpr -laddress_sorting -lupb + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,-soname,libgrpc.so.11 -o $(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_MERGE_LIBS) $(LDLIBS_SECURE) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) $(Q) ln -sf $(SHARED_PREFIX)grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).so.11 $(Q) ln -sf $(SHARED_PREFIX)grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgrpc$(SHARED_VERSION_CORE).so endif @@ -4207,18 +4207,18 @@ endif ifeq ($(SYSTEM),MINGW32) -$(LIBDIR)/$(CONFIG)/grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/grpc$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/gpr$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/address_sorting$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/upb$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(OPENSSL_DEP) +$(LIBDIR)/$(CONFIG)/grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_DEP) $(E) "[LD] Linking $@" $(Q) mkdir -p `dirname $@` - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,--output-def=$(LIBDIR)/$(CONFIG)/grpc_csharp_ext$(SHARED_VERSION_CORE).def -Wl,--out-implib=$(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE)-dll.a -o $(LIBDIR)/$(CONFIG)/grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgrpc$(SHARED_VERSION_CORE)-dll -lgpr$(SHARED_VERSION_CORE)-dll -laddress_sorting$(SHARED_VERSION_CORE)-dll -lupb$(SHARED_VERSION_CORE)-dll + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,--output-def=$(LIBDIR)/$(CONFIG)/grpc_csharp_ext$(SHARED_VERSION_CORE).def -Wl,--out-implib=$(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE)-dll.a -o $(LIBDIR)/$(CONFIG)/grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_CSHARP_EXT_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) else -$(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgrpc.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgpr.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libaddress_sorting.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libupb.$(SHARED_EXT_CORE) $(OPENSSL_DEP) +$(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(OPENSSL_DEP) $(E) "[LD] Linking $@" $(Q) mkdir -p `dirname $@` ifeq ($(SYSTEM),Darwin) - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -install_name $(SHARED_PREFIX)grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) -dynamiclib -o $(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgrpc -lgpr -laddress_sorting -lupb + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -install_name $(SHARED_PREFIX)grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) -dynamiclib -o $(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_CSHARP_EXT_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) else - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,-soname,libgrpc_csharp_ext.so.11 -o $(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_CSHARP_EXT_OBJS) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgrpc -lgpr -laddress_sorting -lupb + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,-soname,libgrpc_csharp_ext.so.11 -o $(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_CSHARP_EXT_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) $(Q) ln -sf $(SHARED_PREFIX)grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).so.11 $(Q) ln -sf $(SHARED_PREFIX)grpc_csharp_ext$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgrpc_csharp_ext$(SHARED_VERSION_CORE).so endif @@ -4776,18 +4776,18 @@ endif ifeq ($(SYSTEM),MINGW32) -$(LIBDIR)/$(CONFIG)/grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/gpr$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/address_sorting$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/upb$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) +$(LIBDIR)/$(CONFIG)/grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(E) "[LD] Linking $@" $(Q) mkdir -p `dirname $@` - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,--output-def=$(LIBDIR)/$(CONFIG)/grpc_unsecure$(SHARED_VERSION_CORE).def -Wl,--out-implib=$(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE)-dll.a -o $(LIBDIR)/$(CONFIG)/grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgpr$(SHARED_VERSION_CORE)-dll -laddress_sorting$(SHARED_VERSION_CORE)-dll -lupb$(SHARED_VERSION_CORE)-dll + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,--output-def=$(LIBDIR)/$(CONFIG)/grpc_unsecure$(SHARED_VERSION_CORE).def -Wl,--out-implib=$(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE)-dll.a -o $(LIBDIR)/$(CONFIG)/grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_UNSECURE_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) else -$(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgpr.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libaddress_sorting.$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libupb.$(SHARED_EXT_CORE) +$(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE): $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_DEP) $(CARES_DEP) $(ADDRESS_SORTING_DEP) $(UPB_DEP) $(GRPC_ABSEIL_DEP) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(E) "[LD] Linking $@" $(Q) mkdir -p `dirname $@` ifeq ($(SYSTEM),Darwin) - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -install_name $(SHARED_PREFIX)grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) -dynamiclib -o $(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgpr -laddress_sorting -lupb + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -install_name $(SHARED_PREFIX)grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) -dynamiclib -o $(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_UNSECURE_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) else - $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,-soname,libgrpc_unsecure.so.11 -o $(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_UNSECURE_OBJS) $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) -lgpr -laddress_sorting -lupb + $(Q) $(LDXX) $(LDFLAGS) -L$(LIBDIR)/$(CONFIG) -shared -Wl,-soname,libgrpc_unsecure.so.11 -o $(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBGRPC_UNSECURE_OBJS) $(LIBDIR)/$(CONFIG)/libgpr.a $(LIBDIR)/$(CONFIG)/libaddress_sorting.a $(LIBDIR)/$(CONFIG)/libupb.a $(ZLIB_MERGE_LIBS) $(CARES_MERGE_LIBS) $(ADDRESS_SORTING_MERGE_LIBS) $(UPB_MERGE_LIBS) $(GRPC_ABSEIL_MERGE_LIBS) $(LDLIBS) $(Q) ln -sf $(SHARED_PREFIX)grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).so.11 $(Q) ln -sf $(SHARED_PREFIX)grpc_unsecure$(SHARED_VERSION_CORE).$(SHARED_EXT_CORE) $(LIBDIR)/$(CONFIG)/libgrpc_unsecure$(SHARED_VERSION_CORE).so endif diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 505bd99ecbd..1ad6a258b25 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1297,6 +1297,7 @@ libs: - absl/strings:strings - absl/container:inlined_vector baselib: true + deps_linkage: static dll: true generate_plugin_registry: true secure: true @@ -1312,6 +1313,7 @@ libs: - gpr - address_sorting - upb + deps_linkage: static dll: only - name: grpc_test_util build: private @@ -2216,6 +2218,7 @@ libs: - absl/strings:strings - absl/container:inlined_vector baselib: true + deps_linkage: static dll: true generate_plugin_registry: true secure: false