From 02b7cb2cdfd76756942c95491ec2533fb088e7da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Alberto=20Hern=C3=A1ndez?= Date: Thu, 23 Mar 2023 19:17:44 +0000 Subject: [PATCH 1/2] Fix invalid value for property api_endpoint_overrides/storage --- gslib/tests/test_shim_util.py | 28 ++++++++++++++++++++++------ gslib/utils/shim_util.py | 4 ++-- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/gslib/tests/test_shim_util.py b/gslib/tests/test_shim_util.py index 39f586019f..fd82059fb6 100644 --- a/gslib/tests/test_shim_util.py +++ b/gslib/tests/test_shim_util.py @@ -1082,7 +1082,7 @@ def test_gcs_json_endpoint_translation(self): self.assertEqual( env_vars, { 'CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE': - 'https://foo_host:1234/storage/v2', + 'https://foo_host:1234/storage/v2/', }) def test_gcs_json_endpoint_translation_with_missing_port(self): @@ -1094,10 +1094,11 @@ def test_gcs_json_endpoint_translation_with_missing_port(self): }): flags, env_vars = self._fake_command._translate_boto_config() self.assertEqual(flags, []) - self.assertEqual(env_vars, { - 'CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE': - 'https://foo_host/storage/v2', - }) + self.assertEqual( + env_vars, { + 'CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE': + 'https://foo_host/storage/v2/', + }) def test_gcs_json_endpoint_translation_usees_default_version_v1(self): with _mock_boto_config( @@ -1110,9 +1111,24 @@ def test_gcs_json_endpoint_translation_usees_default_version_v1(self): self.assertEqual( env_vars, { 'CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE': - 'https://foo_host:1234/storage/v1' + 'https://foo_host:1234/storage/v1/' }) + def test_gcs_json_endpoint_translation_should_start_with_https(self): + with _mock_boto_config({'Credentials': {'gs_json_host': 'foo_host',}}): + flags, env_vars = self._fake_command._translate_boto_config() + self.assertEqual(flags, []) + self.assertTrue( + env_vars['CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE'].startswith( + 'https://')) + + def test_gcs_json_endpoint_translation_should_end_with_trailing_slash(self): + with _mock_boto_config({'Credentials': {'gs_json_host': 'foo_host',}}): + flags, env_vars = self._fake_command._translate_boto_config() + self.assertEqual(flags, []) + self.assertTrue( + env_vars['CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE'].endswith('v1/')) + def test_s3_endpoint_translation(self): with _mock_boto_config( {'Credentials': { diff --git a/gslib/utils/shim_util.py b/gslib/utils/shim_util.py index 41745e49ef..3506235ace 100644 --- a/gslib/utils/shim_util.py +++ b/gslib/utils/shim_util.py @@ -248,8 +248,8 @@ def _get_gcs_json_endpoint_from_boto_config(config): gs_json_port = config.get('Credentials', 'gs_json_port') port = ':' + gs_json_port if gs_json_port else '' json_api_version = config.get('Credentials', 'json_api_version', 'v1') - return 'https://{}{}/storage/{}'.format(gs_json_host, port, - json_api_version) + return 'https://{}{}/storage/{}/'.format(gs_json_host, port, + json_api_version) return None From e32aabe411230f208e51aada2efd2e75cb845654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Alberto=20Hern=C3=A1ndez?= Date: Fri, 24 Mar 2023 17:45:11 +0000 Subject: [PATCH 2/2] Add coments and remove redudant tests --- gslib/tests/test_shim_util.py | 17 ++--------------- gslib/utils/shim_util.py | 2 ++ 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/gslib/tests/test_shim_util.py b/gslib/tests/test_shim_util.py index fd82059fb6..b5ca43f712 100644 --- a/gslib/tests/test_shim_util.py +++ b/gslib/tests/test_shim_util.py @@ -1108,27 +1108,14 @@ def test_gcs_json_endpoint_translation_usees_default_version_v1(self): }}): flags, env_vars = self._fake_command._translate_boto_config() self.assertEqual(flags, []) + # Implicitly testing that the translation must start with + # https:// and end with a trailing forward slash. self.assertEqual( env_vars, { 'CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE': 'https://foo_host:1234/storage/v1/' }) - def test_gcs_json_endpoint_translation_should_start_with_https(self): - with _mock_boto_config({'Credentials': {'gs_json_host': 'foo_host',}}): - flags, env_vars = self._fake_command._translate_boto_config() - self.assertEqual(flags, []) - self.assertTrue( - env_vars['CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE'].startswith( - 'https://')) - - def test_gcs_json_endpoint_translation_should_end_with_trailing_slash(self): - with _mock_boto_config({'Credentials': {'gs_json_host': 'foo_host',}}): - flags, env_vars = self._fake_command._translate_boto_config() - self.assertEqual(flags, []) - self.assertTrue( - env_vars['CLOUDSDK_API_ENDPOINT_OVERRIDES_STORAGE'].endswith('v1/')) - def test_s3_endpoint_translation(self): with _mock_boto_config( {'Credentials': { diff --git a/gslib/utils/shim_util.py b/gslib/utils/shim_util.py index 3506235ace..2bc2297bed 100644 --- a/gslib/utils/shim_util.py +++ b/gslib/utils/shim_util.py @@ -248,6 +248,8 @@ def _get_gcs_json_endpoint_from_boto_config(config): gs_json_port = config.get('Credentials', 'gs_json_port') port = ':' + gs_json_port if gs_json_port else '' json_api_version = config.get('Credentials', 'json_api_version', 'v1') + # Note that the json endpoint must start with https:// + # and end with a trailing forward slash. return 'https://{}{}/storage/{}/'.format(gs_json_host, port, json_api_version) return None