From 09a409c6240a74dcb46d8f3f86d4fb95a52274a7 Mon Sep 17 00:00:00 2001 From: arithmetic1728 <58957152+arithmetic1728@users.noreply.github.com> Date: Wed, 21 Oct 2020 17:38:09 -0700 Subject: [PATCH] fix: fix mtls issue in handwritten layer (#226) * fix: fix mtls issue in handwritten layer * chore: update scripts * chore: update noxfile.py --- .kokoro/docs/common.cfg | 2 +- .kokoro/samples/python3.6/common.cfg | 6 +++++ .kokoro/samples/python3.7/common.cfg | 6 +++++ .kokoro/samples/python3.8/common.cfg | 6 +++++ .kokoro/test-samples.sh | 8 ++++++- docs/conf.py | 1 + google/cloud/pubsub_v1/publisher/client.py | 21 ++++++++++------- google/cloud/pubsub_v1/subscriber/client.py | 23 +++++++++++-------- samples/snippets/noxfile.py | 5 ++++ synth.metadata | 8 +++---- .../publisher/test_publisher_client.py | 12 ++++++++-- .../subscriber/test_subscriber_client.py | 12 ++++++++-- 12 files changed, 82 insertions(+), 28 deletions(-) diff --git a/.kokoro/docs/common.cfg b/.kokoro/docs/common.cfg index 7815c2d6a..b81c7b901 100644 --- a/.kokoro/docs/common.cfg +++ b/.kokoro/docs/common.cfg @@ -30,7 +30,7 @@ env_vars: { env_vars: { key: "V2_STAGING_BUCKET" - value: "docs-staging-v2-staging" + value: "docs-staging-v2" } # It will upload the docker image after successful builds. diff --git a/.kokoro/samples/python3.6/common.cfg b/.kokoro/samples/python3.6/common.cfg index 354ad19ef..6b9890422 100644 --- a/.kokoro/samples/python3.6/common.cfg +++ b/.kokoro/samples/python3.6/common.cfg @@ -13,6 +13,12 @@ env_vars: { value: "py-3.6" } +# Declare build specific Cloud project. +env_vars: { + key: "BUILD_SPECIFIC_GCLOUD_PROJECT" + value: "python-docs-samples-tests-py36" +} + env_vars: { key: "TRAMPOLINE_BUILD_FILE" value: "github/python-pubsub/.kokoro/test-samples.sh" diff --git a/.kokoro/samples/python3.7/common.cfg b/.kokoro/samples/python3.7/common.cfg index becd0399d..e2cb0f168 100644 --- a/.kokoro/samples/python3.7/common.cfg +++ b/.kokoro/samples/python3.7/common.cfg @@ -13,6 +13,12 @@ env_vars: { value: "py-3.7" } +# Declare build specific Cloud project. +env_vars: { + key: "BUILD_SPECIFIC_GCLOUD_PROJECT" + value: "python-docs-samples-tests-py37" +} + env_vars: { key: "TRAMPOLINE_BUILD_FILE" value: "github/python-pubsub/.kokoro/test-samples.sh" diff --git a/.kokoro/samples/python3.8/common.cfg b/.kokoro/samples/python3.8/common.cfg index 685dfdc59..cb7a71d5c 100644 --- a/.kokoro/samples/python3.8/common.cfg +++ b/.kokoro/samples/python3.8/common.cfg @@ -13,6 +13,12 @@ env_vars: { value: "py-3.8" } +# Declare build specific Cloud project. +env_vars: { + key: "BUILD_SPECIFIC_GCLOUD_PROJECT" + value: "python-docs-samples-tests-py38" +} + env_vars: { key: "TRAMPOLINE_BUILD_FILE" value: "github/python-pubsub/.kokoro/test-samples.sh" diff --git a/.kokoro/test-samples.sh b/.kokoro/test-samples.sh index 98851b56b..6064e7ad6 100755 --- a/.kokoro/test-samples.sh +++ b/.kokoro/test-samples.sh @@ -28,6 +28,12 @@ if [[ $KOKORO_BUILD_ARTIFACTS_SUBDIR = *"periodic"* ]]; then git checkout $LATEST_RELEASE fi +# Exit early if samples directory doesn't exist +if [ ! -d "./samples" ]; then + echo "No tests run. `./samples` not found" + exit 0 +fi + # Disable buffering, so that the logs stream through. export PYTHONUNBUFFERED=1 @@ -101,4 +107,4 @@ cd "$ROOT" # Workaround for Kokoro permissions issue: delete secrets rm testing/{test-env.sh,client-secrets.json,service-account.json} -exit "$RTN" \ No newline at end of file +exit "$RTN" diff --git a/docs/conf.py b/docs/conf.py index a785da8a8..48cf73642 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -39,6 +39,7 @@ "sphinx.ext.autosummary", "sphinx.ext.intersphinx", "sphinx.ext.coverage", + "sphinx.ext.doctest", "sphinx.ext.napoleon", "sphinx.ext.todo", "sphinx.ext.viewcode", diff --git a/google/cloud/pubsub_v1/publisher/client.py b/google/cloud/pubsub_v1/publisher/client.py index f1e198b1a..f1de9f1f4 100644 --- a/google/cloud/pubsub_v1/publisher/client.py +++ b/google/cloud/pubsub_v1/publisher/client.py @@ -130,15 +130,19 @@ def __init__(self, batch_settings=(), publisher_options=(), **kwargs): target=os.environ.get("PUBSUB_EMULATOR_HOST") ) + # The GAPIC client has mTLS logic to determine the api endpoint and the + # ssl credentials to use. Here we create a GAPIC client to help compute the + # api endpoint and ssl credentials. The api endpoint will be used to set + # `self._target`, and ssl credentials will be passed to + # `grpc_helpers.create_channel` to establish a mTLS channel (if ssl + # credentials is not None). client_options = kwargs.get("client_options", None) - if ( - client_options - and "api_endpoint" in client_options - and isinstance(client_options["api_endpoint"], six.string_types) - ): - self._target = client_options["api_endpoint"] - else: - self._target = publisher_client.PublisherClient.SERVICE_ADDRESS + credentials = kwargs.get("credentials", None) + client_for_mtls_info = publisher_client.PublisherClient( + credentials=credentials, client_options=client_options + ) + + self._target = client_for_mtls_info._transport._host # Use a custom channel. # We need this in order to set appropriate default message size and @@ -149,6 +153,7 @@ def __init__(self, batch_settings=(), publisher_options=(), **kwargs): channel = grpc_helpers.create_channel( credentials=kwargs.pop("credentials", None), target=self.target, + ssl_credentials=client_for_mtls_info._transport._ssl_channel_credentials, scopes=publisher_client.PublisherClient._DEFAULT_SCOPES, options={ "grpc.max_send_message_length": -1, diff --git a/google/cloud/pubsub_v1/subscriber/client.py b/google/cloud/pubsub_v1/subscriber/client.py index e0b10c888..e33a0e2e6 100644 --- a/google/cloud/pubsub_v1/subscriber/client.py +++ b/google/cloud/pubsub_v1/subscriber/client.py @@ -16,7 +16,6 @@ import os import pkg_resources -import six import grpc @@ -82,16 +81,19 @@ def __init__(self, **kwargs): target=os.environ.get("PUBSUB_EMULATOR_HOST") ) - # api_endpoint wont be applied if 'transport' is passed in. + # The GAPIC client has mTLS logic to determine the api endpoint and the + # ssl credentials to use. Here we create a GAPIC client to help compute the + # api endpoint and ssl credentials. The api endpoint will be used to set + # `self._target`, and ssl credentials will be passed to + # `grpc_helpers.create_channel` to establish a mTLS channel (if ssl + # credentials is not None). client_options = kwargs.get("client_options", None) - if ( - client_options - and "api_endpoint" in client_options - and isinstance(client_options["api_endpoint"], six.string_types) - ): - self._target = client_options["api_endpoint"] - else: - self._target = subscriber_client.SubscriberClient.SERVICE_ADDRESS + credentials = kwargs.get("credentials", None) + client_for_mtls_info = subscriber_client.SubscriberClient( + credentials=credentials, client_options=client_options + ) + + self._target = client_for_mtls_info._transport._host # Use a custom channel. # We need this in order to set appropriate default message size and @@ -102,6 +104,7 @@ def __init__(self, **kwargs): channel = grpc_helpers.create_channel( credentials=kwargs.pop("credentials", None), target=self.target, + ssl_credentials=client_for_mtls_info._transport._ssl_channel_credentials, scopes=subscriber_client.SubscriberClient._DEFAULT_SCOPES, options={ "grpc.max_send_message_length": -1, diff --git a/samples/snippets/noxfile.py b/samples/snippets/noxfile.py index 5660f08be..f3a90583a 100644 --- a/samples/snippets/noxfile.py +++ b/samples/snippets/noxfile.py @@ -199,6 +199,11 @@ def _get_repo_root(): break if Path(p / ".git").exists(): return str(p) + # .git is not available in repos cloned via Cloud Build + # setup.py is always in the library's root, so use that instead + # https://github.com/googleapis/synthtool/issues/792 + if Path(p / "setup.py").exists(): + return str(p) p = p.parent raise Exception("Unable to detect repository root.") diff --git a/synth.metadata b/synth.metadata index 148d44682..347608c98 100644 --- a/synth.metadata +++ b/synth.metadata @@ -4,28 +4,28 @@ "git": { "name": ".", "remote": "https://github.com/googleapis/python-pubsub.git", - "sha": "c957047c84c5586e4a782e9ae297094be6cdba2e" + "sha": "0bf5d593573afea43bba7de90d2bb40ee0fc101e" } }, { "git": { "name": "synthtool", "remote": "https://github.com/googleapis/synthtool.git", - "sha": "6abb59097be84599a1d6091fe534a49e5c5cf948" + "sha": "901ddd44e9ef7887ee681b9183bbdea99437fdcc" } }, { "git": { "name": "synthtool", "remote": "https://github.com/googleapis/synthtool.git", - "sha": "6abb59097be84599a1d6091fe534a49e5c5cf948" + "sha": "901ddd44e9ef7887ee681b9183bbdea99437fdcc" } }, { "git": { "name": "synthtool", "remote": "https://github.com/googleapis/synthtool.git", - "sha": "6abb59097be84599a1d6091fe534a49e5c5cf948" + "sha": "901ddd44e9ef7887ee681b9183bbdea99437fdcc" } } ], diff --git a/tests/unit/pubsub_v1/publisher/test_publisher_client.py b/tests/unit/pubsub_v1/publisher/test_publisher_client.py index 3b6aa1477..0f661c2fa 100644 --- a/tests/unit/pubsub_v1/publisher/test_publisher_client.py +++ b/tests/unit/pubsub_v1/publisher/test_publisher_client.py @@ -18,6 +18,7 @@ import inspect from google.auth import credentials +import grpc import mock import pytest @@ -81,7 +82,7 @@ def test_init_w_api_endpoint(): assert isinstance(client.api, publisher_client.PublisherClient) assert (client.api._transport.grpc_channel._channel.target()).decode( "utf-8" - ) == "testendpoint.google.com" + ) == "testendpoint.google.com:443" def test_init_w_unicode_api_endpoint(): @@ -91,7 +92,7 @@ def test_init_w_unicode_api_endpoint(): assert isinstance(client.api, publisher_client.PublisherClient) assert (client.api._transport.grpc_channel._channel.target()).decode( "utf-8" - ) == "testendpoint.google.com" + ) == "testendpoint.google.com:443" def test_init_w_empty_client_options(): @@ -104,8 +105,13 @@ def test_init_w_empty_client_options(): def test_init_client_options_pass_through(): + mock_ssl_creds = grpc.ssl_channel_credentials() + def init(self, *args, **kwargs): self.kwargs = kwargs + self._transport = mock.Mock() + self._transport._host = "testendpoint.google.com" + self._transport._ssl_channel_credentials = mock_ssl_creds with mock.patch.object(publisher_client.PublisherClient, "__init__", init): client = publisher.Client( @@ -119,6 +125,8 @@ def init(self, *args, **kwargs): assert client_options.get("quota_project_id") == "42" assert client_options.get("scopes") == [] assert client_options.get("credentials_file") == "file.json" + assert client.target == "testendpoint.google.com" + assert client.api.transport._ssl_channel_credentials == mock_ssl_creds def test_init_emulator(monkeypatch): diff --git a/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py b/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py index 634351757..d56289276 100644 --- a/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py +++ b/tests/unit/pubsub_v1/subscriber/test_subscriber_client.py @@ -13,6 +13,7 @@ # limitations under the License. from google.auth import credentials +import grpc import mock from google.cloud.pubsub_v1 import subscriber @@ -42,7 +43,7 @@ def test_init_w_api_endpoint(): assert isinstance(client.api, subscriber_client.SubscriberClient) assert (client.api._transport.grpc_channel._channel.target()).decode( "utf-8" - ) == "testendpoint.google.com" + ) == "testendpoint.google.com:443" def test_init_w_unicode_api_endpoint(): @@ -52,7 +53,7 @@ def test_init_w_unicode_api_endpoint(): assert isinstance(client.api, subscriber_client.SubscriberClient) assert (client.api._transport.grpc_channel._channel.target()).decode( "utf-8" - ) == "testendpoint.google.com" + ) == "testendpoint.google.com:443" def test_init_w_empty_client_options(): @@ -65,8 +66,13 @@ def test_init_w_empty_client_options(): def test_init_client_options_pass_through(): + mock_ssl_creds = grpc.ssl_channel_credentials() + def init(self, *args, **kwargs): self.kwargs = kwargs + self._transport = mock.Mock() + self._transport._host = "testendpoint.google.com" + self._transport._ssl_channel_credentials = mock_ssl_creds with mock.patch.object(subscriber_client.SubscriberClient, "__init__", init): client = subscriber.Client( @@ -80,6 +86,8 @@ def init(self, *args, **kwargs): assert client_options.get("quota_project_id") == "42" assert client_options.get("scopes") == [] assert client_options.get("credentials_file") == "file.json" + assert client.target == "testendpoint.google.com" + assert client.api.transport._ssl_channel_credentials == mock_ssl_creds def test_init_emulator(monkeypatch):