Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(storage): anonymous credentials for private bucket (#107)
* fix(storage): anonymous credentials for private bucket

* fix(storage): use API_BASE_URL for hostname

* fix(storage): nit

* fix(storage): fix system test

* fix(storage): revert changes

Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
  • Loading branch information
HemangChothani and frankyn committed Apr 25, 2020
1 parent 69b2c45 commit 6152ab4
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 25 deletions.
26 changes: 17 additions & 9 deletions google/cloud/storage/blob.py
Expand Up @@ -54,7 +54,6 @@
from google.cloud._helpers import _rfc3339_to_datetime
from google.cloud._helpers import _to_bytes
from google.cloud.exceptions import NotFound
from google.cloud.storage._helpers import _get_storage_host
from google.cloud.storage._helpers import _PropertyMixin
from google.cloud.storage._helpers import _scalar_property
from google.cloud.storage._helpers import _convert_to_timestamp
Expand All @@ -70,12 +69,11 @@
from google.cloud.storage.constants import REGIONAL_LEGACY_STORAGE_CLASS
from google.cloud.storage.constants import STANDARD_STORAGE_CLASS

_STORAGE_HOST = _get_storage_host()

_API_ACCESS_ENDPOINT = "https://storage.googleapis.com"
_DEFAULT_CONTENT_TYPE = u"application/octet-stream"
_DOWNLOAD_URL_TEMPLATE = _STORAGE_HOST + u"/download/storage/v1{path}?alt=media"
_BASE_UPLOAD_TEMPLATE = _STORAGE_HOST + u"/upload/storage/v1{bucket_path}/o?uploadType="
_DOWNLOAD_URL_TEMPLATE = u"{hostname}/download/storage/v1{path}?alt=media"
_BASE_UPLOAD_TEMPLATE = u"{hostname}/upload/storage/v1{bucket_path}/o?uploadType="
_MULTIPART_URL_TEMPLATE = _BASE_UPLOAD_TEMPLATE + u"multipart"
_RESUMABLE_URL_TEMPLATE = _BASE_UPLOAD_TEMPLATE + u"resumable"
# NOTE: "acl" is also writeable but we defer ACL management to
Expand Down Expand Up @@ -650,19 +648,24 @@ def _get_transport(self, client):
client = self._require_client(client)
return client._http

def _get_download_url(self):
def _get_download_url(self, client):
"""Get the download URL for the current blob.
If the ``media_link`` has been loaded, it will be used, otherwise
the URL will be constructed from the current blob's path (and possibly
generation) to avoid a round trip.
:type client: :class:`~google.cloud.storage.client.Client`
:param client: The client to use.
:rtype: str
:returns: The download URL for the current blob.
"""
name_value_pairs = []
if self.media_link is None:
base_url = _DOWNLOAD_URL_TEMPLATE.format(path=self.path)
base_url = _DOWNLOAD_URL_TEMPLATE.format(
hostname=client._connection.API_BASE_URL, path=self.path
)
if self.generation is not None:
name_value_pairs.append(("generation", "{:d}".format(self.generation)))
else:
Expand Down Expand Up @@ -790,7 +793,8 @@ def download_to_file(
:raises: :class:`google.cloud.exceptions.NotFound`
"""
download_url = self._get_download_url()
client = self._require_client(client)
download_url = self._get_download_url(client)
headers = _get_encryption_headers(self._encryption_key)
headers["accept-encoding"] = "gzip"

Expand Down Expand Up @@ -1055,7 +1059,9 @@ def _do_multipart_upload(
info = self._get_upload_arguments(content_type)
headers, object_metadata, content_type = info

base_url = _MULTIPART_URL_TEMPLATE.format(bucket_path=self.bucket.path)
base_url = _MULTIPART_URL_TEMPLATE.format(
hostname=self.client._connection.API_BASE_URL, bucket_path=self.bucket.path
)
name_value_pairs = []

if self.user_project is not None:
Expand Down Expand Up @@ -1190,7 +1196,9 @@ def _initiate_resumable_upload(
if extra_headers is not None:
headers.update(extra_headers)

base_url = _RESUMABLE_URL_TEMPLATE.format(bucket_path=self.bucket.path)
base_url = _RESUMABLE_URL_TEMPLATE.format(
hostname=self.client._connection.API_BASE_URL, bucket_path=self.bucket.path
)
name_value_pairs = []

if self.user_project is not None:
Expand Down
41 changes: 30 additions & 11 deletions tests/unit/test_blob.py
Expand Up @@ -751,7 +751,10 @@ def test__get_download_url_with_media_link(self):
# Set the media link on the blob
blob._properties["mediaLink"] = media_link

download_url = blob._get_download_url()
client = mock.Mock(_connection=_Connection)
client._connection.API_BASE_URL = "https://storage.googleapis.com"
download_url = blob._get_download_url(client)

self.assertEqual(download_url, media_link)

def test__get_download_url_with_media_link_w_user_project(self):
Expand All @@ -763,7 +766,10 @@ def test__get_download_url_with_media_link_w_user_project(self):
# Set the media link on the blob
blob._properties["mediaLink"] = media_link

download_url = blob._get_download_url()
client = mock.Mock(_connection=_Connection)
client._connection.API_BASE_URL = "https://storage.googleapis.com"
download_url = blob._get_download_url(client)

self.assertEqual(
download_url, "{}?userProject={}".format(media_link, user_project)
)
Expand All @@ -774,7 +780,9 @@ def test__get_download_url_on_the_fly(self):
blob = self._make_one(blob_name, bucket=bucket)

self.assertIsNone(blob.media_link)
download_url = blob._get_download_url()
client = mock.Mock(_connection=_Connection)
client._connection.API_BASE_URL = "https://storage.googleapis.com"
download_url = blob._get_download_url(client)
expected_url = (
"https://storage.googleapis.com/download/storage/v1/b/"
"buhkit/o/bzzz-fly.txt?alt=media"
Expand All @@ -790,7 +798,9 @@ def test__get_download_url_on_the_fly_with_generation(self):
blob._properties["generation"] = str(generation)

self.assertIsNone(blob.media_link)
download_url = blob._get_download_url()
client = mock.Mock(_connection=_Connection)
client._connection.API_BASE_URL = "https://storage.googleapis.com"
download_url = blob._get_download_url(client)
expected_url = (
"https://storage.googleapis.com/download/storage/v1/b/"
"fictional/o/pretend.txt?alt=media&generation=1493058489532987"
Expand All @@ -804,7 +814,9 @@ def test__get_download_url_on_the_fly_with_user_project(self):
blob = self._make_one(blob_name, bucket=bucket)

self.assertIsNone(blob.media_link)
download_url = blob._get_download_url()
client = mock.Mock(_connection=_Connection)
client._connection.API_BASE_URL = "https://storage.googleapis.com"
download_url = blob._get_download_url(client)
expected_url = (
"https://storage.googleapis.com/download/storage/v1/b/"
"fictional/o/pretend.txt?alt=media&userProject={}".format(user_project)
Expand All @@ -823,7 +835,9 @@ def test__get_download_url_on_the_fly_with_kms_key_name(self):
blob = self._make_one(blob_name, bucket=bucket, kms_key_name=kms_resource)

self.assertIsNone(blob.media_link)
download_url = blob._get_download_url()
client = mock.Mock(_connection=_Connection)
client._connection.API_BASE_URL = "https://storage.googleapis.com"
download_url = blob._get_download_url(client)
expected_url = (
"https://storage.googleapis.com/download/storage/v1/b/"
"buhkit/o/bzzz-fly.txt?alt=media"
Expand Down Expand Up @@ -1003,7 +1017,8 @@ def test_download_to_file_with_failure(self):

def test_download_to_file_wo_media_link(self):
blob_name = "blob-name"
client = mock.Mock(spec=[u"_http"])
client = mock.Mock(_connection=_Connection, spec=[u"_http"])
client._connection.API_BASE_URL = "https://storage.googleapis.com"
bucket = _Bucket(client)
blob = self._make_one(blob_name, bucket=bucket)
blob._do_download = mock.Mock()
Expand Down Expand Up @@ -1319,7 +1334,8 @@ def _do_multipart_success(
transport = self._mock_transport(http_client.OK, {})

# Create some mock arguments.
client = mock.Mock(_http=transport, spec=["_http"])
client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"])
client._connection.API_BASE_URL = "https://storage.googleapis.com"
data = b"data here hear hier"
stream = io.BytesIO(data)
content_type = u"application/xml"
Expand Down Expand Up @@ -1485,7 +1501,8 @@ def _initiate_resumable_helper(
transport = self._mock_transport(http_client.OK, response_headers)

# Create some mock arguments and call the method under test.
client = mock.Mock(_http=transport, spec=[u"_http"])
client = mock.Mock(_http=transport, _connection=_Connection, spec=[u"_http"])
client._connection.API_BASE_URL = "https://storage.googleapis.com"
data = b"hello hallo halo hi-low"
stream = io.BytesIO(data)
content_type = u"text/plain"
Expand Down Expand Up @@ -1772,7 +1789,8 @@ def _do_resumable_helper(
)

# Create some mock arguments and call the method under test.
client = mock.Mock(_http=transport, spec=["_http"])
client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"])
client._connection.API_BASE_URL = "https://storage.googleapis.com"
stream = io.BytesIO(data)
content_type = u"text/html"
response = blob._do_resumable_upload(
Expand Down Expand Up @@ -2114,7 +2132,8 @@ def _create_resumable_upload_session_helper(self, origin=None, side_effect=None)
# Create some mock arguments and call the method under test.
content_type = u"text/plain"
size = 10000
client = mock.Mock(_http=transport, spec=[u"_http"])
client = mock.Mock(_http=transport, _connection=_Connection, spec=[u"_http"])
client._connection.API_BASE_URL = "https://storage.googleapis.com"
new_url = blob.create_resumable_upload_session(
content_type=content_type, size=size, origin=origin, client=client
)
Expand Down
8 changes: 3 additions & 5 deletions tests/unit/test_client.py
Expand Up @@ -137,18 +137,16 @@ def test_ctor_w_empty_client_options(self):
)

def test_ctor_w_client_options_dict(self):

PROJECT = "PROJECT"
credentials = _make_credentials()
client_options = {"api_endpoint": "https://www.foo-googleapis.com"}
api_endpoint = "https://www.foo-googleapis.com"
client_options = {"api_endpoint": api_endpoint}

client = self._make_one(
project=PROJECT, credentials=credentials, client_options=client_options
)

self.assertEqual(
client._connection.API_BASE_URL, "https://www.foo-googleapis.com"
)
self.assertEqual(client._connection.API_BASE_URL, api_endpoint)

def test_ctor_w_client_options_object(self):
from google.api_core.client_options import ClientOptions
Expand Down

0 comments on commit 6152ab4

Please sign in to comment.