Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: avoid authentication when use emulator #328

Closed
wants to merge 4 commits into from
Closed

feat: avoid authentication when use emulator #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b5ea0b2
feat: avoid authentication when use emulator
HemangChothani Nov 28, 2020
e436dad
feat: changes in unit tests to mock environ
HemangChothani Dec 3, 2020
6915bb4
Merge branch 'master' of https://github.com/googleapis/python-storage…
HemangChothani Dec 3, 2020
2390499
Merge branch 'master' of https://github.com/googleapis/python-storage…
HemangChothani Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions google/cloud/storage/client.py
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@
import datetime
import functools
import json
import os
import warnings
import google.api_core.client_options

Expand All @@ -33,6 +34,7 @@
from google.cloud.exceptions import NotFound
from google.cloud.storage._helpers import _get_storage_host
from google.cloud.storage._helpers import _bucket_bound_hostname_url
from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR
from google.cloud.storage._http import Connection
from google.cloud.storage._signing import (
get_expiration_seconds_v4,
Expand Down Expand Up @@ -119,6 +121,16 @@ def __init__(
if project is _marker:
project = None

if os.getenv(STORAGE_EMULATOR_ENV_VAR) is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to infer this from endpoint value in client_options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry didn't get the point, few questions regarding that

  • what if client_options didn't pass

  • which value use to compare with kw_args["api_endpoint"]? _DEFAULT_STORAGE_HOST which is https://storage.googleapis.com?

  • Right now first we call the super class of client with credentials and then iterate the client_options and _get_storage_host() for an api_endpoint, should we need to change that order?

credentials = AnonymousCredentials()
if project in [_marker, None]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is project validation necessary given the super class will fall back to the expected environment variables?

https://googleapis.dev/python/google-cloud-core/latest/client.html#google.cloud.client.ClientWithProject

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when we are passing credentials as a keyword argument and if not passing project then it found the project in fall back but then throw an error about Default credential from here https://github.com/googleapis/google-auth-library-python/blob/bc92abbce5e0a5e7b5de8157e7772db84349a6cb/google/auth/_default.py#L356 when called a method google.auth.default() method because method unable to find credential from the given four options .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i can remove this condition after merge of cloud-core-51 PR

project = os.getenv("GCLOUD_PROJECT")
if project is None:
raise ValueError(
"To use the GCS emulator, pass 'project' as an argument "
"or set GCLOUD_PROJECT environment variable"
)

super(Client, self).__init__(
project=project,
credentials=credentials,
Expand Down
47 changes: 47 additions & 0 deletions tests/unit/test_client.py
View file
Open in desktop
Expand Up @@ -219,6 +219,53 @@ def test_ctor_w_client_info(self):
self.assertEqual(list(client._batch_stack), [])
self.assertIs(client._connection._client_info, client_info)

def test_ctor_w_emulator_w_env_var_glcoud_project(self):
from google.auth.credentials import AnonymousCredentials
from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR

HOST = "https://api.example.com"
PROJECT = "test"
environ = {
STORAGE_EMULATOR_ENV_VAR: HOST,
"GCLOUD_PROJECT": PROJECT,
}

with mock.patch("os.environ", environ):
client = self._make_one()

self.assertEqual(client.project, PROJECT)
self.assertEqual(client._connection.API_BASE_URL, HOST)
self.assertIsInstance(client._credentials, AnonymousCredentials)

def test_ctor_w_emulator_w_project_argument(self):
from google.auth.credentials import AnonymousCredentials
from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR

HOST = "https://api.example.com"
PROJECT = "test"
ENVIRON_PROJECT = "environ-project"
environ = {
STORAGE_EMULATOR_ENV_VAR: HOST,
"GCLOUD_PROJECT": ENVIRON_PROJECT,
}

with mock.patch("os.environ", environ):
client = self._make_one(project=PROJECT)

self.assertEqual(client.project, PROJECT)
self.assertEqual(client._connection.API_BASE_URL, HOST)
self.assertIsInstance(client._credentials, AnonymousCredentials)

def test_ctor_w_emulator_missing_project(self):
from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR

HOST = "https://api.example.com"
environ = {STORAGE_EMULATOR_ENV_VAR: HOST}

with mock.patch("os.environ", environ):
with self.assertRaises(ValueError):
self._make_one()

def test_create_anonymous_client(self):
from google.auth.credentials import AnonymousCredentials
from google.cloud.storage._http import Connection
Expand Down