Storage: copy_blob doesn't respect preserve_acl

[HenrikOssipoff]

Thanks for stopping by to let us know something could be better!

Environment details

• macOS 10.15
• Python 3.6.5
• google-cloud-storage==1.20.0

Steps to reproduce

1. Upload a blob with a predefined ACL of publicRead
2. Copy this blob with preserve_acl set to true
3. Expected result is that the new blob is set to publicRead, but this ACL isn't actually preserved
4. Calling make_public() on the new blob correct sets it to publicRead

Code example

file_obj = BytesIO(...)
blob.upload_from_file(file_obj, content_type='...', predefined_acl='publicRead')

bucket = storage_client.get_bucket(...)
bucket.copy_blob(blob, destination_bucket=bucket, new_name='new-name.jpg', preserve_acl=True)
# At this point, the blob will not be set to public

[busunkim96]

@frankyn Is this the expected behavior for preserve_acl?

[frankyn]

Taking a look, thanks for the ping.

[frankyn]

Hi folks,

Based on the code, https://github.com/googleapis/google-cloud-python/blob/a79d98de3e9d6890c4e98d33cecae2f80550fdfd/storage/google/cloud/storage/bucket.py#L1121

I don't see anything necessarily copying ACLs. When exercising the example provided by @HenrikOssipoff, It's not something Cloud Storage does on a copy request.

We may want to deprecate this parameter.

[crwilcox]

I agree with @frankyn. Let's mark this deprecated.

[tseaver]

The objects.copy API actually takes an optional Object resource in the request body, and applies any metadata to the destination object. Maybe we need to use pass the source blob's properties (including ACL, which we normally don't pass)?

[frankyn]

I'm okay with not introducing support for copying ACL in this case. It reduces potential security risks.

However, I do see the potential for having a destination_blob with metadata parameter being passed in. That would help in this case.

[tseaver]

@frankyn We currently seem to promise that the ACL will be copied unless the user passes preserve_acl=False. Can you check whether the backend changed the "copy ACL" behavior, and if so, when?

Alternatives:

• Follow up on that promise by passing the source blob's ACL along in the body of the POST request.
• Deprecate the preserve_acl flag properly, preserving the current "erase ACL if False" behavior for a certain number of releases. We should also emit a warning if the user passes an explicit True value, since the backend doesn't copy (or no longer copies) the ACL.

For clarity, we could also add a Blob.copy_from method which passes along the destination blob's metadata / ACL (or at least any of the changed metadata?) in the body of the POST request. E.g.:

source_blob = bucket.get_blob("source-blob-name")
destination_blob = bucket.blob("destination-blob-name")
destination_blob.acl.all().grant_read()
destination_blob.storage_class = "COLDLINE"
destination_blob.copy_from(source_blob)  # passes ACL, storage class in body

[tseaver]

For context, see googleapis/google-cloud-python#5456.

[frankyn]

Apologies for the delay.

I confirmed with the GCS team that "ACL being copied" behavior was never supported by the copy API in GCS. This might have just been an accident.

I prefer we deprecate in this case. I'd prefer to have a user do a subsequent ACL update in this case or that we have a FR to introduce a parameter to supply metadata when using copy_blob().