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


Reply via email to