HDDS-10435. Support S3 object tags for existing requests #6607

ivandika3 · 2024-04-29T10:09:27Z

What changes were proposed in this pull request?

This patch implements the fundamental changes to support object tags. These includes:

Key protobuf field addition for OmKeyInfo to support object tags
Support "x-amz-tagging" to set object tags during PutObject, InitiateMultipartUpload and CopyObject
Support other tagging related metadata (e.g. x-amz-tagging-directive for copy, x-amz-tagging-count for GetObject)
Extension to client APIs (old APIs remains unchanged and should still be compatible, client does not need to change anything)

Since this patch already covers fundamental changes, the S3 object tagging APIs (e.g. PutObjectTagging, GetObjectTagging, and DeleteObjectTagging) support will be implemented in the follow up ticket (HDDS-10655).

Note: Currently OmDirectoryInfo does not support tags yet. We might need to support this in the future.

See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-tagging.html

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10435

How was this patch tested?

Unit, integration, and acceptance tests.

ivandika3 · 2024-04-29T10:13:38Z

@vtutrinov @SaketaChalamchala @tanvipenumudy Could you help take a look when you have time?

SaketaChalamchala · 2024-05-01T20:51:29Z

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java

+      }
+
+      if (tagPair.getName().length() > TAG_KEY_LENGTH_LIMIT) {
+        OS3Exception ex = newError(INVALID_TAG, tagPair.getName());


Thanks for the patch @ivandika3.
nit: Could you move the tag key and value length checks before the tag regex matching checks?

Thank you for the review. Updated.

SaketaChalamchala · 2024-05-07T00:01:12Z

Thanks for addressing the comments @ivandika3 . Just one more question, do the robot tests cover both OBJECT_STORE and FILE_SYSTEM_OPTIMIZED buckets?

vtutrinov · 2024-05-07T10:52:37Z

hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java

@@ -193,6 +204,99 @@ void testPutObjectContentLengthForStreaming()
    assertEquals(15, getKeyDataSize());
  }

+  @Test
+  public void testPutObjectWithTags() throws IOException, OS3Exception {


Let's split the test case into multiple ones (1 case per error or success way) or make it parameterized

Also, if it's possible, I'd like to check the error messages from the responses of fail-cases

Thank you for the review. Updated.

vtutrinov · 2024-05-07T10:55:58Z

hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java

@@ -438,6 +542,92 @@ public void testCopyObjectMessageDigestResetDuringException() throws IOException
    }
  }

+  @Test
+  public void testCopyObjectWithTags() throws IOException, OS3Exception {


The same notice here

vtutrinov · 2024-05-07T11:10:52Z

hadoop-ozone/dist/src/main/smoketest/s3/objectputget.robot

+    ${long_tag_value} =         Execute                               printf 'v%.0s' {1..257}
+    ${result} =                 Execute AWSS3APICli and checkrc       put-object --bucket ${BUCKET} --key ${PREFIX}/putobject/custom-metadata/key2 --body /tmp/testfile2 --tagging="tag-key1=${long_tag_value}"    255
+                                Should contain                        ${result}   InvalidTag
+


nit: might make sense to check the case with a tags list size of more than 10 too

ivandika3 · 2024-05-07T14:34:27Z

Thanks for addressing the comments @ivandika3 . Just one more question, do the robot tests cover both OBJECT_STORE and FILE_SYSTEM_OPTIMIZED buckets?

@SaketaChalamchala I don't think so. Currently it seems "commonawslib.robot" creates bucket using S3 protocol which should default to OBJECT_STORE. We might need to add FSO support for the S3 compatibility test in the future.

I parameterized the integration testing (TestOzoneRpcClientAbstract) to test for different bucket layouts to handle it for now.

SaketaChalamchala · 2024-05-07T17:46:02Z

Thanks for addressing the comments @ivandika3. Raised HDDS-10828. LGTM otherwise.

guohao-rosicky · 2024-05-10T13:43:19Z

I briefly reviewed the code, LGTM. @kerneltime Do you have time to take a look.

kerneltime · 2024-05-10T13:44:07Z

Will take a look today. Thanks

guohao-rosicky · 2024-05-10T13:48:28Z

BTW, @ivandika3 Are you interested in developing User defined object metadata (https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html)?
It's very similar to this feature.

ivandika3 · 2024-05-10T13:52:05Z

Thanks @SaketaChalamchala @guohao-rosicky for the reviews.

BTW, @ivandika3 Are you interested in developing User defined object metadata (https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html)?

It should have been implemented in #3728 and #6489. Please let me know if I miss anything. Tagging is similar to custom metadata, but the use case and constraints are different.

kerneltime · 2024-05-10T20:45:45Z

One requirement is for the client code to move independently of OM version. It is possible that there is a newer S3G client talking to an older OM. In this case how will a new S3G work with an old OM? We can bump up the version between client and OM so that if the client detects an old OM, it will reject the requests with tags.

guohao-rosicky · 2024-05-15T02:09:03Z

Based on the code, the old OM should ignore the tag field. @ivandika3 @kerneltime

ivandika3 · 2024-05-17T08:19:51Z

We can bump up the version between client and OM so that if the client detects an old OM, it will reject the requests with tags.

@kerneltime Updated, I created a new OzoneManagerVersion for object tag support and add logic to reject requests if OM does not support tags. The new OzoneManagerVersion would also be used for client-side OM version validation for PutObjectTagging, DeleteObjectTagging, and GetObjectTagging in HDDS-10655. PTAL thanks.

@guohao-rosicky is correct that the current behavior is the old OM would just ignore the tag fields. It might be better to ignore than to reject for availability reason, but I'm fine with either approach.

adoroszlai · 2024-05-22T06:43:12Z

Thanks @ivandika3 for the patch, @guohao-rosicky, @kerneltime, @SaketaChalamchala, @vtutrinov for the review.

guohao-rosicky · 2024-05-22T08:04:00Z

Thanks @ivandika3 for the patch.
BTW, when you submit https://issues.apache.org/jira/browse/HDDS-10655 Can notify me to review it.

…concile-cli * HDDS-10239-container-reconciliation: (296 commits) HDDS-10897. Refactor OzoneQuota (apache#6714) HDDS-10422. Fix some warnings about exposing internal representation in hdds-common (apache#6351) HDDS-10899. Refactor Lease callbacks (apache#6715) HDDS-10890. Increase default value for hdds.container.ratis.log.appender.queue.num-elements (apache#6711) HDDS-10832. Client should switch to streaming based on OpenKeySession replication (apache#6683) HDDS-10435. Support S3 object tags for existing requests (apache#6607) HDDS-10883. Improve logging in Recon for finalising DN logic. (apache#6704) HDDS-8752. Enable TestOzoneRpcClientAbstract#testOverWriteKeyWithAndWithOutVersioning (apache#6702) HDDS-10875. XceiverRatisServer#getRaftPeersInPipeline should be called before XceiverRatisServer#removeGroup (apache#6696) HDDS-10514. Recon - Provide DN decommissioning detailed status and info inline with current CLI command output. (apache#6376) HDDS-10878. Bump zstd-jni to 1.5.6-3 (apache#6701) HDDS-10877. Bump Dropwizard metrics to 3.2.6 (apache#6699) HDDS-10876. Bump jackson to 2.16.2 (apache#6697) HDDS-6116. Remove flaky tag from TestSCMInstallSnapshot (apache#6695) HDDS-2643. TestOzoneDelegationTokenSecretManager#testRenewTokenFailureRenewalTime fails intermittently. HDDS-10699. Refactor ContainerBalancerTask and TestContainerBalancerTask (apache#6537) HDDS-10861. Ozone cli supports default ozone.om.service.id (apache#6680) HDDS-10859. Improve error messages when decommission and maintenance fail-early (apache#6678) HDDS-9031. Upgrade acceptance tests to Docker Compose v2 (apache#6667) HDDS-10559. Add a warning or a check to run repair tool as System user (apache#6574) ... Conflicts: hadoop-ozone/dist/src/main/smoketest/admincli/container.robot

HDDS-10435. Support S3 object tags for existing requests

c8105e9

ivandika3 self-assigned this Apr 29, 2024

ivandika3 added the s3 S3 Gateway label Apr 29, 2024

ivandika3 requested a review from kerneltime April 29, 2024 10:12

SaketaChalamchala reviewed May 1, 2024

View reviewed changes

Move length validations before regex validations

453af1f

vtutrinov reviewed May 7, 2024

View reviewed changes

ivandika3 added 6 commits May 7, 2024 21:05

Add FSO bucket layout to testCreateKeyWithMetadataAndTags

805183e

Add FSO bucket layout to testMultipartUploadWithTags

c2cdc49

Remove unnecessary set tags operation in OMKeyCommitRequestWithFSO

d02926c

Fix test paremeters

4e9bb13

Separate test methods for failure cases

b9726eb

Acceptance test for too many tags

6fb9131

guohao-rosicky approved these changes May 15, 2024

View reviewed changes

ivandika3 added 2 commits May 17, 2024 16:08

Check OM version before creating key with tags

83a592d

Merge branch 'master' into HDDS-10435

56ba576

adoroszlai mentioned this pull request May 17, 2024

HDDS-10832. Client should switch to streaming based on OpenKeySession replication #6683

Merged

kerneltime approved these changes May 21, 2024

View reviewed changes

adoroszlai merged commit 8a23991 into apache:master May 22, 2024
39 checks passed

ivandika3 deleted the HDDS-10435 branch May 22, 2024 09:27

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 23, 2024

HDDS-10435. Support S3 object tags for existing requests (apache#6607)

64b7f01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-10435. Support S3 object tags for existing requests #6607

HDDS-10435. Support S3 object tags for existing requests #6607

ivandika3 commented Apr 29, 2024 •

edited

Loading

ivandika3 commented Apr 29, 2024

SaketaChalamchala May 1, 2024

ivandika3 May 2, 2024

SaketaChalamchala commented May 7, 2024

vtutrinov May 7, 2024

vtutrinov May 7, 2024

ivandika3 May 7, 2024

vtutrinov May 7, 2024

ivandika3 May 7, 2024

vtutrinov May 7, 2024

ivandika3 May 7, 2024

ivandika3 commented May 7, 2024

SaketaChalamchala commented May 7, 2024

guohao-rosicky commented May 10, 2024

kerneltime commented May 10, 2024

guohao-rosicky commented May 10, 2024

ivandika3 commented May 10, 2024

kerneltime commented May 10, 2024

guohao-rosicky commented May 15, 2024

ivandika3 commented May 17, 2024 •

edited

Loading

adoroszlai commented May 22, 2024

guohao-rosicky commented May 22, 2024 •

edited

Loading

HDDS-10435. Support S3 object tags for existing requests #6607

HDDS-10435. Support S3 object tags for existing requests #6607

Conversation

ivandika3 commented Apr 29, 2024 • edited Loading

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

ivandika3 commented Apr 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaketaChalamchala commented May 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivandika3 commented May 7, 2024

SaketaChalamchala commented May 7, 2024

guohao-rosicky commented May 10, 2024

kerneltime commented May 10, 2024

guohao-rosicky commented May 10, 2024

ivandika3 commented May 10, 2024

kerneltime commented May 10, 2024

guohao-rosicky commented May 15, 2024

ivandika3 commented May 17, 2024 • edited Loading

adoroszlai commented May 22, 2024

guohao-rosicky commented May 22, 2024 • edited Loading

ivandika3 commented Apr 29, 2024 •

edited

Loading

ivandika3 commented May 17, 2024 •

edited

Loading

guohao-rosicky commented May 22, 2024 •

edited

Loading