This is an automated email from the ASF dual-hosted git repository.
adoroszlai 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 5d68ac93f9 HDDS-8484. Allow Ozone volume name to have special
characters (#4630)
5d68ac93f9 is described below
commit 5d68ac93f9b27f2b1392f00d8ef05eecdf3a6469
Author: DaveTeng0 <[email protected]>
AuthorDate: Sun May 7 23:56:03 2023 -0700
HDDS-8484. Allow Ozone volume name to have special characters (#4630)
---
.../apache/hadoop/ozone/client/rpc/RpcClient.java | 6 +-
.../main/java/org/apache/hadoop/ozone/OmUtils.java | 5 +-
.../om/multitenant/TestMultiTenantVolume.java | 18 ++
.../request/s3/tenant/OMTenantCreateRequest.java | 31 ++--
.../om/request/volume/OMVolumeCreateRequest.java | 3 +-
.../hadoop/ozone/om/TestOMTenantCreateRequest.java | 182 +++++++++++++++++++++
.../request/volume/TestOMVolumeCreateRequest.java | 72 ++++++++
7 files changed, 300 insertions(+), 17 deletions(-)
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 d965fd7b90..268f333d8f 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
@@ -491,7 +491,7 @@ public class RpcClient implements ClientProtocol {
@Override
public void setVolumeQuota(String volumeName, long quotaInNamespace,
long quotaInBytes) throws IOException {
- HddsClientUtils.verifyResourceName(volumeName);
+ verifyVolumeName(volumeName);
verifyCountsQuota(quotaInNamespace);
verifySpaceQuota(quotaInBytes);
// If the volume is old, we need to remind the user on the client side
@@ -707,7 +707,7 @@ public class RpcClient implements ClientProtocol {
private static void verifyVolumeName(String volumeName) throws OMException {
try {
- HddsClientUtils.verifyResourceName(volumeName);
+ HddsClientUtils.verifyResourceName(volumeName, false);
} catch (IllegalArgumentException e) {
throw new OMException(e.getMessage(),
OMException.ResultCodes.INVALID_VOLUME_NAME);
@@ -1095,7 +1095,7 @@ public class RpcClient implements ClientProtocol {
@Override
public void setBucketQuota(String volumeName, String bucketName,
long quotaInNamespace, long quotaInBytes) throws IOException {
- HddsClientUtils.verifyResourceName(volumeName);
+ verifyVolumeName(volumeName);
verifyCountsQuota(quotaInNamespace);
verifySpaceQuota(quotaInBytes);
OmBucketArgs.Builder builder = OmBucketArgs.newBuilder();
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 358f8ff3f9..940fffb603 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
@@ -487,9 +487,10 @@ public final class OmUtils {
/**
* Verify volume name is a valid DNS name.
*/
- public static void validateVolumeName(String volumeName) throws OMException {
+ public static void validateVolumeName(String volumeName, boolean isStrictS3)
+ throws OMException {
try {
- HddsClientUtils.verifyResourceName(volumeName);
+ HddsClientUtils.verifyResourceName(volumeName, isStrictS3);
} catch (IllegalArgumentException e) {
throw new OMException("Invalid volume name: " + volumeName,
OMException.ResultCodes.INVALID_VOLUME_NAME);
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
index 01cebd3d4e..4a0dfcd186 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestMultiTenantVolume.java
@@ -51,6 +51,7 @@ import java.util.concurrent.TimeoutException;
import static
org.apache.hadoop.ozone.admin.scm.FinalizeUpgradeCommandUtil.isDone;
import static
org.apache.hadoop.ozone.admin.scm.FinalizeUpgradeCommandUtil.isStarting;
import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_MULTITENANCY_ENABLED;
+import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* Tests that S3 requests for a tenant are directed to that tenant's volume,
@@ -295,4 +296,21 @@ public class TestMultiTenantVolume {
store.deleteTenant(TENANT_ID);
store.deleteVolume(TENANT_ID);
}
+
+ @Test
+ public void testRejectNonS3CompliantTenantIdCreationWithDefaultStrictS3True()
+ throws Exception {
+ ObjectStore store = getStoreForAccessID(ACCESS_ID);
+ String[] nonS3CompliantTenantId =
+ {"tenantid_underscore", "_tenantid___multi_underscore_", "tenantid_"};
+
+ for (String tenantId : nonS3CompliantTenantId) {
+ OMException e = assertThrows(
+ OMException.class,
+ () -> store.createTenant(tenantId));
+
+ Assert.assertTrue(e.getMessage().contains("Invalid volume name: "
+ + tenantId));
+ }
+ }
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantCreateRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantCreateRequest.java
index 2ff87ab140..2c7158c4f4 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantCreateRequest.java
@@ -57,6 +57,7 @@ import java.util.HashMap;
import java.util.Map;
import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_ALREADY_EXISTS;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.USER_NOT_FOUND;
import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_ALREADY_EXISTS;
import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK;
import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK;
@@ -137,20 +138,11 @@ public class OMTenantCreateRequest extends
OMVolumeRequest {
TENANT_ALREADY_EXISTS);
}
- // getUserName returns:
- // - Kerberos principal when Kerberos security is enabled
- // - User's login name when security is not enabled
- // - AWS_ACCESS_KEY_ID if the original request comes from S3 Gateway.
- // Not Applicable to TenantCreateRequest.
- final UserGroupInformation ugi = ProtobufRpcEngine.Server.getRemoteUser();
- // getShortUserName here follows RpcClient#createVolume
- // A caveat is that this assumes OM's auth_to_local is the same as
- // the client's. Maybe move this logic to the client and pass VolumeArgs?
- final String owner = ugi.getShortUserName();
+ final String owner = getUserName();
// Volume name defaults to tenant name if unspecified in the request
final String volumeName = request.getVolumeName();
// Validate volume name
- OmUtils.validateVolumeName(volumeName);
+ OmUtils.validateVolumeName(volumeName, ozoneManager.isStrictS3());
final String dbVolumeKey = ozoneManager.getMetadataManager()
.getVolumeKey(volumeName);
@@ -361,4 +353,21 @@ public class OMTenantCreateRequest extends OMVolumeRequest
{
}
return omClientResponse;
}
+
+ public String getUserName() throws IOException {
+ // getUserName returns:
+ // - Kerberos principal when Kerberos security is enabled
+ // - User's login name when security is not enabled
+ // - AWS_ACCESS_KEY_ID if the original request comes from S3 Gateway.
+ // Not Applicable to TenantCreateRequest.
+ final UserGroupInformation ugi = ProtobufRpcEngine.Server.getRemoteUser();
+ // getShortUserName here follows RpcClient#createVolume
+ // A caveat is that this assumes OM's auth_to_local is the same as
+ // the client's. Maybe move this logic to the client and pass VolumeArgs?
+ if (ugi != null) {
+ return ugi.getShortUserName();
+ } else {
+ throw new OMException("User name is null.", USER_NOT_FOUND);
+ }
+ }
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java
index ca2a8ff82a..aa6cec9600 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java
@@ -74,7 +74,8 @@ public class OMVolumeCreateRequest extends OMVolumeRequest {
VolumeInfo volumeInfo =
getOmRequest().getCreateVolumeRequest().getVolumeInfo();
// Verify resource name
- OmUtils.validateVolumeName(volumeInfo.getVolume());
+ OmUtils.validateVolumeName(volumeInfo.getVolume(),
+ ozoneManager.isStrictS3());
// Set creation time & set modification time
long initialTime = Time.now();
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMTenantCreateRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMTenantCreateRequest.java
new file mode 100644
index 0000000000..fb95fba6ed
--- /dev/null
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMTenantCreateRequest.java
@@ -0,0 +1,182 @@
+/*
+ * 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.
+ *
+ */
+package org.apache.hadoop.ozone.om;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.AuditMessage;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.multitenant.AuthorizerLock;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
+import org.apache.hadoop.ozone.om.request.s3.tenant.OMTenantCreateRequest;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.ozone.test.LambdaTestUtils;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.rules.TemporaryFolder;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.UUID;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests create tenant request.
+ */
+public class TestOMTenantCreateRequest {
+ @Rule
+ public TemporaryFolder folder = new TemporaryFolder();
+ private OzoneManager ozoneManager;
+ private OMMetrics omMetrics;
+ private OMMetadataManager omMetadataManager;
+ private AuditLogger auditLogger;
+ // Just setting ozoneManagerDoubleBuffer which does nothing.
+ private OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper =
+ ((response, transactionIndex) -> {
+ return null;
+ });
+
+ @Before
+ public void setup() throws Exception {
+ ozoneManager = mock(OzoneManager.class);
+ omMetrics = OMMetrics.create();
+ OzoneConfiguration ozoneConfiguration = new OzoneConfiguration();
+ ozoneConfiguration.set(OMConfigKeys.OZONE_OM_DB_DIRS,
+ folder.newFolder().getAbsolutePath());
+ omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration);
+ when(ozoneManager.getMetrics()).thenReturn(omMetrics);
+ when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
+ when(ozoneManager.getMaxUserVolumeCount()).thenReturn(10L);
+ OMLayoutVersionManager lvm = mock(OMLayoutVersionManager.class);
+ when(lvm.getMetadataLayoutVersion()).thenReturn(0);
+ when(lvm.isAllowed(anyString())).thenReturn(true);
+ when(ozoneManager.getVersionManager()).thenReturn(lvm);
+ when(ozoneManager.isRatisEnabled()).thenReturn(true);
+ auditLogger = mock(AuditLogger.class);
+ when(ozoneManager.getAuditLogger()).thenReturn(auditLogger);
+ Mockito.doNothing().when(auditLogger).logWrite(any(AuditMessage.class));
+
+ OMMultiTenantManager multiTenantManager = mock(OMMultiTenantManager.class);
+ Mockito.doNothing().when(multiTenantManager).checkAdmin();
+ AuthorizerLock authorizerLock = mock(AuthorizerLock.class);
+ Mockito.doNothing().when(authorizerLock).tryWriteLockInOMRequest();
+ when(multiTenantManager.getAuthorizerLock()).thenReturn(authorizerLock);
+ TenantOp tenantOp = mock(TenantOp.class);
+ Mockito.doNothing().when(tenantOp).createTenant(any(), any(), any());
+ when(multiTenantManager.getAuthorizerOp()).thenReturn(tenantOp);
+ when(multiTenantManager.getCacheOp()).thenReturn(tenantOp);
+
+ when(ozoneManager.getMultiTenantManager()).thenReturn(multiTenantManager);
+ }
+
+ @After
+ public void stop() {
+ omMetrics.unRegister();
+ Mockito.framework().clearInlineMocks();
+ }
+
+
+ @Test
+ public void
+ testAcceptS3CompliantTenantIdCreationRegardlessOfStrictS3Setting()
+ throws Exception {
+ boolean[] omStrictS3Configs = {true, false};
+ for (boolean isStrictS3 : omStrictS3Configs) {
+ when(ozoneManager.isStrictS3()).thenReturn(isStrictS3);
+ String tenantId = UUID.randomUUID().toString();
+ acceptTenantIdCreationHelper(tenantId);
+ }
+ }
+
+ @Test
+ public void testRejectNonS3CompliantTenantIdCreationWithStrictS3True()
+ throws Exception {
+ String[] nonS3CompliantTenantId =
+ {"tenantid_underscore", "_tenantid___multi_underscore_", "tenantid_"};
+ when(ozoneManager.isStrictS3()).thenReturn(true);
+ for (String tenantId : nonS3CompliantTenantId) {
+ rejectTenantIdCreationHelper(tenantId);
+ }
+ }
+
+ @Test
+ public void testAcceptNonS3CompliantTenantIdCreationWithStrictS3False()
+ throws Exception {
+ String[] nonS3CompliantTenantId =
+ {"tenantid_underscore", "_tenantid___multi_underscore_", "tenantid_"};
+ when(ozoneManager.isStrictS3()).thenReturn(false);
+ for (String tenantId : nonS3CompliantTenantId) {
+ acceptTenantIdCreationHelper(tenantId);
+ }
+ }
+
+ private void acceptTenantIdCreationHelper(String tenantId)
+ throws Exception {
+ OMRequest originalRequest =
+ OMRequestTestUtils.createTenantRequest(tenantId);
+ OMTenantCreateRequest omTenantCreateRequest =
+ Mockito.spy(new OMTenantCreateRequest(originalRequest));
+ Mockito.doReturn("username").when(omTenantCreateRequest).getUserName();
+
+ OMRequest modifiedRequest = omTenantCreateRequest.preExecute(ozoneManager);
+ omTenantCreateRequest = new OMTenantCreateRequest(modifiedRequest);
+ long txLogIndex = 1;
+ OMClientResponse omClientResponse =
+ omTenantCreateRequest.validateAndUpdateCache(ozoneManager, txLogIndex,
+ ozoneManagerDoubleBufferHelper);
+ OzoneManagerProtocolProtos.OMResponse omResponse =
+ omClientResponse.getOMResponse();
+
+ Assert.assertNotNull(omResponse.getCreateTenantResponse());
+ Assert.assertEquals(OzoneManagerProtocolProtos.Status.OK,
+ omResponse.getStatus());
+ Assert.assertNotNull(omMetadataManager.getVolumeTable().get(
+ omMetadataManager.getVolumeKey(tenantId)));
+ }
+
+ private void rejectTenantIdCreationHelper(String tenantId)
+ throws Exception {
+ // Verify exception thrown on invalid volume name
+ LambdaTestUtils.intercept(OMException.class, "Invalid volume name: "
+ + tenantId,
+ () -> doPreExecute(tenantId));
+ }
+
+ private void doPreExecute(String tenantId) throws Exception {
+ OMRequest originalRequest =
+ OMRequestTestUtils.createTenantRequest(tenantId);
+
+ OMTenantCreateRequest omTenantCreateRequest =
+ Mockito.spy(new OMTenantCreateRequest(originalRequest));
+ Mockito.doReturn("username").when(omTenantCreateRequest).getUserName();
+
+ omTenantCreateRequest.preExecute(ozoneManager);
+ }
+
+}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeCreateRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeCreateRequest.java
index a0b94d1626..164ea386b5 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeCreateRequest.java
@@ -210,6 +210,78 @@ public class TestOMVolumeCreateRequest extends
TestOMVolumeRequest {
omMetadataManager.getVolumeKey(volumeName)));
}
+ @Test
+ public void
+ testAcceptS3CompliantVolumeNameCreationRegardlessOfStrictS3Setting()
+ throws Exception {
+ String adminName = UUID.randomUUID().toString();
+ String ownerName = UUID.randomUUID().toString();
+ boolean[] omStrictS3Configs = {true, false};
+ for (boolean isStrictS3 : omStrictS3Configs) {
+ when(ozoneManager.isStrictS3()).thenReturn(isStrictS3);
+ String volumeName = UUID.randomUUID().toString();
+ acceptVolumeCreationHelper(volumeName, adminName, ownerName);
+ }
+ }
+
+ @Test
+ public void testRejectNonS3CompliantVolumeNameCreationWithStrictS3True()
+ throws Exception {
+ String adminName = UUID.randomUUID().toString();
+ String ownerName = UUID.randomUUID().toString();
+ String[] nonS3CompliantVolumeName =
+ {"volume_underscore", "_volume___multi_underscore_", "volume_"};
+ when(ozoneManager.isStrictS3()).thenReturn(true);
+ for (String volumeName : nonS3CompliantVolumeName) {
+ rejectVolumeCreationHelper(volumeName, adminName, ownerName);
+ }
+ }
+
+ @Test
+ public void testAcceptNonS3CompliantVolumeNameCreationWithStrictS3False()
+ throws Exception {
+ String adminName = UUID.randomUUID().toString();
+ String ownerName = UUID.randomUUID().toString();
+ String[] nonS3CompliantVolumeName =
+ {"volume_underscore", "_volume___multi_underscore_", "volume_"};
+ when(ozoneManager.isStrictS3()).thenReturn(false);
+ for (String volumeName : nonS3CompliantVolumeName) {
+ acceptVolumeCreationHelper(volumeName, adminName, ownerName);
+ }
+ }
+
+ private void acceptVolumeCreationHelper(String volumeName, String adminName,
+ String ownerName)
+ throws Exception {
+ OMRequest originalRequest = createVolumeRequest(volumeName, adminName,
+ ownerName);
+ OMVolumeCreateRequest omVolumeCreateRequest =
+ new OMVolumeCreateRequest(originalRequest);
+ OMRequest modifiedRequest = omVolumeCreateRequest.preExecute(ozoneManager);
+ omVolumeCreateRequest = new OMVolumeCreateRequest(modifiedRequest);
+ long txLogIndex = 1;
+ OMClientResponse omClientResponse =
+ omVolumeCreateRequest.validateAndUpdateCache(ozoneManager, txLogIndex,
+ ozoneManagerDoubleBufferHelper);
+ OzoneManagerProtocolProtos.OMResponse omResponse =
+ omClientResponse.getOMResponse();
+
+ Assert.assertNotNull(omResponse.getCreateVolumeResponse());
+ Assert.assertEquals(OzoneManagerProtocolProtos.Status.OK,
+ omResponse.getStatus());
+ Assert.assertNotNull(omMetadataManager.getVolumeTable().get(
+ omMetadataManager.getVolumeKey(volumeName)));
+ }
+
+ private void rejectVolumeCreationHelper(String volumeName, String adminName,
+ String ownerName)
+ throws Exception {
+ // Verify exception thrown on invalid volume name
+ LambdaTestUtils.intercept(OMException.class, "Invalid volume name: "
+ + volumeName,
+ () -> doPreExecute(volumeName, adminName, ownerName));
+ }
+
private void doPreExecute(String volumeName,
String adminName, String ownerName) throws Exception {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
