Skip to content

Commit

Permalink
feat(storage): add UBLA attrs to IAMConfiguration (#9475)
Browse files Browse the repository at this point in the history
Deprecate passing / setting BPO.

Closes #8552.
  • Loading branch information
tseaver authored Oct 31, 2019
1 parent cd91dc8 commit ba44860
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 46 deletions.
122 changes: 100 additions & 22 deletions google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@
from google.cloud.storage.notification import NONE_PAYLOAD_FORMAT


_UBLA_BPO_ENABLED_MESSAGE = (
"Pass only one of 'uniform_bucket_level_access_enabled' / "
"'bucket_policy_only_enabled' to 'IAMConfiguration'."
)
_BPO_ENABLED_MESSAGE = (
"'IAMConfiguration.bucket_policy_only_enabled' is deprecated. "
"Instead, use 'IAMConfiguration.uniform_bucket_level_access_enabled'."
)
_UBLA_BPO_LOCK_TIME_MESSAGE = (
"Pass only one of 'uniform_bucket_level_access_lock_time' / "
"'bucket_policy_only_lock_time' to 'IAMConfiguration'."
)
_BPO_LOCK_TIME_MESSAGE = (
"'IAMConfiguration.bucket_policy_only_lock_time' is deprecated. "
"Instead, use 'IAMConfiguration.uniform_bucket_level_access_lock_time'."
)
_LOCATION_SETTER_MESSAGE = (
"Assignment to 'Bucket.location' is deprecated, as it is only "
"valid before the bucket is created. Instead, pass the location "
Expand Down Expand Up @@ -286,29 +302,66 @@ def from_api_repr(cls, resource):
return instance


_default = object()


class IAMConfiguration(dict):
"""Map a bucket's IAM configuration.
:type bucket: :class:`Bucket`
:params bucket: Bucket for which this instance is the policy.
:type uniform_bucket_level_access_enabled: bool
:params bucket_policy_only_enabled:
(optional) whether the IAM-only policy is enabled for the bucket.
:type uniform_bucket_level_locked_time: :class:`datetime.datetime`
:params uniform_bucket_level_locked_time:
(optional) When the bucket's IAM-only policy was enabled.
This value should normally only be set by the back-end API.
:type bucket_policy_only_enabled: bool
:params bucket_policy_only_enabled: (optional) whether the IAM-only policy is enabled for the bucket.
:params bucket_policy_only_enabled:
Deprecated alias for :data:`uniform_bucket_level_access_enabled`.
:type bucket_policy_only_locked_time: :class:`datetime.datetime`
:params bucket_policy_only_locked_time: (optional) When the bucket's IAM-only policy was ehabled. This value should normally only be set by the back-end API.
:params bucket_policy_only_locked_time:
Deprecated alias for :data:`uniform_bucket_level_access_locked_time`.
"""

def __init__(
self,
bucket,
bucket_policy_only_enabled=False,
bucket_policy_only_locked_time=None,
uniform_bucket_level_access_enabled=_default,
uniform_bucket_level_access_locked_time=_default,
bucket_policy_only_enabled=_default,
bucket_policy_only_locked_time=_default,
):
data = {"bucketPolicyOnly": {"enabled": bucket_policy_only_enabled}}
if bucket_policy_only_locked_time is not None:
data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339(
bucket_policy_only_locked_time
if bucket_policy_only_enabled is not _default:

if uniform_bucket_level_access_enabled is not _default:
raise ValueError(_UBLA_BPO_ENABLED_MESSAGE)

warnings.warn(_BPO_ENABLED_MESSAGE, DeprecationWarning, stacklevel=2)
uniform_bucket_level_access_enabled = bucket_policy_only_enabled

if bucket_policy_only_locked_time is not _default:

if uniform_bucket_level_access_locked_time is not _default:
raise ValueError(_UBLA_BPO_LOCK_TIME_MESSAGE)

warnings.warn(_BPO_LOCK_TIME_MESSAGE, DeprecationWarning, stacklevel=2)
uniform_bucket_level_access_locked_time = bucket_policy_only_locked_time

if uniform_bucket_level_access_enabled is _default:
uniform_bucket_level_access_enabled = False

data = {
"uniformBucketLevelAccess": {"enabled": uniform_bucket_level_access_enabled}
}
if uniform_bucket_level_access_locked_time is not _default:
data["uniformBucketLevelAccess"]["lockedTime"] = _datetime_to_rfc3339(
uniform_bucket_level_access_locked_time
)
super(IAMConfiguration, self).__init__(data)
self._bucket = bucket
Expand Down Expand Up @@ -340,41 +393,66 @@ def bucket(self):
return self._bucket

@property
def bucket_policy_only_enabled(self):
def uniform_bucket_level_access_enabled(self):
"""If set, access checks only use bucket-level IAM policies or above.
:rtype: bool
:returns: whether the bucket is configured to allow only IAM.
"""
bpo = self.get("bucketPolicyOnly", {})
return bpo.get("enabled", False)
ubla = self.get("uniformBucketLevelAccess", {})
return ubla.get("enabled", False)

@bucket_policy_only_enabled.setter
def bucket_policy_only_enabled(self, value):
bpo = self.setdefault("bucketPolicyOnly", {})
bpo["enabled"] = bool(value)
@uniform_bucket_level_access_enabled.setter
def uniform_bucket_level_access_enabled(self, value):
ubla = self.setdefault("uniformBucketLevelAccess", {})
ubla["enabled"] = bool(value)
self.bucket._patch_property("iamConfiguration", self)

@property
def bucket_policy_only_locked_time(self):
"""Deadline for changing :attr:`bucket_policy_only_enabled` from true to false.
def uniform_bucket_level_access_locked_time(self):
"""Deadline for changing :attr:`uniform_bucket_level_access_enabled` from true to false.
If the bucket's :attr:`bucket_policy_only_enabled` is true, this property
If the bucket's :attr:`uniform_bucket_level_access_enabled` is true, this property
is time time after which that setting becomes immutable.
If the bucket's :attr:`bucket_policy_only_enabled` is false, this property
If the bucket's :attr:`uniform_bucket_level_access_enabled` is false, this property
is ``None``.
:rtype: Union[:class:`datetime.datetime`, None]
:returns: (readonly) Time after which :attr:`bucket_policy_only_enabled` will
:returns: (readonly) Time after which :attr:`uniform_bucket_level_access_enabled` will
be frozen as true.
"""
bpo = self.get("bucketPolicyOnly", {})
stamp = bpo.get("lockedTime")
ubla = self.get("uniformBucketLevelAccess", {})
stamp = ubla.get("lockedTime")
if stamp is not None:
stamp = _rfc3339_to_datetime(stamp)
return stamp

@property
def bucket_policy_only_enabled(self):
"""Deprecated alias for :attr:`uniform_bucket_level_access_enabled`.
:rtype: bool
:returns: whether the bucket is configured to allow only IAM.
"""
return self.uniform_bucket_level_access_enabled

@bucket_policy_only_enabled.setter
def bucket_policy_only_enabled(self, value):
warnings.warn(_BPO_ENABLED_MESSAGE, DeprecationWarning, stacklevel=2)
self.uniform_bucket_level_access_enabled = value

@property
def bucket_policy_only_locked_time(self):
"""Deprecated alias for :attr:`uniform_bucket_level_access_locked_time`.
:rtype: Union[:class:`datetime.datetime`, None]
:returns:
(readonly) Time after which :attr:`bucket_policy_only_enabled` will
be frozen as true.
"""
return self.uniform_bucket_level_access_locked_time


class Bucket(_PropertyMixin):
"""A class representing a Bucket on Cloud Storage.
Expand Down
27 changes: 13 additions & 14 deletions tests/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -1726,13 +1726,13 @@ def tearDown(self):
bucket = Config.CLIENT.bucket(bucket_name)
retry_429_harder(bucket.delete)(force=True)

def test_new_bucket_w_bpo(self):
new_bucket_name = "new-w-bpo" + unique_resource_id("-")
def test_new_bucket_w_ubla(self):
new_bucket_name = "new-w-ubla" + unique_resource_id("-")
self.assertRaises(
exceptions.NotFound, Config.CLIENT.get_bucket, new_bucket_name
)
bucket = Config.CLIENT.bucket(new_bucket_name)
bucket.iam_configuration.bucket_policy_only_enabled = True
bucket.iam_configuration.uniform_bucket_level_access_enabled = True
retry_429_503(bucket.create)()
self.case_buckets_to_delete.append(new_bucket_name)

Expand Down Expand Up @@ -1762,9 +1762,8 @@ def test_new_bucket_w_bpo(self):
with self.assertRaises(exceptions.BadRequest):
blob_acl.save()

@unittest.skipUnless(False, "Back-end fix for BPO/UBLA expected 2019-07-12")
def test_bpo_set_unset_preserves_acls(self):
new_bucket_name = "bpo-acls" + unique_resource_id("-")
def test_ubla_set_unset_preserves_acls(self):
new_bucket_name = "ubla-acls" + unique_resource_id("-")
self.assertRaises(
exceptions.NotFound, Config.CLIENT.get_bucket, new_bucket_name
)
Expand All @@ -1776,25 +1775,25 @@ def test_bpo_set_unset_preserves_acls(self):
payload = b"DEADBEEF"
blob.upload_from_string(payload)

# Preserve ACLs before setting BPO
# Preserve ACLs before setting UBLA
bucket_acl_before = list(bucket.acl)
blob_acl_before = list(bucket.acl)

# Set BPO
bucket.iam_configuration.bucket_policy_only_enabled = True
# Set UBLA
bucket.iam_configuration.uniform_bucket_level_access_enabled = True
bucket.patch()

self.assertTrue(bucket.iam_configuration.bucket_policy_only_enabled)
self.assertTrue(bucket.iam_configuration.uniform_bucket_level_access_enabled)

# While BPO is set, cannot get / set ACLs
# While UBLA is set, cannot get / set ACLs
with self.assertRaises(exceptions.BadRequest):
bucket.acl.reload()

# Clear BPO
bucket.iam_configuration.bucket_policy_only_enabled = False
# Clear UBLA
bucket.iam_configuration.uniform_bucket_level_access_enabled = False
bucket.patch()

# Query ACLs after clearing BPO
# Query ACLs after clearing UBLA
bucket.acl.reload()
bucket_acl_after = list(bucket.acl)
blob.acl.reload()
Expand Down
85 changes: 75 additions & 10 deletions tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,24 +198,75 @@ def test_ctor_defaults(self):
config = self._make_one(bucket)

self.assertIs(config.bucket, bucket)
self.assertFalse(config.uniform_bucket_level_access_enabled)
self.assertIsNone(config.uniform_bucket_level_access_locked_time)
self.assertFalse(config.bucket_policy_only_enabled)
self.assertIsNone(config.bucket_policy_only_locked_time)

def test_ctor_explicit(self):
def test_ctor_explicit_ubla(self):
import datetime
import pytz

bucket = self._make_bucket()
now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)

config = self._make_one(
bucket, bucket_policy_only_enabled=True, bucket_policy_only_locked_time=now
bucket,
uniform_bucket_level_access_enabled=True,
uniform_bucket_level_access_locked_time=now,
)

self.assertIs(config.bucket, bucket)
self.assertTrue(config.uniform_bucket_level_access_enabled)
self.assertEqual(config.uniform_bucket_level_access_locked_time, now)
self.assertTrue(config.bucket_policy_only_enabled)
self.assertEqual(config.bucket_policy_only_locked_time, now)

def test_ctor_explicit_bpo(self):
import datetime
import pytz

bucket = self._make_bucket()
now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)

config = pytest.deprecated_call(
self._make_one,
bucket,
bucket_policy_only_enabled=True,
bucket_policy_only_locked_time=now,
)

self.assertIs(config.bucket, bucket)
self.assertTrue(config.uniform_bucket_level_access_enabled)
self.assertEqual(config.uniform_bucket_level_access_locked_time, now)
self.assertTrue(config.bucket_policy_only_enabled)
self.assertEqual(config.bucket_policy_only_locked_time, now)

def test_ctor_ubla_and_bpo_enabled(self):
bucket = self._make_bucket()

with self.assertRaises(ValueError):
self._make_one(
bucket,
uniform_bucket_level_access_enabled=True,
bucket_policy_only_enabled=True,
)

def test_ctor_ubla_and_bpo_time(self):
import datetime
import pytz

bucket = self._make_bucket()
now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)

with self.assertRaises(ValueError):
self._make_one(
bucket,
uniform_bucket_level_access_enabled=True,
uniform_bucket_level_access_locked_time=now,
bucket_policy_only_locked_time=now,
)

def test_from_api_repr_w_empty_resource(self):
klass = self._get_target_class()
bucket = self._make_bucket()
Expand All @@ -230,7 +281,7 @@ def test_from_api_repr_w_empty_resource(self):
def test_from_api_repr_w_empty_bpo(self):
klass = self._get_target_class()
bucket = self._make_bucket()
resource = {"bucketPolicyOnly": {}}
resource = {"uniformBucketLevelAccess": {}}

config = klass.from_api_repr(resource, bucket)

Expand All @@ -241,7 +292,7 @@ def test_from_api_repr_w_empty_bpo(self):
def test_from_api_repr_w_disabled(self):
klass = self._get_target_class()
bucket = self._make_bucket()
resource = {"bucketPolicyOnly": {"enabled": False}}
resource = {"uniformBucketLevelAccess": {"enabled": False}}

config = klass.from_api_repr(resource, bucket)

Expand All @@ -258,7 +309,7 @@ def test_from_api_repr_w_enabled(self):
bucket = self._make_bucket()
now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)
resource = {
"bucketPolicyOnly": {
"uniformBucketLevelAccess": {
"enabled": True,
"lockedTime": _datetime_to_rfc3339(now),
}
Expand All @@ -267,16 +318,30 @@ def test_from_api_repr_w_enabled(self):
config = klass.from_api_repr(resource, bucket)

self.assertIs(config.bucket, bucket)
self.assertTrue(config.uniform_bucket_level_access_enabled)
self.assertEqual(config.uniform_bucket_level_access_locked_time, now)
self.assertTrue(config.bucket_policy_only_enabled)
self.assertEqual(config.bucket_policy_only_locked_time, now)

def test_uniform_bucket_level_access_enabled_setter(self):
bucket = self._make_bucket()
config = self._make_one(bucket)

config.uniform_bucket_level_access_enabled = True
self.assertTrue(config.bucket_policy_only_enabled)

self.assertTrue(config["uniformBucketLevelAccess"]["enabled"])
bucket._patch_property.assert_called_once_with("iamConfiguration", config)

def test_bucket_policy_only_enabled_setter(self):
bucket = self._make_bucket()
config = self._make_one(bucket)

config.bucket_policy_only_enabled = True
with pytest.deprecated_call():
config.bucket_policy_only_enabled = True

self.assertTrue(config["bucketPolicyOnly"]["enabled"])
self.assertTrue(config.uniform_bucket_level_access_enabled)
self.assertTrue(config["uniformBucketLevelAccess"]["enabled"])
bucket._patch_property.assert_called_once_with("iamConfiguration", config)


Expand Down Expand Up @@ -1317,7 +1382,7 @@ def test_iam_configuration_policy_w_entry(self):
NAME = "name"
properties = {
"iamConfiguration": {
"bucketPolicyOnly": {
"uniformBucketLevelAccess": {
"enabled": True,
"lockedTime": _datetime_to_rfc3339(now),
}
Expand All @@ -1329,8 +1394,8 @@ def test_iam_configuration_policy_w_entry(self):

self.assertIsInstance(config, IAMConfiguration)
self.assertIs(config.bucket, bucket)
self.assertTrue(config.bucket_policy_only_enabled)
self.assertEqual(config.bucket_policy_only_locked_time, now)
self.assertTrue(config.uniform_bucket_level_access_enabled)
self.assertEqual(config.uniform_bucket_level_access_locked_time, now)

def test_lifecycle_rules_getter_unknown_action_type(self):
NAME = "name"
Expand Down

0 comments on commit ba44860

Please sign in to comment.