Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support dynamic impermission user V2 #17305

Open
wants to merge 4 commits into
base: master-2.x
Choose a base branch
from
Open

Support dynamic impermission user V2 #17305

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8727c17
feat: support dynamic impermission user v2.
qian0817 Apr 23, 2023
75887d8
feat: remove impersonation authenticator singleton.
qian0817 Apr 24, 2023
e824fc4
feat: fix update configuration test.
qian0817 Apr 24, 2023
27517d3
pref: use userKeySet to replace keySet
qian0817 Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/common/src/main/java/alluxio/security/authentication/DefaultAuthenticationServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ 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 = ImpersonationAuthenticator.getInstance();
qian0817 marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
87 changes: 58 additions & 29 deletions core/common/src/main/java/alluxio/security/authentication/ImpersonationAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@

import alluxio.RuntimeConstants;
import alluxio.conf.AlluxioConfiguration;
import alluxio.conf.Configuration;
import alluxio.conf.PropertyKey;
import alluxio.conf.Reconfigurable;
import alluxio.conf.ReconfigurableRegistry;
import alluxio.util.CommonUtils;

import com.google.common.base.Splitter;
import com.google.common.base.Suppliers;
import com.google.common.collect.Sets;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import javax.annotation.concurrent.ThreadSafe;
import javax.security.sasl.AuthenticationException;
Expand All @@ -36,15 +41,28 @@
* 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;
private static final Supplier<ImpersonationAuthenticator> SINGLETON =
Suppliers.memoize(() -> new ImpersonationAuthenticator(Configuration.global()));

/**
* return a {@link ImpersonationAuthenticator} singleton instance.
*
* @return ImpersonationAuthenticator
*/
public static ImpersonationAuthenticator getInstance() {
return SINGLETON.get();
}

/**
* Constructs a new {@link ImpersonationAuthenticator}.
Expand All @@ -54,33 +72,10 @@ public final class ImpersonationAuthenticator {
*
* @param conf conf Alluxio configuration
*/
public ImpersonationAuthenticator(AlluxioConfiguration conf) {
mImpersonationGroups = new HashMap<>();
mImpersonationUsers = new HashMap<>();
ImpersonationAuthenticator(AlluxioConfiguration conf) {
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)));
}
}
}
loadImpersonationUser(conf);
ReconfigurableRegistry.register(this);
}

/**
Expand Down Expand Up @@ -146,4 +141,38 @@ 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.keySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just concern to the effect of old implementation, we have to iterate all PropertyKey to find the template property, although the updated propertykeys are not them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, there are approximately 1000 property keys in Alluxio. Traversing them does not significantly affect performance. However, it would be more effective to operate based on the PropertyKey that changes with modifications.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For current implementation, we can have two idea to reduce waste loop templete key improve the performance

  • Improve the update configuration and reconfig framework, support fire change and listen changed properties only.
  • Improve the propertykey framework to support index template keys, so we can get the template key efficiently.

qian0817 marked this conversation as resolved.
Show resolved Hide resolved
// 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)));
}
}
}
mImpersonationGroups = impersonationGroups;
mImpersonationUsers = impersonationUsers;
}
}