You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ga...@apache.org on 2019/11/04 11:55:57 UTC

[hadoop] branch trunk updated: HADOOP-16484. S3A to warn or fail if S3Guard is disabled (#1661). Contributed by Gabor Bota.

This is an automated email from the ASF dual-hosted git repository.

gabota pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/trunk by this push:
     new dca19fc  HADOOP-16484. S3A to warn or fail if S3Guard is disabled (#1661). Contributed by Gabor Bota.
dca19fc is described below

commit dca19fc3aa509949daf29bc902b2f74760407fc4
Author: Gabor Bota <ga...@cloudera.com>
AuthorDate: Mon Nov 4 12:55:36 2019 +0100

    HADOOP-16484. S3A to warn or fail if S3Guard is disabled (#1661). Contributed by Gabor Bota.
---
 .../src/main/resources/core-default.xml            |  1 -
 .../java/org/apache/hadoop/fs/s3a/Constants.java   |  8 +++++
 .../org/apache/hadoop/fs/s3a/S3AFileSystem.java    | 10 +++++-
 .../org/apache/hadoop/fs/s3a/s3guard/S3Guard.java  | 42 ++++++++++++++++++++++
 .../src/site/markdown/tools/hadoop-aws/s3guard.md  | 16 +++++++++
 .../apache/hadoop/fs/s3a/s3guard/TestS3Guard.java  | 29 +++++++++++++++
 6 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
index f0aa44d..b1c7be8 100644
--- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
+++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
@@ -1553,7 +1553,6 @@
   </description>
 </property>
 
-
 <property>
     <name>fs.s3a.s3guard.cli.prune.age</name>
     <value>86400000</value>
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
index 9f120b8..8593538 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
@@ -640,6 +640,14 @@ public final class Constants {
       = "org.apache.hadoop.fs.s3a.s3guard.DynamoDBMetadataStore";
 
   /**
+   * The warn level if S3Guard is disabled.
+   */
+  public static final String S3GUARD_DISABLED_WARN_LEVEL
+      = "org.apache.hadoop.fs.s3a.s3guard.disabled.warn.level";
+  public static final String DEFAULT_S3GUARD_DISABLED_WARN_LEVEL =
+      "INFORM";
+
+  /**
    * Inconsistency (visibility delay) injection settings.
    */
   @InterfaceStability.Unstable
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
index dcc1da5..f0ddf44 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
@@ -426,6 +426,14 @@ public class S3AFileSystem extends FileSystem implements StreamCapabilities,
         LOG.debug("Using metadata store {}, authoritative store={}, authoritative path={}",
             getMetadataStore(), allowAuthoritativeMetadataStore, allowAuthoritativePaths);
       }
+
+      // LOG if S3Guard is disabled on the warn level set in config
+      if (!hasMetadataStore()) {
+        String warnLevel = conf.getTrimmed(S3GUARD_DISABLED_WARN_LEVEL,
+            DEFAULT_S3GUARD_DISABLED_WARN_LEVEL);
+        S3Guard.logS3GuardDisabled(LOG, warnLevel, bucket);
+      }
+
       initMultipartUploads(conf);
     } catch (AmazonClientException e) {
       throw translateException("initializing ", new Path(name), e);
@@ -2192,7 +2200,7 @@ public class S3AFileSystem extends FileSystem implements StreamCapabilities,
   }
 
   /**
-   * Invoke {@link #removeKeysS3(List, boolean)} with handling of
+   * Invoke {@link #removeKeysS3(List, boolean, boolean)} with handling of
    * {@code MultiObjectDeleteException}.
    *
    * @param keysToDelete collection of keys to delete on the s3-backend.
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
index 2e9ba66..5b80f76 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
@@ -24,6 +24,7 @@ import java.net.URI;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -816,4 +817,45 @@ public final class S3Guard {
     }
     return false;
   }
+
+  public static final String DISABLED_LOG_MSG =
+      "S3Guard is disabled on this bucket: {}";
+
+  public static final String UNKNOWN_WARN_LEVEL =
+      "Unknown S3Guard disabled warn level: ";
+
+  public enum DisabledWarnLevel {
+    SILENT,
+    INFORM,
+    WARN,
+    FAIL
+  }
+
+  public static void logS3GuardDisabled(Logger logger, String warnLevelStr,
+      String bucket)
+      throws UnsupportedOperationException, IllegalArgumentException {
+    final DisabledWarnLevel warnLevel;
+    try {
+      warnLevel = DisabledWarnLevel.valueOf(warnLevelStr.toUpperCase(Locale.US));
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException(UNKNOWN_WARN_LEVEL + warnLevelStr, e);
+    }
+
+    switch (warnLevel) {
+    case SILENT:
+      logger.debug(DISABLED_LOG_MSG, bucket);
+      break;
+    case INFORM:
+      logger.info(DISABLED_LOG_MSG, bucket);
+      break;
+    case WARN:
+      logger.warn(DISABLED_LOG_MSG, bucket);
+      break;
+    case FAIL:
+      logger.error(DISABLED_LOG_MSG, bucket);
+      throw new UnsupportedOperationException(DISABLED_LOG_MSG + bucket);
+    default:
+      throw new IllegalArgumentException(UNKNOWN_WARN_LEVEL + warnLevelStr);
+    }
+  }
 }
diff --git a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md
index 0a05790..33bc76c 100644
--- a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md
+++ b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md
@@ -62,6 +62,22 @@ with the S3A data, in which case the normal S3 consistency guarantees apply.
 
 ## Setting up S3Guard
 
+### S3A to warn or fail if S3Guard is disabled
+A seemingly recurrent problem with S3Guard is that people think S3Guard is
+turned on but it isn't.
+You can set `org.apache.hadoop.fs.s3a.s3guard.disabled.warn.level`
+to avoid this. The property sets what to do when an S3A FS is instantiated
+without S3Guard. The following values are available:
+
+* `SILENT`: Do nothing.
+* `INFORM`: Log at info level that FS is instantiated without S3Guard.
+* `WARN`: Warn that data may be at risk in workflows.
+* `FAIL`: S3AFileSystem instantiation will fail.
+
+The default setting is INFORM. The setting is case insensitive.
+The required level can be set in the `core-site.xml`.
+
+---
 The latest configuration parameters are defined in `core-default.xml`.  You
 should consult that file for full information, but a summary is provided here.
 
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestS3Guard.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestS3Guard.java
index 90962ad..ecad850 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestS3Guard.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestS3Guard.java
@@ -22,16 +22,21 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Locale;
+import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.s3a.S3AFileStatus;
 import org.apache.hadoop.fs.s3a.Tristate;
+import org.apache.hadoop.test.LambdaTestUtils;
 
 import static org.apache.hadoop.fs.s3a.Constants.DEFAULT_METADATASTORE_METADATA_TTL;
 import static org.apache.hadoop.fs.s3a.Constants.METADATASTORE_METADATA_TTL;
@@ -261,6 +266,30 @@ public class TestS3Guard extends Assert {
         new S3Guard.TtlTimeProvider(conf));
   }
 
+  @Test
+  public void testLogS3GuardDisabled() throws Exception {
+    final Logger localLogger = LoggerFactory.getLogger(
+        TestS3Guard.class.toString() + UUID.randomUUID());
+    S3Guard.logS3GuardDisabled(localLogger,
+        S3Guard.DisabledWarnLevel.SILENT.toString(), "bucket");
+    S3Guard.logS3GuardDisabled(localLogger,
+        S3Guard.DisabledWarnLevel.INFORM.toString(), "bucket");
+    S3Guard.logS3GuardDisabled(localLogger,
+        S3Guard.DisabledWarnLevel.WARN.toString(), "bucket");
+
+    // Test that lowercase setting is accepted
+    S3Guard.logS3GuardDisabled(localLogger,
+        S3Guard.DisabledWarnLevel.WARN.toString()
+            .toLowerCase(Locale.US), "bucket");
+
+    LambdaTestUtils.intercept(UnsupportedOperationException.class,
+        S3Guard.DISABLED_LOG_MSG, () -> S3Guard.logS3GuardDisabled(
+            localLogger, S3Guard.DisabledWarnLevel.FAIL.toString(), "bucket"));
+    LambdaTestUtils.intercept(IllegalArgumentException.class,
+        S3Guard.UNKNOWN_WARN_LEVEL, () -> S3Guard.logS3GuardDisabled(
+            localLogger, "FOO_BAR_LEVEL", "bucket"));
+  }
+
   void assertContainsPath(FileStatus[] statuses, String pathStr) {
     assertTrue("listing doesn't contain " + pathStr,
         containsPath(statuses, pathStr));


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org