HADOOP-16484. S3A to warn or fail if S3Guard is disabled (#1661). Contributed by Gabor Bota.

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>

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java

@@ -639,6 +639,14 @@ public final class Constants {
   public static final String S3GUARD_METASTORE_DYNAMO
       = "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.
    */

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.

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

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.
 

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