github-actions[bot] commented on code in PR #69608:
URL: https://github.com/apache/doris/pull/69608#discussion_r4373944106
##########
fe/fe-kerberos/src/main/java/org/apache/doris/kerberos/HadoopKerberosAuthenticator.java:
##########
@@ -47,38 +50,86 @@ public class HadoopKerberosAuthenticator implements
HadoopAuthenticator {
private long nextRefreshTime;
private UserGroupInformation ugi;
- // The authentication method the first caller published to the
process-wide UGI configuration.
- // Guarded by HadoopKerberosAuthenticator.class; null until the first
setConfiguration.
- private static UserGroupInformation.AuthenticationMethod
configuredAuthMethod = null;
+ // Hadoop's rule grammar keeps whitespace inside a regex or substitution
distinct from rule separators.
+ private static final Pattern AUTH_TO_LOCAL_RULE = Pattern.compile(
+ "\\s*((DEFAULT)|(RULE:\\[(\\d*):([^\\]]*)](\\(([^)]*)\\))?"
+ + "(s/([^/]*)/([^/]*)/(g)?)?))/?(L)?");
public HadoopKerberosAuthenticator(KerberosAuthenticationConfig config) {
this.config = config;
}
public static void initializeAuthConfig(Configuration hadoopConf) {
hadoopConf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION,
"true");
- // UserGroupInformation.setConfiguration mutates a single process-wide
global, so multiple HDFS
- // catalogs with differing hadoop.security.authentication cannot truly
coexist. First-writer-wins:
- // only the first caller publishes the global config; later catalogs
(and every ticket refresh,
- // which re-enters this method via login()) skip it so they don't
stomp on each other. A WARN
- // fires only on a genuine auth-method mismatch. We compare against a
remembered method rather than
- // UserGroupInformation.getLoginUser() on purpose: Doris never
establishes a process-wide login user
- // (per-instance getUGIFromSubject only), and getLoginUser() would
trigger exactly that.
- UserGroupInformation.AuthenticationMethod desired =
SecurityUtil.getAuthenticationMethod(hadoopConf);
- synchronized (HadoopKerberosAuthenticator.class) {
- if (configuredAuthMethod == null) {
+ String additionalRules =
hadoopConf.get(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL);
+ String mechanism =
hadoopConf.get(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL_MECHANISM,
+ KerberosName.DEFAULT_MECHANISM);
+ // UGI changes its auth mode before parsing rules. Reject malformed
input before it can leave
+ // an enabled UGI with the permissive rules inherited from a previous
SIMPLE initialization.
+
Preconditions.checkArgument(KerberosName.MECHANISM_HADOOP.equalsIgnoreCase(mechanism)
+ ||
KerberosName.MECHANISM_MIT.equalsIgnoreCase(mechanism),
+ "Invalid auth_to_local mechanism: %s", mechanism);
+ Set<String> requestedRules = additionalRules == null
+ ? Collections.emptySet() :
parseAuthToLocalRules(additionalRules);
+ // Plugins can load separate copies of this class while sharing
Hadoop. Lock on the shared
+ // UGI class, and inspect its state instead of keeping a per-plugin
initialization flag.
+ synchronized (UserGroupInformation.class) {
+ if (!UserGroupInformation.isSecurityEnabled()) {
Review Comment:
[P2] Avoid initializing UGI while checking its initial state
On the first UGI use, `isSecurityEnabled()` [initializes UGI from `new
Configuration()`](https://raw.githubusercontent.com/apache/hadoop/rel/release-3.4.2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java),
also constructing Hadoop's [singleton Groups
service](https://raw.githubusercontent.com/apache/hadoop/rel/release-3.4.2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java).
The following `setConfiguration(hadoopConf)` reuses that service, so catalog
settings such as `hadoop.user.group.static.mapping.overrides=hdfs=analytics` or
a custom `hadoop.security.group.mapping` are ignored. The old connector path
initialized directly from the catalog configuration. Check `isInitialized()`
before invoking the initializing predicate, and test a fresh UGI/Groups runtime
with a nondefault group mapping.
##########
fe/fe-kerberos/src/main/java/org/apache/doris/kerberos/HadoopKerberosAuthenticator.java:
##########
@@ -47,38 +50,86 @@ public class HadoopKerberosAuthenticator implements
HadoopAuthenticator {
private long nextRefreshTime;
private UserGroupInformation ugi;
- // The authentication method the first caller published to the
process-wide UGI configuration.
- // Guarded by HadoopKerberosAuthenticator.class; null until the first
setConfiguration.
- private static UserGroupInformation.AuthenticationMethod
configuredAuthMethod = null;
+ // Hadoop's rule grammar keeps whitespace inside a regex or substitution
distinct from rule separators.
+ private static final Pattern AUTH_TO_LOCAL_RULE = Pattern.compile(
+ "\\s*((DEFAULT)|(RULE:\\[(\\d*):([^\\]]*)](\\(([^)]*)\\))?"
+ + "(s/([^/]*)/([^/]*)/(g)?)?))/?(L)?");
public HadoopKerberosAuthenticator(KerberosAuthenticationConfig config) {
this.config = config;
}
public static void initializeAuthConfig(Configuration hadoopConf) {
hadoopConf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION,
"true");
- // UserGroupInformation.setConfiguration mutates a single process-wide
global, so multiple HDFS
- // catalogs with differing hadoop.security.authentication cannot truly
coexist. First-writer-wins:
- // only the first caller publishes the global config; later catalogs
(and every ticket refresh,
- // which re-enters this method via login()) skip it so they don't
stomp on each other. A WARN
- // fires only on a genuine auth-method mismatch. We compare against a
remembered method rather than
- // UserGroupInformation.getLoginUser() on purpose: Doris never
establishes a process-wide login user
- // (per-instance getUGIFromSubject only), and getLoginUser() would
trigger exactly that.
- UserGroupInformation.AuthenticationMethod desired =
SecurityUtil.getAuthenticationMethod(hadoopConf);
- synchronized (HadoopKerberosAuthenticator.class) {
- if (configuredAuthMethod == null) {
+ String additionalRules =
hadoopConf.get(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL);
+ String mechanism =
hadoopConf.get(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL_MECHANISM,
+ KerberosName.DEFAULT_MECHANISM);
+ // UGI changes its auth mode before parsing rules. Reject malformed
input before it can leave
+ // an enabled UGI with the permissive rules inherited from a previous
SIMPLE initialization.
+
Preconditions.checkArgument(KerberosName.MECHANISM_HADOOP.equalsIgnoreCase(mechanism)
+ ||
KerberosName.MECHANISM_MIT.equalsIgnoreCase(mechanism),
+ "Invalid auth_to_local mechanism: %s", mechanism);
+ Set<String> requestedRules = additionalRules == null
+ ? Collections.emptySet() :
parseAuthToLocalRules(additionalRules);
+ // Plugins can load separate copies of this class while sharing
Hadoop. Lock on the shared
+ // UGI class, and inspect its state instead of keeping a per-plugin
initialization flag.
+ synchronized (UserGroupInformation.class) {
+ if (!UserGroupInformation.isSecurityEnabled()) {
UserGroupInformation.setConfiguration(hadoopConf);
- configuredAuthMethod = desired;
return;
}
- if (configuredAuthMethod != desired) {
- LOG.warn("UGI already configured with authentication={} but
this catalog requests {}; "
- + "keeping existing JVM-global setting
(first-writer-wins).",
- configuredAuthMethod, desired);
+ if (additionalRules == null) {
+ return;
+ }
+
Preconditions.checkArgument(mechanism.equalsIgnoreCase(KerberosName.getRuleMechanism()),
+ "Kerberos catalogs sharing a Hadoop runtime must use the
same auth_to_local mechanism "
+ + "(existing=%s, requested=%s)",
KerberosName.getRuleMechanism(), mechanism);
+ Set<String> rules = parseAuthToLocalRules(KerberosName.getRules());
+ int previousSize = rules.size();
+ // Append after all existing rules, including DEFAULT: a later
realm must not change an
+ // earlier principal's mapping. Replacing the global configuration
would break its next login.
+ if (rules.addAll(requestedRules)) {
+ // MIT accepts an unmatched principal unchanged; extending its
rules could rename a user.
+
Preconditions.checkArgument(KerberosName.MECHANISM_HADOOP.equalsIgnoreCase(mechanism),
+ "MIT auth_to_local rules cannot be extended after
Hadoop initialization; "
+ + "configure a common policy before
initializing catalogs");
+ KerberosName.setRules(String.join("\n", rules));
Review Comment:
[P2] Roll back rule extensions when UGI construction fails
With FIRST.EXAMPLE already configured and JVM default realm DEFAULT.EXAMPLE,
a SECOND.EXAMPLE catalog can supply `RULE:[2:$3]`. This passes
`parseAuthToLocalRules`, but Hadoop's [parameter
expansion](https://raw.githubusercontent.com/apache/hadoop/rel/release-3.4.2/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java)
throws because a two-component principal only has parameters `$0` through
`$2`. This line publishes the rule before `getUGIFromSubject` fails, and
neither login caller restores the previous rules. Correcting SECOND's rules or
adding THIRD then appends behind the bad rule, so those logins keep failing.
Only retain an extension after successful principal mapping, or restore the
prior rules on failure; add a failed-then-corrected catalog test.
##########
fe/fe-kerberos/src/main/java/org/apache/doris/kerberos/HadoopKerberosAuthenticator.java:
##########
@@ -47,38 +50,86 @@ public class HadoopKerberosAuthenticator implements
HadoopAuthenticator {
private long nextRefreshTime;
private UserGroupInformation ugi;
- // The authentication method the first caller published to the
process-wide UGI configuration.
- // Guarded by HadoopKerberosAuthenticator.class; null until the first
setConfiguration.
- private static UserGroupInformation.AuthenticationMethod
configuredAuthMethod = null;
+ // Hadoop's rule grammar keeps whitespace inside a regex or substitution
distinct from rule separators.
+ private static final Pattern AUTH_TO_LOCAL_RULE = Pattern.compile(
+ "\\s*((DEFAULT)|(RULE:\\[(\\d*):([^\\]]*)](\\(([^)]*)\\))?"
+ + "(s/([^/]*)/([^/]*)/(g)?)?))/?(L)?");
public HadoopKerberosAuthenticator(KerberosAuthenticationConfig config) {
this.config = config;
}
public static void initializeAuthConfig(Configuration hadoopConf) {
hadoopConf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION,
"true");
- // UserGroupInformation.setConfiguration mutates a single process-wide
global, so multiple HDFS
- // catalogs with differing hadoop.security.authentication cannot truly
coexist. First-writer-wins:
- // only the first caller publishes the global config; later catalogs
(and every ticket refresh,
- // which re-enters this method via login()) skip it so they don't
stomp on each other. A WARN
- // fires only on a genuine auth-method mismatch. We compare against a
remembered method rather than
- // UserGroupInformation.getLoginUser() on purpose: Doris never
establishes a process-wide login user
- // (per-instance getUGIFromSubject only), and getLoginUser() would
trigger exactly that.
- UserGroupInformation.AuthenticationMethod desired =
SecurityUtil.getAuthenticationMethod(hadoopConf);
- synchronized (HadoopKerberosAuthenticator.class) {
- if (configuredAuthMethod == null) {
+ String additionalRules =
hadoopConf.get(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL);
+ String mechanism =
hadoopConf.get(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTH_TO_LOCAL_MECHANISM,
+ KerberosName.DEFAULT_MECHANISM);
+ // UGI changes its auth mode before parsing rules. Reject malformed
input before it can leave
+ // an enabled UGI with the permissive rules inherited from a previous
SIMPLE initialization.
+
Preconditions.checkArgument(KerberosName.MECHANISM_HADOOP.equalsIgnoreCase(mechanism)
+ ||
KerberosName.MECHANISM_MIT.equalsIgnoreCase(mechanism),
+ "Invalid auth_to_local mechanism: %s", mechanism);
+ Set<String> requestedRules = additionalRules == null
+ ? Collections.emptySet() :
parseAuthToLocalRules(additionalRules);
+ // Plugins can load separate copies of this class while sharing
Hadoop. Lock on the shared
+ // UGI class, and inspect its state instead of keeping a per-plugin
initialization flag.
+ synchronized (UserGroupInformation.class) {
+ if (!UserGroupInformation.isSecurityEnabled()) {
UserGroupInformation.setConfiguration(hadoopConf);
Review Comment:
[P2] Preserve retries after partial UGI initialization
Starting from SIMPLE UGI, use valid mapping `RULE:[2:$1]s/^hdfs$/old-user/`
with `hadoop.kerberos.min.seconds.before.relogin=oops`. Hadoop [sets the
authentication mode and rules before parsing that
number](https://raw.githubusercontent.com/apache/hadoop/rel/release-3.4.2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java),
then throws. The next attempt with a valid number and a corrected `new-user`
mapping now sees security enabled and appends instead of retrying
configuration, so the successful catalog still resolves to `old-user`.
Previously `configuredAuthMethod` was set only after successful setup, allowing
the retry to replace the failed policy. Restore partial state on failure or
track successful shared initialization; test this failed-first-setup retry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
