Skip to content

Commit

Permalink
feat(storage): support optionsRequestedPolicyVersion (#9989)
Browse files Browse the repository at this point in the history 
* iam proposal #3

maintain compatibility with defaultdict

remove in place

raise KeyError on delete

update deprecation for dict-key access and factory  methods

clean up

maintain compatibility - removing duplicate in __setitems__

check for conditions for dict access

remove empty binding

fix test accessing private var _bindings

fix(tests): change version to make existing tests pass

tests: add tests for getitem, delitem, setitem on v3 and conditions

test policy.bindings property

fixlint

black

sort bindings by role when converting to api repr

add deprecation warning for iam factory methods

update deprecation message for role methods

make Policy#bindings.members a set

update policy docs

fix docs

make docs better

fix: Bigtable policy class to use Policy.bindings

add from_pb with conditions test

add to_pb condition test

blacken

fix policy __delitem__

add docs on dict access

do not modify binding in to_apr_repr

* feat(storage): support requested_policy_version for get_iam_policy

* add system-test

* add ref doc sample to get_iam_policy

* add requested_policy_version to blob

* fix tests

* nit: typo

* blacken

* fix docs build

* format docs

* remove unused variables
  • Loading branch information
@jkwlui
jkwlui authored Jan 13, 2020
1 parent d897d56 commit 9b856cf
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 14 deletions.
8 changes: 4 additions & 4 deletions api_core/google/api_core/iam.py
View file
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
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,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 @@ -1473,6 +1473,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 @@ -1484,6 +1496,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
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.
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
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},
"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
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
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
OWNER1 = "user:phred@example.com"
OWNER2 = "group:cloud-logs@google.com"
EDITOR1 = "domain:google.com"
Expand Down

0 comments on commit 9b856cf

Please sign in to comment.