From c6e0ff69faeda614aa6088af59d3420e16720d27 Mon Sep 17 00:00:00 2001 From: "gcf-owl-bot[bot]" <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Date: Sat, 24 Jul 2021 22:04:23 +0000 Subject: [PATCH] fix: enable self signed jwt for grpc (#458) PiperOrigin-RevId: 386504689 Source-Link: https://github.com/googleapis/googleapis/commit/762094a99ac6e03a17516b13dfbef37927267a70 Source-Link: https://github.com/googleapis/googleapis-gen/commit/6bfc480e1a161d5de121c2bcc3745885d33b265a --- .../services/publisher/async_client.py | 1 - google/pubsub_v1/services/publisher/client.py | 5 +- .../services/schema_service/client.py | 4 + .../pubsub_v1/services/subscriber/client.py | 4 + owlbot.py | 105 +++++++++++++++++- scripts/fixup_pubsub_v1_keywords.py | 4 +- tests/unit/gapic/pubsub_v1/test_publisher.py | 30 +++-- .../gapic/pubsub_v1/test_schema_service.py | 31 +++--- tests/unit/gapic/pubsub_v1/test_subscriber.py | 29 +++-- 9 files changed, 169 insertions(+), 44 deletions(-) diff --git a/google/pubsub_v1/services/publisher/async_client.py b/google/pubsub_v1/services/publisher/async_client.py index df436e721..09e4b0e55 100644 --- a/google/pubsub_v1/services/publisher/async_client.py +++ b/google/pubsub_v1/services/publisher/async_client.py @@ -29,7 +29,6 @@ from google.iam.v1 import iam_policy_pb2 # type: ignore from google.iam.v1 import policy_pb2 # type: ignore -from google.protobuf import duration_pb2 # type: ignore from google.pubsub_v1.services.publisher import pagers from google.pubsub_v1.types import pubsub from google.pubsub_v1.types import TimeoutType diff --git a/google/pubsub_v1/services/publisher/client.py b/google/pubsub_v1/services/publisher/client.py index e68443254..cb60f2810 100644 --- a/google/pubsub_v1/services/publisher/client.py +++ b/google/pubsub_v1/services/publisher/client.py @@ -34,7 +34,6 @@ from google.iam.v1 import iam_policy_pb2 # type: ignore from google.iam.v1 import policy_pb2 # type: ignore -from google.protobuf import duration_pb2 # type: ignore from google.pubsub_v1.services.publisher import pagers from google.pubsub_v1.types import pubsub from google.pubsub_v1.types import TimeoutType @@ -399,6 +398,10 @@ def __init__( client_cert_source_for_mtls=client_cert_source_func, quota_project_id=client_options.quota_project_id, client_info=client_info, + always_use_jwt_access=( + Transport == type(self).get_transport_class("grpc") + or Transport == type(self).get_transport_class("grpc_asyncio") + ), ) def create_topic( diff --git a/google/pubsub_v1/services/schema_service/client.py b/google/pubsub_v1/services/schema_service/client.py index 106afa850..5701a9bb0 100644 --- a/google/pubsub_v1/services/schema_service/client.py +++ b/google/pubsub_v1/services/schema_service/client.py @@ -342,6 +342,10 @@ def __init__( client_cert_source_for_mtls=client_cert_source_func, quota_project_id=client_options.quota_project_id, client_info=client_info, + always_use_jwt_access=( + Transport == type(self).get_transport_class("grpc") + or Transport == type(self).get_transport_class("grpc_asyncio") + ), ) def create_schema( diff --git a/google/pubsub_v1/services/subscriber/client.py b/google/pubsub_v1/services/subscriber/client.py index 9f506214a..7f3c3a9dc 100644 --- a/google/pubsub_v1/services/subscriber/client.py +++ b/google/pubsub_v1/services/subscriber/client.py @@ -411,6 +411,10 @@ def __init__( client_cert_source_for_mtls=client_cert_source_func, quota_project_id=client_options.quota_project_id, client_info=client_info, + always_use_jwt_access=( + Transport == type(self).get_transport_class("grpc") + or Transport == type(self).get_transport_class("grpc_asyncio") + ), ) def create_subscription( diff --git a/owlbot.py b/owlbot.py index 6cceccf8a..d6a43e71a 100644 --- a/owlbot.py +++ b/owlbot.py @@ -299,14 +299,111 @@ ), ) + # Add development feature `message_retention_duration` from pubsub_dev branch of googleapis + # See PR https://github.com/googleapis/python-pubsub/pull/456 + count = s.replace( + library / f"google/pubsub_{library.name}/types/pubsub.py", + """satisfies_pzs \(bool\): + Reserved for future use. This field is set + only in responses from the server; it is ignored + if it is set in any requests.""", + """satisfies_pzs (bool): + Reserved for future use. This field is set + only in responses from the server; it is ignored + if it is set in any requests. + message_retention_duration (google.protobuf.duration_pb2.Duration): + Indicates the minimum duration to retain a message after it + is published to the topic. If this field is set, messages + published to the topic in the last + ``message_retention_duration`` are always available to + subscribers. For instance, it allows any attached + subscription to `seek to a + timestamp `__ + that is up to ``message_retention_duration`` in the past. If + this field is not set, message retention is controlled by + settings on individual subscriptions. Cannot be more than 7 + days or less than 10 minutes.""" + ) + + # Add development feature `message_retention_duration` from pubsub_dev branch of googleapis + # See PR https://github.com/googleapis/python-pubsub/pull/456 + count += s.replace( + library / f"google/pubsub_{library.name}/types/pubsub.py", + """satisfies_pzs = proto.Field\( + proto.BOOL, + number=7, + \)""", + """satisfies_pzs = proto.Field( + proto.BOOL, + number=7, + ) + message_retention_duration = proto.Field( + proto.MESSAGE, number=8, message=duration_pb2.Duration, + )""" + ) + + # Add development feature `topic_message_retention_duration` from pubsub_dev branch of googleapis + # See PR https://github.com/googleapis/python-pubsub/pull/456 + count += s.replace( + library / f"google/pubsub_{library.name}/types/pubsub.py", + """detached \(bool\): + Indicates whether the subscription is detached from its + topic. Detached subscriptions don't receive messages from + their topic and don't retain any backlog. ``Pull`` and + ``StreamingPull`` requests will return FAILED_PRECONDITION. + If the subscription is a push subscription, pushes to the + endpoint will not be made.""", + """detached (bool): + Indicates whether the subscription is detached from its + topic. Detached subscriptions don't receive messages from + their topic and don't retain any backlog. ``Pull`` and + ``StreamingPull`` requests will return FAILED_PRECONDITION. + If the subscription is a push subscription, pushes to the + endpoint will not be made. + topic_message_retention_duration (google.protobuf.duration_pb2.Duration): + Output only. Indicates the minimum duration for which a + message is retained after it is published to the + subscription's topic. If this field is set, messages + published to the subscription's topic in the last + ``topic_message_retention_duration`` are always available to + subscribers. See the ``message_retention_duration`` field in + ``Topic``. This field is set only in responses from the + server; it is ignored if it is set in any requests.""" + ) + + # Add development feature `topic_message_retention_duration` from pubsub_dev branch of googleapis + # See PR https://github.com/googleapis/python-pubsub/pull/456 + count += s.replace( + library / f"google/pubsub_{library.name}/types/pubsub.py", + """detached = proto.Field\( + proto.BOOL, + number=15, + \)""", + """detached = proto.Field( + proto.BOOL, + number=15, + ) + topic_message_retention_duration = proto.Field( + proto.MESSAGE, number=17, message=duration_pb2.Duration, + ) + """ + ) + + if count != 4: + raise Exception("Pub/Sub topic retention feature not added") + # The namespace package declaration in google/cloud/__init__.py should be excluded # from coverage. - s.replace( - ".coveragerc", - r"((?P[^\n\S]+)google/pubsub/__init__\.py)", - "\ggoogle/cloud/__init__.py\n\g<0>", + count = s.replace( + library / ".coveragerc", + "google/pubsub/__init__.py", + """google/cloud/__init__.py + google/pubsub/__init__.py""", ) + if count < 1: + raise Exception(".coveragerc replacement failed.") + s.move( library, excludes=[ diff --git a/scripts/fixup_pubsub_v1_keywords.py b/scripts/fixup_pubsub_v1_keywords.py index da668f42f..7262e021e 100644 --- a/scripts/fixup_pubsub_v1_keywords.py +++ b/scripts/fixup_pubsub_v1_keywords.py @@ -42,8 +42,8 @@ class pubsubCallTransformer(cst.CSTTransformer): 'acknowledge': ('subscription', 'ack_ids', ), 'create_schema': ('parent', 'schema', 'schema_id', ), 'create_snapshot': ('name', 'subscription', 'labels', ), - 'create_subscription': ('name', 'topic', 'push_config', 'ack_deadline_seconds', 'retain_acked_messages', 'message_retention_duration', 'labels', 'enable_message_ordering', 'expiration_policy', 'filter', 'dead_letter_policy', 'retry_policy', 'detached', 'topic_message_retention_duration', ), - 'create_topic': ('name', 'labels', 'message_storage_policy', 'kms_key_name', 'schema_settings', 'satisfies_pzs', 'message_retention_duration', ), + 'create_subscription': ('name', 'topic', 'push_config', 'ack_deadline_seconds', 'retain_acked_messages', 'message_retention_duration', 'labels', 'enable_message_ordering', 'expiration_policy', 'filter', 'dead_letter_policy', 'retry_policy', 'detached', ), + 'create_topic': ('name', 'labels', 'message_storage_policy', 'kms_key_name', 'schema_settings', 'satisfies_pzs', ), 'delete_schema': ('name', ), 'delete_snapshot': ('snapshot', ), 'delete_subscription': ('subscription', ), diff --git a/tests/unit/gapic/pubsub_v1/test_publisher.py b/tests/unit/gapic/pubsub_v1/test_publisher.py index ae5654d87..9f122309b 100644 --- a/tests/unit/gapic/pubsub_v1/test_publisher.py +++ b/tests/unit/gapic/pubsub_v1/test_publisher.py @@ -35,7 +35,6 @@ from google.iam.v1 import options_pb2 # type: ignore from google.iam.v1 import policy_pb2 # type: ignore from google.oauth2 import service_account -from google.protobuf import duration_pb2 # type: ignore from google.protobuf import field_mask_pb2 # type: ignore from google.protobuf import timestamp_pb2 # type: ignore from google.pubsub_v1.services.publisher import PublisherAsyncClient @@ -116,16 +115,6 @@ def test_publisher_client_from_service_account_info(client_class): assert client.transport._host == "pubsub.googleapis.com:443" -@pytest.mark.parametrize("client_class", [PublisherClient, PublisherAsyncClient,]) -def test_publisher_client_service_account_always_use_jwt(client_class): - with mock.patch.object( - service_account.Credentials, "with_always_use_jwt_access", create=True - ) as use_jwt: - creds = service_account.Credentials(None, None, None) - client = client_class(credentials=creds) - use_jwt.assert_not_called() - - @pytest.mark.parametrize( "transport_class,transport_name", [ @@ -133,7 +122,7 @@ def test_publisher_client_service_account_always_use_jwt(client_class): (transports.PublisherGrpcAsyncIOTransport, "grpc_asyncio"), ], ) -def test_publisher_client_service_account_always_use_jwt_true( +def test_publisher_client_service_account_always_use_jwt( transport_class, transport_name ): with mock.patch.object( @@ -143,6 +132,13 @@ def test_publisher_client_service_account_always_use_jwt_true( transport = transport_class(credentials=creds, always_use_jwt_access=True) use_jwt.assert_called_once_with(True) + with mock.patch.object( + service_account.Credentials, "with_always_use_jwt_access", create=True + ) as use_jwt: + creds = service_account.Credentials(None, None, None) + transport = transport_class(credentials=creds, always_use_jwt_access=False) + use_jwt.assert_not_called() + @pytest.mark.parametrize("client_class", [PublisherClient, PublisherAsyncClient,]) def test_publisher_client_from_service_account_file(client_class): @@ -217,6 +213,7 @@ def test_publisher_client_client_options(client_class, transport_class, transpor client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT is @@ -233,6 +230,7 @@ def test_publisher_client_client_options(client_class, transport_class, transpor client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT is @@ -249,6 +247,7 @@ def test_publisher_client_client_options(client_class, transport_class, transpor client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT has @@ -277,6 +276,7 @@ def test_publisher_client_client_options(client_class, transport_class, transpor client_cert_source_for_mtls=None, quota_project_id="octopus", client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -341,6 +341,7 @@ def test_publisher_client_mtls_env_auto( client_cert_source_for_mtls=expected_client_cert_source, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case ADC client cert is provided. Whether client cert is used depends on @@ -374,6 +375,7 @@ def test_publisher_client_mtls_env_auto( client_cert_source_for_mtls=expected_client_cert_source, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case client_cert_source and ADC client cert are not provided. @@ -395,6 +397,7 @@ def test_publisher_client_mtls_env_auto( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -425,6 +428,7 @@ def test_publisher_client_client_options_scopes( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -455,6 +459,7 @@ def test_publisher_client_client_options_credentials_file( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -472,6 +477,7 @@ def test_publisher_client_client_options_from_dict(): client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) diff --git a/tests/unit/gapic/pubsub_v1/test_schema_service.py b/tests/unit/gapic/pubsub_v1/test_schema_service.py index fcc8b68e1..91b58aff4 100644 --- a/tests/unit/gapic/pubsub_v1/test_schema_service.py +++ b/tests/unit/gapic/pubsub_v1/test_schema_service.py @@ -122,18 +122,6 @@ def test_schema_service_client_from_service_account_info(client_class): assert client.transport._host == "pubsub.googleapis.com:443" -@pytest.mark.parametrize( - "client_class", [SchemaServiceClient, SchemaServiceAsyncClient,] -) -def test_schema_service_client_service_account_always_use_jwt(client_class): - with mock.patch.object( - service_account.Credentials, "with_always_use_jwt_access", create=True - ) as use_jwt: - creds = service_account.Credentials(None, None, None) - client = client_class(credentials=creds) - use_jwt.assert_not_called() - - @pytest.mark.parametrize( "transport_class,transport_name", [ @@ -141,7 +129,7 @@ def test_schema_service_client_service_account_always_use_jwt(client_class): (transports.SchemaServiceGrpcAsyncIOTransport, "grpc_asyncio"), ], ) -def test_schema_service_client_service_account_always_use_jwt_true( +def test_schema_service_client_service_account_always_use_jwt( transport_class, transport_name ): with mock.patch.object( @@ -151,6 +139,13 @@ def test_schema_service_client_service_account_always_use_jwt_true( transport = transport_class(credentials=creds, always_use_jwt_access=True) use_jwt.assert_called_once_with(True) + with mock.patch.object( + service_account.Credentials, "with_always_use_jwt_access", create=True + ) as use_jwt: + creds = service_account.Credentials(None, None, None) + transport = transport_class(credentials=creds, always_use_jwt_access=False) + use_jwt.assert_not_called() + @pytest.mark.parametrize( "client_class", [SchemaServiceClient, SchemaServiceAsyncClient,] @@ -231,6 +226,7 @@ def test_schema_service_client_client_options( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT is @@ -247,6 +243,7 @@ def test_schema_service_client_client_options( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT is @@ -263,6 +260,7 @@ def test_schema_service_client_client_options( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT has @@ -291,6 +289,7 @@ def test_schema_service_client_client_options( client_cert_source_for_mtls=None, quota_project_id="octopus", client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -357,6 +356,7 @@ def test_schema_service_client_mtls_env_auto( client_cert_source_for_mtls=expected_client_cert_source, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case ADC client cert is provided. Whether client cert is used depends on @@ -390,6 +390,7 @@ def test_schema_service_client_mtls_env_auto( client_cert_source_for_mtls=expected_client_cert_source, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case client_cert_source and ADC client cert are not provided. @@ -411,6 +412,7 @@ def test_schema_service_client_mtls_env_auto( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -441,6 +443,7 @@ def test_schema_service_client_client_options_scopes( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -471,6 +474,7 @@ def test_schema_service_client_client_options_credentials_file( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -490,6 +494,7 @@ def test_schema_service_client_client_options_from_dict(): client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) diff --git a/tests/unit/gapic/pubsub_v1/test_subscriber.py b/tests/unit/gapic/pubsub_v1/test_subscriber.py index 67e6b26cc..8242d636e 100644 --- a/tests/unit/gapic/pubsub_v1/test_subscriber.py +++ b/tests/unit/gapic/pubsub_v1/test_subscriber.py @@ -118,16 +118,6 @@ def test_subscriber_client_from_service_account_info(client_class): assert client.transport._host == "pubsub.googleapis.com:443" -@pytest.mark.parametrize("client_class", [SubscriberClient, SubscriberAsyncClient,]) -def test_subscriber_client_service_account_always_use_jwt(client_class): - with mock.patch.object( - service_account.Credentials, "with_always_use_jwt_access", create=True - ) as use_jwt: - creds = service_account.Credentials(None, None, None) - client = client_class(credentials=creds) - use_jwt.assert_not_called() - - @pytest.mark.parametrize( "transport_class,transport_name", [ @@ -135,7 +125,7 @@ def test_subscriber_client_service_account_always_use_jwt(client_class): (transports.SubscriberGrpcAsyncIOTransport, "grpc_asyncio"), ], ) -def test_subscriber_client_service_account_always_use_jwt_true( +def test_subscriber_client_service_account_always_use_jwt( transport_class, transport_name ): with mock.patch.object( @@ -145,6 +135,13 @@ def test_subscriber_client_service_account_always_use_jwt_true( transport = transport_class(credentials=creds, always_use_jwt_access=True) use_jwt.assert_called_once_with(True) + with mock.patch.object( + service_account.Credentials, "with_always_use_jwt_access", create=True + ) as use_jwt: + creds = service_account.Credentials(None, None, None) + transport = transport_class(credentials=creds, always_use_jwt_access=False) + use_jwt.assert_not_called() + @pytest.mark.parametrize("client_class", [SubscriberClient, SubscriberAsyncClient,]) def test_subscriber_client_from_service_account_file(client_class): @@ -221,6 +218,7 @@ def test_subscriber_client_client_options( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT is @@ -237,6 +235,7 @@ def test_subscriber_client_client_options( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT is @@ -253,6 +252,7 @@ def test_subscriber_client_client_options( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case api_endpoint is not provided and GOOGLE_API_USE_MTLS_ENDPOINT has @@ -281,6 +281,7 @@ def test_subscriber_client_client_options( client_cert_source_for_mtls=None, quota_project_id="octopus", client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -345,6 +346,7 @@ def test_subscriber_client_mtls_env_auto( client_cert_source_for_mtls=expected_client_cert_source, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case ADC client cert is provided. Whether client cert is used depends on @@ -378,6 +380,7 @@ def test_subscriber_client_mtls_env_auto( client_cert_source_for_mtls=expected_client_cert_source, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) # Check the case client_cert_source and ADC client cert are not provided. @@ -399,6 +402,7 @@ def test_subscriber_client_mtls_env_auto( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -429,6 +433,7 @@ def test_subscriber_client_client_options_scopes( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -459,6 +464,7 @@ def test_subscriber_client_client_options_credentials_file( client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, ) @@ -476,6 +482,7 @@ def test_subscriber_client_client_options_from_dict(): client_cert_source_for_mtls=None, quota_project_id=None, client_info=transports.base.DEFAULT_CLIENT_INFO, + always_use_jwt_access=True, )