Skip to content

Commit

Permalink
Avoid that gcsfs requires storage.buckets.get permissions (#31)
Browse files Browse the repository at this point in the history
  • Loading branch information
mathiaseitz committed May 20, 2020
1 parent 331120e commit c7cb2f2
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 16 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -20,6 +20,13 @@ Possible types of changes are:
Unreleased
----------

1.3.0 - 20.05.2020
------------------

Changed
'''''''
- Removed the (non-required) call to ``get_bucket()`` from the constructor of ``GCSFS``. This avoids the need for storage.buckets.get permissions on that bucket and thus now allows users to simplify access management by using tight, predefined IAM roles.
This change has two implications: a) the constructor is now a bit faster as one less RPC is performed; b) the error message in case a bucket does not exist is slightly less informative.

1.2.0 - 01.04.2020
------------------
Expand Down
11 changes: 8 additions & 3 deletions README.rst
Expand Up @@ -142,21 +142,26 @@ This will create a virtualenv with all packages and dev-packages installed.

Tests
-----
All CI tests run against an actual GCS bucket provided by `Othoz <http://othoz.com/>`__. In order to run the tests against your own bucket,
All CI tests run against an actual GCS bucket provided by `Othoz <http://othoz.com/>`__.

In order to run the tests against your own bucket,
make sure to set up a `Service Account <https://cloud.google.com/iam/docs/service-accounts>`__ with all necessary permissions:

- storage.buckets.get
- storage.objects.get
- storage.objects.list
- storage.objects.create
- storage.objects.update
- storage.objects.delete

All five permissions listed above are e.g. included in the `predefined Cloud Storage IAM Role <https://cloud.google.com/storage/docs/access-control/iam-roles>`__ ``roles/storage.objectAdmin``.

Expose your bucket name as an environment variable ``$TEST_BUCKET`` and run the tests via::

$ pipenv run pytest

Note that the tests mostly wait for I/O, therefore it makes sense to highly parallelize them with `xdist <https://github.com/pytest-dev/pytest-xdist>`__.
Note that the tests mostly wait for I/O, therefore it makes sense to highly parallelize them with `xdist <https://github.com/pytest-dev/pytest-xdist>`__, e.g. by running the tests with::

$ pipenv run pytest -n 10


Credits
Expand Down
8 changes: 1 addition & 7 deletions fs_gcsfs/_gcsfs.py
Expand Up @@ -11,7 +11,6 @@
import google
from fs import ResourceType, errors, tools
from fs.base import FS
from fs.errors import CreateFailed
from fs.info import Info
from fs.mode import Mode
from fs.path import basename, dirname, forcedir, normpath, relpath, join
Expand Down Expand Up @@ -72,12 +71,7 @@ def __init__(self,
if self.client is None:
self.client = Client()

try:
self.bucket = self.client.get_bucket(self._bucket_name)
except google.api_core.exceptions.NotFound as err:
raise CreateFailed("The bucket \"{}\" does not seem to exist".format(self._bucket_name)) from err
except google.api_core.exceptions.Forbidden as err:
raise CreateFailed("You don't have access to the bucket \"{}\"".format(self._bucket_name)) from err
self.bucket = self.client.bucket(self._bucket_name)

if self._prefix != "":
if create:
Expand Down
9 changes: 3 additions & 6 deletions fs_gcsfs/tests/test_gcsfs.py
Expand Up @@ -38,8 +38,10 @@ def make_fs(self):
def client_mock():
class ClientMock:
"""A client mock class to instantiate GCSFS without making any requests in the constructor"""
def get_bucket(self, _):

def bucket(self, _):
pass

return ClientMock()


Expand Down Expand Up @@ -135,11 +137,6 @@ def test_fix_storage_does_not_overwrite_existing_directory_markers_with_custom_c
assert blob.download_as_string() == content


def test_instantiation_fails_if_no_access_to_bucket():
with pytest.raises(CreateFailed):
GCSFS(bucket_name=str(uuid.uuid4()))


def test_instantiation_with_create_false_fails_for_non_existing_root_path():
with pytest.raises(CreateFailed):
GCSFS(bucket_name=TEST_BUCKET, root_path=str(uuid.uuid4()), create=False)
Expand Down

0 comments on commit c7cb2f2

Please sign in to comment.