This is an automated email from the ASF dual-hosted git repository.
hemant pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new d83f434274 HDDS-9423. Throw appropriate error messages when deleting a
file in .snapshot path (#5814)
d83f434274 is described below
commit d83f434274b9c2d3fe4b7bdcff0b1862a8291c7d
Author: Hemant Kumar <[email protected]>
AuthorDate: Mon Dec 18 23:17:40 2023 -0800
HDDS-9423. Throw appropriate error messages when deleting a file in
.snapshot path (#5814)
---
.../main/java/org/apache/hadoop/ozone/OmUtils.java | 31 +++++++++--
.../ozone/om/request/key/OMKeyDeleteRequest.java | 4 +-
.../om/request/file/TestOMFileCreateRequest.java | 64 ++++++++--------------
.../om/request/key/TestOMKeyDeleteRequest.java | 40 ++++++++++----
.../ozone/BasicRootedOzoneClientAdapterImpl.java | 7 ++-
.../hadoop/ozone/shell/keys/DeleteKeyHandler.java | 9 +++
6 files changed, 95 insertions(+), 60 deletions(-)
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
index babeb30548..f23a703bd0 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
@@ -629,15 +629,36 @@ public final class OmUtils {
if (keyName.substring(OM_SNAPSHOT_INDICATOR.length())
.startsWith(OM_KEY_PREFIX)) {
throw new OMException(
- "Cannot create key under path reserved for "
- + "snapshot: " + OM_SNAPSHOT_INDICATOR + OM_KEY_PREFIX,
+ "Cannot create key under path reserved for snapshot: " +
OM_SNAPSHOT_INDICATOR + OM_KEY_PREFIX,
OMException.ResultCodes.INVALID_KEY_NAME);
}
} else {
- // We checked for startsWith OM_SNAPSHOT_INDICATOR and the length is
+ // We checked for startsWith OM_SNAPSHOT_INDICATOR, and the length is
// the same, so it must be equal OM_SNAPSHOT_INDICATOR.
- throw new OMException(
- "Cannot create key with reserved name: " + OM_SNAPSHOT_INDICATOR,
+ throw new OMException("Cannot create key with reserved name: " +
OM_SNAPSHOT_INDICATOR,
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ }
+ }
+
+ /**
+ * Verify if key name contains snapshot reserved word.
+ * This is similar to verifyKeyNameWithSnapshotReservedWord. The only
difference is exception message.
+ */
+ public static void verifyKeyNameWithSnapshotReservedWordForDeletion(String
keyName) throws OMException {
+ if (keyName != null &&
+ keyName.startsWith(OM_SNAPSHOT_INDICATOR)) {
+ if (keyName.length() > OM_SNAPSHOT_INDICATOR.length()) {
+ if (keyName.substring(OM_SNAPSHOT_INDICATOR.length())
+ .startsWith(OM_KEY_PREFIX)) {
+ throw new OMException(
+ "Cannot delete key under path reserved for snapshot: " +
OM_SNAPSHOT_INDICATOR + OM_KEY_PREFIX,
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ } else {
+ // We checked for startsWith OM_SNAPSHOT_INDICATOR, and the length is
+ // the same, so it must be equal OM_SNAPSHOT_INDICATOR.
+ throw new OMException("Cannot delete key with reserved name: " +
OM_SNAPSHOT_INDICATOR,
OMException.ResultCodes.INVALID_KEY_NAME);
}
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
index 9fefd70a2d..0998d00175 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.nio.file.InvalidPathException;
import java.util.Map;
+import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
@@ -76,8 +77,9 @@ public class OMKeyDeleteRequest extends OMKeyRequest {
Preconditions.checkNotNull(deleteKeyRequest);
OzoneManagerProtocolProtos.KeyArgs keyArgs = deleteKeyRequest.getKeyArgs();
-
String keyPath = keyArgs.getKeyName();
+
+ OmUtils.verifyKeyNameWithSnapshotReservedWordForDeletion(keyPath);
keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
keyPath, getBucketLayout());
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
index dbd2a80964..0a7a352b38 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
@@ -23,8 +23,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;
-import java.util.Map;
-import java.util.HashMap;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.OzoneAcl;
@@ -41,15 +39,12 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.ozone.om.request.key.TestOMKeyRequest;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .CreateFileRequest;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .KeyArgs;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .OMRequest;
-
-import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
-import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CreateFileRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.VOLUME_NOT_FOUND;
import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.BUCKET_NOT_FOUND;
import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.FILE_ALREADY_EXISTS;
@@ -481,40 +476,25 @@ public class TestOMFileCreateRequest extends
TestOMKeyRequest {
}
}
- @Test
- public void testPreExecuteWithInvalidKeyPrefix() throws Exception {
- Map<String, String> invalidKeyScenarios = new HashMap<String, String>() {
- {
- put(OM_SNAPSHOT_INDICATOR + "/" + keyName,
- "Cannot create key under path reserved for snapshot: "
- + OM_SNAPSHOT_INDICATOR + OM_KEY_PREFIX);
- put(OM_SNAPSHOT_INDICATOR + "/a/" + keyName,
- "Cannot create key under path reserved for snapshot: "
- + OM_SNAPSHOT_INDICATOR + OM_KEY_PREFIX);
- put(OM_SNAPSHOT_INDICATOR + "/a/b" + keyName,
- "Cannot create key under path reserved for snapshot: "
- + OM_SNAPSHOT_INDICATOR + OM_KEY_PREFIX);
- put(OM_SNAPSHOT_INDICATOR,
- "Cannot create key with reserved name: " + OM_SNAPSHOT_INDICATOR);
- }
- };
-
- for (Map.Entry<String, String> entry : invalidKeyScenarios.entrySet()) {
- String invalidKeyName = entry.getKey();
- String expectedErrorMessage = entry.getValue();
+ @ParameterizedTest
+ @CsvSource(value = {
+ ".snapshot/keyName,Cannot create key under path reserved for snapshot:
.snapshot/",
+ ".snapshot/a/keyName,Cannot create key under path reserved for snapshot:
.snapshot/",
+ ".snapshot/a/b/keyName,Cannot create key under path reserved for
snapshot: .snapshot/",
+ ".snapshot,Cannot create key with reserved name: .snapshot"})
+ public void testPreExecuteWithInvalidKeyPrefix(String invalidKeyName,
+ String expectedErrorMessage) {
- OMRequest omRequest = createFileRequest(volumeName, bucketName,
- invalidKeyName, HddsProtos.ReplicationFactor.ONE,
- HddsProtos.ReplicationType.RATIS, false, false);
-
- OMFileCreateRequest omFileCreateRequest =
- getOMFileCreateRequest(omRequest);
+ OMRequest omRequest = createFileRequest(volumeName, bucketName,
+ invalidKeyName, HddsProtos.ReplicationFactor.ONE,
+ HddsProtos.ReplicationType.RATIS, false, false);
- OMException ex = Assertions.assertThrows(OMException.class,
- () -> omFileCreateRequest.preExecute(ozoneManager));
+ OMFileCreateRequest omFileCreateRequest =
+ getOMFileCreateRequest(omRequest);
- Assertions.assertTrue(ex.getMessage().contains(expectedErrorMessage));
- }
+ OMException ex = Assertions.assertThrows(OMException.class,
+ () -> omFileCreateRequest.preExecute(ozoneManager));
+ Assertions.assertTrue(ex.getMessage().contains(expectedErrorMessage));
}
protected void testNonRecursivePath(String key,
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java
index fe84e3cfbe..907022cedd 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java
@@ -20,6 +20,7 @@ package org.apache.hadoop.ozone.om.request.key;
import java.util.UUID;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
import org.junit.jupiter.api.Assertions;
@@ -28,21 +29,36 @@ import org.junit.jupiter.api.Test;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .DeleteKeyRequest;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .OMRequest;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .KeyArgs;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Tests OmKeyDelete request.
*/
public class TestOMKeyDeleteRequest extends TestOMKeyRequest {
- @Test
- public void testPreExecute() throws Exception {
- doPreExecute(createDeleteKeyRequest());
+ @ParameterizedTest
+ @ValueSource(strings = {"keyName", "a/b/keyName", "a/.snapshot/keyName",
"a.snapshot/b/keyName"})
+ public void testPreExecute(String testKeyName) throws Exception {
+ doPreExecute(createDeleteKeyRequest(testKeyName));
+ }
+
+ @ParameterizedTest
+ @CsvSource(value = {".snapshot,Cannot delete key with reserved name:
.snapshot",
+ ".snapshot/snapName,Cannot delete key under path reserved for snapshot:
.snapshot/",
+ ".snapshot/snapName/keyName,Cannot delete key under path reserved for
snapshot: .snapshot/"})
+ public void testPreExecuteFailure(String testKeyName,
+ String expectedExceptionMessage) {
+ OMKeyDeleteRequest deleteKeyRequest =
+ getOmKeyDeleteRequest(createDeleteKeyRequest(testKeyName));
+ OMException omException = Assertions.assertThrows(OMException.class,
+ () -> deleteKeyRequest.preExecute(ozoneManager));
+ Assertions.assertEquals(expectedExceptionMessage,
omException.getMessage());
+ Assertions.assertEquals(OMException.ResultCodes.INVALID_KEY_NAME,
omException.getResult());
}
@Test
@@ -154,8 +170,12 @@ public class TestOMKeyDeleteRequest extends
TestOMKeyRequest {
* @return OMRequest
*/
private OMRequest createDeleteKeyRequest() {
+ return createDeleteKeyRequest(keyName);
+ }
+
+ private OMRequest createDeleteKeyRequest(String testKeyName) {
KeyArgs keyArgs = KeyArgs.newBuilder().setBucketName(bucketName)
- .setVolumeName(volumeName).setKeyName(keyName).build();
+ .setVolumeName(volumeName).setKeyName(testKeyName).build();
DeleteKeyRequest deleteKeyRequest =
DeleteKeyRequest.newBuilder().setKeyArgs(keyArgs).build();
diff --git
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
index e565c2bedf..193e080f0e 100644
---
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
+++
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
@@ -40,6 +40,7 @@ import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
+import org.apache.hadoop.fs.PathPermissionException;
import org.apache.hadoop.fs.SafeModeAction;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdds.client.ReplicationConfig;
@@ -545,13 +546,15 @@ public class BasicRootedOzoneClientAdapterImpl
bucket.deleteDirectory(keyName, recursive);
return true;
} catch (OMException ome) {
- LOG.error("delete key failed {}", ome.getMessage());
+ LOG.error("Delete key failed. {}", ome.getMessage());
if (OMException.ResultCodes.DIRECTORY_NOT_EMPTY == ome.getResult()) {
throw new PathIsNotEmptyDirectoryException(ome.getMessage());
+ } else if (OMException.ResultCodes.INVALID_KEY_NAME == ome.getResult()) {
+ throw new PathPermissionException(ome.getMessage());
}
return false;
} catch (IOException ioe) {
- LOG.error("delete key failed " + ioe.getMessage());
+ LOG.error("Delete key failed. {}", ioe.getMessage());
return false;
}
}
diff --git
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/DeleteKeyHandler.java
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/DeleteKeyHandler.java
index 5e56cda478..d1a6a4e156 100644
---
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/DeleteKeyHandler.java
+++
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/DeleteKeyHandler.java
@@ -19,11 +19,13 @@
package org.apache.hadoop.ozone.shell.keys;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneClientException;
import org.apache.hadoop.ozone.client.OzoneKeyDetails;
import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.apache.hadoop.ozone.shell.OzoneAddress;
import org.apache.hadoop.ozone.om.OMConfigKeys;
@@ -59,6 +61,13 @@ public class DeleteKeyHandler extends KeyHandler {
OzoneBucket bucket = vol.getBucket(bucketName);
String keyName = address.getKeyName();
+ try {
+ OmUtils.verifyKeyNameWithSnapshotReservedWordForDeletion(keyName);
+ } catch (OMException omException) {
+ out().printf("Operation not permitted: %s %n", omException.getMessage());
+ return;
+ }
+
if (bucket.getBucketLayout().isFileSystemOptimized()) {
// Handle FSO delete key which supports trash also
deleteFSOKey(bucket, keyName);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
