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

feat(storage): support optionsRequestedPolicyVersion #9989

Merged
merged 12 commits into from
Jan 13, 2020
Merged

feat(storage): support optionsRequestedPolicyVersion #9989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
583ca42
iam proposal #3
jkwlui Nov 20, 2019
7a9bc44
feat(storage): support requested_policy_version for get_iam_policy
jkwlui Dec 17, 2019
33898d7
add system-test
jkwlui Dec 17, 2019
2dd85e1
add ref doc sample to get_iam_policy
jkwlui Dec 17, 2019
4077045
add requested_policy_version to blob
jkwlui Dec 17, 2019
c210d29
fix tests
jkwlui Dec 17, 2019
cc717bd
nit: typo
jkwlui Dec 20, 2019
bc2f40a
Merge branch 'master' into storage-policy-version
jkwlui Jan 9, 2020
62b003a
blacken
jkwlui Jan 9, 2020
70a1886
fix docs build
jkwlui Jan 9, 2020
27c3e6a
format docs
jkwlui Jan 9, 2020
a17cbc1
remove unused variables
jkwlui Jan 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions api_core/google/api_core/iam.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,10 @@ def bindings(self):
policy.version = 3

policy.bindings = [
{
"role": "roles/viewer",
"members": {USER, ADMIN_GROUP, SERVICE_ACCOUNT},
"condition": CONDITION
{
"role": "roles/viewer",
"members": {USER, ADMIN_GROUP, SERVICE_ACCOUNT},
"condition": CONDITION
},
...
]
Expand Down
17 changes: 16 additions & 1 deletion storage/google/cloud/storage/blob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,7 @@ def create_resumable_upload_session(
except resumable_media.InvalidResponse as exc:
_raise_from_invalid_response(exc)

def get_iam_policy(self, client=None):
def get_iam_policy(self, client=None, requested_policy_version=None):
"""Retrieve the IAM policy for the object.

.. note:
Expand All @@ -1477,6 +1477,18 @@ def get_iam_policy(self, client=None):
:param client: Optional. The client to use. If not passed, falls back
to the ``client`` stored on the current object's bucket.

:type requested_policy_version: int or ``NoneType``
:param requested_policy_version: Optional. The version of IAM policies to request.
If a policy with a condition is requested without
setting this, the server will return an error.
This must be set to a value of 3 to retrieve IAM
policies containing conditions. This is to prevent
client code that isn't aware of IAM conditions from
interpreting and modifying policies incorrectly.
The service might return a policy with version lower
than the one that was requested, based on the
feature syntax in the policy fetched.

:rtype: :class:`google.api_core.iam.Policy`
:returns: the policy instance, based on the resource returned from
the ``getIamPolicy`` API request.
Expand All @@ -1488,6 +1500,9 @@ def get_iam_policy(self, client=None):
if self.user_project is not None:
query_params["userProject"] = self.user_project

if requested_policy_version is not None:
query_params["optionsRequestedPolicyVersion"] = requested_policy_version

info = client._connection.api_request(
method="GET",
path="%s/iam" % (self.path,),
Expand Down
41 changes: 40 additions & 1 deletion storage/google/cloud/storage/bucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@ def disable_website(self):
"""
return self.configure_website(None, None)

def get_iam_policy(self, client=None):
def get_iam_policy(self, client=None, requested_policy_version=None):
"""Retrieve the IAM policy for the bucket.

See
Expand All @@ -1878,16 +1878,55 @@ def get_iam_policy(self, client=None):
:param client: Optional. The client to use. If not passed, falls back
to the ``client`` stored on the current bucket.

:type requested_policy_version: int or ``NoneType``
:param requested_policy_version: Optional. The version of IAM policies to request.
If a policy with a condition is requested without
setting this, the server will return an error.
This must be set to a value of 3 to retrieve IAM
policies containing conditions. This is to prevent
client code that isn't aware of IAM conditions from
interpreting and modifying policies incorrectly.
jkwlui marked this conversation as resolved.
Show resolved Hide resolved
The service might return a policy with version lower
than the one that was requested, based on the
feature syntax in the policy fetched.

:rtype: :class:`google.api_core.iam.Policy`
:returns: the policy instance, based on the resource returned from
the ``getIamPolicy`` API request.

Example:

.. code-block:: python

from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE

policy = bucket.get_iam_policy(requested_policy_version=3)

policy.version = 3

# Add a binding to the policy via it's bindings property
policy.bindings.append({
"role": STORAGE_OBJECT_VIEWER_ROLE,
"members": {"serviceAccount:account@project.iam.gserviceaccount.com", ...},
# Optional:
"condition": {
"title": "prefix"
"description": "Objects matching prefix"
"expression": "resource.name.startsWith(\"projects/project-name/buckets/bucket-name/objects/prefix\")"
}
})

bucket.set_iam_policy(policy)
"""
client = self._require_client(client)
query_params = {}

if self.user_project is not None:
query_params["userProject"] = self.user_project

if requested_policy_version is not None:
query_params["optionsRequestedPolicyVersion"] = requested_policy_version

info = client._connection.api_request(
method="GET",
path="%s/iam" % (self.path,),
Expand Down
60 changes: 60 additions & 0 deletions storage/tests/system.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,66 @@ def test_bucket_update_labels(self):
bucket.update()
self.assertEqual(bucket.labels, {})

def test_get_set_iam_policy(self):
import pytest
from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE
from google.api_core.exceptions import BadRequest, PreconditionFailed

bucket_name = "iam-policy" + unique_resource_id("-")
bucket = retry_429_503(Config.CLIENT.create_bucket)(bucket_name)
self.case_buckets_to_delete.append(bucket_name)
self.assertTrue(bucket.exists())

policy_no_version = bucket.get_iam_policy()
self.assertEqual(policy_no_version.version, 1)

policy = bucket.get_iam_policy(requested_policy_version=3)
self.assertEqual(policy, policy_no_version)

member = "serviceAccount:{}".format(Config.CLIENT.get_service_account_email())

BINDING_W_CONDITION = {
"role": STORAGE_OBJECT_VIEWER_ROLE,
"members": {member},
jkwlui marked this conversation as resolved.
Show resolved Hide resolved
"condition": {
"title": "always-true",
"description": "test condition always-true",
"expression": "true",
},
}
policy.bindings.append(BINDING_W_CONDITION)

with pytest.raises(
PreconditionFailed, match="enable uniform bucket-level access"
):
bucket.set_iam_policy(policy)

bucket.iam_configuration.uniform_bucket_level_access_enabled = True
bucket.patch()

policy = bucket.get_iam_policy(requested_policy_version=3)
policy.bindings.append(BINDING_W_CONDITION)

with pytest.raises(BadRequest, match="at least 3"):
bucket.set_iam_policy(policy)

policy.version = 3
returned_policy = bucket.set_iam_policy(policy)
self.assertEqual(returned_policy.version, 3)
self.assertEqual(returned_policy.bindings, policy.bindings)

with pytest.raises(
BadRequest, match="cannot be less than the existing policy version"
):
bucket.get_iam_policy()
with pytest.raises(
BadRequest, match="cannot be less than the existing policy version"
):
bucket.get_iam_policy(requested_policy_version=2)

fetched_policy = bucket.get_iam_policy(requested_policy_version=3)
self.assertEqual(fetched_policy.bindings, returned_policy.bindings)

@unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.")
def test_crud_bucket_with_requester_pays(self):
new_bucket_name = "w-requester-pays" + unique_resource_id("-")
Expand Down
43 changes: 39 additions & 4 deletions storage/tests/unit/test_blob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1928,7 +1928,7 @@ def test_get_iam_policy(self):
BLOB_NAME = "blob-name"
PATH = "/b/name/o/%s" % (BLOB_NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
EDITOR1 = "domain:google.com"
Expand Down Expand Up @@ -1973,14 +1973,49 @@ def test_get_iam_policy(self):
},
)

def test_get_iam_policy_w_requested_policy_version(self):
from google.cloud.storage.iam import STORAGE_OWNER_ROLE

BLOB_NAME = "blob-name"
PATH = "/b/name/o/%s" % (BLOB_NAME,)
ETAG = "DEADBEEF"
VERSION = 1
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
RETURNED = {
"resourceId": PATH,
"etag": ETAG,
"version": VERSION,
"bindings": [{"role": STORAGE_OWNER_ROLE, "members": [OWNER1, OWNER2]}],
}
after = ({"status": http_client.OK}, RETURNED)
connection = _Connection(after)
client = _Client(connection)
bucket = _Bucket(client=client)
blob = self._make_one(BLOB_NAME, bucket=bucket)

blob.get_iam_policy(requested_policy_version=3)

kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(
kw[0],
{
"method": "GET",
"path": "%s/iam" % (PATH,),
"query_params": {"optionsRequestedPolicyVersion": 3},
"_target_object": None,
},
)

def test_get_iam_policy_w_user_project(self):
from google.api_core.iam import Policy

BLOB_NAME = "blob-name"
USER_PROJECT = "user-project-123"
PATH = "/b/name/o/%s" % (BLOB_NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
RETURNED = {
"resourceId": PATH,
"etag": ETAG,
Expand Down Expand Up @@ -2023,7 +2058,7 @@ def test_set_iam_policy(self):
BLOB_NAME = "blob-name"
PATH = "/b/name/o/%s" % (BLOB_NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
EDITOR1 = "domain:google.com"
Expand Down Expand Up @@ -2074,7 +2109,7 @@ def test_set_iam_policy_w_user_project(self):
USER_PROJECT = "user-project-123"
PATH = "/b/name/o/%s" % (BLOB_NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
BINDINGS = []
RETURNED = {"etag": ETAG, "version": VERSION, "bindings": BINDINGS}
after = ({"status": http_client.OK}, RETURNED)
Expand Down
37 changes: 33 additions & 4 deletions storage/tests/unit/test_bucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2023,7 +2023,7 @@ def test_get_iam_policy(self):
NAME = "name"
PATH = "/b/%s" % (NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
EDITOR1 = "domain:google.com"
Expand Down Expand Up @@ -2067,7 +2067,7 @@ def test_get_iam_policy_w_user_project(self):
USER_PROJECT = "user-project-123"
PATH = "/b/%s" % (NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
RETURNED = {
"resourceId": PATH,
"etag": ETAG,
Expand All @@ -2092,6 +2092,35 @@ def test_get_iam_policy_w_user_project(self):
self.assertEqual(kw[0]["path"], "%s/iam" % (PATH,))
self.assertEqual(kw[0]["query_params"], {"userProject": USER_PROJECT})

def test_get_iam_policy_w_requested_policy_version(self):
from google.cloud.storage.iam import STORAGE_OWNER_ROLE

NAME = "name"
PATH = "/b/%s" % (NAME,)
ETAG = "DEADBEEF"
VERSION = 1
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
RETURNED = {
"resourceId": PATH,
"etag": ETAG,
"version": VERSION,
"bindings": [{"role": STORAGE_OWNER_ROLE, "members": [OWNER1, OWNER2]}],
}
connection = _Connection(RETURNED)
client = _Client(connection, None)
bucket = self._make_one(client=client, name=NAME)

policy = bucket.get_iam_policy(requested_policy_version=3)

self.assertEqual(policy.version, VERSION)

kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(kw[0]["method"], "GET")
self.assertEqual(kw[0]["path"], "%s/iam" % (PATH,))
self.assertEqual(kw[0]["query_params"], {"optionsRequestedPolicyVersion": 3})

def test_set_iam_policy(self):
import operator
from google.cloud.storage.iam import STORAGE_OWNER_ROLE
Expand All @@ -2102,7 +2131,7 @@ def test_set_iam_policy(self):
NAME = "name"
PATH = "/b/%s" % (NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
EDITOR1 = "domain:google.com"
Expand Down Expand Up @@ -2155,7 +2184,7 @@ def test_set_iam_policy_w_user_project(self):
USER_PROJECT = "user-project-123"
PATH = "/b/%s" % (NAME,)
ETAG = "DEADBEEF"
VERSION = 17
VERSION = 1
jkwlui marked this conversation as resolved.
Show resolved Hide resolved
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
EDITOR1 = "domain:google.com"
Expand Down