[SMALLFIX] - Warn users if an unkown authority is passed to the hadoo…

…p client (#7845)

* Warn users if an unkown authority is passed and ignore it

* Respond to comments

- Added unit tests
- Refactored log checker to Junit rule
- Updated warning message.

* final nits

- Make mAppender private in TestLoggerRule
- Fix typos in test method name
- Add space around bracket in TestLoggerRule

core/client/fs/src/test/java/alluxio/client/file/BaseFileSystemTest.java

@@ -29,6 +29,7 @@
 import alluxio.Configuration;
 import alluxio.ConfigurationTestUtils;
 import alluxio.PropertyKey;
+import alluxio.TestLoggerRule;
 import alluxio.client.file.options.CreateDirectoryOptions;
 import alluxio.client.file.options.CreateFileOptions;
 import alluxio.client.file.options.DeleteOptions;
@@ -44,11 +45,9 @@
 import alluxio.wire.FileInfo;
 import alluxio.wire.LoadMetadataType;
 
-import org.apache.log4j.AppenderSkeleton;
-import org.apache.log4j.Logger;
-import org.apache.log4j.spi.LoggingEvent;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.powermock.api.mockito.PowerMockito;
@@ -69,7 +68,8 @@ public final class BaseFileSystemTest {
   private static final String SHOULD_HAVE_PROPAGATED_MESSAGE =
       "Exception should have been propagated";
 
-  private TestAppender mAppender;
+  @Rule
+  private TestLoggerRule mTestLogger = new TestLoggerRule();
 
   private FileSystem mFileSystem;
   private FileSystemContext mFileContext;
@@ -90,14 +90,11 @@ public void before() {
     mFileSystem = new DummyAlluxioFileSystem(mFileContext);
     mFileSystemMasterClient = PowerMockito.mock(FileSystemMasterClient.class);
     when(mFileContext.acquireMasterClient()).thenReturn(mFileSystemMasterClient);
-    mAppender = new TestAppender();
-    Logger.getRootLogger().addAppender(mAppender);
   }
 
   @After
   public void after() {
     ConfigurationTestUtils.resetConfiguration();
-    Logger.getRootLogger().removeAppender(mAppender);
   }
 
   /**
@@ -520,8 +517,8 @@ public void uriCheckBadAuthority() throws Exception {
       assertThat(e.getMessage(), containsString("does not match the configured value of"));
     }
 
-    assertTrue(mAppender.wasLogged("The URI scheme"));
-    assertTrue(mAppender.wasLogged("The URI authority"));
+    assertTrue(mTestLogger.wasLogged("The URI scheme"));
+    assertTrue(mTestLogger.wasLogged("The URI authority"));
 
   }
 
@@ -553,8 +550,8 @@ public void uriCheckGoodSchemeAndAuthority() throws Exception {
     AlluxioURI uri = new AlluxioURI("alluxio://localhost:19998/root");
     mFileSystem.createDirectory(uri);
 
-    assertTrue(mAppender.wasLogged("The URI scheme"));
-    assertTrue(mAppender.wasLogged("The URI authority"));
+    assertTrue(mTestLogger.wasLogged("The URI scheme"));
+    assertTrue(mTestLogger.wasLogged("The URI authority"));
 
   }
 
@@ -570,35 +567,8 @@ public void uriCheckNoSchemeAuthority() throws Exception {
     AlluxioURI uri = new AlluxioURI("/root");
     mFileSystem.createDirectory(uri);
 
-    assertFalse(mAppender.wasLogged("The URI authority"));
-    assertFalse(mAppender.wasLogged("The URI scheme"));
+    assertFalse(mTestLogger.wasLogged("The URI authority"));
+    assertFalse(mTestLogger.wasLogged("The URI scheme"));
   }
 
-  private static class TestAppender extends AppenderSkeleton {
-
-    public List<LoggingEvent> mEvents = new ArrayList<LoggingEvent>();
-
-    public void close() { }
-
-      /**
-       * Determines whether string was logged.
-       */
-    public boolean wasLogged(String eventString) {
-      for (LoggingEvent e : mEvents) {
-        if (e.getRenderedMessage().contains(eventString)) {
-          return true;
-        }
-      }
-      return false;
-    }
-
-    public boolean requiresLayout() {
-      return false;
-    }
-
-    @Override
-    protected void append(LoggingEvent event) {
-      mEvents.add(event);
-    }
-  }
 }

core/client/hdfs/src/main/java/alluxio/hadoop/AbstractFileSystem.java

@@ -39,7 +39,9 @@
 import alluxio.master.MasterInquireClient.Factory;
 import alluxio.security.User;
 import alluxio.security.authorization.Mode;
+import alluxio.uri.Authority;
 import alluxio.uri.SingleMasterAuthority;
+import alluxio.uri.UnknownAuthority;
 import alluxio.uri.ZookeeperAuthority;
 import alluxio.wire.FileBlockInfo;
 import alluxio.wire.WorkerNetAddress;
@@ -460,6 +462,17 @@ public void initialize(URI uri, org.apache.hadoop.conf.Configuration conf) throw
     mAlluxioHeader = getScheme() + "://" + authority;
     // Set the statistics member. Use mStatistics instead of the parent class's variable.
     mStatistics = statistics;
+
+    Authority auth = Authority.fromString(uri.getAuthority());
+    if (auth instanceof UnknownAuthority) {
+      // TODO(zac): In Alluxio 2.0 this warning will be upgraded to an exception
+      LOG.warn("Authority \"{}\" is unknown. The client will not be configured with this"
+          + " authority. The authority connection details will be loaded from your client"
+          + " configuration.",
+          auth);
+      mAlluxioHeader = getScheme() + ":///";
+    }
+
     mUri = URI.create(mAlluxioHeader);
 
     Map<String, Object> uriConfProperties = getConfigurationFromUri(uri);

core/client/hdfs/src/test/java/alluxio/hadoop/AbstractFileSystemApiTest.java

@@ -0,0 +1,60 @@
+/*
+ * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0
+ * (the "License"). You may not use this work except in compliance with the License, which is
+ * available at www.apache.org/licenses/LICENSE-2.0
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied, as more fully set forth in the License.
+ *
+ * See the NOTICE file distributed with this work for information regarding copyright ownership.
+ */
+
+package alluxio.hadoop;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import alluxio.TestLoggerRule;
+
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.net.URI;
+
+/**
+ * Tests for {@link AbstractFileSystem}. Unlike {@link AbstractFileSystemTest}, these tests only
+ * exercise the public API of {@link AbstractFileSystem}.
+ */
+public final class AbstractFileSystemApiTest {
+
+  @Rule
+  public TestLoggerRule mTestLogger = new TestLoggerRule();
+
+  @After
+  public void after() {
+    HadoopClientTestUtils.resetClient();
+  }
+
+  @Test
+  public void unknownAuthorityTriggersWarning() throws IOException {
+    URI unknown = URI.create("alluxio://test/");
+    FileSystem.get(unknown, new org.apache.hadoop.conf.Configuration());
+    assertTrue(mTestLogger.wasLogged("Authority \"test\" is unknown"));
+  }
+
+  @Test
+  public void noAuthorityNoWarning() throws IOException {
+    URI unknown = URI.create("alluxio:///");
+    FileSystem.get(unknown, new org.apache.hadoop.conf.Configuration());
+    assertFalse(mTestLogger.wasLogged("Authority \"\" is unknown"));
+  }
+
+  @Test
+  public void validAuthorityNoWarning() throws IOException {
+    URI unknown = URI.create("alluxio://localhost:12345/");
+    FileSystem.get(unknown, new org.apache.hadoop.conf.Configuration());
+    assertFalse(mTestLogger.wasLogged("Authority \"localhost:12345\" is unknown"));
+  }
+}

core/common/src/test/java/alluxio/TestLoggerRule.java

@@ -0,0 +1,77 @@
+/*
+ * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0
+ * (the "License"). You may not use this work except in compliance with the License, which is
+ * available at www.apache.org/licenses/LICENSE-2.0
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied, as more fully set forth in the License.
+ *
+ * See the NOTICE file distributed with this work for information regarding copyright ownership.
+ */
+
+package alluxio;
+
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Logger;
+import org.apache.log4j.spi.LoggingEvent;
+import org.junit.After;
+import org.junit.Before;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class TestLoggerRule extends AbstractResourceRule {
+
+  private TestAppender mAppender;
+
+  @Before
+  public void before() {
+    mAppender = new TestAppender();
+    Logger.getRootLogger().addAppender(mAppender);
+  }
+
+  @After
+  public void after() {
+    Logger.getRootLogger().removeAppender(mAppender);
+  }
+
+  /**
+   * Determine if a specific piece of text appears in log output.
+   *
+   * @param eventString The piece of text to search for in log events
+   * @return True if an event containing the string exists, false otherwise
+   */
+  public boolean wasLogged(String eventString) {
+    return mAppender.wasLogged(eventString);
+  }
+
+  public class TestAppender extends AppenderSkeleton {
+
+    public List<LoggingEvent> mEvents = new ArrayList<LoggingEvent>();
+
+    public void close() { }
+
+    /**
+     * Determines whether string was logged.
+     */
+    public boolean wasLogged(String eventString) {
+      for (LoggingEvent e : mEvents) {
+        if (e.getRenderedMessage().contains(eventString)) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+    public boolean requiresLayout() {
+      return false;
+    }
+
+    @Override
+    protected void append(LoggingEvent event) {
+      mEvents.add(event);
+    }
+  }
+
+}
+

core/common/src/test/java/alluxio/TestLoggerRuleTest.java

@@ -0,0 +1,38 @@
+/*
+ * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0
+ * (the "License"). You may not use this work except in compliance with the License, which is
+ * available at www.apache.org/licenses/LICENSE-2.0
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied, as more fully set forth in the License.
+ *
+ * See the NOTICE file distributed with this work for information regarding copyright ownership.
+ */
+
+package alluxio;
+
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Unit tests for {@link TestLoggerRule}.
+ */
+public final class TestLoggerRuleTest {
+
+  private static final Logger LOG = LoggerFactory.getLogger(TestLoggerRuleTest.class);
+
+  @Rule
+  public TestLoggerRule mLogger = new TestLoggerRule();
+
+  @Test
+  public void wasLoggedTest() {
+    String logEvent = "This is a test";
+    LOG.info(logEvent);
+    assertTrue(mLogger.wasLogged(logEvent));
+  }
+
+}