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.

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

Storage: 'test_bpo_set_unset_preserves_acls' no longer sees expected 'BadRequest'. #8552

Closed
tseaver opened this issue Jul 1, 2019 · 10 comments · Fixed by #9475
Closed

Storage: 'test_bpo_set_unset_preserves_acls' no longer sees expected 'BadRequest'. #8552

tseaver opened this issue Jul 1, 2019 · 10 comments · Fixed by #9475
Assignees
Labels
api: storage Issues related to the Cloud Storage API. backend testing type: process A process-related concern. May include testing, release, or the like.

Comments

@tseaver
Copy link
Contributor

tseaver commented Jul 1, 2019

From this Kokoro job:

____________ TestIAMConfiguration.test_bpo_set_unset_preserves_acls ____________
self = <tests.system.TestIAMConfiguration testMethod=test_bpo_set_unset_preserves_acls>
    def test_bpo_set_unset_preserves_acls(self):
        new_bucket_name = "bpo-acls" + unique_resource_id("-")
        self.assertRaises(
            exceptions.NotFound, Config.CLIENT.get_bucket, new_bucket_name
        )
        bucket = retry_429(Config.CLIENT.create_bucket)(new_bucket_name)
        self.case_buckets_to_delete.append(new_bucket_name)
        blob_name = "my-blob.txt"
        blob = bucket.blob(blob_name)
        payload = b"DEADBEEF"
        blob.upload_from_string(payload)
        # Preserve ACLs before setting BPO
        bucket_acl_before = list(bucket.acl)
        blob_acl_before = list(bucket.acl)
        # Set BPO
        bucket.iam_configuration.bucket_policy_only_enabled = True
        bucket.patch()
        # While BPO is set, cannot get / set ACLs
        with self.assertRaises(exceptions.BadRequest):
>           bucket.acl.reload()
E           AssertionError: BadRequest not raised
tests/system.py:1709: AssertionError
@tseaver tseaver added api: storage Issues related to the Cloud Storage API. testing type: process A process-related concern. May include testing, release, or the like. flaky labels Jul 1, 2019
@tseaver
Copy link
Contributor Author

tseaver commented Jul 2, 2019

I'm not sure this is actually "flaky": the back-end appears to no longer return a BadRequest for bucketAccessControls.list when the bucket-policy-only flag is set.

@frankyn Can you confirm that the back-end behavior is changed?

@tseaver tseaver added backend and removed flaky labels Jul 2, 2019
@tseaver tseaver changed the title Storage: 'test_bpo_set_unset_preserves_acls' systest flakes (no error raised). Storage: 'test_bpo_set_unset_preserves_acls' no longer sees expected 'BadRequest'. Jul 2, 2019
@tseaver
Copy link
Contributor Author

tseaver commented Jul 8, 2019

Very strange: the call to Bucket.acl.reload does raise the expected BadRequest when running under Python 2.7, but does not raise as expected under Python 3.6.

@tseaver
Copy link
Contributor Author

tseaver commented Jul 8, 2019

Running the system test with Python 3.6, here is what I see from a breakpoint inside BucketACL.reload:

(Pdb) l
432  	        if self.user_project is not None:
433  	            query_params["userProject"] = self.user_project
434  	
435  	        self.entities.clear()
436  	
437  ->	        found = client._connection.api_request(
438  	            method="GET", path=path, query_params=query_params
439  	        )
440  	        self.loaded = True
441  	        for entry in found.get("items", ()):
442  	            self.add_entity(self.entity_from_dict(entry))
(Pdb) pp path
'/b/bpo-acls-1562617375855/acl'
(Pdb) pp query_params
{}
(Pdb) n
> /home/tseaver/projects/agendaless/Google/src/gcp/storage/google/cloud/storage/acl.py(438)reload()
-> method="GET", path=path, query_params=query_params
(Pdb) 
> /home/tseaver/projects/agendaless/Google/src/gcp/storage/google/cloud/storage/acl.py(440)reload()
-> self.loaded = True
(Pdb) pp found
{'items': [{'bucket': 'bpo-acls-1562617375855',
            'entity': 'project-owners-455332057385',
            'etag': 'CAI=',
            'id': 'bpo-acls-1562617375855/project-owners-455332057385',
            'kind': 'storage#bucketAccessControl',
            'projectTeam': {'projectNumber': '455332057385', 'team': 'owners'},
            'role': 'OWNER',
            'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562617375855/acl/project-owners-455332057385'},
           {'bucket': 'bpo-acls-1562617375855',
            'entity': 'project-editors-455332057385',
            'etag': 'CAI=',
            'id': 'bpo-acls-1562617375855/project-editors-455332057385',
            'kind': 'storage#bucketAccessControl',
            'projectTeam': {'projectNumber': '455332057385', 'team': 'editors'},
            'role': 'OWNER',
            'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562617375855/acl/project-editors-455332057385'},
           {'bucket': 'bpo-acls-1562617375855',
            'entity': 'project-viewers-455332057385',
            'etag': 'CAI=',
            'id': 'bpo-acls-1562617375855/project-viewers-455332057385',
            'kind': 'storage#bucketAccessControl',
            'projectTeam': {'projectNumber': '455332057385', 'team': 'viewers'},
            'role': 'READER',
            'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562617375855/acl/project-viewers-455332057385'}],
 'kind': 'storage#bucketAccessControls'}
(Pdb) client._connection.api_request(method="GET", path=self.bucket.path)
{'kind': 'storage#bucket', 'id': 'bpo-acls-1562617375855', 'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562617375855', 'projectNumber': '455332057385', 'name': 'bpo-acls-1562617375855', 'timeCreated': '2019-07-08T20:22:56.637Z', 'updated': '2019-07-08T20:22:58.175Z', 'metageneration': '2', 'iamConfiguration': {'bucketPolicyOnly': {'enabled': False}, 'uniformBucketLevelAccess': {'enabled': False}}, 'location': 'US', 'locationType': 'multi-region', 'storageClass': 'STANDARD', 'etag': 'CAI='}

Note that the resource's iamConfiguration contains 'bucketPolicyOnly': {'enabled': False} (?!)

@tseaver
Copy link
Contributor Author

tseaver commented Jul 8, 2019

Under Python 2.7:

(Pdb) l
430  	        query_params = {}
431  	
432  	        if self.user_project is not None:
433  	            query_params["userProject"] = self.user_project
434  	
435  ->	        self.entities.clear()
436  	
437  	        found = client._connection.api_request(
438  	            method="GET", path=path, query_params=query_params
439  	        )
440  	        self.loaded = True
(Pdb) n
> /home/tseaver/projects/agendaless/Google/src/gcp/storage/google/cloud/storage/acl.py(437)reload()
-> found = client._connection.api_request(
(Pdb) pp path
'/b/bpo-acls-1562617750458/acl'
(Pdb) pp query_params
{}
(Pdb) n
> /home/tseaver/projects/agendaless/Google/src/gcp/storage/google/cloud/storage/acl.py(438)reload()
-> method="GET", path=path, query_params=query_params
(Pdb) 
BadRequest: BadReque...-only.',)
> /home/tseaver/projects/agendaless/Google/src/gcp/storage/google/cloud/storage/acl.py(438)reload()
-> method="GET", path=path, query_params=query_params
(Pdb) client._connection.api_request(method="GET", path=self.bucket.path)
{u'kind': u'storage#bucket', u'name': u'bpo-acls-1562617750458', u'timeCreated': u'2019-07-08T20:29:11.039Z', u'locationType': u'multi-region', u'updated': u'2019-07-08T20:29:12.575Z', u'projectNumber': u'455332057385', u'metageneration': u'2', u'location': u'US', u'iamConfiguration': {u'uniformBucketLevelAccess': {u'enabled': True, u'lockedTime': u'2019-10-06T20:29:12.539Z'}, u'bucketPolicyOnly': {u'enabled': True, u'lockedTime': u'2019-10-06T20:29:12.539Z'}}, u'etag': u'CAI=', u'id': u'bpo-acls-1562617750458', u'selfLink': u'https://www.googleapis.com/storage/v1/b/bpo-acls-1562617750458', u'storageClass': u'STANDARD'}

Here, the resource's iamConfiguration contains 'bucketPolicyOnly': {'enabled': True}, as expected.

@tseaver
Copy link
Contributor Author

tseaver commented Jul 8, 2019

Stepping into the calls to bucket.patch under Python 3.6:

(Pdb) l
177  	        # to work properly w/ 'noAcl'.
178  	        query_params["projection"] = "full"
179  	        update_properties = {key: self._properties[key] for key in self._changes}
180  	
181  	        # Make the API call.
182  ->	        api_response = client._connection.api_request(
183  	            method="PATCH",
184  	            path=self.path,
185  	            data=update_properties,
186  	            query_params=query_params,
187  	            _target_object=self,
(Pdb) pp self.path
'/b/bpo-acls-1562618298403'
(Pdb) pp update_properties
{'iamConfiguration': {'bucketPolicyOnly': {'enabled': True},
                      'uniformBucketLevelAccess': {'enabled': False}}}
...
(Pdb) pp api_response
{'acl': [{'bucket': 'bpo-acls-1562618298403',
          'entity': 'project-owners-455332057385',
          'etag': 'CAI=',
          'id': 'bpo-acls-1562618298403/project-owners-455332057385',
          'kind': 'storage#bucketAccessControl',
          'projectTeam': {'projectNumber': '455332057385', 'team': 'owners'},
          'role': 'OWNER',
          'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562618298403/acl/project-owners-455332057385'},
         {'bucket': 'bpo-acls-1562618298403',
          'entity': 'project-editors-455332057385',
          'etag': 'CAI=',
          'id': 'bpo-acls-1562618298403/project-editors-455332057385',
          'kind': 'storage#bucketAccessControl',
          'projectTeam': {'projectNumber': '455332057385', 'team': 'editors'},
          'role': 'OWNER',
          'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562618298403/acl/project-editors-455332057385'},
         {'bucket': 'bpo-acls-1562618298403',
          'entity': 'project-viewers-455332057385',
          'etag': 'CAI=',
          'id': 'bpo-acls-1562618298403/project-viewers-455332057385',
          'kind': 'storage#bucketAccessControl',
          'projectTeam': {'projectNumber': '455332057385', 'team': 'viewers'},
          'role': 'READER',
          'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562618298403/acl/project-viewers-455332057385'}],
 'defaultObjectAcl': [{'entity': 'project-owners-455332057385',
                       'etag': 'CAI=',
                       'kind': 'storage#objectAccessControl',
                       'projectTeam': {'projectNumber': '455332057385',
                                       'team': 'owners'},
                       'role': 'OWNER'},
                      {'entity': 'project-editors-455332057385',
                       'etag': 'CAI=',
                       'kind': 'storage#objectAccessControl',
                       'projectTeam': {'projectNumber': '455332057385',
                                       'team': 'editors'},
                       'role': 'OWNER'},
                      {'entity': 'project-viewers-455332057385',
                       'etag': 'CAI=',
                       'kind': 'storage#objectAccessControl',
                       'projectTeam': {'projectNumber': '455332057385',
                                       'team': 'viewers'},
                       'role': 'READER'}],
 'etag': 'CAI=',
 'iamConfiguration': {'bucketPolicyOnly': {'enabled': False},
                      'uniformBucketLevelAccess': {'enabled': False}},
 'id': 'bpo-acls-1562618298403',
 'kind': 'storage#bucket',
 'location': 'US',
 'locationType': 'multi-region',
 'metageneration': '2',
 'name': 'bpo-acls-1562618298403',
 'owner': {'entity': 'project-owners-455332057385'},
 'projectNumber': '455332057385',
 'selfLink': 'https://www.googleapis.com/storage/v1/b/bpo-acls-1562618298403',
 'storageClass': 'STANDARD',
 'timeCreated': '2019-07-08T20:38:19.035Z',
 'updated': '2019-07-08T20:39:40.402Z'}

and under Python 2.7:

(Pdb) l
177  	        # to work properly w/ 'noAcl'.
178  	        query_params["projection"] = "full"
179  	        update_properties = {key: self._properties[key] for key in self._changes}
180  	
181  	        # Make the API call.
182  ->	        api_response = client._connection.api_request(
183  	            method="PATCH",
184  	            path=self.path,
185  	            data=update_properties,
186  	            query_params=query_params,
187  	            _target_object=self,
(Pdb) pp self.path
'/b/bpo-acls-1562618105061'
(Pdb) pp update_properties
{'iamConfiguration': {'bucketPolicyOnly': {u'enabled': True},
                      u'uniformBucketLevelAccess': {u'enabled': False}}}
...
-> self._set_properties(api_response)
(Pdb) pp api_response
{u'etag': u'CAI=',
 u'iamConfiguration': {u'bucketPolicyOnly': {u'enabled': True,
                                             u'lockedTime': u'2019-10-06T20:37:19.650Z'},
                       u'uniformBucketLevelAccess': {u'enabled': True,
                                                     u'lockedTime': u'2019-10-06T20:37:19.650Z'}},
 u'id': u'bpo-acls-1562618105061',
 u'kind': u'storage#bucket',
 u'location': u'US',
 u'locationType': u'multi-region',
 u'metageneration': u'2',
 u'name': u'bpo-acls-1562618105061',
 u'projectNumber': u'455332057385',
 u'selfLink': u'https://www.googleapis.com/storage/v1/b/bpo-acls-1562618105061',
 u'storageClass': u'STANDARD',
 u'timeCreated': u'2019-07-08T20:35:05.747Z',
 u'updated': u'2019-07-08T20:37:19.687Z'}

Clearly, the PATCH for setting BPO is not working under Python 3.6, but is somehow working under Python 2.7.

@tseaver
Copy link
Contributor Author

tseaver commented Jul 8, 2019

@frankyn reports that a fix for the back-end change which causes this weird failure is due by 2019-07-12. In the meanwhile, PR #8617 will allow us to get CI back to green.

I'm leaving this issue open to track that rollout.

tseaver added a commit that referenced this issue Jul 8, 2019
Back-end fix for the issue expected 2019-07-12.

See #8552.
@frankyn
Copy link
Member

frankyn commented Jul 12, 2019

Issue is not yet fixed in production. Rollout was paused and will have an update next week.

@frankyn
Copy link
Member

frankyn commented Aug 12, 2019

@tseaver the fix for this might take a few more weeks at the best case.

The mitigation is to supply the same value set by BPO to UBLA in the client on a PATCH request.

PATCH uri
Body:

{
'iamConfiguration': 
  {
     'bucketPolicyOnly': {u'enabled': True},
     'uniformBucketLevelAccess': {u'enabled': True}
  }
}

@crwilcox
Copy link
Contributor

This test should no longer need to be skipped.

@crwilcox crwilcox added the help wanted We'd love to have community involvement on this issue. label Oct 18, 2019
@tseaver tseaver removed the help wanted We'd love to have community involvement on this issue. label Oct 22, 2019
@tseaver
Copy link
Contributor Author

tseaver commented Oct 22, 2019

PR #9475 will close this issue.

tseaver added a commit that referenced this issue Oct 31, 2019
Deprecate passing / setting BPO.

Closes #8552.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. backend testing type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants