From ba448606aeab97d27487405fe65643af7eb8fb69 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 31 Oct 2019 11:46:16 -0400 Subject: [PATCH] feat(storage): add UBLA attrs to IAMConfiguration (#9475) Deprecate passing / setting BPO. Closes #8552. --- google/cloud/storage/bucket.py | 122 +++++++++++++++++++++++++++------ tests/system.py | 27 ++++---- tests/unit/test_bucket.py | 85 ++++++++++++++++++++--- 3 files changed, 188 insertions(+), 46 deletions(-) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 48ab09e23..2ae9aebfd 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -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 " @@ -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 @@ -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. diff --git a/tests/system.py b/tests/system.py index 65f4f976a..05b7e96e4 100644 --- a/tests/system.py +++ b/tests/system.py @@ -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) @@ -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 ) @@ -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() diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index de943339e..2646e0583 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -198,10 +198,12 @@ 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 @@ -209,13 +211,62 @@ def test_ctor_explicit(self): 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() @@ -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) @@ -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) @@ -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), } @@ -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) @@ -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), } @@ -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"