HDDS-15852. Allow CopyObject to the same key when the metadata or tagging directive is REPLACE#10752
HDDS-15852. Allow CopyObject to the same key when the metadata or tagging directive is REPLACE#10752rich7420 wants to merge 2 commits into
Conversation
…ging directive is REPLACE A self-copy (same source and destination key) was rejected with 400 InvalidRequest whenever no storage class was set, ignoring the metadata and tagging directives. AWS permits a self-copy as long as at least one attribute is changed, so gate the rejection on the metadata/tag directive not being REPLACE and let REPLACE fall through to the normal copy path.
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for working on this, @rich7420. the metadata self-copy case looks good!
There are two areas where I may be missing some context, so I’m raising them here for discussion:
- Was the
tag-onlyself-copy behavior verified against AWS? I found documentation for replacing tags as part ofCopyObject, but not for using tag replacement alone to
make a same-key copy legal. It is quite possible there is AWS behavior or testing behind this change that I have not found. If available, could you share the reproduction? The bucket encryption configuration would also be helpful because encryption can independently permit in-place copies. - Would you mind extending the metadata integration test with a
HeadObjectcheck? Verifying that the new metadata exists and the old metadata is gone would make the
end-to-end coverage stronger.
References:
| // The object is still readable with its original content after the in-place copy. | ||
| ResponseBytes<GetObjectResponse> objectBytes = s3Client.getObjectAsBytes( | ||
| b -> b.bucket(bucketName).key(key)); | ||
| assertEquals(content, objectBytes.asUtf8String()); |
There was a problem hiding this comment.
Should we also verify the resulting metadata through HeadObject here?
This test currently confirms that the self-copy succeeds and that the object content is preserved, but it would still pass if Ozone accepted the request without replacing the metadata. Since metadata replacement is the behavior this PR fixes, it would be useful to assert both that meta2=v2 is present and that meta1 is absent.
For example:
HeadObjectResponse head = s3Client.headObject(
b -> b.bucket(bucketName).key(key));
assertEquals("v2", head.metadata().get("meta2"));
assertFalse(head.metadata().containsKey("meta1"));The unit test verifies the internal OzoneKeyDetails; this assertion would additionally cover the complete AWS SDK → S3 Gateway → OM → HeadObject path. Thanks!
|
Do we have a performance benchmark for update metadata? Do we have a confirmation that s3g just do metadata update and it takes a constant time and does not depend on the object size? |
…test Per review, verify end-to-end that a metadata-REPLACE self-copy leaves the new x-amz-meta entry and drops the old one, covering the AWS SDK -> S3 Gateway -> OM -> HeadObject path (verified locally on a mini-cluster).
What changes were proposed in this pull request?
A CopyObject request whose source and destination are the same key was rejected with 400 InvalidRequest ("This copy request is illegal because it is trying to copy an object to it self ...") whenever no x-amz-storage-class header was present. This ignored the metadata and tagging directives.
AWS S3 permits a self-copy as long as at least one attribute is changed, including user metadata (x-amz-metadata-directive: REPLACE) or the tag set (x-amz-tagging-directive: REPLACE). The in-place metadata update (self-copy with MetadataDirective=REPLACE) is the canonical way to change an object's metadata.
The self-copy guard in ObjectEndpoint only accounted for the storage-class case. This change reads the metadata and tagging directives up front and only rejects the self-copy when neither directive is REPLACE (and the storage type is default). When either is REPLACE, the request falls through to the normal copy path, which replaces the metadata or tags in place. The storage-class-only self-copy behaviour is unchanged.
What is the link to the Apache JIRA?
https://issues.apache.org/jira/browse/HDDS-15852
How was this patch tested?
https://github.com/rich7420/ozone/actions/runs/29305530153