storj / edge

Storj edge services (including multi-tenant, S3-compatible server to interact with the Storj network)
GNU Affero General Public License v3.0
53 stars 18 forks source link

CopyObject with retention does not work as expected #499

Closed halkyon closed 1 month ago

halkyon commented 2 months ago

Goal

CopyObject without specifying retention settings should not copy any settings from the source object. We seem to be copying across lock settings to the destination object which misleads the user into thinking locks were copied. However, the lock on the destination is not enforced, which is correct. I think this may be because object lock settings are stored in object metadata, and CopyObjectHandler in minio blindly copies over all the metadata to the destination. Quite possibly this line here is causing the trouble and isn't filtering out the metadata (https://github.com/storj/minio/blob/main/cmd/object-handlers.go#L1197):

srcInfo.UserDefined = objectlock.FilterObjectLockMetadata(srcInfo.UserDefined, true, true)

When you do specify retention settings on CopyObject, then it fails with an InvalidRequest: Bucket is missing ObjectLockConfiguration error, even if the bucket has locking enabled. Minio has code to check if locking is enabled on the bucket, but this only checks a global minio-specific flag for the bucket: https://github.com/storj/minio/blob/main/cmd/bucket-object-lock.go#L266. Do we still need all the validation here? One idea might be to remove checkPutObjectLockAllowed and not call it from CopyObjectHandler so our object layer does the validation instead.

Acceptance Criteria

storj-gerrit[bot] commented 2 months ago

Change pkg/bucket/object/lock: fix lock settings copied incorrectly mentions this issue.

storj-gerrit[bot] commented 2 months ago

Change cmd: remove global lock setting checks for CopyObjectHandler mentions this issue.

halkyon commented 1 month ago

Fixed in https://review.dev.storj.io/c/storj/minio/+/14433 and https://review.dev.storj.io/c/storj/minio/+/14434