Support dynamic impermission user V2

core/common/src/main/java/alluxio/security/authentication/DefaultAuthenticationServer.java

@@ -13,6 +13,7 @@
 
 import alluxio.conf.AlluxioConfiguration;
 import alluxio.conf.PropertyKey;
+import alluxio.conf.ReconfigurableRegistry;
 import alluxio.exception.status.UnauthenticatedException;
 import alluxio.grpc.ChannelAuthenticationScheme;
 import alluxio.grpc.SaslAuthenticationServiceGrpc;
@@ -80,7 +81,8 @@ public DefaultAuthenticationServer(String hostName, AlluxioConfiguration conf) {
         Executors.newScheduledThreadPool(1, ThreadFactoryUtils.build("auth-cleanup", true));
     mScheduler.scheduleAtFixedRate(this::cleanupStaleClients, mCleanupIntervalMs,
         mCleanupIntervalMs, TimeUnit.MILLISECONDS);
-    mImpersonationAuthenticator = new ImpersonationAuthenticator(conf);
+    mImpersonationAuthenticator = new ImpersonationAuthenticator();
+    ReconfigurableRegistry.register(mImpersonationAuthenticator);
   }
 
   @Override
@@ -146,6 +148,7 @@ public void close() {
             exc);
       }
     }
+    ReconfigurableRegistry.unregister(mImpersonationAuthenticator);
   }
 
   /**

core/common/src/main/java/alluxio/security/authentication/ImpersonationAuthenticator.java

@@ -13,7 +13,9 @@
 
 import alluxio.RuntimeConstants;
 import alluxio.conf.AlluxioConfiguration;
+import alluxio.conf.Configuration;
 import alluxio.conf.PropertyKey;
+import alluxio.conf.Reconfigurable;
 import alluxio.util.CommonUtils;
 
 import com.google.common.base.Splitter;
@@ -36,13 +38,15 @@
  * users and groups the connection user is allowed to impersonate.
  */
 @ThreadSafe
-public final class ImpersonationAuthenticator {
+public final class ImpersonationAuthenticator implements Reconfigurable {
   public static final String WILDCARD = "*";
   private static final Splitter SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();
   // Maps users configured for impersonation to the set of groups which they can impersonate.
-  private final Map<String, Set<String>> mImpersonationGroups;
+  // Because volatile visibility piggy-backs, there is no need to add volatile on the
+  // mImpersonationGroups property.
+  private Map<String, Set<String>> mImpersonationGroups;
   // Maps users configured for impersonation to the set of users which they can impersonate.
-  private final Map<String, Set<String>> mImpersonationUsers;
+  private volatile Map<String, Set<String>> mImpersonationUsers;
 
   private AlluxioConfiguration mConfiguration;
 
@@ -51,36 +55,9 @@ public final class ImpersonationAuthenticator {
    *
    * Note the constructor for this object is expensive. Take care with how many of these are
    * instantiated.
-   *
-   * @param conf conf Alluxio configuration
    */
-  public ImpersonationAuthenticator(AlluxioConfiguration conf) {
-    mImpersonationGroups = new HashMap<>();
-    mImpersonationUsers = new HashMap<>();
-    mConfiguration = conf;
-
-    for (PropertyKey key : conf.keySet()) {
-      // Process impersonation groups
-      Matcher matcher =
-          PropertyKey.Template.MASTER_IMPERSONATION_GROUPS_OPTION.match(key.getName());
-      if (matcher.matches()) {
-        String connectionUser = matcher.group(1);
-        String value = conf.getOrDefault(key, null);
-        if (connectionUser != null) {
-          mImpersonationGroups.put(connectionUser, Sets.newHashSet(SPLITTER.split(value)));
-        }
-      }
-
-      // Process impersonation users
-      matcher = PropertyKey.Template.MASTER_IMPERSONATION_USERS_OPTION.match(key.getName());
-      if (matcher.matches()) {
-        String connectionUser = matcher.group(1);
-        String value = conf.getOrDefault(key, null);
-        if (connectionUser != null) {
-          mImpersonationUsers.put(connectionUser, Sets.newHashSet(SPLITTER.split(value)));
-        }
-      }
-    }
+  public ImpersonationAuthenticator() {
+    loadImpersonationUser(Configuration.global());
   }
 
   /**
@@ -146,4 +123,39 @@ public void authenticate(String connectionUser, String impersonationUser)
         connectionUser, impersonationUser, connectionUser, impersonationUser,
         RuntimeConstants.ALLUXIO_SECURITY_DOCS_URL));
   }
+
+  @Override
+  public void update() {
+    loadImpersonationUser(Configuration.global());
+  }
+
+  private void loadImpersonationUser(AlluxioConfiguration conf) {
+    Map<String, Set<String>> impersonationGroups = new HashMap<>();
+    Map<String, Set<String>> impersonationUsers = new HashMap<>();
+    for (PropertyKey key : conf.userKeySet()) {
+      // Process impersonation groups
+      Matcher matcher =
+          PropertyKey.Template.MASTER_IMPERSONATION_GROUPS_OPTION.match(key.getName());
+      if (matcher.matches()) {
+        String connectionUser = matcher.group(1);
+        String value = conf.getOrDefault(key, null);
+        if (connectionUser != null) {
+          impersonationGroups.put(connectionUser, Sets.newHashSet(SPLITTER.split(value)));
+        }
+      }
+
+      // Process impersonation users
+      matcher = PropertyKey.Template.MASTER_IMPERSONATION_USERS_OPTION.match(key.getName());
+      if (matcher.matches()) {
+        String connectionUser = matcher.group(1);
+        String value = conf.getOrDefault(key, null);
+        if (connectionUser != null) {
+          impersonationUsers.put(connectionUser, Sets.newHashSet(SPLITTER.split(value)));
+        }
+      }
+    }
+    mConfiguration = conf;
+    mImpersonationGroups = impersonationGroups;
+    mImpersonationUsers = impersonationUsers;
+  }
 }

core/common/src/test/java/alluxio/security/authentication/PlainSaslServerCallbackHandlerTest.java

@@ -56,7 +56,7 @@ public final class PlainSaslServerCallbackHandlerTest {
   public void before() throws Exception {
     mPlainServerCBHandler = new PlainSaslServerCallbackHandler(
         AuthenticationProvider.Factory.create(AuthType.CUSTOM, mConfiguration),
-        new ImpersonationAuthenticator(mConfiguration));
+        new ImpersonationAuthenticator());
   }
 
   @After

core/server/master/src/main/java/alluxio/master/meta/DefaultMetaMaster.java

@@ -684,7 +684,7 @@ public Map<String, Boolean> updateConfiguration(Map<String, String> propertiesMa
         PropertyKey key = PropertyKey.fromString(entry.getKey());
         if (Configuration.getBoolean(PropertyKey.CONF_DYNAMIC_UPDATE_ENABLED)
             && key.isDynamic()) {
-          Object oldValue = Configuration.get(key);
+          Object oldValue = Configuration.getOrDefault(key, null);
           Object value = key.parseValue(entry.getValue());
           Configuration.set(key, value, Source.RUNTIME);
           result.put(entry.getKey(), true);

tests/src/test/java/alluxio/client/cli/fsadmin/command/UpdateConfIntegrationTest.java

@@ -33,7 +33,7 @@ public void unknownOption() {
   @Test
   public void updateUnknownKey() {
     int ret = mFsAdminShell.run("updateConf", "unknown-key=unknown-value");
-    Assert.assertEquals(-2, ret);
+    Assert.assertEquals(0, ret);
     ret = mFsAdminShell.run("updateConf", "unknown-key");
     Assert.assertEquals(-1, ret);
     ret = mFsAdminShell.run("updateConf", "unknown-key=1=2");