Browse Source

HADOOP-15845. Require explicit URI on CLI for s3guard init and destroy. Contributed by Gabor Bota.

Sean Mackrory 6 years ago
parent
commit
1a25bbe9ec

+ 28 - 4
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java

@@ -218,7 +218,8 @@ public abstract class S3GuardTool extends Configured implements Tool {
     format.addOptionWithValue(SECONDS_FLAG);
   }
 
-  protected void checkMetadataStoreUri(List<String> paths) throws IOException {
+  protected void checkIfS3BucketIsGuarded(List<String> paths)
+      throws IOException {
     // be sure that path is provided in params, so there's no IOoBE
     String s3Path = "";
     if(!paths.isEmpty()) {
@@ -239,6 +240,23 @@ public abstract class S3GuardTool extends Configured implements Tool {
     }
   }
 
+  /**
+   * Check if bucket or DDB table name is set.
+   */
+  protected void checkBucketNameOrDDBTableNameProvided(List<String> paths) {
+    String s3Path = null;
+    if(!paths.isEmpty()) {
+      s3Path = paths.get(0);
+    }
+
+    String metadataStoreUri = getCommandFormat().getOptValue(META_FLAG);
+
+    if(metadataStoreUri == null && s3Path == null) {
+      throw invalidArgs("S3 bucket url or DDB table name have to be provided "
+          + "explicitly to use " + getName() + " command.");
+    }
+  }
+
   /**
    * Parse metadata store from command line option or HDFS configuration.
    *
@@ -433,6 +451,12 @@ public abstract class S3GuardTool extends Configured implements Tool {
     @Override
     public int run(String[] args, PrintStream out) throws Exception {
       List<String> paths = parseArgs(args);
+      try {
+        checkBucketNameOrDDBTableNameProvided(paths);
+      } catch (ExitUtil.ExitException e) {
+        errorln(USAGE);
+        throw e;
+      }
 
       String readCap = getCommandFormat().getOptValue(READ_FLAG);
       if (readCap != null && !readCap.isEmpty()) {
@@ -521,7 +545,7 @@ public abstract class S3GuardTool extends Configured implements Tool {
     public int run(String[] args, PrintStream out) throws Exception {
       List<String> paths = parseArgs(args);
       Map<String, String> options = new HashMap<>();
-      checkMetadataStoreUri(paths);
+      checkIfS3BucketIsGuarded(paths);
 
       String readCap = getCommandFormat().getOptValue(READ_FLAG);
       if (StringUtils.isNotEmpty(readCap)) {
@@ -592,14 +616,14 @@ public abstract class S3GuardTool extends Configured implements Tool {
     public int run(String[] args, PrintStream out) throws Exception {
       List<String> paths = parseArgs(args);
       try {
+        checkBucketNameOrDDBTableNameProvided(paths);
+        checkIfS3BucketIsGuarded(paths);
         parseDynamoDBRegion(paths);
       } catch (ExitUtil.ExitException e) {
         errorln(USAGE);
         throw e;
       }
 
-      checkMetadataStoreUri(paths);
-
       try {
         initMetadataStore(false);
       } catch (FileNotFoundException e) {

+ 13 - 0
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/AbstractS3GuardToolTestBase.java

@@ -381,6 +381,19 @@ public abstract class AbstractS3GuardToolTestBase extends AbstractS3ATestBase {
     }
   }
 
+  @Test
+  public void testDestroyFailsIfNoBucketNameOrDDBTableSet()
+      throws Exception {
+    intercept(ExitUtil.ExitException.class,
+        () -> run(S3GuardTool.Destroy.NAME));
+  }
+
+  @Test
+  public void testInitFailsIfNoBucketNameOrDDBTableSet() throws Exception {
+    intercept(ExitUtil.ExitException.class,
+        () -> run(S3GuardTool.Init.NAME));
+  }
+
   /**
    * Get the test CSV file; assume() that it is not modified (i.e. we haven't
    * switched to a new storage infrastructure where the bucket is no longer

+ 2 - 1
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/ITestS3GuardToolDynamoDB.java

@@ -126,7 +126,8 @@ public class ITestS3GuardToolDynamoDB extends AbstractS3GuardToolTestBase {
 
     String[] argsR = new String[]{
         cmdR.getName(),
-        "-tag", tagMapToStringParams(tagMap)
+        "-tag", tagMapToStringParams(tagMap),
+        getFileSystem().getBucket()
     };
 
     // run