Skip to content

Commit 9b856cf

Browse files
jkwluijkwlui
authored
feat(storage): support optionsRequestedPolicyVersion (#9989)
* 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
1 parent d897d56 commit 9b856cf

File tree

6 files changed

+192
-14
lines changed

6 files changed

+192
-14
lines changed

‎api_core/google/api_core/iam.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,10 @@ def bindings(self):
210210
policy.version = 3
211211
212212
policy.bindings = [
213-
{
214-
"role": "roles/viewer",
215-
"members": {USER, ADMIN_GROUP, SERVICE_ACCOUNT},
216-
"condition": CONDITION
213+
{
214+
"role": "roles/viewer",
215+
"members": {USER, ADMIN_GROUP, SERVICE_ACCOUNT},
216+
"condition": CONDITION
217217
},
218218
...
219219
]

‎storage/google/cloud/storage/blob.py

+16-1
Original file line numberDiff line numberDiff line change
@@ -1454,7 +1454,7 @@ def create_resumable_upload_session(
14541454
except resumable_media.InvalidResponse as exc:
14551455
_raise_from_invalid_response(exc)
14561456

1457-
def get_iam_policy(self, client=None):
1457+
def get_iam_policy(self, client=None, requested_policy_version=None):
14581458
"""Retrieve the IAM policy for the object.
14591459
14601460
.. note:
@@ -1473,6 +1473,18 @@ def get_iam_policy(self, client=None):
14731473
:param client: Optional. The client to use. If not passed, falls back
14741474
to the ``client`` stored on the current object's bucket.
14751475
1476+
:type requested_policy_version: int or ``NoneType``
1477+
:param requested_policy_version: Optional. The version of IAM policies to request.
1478+
If a policy with a condition is requested without
1479+
setting this, the server will return an error.
1480+
This must be set to a value of 3 to retrieve IAM
1481+
policies containing conditions. This is to prevent
1482+
client code that isn't aware of IAM conditions from
1483+
interpreting and modifying policies incorrectly.
1484+
The service might return a policy with version lower
1485+
than the one that was requested, based on the
1486+
feature syntax in the policy fetched.
1487+
14761488
:rtype: :class:`google.api_core.iam.Policy`
14771489
:returns: the policy instance, based on the resource returned from
14781490
the ``getIamPolicy`` API request.
@@ -1484,6 +1496,9 @@ def get_iam_policy(self, client=None):
14841496
if self.user_project is not None:
14851497
query_params["userProject"] = self.user_project
14861498

1499+
if requested_policy_version is not None:
1500+
query_params["optionsRequestedPolicyVersion"] = requested_policy_version
1501+
14871502
info = client._connection.api_request(
14881503
method="GET",
14891504
path="%s/iam" % (self.path,),

‎storage/google/cloud/storage/bucket.py

+40-1
Original file line numberDiff line numberDiff line change
@@ -1865,7 +1865,7 @@ def disable_website(self):
18651865
"""
18661866
return self.configure_website(None, None)
18671867

1868-
def get_iam_policy(self, client=None):
1868+
def get_iam_policy(self, client=None, requested_policy_version=None):
18691869
"""Retrieve the IAM policy for the bucket.
18701870
18711871
See
@@ -1878,16 +1878,55 @@ def get_iam_policy(self, client=None):
18781878
:param client: Optional. The client to use. If not passed, falls back
18791879
to the ``client`` stored on the current bucket.
18801880
1881+
:type requested_policy_version: int or ``NoneType``
1882+
:param requested_policy_version: Optional. The version of IAM policies to request.
1883+
If a policy with a condition is requested without
1884+
setting this, the server will return an error.
1885+
This must be set to a value of 3 to retrieve IAM
1886+
policies containing conditions. This is to prevent
1887+
client code that isn't aware of IAM conditions from
1888+
interpreting and modifying policies incorrectly.
1889+
The service might return a policy with version lower
1890+
than the one that was requested, based on the
1891+
feature syntax in the policy fetched.
1892+
18811893
:rtype: :class:`google.api_core.iam.Policy`
18821894
:returns: the policy instance, based on the resource returned from
18831895
the ``getIamPolicy`` API request.
1896+
1897+
Example:
1898+
1899+
.. code-block:: python
1900+
1901+
from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE
1902+
1903+
policy = bucket.get_iam_policy(requested_policy_version=3)
1904+
1905+
policy.version = 3
1906+
1907+
# Add a binding to the policy via it's bindings property
1908+
policy.bindings.append({
1909+
"role": STORAGE_OBJECT_VIEWER_ROLE,
1910+
"members": {"serviceAccount:[email protected]", ...},
1911+
# Optional:
1912+
"condition": {
1913+
"title": "prefix"
1914+
"description": "Objects matching prefix"
1915+
"expression": "resource.name.startsWith(\"projects/project-name/buckets/bucket-name/objects/prefix\")"
1916+
}
1917+
})
1918+
1919+
bucket.set_iam_policy(policy)
18841920
"""
18851921
client = self._require_client(client)
18861922
query_params = {}
18871923

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

1927+
if requested_policy_version is not None:
1928+
query_params["optionsRequestedPolicyVersion"] = requested_policy_version
1929+
18911930
info = client._connection.api_request(
18921931
method="GET",
18931932
path="%s/iam" % (self.path,),

‎storage/tests/system.py

+60
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,66 @@ def test_bucket_update_labels(self):
262262
bucket.update()
263263
self.assertEqual(bucket.labels, {})
264264

265+
def test_get_set_iam_policy(self):
266+
import pytest
267+
from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE
268+
from google.api_core.exceptions import BadRequest, PreconditionFailed
269+
270+
bucket_name = "iam-policy" + unique_resource_id("-")
271+
bucket = retry_429_503(Config.CLIENT.create_bucket)(bucket_name)
272+
self.case_buckets_to_delete.append(bucket_name)
273+
self.assertTrue(bucket.exists())
274+
275+
policy_no_version = bucket.get_iam_policy()
276+
self.assertEqual(policy_no_version.version, 1)
277+
278+
policy = bucket.get_iam_policy(requested_policy_version=3)
279+
self.assertEqual(policy, policy_no_version)
280+
281+
member = "serviceAccount:{}".format(Config.CLIENT.get_service_account_email())
282+
283+
BINDING_W_CONDITION = {
284+
"role": STORAGE_OBJECT_VIEWER_ROLE,
285+
"members": {member},
286+
"condition": {
287+
"title": "always-true",
288+
"description": "test condition always-true",
289+
"expression": "true",
290+
},
291+
}
292+
policy.bindings.append(BINDING_W_CONDITION)
293+
294+
with pytest.raises(
295+
PreconditionFailed, match="enable uniform bucket-level access"
296+
):
297+
bucket.set_iam_policy(policy)
298+
299+
bucket.iam_configuration.uniform_bucket_level_access_enabled = True
300+
bucket.patch()
301+
302+
policy = bucket.get_iam_policy(requested_policy_version=3)
303+
policy.bindings.append(BINDING_W_CONDITION)
304+
305+
with pytest.raises(BadRequest, match="at least 3"):
306+
bucket.set_iam_policy(policy)
307+
308+
policy.version = 3
309+
returned_policy = bucket.set_iam_policy(policy)
310+
self.assertEqual(returned_policy.version, 3)
311+
self.assertEqual(returned_policy.bindings, policy.bindings)
312+
313+
with pytest.raises(
314+
BadRequest, match="cannot be less than the existing policy version"
315+
):
316+
bucket.get_iam_policy()
317+
with pytest.raises(
318+
BadRequest, match="cannot be less than the existing policy version"
319+
):
320+
bucket.get_iam_policy(requested_policy_version=2)
321+
322+
fetched_policy = bucket.get_iam_policy(requested_policy_version=3)
323+
self.assertEqual(fetched_policy.bindings, returned_policy.bindings)
324+
265325
@unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.")
266326
def test_crud_bucket_with_requester_pays(self):
267327
new_bucket_name = "w-requester-pays" + unique_resource_id("-")

‎storage/tests/unit/test_blob.py

+39-4
Original file line numberDiff line numberDiff line change
@@ -1928,7 +1928,7 @@ def test_get_iam_policy(self):
19281928
BLOB_NAME = "blob-name"
19291929
PATH = "/b/name/o/%s" % (BLOB_NAME,)
19301930
ETAG = "DEADBEEF"
1931-
VERSION = 17
1931+
VERSION = 1
19321932
OWNER1 = "user:[email protected]"
19331933
OWNER2 = "group:[email protected]"
19341934
EDITOR1 = "domain:google.com"
@@ -1973,14 +1973,49 @@ def test_get_iam_policy(self):
19731973
},
19741974
)
19751975

1976+
def test_get_iam_policy_w_requested_policy_version(self):
1977+
from google.cloud.storage.iam import STORAGE_OWNER_ROLE
1978+
1979+
BLOB_NAME = "blob-name"
1980+
PATH = "/b/name/o/%s" % (BLOB_NAME,)
1981+
ETAG = "DEADBEEF"
1982+
VERSION = 1
1983+
OWNER1 = "user:[email protected]"
1984+
OWNER2 = "group:[email protected]"
1985+
RETURNED = {
1986+
"resourceId": PATH,
1987+
"etag": ETAG,
1988+
"version": VERSION,
1989+
"bindings": [{"role": STORAGE_OWNER_ROLE, "members": [OWNER1, OWNER2]}],
1990+
}
1991+
after = ({"status": http_client.OK}, RETURNED)
1992+
connection = _Connection(after)
1993+
client = _Client(connection)
1994+
bucket = _Bucket(client=client)
1995+
blob = self._make_one(BLOB_NAME, bucket=bucket)
1996+
1997+
blob.get_iam_policy(requested_policy_version=3)
1998+
1999+
kw = connection._requested
2000+
self.assertEqual(len(kw), 1)
2001+
self.assertEqual(
2002+
kw[0],
2003+
{
2004+
"method": "GET",
2005+
"path": "%s/iam" % (PATH,),
2006+
"query_params": {"optionsRequestedPolicyVersion": 3},
2007+
"_target_object": None,
2008+
},
2009+
)
2010+
19762011
def test_get_iam_policy_w_user_project(self):
19772012
from google.api_core.iam import Policy
19782013

19792014
BLOB_NAME = "blob-name"
19802015
USER_PROJECT = "user-project-123"
19812016
PATH = "/b/name/o/%s" % (BLOB_NAME,)
19822017
ETAG = "DEADBEEF"
1983-
VERSION = 17
2018+
VERSION = 1
19842019
RETURNED = {
19852020
"resourceId": PATH,
19862021
"etag": ETAG,
@@ -2023,7 +2058,7 @@ def test_set_iam_policy(self):
20232058
BLOB_NAME = "blob-name"
20242059
PATH = "/b/name/o/%s" % (BLOB_NAME,)
20252060
ETAG = "DEADBEEF"
2026-
VERSION = 17
2061+
VERSION = 1
20272062
OWNER1 = "user:[email protected]"
20282063
OWNER2 = "group:[email protected]"
20292064
EDITOR1 = "domain:google.com"
@@ -2074,7 +2109,7 @@ def test_set_iam_policy_w_user_project(self):
20742109
USER_PROJECT = "user-project-123"
20752110
PATH = "/b/name/o/%s" % (BLOB_NAME,)
20762111
ETAG = "DEADBEEF"
2077-
VERSION = 17
2112+
VERSION = 1
20782113
BINDINGS = []
20792114
RETURNED = {"etag": ETAG, "version": VERSION, "bindings": BINDINGS}
20802115
after = ({"status": http_client.OK}, RETURNED)

‎storage/tests/unit/test_bucket.py

+33-4
Original file line numberDiff line numberDiff line change
@@ -2023,7 +2023,7 @@ def test_get_iam_policy(self):
20232023
NAME = "name"
20242024
PATH = "/b/%s" % (NAME,)
20252025
ETAG = "DEADBEEF"
2026-
VERSION = 17
2026+
VERSION = 1
20272027
OWNER1 = "user:[email protected]"
20282028
OWNER2 = "group:[email protected]"
20292029
EDITOR1 = "domain:google.com"
@@ -2067,7 +2067,7 @@ def test_get_iam_policy_w_user_project(self):
20672067
USER_PROJECT = "user-project-123"
20682068
PATH = "/b/%s" % (NAME,)
20692069
ETAG = "DEADBEEF"
2070-
VERSION = 17
2070+
VERSION = 1
20712071
RETURNED = {
20722072
"resourceId": PATH,
20732073
"etag": ETAG,
@@ -2092,6 +2092,35 @@ def test_get_iam_policy_w_user_project(self):
20922092
self.assertEqual(kw[0]["path"], "%s/iam" % (PATH,))
20932093
self.assertEqual(kw[0]["query_params"], {"userProject": USER_PROJECT})
20942094

2095+
def test_get_iam_policy_w_requested_policy_version(self):
2096+
from google.cloud.storage.iam import STORAGE_OWNER_ROLE
2097+
2098+
NAME = "name"
2099+
PATH = "/b/%s" % (NAME,)
2100+
ETAG = "DEADBEEF"
2101+
VERSION = 1
2102+
OWNER1 = "user:[email protected]"
2103+
OWNER2 = "group:[email protected]"
2104+
RETURNED = {
2105+
"resourceId": PATH,
2106+
"etag": ETAG,
2107+
"version": VERSION,
2108+
"bindings": [{"role": STORAGE_OWNER_ROLE, "members": [OWNER1, OWNER2]}],
2109+
}
2110+
connection = _Connection(RETURNED)
2111+
client = _Client(connection, None)
2112+
bucket = self._make_one(client=client, name=NAME)
2113+
2114+
policy = bucket.get_iam_policy(requested_policy_version=3)
2115+
2116+
self.assertEqual(policy.version, VERSION)
2117+
2118+
kw = connection._requested
2119+
self.assertEqual(len(kw), 1)
2120+
self.assertEqual(kw[0]["method"], "GET")
2121+
self.assertEqual(kw[0]["path"], "%s/iam" % (PATH,))
2122+
self.assertEqual(kw[0]["query_params"], {"optionsRequestedPolicyVersion": 3})
2123+
20952124
def test_set_iam_policy(self):
20962125
import operator
20972126
from google.cloud.storage.iam import STORAGE_OWNER_ROLE
@@ -2102,7 +2131,7 @@ def test_set_iam_policy(self):
21022131
NAME = "name"
21032132
PATH = "/b/%s" % (NAME,)
21042133
ETAG = "DEADBEEF"
2105-
VERSION = 17
2134+
VERSION = 1
21062135
OWNER1 = "user:[email protected]"
21072136
OWNER2 = "group:[email protected]"
21082137
EDITOR1 = "domain:google.com"
@@ -2155,7 +2184,7 @@ def test_set_iam_policy_w_user_project(self):
21552184
USER_PROJECT = "user-project-123"
21562185
PATH = "/b/%s" % (NAME,)
21572186
ETAG = "DEADBEEF"
2158-
VERSION = 17
2187+
VERSION = 1
21592188
OWNER1 = "user:[email protected]"
21602189
OWNER2 = "group:[email protected]"
21612190
EDITOR1 = "domain:google.com"

0 commit comments

Comments
 (0)