chrajeshbabu commented on a change in pull request #86: URL: https://github.com/apache/phoenix-omid/pull/86#discussion_r573566977
########## File path: hbase-common/src/main/java/org/apache/omid/tools/hbase/HBaseLogin.java ########## @@ -29,27 +33,95 @@ private static final Logger LOG = LoggerFactory.getLogger(HBaseLogin.class); private static volatile UserGroupInformation ugi; + private static final Object KERBEROS_LOGIN_LOCK = new Object(); @Nullable public static UserGroupInformation loginIfNeeded(SecureHBaseConfig config) throws IOException { + return loginIfNeeded(config, null); + } - if (UserGroupInformation.isSecurityEnabled()) { - LOG.info("Security enabled when connecting to HBase"); - if (ugi == null) { // Use lazy initialization with double-checked locking - synchronized (HBaseLogin.class) { - if (ugi == null) { - LOG.info("Login with Kerberos. User={}, keytab={}", config.getPrincipal(), config.getKeytab()); - UserGroupInformation.loginUserFromKeytab(config.getPrincipal(), config.getKeytab()); - ugi = UserGroupInformation.getCurrentUser(); + @Nullable + public static UserGroupInformation loginIfNeeded(SecureHBaseConfig config, Configuration hbaseConf) throws IOException { + boolean credsProvided = null != config.getPrincipal() && null != config.getKeytab(); + if (UserGroupInformation.isSecurityEnabled() && credsProvided) { + // Check if we need to authenticate with kerberos so that we cache the correct ConnectionInfo + UserGroupInformation currentUser = UserGroupInformation.getCurrentUser(); + if (!currentUser.hasKerberosCredentials() || !isSameName(currentUser.getUserName(), config.getPrincipal())) { + synchronized (KERBEROS_LOGIN_LOCK) { + // Double check the current user, might have changed since we checked last. Don't want + // to re-login if it's the same user. + currentUser = UserGroupInformation.getCurrentUser(); + if (!currentUser.hasKerberosCredentials() || !isSameName(currentUser.getUserName(), config.getPrincipal())) { + final Configuration hbaseConfig = getConfiguration(hbaseConf, config.getPrincipal(), config.getKeytab()); + LOG.info("Trying to connect to a secure cluster as {} " + + "with keytab {}", + hbaseConfig.get(SecureHBaseConfig.HBASE_CLIENT_PRINCIPAL_KEY), + hbaseConfig.get(SecureHBaseConfig.HBASE_CLIENT_KEYTAB_KEY)); + UserGroupInformation.setConfiguration(hbaseConfig); + User.login(hbaseConfig, SecureHBaseConfig.HBASE_CLIENT_KEYTAB_KEY, SecureHBaseConfig.HBASE_CLIENT_PRINCIPAL_KEY, null); + LOG.info("Successful login to secure cluster"); } } } else { - LOG.info("User {}, already trusted (Kerberos). Avoiding 2nd login as it causes problems", ugi.toString()); + // The user already has Kerberos creds, so there isn't anything to change in the ConnectionInfo. + LOG.debug("Already logged in as {}", currentUser); Review comment: > Nit: logging the UGI even when the config doesn't contain kerberos principal/keytab may be beneficial. This is already there else { LOG.warn("Security NOT enabled when connecting to HBase. Act at your own risk. NULL UGI returned"); } ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org