HDDS-13262. Simplify key name validation

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);
     }
   }
 

hadoop-hdds/common/src/main/resources/ozone-default.xml

@@ -3611,16 +3611,6 @@
       Thresholds for printing slow-operation audit logs.
     </description>
   </property>
-  <property>
-    <name>ozone.om.keyname.character.check.enabled</name>
-    <tag>OZONE, OM</tag>
-    <value>false</value>
-    <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.
-    </description>
-  </property>
 
   <property>
     <name>ozone.om.key.path.lock.enabled</name>

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);

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;

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;

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());
   }
 

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());

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()) {

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();

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(),

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);

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.

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());
-  }
-
 }