feat: avoid authentication when use emulator #328

HemangChothani · 2020-11-28T14:46:47Z

Fixes #324

tseaver · 2020-11-30T18:26:42Z

google/cloud/storage/client.py

+                project = os.getenv("GCLOUD_PROJECT")
+                if project is None:
+                    raise ValueError(
+                        "For emulator pass `project` as a argument or set `GCLOUD_PROJECT` via environment variable"


Suggested change

"For emulator pass `project` as a argument or set `GCLOUD_PROJECT` via environment variable"

"To use the GCS emulator, pass 'project' as an argument "

"or set GCLOUD_PROJECT environment variable"

tseaver · 2020-11-30T18:36:39Z

tests/unit/test_client.py

+    def test_ctor_w_emulator_w_env_var_glcoud_project(self):
+        from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR
+
+        HOST = "https://api.example.com"
+        PROJECT = "test"
+
+        with mock.patch("os.environ", {STORAGE_EMULATOR_ENV_VAR: HOST}):
+            with mock.patch("os.getenv", side_effect=[HOST, PROJECT]):
+                client = self._make_one()
+                self.assertEqual(client.project, PROJECT)
+                self.assertEqual(client._connection.API_BASE_URL, HOST)


This test needs an assertion that the client's credentials is an instance of AnonymousCredentials.

Also, rather than patching os.getenv, this would be clearer if we just patched in the expected environment:

Suggested change

def test_ctor_w_emulator_w_env_var_glcoud_project(self):

from google.cloud.storage._helpers import STORAGE_EMULATOR_ENV_VAR

HOST = "https://api.example.com"

PROJECT = "test"

with mock.patch("os.environ", {STORAGE_EMULATOR_ENV_VAR: HOST}):

with mock.patch("os.getenv", side_effect=[HOST, PROJECT]):

client = self._make_one()

self.assertEqual(client.project, PROJECT)

self.assertEqual(client._connection.API_BASE_URL, HOST)

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)

tseaver · 2020-11-30T18:45:13Z

tests/unit/test_client.py

+    def test_ctor_w_emulator_w_project_argument(self):
+        HOST = "https://api.example.com"
+        PROJECT = "test"
+
+        with mock.patch("os.getenv", side_effect=[HOST]):
+            client = self._make_one(project=PROJECT)
+            self.assertEqual(client.project, PROJECT)
+
+    def test_ctor_w_emulator_missing_project(self):
+        HOST = "https://api.example.com"
+        with mock.patch("os.getenv", side_effect=[HOST, None]):
+            with self.assertRaises(ValueError):
+                self._make_one()


We need to verify that the passed-in project overrides the one in the environment. Likewise for these tests, just patch os.environ:

Suggested change

def test_ctor_w_emulator_w_project_argument(self):

HOST = "https://api.example.com"

PROJECT = "test"

with mock.patch("os.getenv", side_effect=[HOST]):

client = self._make_one(project=PROJECT)

self.assertEqual(client.project, PROJECT)

def test_ctor_w_emulator_missing_project(self):

HOST = "https://api.example.com"

with mock.patch("os.getenv", side_effect=[HOST, None]):

with self.assertRaises(ValueError):

self._make_one()

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()

frankyn · 2020-11-30T18:50:25Z

google/cloud/storage/client.py

@@ -112,6 +114,15 @@ def __init__(
        if project is _marker:
            project = None

+        if os.getenv(STORAGE_EMULATOR_ENV_VAR) is not None:
+            credentials = AnonymousCredentials()
+            if project in [_marker, None]:


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

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 .

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

frankyn · 2020-11-30T18:51:30Z

google/cloud/storage/client.py

@@ -112,6 +114,15 @@ def __init__(
        if project is _marker:
            project = None

+        if os.getenv(STORAGE_EMULATOR_ENV_VAR) is not None:


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

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?

…into storage_issue_324

cojenco · 2022-02-10T21:26:44Z

Replaced by #679

feat: avoid authentication when use emulator

b5ea0b2

HemangChothani requested review from tseaver and frankyn November 28, 2020 14:46

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 28, 2020

product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Nov 28, 2020

tseaver suggested changes Nov 30, 2020

View reviewed changes

frankyn requested changes Nov 30, 2020

View reviewed changes

HemangChothani added 2 commits December 3, 2020 17:03

feat: changes in unit tests to mock environ

e436dad

Merge branch 'master' of https://github.com/googleapis/python-storage …

6915bb4

…into storage_issue_324

HemangChothani requested a review from a team December 3, 2020 11:34

Merge branch 'master' of https://github.com/googleapis/python-storage …

2390499

…into storage_issue_324

cojenco closed this Feb 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: avoid authentication when use emulator #328

feat: avoid authentication when use emulator #328

HemangChothani commented Nov 28, 2020

tseaver Nov 30, 2020

tseaver Nov 30, 2020

tseaver Nov 30, 2020

frankyn Nov 30, 2020

HemangChothani Dec 3, 2020

HemangChothani Dec 14, 2020

frankyn Nov 30, 2020

HemangChothani Dec 3, 2020

cojenco commented Feb 10, 2022

	"For emulator pass `project` as a argument or set `GCLOUD_PROJECT` via environment variable"
	"To use the GCS emulator, pass 'project' as an argument "
	"or set GCLOUD_PROJECT environment variable"

feat: avoid authentication when use emulator #328

feat: avoid authentication when use emulator #328

Conversation

HemangChothani commented Nov 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cojenco commented Feb 10, 2022