fix(ObjectStore): Make copying behavior consistent with local storage #41565
Conversation
susnux
requested review from
a team,
Altahrim,
icewind1991 and
sorbaugh
and removed request for
a team
|
/backport to stable27 |
Drop file permissions on copy like we do on local storage. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
susnux
force-pushed
the
fix/object-storage-inconsitent-behavior
branch
from
6c30781 to
5172baa
Compare
| if (isset($params['validateWrites'])) { | ||
| $this->validateWrites = (bool)$params['validateWrites']; | ||
| } | ||
| $this->handleCopiesAsOwned = (bool)($params['handleCopiesAsOwned'] ?? false); |
There was a problem hiding this comment.
Do we need a parameter for that actually? If we keep it it should be documented
There was a problem hiding this comment.
From your inline comments it seems you think the local (non-object) policy is the broken one. I'm not sure local's behavior is incorrect (seems object is), but I appreciate the conservativeness to add a config option to avoid changing existing behavior for object (in case someone is relying on it).
But I think it should just outright be made consistent with local as-is w/o config option if at all possible. There's just no justification for having different behavior (or a switch for one but not the other/etc)
Do we have any idea when this inconsistent behavior was introduced or has it seemingly "always" been this way?
@joshtrichards Has always been like this, since native S3 copying respects the original file's permissions. One could argue that it does so for a reason because the permission had been the decision of the original owner of the file. At the same time, like you mentioned you could also argue the other way around that the copy belongs to the copying party and hence all permission-limitations are dropped. |
|
I think the new behavior is wrong, the permissions on object storages are not related to the permissions of the objects in the objectstore, but only the permissions from things like sharing/acls. See #53733 for making the local storage behave as the object store used to. |
Summary
Drop file permissions on copy like we do on local storage.
Checklist