diff --git a/core/common/src/main/java/alluxio/security/authentication/DefaultAuthenticationServer.java b/core/common/src/main/java/alluxio/security/authentication/DefaultAuthenticationServer.java index 47de89acbdbc..13c343aa39bd 100644 --- a/core/common/src/main/java/alluxio/security/authentication/DefaultAuthenticationServer.java +++ b/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); } /** diff --git a/core/common/src/main/java/alluxio/security/authentication/ImpersonationAuthenticator.java b/core/common/src/main/java/alluxio/security/authentication/ImpersonationAuthenticator.java index 470f8544fcbc..ca2f16a891e1 100644 --- a/core/common/src/main/java/alluxio/security/authentication/ImpersonationAuthenticator.java +++ b/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> mImpersonationGroups; + // Because volatile visibility piggy-backs, there is no need to add volatile on the + // mImpersonationGroups property. + private Map> mImpersonationGroups; // Maps users configured for impersonation to the set of users which they can impersonate. - private final Map> mImpersonationUsers; + private volatile Map> 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> impersonationGroups = new HashMap<>(); + Map> 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; + } } diff --git a/core/common/src/test/java/alluxio/security/authentication/PlainSaslServerCallbackHandlerTest.java b/core/common/src/test/java/alluxio/security/authentication/PlainSaslServerCallbackHandlerTest.java index 43fff3cb89b5..3715ac330fb3 100644 --- a/core/common/src/test/java/alluxio/security/authentication/PlainSaslServerCallbackHandlerTest.java +++ b/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 diff --git a/core/server/master/src/main/java/alluxio/master/meta/DefaultMetaMaster.java b/core/server/master/src/main/java/alluxio/master/meta/DefaultMetaMaster.java index b1b8fe6bb393..1112b089c39a 100644 --- a/core/server/master/src/main/java/alluxio/master/meta/DefaultMetaMaster.java +++ b/core/server/master/src/main/java/alluxio/master/meta/DefaultMetaMaster.java @@ -684,7 +684,7 @@ public Map updateConfiguration(Map 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); diff --git a/tests/src/test/java/alluxio/client/cli/fsadmin/command/UpdateConfIntegrationTest.java b/tests/src/test/java/alluxio/client/cli/fsadmin/command/UpdateConfIntegrationTest.java index bb218e493637..cac6e47ba89c 100644 --- a/tests/src/test/java/alluxio/client/cli/fsadmin/command/UpdateConfIntegrationTest.java +++ b/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");