You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ivandika3 (via GitHub)" <gi...@apache.org> on 2024/05/31 07:58:14 UTC
[PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
ivandika3 opened a new pull request, #6756:
URL: https://github.com/apache/ozone/pull/6756
## What changes were proposed in this pull request?
After [HDDS-10435](https://issues.apache.org/jira/browse/HDDS-10435) has implemented the fundamental changes to support object tags. This patch supports the S3 object tagging APIs.
This patch will create add new S3 tagging endpoints as well as add OM request to object tagging requests. Additionally, this patch also added related tagging metrics (S3 and OM metrics).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10655
## How was this patch tested?
Unit, integration, and acceptance tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836284210
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -1381,6 +1412,75 @@ public static boolean checkCopySourceModificationTime(
(lastModificationTime <= copySourceIfUnmodifiedSince);
}
+ private Response putObjectTagging(OzoneVolume volume, String bucketName, String keyName, InputStream body)
+ throws IOException, OS3Exception {
+ long startNanos = Time.monotonicNowNanos();
+ S3Tagging tagging = null;
+ try {
+ tagging = new PutTaggingUnmarshaller().readFrom(body);
+ tagging.validate();
+ } catch (WebApplicationException ex) {
+ OS3Exception exception = S3ErrorTable.newError(S3ErrorTable.MALFORMED_XML, keyName);
+ exception.setErrorMessage(exception.getErrorMessage() + ". " + ex.getMessage());
+ throw exception;
+ }
+
+ Map<String, String> tags = validateAndGetTagging(
+ tagging.getTagSet().getTags(), // Nullity check was done in previous parsing step
+ Tag::getKey,
+ Tag::getValue
+ );
+
+ try {
+ volume.getBucket(bucketName).putObjectTagging(keyName, tags);
+ } catch (OMException ex) {
+ if (ex.getResult() == ResultCodes.INVALID_REQUEST) {
+ throw S3ErrorTable.newError(S3ErrorTable.INVALID_REQUEST, bucketName);
+ } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+ throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, bucketName);
+ } else if (ex.getResult() == ResultCodes.KEY_NOT_FOUND) {
+ throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_KEY, keyName);
+ } else if (ex.getResult() == ResultCodes.NOT_SUPPORTED_OPERATION) {
+ // When putObjectTagging operation is applied on FSO directory
+ throw S3ErrorTable.newError(S3ErrorTable.NOT_IMPLEMENTED, keyName);
+ }
Review Comment:
Thanks for catching it. Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1844904066
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tagging/S3DeleteObjectTaggingRequest.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.s3.tagging;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tagging.S3DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles delete object tagging request.
+ */
+public class S3DeleteObjectTaggingRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(S3DeleteObjectTaggingRequest.class);
+
+ public S3DeleteObjectTaggingRequest(OMRequest omRequest, BucketLayout bucketLayout) {
+ super(omRequest, bucketLayout);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest =
+ super.preExecute(ozoneManager).getDeleteObjectTaggingRequest();
+ Preconditions.checkNotNull(deleteObjectTaggingRequest);
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+
+ String keyPath = keyArgs.getKeyName();
+ keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+ keyPath, getBucketLayout());
+
+ KeyArgs.Builder newKeyArgs =
+ keyArgs.toBuilder()
+ .setKeyName(keyPath);
+
+ KeyArgs resolvedArgs = resolveBucketAndCheckKeyAcls(newKeyArgs.build(),
+ ozoneManager, ACLType.WRITE);
+ return getOmRequest().toBuilder()
+ .setUserInfo(getUserInfo())
+ .setDeleteObjectTaggingRequest(
+ deleteObjectTaggingRequest.toBuilder().setKeyArgs(resolvedArgs))
+ .build();
+ }
+
+ @Override
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIndex termIndex) {
+ final long trxnLogIndex = termIndex.getIndex();
+
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest = getOmRequest().getDeleteObjectTaggingRequest();
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+ String volumeName = keyArgs.getVolumeName();
+ String bucketName = keyArgs.getBucketName();
+ String keyName = keyArgs.getKeyName();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumDeleteObjectTagging();
+
+ Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ boolean acquiredLock = false;
+ OMClientResponse omClientResponse = null;
+ IOException exception = null;
+ Result result = null;
+ try {
+ mergeOmLockDetails(
+ omMetadataManager.getLock()
+ .acquireWriteLock(BUCKET_LOCK, volumeName, bucketName)
+ );
+ acquiredLock = getOmLockDetails().isLockAcquired();
+
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ String dbOzoneKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, keyName);
+
+ OmKeyInfo omKeyInfo =
+ omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+ if (omKeyInfo == null) {
+ throw new OMException("Key not found", KEY_NOT_FOUND);
+ }
+
+ // Clear / delete the tags
+ omKeyInfo.getTags().clear();
+ // Set the UpdateID to the current transactionLogIndex
+ omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ // Note: Key modification time is not changed because S3 last modified
+ // time only changes when there are changes in the object content
+
+ // Update table cache
+ omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+ new CacheKey<>(dbOzoneKey),
+ CacheValue.get(trxnLogIndex, omKeyInfo)
+ );
+
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ omResponse.setDeleteObjectTaggingResponse(DeleteObjectTaggingResponse.newBuilder()).build(),
+ omKeyInfo
+ );
+
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ createErrorOMResponse(omResponse, exception),
+ getBucketLayout()
+ );
+ } finally {
+ if (acquiredLock) {
+ mergeOmLockDetails(omMetadataManager.getLock()
+ .releaseWriteLock(BUCKET_LOCK, volumeName, bucketName));
+ }
+ if (omClientResponse != null) {
+ omClientResponse.setOmLockDetails(getOmLockDetails());
+ }
+ }
+
+ markForAudit(ozoneManager.getAuditLogger(), buildAuditMessage(
+ OMAction.DELETE_OBJECT_TAGGING, auditMap, exception, getOmRequest().getUserInfo()
+ ));
+
+ switch (result) {
+ case SUCCESS:
+ LOG.debug("Delete object tagging success. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName);
+ break;
+ case FAILURE:
+ omMetrics.incNumDeleteObjectTaggingFails();
+ LOG.error("Delete object tagging failed. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName, exception);
Review Comment:
@adoroszlai Sure, I have added the check to prevent `KEY_NOT_FOUND` to emit error logs. We can file a new ticket to update the other OM requests or add other exception types that should not be logged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836333376
##########
hadoop-ozone/dist/src/main/smoketest/s3/objecttagging.robot:
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+*** Settings ***
+Documentation S3 gateway test with aws cli
+Library OperatingSystem
+Library String
+Resource ../commonlib.robot
+Resource commonawslib.robot
+Test Timeout 5 minutes
+Suite Setup Setup s3 tests
+
+*** Variables ***
+${ENDPOINT_URL} http://s3g:9878
+${OZONE_TEST} true
+${BUCKET} generated
+
+
+*** Test Cases ***
+
+Put object tagging
Review Comment:
Filed https://issues.apache.org/jira/browse/HDDS-11678 for the extract the object tagging keywords.
##########
hadoop-ozone/dist/src/main/smoketest/s3/objecttagging.robot:
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+*** Settings ***
+Documentation S3 gateway test with aws cli
+Library OperatingSystem
+Library String
+Resource ../commonlib.robot
+Resource commonawslib.robot
+Test Timeout 5 minutes
+Suite Setup Setup s3 tests
+
+*** Variables ***
+${ENDPOINT_URL} http://s3g:9878
+${OZONE_TEST} true
+${BUCKET} generated
+
+
+*** Test Cases ***
+
+Put object tagging
Review Comment:
Filed https://issues.apache.org/jira/browse/HDDS-11678 for extracting the object tagging keywords.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6756:
URL: https://github.com/apache/ozone/pull/6756#issuecomment-2481019647
Thanks @ivandika3 for the patch, @SaketaChalamchala, @Tejaskriya for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1673293893
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectTaggingPut.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientStub;
+import org.apache.hadoop.ozone.client.OzoneKeyDetails;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import javax.ws.rs.core.HttpHeaders;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.function.Supplier;
+
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.MALFORMED_XML;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NO_SUCH_BUCKET;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NO_SUCH_KEY;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Tests for PutObjectTagging.
+ */
+public class TestObjectTaggingPut {
+
+ private OzoneClient clientStub;
+ private ObjectEndpoint objectEndpoint;
+
+ private static final String BUCKET_NAME = "b1";
+ private static final String KEY_NAME = "key=value/1";
+
+ @BeforeEach
+ void setup() throws IOException, OS3Exception {
+ OzoneConfiguration config = new OzoneConfiguration();
+
+ //Create client stub and object store stub.
+ clientStub = new OzoneClientStub();
+
+ // Create bucket
+ clientStub.getObjectStore().createS3Bucket(BUCKET_NAME);
+
+ // Create PutObject and setClient to OzoneClientStub
+ objectEndpoint = new ObjectEndpoint();
+ objectEndpoint.setClient(clientStub);
+ objectEndpoint.setOzoneConfiguration(config);
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ ByteArrayInputStream body =
+ new ByteArrayInputStream("".getBytes(UTF_8));
+ objectEndpoint.setHeaders(headers);
+
+ objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1, null, null, body);
+ }
+
+ @Test
+ public void testPutObjectTaggingWithEmptyBody() throws Exception {
+ try {
+ objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1, null, "",
+ null);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(HTTP_BAD_REQUEST, ex.getHttpCode());
+ assertEquals(MALFORMED_XML.getCode(), ex.getCode());
+ }
+ }
+
+ @Test
+ public void testPutValidObjectTagging() throws Exception {
+ assertEquals(HTTP_OK, objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1,
+ null, "", twoTags()).getStatus());
+ OzoneKeyDetails keyDetails =
+ clientStub.getObjectStore().getS3Bucket(BUCKET_NAME).getKey(KEY_NAME);
+ assertEquals(2, keyDetails.getTags().size());
+ assertEquals("val1", keyDetails.getTags().get("tag1"));
+ assertEquals("val2", keyDetails.getTags().get("tag2"));
+ }
+
+ @Test
+ public void testPutInvalidObjectTagging() throws Exception {
+ testInvalidObjectTagging(this::emptyBody, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::invalidXmlStructure, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::noTagSet, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::emptyTags, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::tagKeyNotSpecified, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::tagValueNotSpecified, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ }
+
+ private void testInvalidObjectTagging(Supplier<InputStream> inputStream,
+ int expectedHttpCode, String expectedErrorCode) throws Exception {
+ try {
+ objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1, null, "",
+ inputStream.get());
+ fail("Expected an OS3Exception to be thrown");
+ } catch (OS3Exception ex) {
+ assertEquals(expectedHttpCode, ex.getHttpCode());
+ assertEquals(expectedErrorCode, ex.getCode());
+ }
+ }
+
+ @Test
+ public void testPutObjectTaggingNoKeyFound() throws Exception {
+ try {
+ objectEndpoint.put(BUCKET_NAME, "nonexistent", 0, 1,
+ null, "", twoTags());
+ fail("Expected an OS3Exception to be thrown");
+ } catch (OS3Exception ex) {
+ assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
+ assertEquals(NO_SUCH_KEY.getCode(), ex.getCode());
+ }
+ }
+
+ @Test
+ public void testPutObjectTaggingNoBucketFound() throws Exception {
+ try {
+ objectEndpoint.put("nonexistent", "nonexistent", 0, 1,
+ null, "", twoTags());
+ fail("Expected an OS3Exception to be thrown");
+ } catch (OS3Exception ex) {
+ assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
+ assertEquals(NO_SUCH_BUCKET.getCode(), ex.getCode());
+ }
+ }
Review Comment:
Could you add another test for NOT_IMPLEMENTED error code here and in TestObjectDeleteTagging.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6756:
URL: https://github.com/apache/ozone/pull/6756
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1630702738
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/KeyValueUtil.java:
##########
@@ -34,8 +34,8 @@ private KeyValueUtil() {
/**
* Parse Key,Value map data from protobuf representation.
*/
- public static Map<String, String> getFromProtobuf(List<KeyValue> metadata) {
- return metadata.stream()
+ public static Map<String, String> getFromProtobuf(List<KeyValue> keyValueList) {
+ return keyValueList.stream()
Review Comment:
We can avoid this change as it is only variable name change in the whole file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1634552750
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/KeyValueUtil.java:
##########
@@ -34,8 +34,8 @@ private KeyValueUtil() {
/**
* Parse Key,Value map data from protobuf representation.
*/
- public static Map<String, String> getFromProtobuf(List<KeyValue> metadata) {
- return metadata.stream()
+ public static Map<String, String> getFromProtobuf(List<KeyValue> keyValueList) {
+ return keyValueList.stream()
Review Comment:
Thanks for the review. I changed it since the user of this method is not for metadata only (i.e. tags).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1828937527
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -1381,6 +1412,75 @@ public static boolean checkCopySourceModificationTime(
(lastModificationTime <= copySourceIfUnmodifiedSince);
}
+ private Response putObjectTagging(OzoneVolume volume, String bucketName, String keyName, InputStream body)
+ throws IOException, OS3Exception {
+ long startNanos = Time.monotonicNowNanos();
+ S3Tagging tagging = null;
+ try {
+ tagging = new PutTaggingUnmarshaller().readFrom(body);
+ tagging.validate();
+ } catch (WebApplicationException ex) {
+ OS3Exception exception = S3ErrorTable.newError(S3ErrorTable.MALFORMED_XML, keyName);
+ exception.setErrorMessage(exception.getErrorMessage() + ". " + ex.getMessage());
+ throw exception;
+ }
+
+ Map<String, String> tags = validateAndGetTagging(
+ tagging.getTagSet().getTags(), // Nullity check was done in previous parsing step
+ Tag::getKey,
+ Tag::getValue
+ );
+
+ try {
+ volume.getBucket(bucketName).putObjectTagging(keyName, tags);
+ } catch (OMException ex) {
+ if (ex.getResult() == ResultCodes.INVALID_REQUEST) {
+ throw S3ErrorTable.newError(S3ErrorTable.INVALID_REQUEST, bucketName);
+ } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+ throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, bucketName);
+ } else if (ex.getResult() == ResultCodes.KEY_NOT_FOUND) {
+ throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_KEY, keyName);
+ } else if (ex.getResult() == ResultCodes.NOT_SUPPORTED_OPERATION) {
+ // When putObjectTagging operation is applied on FSO directory
+ throw S3ErrorTable.newError(S3ErrorTable.NOT_IMPLEMENTED, keyName);
+ }
Review Comment:
Why do some of these include `bucketName` in the error, while others `keyName`?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java:
##########
@@ -1145,11 +1145,37 @@ void setTimes(OmKeyArgs keyArgs, long mtime, long atime)
boolean setSafeMode(SafeModeAction action, boolean isChecked)
throws IOException;
+ /**
+ * Gets the tags for the specified key.
+ * @param args Key args
+ * @return Tags associated with the key.
+ */
+ Map<String, String> getObjectTagging(OmKeyArgs args) throws IOException;
+
+ /**
+ * Sets the tags to an existing key.
+ * @param args Key args
+ */
+ default void putObjectTagging(OmKeyArgs args) throws IOException {
+ throw new UnsupportedOperationException("OzoneManager does not require " +
+ "this to be implemented, as write requests use a new approach.");
+ }
+
+ /**
+ * Removes all the tags from the specified key.
+ * @param args Key args
+ */
+ default void deleteObjectTagging(OmKeyArgs args) throws IOException {
+ throw new UnsupportedOperationException("OzoneManager does not require " +
+ "this to be implemented, as write requests use a new approach.");
+ }
+
/**
* Get status of last triggered quota repair in OM.
* @return String
* @throws IOException
*/
+
String getQuotaRepairStatus() throws IOException;
Review Comment:
nit: I guess this new line is unintentional
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4767,6 +4767,18 @@ public void startQuotaRepair(List<String> buckets) throws IOException {
new QuotaRepairTask(this).repair(buckets);
}
+ /**
+ * {@inheritDoc}
+ */
Review Comment:
nit: `{@inheritDoc}` is needed only when adding extra doc to the parent's; otherwise doc is inherited automatically.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -2727,6 +2727,61 @@ public void recoverKey(OmKeyArgs args, long clientID) throws IOException {
ozoneManagerClient.recoverKey(args, clientID);
}
+ @Override
+ public Map<String, String> getObjectTagging(String volumeName, String bucketName, String keyName)
+ throws IOException {
+ if (omVersion.compareTo(OzoneManagerVersion.OBJECT_TAG) < 0) {
+ throw new IOException("OzoneManager does not support object tags");
+ }
Review Comment:
`OzoneManagerVersion.OBJECT_TAG` was introduced in 8a239912378f8b2aa5de84d9f913821096b74fcb. Any OM built since that commit but preceding this change does not support these methods. So request from client to such OM would fail with `InvalidProtocolBufferException`. Failing with `IOException("OzoneManager does not support object tags")` might be friendlier, but we need a new `OzoneManagerVersion` for that.
CC @errose28
##########
hadoop-ozone/dist/src/main/smoketest/s3/objecttagging.robot:
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+*** Settings ***
+Documentation S3 gateway test with aws cli
+Library OperatingSystem
+Library String
+Resource ../commonlib.robot
+Resource commonawslib.robot
+Test Timeout 5 minutes
+Suite Setup Setup s3 tests
+
+*** Variables ***
+${ENDPOINT_URL} http://s3g:9878
+${OZONE_TEST} true
+${BUCKET} generated
+
+
+*** Test Cases ***
+
+Put object tagging
Review Comment:
Would be nice to add negative test cases (e.g. request for non-existent key). Can be done in follow-up.
Also would be nice to create reusable keywords to avoid duplication. Also can be in follow-up.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Tagging.java:
##########
@@ -0,0 +1,156 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import javax.ws.rs.WebApplicationException;
+import javax.xml.bind.annotation.XmlAccessType;
+import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * S3 tagging.
+ */
+@XmlAccessorType(XmlAccessType.FIELD)
+@XmlRootElement(name = "Tagging",
+ namespace = "http://s3.amazonaws.com/doc/2006-03-01/")
+public class S3Tagging {
+
+ @XmlElement(name = "TagSet")
+ private TagSet tagSet;
+
+ public S3Tagging() {
+
+ }
+
+ public S3Tagging(TagSet tagSet) {
+ this.tagSet = tagSet;
+ }
+
+ public TagSet getTagSet() {
+ return tagSet;
+ }
+
+ public void setTagSet(TagSet tagSet) {
+ this.tagSet = tagSet;
+ }
+
+ /**
+ * Entity for child element TagSet.
+ */
+ @XmlAccessorType(XmlAccessType.FIELD)
+ @XmlRootElement(name = "TagSet")
+ public static class TagSet {
+ @XmlElement(name = "Tag")
+ private List<Tag> tags = new ArrayList<>();
+
+ public TagSet() {
+ }
+
+ public TagSet(List<Tag> tags) {
+ this.tags = tags;
+ }
+
+ public List<Tag> getTags() {
+ return tags;
+ }
+
+ public void setTags(List<Tag> tags) {
+ this.tags = tags;
+ }
+ }
+
+ /**
+ * Entity for child element Tag.
+ */
+ @XmlAccessorType(XmlAccessType.FIELD)
+ @XmlRootElement(name = "Tag")
+ public static class Tag {
+ @XmlElement(name = "Key")
+ private String key;
+
+ @XmlElement(name = "Value")
+ private String value;
+
+ public Tag() {
+ }
+
+ public Tag(String key, String value) {
+ this.key = key;
+ this.value = value;
+ }
+
+ public String getKey() {
+ return key;
+ }
+
+ public void setKey(String key) {
+ this.key = key;
+ }
+
+ public String getValue() {
+ return value;
+ }
+
+ public void setValue(String value) {
+ this.value = value;
+ }
+ }
+
+ /**
+ * Creates a S3 tagging instance (xml representation) from a Map retrieved
+ * from OM.
+ * @param tagMap Map representing the tags.
+ * @return {@link S3Tagging}
+ */
+ public static S3Tagging fromMap(Map<String, String> tagMap) {
+ List<Tag> tags = tagMap.entrySet()
+ .stream()
+ .map(
+ tagEntry -> new Tag(tagEntry.getKey(), tagEntry.getValue())
+ )
+ .collect(Collectors.toList());
+ return new S3Tagging(new TagSet(tags));
+ }
+
+ /**
+ * Additional XML validation logic for S3 tagging.
+ */
+ public void validate() {
+ if (tagSet == null) {
+ throw new WebApplicationException("TagSet needs to be specified");
+ }
+
+ if (tagSet.getTags().isEmpty()) {
+ throw new WebApplicationException("Tags need to be specified and cannot be empty");
+ }
+
+ for (Tag tag: tagSet.getTags()) {
+ if (tag.getKey() == null) {
+ throw new WebApplicationException("Some tag keys are not specified");
+ }
+ if (tag.getValue() == null) {
+ throw new WebApplicationException("Tag value for tag " + tag.getKey() + " is not specified");
+ }
+ }
+ }
Review Comment:
nit: I would use `WebApplicationException` if a specific HTTP response status needs to be set, or if it is to be handled by some framework not by application code. However, these instances are created with `INTERNAL_SERVER_ERROR` (default value), which does not reflect that these are thrown due to invalid request content. Also, we are catching it in `ObjectEndpoint` and translating to `OS3Exception`. So I think we may throw regular `IllegalStateException` or `IllegalArgumentException` here and in `PutTaggingUnmarshaller`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tagging/S3DeleteObjectTaggingRequest.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.s3.tagging;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tagging.S3DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles delete object tagging request.
+ */
+public class S3DeleteObjectTaggingRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(S3DeleteObjectTaggingRequest.class);
+
+ public S3DeleteObjectTaggingRequest(OMRequest omRequest, BucketLayout bucketLayout) {
+ super(omRequest, bucketLayout);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest =
+ super.preExecute(ozoneManager).getDeleteObjectTaggingRequest();
+ Preconditions.checkNotNull(deleteObjectTaggingRequest);
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+
+ String keyPath = keyArgs.getKeyName();
+ keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+ keyPath, getBucketLayout());
+
+ KeyArgs.Builder newKeyArgs =
+ keyArgs.toBuilder()
+ .setKeyName(keyPath);
+
+ KeyArgs resolvedArgs = resolveBucketAndCheckKeyAcls(newKeyArgs.build(),
+ ozoneManager, ACLType.WRITE);
+ return getOmRequest().toBuilder()
+ .setUserInfo(getUserInfo())
+ .setDeleteObjectTaggingRequest(
+ deleteObjectTaggingRequest.toBuilder().setKeyArgs(resolvedArgs))
+ .build();
+ }
+
+ @Override
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIndex termIndex) {
+ final long trxnLogIndex = termIndex.getIndex();
+
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest = getOmRequest().getDeleteObjectTaggingRequest();
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+ String volumeName = keyArgs.getVolumeName();
+ String bucketName = keyArgs.getBucketName();
+ String keyName = keyArgs.getKeyName();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumDeleteObjectTagging();
+
+ Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ boolean acquiredLock = false;
+ OMClientResponse omClientResponse = null;
+ IOException exception = null;
+ Result result = null;
+ try {
+ mergeOmLockDetails(
+ omMetadataManager.getLock()
+ .acquireWriteLock(BUCKET_LOCK, volumeName, bucketName)
+ );
+ acquiredLock = getOmLockDetails().isLockAcquired();
+
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ String dbOzoneKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, keyName);
+
+ OmKeyInfo omKeyInfo =
+ omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+ if (omKeyInfo == null) {
+ throw new OMException("Key not found", KEY_NOT_FOUND);
+ }
+
+ // Clear / delete the tags
+ omKeyInfo.getTags().clear();
+ // Set the UpdateID to the current transactionLogIndex
+ omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ // Note: Key modification time is not changed because S3 last modified
+ // time only changes when there are changes in the object content
+
+ // Update table cache
+ omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+ new CacheKey<>(dbOzoneKey),
+ CacheValue.get(trxnLogIndex, omKeyInfo)
+ );
+
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ omResponse.setDeleteObjectTaggingResponse(DeleteObjectTaggingResponse.newBuilder()).build(),
+ omKeyInfo
+ );
+
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ createErrorOMResponse(omResponse, exception),
+ getBucketLayout()
+ );
+ } finally {
+ if (acquiredLock) {
+ mergeOmLockDetails(omMetadataManager.getLock()
+ .releaseWriteLock(BUCKET_LOCK, volumeName, bucketName));
+ }
+ if (omClientResponse != null) {
+ omClientResponse.setOmLockDetails(getOmLockDetails());
+ }
+ }
+
+ markForAudit(ozoneManager.getAuditLogger(), buildAuditMessage(
+ OMAction.DELETE_OBJECT_TAGGING, auditMap, exception, getOmRequest().getUserInfo()
+ ));
+
+ switch (result) {
+ case SUCCESS:
+ LOG.debug("Delete object tagging success. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName);
+ break;
+ case FAILURE:
+ omMetrics.incNumDeleteObjectTaggingFails();
+ LOG.error("Delete object tagging failed. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName, exception);
Review Comment:
I don't think we should log all failures at `ERROR` level. System errors are OK, but requests can also fail due to input from user:
```
2024-11-05 11:27:58,742 [om1-OMStateMachineApplyTransactionThread - 0] ERROR tagging.S3DeleteObjectTaggingRequest: Delete object tagging failed. Volume:s3v, Bucket:bucket-azwyiqbquu, Key:ozone-test-2023699972/putobject/key=value/f2.
KEY_NOT_FOUND org.apache.hadoop.ozone.om.exceptions.OMException: Key not found
at org.apache.hadoop.ozone.om.request.s3.tagging.S3DeleteObjectTaggingRequest.validateAndUpdateCache(S3DeleteObjectTaggingRequest.java:126)
at org.apache.hadoop.ozone.protocolPB.OzoneManagerRequestHandler.lambda$0(OzoneManagerRequestHandler.java:421)
at org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs(MetricUtil.java:46)
at org.apache.hadoop.ozone.protocolPB.OzoneManagerRequestHandler.handleWriteRequestImpl(OzoneManagerRequestHandler.java:419)
at org.apache.hadoop.ozone.protocolPB.RequestHandler.handleWriteRequest(RequestHandler.java:63)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.runCommand(OzoneManagerStateMachine.java:551)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.lambda$1(OzoneManagerStateMachine.java:367)
at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:833)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836289134
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tagging/S3DeleteObjectTaggingRequest.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.s3.tagging;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tagging.S3DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles delete object tagging request.
+ */
+public class S3DeleteObjectTaggingRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(S3DeleteObjectTaggingRequest.class);
+
+ public S3DeleteObjectTaggingRequest(OMRequest omRequest, BucketLayout bucketLayout) {
+ super(omRequest, bucketLayout);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest =
+ super.preExecute(ozoneManager).getDeleteObjectTaggingRequest();
+ Preconditions.checkNotNull(deleteObjectTaggingRequest);
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+
+ String keyPath = keyArgs.getKeyName();
+ keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+ keyPath, getBucketLayout());
+
+ KeyArgs.Builder newKeyArgs =
+ keyArgs.toBuilder()
+ .setKeyName(keyPath);
+
+ KeyArgs resolvedArgs = resolveBucketAndCheckKeyAcls(newKeyArgs.build(),
+ ozoneManager, ACLType.WRITE);
+ return getOmRequest().toBuilder()
+ .setUserInfo(getUserInfo())
+ .setDeleteObjectTaggingRequest(
+ deleteObjectTaggingRequest.toBuilder().setKeyArgs(resolvedArgs))
+ .build();
+ }
+
+ @Override
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIndex termIndex) {
+ final long trxnLogIndex = termIndex.getIndex();
+
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest = getOmRequest().getDeleteObjectTaggingRequest();
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+ String volumeName = keyArgs.getVolumeName();
+ String bucketName = keyArgs.getBucketName();
+ String keyName = keyArgs.getKeyName();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumDeleteObjectTagging();
+
+ Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ boolean acquiredLock = false;
+ OMClientResponse omClientResponse = null;
+ IOException exception = null;
+ Result result = null;
+ try {
+ mergeOmLockDetails(
+ omMetadataManager.getLock()
+ .acquireWriteLock(BUCKET_LOCK, volumeName, bucketName)
+ );
+ acquiredLock = getOmLockDetails().isLockAcquired();
+
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ String dbOzoneKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, keyName);
+
+ OmKeyInfo omKeyInfo =
+ omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+ if (omKeyInfo == null) {
+ throw new OMException("Key not found", KEY_NOT_FOUND);
+ }
+
+ // Clear / delete the tags
+ omKeyInfo.getTags().clear();
+ // Set the UpdateID to the current transactionLogIndex
+ omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ // Note: Key modification time is not changed because S3 last modified
+ // time only changes when there are changes in the object content
+
+ // Update table cache
+ omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+ new CacheKey<>(dbOzoneKey),
+ CacheValue.get(trxnLogIndex, omKeyInfo)
+ );
+
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ omResponse.setDeleteObjectTaggingResponse(DeleteObjectTaggingResponse.newBuilder()).build(),
+ omKeyInfo
+ );
+
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ createErrorOMResponse(omResponse, exception),
+ getBucketLayout()
+ );
+ } finally {
+ if (acquiredLock) {
+ mergeOmLockDetails(omMetadataManager.getLock()
+ .releaseWriteLock(BUCKET_LOCK, volumeName, bucketName));
+ }
+ if (omClientResponse != null) {
+ omClientResponse.setOmLockDetails(getOmLockDetails());
+ }
+ }
+
+ markForAudit(ozoneManager.getAuditLogger(), buildAuditMessage(
+ OMAction.DELETE_OBJECT_TAGGING, auditMap, exception, getOmRequest().getUserInfo()
+ ));
+
+ switch (result) {
+ case SUCCESS:
+ LOG.debug("Delete object tagging success. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName);
+ break;
+ case FAILURE:
+ omMetrics.incNumDeleteObjectTaggingFails();
+ LOG.error("Delete object tagging failed. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName, exception);
Review Comment:
I think most OM requests will do error log for key not found.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1720974118
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectTaggingPut.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientStub;
+import org.apache.hadoop.ozone.client.OzoneKeyDetails;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import javax.ws.rs.core.HttpHeaders;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.function.Supplier;
+
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.MALFORMED_XML;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NO_SUCH_BUCKET;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NO_SUCH_KEY;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Tests for PutObjectTagging.
+ */
+public class TestObjectTaggingPut {
+
+ private OzoneClient clientStub;
+ private ObjectEndpoint objectEndpoint;
+
+ private static final String BUCKET_NAME = "b1";
+ private static final String KEY_NAME = "key=value/1";
+
+ @BeforeEach
+ void setup() throws IOException, OS3Exception {
+ OzoneConfiguration config = new OzoneConfiguration();
+
+ //Create client stub and object store stub.
+ clientStub = new OzoneClientStub();
+
+ // Create bucket
+ clientStub.getObjectStore().createS3Bucket(BUCKET_NAME);
+
+ // Create PutObject and setClient to OzoneClientStub
+ objectEndpoint = new ObjectEndpoint();
+ objectEndpoint.setClient(clientStub);
+ objectEndpoint.setOzoneConfiguration(config);
+
+ HttpHeaders headers = mock(HttpHeaders.class);
+ ByteArrayInputStream body =
+ new ByteArrayInputStream("".getBytes(UTF_8));
+ objectEndpoint.setHeaders(headers);
+
+ objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1, null, null, body);
+ }
+
+ @Test
+ public void testPutObjectTaggingWithEmptyBody() throws Exception {
+ try {
+ objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1, null, "",
+ null);
+ fail();
+ } catch (OS3Exception ex) {
+ assertEquals(HTTP_BAD_REQUEST, ex.getHttpCode());
+ assertEquals(MALFORMED_XML.getCode(), ex.getCode());
+ }
+ }
+
+ @Test
+ public void testPutValidObjectTagging() throws Exception {
+ assertEquals(HTTP_OK, objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1,
+ null, "", twoTags()).getStatus());
+ OzoneKeyDetails keyDetails =
+ clientStub.getObjectStore().getS3Bucket(BUCKET_NAME).getKey(KEY_NAME);
+ assertEquals(2, keyDetails.getTags().size());
+ assertEquals("val1", keyDetails.getTags().get("tag1"));
+ assertEquals("val2", keyDetails.getTags().get("tag2"));
+ }
+
+ @Test
+ public void testPutInvalidObjectTagging() throws Exception {
+ testInvalidObjectTagging(this::emptyBody, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::invalidXmlStructure, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::noTagSet, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::emptyTags, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::tagKeyNotSpecified, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ testInvalidObjectTagging(this::tagValueNotSpecified, HTTP_BAD_REQUEST, MALFORMED_XML.getCode());
+ }
+
+ private void testInvalidObjectTagging(Supplier<InputStream> inputStream,
+ int expectedHttpCode, String expectedErrorCode) throws Exception {
+ try {
+ objectEndpoint.put(BUCKET_NAME, KEY_NAME, 0, 1, null, "",
+ inputStream.get());
+ fail("Expected an OS3Exception to be thrown");
+ } catch (OS3Exception ex) {
+ assertEquals(expectedHttpCode, ex.getHttpCode());
+ assertEquals(expectedErrorCode, ex.getCode());
+ }
+ }
+
+ @Test
+ public void testPutObjectTaggingNoKeyFound() throws Exception {
+ try {
+ objectEndpoint.put(BUCKET_NAME, "nonexistent", 0, 1,
+ null, "", twoTags());
+ fail("Expected an OS3Exception to be thrown");
+ } catch (OS3Exception ex) {
+ assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
+ assertEquals(NO_SUCH_KEY.getCode(), ex.getCode());
+ }
+ }
+
+ @Test
+ public void testPutObjectTaggingNoBucketFound() throws Exception {
+ try {
+ objectEndpoint.put("nonexistent", "nonexistent", 0, 1,
+ null, "", twoTags());
+ fail("Expected an OS3Exception to be thrown");
+ } catch (OS3Exception ex) {
+ assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
+ assertEquals(NO_SUCH_BUCKET.getCode(), ex.getCode());
+ }
+ }
Review Comment:
Thanks for the review @SaketaChalamchala, updated PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836279247
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java:
##########
@@ -1145,11 +1145,37 @@ void setTimes(OmKeyArgs keyArgs, long mtime, long atime)
boolean setSafeMode(SafeModeAction action, boolean isChecked)
throws IOException;
+ /**
+ * Gets the tags for the specified key.
+ * @param args Key args
+ * @return Tags associated with the key.
+ */
+ Map<String, String> getObjectTagging(OmKeyArgs args) throws IOException;
+
+ /**
+ * Sets the tags to an existing key.
+ * @param args Key args
+ */
+ default void putObjectTagging(OmKeyArgs args) throws IOException {
+ throw new UnsupportedOperationException("OzoneManager does not require " +
+ "this to be implemented, as write requests use a new approach.");
+ }
+
+ /**
+ * Removes all the tags from the specified key.
+ * @param args Key args
+ */
+ default void deleteObjectTagging(OmKeyArgs args) throws IOException {
+ throw new UnsupportedOperationException("OzoneManager does not require " +
+ "this to be implemented, as write requests use a new approach.");
+ }
+
/**
* Get status of last triggered quota repair in OM.
* @return String
* @throws IOException
*/
+
String getQuotaRepairStatus() throws IOException;
Review Comment:
Updated.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4767,6 +4767,18 @@ public void startQuotaRepair(List<String> buckets) throws IOException {
new QuotaRepairTask(this).repair(buckets);
}
+ /**
+ * {@inheritDoc}
+ */
Review Comment:
Thanks for the tip. Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836434482
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tagging/S3DeleteObjectTaggingRequest.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.s3.tagging;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tagging.S3DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteObjectTaggingResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.ratis.server.protocol.TermIndex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles delete object tagging request.
+ */
+public class S3DeleteObjectTaggingRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(S3DeleteObjectTaggingRequest.class);
+
+ public S3DeleteObjectTaggingRequest(OMRequest omRequest, BucketLayout bucketLayout) {
+ super(omRequest, bucketLayout);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest =
+ super.preExecute(ozoneManager).getDeleteObjectTaggingRequest();
+ Preconditions.checkNotNull(deleteObjectTaggingRequest);
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+
+ String keyPath = keyArgs.getKeyName();
+ keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+ keyPath, getBucketLayout());
+
+ KeyArgs.Builder newKeyArgs =
+ keyArgs.toBuilder()
+ .setKeyName(keyPath);
+
+ KeyArgs resolvedArgs = resolveBucketAndCheckKeyAcls(newKeyArgs.build(),
+ ozoneManager, ACLType.WRITE);
+ return getOmRequest().toBuilder()
+ .setUserInfo(getUserInfo())
+ .setDeleteObjectTaggingRequest(
+ deleteObjectTaggingRequest.toBuilder().setKeyArgs(resolvedArgs))
+ .build();
+ }
+
+ @Override
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIndex termIndex) {
+ final long trxnLogIndex = termIndex.getIndex();
+
+ DeleteObjectTaggingRequest deleteObjectTaggingRequest = getOmRequest().getDeleteObjectTaggingRequest();
+
+ KeyArgs keyArgs = deleteObjectTaggingRequest.getKeyArgs();
+ String volumeName = keyArgs.getVolumeName();
+ String bucketName = keyArgs.getBucketName();
+ String keyName = keyArgs.getKeyName();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumDeleteObjectTagging();
+
+ Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ boolean acquiredLock = false;
+ OMClientResponse omClientResponse = null;
+ IOException exception = null;
+ Result result = null;
+ try {
+ mergeOmLockDetails(
+ omMetadataManager.getLock()
+ .acquireWriteLock(BUCKET_LOCK, volumeName, bucketName)
+ );
+ acquiredLock = getOmLockDetails().isLockAcquired();
+
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ String dbOzoneKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, keyName);
+
+ OmKeyInfo omKeyInfo =
+ omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+ if (omKeyInfo == null) {
+ throw new OMException("Key not found", KEY_NOT_FOUND);
+ }
+
+ // Clear / delete the tags
+ omKeyInfo.getTags().clear();
+ // Set the UpdateID to the current transactionLogIndex
+ omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ // Note: Key modification time is not changed because S3 last modified
+ // time only changes when there are changes in the object content
+
+ // Update table cache
+ omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+ new CacheKey<>(dbOzoneKey),
+ CacheValue.get(trxnLogIndex, omKeyInfo)
+ );
+
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ omResponse.setDeleteObjectTaggingResponse(DeleteObjectTaggingResponse.newBuilder()).build(),
+ omKeyInfo
+ );
+
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new S3DeleteObjectTaggingResponse(
+ createErrorOMResponse(omResponse, exception),
+ getBucketLayout()
+ );
+ } finally {
+ if (acquiredLock) {
+ mergeOmLockDetails(omMetadataManager.getLock()
+ .releaseWriteLock(BUCKET_LOCK, volumeName, bucketName));
+ }
+ if (omClientResponse != null) {
+ omClientResponse.setOmLockDetails(getOmLockDetails());
+ }
+ }
+
+ markForAudit(ozoneManager.getAuditLogger(), buildAuditMessage(
+ OMAction.DELETE_OBJECT_TAGGING, auditMap, exception, getOmRequest().getUserInfo()
+ ));
+
+ switch (result) {
+ case SUCCESS:
+ LOG.debug("Delete object tagging success. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName);
+ break;
+ case FAILURE:
+ omMetrics.incNumDeleteObjectTaggingFails();
+ LOG.error("Delete object tagging failed. Volume:{}, Bucket:{}, Key:{}.", volumeName,
+ bucketName, keyName, exception);
Review Comment:
We should revisit that.
So far we have tried to get rid of such logging in S3 Gateway, but I think the same principle applies to OM, too.
HDDS-6206
HDDS-6247
HDDS-6559
HDDS-8011
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "guohao-rosicky (via GitHub)" <gi...@apache.org>.
guohao-rosicky commented on PR #6756:
URL: https://github.com/apache/ozone/pull/6756#issuecomment-2145014440
Thanks @ivandika3 for the patch, let's me see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6756:
URL: https://github.com/apache/ozone/pull/6756#issuecomment-2143746571
@SaketaChalamchala @tanvipenumudy @Tejaskriya Could you help take a look as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836283121
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Tagging.java:
##########
@@ -0,0 +1,156 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import javax.ws.rs.WebApplicationException;
+import javax.xml.bind.annotation.XmlAccessType;
+import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * S3 tagging.
+ */
+@XmlAccessorType(XmlAccessType.FIELD)
+@XmlRootElement(name = "Tagging",
+ namespace = "http://s3.amazonaws.com/doc/2006-03-01/")
+public class S3Tagging {
+
+ @XmlElement(name = "TagSet")
+ private TagSet tagSet;
+
+ public S3Tagging() {
+
+ }
+
+ public S3Tagging(TagSet tagSet) {
+ this.tagSet = tagSet;
+ }
+
+ public TagSet getTagSet() {
+ return tagSet;
+ }
+
+ public void setTagSet(TagSet tagSet) {
+ this.tagSet = tagSet;
+ }
+
+ /**
+ * Entity for child element TagSet.
+ */
+ @XmlAccessorType(XmlAccessType.FIELD)
+ @XmlRootElement(name = "TagSet")
+ public static class TagSet {
+ @XmlElement(name = "Tag")
+ private List<Tag> tags = new ArrayList<>();
+
+ public TagSet() {
+ }
+
+ public TagSet(List<Tag> tags) {
+ this.tags = tags;
+ }
+
+ public List<Tag> getTags() {
+ return tags;
+ }
+
+ public void setTags(List<Tag> tags) {
+ this.tags = tags;
+ }
+ }
+
+ /**
+ * Entity for child element Tag.
+ */
+ @XmlAccessorType(XmlAccessType.FIELD)
+ @XmlRootElement(name = "Tag")
+ public static class Tag {
+ @XmlElement(name = "Key")
+ private String key;
+
+ @XmlElement(name = "Value")
+ private String value;
+
+ public Tag() {
+ }
+
+ public Tag(String key, String value) {
+ this.key = key;
+ this.value = value;
+ }
+
+ public String getKey() {
+ return key;
+ }
+
+ public void setKey(String key) {
+ this.key = key;
+ }
+
+ public String getValue() {
+ return value;
+ }
+
+ public void setValue(String value) {
+ this.value = value;
+ }
+ }
+
+ /**
+ * Creates a S3 tagging instance (xml representation) from a Map retrieved
+ * from OM.
+ * @param tagMap Map representing the tags.
+ * @return {@link S3Tagging}
+ */
+ public static S3Tagging fromMap(Map<String, String> tagMap) {
+ List<Tag> tags = tagMap.entrySet()
+ .stream()
+ .map(
+ tagEntry -> new Tag(tagEntry.getKey(), tagEntry.getValue())
+ )
+ .collect(Collectors.toList());
+ return new S3Tagging(new TagSet(tags));
+ }
+
+ /**
+ * Additional XML validation logic for S3 tagging.
+ */
+ public void validate() {
+ if (tagSet == null) {
+ throw new WebApplicationException("TagSet needs to be specified");
+ }
+
+ if (tagSet.getTags().isEmpty()) {
+ throw new WebApplicationException("Tags need to be specified and cannot be empty");
+ }
+
+ for (Tag tag: tagSet.getTags()) {
+ if (tag.getKey() == null) {
+ throw new WebApplicationException("Some tag keys are not specified");
+ }
+ if (tag.getValue() == null) {
+ throw new WebApplicationException("Tag value for tag " + tag.getKey() + " is not specified");
+ }
+ }
+ }
Review Comment:
Thanks for the review. I changed `WebApplicationException` to the `IllegalArgumentException`. Please let me know if the change makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836291834
##########
hadoop-ozone/dist/src/main/smoketest/s3/objecttagging.robot:
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+*** Settings ***
+Documentation S3 gateway test with aws cli
+Library OperatingSystem
+Library String
+Resource ../commonlib.robot
+Resource commonawslib.robot
+Test Timeout 5 minutes
+Suite Setup Setup s3 tests
+
+*** Variables ***
+${ENDPOINT_URL} http://s3g:9878
+${OZONE_TEST} true
+${BUCKET} generated
+
+
+*** Test Cases ***
+
+Put object tagging
Review Comment:
Thanks for the review. I added for case non-existent key. The other cases like empty tag key and empty tag values are usually thrown in the AWS SDK client-side. I added the validation server-side just in case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6756:
URL: https://github.com/apache/ozone/pull/6756#discussion_r1836277806
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -2727,6 +2727,61 @@ public void recoverKey(OmKeyArgs args, long clientID) throws IOException {
ozoneManagerClient.recoverKey(args, clientID);
}
+ @Override
+ public Map<String, String> getObjectTagging(String volumeName, String bucketName, String keyName)
+ throws IOException {
+ if (omVersion.compareTo(OzoneManagerVersion.OBJECT_TAG) < 0) {
+ throw new IOException("OzoneManager does not support object tags");
+ }
Review Comment:
Introduced a new `OzoneManagerVersion`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10655. Support PutObjectTagging, GetObjectTagging, and DeleteObjectTagging [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6756:
URL: https://github.com/apache/ozone/pull/6756#issuecomment-2467742141
@adoroszlai Thanks a lot for reviewing such a large patch.
> In the future I think it would be better to split such huge changes (255KB patch) into sub-tasks. E.g. introduce OM-side feature first, then add usage in S3G; maybe you can come up with more granular items.
Thanks, I'll keep this in mind. In the future I will create a feature branch so that we don't have a situation like https://issues.apache.org/jira/browse/HDDS-11251 that causes dead code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org