diff --git a/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java b/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java index 7f80d4732114..a18d008ab8eb 100644 --- a/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java +++ b/hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java @@ -362,6 +362,7 @@ void testVerifyKeyName() throws IllegalArgumentException { for (String name : validNames) { HddsClientUtils.verifyKeyName(name); + HddsClientUtils.verifyKeyName(name + OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX); } } diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 1544492cd72b..f240d0cfbd45 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -3611,16 +3611,6 @@ Thresholds for printing slow-operation audit logs. - - ozone.om.keyname.character.check.enabled - OZONE, OM - false - If true, then enable to check if the key name - contains illegal characters when creating/renaming key. - For the definition of illegal characters, follow the - rules in Amazon S3's object key naming guide. - - ozone.om.key.path.lock.enabled diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 293437c195f4..77af5a9176be 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -122,7 +122,7 @@ import org.apache.hadoop.ozone.client.io.OzoneInputStream; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; -import org.apache.hadoop.ozone.om.OMConfigKeys; +import org.apache.hadoop.ozone.om.OmConfig; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BasicOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; @@ -294,9 +294,8 @@ public RpcClient(ConfigurationSource conf, String omServiceId) topologyAwareReadEnabled = conf.getBoolean( OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_KEY, OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_DEFAULT); - checkKeyNameEnabled = conf.getBoolean( - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + checkKeyNameEnabled = conf.getObject(OmConfig.class) + .isKeyNameCharacterCheckEnabled(); getLatestVersionLocation = conf.getBoolean( OzoneConfigKeys.OZONE_CLIENT_KEY_LATEST_VERSION_LOCATION, OzoneConfigKeys.OZONE_CLIENT_KEY_LATEST_VERSION_LOCATION_DEFAULT); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 1a7cafd8ee27..5314a8488455 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -323,11 +323,6 @@ public final class OMConfigKeys { public static final String OZONE_OM_SECURITY_ADMIN_PROTOCOL_ACL = "ozone.om.security.admin.protocol.acl"; - public static final String OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY = - "ozone.om.keyname.character.check.enabled"; - public static final boolean OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT = - false; - @Deprecated public static final String OZONE_OM_ENABLE_FILESYSTEM_PATHS = OmConfig.Keys.ENABLE_FILESYSTEM_PATHS; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OmConfig.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OmConfig.java index 2c3e94c9116d..a4ef5827387a 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OmConfig.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OmConfig.java @@ -52,6 +52,17 @@ public class OmConfig extends ReconfigurableConfig { ) private boolean fileSystemPathEnabled; + @Config( + key = "ozone.om.keyname.character.check.enabled", + defaultValue = "false", + description = "If true, then enable to check if the key name " + + "contains illegal characters when creating/renaming key. " + + "For the definition of illegal characters, follow the " + + "rules in Amazon S3's object key naming guide.", + tags = { ConfigTag.OM, ConfigTag.OZONE } + ) + private boolean keyNameCharacterCheckEnabled; + @Config( key = "server.list.max.size", defaultValue = "1000", @@ -118,6 +129,14 @@ public void setFileSystemPathEnabled(boolean newValue) { fileSystemPathEnabled = newValue; } + public boolean isKeyNameCharacterCheckEnabled() { + return keyNameCharacterCheckEnabled; + } + + public void setKeyNameCharacterCheckEnabled(boolean newValue) { + this.keyNameCharacterCheckEnabled = newValue; + } + public long getMaxListSize() { return maxListSize; } @@ -183,6 +202,7 @@ public OmConfig copy() { public void setFrom(OmConfig other) { fileSystemPathEnabled = other.fileSystemPathEnabled; + keyNameCharacterCheckEnabled = other.keyNameCharacterCheckEnabled; maxListSize = other.maxListSize; maxUserVolumeCount = other.maxUserVolumeCount; userDefaultRights = other.userDefaultRights; diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/TestOmConfig.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/TestOmConfig.java index f02f4bc57d08..6703da0e50d5 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/TestOmConfig.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/TestOmConfig.java @@ -77,6 +77,7 @@ void testSetFrom() { OmConfig subject = conf.getObject(OmConfig.class); OmConfig updated = conf.getObject(OmConfig.class); updated.setFileSystemPathEnabled(!updated.isFileSystemPathEnabled()); + updated.setKeyNameCharacterCheckEnabled(!updated.isKeyNameCharacterCheckEnabled()); updated.setMaxListSize(updated.getMaxListSize() + 1); updated.setMaxUserVolumeCount(updated.getMaxUserVolumeCount() + 1); @@ -88,6 +89,7 @@ void testSetFrom() { private static void assertConfigEquals(OmConfig expected, OmConfig actual) { assertEquals(expected.getMaxListSize(), actual.getMaxListSize()); assertEquals(expected.isFileSystemPathEnabled(), actual.isFileSystemPathEnabled()); + assertEquals(expected.isKeyNameCharacterCheckEnabled(), actual.isKeyNameCharacterCheckEnabled()); assertEquals(expected.getMaxUserVolumeCount(), actual.getMaxUserVolumeCount()); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java index a877374cdbe0..4e6ac64edcd2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java @@ -31,6 +31,7 @@ import java.nio.file.Paths; import java.util.List; import java.util.Map; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; @@ -95,9 +96,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { Preconditions.checkNotNull(createDirectoryRequest); KeyArgs keyArgs = createDirectoryRequest.getKeyArgs(); - ValidateKeyArgs validateArgs = new ValidateKeyArgs.Builder() - .setSnapshotReservedWord(keyArgs.getKeyName()).build(); - validateKey(ozoneManager, validateArgs); + + OmUtils.verifyKeyNameWithSnapshotReservedWord(keyArgs.getKeyName()); KeyArgs.Builder newKeyArgs = createDirectoryRequest.getKeyArgs() .toBuilder().setModificationTime(Time.now()); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java index f7f38474fda1..b8812ddda99b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java @@ -31,12 +31,11 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.hdds.utils.UniqueId; -import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; @@ -88,11 +87,11 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { Preconditions.checkNotNull(createFileRequest); KeyArgs keyArgs = createFileRequest.getKeyArgs(); - ValidateKeyArgs validateArgs = new ValidateKeyArgs.Builder() - .setSnapshotReservedWord(keyArgs.getKeyName()) - .setKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), - OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)).build(); - validateKey(ozoneManager, validateArgs); + + OmUtils.verifyKeyNameWithSnapshotReservedWord(keyArgs.getKeyName()); + if (ozoneManager.getConfig().isKeyNameCharacterCheckEnabled()) { + OmUtils.validateKeyName(keyArgs.getKeyName()); + } UserInfo userInfo = getUserInfo(); if (keyArgs.getKeyName().isEmpty()) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index b5fad1ade600..fc13f0462e8b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -33,7 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.OzoneManagerVersion; import org.apache.hadoop.ozone.audit.AuditLogger; @@ -95,10 +95,9 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { ozoneManager.checkFeatureEnabled(OzoneManagerVersion.ATOMIC_REWRITE_KEY); } - ValidateKeyArgs validateArgs = new ValidateKeyArgs.Builder() - .setKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), - OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)).build(); - validateKey(ozoneManager, validateArgs); + if (ozoneManager.getConfig().isKeyNameCharacterCheckEnabled()) { + OmUtils.validateKeyName(keyArgs.getKeyName()); + } boolean isHsync = commitKeyRequest.hasHsync() && commitKeyRequest.getHsync(); boolean isRecovery = commitKeyRequest.hasRecovery() && commitKeyRequest.getRecovery(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java index 477b9d8a9289..054c3a3cef71 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java @@ -33,6 +33,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.hdds.utils.UniqueId; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneManagerVersion; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; @@ -92,10 +93,11 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { ozoneManager.checkFeatureEnabled(OzoneManagerVersion.ATOMIC_REWRITE_KEY); } - ValidateKeyArgs validateArgs = new ValidateKeyArgs.Builder() - .setSnapshotReservedWord(keyArgs.getKeyName()) - .setKeyName(keyArgs.getKeyName()).build(); - validateKey(ozoneManager, validateArgs); + OmUtils.verifyKeyNameWithSnapshotReservedWord(keyArgs.getKeyName()); + if (ozoneManager.getConfig().isKeyNameCharacterCheckEnabled()) { + OmUtils.validateKeyName(keyArgs.getKeyName()); + } + String keyPath = keyArgs.getKeyName(); keyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(), diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java index ae10fb932c8b..f1d71d99fdfe 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java @@ -27,6 +27,7 @@ import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; @@ -76,9 +77,10 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { Preconditions.checkNotNull(renameKeyRequest); KeyArgs renameKeyArgs = renameKeyRequest.getKeyArgs(); - ValidateKeyArgs validateArgs = new ValidateKeyArgs.Builder() - .setKeyName(renameKeyRequest.getToKeyName()).build(); - validateKey(ozoneManager, validateArgs); + + if (ozoneManager.getConfig().isKeyNameCharacterCheckEnabled()) { + OmUtils.validateKeyName(renameKeyRequest.getToKeyName()); + } String srcKey = extractSrcKey(renameKeyArgs); String dstKey = extractDstKey(renameKeyRequest); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index 133af085dfe0..ad21d8330811 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -64,7 +64,6 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneConsts; -import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; import org.apache.hadoop.ozone.om.OmConfig; @@ -178,80 +177,6 @@ protected KeyArgs resolveBucketAndCheckOpenKeyAcls(KeyArgs keyArgs, return resolvedArgs; } - /** - * Define the parameters carried when verifying the Key. - */ - public static class ValidateKeyArgs { - private String snapshotReservedWord; - private String keyName; - private boolean validateSnapshotReserved; - private boolean validateKeyName; - - ValidateKeyArgs(String snapshotReservedWord, String keyName, - boolean validateSnapshotReserved, boolean validateKeyName) { - this.snapshotReservedWord = snapshotReservedWord; - this.keyName = keyName; - this.validateSnapshotReserved = validateSnapshotReserved; - this.validateKeyName = validateKeyName; - } - - public String getSnapshotReservedWord() { - return snapshotReservedWord; - } - - public String getKeyName() { - return keyName; - } - - public boolean isValidateSnapshotReserved() { - return validateSnapshotReserved; - } - - public boolean isValidateKeyName() { - return validateKeyName; - } - - /** - * Tools for building {@link ValidateKeyArgs}. - */ - public static class Builder { - private String snapshotReservedWord; - private String keyName; - private boolean validateSnapshotReserved; - private boolean validateKeyName; - - public Builder setSnapshotReservedWord(String snapshotReservedWord) { - this.snapshotReservedWord = snapshotReservedWord; - this.validateSnapshotReserved = true; - return this; - } - - public Builder setKeyName(String keyName) { - this.keyName = keyName; - this.validateKeyName = true; - return this; - } - - public ValidateKeyArgs build() { - return new ValidateKeyArgs(snapshotReservedWord, keyName, - validateSnapshotReserved, validateKeyName); - } - } - } - - protected void validateKey(OzoneManager ozoneManager, ValidateKeyArgs validateKeyArgs) - throws OMException { - if (validateKeyArgs.isValidateSnapshotReserved()) { - OmUtils.verifyKeyNameWithSnapshotReservedWord(validateKeyArgs.getSnapshotReservedWord()); - } - final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() - .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); - if (validateKeyArgs.isValidateKeyName() && checkKeyNameEnabled) { - OmUtils.validateKeyName(validateKeyArgs.getKeyName()); - } - } - /** * This methods avoids multiple rpc calls to SCM by allocating multiple blocks * in one rpc call. diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java index 4354ef8b1256..1c11395e6947 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java @@ -18,11 +18,7 @@ package org.apache.hadoop.ozone.om.request.key; import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.setupReplicationConfigValidation; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyLong; @@ -93,7 +89,6 @@ import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.mockito.Mockito; import org.slf4j.event.Level; @@ -338,21 +333,4 @@ protected SnapshotInfo createSnapshot(String volume, String bucket, String snaps return snapshotInfo; } - @Test - public void testValidateKeyArgs() { - OMKeyRequest.ValidateKeyArgs validateKeyArgs1 = new OMKeyRequest.ValidateKeyArgs.Builder() - .setKeyName("tmpKey").setSnapshotReservedWord("tmpSnapshotReservedWord").build(); - assertEquals("tmpSnapshotReservedWord", validateKeyArgs1.getSnapshotReservedWord()); - assertEquals("tmpKey", validateKeyArgs1.getKeyName()); - assertTrue(validateKeyArgs1.isValidateKeyName()); - assertTrue(validateKeyArgs1.isValidateSnapshotReserved()); - - OMKeyRequest.ValidateKeyArgs validateKeyArgs2 = new OMKeyRequest.ValidateKeyArgs.Builder() - .setKeyName("tmpKey2").build(); - assertNull(validateKeyArgs2.getSnapshotReservedWord()); - assertEquals("tmpKey2", validateKeyArgs2.getKeyName()); - assertTrue(validateKeyArgs2.isValidateKeyName()); - assertFalse(validateKeyArgs2.isValidateSnapshotReserved()); - } - }