From 4de2dc2699fc371b2de83ba55ecbcecef1f0423b Mon Sep 17 00:00:00 2001 From: Anu Engineer Date: Tue, 9 Oct 2018 17:16:52 -0700 Subject: [PATCH] HDDS-568. ozone sh volume info, update, delete operations fail when volume name is not prefixed by /. Contributed by Dinesh Chitlangia. --- .../hadoop/ozone/ozShell/TestOzoneShell.java | 48 +++++++++++++++++++ .../hadoop/ozone/web/ozShell/Handler.java | 28 +++++++++++ .../ozShell/volume/DeleteVolumeHandler.java | 12 +---- .../web/ozShell/volume/InfoVolumeHandler.java | 22 +-------- .../ozShell/volume/UpdateVolumeHandler.java | 12 +---- 5 files changed, 79 insertions(+), 43 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestOzoneShell.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestOzoneShell.java index 6e73b8c5745..d5f25542214 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestOzoneShell.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestOzoneShell.java @@ -283,6 +283,33 @@ public class TestOzoneShell { GenericTestUtils.assertExceptionContains( "Info Volume failed, error:VOLUME_NOT_FOUND", e); } + + + volumeName = "volume" + RandomStringUtils.randomNumeric(5); + volumeArgs = VolumeArgs.newBuilder() + .setOwner("bilbo") + .setQuota("100TB") + .build(); + client.createVolume(volumeName, volumeArgs); + volume = client.getVolumeDetails(volumeName); + assertNotNull(volume); + + //volumeName prefixed with / + String volumeNameWithSlashPrefix = "/" + volumeName; + args = new String[] {"volume", "delete", + url + "/" + volumeNameWithSlashPrefix}; + execute(shell, args); + output = out.toString(); + assertTrue(output.contains("Volume " + volumeName + " is deleted")); + + // verify if volume has been deleted + try { + client.getVolumeDetails(volumeName); + fail("Get volume call should have thrown."); + } catch (IOException e) { + GenericTestUtils.assertExceptionContains( + "Info Volume failed, error:VOLUME_NOT_FOUND", e); + } } @Test @@ -295,6 +322,7 @@ public class TestOzoneShell { .build(); client.createVolume(volumeName, volumeArgs); + //volumeName supplied as-is String[] args = new String[] {"volume", "info", url + "/" + volumeName}; execute(shell, args); @@ -303,6 +331,17 @@ public class TestOzoneShell { assertTrue(output.contains("createdOn") && output.contains(OzoneConsts.OZONE_TIME_ZONE)); + //volumeName prefixed with / + String volumeNameWithSlashPrefix = "/" + volumeName; + args = new String[] {"volume", "info", + url + "/" + volumeNameWithSlashPrefix}; + execute(shell, args); + + output = out.toString(); + assertTrue(output.contains(volumeName)); + assertTrue(output.contains("createdOn") + && output.contains(OzoneConsts.OZONE_TIME_ZONE)); + // test infoVolume with invalid volume name args = new String[] {"volume", "info", url + "/" + volumeName + "/invalid-name"}; @@ -365,6 +404,15 @@ public class TestOzoneShell { vol = client.getVolumeDetails(volumeName); assertEquals(newUser, vol.getOwner()); + //volume with / prefix + String volumeWithPrefix = "/" + volumeName; + String newUser2 = "new-user2"; + args = new String[] {"volume", "update", url + "/" + volumeWithPrefix, + "--user", newUser2}; + execute(shell, args); + vol = client.getVolumeDetails(volumeName); + assertEquals(newUser2, vol.getOwner()); + // test error conditions args = new String[] {"volume", "update", url + "/invalid-volume", "--user", newUser}; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/Handler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/Handler.java index a9550c2e26f..c8549c95803 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/Handler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/Handler.java @@ -21,6 +21,8 @@ package org.apache.hadoop.ozone.web.ozShell; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.concurrent.Callable; import org.apache.hadoop.conf.Configuration; @@ -153,6 +155,32 @@ public abstract class Handler implements Callable { } } + /** + * + * @param uri + * @return volumeName + * @throws Exception + * @throws OzoneClientException when uri is null or invalid volume name + */ + protected String parseVolumeName(String uri) throws Exception{ + URI ozoneURI = verifyURI(uri); + Path path = Paths.get(ozoneURI.getPath()); + int pathNameCount = path.getNameCount(); + if (pathNameCount != 1) { + String errorMessage; + if (pathNameCount < 1) { + errorMessage = "Volume name is required to perform volume " + + "operations like info, update, create and delete. "; + } else { + errorMessage = "Invalid volume name. Delimiters (/) not allowed in " + + "volume name"; + } + throw new OzoneClientException(errorMessage); + } + + return ozoneURI.getPath().replaceAll("^/+", ""); + } + public boolean isVerbose() { return parent.isVerbose(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/DeleteVolumeHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/DeleteVolumeHandler.java index d1e96fcf74f..d757e20b494 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/DeleteVolumeHandler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/DeleteVolumeHandler.java @@ -18,9 +18,6 @@ package org.apache.hadoop.ozone.web.ozShell.volume; -import java.net.URI; - -import org.apache.hadoop.ozone.client.OzoneClientException; import org.apache.hadoop.ozone.web.ozShell.Handler; import org.apache.hadoop.ozone.web.ozShell.Shell; @@ -43,14 +40,7 @@ public class DeleteVolumeHandler extends Handler { @Override public Void call() throws Exception { - URI ozoneURI = verifyURI(uri); - if (ozoneURI.getPath().isEmpty()) { - throw new OzoneClientException( - "Volume name is required to delete a volume"); - } - - // we need to skip the slash in the URI path - String volumeName = ozoneURI.getPath().substring(1); + String volumeName = parseVolumeName(uri); if (isVerbose()) { System.out.printf("Volume name : %s%n", volumeName); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/InfoVolumeHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/InfoVolumeHandler.java index 16c0342818f..48ed9f6fc2a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/InfoVolumeHandler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/InfoVolumeHandler.java @@ -18,11 +18,6 @@ package org.apache.hadoop.ozone.web.ozShell.volume; -import java.net.URI; -import java.nio.file.Path; -import java.nio.file.Paths; - -import org.apache.hadoop.ozone.client.OzoneClientException; import org.apache.hadoop.ozone.client.OzoneClientUtils; import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.web.ozShell.Handler; @@ -48,22 +43,7 @@ public class InfoVolumeHandler extends Handler{ @Override public Void call() throws Exception { - URI ozoneURI = verifyURI(uri); - Path path = Paths.get(ozoneURI.getPath()); - int pathNameCount = path.getNameCount(); - if (pathNameCount != 1) { - String errorMessage; - if (pathNameCount < 1) { - errorMessage = "Volume name is required to get info of a volume"; - } else { - errorMessage = "Invalid volume name. Delimiters (/) not allowed in " + - "volume name"; - } - throw new OzoneClientException(errorMessage); - } - - // we need to skip the slash in the URI path - String volumeName = ozoneURI.getPath().substring(1); + String volumeName = parseVolumeName(uri); OzoneVolume vol = client.getObjectStore().getVolume(volumeName); System.out.printf("%s%n", JsonUtils.toJsonStringWithDefaultPrettyPrinter( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/UpdateVolumeHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/UpdateVolumeHandler.java index 0336fc2bbf8..46803d83ac4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/UpdateVolumeHandler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/UpdateVolumeHandler.java @@ -25,13 +25,10 @@ import picocli.CommandLine.Parameters; import org.apache.hadoop.hdds.client.OzoneQuota; import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.client.OzoneClientUtils; -import org.apache.hadoop.ozone.client.OzoneClientException; import org.apache.hadoop.ozone.web.ozShell.Handler; import org.apache.hadoop.ozone.web.ozShell.Shell; import org.apache.hadoop.ozone.web.utils.JsonUtils; -import java.net.URI; - /** * Executes update volume calls. */ @@ -57,14 +54,7 @@ public class UpdateVolumeHandler extends Handler { @Override public Void call() throws Exception { - URI ozoneURI = verifyURI(uri); - if (ozoneURI.getPath().isEmpty()) { - throw new OzoneClientException( - "Volume name is required to update a volume"); - } - - // we need to skip the slash in the URI path - String volumeName = ozoneURI.getPath().substring(1); + String volumeName = parseVolumeName(uri); OzoneVolume volume = client.getObjectStore().getVolume(volumeName); if (quota != null && !quota.isEmpty()) {