From f44e4e2138925952f8e184185b1b770e80746117 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Tue, 14 Jan 2020 16:24:04 +0000 Subject: [PATCH 01/20] Renames according to PR comments --- src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi index dbe673d0bf6..5129e2efed4 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi @@ -125,7 +125,7 @@ cdef class _AioCall(GrpcCallWrapper): if credentials is not None: set_credentials_error = grpc_call_set_credentials(self.call, credentials.c()) if set_credentials_error != GRPC_CALL_OK: - raise Exception("Credentials couldn't have been set") + raise RuntimeError("Credentials couldn't have been set") grpc_slice_unref(method_slice) @@ -178,7 +178,7 @@ cdef class _AioCall(GrpcCallWrapper): def cancel(self, str details): """Cancels the RPC in Core with given RPC status. - + Above abstractions must invoke this method to set Core objects into proper state. """ @@ -286,7 +286,7 @@ cdef class _AioCall(GrpcCallWrapper): bytes request, tuple outbound_initial_metadata): """Performs a unary unary RPC. - + Args: request: the serialized requests in bytes. outbound_initial_metadata: optional outbound metadata. From 88e922c03f5efa8a62b4de433e4116856e41afc4 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Tue, 14 Jan 2020 16:57:07 +0000 Subject: [PATCH 02/20] Make the server test use SSL credentials --- .../grpcio/grpc/_cython/_cygrpc/aio/channel.pxd.pxi | 1 + .../grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi | 5 +++++ .../grpcio_tests/tests_aio/unit/_test_server.py | 13 +++++++++++-- src/python/grpcio_tests/tests_aio/unit/call_test.py | 1 + src/python/grpcio_tests/tests_aio/unit/init_test.py | 13 +++++++++++-- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pxd.pxi index f49681a4588..03b4990e488 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pxd.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pxd.pxi @@ -24,3 +24,4 @@ cdef class AioChannel: object loop bytes _target AioChannelStatus _status + bint _is_secure diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi index a2882e64b7f..21100444863 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi @@ -36,11 +36,13 @@ cdef class AioChannel: self._status = AIO_CHANNEL_STATUS_READY if credentials is None: + self._is_secure = False self.channel = grpc_insecure_channel_create( target, channel_args.c_args(), NULL) else: + self._is_secure = True self.channel = grpc_secure_channel_create( credentials.c(), target, @@ -122,6 +124,9 @@ cdef class AioChannel: cdef CallCredentials cython_call_credentials if python_call_credentials is not None: + if not self._is_secure: + raise RuntimeError("Call credentials are only valid on secure channels") + cython_call_credentials = python_call_credentials._credentials else: cython_call_credentials = None diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index 7c8afa8ff5c..bcc6e3bc304 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -17,6 +17,7 @@ import datetime import grpc from grpc.experimental import aio +from tests.unit import resources from src.proto.grpc.testing import empty_pb2, messages_pb2, test_pb2_grpc from tests_aio.unit import _constants @@ -37,6 +38,11 @@ async def _maybe_echo_metadata(servicer_context): invocation_metadata[_TRAILING_METADATA_KEY]) servicer_context.set_trailing_metadata((trailing_metadatum,)) +_PRIVATE_KEY = resources.private_key() +_CERTIFICATE_CHAIN = resources.certificate_chain() +_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() +_SERVER_CERTS = ((_PRIVATE_KEY, _CERTIFICATE_CHAIN),) + async def _maybe_echo_status(request: messages_pb2.SimpleRequest, servicer_context): @@ -129,8 +135,11 @@ async def start_test_server(port=0, if secure: if server_credentials is None: - server_credentials = grpc.local_server_credentials( - grpc.LocalConnectionType.LOCAL_TCP) + server_credentials = grpc.ssl_server_credentials( + _SERVER_CERTS, + root_certificates=_TEST_ROOT_CERTIFICATES, + require_client_auth=True + ) port = server.add_secure_port('[::]:%d' % port, server_credentials) else: port = server.add_insecure_port('[::]:%d' % port) diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index f845e078684..68cfe3831a8 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -24,6 +24,7 @@ from grpc.experimental import aio from src.proto.grpc.testing import messages_pb2, test_pb2_grpc from tests.unit.framework.common import test_constants from tests_aio.unit._test_base import AioTestBase + from tests_aio.unit._test_server import start_test_server _NUM_STREAM_RESPONSES = 5 diff --git a/src/python/grpcio_tests/tests_aio/unit/init_test.py b/src/python/grpcio_tests/tests_aio/unit/init_test.py index 8b9a03e2dd3..c415faaf6ac 100644 --- a/src/python/grpcio_tests/tests_aio/unit/init_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/init_test.py @@ -20,6 +20,12 @@ from grpc.experimental import aio from tests_aio.unit._test_server import start_test_server from tests_aio.unit._test_base import AioTestBase +from tests.unit import resources + +_PRIVATE_KEY = resources.private_key() +_CERTIFICATE_CHAIN = resources.certificate_chain() +_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() + class TestInsecureChannel(AioTestBase): @@ -37,8 +43,11 @@ class TestSecureChannel(AioTestBase): async def coro(): server_target, _ = await start_test_server(secure=True) # pylint: disable=unused-variable - credentials = grpc.local_channel_credentials( - grpc.LocalConnectionType.LOCAL_TCP) + credentials = grpc.ssl_channel_credentials( + root_certificates=_TEST_ROOT_CERTIFICATES, + private_key=_PRIVATE_KEY, + certificate_chain=_CERTIFICATE_CHAIN, + ) secure_channel = aio.secure_channel(server_target, credentials) self.assertIsInstance(secure_channel, aio.Channel) From 4ec09a0a2623927361fe73195c8a8f6c7d7bdb4f Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Tue, 14 Jan 2020 17:02:40 +0000 Subject: [PATCH 03/20] [WIP] test call credentials --- .../grpcio_tests/tests_aio/unit/call_test.py | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index 68cfe3831a8..836757ca961 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -14,7 +14,6 @@ """Tests behavior of the Call classes.""" import asyncio -import datetime import logging import unittest @@ -24,6 +23,7 @@ from grpc.experimental import aio from src.proto.grpc.testing import messages_pb2, test_pb2_grpc from tests.unit.framework.common import test_constants from tests_aio.unit._test_base import AioTestBase +from tests.unit import resources from tests_aio.unit._test_server import start_test_server @@ -35,6 +35,10 @@ _RESPONSE_INTERVAL_US = test_constants.SHORT_TIMEOUT * 1000 * 1000 _UNREACHABLE_TARGET = '0.1:1111' _INFINITE_INTERVAL_US = 2**31 - 1 +_PRIVATE_KEY = resources.private_key() +_CERTIFICATE_CHAIN = resources.certificate_chain() +_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() + class _MulticallableTestMixin(): @@ -203,6 +207,30 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): with self.assertRaises(asyncio.CancelledError): await task + def test_call_credentials(self): # FIXME + + async def coro(): + server_target, _ = await start_test_server(secure=True) # pylint: disable=unused-variable + channel_credentials = grpc.ssl_channel_credentials( + root_certificates=_TEST_ROOT_CERTIFICATES, + private_key=_PRIVATE_KEY, + certificate_chain=_CERTIFICATE_CHAIN, + ) + + async with aio.secure_channel(server_target, channel_credentials) as channel: + hi = channel.unary_unary('/grpc.testing.TestService/UnaryCall', + request_serializer=messages_pb2. + SimpleRequest.SerializeToString, + response_deserializer=messages_pb2. + SimpleResponse.FromString) + call = hi(messages_pb2.SimpleRequest()) # , credentials=call_credentials) + response = await call + + self.assertIsInstance(response, messages_pb2.SimpleResponse) + self.assertEqual(await call.code(), grpc.StatusCode.OK) + + self.loop.run_until_complete(coro()) + class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): @@ -411,33 +439,6 @@ class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): with self.assertRaises(asyncio.CancelledError): await task - def test_call_credentials(self): - - class DummyAuth(grpc.AuthMetadataPlugin): - - def __call__(self, context, callback): - signature = context.method_name[::-1] - callback((("test", signature),), None) - - async def coro(): - server_target, _ = await start_test_server(secure=False) # pylint: disable=unused-variable - - async with aio.insecure_channel(server_target) as channel: - hi = channel.unary_unary('/grpc.testing.TestService/UnaryCall', - request_serializer=messages_pb2. - SimpleRequest.SerializeToString, - response_deserializer=messages_pb2. - SimpleResponse.FromString) - call_credentials = grpc.metadata_call_credentials(DummyAuth()) - call = hi(messages_pb2.SimpleRequest(), - credentials=call_credentials) - response = await call - - self.assertIsInstance(response, messages_pb2.SimpleResponse) - self.assertEqual(await call.code(), grpc.StatusCode.OK) - - self.loop.run_until_complete(coro()) - async def test_time_remaining(self): request = messages_pb2.StreamingOutputCallRequest() # First message comes back immediately From 764be7ed9293a2017ce3b1fe4040389a54fa89b7 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Mon, 20 Jan 2020 09:14:24 +0100 Subject: [PATCH 04/20] Improve error message by including the error code --- .../grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi | 14 +++++++------- .../grpcio_tests/tests_aio/unit/_test_server.py | 9 ++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi index 5129e2efed4..b64f952fca5 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi @@ -125,7 +125,7 @@ cdef class _AioCall(GrpcCallWrapper): if credentials is not None: set_credentials_error = grpc_call_set_credentials(self.call, credentials.c()) if set_credentials_error != GRPC_CALL_OK: - raise RuntimeError("Credentials couldn't have been set") + raise RuntimeError(f"Credentials couldn't have been set: {set_credentials_error}") grpc_slice_unref(method_slice) @@ -209,7 +209,7 @@ cdef class _AioCall(GrpcCallWrapper): def done(self): """Returns if the RPC call has finished. - + Checks if the status has been provided, either because the RPC finished or because was cancelled.. @@ -220,7 +220,7 @@ cdef class _AioCall(GrpcCallWrapper): def cancelled(self): """Returns if the RPC was cancelled. - + Returns: True if the RPC was cancelled. """ @@ -231,7 +231,7 @@ cdef class _AioCall(GrpcCallWrapper): async def status(self): """Returns the status of the RPC call. - + It returns the finshed status of the RPC. If the RPC has not finished yet this function will wait until the RPC gets finished. @@ -254,7 +254,7 @@ cdef class _AioCall(GrpcCallWrapper): async def initial_metadata(self): """Returns the initial metadata of the RPC call. - + If the initial metadata has not been received yet this function will wait until the RPC gets finished. @@ -420,7 +420,7 @@ cdef class _AioCall(GrpcCallWrapper): tuple outbound_initial_metadata, object metadata_sent_observer): """Actual implementation of the complete unary-stream call. - + Needs to pay extra attention to the raise mechanism. If we want to propagate the final status exception, then we have to raise it. Othersize, it would end normally and raise `StopAsyncIteration()`. @@ -490,7 +490,7 @@ cdef class _AioCall(GrpcCallWrapper): outbound_initial_metadata, self._send_initial_metadata_flags, self._loop) - # Notify upper level that sending messages are allowed now. + # Notify upper level that sending messages are allowed now. metadata_sent_observer() # Receives initial metadata. diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index bcc6e3bc304..e5ec53f1476 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -24,6 +24,10 @@ from tests_aio.unit import _constants _INITIAL_METADATA_KEY = "x-grpc-test-echo-initial" _TRAILING_METADATA_KEY = "x-grpc-test-echo-trailing-bin" +_PRIVATE_KEY = resources.private_key() +_CERTIFICATE_CHAIN = resources.certificate_chain() +_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() +_SERVER_CERTS = ((_PRIVATE_KEY, _CERTIFICATE_CHAIN),) async def _maybe_echo_metadata(servicer_context): @@ -38,11 +42,6 @@ async def _maybe_echo_metadata(servicer_context): invocation_metadata[_TRAILING_METADATA_KEY]) servicer_context.set_trailing_metadata((trailing_metadatum,)) -_PRIVATE_KEY = resources.private_key() -_CERTIFICATE_CHAIN = resources.certificate_chain() -_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() -_SERVER_CERTS = ((_PRIVATE_KEY, _CERTIFICATE_CHAIN),) - async def _maybe_echo_status(request: messages_pb2.SimpleRequest, servicer_context): From a1bb0bc6f8373226e602548ae437cb7199310909 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Wed, 5 Feb 2020 16:45:07 +0100 Subject: [PATCH 05/20] Set error description --- src/python/grpcio/grpc/experimental/aio/_call.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/python/grpcio/grpc/experimental/aio/_call.py b/src/python/grpcio/grpc/experimental/aio/_call.py index d06cc18d872..d7a963bb892 100644 --- a/src/python/grpcio/grpc/experimental/aio/_call.py +++ b/src/python/grpcio/grpc/experimental/aio/_call.py @@ -281,8 +281,13 @@ class _UnaryResponseMixin(Call): if self._cython_call.is_locally_cancelled(): raise asyncio.CancelledError() else: + call_status = self._cython_call._status + debug_error_string = None + if call_status is not None: + debug_error_string = call_status._debug_error_string raise _create_rpc_error(self._cython_call._initial_metadata, - self._cython_call._status) + call_status, + debug_error_string) else: return response From f2aad7e54c6cf56ce41401bf2e8a5920422966e8 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Wed, 5 Feb 2020 18:17:10 +0100 Subject: [PATCH 06/20] Extend tests for secure channels & credentials --- .../grpcio/grpc/experimental/aio/_call.py | 2 +- src/python/grpcio_tests/tests_aio/tests.json | 1 + .../tests_aio/unit/_test_server.py | 9 +-- .../grpcio_tests/tests_aio/unit/call_test.py | 65 ++++++++++++------- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/python/grpcio/grpc/experimental/aio/_call.py b/src/python/grpcio/grpc/experimental/aio/_call.py index d7a963bb892..452856d472e 100644 --- a/src/python/grpcio/grpc/experimental/aio/_call.py +++ b/src/python/grpcio/grpc/experimental/aio/_call.py @@ -282,7 +282,7 @@ class _UnaryResponseMixin(Call): raise asyncio.CancelledError() else: call_status = self._cython_call._status - debug_error_string = None + debug_error_string = "" if call_status is not None: debug_error_string = call_status._debug_error_string raise _create_rpc_error(self._cython_call._initial_metadata, diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index d79ed422596..91725b53e83 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -9,6 +9,7 @@ "unit.call_test.TestStreamUnaryCall", "unit.call_test.TestUnaryStreamCall", "unit.call_test.TestUnaryUnaryCall", + "unit.call_test.TestUnaryUnarySecureCall", "unit.channel_argument_test.TestChannelArgument", "unit.channel_ready_test.TestChannelReady", "unit.channel_test.TestChannel", diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index e5ec53f1476..6face0ae41e 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -134,11 +134,8 @@ async def start_test_server(port=0, if secure: if server_credentials is None: - server_credentials = grpc.ssl_server_credentials( - _SERVER_CERTS, - root_certificates=_TEST_ROOT_CERTIFICATES, - require_client_auth=True - ) + server_credentials = grpc.local_server_credentials( + grpc.LocalConnectionType.LOCAL_TCP) port = server.add_secure_port('[::]:%d' % port, server_credentials) else: port = server.add_insecure_port('[::]:%d' % port) @@ -146,4 +143,4 @@ async def start_test_server(port=0, await server.start() # NOTE(lidizheng) returning the server to prevent it from deallocation - return 'localhost:%d' % port, server + return '0.0.0.0:%d' % port, server diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index 836757ca961..13181028b9c 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -52,6 +52,34 @@ class _MulticallableTestMixin(): await self._server.stop(None) + +class _SecureCallMixin: + """A Mixin to run the call tests over a secure channel.""" + + async def setUp(self): + server_credentials = grpc.ssl_server_credentials([ + (resources.private_key(), resources.certificate_chain()) + ]) + channel_credentials = grpc.ssl_channel_credentials( + resources.test_root_certificates()) + + self._server_address, self._server = await start_test_server( + secure=True, server_credentials=server_credentials) + _SERVER_HOST_OVERRIDE = 'foo.test.google.fr' + channel_options = ( + ( + 'grpc.ssl_target_name_override', + _SERVER_HOST_OVERRIDE, + ), + ) + self._channel = aio.secure_channel(self._server_address, channel_credentials, channel_options) + self._stub = test_pb2_grpc.TestServiceStub(self._channel) + + async def tearDown(self): + await self._channel.close() + await self._server.stop(None) + + class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): async def test_call_to_string(self): @@ -60,7 +88,7 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): self.assertTrue(str(call) is not None) self.assertTrue(repr(call) is not None) - response = await call + await call self.assertTrue(str(call) is not None) self.assertTrue(repr(call) is not None) @@ -207,29 +235,21 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): with self.assertRaises(asyncio.CancelledError): await task - def test_call_credentials(self): # FIXME - - async def coro(): - server_target, _ = await start_test_server(secure=True) # pylint: disable=unused-variable - channel_credentials = grpc.ssl_channel_credentials( - root_certificates=_TEST_ROOT_CERTIFICATES, - private_key=_PRIVATE_KEY, - certificate_chain=_CERTIFICATE_CHAIN, - ) - - async with aio.secure_channel(server_target, channel_credentials) as channel: - hi = channel.unary_unary('/grpc.testing.TestService/UnaryCall', - request_serializer=messages_pb2. - SimpleRequest.SerializeToString, - response_deserializer=messages_pb2. - SimpleResponse.FromString) - call = hi(messages_pb2.SimpleRequest()) # , credentials=call_credentials) - response = await call + async def test_passing_credentials_fails_over_insecure_channel(self): + call_credentials = grpc.composite_call_credentials( + grpc.access_token_call_credentials("abc"), + grpc.access_token_call_credentials("def"), + ) + with self.assertRaisesRegex(RuntimeError, "Call credentials are only valid on secure channels"): + self._stub.UnaryCall(messages_pb2.SimpleRequest(), credentials=call_credentials) - self.assertIsInstance(response, messages_pb2.SimpleResponse) - self.assertEqual(await call.code(), grpc.StatusCode.OK) - self.loop.run_until_complete(coro()) +class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): + """Calls made over a secure channel.""" + async def test_call_ok_with_credentials(self): + call = self._stub.UnaryCall(messages_pb2.SimpleRequest()) + response = await call + self.assertIsInstance(response, messages_pb2.SimpleResponse) class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): @@ -584,7 +604,6 @@ _STREAM_OUTPUT_REQUEST_ONE_RESPONSE.response_parameters.append( class TestStreamStreamCall(_MulticallableTestMixin, AioTestBase): - async def test_cancel(self): # Invokes the actual RPC call = self._stub.FullDuplexCall() From a3459d371dd1691835cc53839138c2e0d5785152 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Wed, 5 Feb 2020 18:25:35 +0100 Subject: [PATCH 07/20] Code cleanup --- src/python/grpcio_tests/tests_aio/unit/_test_server.py | 4 ---- src/python/grpcio_tests/tests_aio/unit/call_test.py | 7 ++----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index 6face0ae41e..7a8a3139e12 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -24,10 +24,6 @@ from tests_aio.unit import _constants _INITIAL_METADATA_KEY = "x-grpc-test-echo-initial" _TRAILING_METADATA_KEY = "x-grpc-test-echo-trailing-bin" -_PRIVATE_KEY = resources.private_key() -_CERTIFICATE_CHAIN = resources.certificate_chain() -_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() -_SERVER_CERTS = ((_PRIVATE_KEY, _CERTIFICATE_CHAIN),) async def _maybe_echo_metadata(servicer_context): diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index 13181028b9c..a5972f838f1 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -34,10 +34,7 @@ _LOCAL_CANCEL_DETAILS_EXPECTATION = 'Locally cancelled by application!' _RESPONSE_INTERVAL_US = test_constants.SHORT_TIMEOUT * 1000 * 1000 _UNREACHABLE_TARGET = '0.1:1111' _INFINITE_INTERVAL_US = 2**31 - 1 - -_PRIVATE_KEY = resources.private_key() -_CERTIFICATE_CHAIN = resources.certificate_chain() -_TEST_ROOT_CERTIFICATES = resources.test_root_certificates() +_SERVER_HOST_OVERRIDE = 'foo.test.google.fr' class _MulticallableTestMixin(): @@ -65,7 +62,6 @@ class _SecureCallMixin: self._server_address, self._server = await start_test_server( secure=True, server_credentials=server_credentials) - _SERVER_HOST_OVERRIDE = 'foo.test.google.fr' channel_options = ( ( 'grpc.ssl_target_name_override', @@ -250,6 +246,7 @@ class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): call = self._stub.UnaryCall(messages_pb2.SimpleRequest()) response = await call self.assertIsInstance(response, messages_pb2.SimpleResponse) + self.assertEqual(await call.code(), grpc.StatusCode.OK) class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): From 33e15156bbcc14a51e78a43a705fd290e4e609e6 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Tue, 11 Feb 2020 13:58:42 +0100 Subject: [PATCH 08/20] [WIP] Feedback on the PR --- .../grpc/_cython/_cygrpc/aio/call.pyx.pxi | 2 +- .../grpcio/grpc/experimental/aio/_call.py | 7 +-- src/python/grpcio_tests/tests_aio/tests.json | 8 +-- .../tests_aio/unit/_test_server.py | 7 +-- .../grpcio_tests/tests_aio/unit/call_test.py | 45 +++------------- .../grpcio_tests/tests_aio/unit/init_test.py | 28 ++++------ .../tests_aio/unit/secure_call_test.py | 51 +++++++++++++++++++ 7 files changed, 77 insertions(+), 71 deletions(-) create mode 100644 src/python/grpcio_tests/tests_aio/unit/secure_call_test.py diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi index b64f952fca5..2e39c75b09f 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi @@ -125,7 +125,7 @@ cdef class _AioCall(GrpcCallWrapper): if credentials is not None: set_credentials_error = grpc_call_set_credentials(self.call, credentials.c()) if set_credentials_error != GRPC_CALL_OK: - raise RuntimeError(f"Credentials couldn't have been set: {set_credentials_error}") + raise RuntimeError("Credentials couldn't have been set: {0}".format(set_credentials_error)) grpc_slice_unref(method_slice) diff --git a/src/python/grpcio/grpc/experimental/aio/_call.py b/src/python/grpcio/grpc/experimental/aio/_call.py index 452856d472e..d06cc18d872 100644 --- a/src/python/grpcio/grpc/experimental/aio/_call.py +++ b/src/python/grpcio/grpc/experimental/aio/_call.py @@ -281,13 +281,8 @@ class _UnaryResponseMixin(Call): if self._cython_call.is_locally_cancelled(): raise asyncio.CancelledError() else: - call_status = self._cython_call._status - debug_error_string = "" - if call_status is not None: - debug_error_string = call_status._debug_error_string raise _create_rpc_error(self._cython_call._initial_metadata, - call_status, - debug_error_string) + self._cython_call._status) else: return response diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index 91725b53e83..ace685d169f 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -9,7 +9,6 @@ "unit.call_test.TestStreamUnaryCall", "unit.call_test.TestUnaryStreamCall", "unit.call_test.TestUnaryUnaryCall", - "unit.call_test.TestUnaryUnarySecureCall", "unit.channel_argument_test.TestChannelArgument", "unit.channel_ready_test.TestChannelReady", "unit.channel_test.TestChannel", @@ -20,10 +19,11 @@ "unit.compression_test.TestCompression", "unit.connectivity_test.TestConnectivityState", "unit.done_callback_test.TestDoneCallback", - "unit.init_test.TestInsecureChannel", - "unit.init_test.TestSecureChannel", + "unit.init_test.TestChannel", + "unit.interceptor_test.TestInterceptedUnaryUnaryCall", + "unit.interceptor_test.TestUnaryUnaryClientInterceptor", "unit.metadata_test.TestMetadata", - "unit.server_interceptor_test.TestServerInterceptor", + "unit.secure_call_test.TestUnaryUnarySecureCall", "unit.server_test.TestServer", "unit.timeout_test.TestTimeout", "unit.wait_for_ready_test.TestWaitForReady" diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_server.py b/src/python/grpcio_tests/tests_aio/unit/_test_server.py index 7a8a3139e12..2396608e5cc 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_server.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_server.py @@ -130,8 +130,9 @@ async def start_test_server(port=0, if secure: if server_credentials is None: - server_credentials = grpc.local_server_credentials( - grpc.LocalConnectionType.LOCAL_TCP) + server_credentials = grpc.ssl_server_credentials([ + (resources.private_key(), resources.certificate_chain()) + ]) port = server.add_secure_port('[::]:%d' % port, server_credentials) else: port = server.add_insecure_port('[::]:%d' % port) @@ -139,4 +140,4 @@ async def start_test_server(port=0, await server.start() # NOTE(lidizheng) returning the server to prevent it from deallocation - return '0.0.0.0:%d' % port, server + return 'localhost:%d' % port, server diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index a5972f838f1..aba6d319d15 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -34,7 +34,6 @@ _LOCAL_CANCEL_DETAILS_EXPECTATION = 'Locally cancelled by application!' _RESPONSE_INTERVAL_US = test_constants.SHORT_TIMEOUT * 1000 * 1000 _UNREACHABLE_TARGET = '0.1:1111' _INFINITE_INTERVAL_US = 2**31 - 1 -_SERVER_HOST_OVERRIDE = 'foo.test.google.fr' class _MulticallableTestMixin(): @@ -49,33 +48,6 @@ class _MulticallableTestMixin(): await self._server.stop(None) - -class _SecureCallMixin: - """A Mixin to run the call tests over a secure channel.""" - - async def setUp(self): - server_credentials = grpc.ssl_server_credentials([ - (resources.private_key(), resources.certificate_chain()) - ]) - channel_credentials = grpc.ssl_channel_credentials( - resources.test_root_certificates()) - - self._server_address, self._server = await start_test_server( - secure=True, server_credentials=server_credentials) - channel_options = ( - ( - 'grpc.ssl_target_name_override', - _SERVER_HOST_OVERRIDE, - ), - ) - self._channel = aio.secure_channel(self._server_address, channel_credentials, channel_options) - self._stub = test_pb2_grpc.TestServiceStub(self._channel) - - async def tearDown(self): - await self._channel.close() - await self._server.stop(None) - - class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): async def test_call_to_string(self): @@ -236,17 +208,11 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): grpc.access_token_call_credentials("abc"), grpc.access_token_call_credentials("def"), ) - with self.assertRaisesRegex(RuntimeError, "Call credentials are only valid on secure channels"): - self._stub.UnaryCall(messages_pb2.SimpleRequest(), credentials=call_credentials) - - -class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): - """Calls made over a secure channel.""" - async def test_call_ok_with_credentials(self): - call = self._stub.UnaryCall(messages_pb2.SimpleRequest()) - response = await call - self.assertIsInstance(response, messages_pb2.SimpleResponse) - self.assertEqual(await call.code(), grpc.StatusCode.OK) + with self.assertRaisesRegex( + RuntimeError, + "Call credentials are only valid on secure channels"): + self._stub.UnaryCall(messages_pb2.SimpleRequest(), + credentials=call_credentials) class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): @@ -601,6 +567,7 @@ _STREAM_OUTPUT_REQUEST_ONE_RESPONSE.response_parameters.append( class TestStreamStreamCall(_MulticallableTestMixin, AioTestBase): + async def test_cancel(self): # Invokes the actual RPC call = self._stub.FullDuplexCall() diff --git a/src/python/grpcio_tests/tests_aio/unit/init_test.py b/src/python/grpcio_tests/tests_aio/unit/init_test.py index c415faaf6ac..9104a0368c5 100644 --- a/src/python/grpcio_tests/tests_aio/unit/init_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/init_test.py @@ -27,7 +27,7 @@ _CERTIFICATE_CHAIN = resources.certificate_chain() _TEST_ROOT_CERTIFICATES = resources.test_root_certificates() -class TestInsecureChannel(AioTestBase): +class TestChannel(AioTestBase): async def test_insecure_channel(self): server_target, _ = await start_test_server() # pylint: disable=unused-variable @@ -35,24 +35,16 @@ class TestInsecureChannel(AioTestBase): channel = aio.insecure_channel(server_target) self.assertIsInstance(channel, aio.Channel) + async def tests_secure_channel(self): + server_target, _ = await start_test_server(secure=True) # pylint: disable=unused-variable + credentials = grpc.ssl_channel_credentials( + root_certificates=_TEST_ROOT_CERTIFICATES, + private_key=_PRIVATE_KEY, + certificate_chain=_CERTIFICATE_CHAIN, + ) + secure_channel = aio.secure_channel(server_target, credentials) -class TestSecureChannel(AioTestBase): - """Test a secure channel connected to a secure server""" - - def test_secure_channel(self): - - async def coro(): - server_target, _ = await start_test_server(secure=True) # pylint: disable=unused-variable - credentials = grpc.ssl_channel_credentials( - root_certificates=_TEST_ROOT_CERTIFICATES, - private_key=_PRIVATE_KEY, - certificate_chain=_CERTIFICATE_CHAIN, - ) - secure_channel = aio.secure_channel(server_target, credentials) - - self.assertIsInstance(secure_channel, aio.Channel) - - self.loop.run_until_complete(coro()) + self.assertIsInstance(secure_channel, aio.Channel) if __name__ == '__main__': diff --git a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py new file mode 100644 index 00000000000..cff870c3054 --- /dev/null +++ b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py @@ -0,0 +1,51 @@ +import unittest +import logging + +import grpc +from grpc.experimental import aio +from src.proto.grpc.testing import messages_pb2, test_pb2_grpc +from tests_aio.unit._test_base import AioTestBase +from tests_aio.unit._test_server import start_test_server +from tests.unit import resources + +_SERVER_HOST_OVERRIDE = 'foo.test.google.fr' + + +class _SecureCallMixin: + """A Mixin to run the call tests over a secure channel.""" + + async def setUp(self): + server_credentials = grpc.ssl_server_credentials([ + (resources.private_key(), resources.certificate_chain()) + ]) + channel_credentials = grpc.ssl_channel_credentials( + resources.test_root_certificates()) + + self._server_address, self._server = await start_test_server( + secure=True, server_credentials=server_credentials) + channel_options = (( + 'grpc.ssl_target_name_override', + _SERVER_HOST_OVERRIDE, + ),) + self._channel = aio.secure_channel(self._server_address, + channel_credentials, channel_options) + self._stub = test_pb2_grpc.TestServiceStub(self._channel) + + async def tearDown(self): + await self._channel.close() + await self._server.stop(None) + + +class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): + """Calls made over a secure channel.""" + + async def test_call_ok_with_credentials(self): + call = self._stub.UnaryCall(messages_pb2.SimpleRequest()) + response = await call + self.assertIsInstance(response, messages_pb2.SimpleResponse) + self.assertEqual(await call.code(), grpc.StatusCode.OK) + + +if __name__ == '__main__': + logging.basicConfig() + unittest.main(verbosity=2) From 8165cc2d9702cac002d8b0cf4f7d242d22812629 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Fri, 14 Feb 2020 17:13:26 +0100 Subject: [PATCH 09/20] Extend with more tests --- src/python/grpcio_tests/tests_aio/tests.json | 1 + .../tests_aio/unit/secure_call_test.py | 37 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index ace685d169f..2aae8533b96 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -23,6 +23,7 @@ "unit.interceptor_test.TestInterceptedUnaryUnaryCall", "unit.interceptor_test.TestUnaryUnaryClientInterceptor", "unit.metadata_test.TestMetadata", + "unit.secure_call_test.TestUnaryStreamSecureCall", "unit.secure_call_test.TestUnaryUnarySecureCall", "unit.server_test.TestServer", "unit.timeout_test.TestTimeout", diff --git a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py index cff870c3054..038a684f509 100644 --- a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py @@ -9,6 +9,8 @@ from tests_aio.unit._test_server import start_test_server from tests.unit import resources _SERVER_HOST_OVERRIDE = 'foo.test.google.fr' +_NUM_STREAM_RESPONSES = 5 +_RESPONSE_PAYLOAD_SIZE = 42 class _SecureCallMixin: @@ -37,14 +39,45 @@ class _SecureCallMixin: class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): - """Calls made over a secure channel.""" + """unary_unary Calls made over a secure channel.""" - async def test_call_ok_with_credentials(self): + async def test_call_ok_over_secure_channel(self): call = self._stub.UnaryCall(messages_pb2.SimpleRequest()) response = await call self.assertIsInstance(response, messages_pb2.SimpleResponse) self.assertEqual(await call.code(), grpc.StatusCode.OK) + async def test_call_with_credentials(self): + call_credentials = grpc.composite_call_credentials( + grpc.access_token_call_credentials("abc"), + grpc.access_token_call_credentials("def"), + ) + call = self._stub.UnaryCall(messages_pb2.SimpleRequest(), credentials=call_credentials) + response = await call + + self.assertIsInstance(response, messages_pb2.SimpleResponse) + + +class TestUnaryStreamSecureCall(_SecureCallMixin, AioTestBase): + """unary_stream calls over a secure channel""" + + async def test_unary_stream_async_generator_credentials(self): + request = messages_pb2.StreamingOutputCallRequest() + request.response_parameters.extend( + messages_pb2.ResponseParameters(size=_RESPONSE_PAYLOAD_SIZE,) + for _ in range(_NUM_STREAM_RESPONSES) + ) + call = self._stub.StreamingOutputCall(request) + + async for response in call: + self.assertIsInstance( + response, + messages_pb2.StreamingOutputCallResponse + ) + self.assertEqual(len(response.payload.body), _RESPONSE_PAYLOAD_SIZE) + + self.assertEqual(await call.code(), grpc.StatusCode.OK) + if __name__ == '__main__': logging.basicConfig() From 4d492d28a72d2b82fa28746e9f423efd41cd98ab Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Mon, 17 Feb 2020 21:12:34 +0100 Subject: [PATCH 10/20] Add test for full-duplex call --- src/python/grpcio_tests/tests_aio/tests.json | 1 + .../tests_aio/unit/secure_call_test.py | 46 +++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index 2aae8533b96..c8481b912ad 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -23,6 +23,7 @@ "unit.interceptor_test.TestInterceptedUnaryUnaryCall", "unit.interceptor_test.TestUnaryUnaryClientInterceptor", "unit.metadata_test.TestMetadata", + "unit.secure_call_test.TestStreamStreamSecureCall", "unit.secure_call_test.TestUnaryStreamSecureCall", "unit.secure_call_test.TestUnaryUnarySecureCall", "unit.server_test.TestServer", diff --git a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py index 038a684f509..a5d22d483f3 100644 --- a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py @@ -52,7 +52,8 @@ class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): grpc.access_token_call_credentials("abc"), grpc.access_token_call_credentials("def"), ) - call = self._stub.UnaryCall(messages_pb2.SimpleRequest(), credentials=call_credentials) + call = self._stub.UnaryCall(messages_pb2.SimpleRequest(), + credentials=call_credentials) response = await call self.assertIsInstance(response, messages_pb2.SimpleResponse) @@ -61,24 +62,53 @@ class TestUnaryUnarySecureCall(_SecureCallMixin, AioTestBase): class TestUnaryStreamSecureCall(_SecureCallMixin, AioTestBase): """unary_stream calls over a secure channel""" - async def test_unary_stream_async_generator_credentials(self): + async def test_unary_stream_async_generator_secure(self): request = messages_pb2.StreamingOutputCallRequest() request.response_parameters.extend( messages_pb2.ResponseParameters(size=_RESPONSE_PAYLOAD_SIZE,) - for _ in range(_NUM_STREAM_RESPONSES) + for _ in range(_NUM_STREAM_RESPONSES)) + call_credentials = grpc.composite_call_credentials( + grpc.access_token_call_credentials("abc"), + grpc.access_token_call_credentials("def"), ) - call = self._stub.StreamingOutputCall(request) + call = self._stub.StreamingOutputCall(request, + credentials=call_credentials) async for response in call: - self.assertIsInstance( - response, - messages_pb2.StreamingOutputCallResponse - ) + self.assertIsInstance(response, + messages_pb2.StreamingOutputCallResponse) self.assertEqual(len(response.payload.body), _RESPONSE_PAYLOAD_SIZE) self.assertEqual(await call.code(), grpc.StatusCode.OK) +# Prepares the request that stream in a ping-pong manner. +_STREAM_OUTPUT_REQUEST_ONE_RESPONSE = messages_pb2.StreamingOutputCallRequest() +_STREAM_OUTPUT_REQUEST_ONE_RESPONSE.response_parameters.append( + messages_pb2.ResponseParameters(size=_RESPONSE_PAYLOAD_SIZE)) + + +class TestStreamStreamSecureCall(_SecureCallMixin, AioTestBase): + + async def test_async_generator_secure_channel(self): + + async def request_generator(): + for _ in range(2): + yield _STREAM_OUTPUT_REQUEST_ONE_RESPONSE + + call_credentials = grpc.composite_call_credentials( + grpc.access_token_call_credentials("abc"), + grpc.access_token_call_credentials("def"), + ) + + call = self._stub.FullDuplexCall(request_generator(), + credentials=call_credentials) + async for response in call: + self.assertEqual(_RESPONSE_PAYLOAD_SIZE, len(response.payload.body)) + + self.assertEqual(await call.code(), grpc.StatusCode.OK) + + if __name__ == '__main__': logging.basicConfig() unittest.main(verbosity=2) From dff4a30b6cfac08df4be687e2637ddd87134748a Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Wed, 26 Feb 2020 13:44:48 +0100 Subject: [PATCH 11/20] Fix build & address review comments Fixes https://github.com/grpc/grpc/issues/20532 --- .../grpc/_cython/_cygrpc/aio/call.pyx.pxi | 2 +- .../grpc/_cython/_cygrpc/aio/channel.pyx.pxi | 2 +- .../tests_aio/unit/secure_call_test.py | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi index 2e39c75b09f..bfbcb8d4fc8 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/call.pyx.pxi @@ -125,7 +125,7 @@ cdef class _AioCall(GrpcCallWrapper): if credentials is not None: set_credentials_error = grpc_call_set_credentials(self.call, credentials.c()) if set_credentials_error != GRPC_CALL_OK: - raise RuntimeError("Credentials couldn't have been set: {0}".format(set_credentials_error)) + raise InternalError("Credentials couldn't have been set: {0}".format(set_credentials_error)) grpc_slice_unref(method_slice) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi index 21100444863..beadce67b4a 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/channel.pyx.pxi @@ -125,7 +125,7 @@ cdef class AioChannel: cdef CallCredentials cython_call_credentials if python_call_credentials is not None: if not self._is_secure: - raise RuntimeError("Call credentials are only valid on secure channels") + raise UsageError("Call credentials are only valid on secure channels") cython_call_credentials = python_call_credentials._credentials else: diff --git a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py index a5d22d483f3..e6b69331d24 100644 --- a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py @@ -1,3 +1,18 @@ +# 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. +"""Tests the behaviour of the Call classes under a secure channel.""" + import unittest import logging @@ -89,11 +104,12 @@ _STREAM_OUTPUT_REQUEST_ONE_RESPONSE.response_parameters.append( class TestStreamStreamSecureCall(_SecureCallMixin, AioTestBase): + _STREAM_ITERATIONS = 2 async def test_async_generator_secure_channel(self): async def request_generator(): - for _ in range(2): + for _ in range(self._STREAM_ITERATIONS): yield _STREAM_OUTPUT_REQUEST_ONE_RESPONSE call_credentials = grpc.composite_call_credentials( From a8f7dfe4c5988f2e2e8fa6de02fca48ca85223e8 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Wed, 26 Feb 2020 18:12:25 +0100 Subject: [PATCH 12/20] Update exception used in test --- src/python/grpcio_tests/tests_aio/unit/call_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index aba6d319d15..93b27853023 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -209,7 +209,7 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): grpc.access_token_call_credentials("def"), ) with self.assertRaisesRegex( - RuntimeError, + grpc._cygrpc.UsageError, "Call credentials are only valid on secure channels"): self._stub.UnaryCall(messages_pb2.SimpleRequest(), credentials=call_credentials) From bd4679439c27aec791f3c61f814db22c0b8d3efa Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Mon, 2 Mar 2020 14:09:14 +0100 Subject: [PATCH 13/20] Add 'resources' to bazel build file --- src/python/grpcio_tests/tests_aio/interop/BUILD.bazel | 2 ++ src/python/grpcio_tests/tests_aio/tests.json | 1 + src/python/grpcio_tests/tests_aio/unit/BUILD.bazel | 2 ++ 3 files changed, 5 insertions(+) diff --git a/src/python/grpcio_tests/tests_aio/interop/BUILD.bazel b/src/python/grpcio_tests/tests_aio/interop/BUILD.bazel index b5bbdc6df4e..f67ad35cca7 100644 --- a/src/python/grpcio_tests/tests_aio/interop/BUILD.bazel +++ b/src/python/grpcio_tests/tests_aio/interop/BUILD.bazel @@ -56,6 +56,7 @@ py_binary( python_version = "PY3", deps = [ "//src/python/grpcio/grpc:grpcio", + "//src/python/grpcio_tests/tests/interop:resources", "//src/python/grpcio_tests/tests/interop:server", "//src/python/grpcio_tests/tests_aio/unit:_test_server", ], @@ -70,5 +71,6 @@ py_binary( ":methods", "//src/python/grpcio/grpc:grpcio", "//src/python/grpcio_tests/tests/interop:client", + "//src/python/grpcio_tests/tests/interop:resources", ], ) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index c8481b912ad..2fb6893db04 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -26,6 +26,7 @@ "unit.secure_call_test.TestStreamStreamSecureCall", "unit.secure_call_test.TestUnaryStreamSecureCall", "unit.secure_call_test.TestUnaryUnarySecureCall", + "unit.server_interceptor_test.TestServerInterceptor", "unit.server_test.TestServer", "unit.timeout_test.TestTimeout", "unit.wait_for_ready_test.TestWaitForReady" diff --git a/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel b/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel index ab475bcf97c..1847e9cff6e 100644 --- a/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel +++ b/src/python/grpcio_tests/tests_aio/unit/BUILD.bazel @@ -41,6 +41,7 @@ py_library( "//src/proto/grpc/testing:py_messages_proto", "//src/proto/grpc/testing:test_py_pb2_grpc", "//src/python/grpcio/grpc:grpcio", + "//src/python/grpcio_tests/tests/unit:resources", ], ) @@ -76,6 +77,7 @@ _FLAKY_TESTS = [ "//src/proto/grpc/testing:benchmark_service_py_pb2_grpc", "//src/proto/grpc/testing:py_messages_proto", "//src/python/grpcio/grpc:grpcio", + "//src/python/grpcio_tests/tests/unit:resources", "//src/python/grpcio_tests/tests/unit/framework/common", "@six", ], From 963429864ed8ac2e98c908d38b2dd98ca6c1ff3c Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Wed, 11 Mar 2020 15:28:33 +0100 Subject: [PATCH 14/20] Skip failing tests due to a regression Reported at https://github.com/grpc/grpc/issues/22302 --- src/python/grpcio_tests/tests_aio/unit/call_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index 93b27853023..fc9529d9607 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -184,6 +184,8 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): self.assertEqual(await call.details(), 'Locally cancelled by application!') + @unittest.skip( + "segmentation fault: https://github.com/grpc/grpc/issues/22302") async def test_cancel_unary_unary_in_task(self): coro_started = asyncio.Event() call = self._stub.EmptyCall(messages_pb2.SimpleRequest()) @@ -363,6 +365,8 @@ class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): self.assertEqual(await call.code(), grpc.StatusCode.OK) + @unittest.skip( + "segmentation fault: https://github.com/grpc/grpc/issues/22302") async def test_cancel_unary_stream_in_task_using_read(self): coro_started = asyncio.Event() @@ -392,6 +396,8 @@ class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): with self.assertRaises(asyncio.CancelledError): await task + @unittest.skip( + "segmentation fault: https://github.com/grpc/grpc/issues/22302") async def test_cancel_unary_stream_in_task_using_async_for(self): coro_started = asyncio.Event() From da698286e4474188447e3c40041ca3dd2b576450 Mon Sep 17 00:00:00 2001 From: Mariano Anaya Date: Thu, 12 Mar 2020 10:10:20 +0100 Subject: [PATCH 15/20] Fix tests.json Remove tests from the old branch (conflict resolution leftover issue). --- src/python/grpcio_tests/tests_aio/tests.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/tests.json b/src/python/grpcio_tests/tests_aio/tests.json index 2fb6893db04..b2b53a3ad65 100644 --- a/src/python/grpcio_tests/tests_aio/tests.json +++ b/src/python/grpcio_tests/tests_aio/tests.json @@ -20,8 +20,6 @@ "unit.connectivity_test.TestConnectivityState", "unit.done_callback_test.TestDoneCallback", "unit.init_test.TestChannel", - "unit.interceptor_test.TestInterceptedUnaryUnaryCall", - "unit.interceptor_test.TestUnaryUnaryClientInterceptor", "unit.metadata_test.TestMetadata", "unit.secure_call_test.TestStreamStreamSecureCall", "unit.secure_call_test.TestUnaryStreamSecureCall", From 3779b12203261ad90b6bcfb17f77a3186446e8c7 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 13 Mar 2020 12:10:29 -0700 Subject: [PATCH 16/20] Enable skipped tests --- src/python/grpcio_tests/tests_aio/unit/call_test.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/python/grpcio_tests/tests_aio/unit/call_test.py b/src/python/grpcio_tests/tests_aio/unit/call_test.py index fc9529d9607..93b27853023 100644 --- a/src/python/grpcio_tests/tests_aio/unit/call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/call_test.py @@ -184,8 +184,6 @@ class TestUnaryUnaryCall(_MulticallableTestMixin, AioTestBase): self.assertEqual(await call.details(), 'Locally cancelled by application!') - @unittest.skip( - "segmentation fault: https://github.com/grpc/grpc/issues/22302") async def test_cancel_unary_unary_in_task(self): coro_started = asyncio.Event() call = self._stub.EmptyCall(messages_pb2.SimpleRequest()) @@ -365,8 +363,6 @@ class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): self.assertEqual(await call.code(), grpc.StatusCode.OK) - @unittest.skip( - "segmentation fault: https://github.com/grpc/grpc/issues/22302") async def test_cancel_unary_stream_in_task_using_read(self): coro_started = asyncio.Event() @@ -396,8 +392,6 @@ class TestUnaryStreamCall(_MulticallableTestMixin, AioTestBase): with self.assertRaises(asyncio.CancelledError): await task - @unittest.skip( - "segmentation fault: https://github.com/grpc/grpc/issues/22302") async def test_cancel_unary_stream_in_task_using_async_for(self): coro_started = asyncio.Event() From 82050ae19264389cdc1746905d7ebddff6a0506d Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Tue, 17 Mar 2020 13:23:11 -0700 Subject: [PATCH 17/20] Initialize credentials after IO engine set --- src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi | 4 ++-- src/python/grpcio/grpc/experimental/aio/__init__.py | 5 ++++- src/python/grpcio_tests/tests_aio/unit/_test_base.py | 3 +++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi index 8cb113c462c..35e7c2a38f2 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi @@ -114,7 +114,7 @@ cdef _actual_aio_shutdown(): raise ValueError('Unsupported engine type [%s]' % _global_aio_state.engine) -cdef init_grpc_aio(): +cpdef init_grpc_aio(): """Initialis the gRPC AsyncIO module. Expected to be invoked on critical class constructors. @@ -126,7 +126,7 @@ cdef init_grpc_aio(): _actual_aio_initialization() -cdef shutdown_grpc_aio(): +cpdef shutdown_grpc_aio(): """Shuts down the gRPC AsyncIO module. Expected to be invoked on critical class destructors. diff --git a/src/python/grpcio/grpc/experimental/aio/__init__.py b/src/python/grpcio/grpc/experimental/aio/__init__.py index 31882be24fc..3613908a961 100644 --- a/src/python/grpcio/grpc/experimental/aio/__init__.py +++ b/src/python/grpcio/grpc/experimental/aio/__init__.py @@ -20,7 +20,8 @@ created. AsyncIO doesn't provide thread safety for most of its APIs. from typing import Any, Optional, Sequence, Tuple import grpc -from grpc._cython.cygrpc import (EOF, AbortError, BaseError, InternalError, +from grpc._cython.cygrpc import (init_grpc_aio, shutdown_grpc_aio, EOF, + AbortError, BaseError, InternalError, UsageError) from ._base_call import (Call, RpcContext, StreamStreamCall, StreamUnaryCall, @@ -39,6 +40,8 @@ from ._channel import insecure_channel, secure_channel ################################### __all__ ################################# __all__ = ( + 'init_grpc_aio', + 'shutdown_grpc_aio', 'AioRpcError', 'RpcContext', 'Call', diff --git a/src/python/grpcio_tests/tests_aio/unit/_test_base.py b/src/python/grpcio_tests/tests_aio/unit/_test_base.py index ec5f2112da0..82ec7b456ad 100644 --- a/src/python/grpcio_tests/tests_aio/unit/_test_base.py +++ b/src/python/grpcio_tests/tests_aio/unit/_test_base.py @@ -64,3 +64,6 @@ class AioTestBase(unittest.TestCase): return _async_to_sync_decorator(attr, self._TEST_LOOP) # For other attributes, let them pass. return attr + + +aio.init_grpc_aio() From 030b1b6a84967033d790a0da7071fa1a17ec11d2 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Tue, 17 Mar 2020 13:28:24 -0700 Subject: [PATCH 18/20] Update the pxd as well --- src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pxd.pxi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pxd.pxi index 1755b702015..ebf0660174d 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pxd.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pxd.pxi @@ -23,10 +23,10 @@ cdef class _AioState: cdef grpc_completion_queue *global_completion_queue() -cdef init_grpc_aio() +cpdef init_grpc_aio() -cdef shutdown_grpc_aio() +cpdef shutdown_grpc_aio() cdef extern from "src/core/lib/iomgr/timer_manager.h": From 46bb3769a9344d294270a05f3ee898f4900f76b4 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 18 Mar 2020 10:13:05 -0700 Subject: [PATCH 19/20] Override the auth plugin behavior --- .../grpc/_cython/_cygrpc/aio/iomgr/iomgr.pyx.pxi | 11 +++++++++++ .../grpc/_cython/_cygrpc/credentials.pyx.pxi | 14 ++++++++------ .../tests_aio/unit/secure_call_test.py | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/iomgr/iomgr.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/iomgr/iomgr.pyx.pxi index ac62c41e0f2..f5b62af5287 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/iomgr/iomgr.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/iomgr/iomgr.pyx.pxi @@ -212,7 +212,18 @@ cdef void asyncio_run_loop(size_t timeout_ms) with gil: pass +def _auth_plugin_callback_wrapper(object cb, + str service_url, + str method_name, + object callback): + asyncio.get_event_loop().call_soon(cb, service_url, method_name, callback) + + def install_asyncio_iomgr(): + # Auth plugins invoke user provided logic in another thread by default. We + # need to override that behavior by registering the call to the event loop. + set_async_callback_func(_auth_plugin_callback_wrapper) + asyncio_resolver_vtable.resolve = asyncio_resolve asyncio_resolver_vtable.resolve_async = asyncio_resolve_async diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi index c736b7a10c5..24d1e2a3b77 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi @@ -34,12 +34,14 @@ cdef class CallCredentials: raise NotImplementedError() -cdef int _get_metadata( - void *state, grpc_auth_metadata_context context, - grpc_credentials_plugin_metadata_cb cb, void *user_data, - grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], - size_t *num_creds_md, grpc_status_code *status, - const char **error_details) except * with gil: +cdef int _get_metadata(void *state, + grpc_auth_metadata_context context, + grpc_credentials_plugin_metadata_cb cb, + void *user_data, + grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], + size_t *num_creds_md, + grpc_status_code *status, + const char **error_details) except * with gil: cdef size_t metadata_count cdef grpc_metadata *c_metadata def callback(metadata, grpc_status_code status, bytes error_details): diff --git a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py index e6b69331d24..7efaddd607e 100644 --- a/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py +++ b/src/python/grpcio_tests/tests_aio/unit/secure_call_test.py @@ -126,5 +126,5 @@ class TestStreamStreamSecureCall(_SecureCallMixin, AioTestBase): if __name__ == '__main__': - logging.basicConfig() + logging.basicConfig(level=logging.DEBUG) unittest.main(verbosity=2) From ae0dc69cb1df8793f3ec2b020202dd48a53da472 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Thu, 19 Mar 2020 13:19:41 -0700 Subject: [PATCH 20/20] Fix the typo; I got the wrong branch and waited two hours --- src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi index 35e7c2a38f2..0d01b4ae838 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/aio/grpc_aio.pyx.pxi @@ -115,8 +115,8 @@ cdef _actual_aio_shutdown(): cpdef init_grpc_aio(): - """Initialis the gRPC AsyncIO module. - + """Initializes the gRPC AsyncIO module. + Expected to be invoked on critical class constructors. E.g., AioChannel, AioServer. """