You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2020/11/10 08:11:19 UTC

[zeppelin] branch branch-0.9 updated: [ZEPPELIN-5121]. Improve the support of hive on kerbose

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/branch-0.9 by this push:
     new 6d2e3d9  [ZEPPELIN-5121]. Improve the support of hive on kerbose
6d2e3d9 is described below

commit 6d2e3d956f95dfdfaa3702eaf36f403bcd66753b
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Tue Nov 10 11:26:21 2020 +0800

    [ZEPPELIN-5121]. Improve the support of hive on kerbose
    
    ### What is this PR for?
    
    Several improvements of hive on kerbors support
    1. Remove ยท`zeppelin.jdbc.auth.kerberos.proxy.enable`, it is never documented, use `prefix.proxy.user.property` is enough.
    2. Append principal to url automatically for users.
    3. Remove the doAs code block, it looks like is not necessary.
    
    ### What type of PR is it?
    [Improvement ]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-5121
    
    ### How should this be tested?
    * CI pass
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: Jeff Zhang <zj...@apache.org>
    
    Closes #3965 from zjffdu/ZEPPELIN-5121 and squashes the following commits:
    
    ea179ace6 [Jeff Zhang] remove the append principal
    d21a00167 [Jeff Zhang] [ZEPPELIN-5121]. Improve the support of hive on kerbose
    
    (cherry picked from commit ed0cb09af1d0f525b5ee5b21c4c4d864b9402f45)
    Signed-off-by: Jeff Zhang <zj...@apache.org>
---
 .../org/apache/zeppelin/jdbc/JDBCInterpreter.java  | 66 +++++++++-------------
 .../org/apache/zeppelin/jdbc/hive/HiveUtils.java   |  4 +-
 .../zeppelin/jdbc/security/JDBCSecurityImpl.java   |  9 +--
 3 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
index f54f5b5..5d792ec 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
@@ -100,7 +100,6 @@ import org.apache.zeppelin.user.UsernamePassword;
 public class JDBCInterpreter extends KerberosInterpreter {
   private static final Logger LOGGER = LoggerFactory.getLogger(JDBCInterpreter.class);
 
-  static final String INTERPRETER_NAME = "jdbc";
   static final String COMMON_KEY = "common";
   static final String MAX_LINE_KEY = "max_count";
   static final int MAX_LINE_DEFAULT = 1000;
@@ -243,7 +242,7 @@ public class JDBCInterpreter extends KerberosInterpreter {
 
   protected boolean isKerboseEnabled() {
     if (!isEmpty(getProperty("zeppelin.jdbc.auth.type"))) {
-      UserGroupInformation.AuthenticationMethod authType = JDBCSecurityImpl.getAuthtype(properties);
+      UserGroupInformation.AuthenticationMethod authType = JDBCSecurityImpl.getAuthType(properties);
       if (authType.equals(KERBEROS)) {
         return true;
       }
@@ -447,6 +446,9 @@ public class JDBCInterpreter extends KerberosInterpreter {
   private void createConnectionPool(String url, String user, String dbPrefix,
       Properties properties) throws SQLException, ClassNotFoundException {
 
+    LOGGER.info("Creating connection pool for url: {}, user: {}, dbPrefix: {}, properties: {}",
+            url, user, dbPrefix, properties);
+
     String driverClass = properties.getProperty(DRIVER_KEY);
     if (driverClass != null && (driverClass.equals("com.facebook.presto.jdbc.PrestoDriver")
             || driverClass.equals("io.prestosql.jdbc.PrestoDriver"))) {
@@ -500,53 +502,37 @@ public class JDBCInterpreter extends KerberosInterpreter {
     setUserProperty(dbPrefix, context);
 
     final Properties properties = jdbcUserConfigurations.getPropertyMap(dbPrefix);
-    final String url = properties.getProperty(URL_KEY);
-
+    String url = properties.getProperty(URL_KEY);
+    
     if (isEmpty(getProperty("zeppelin.jdbc.auth.type"))) {
       connection = getConnectionFromPool(url, user, dbPrefix, properties);
     } else {
       UserGroupInformation.AuthenticationMethod authType =
-          JDBCSecurityImpl.getAuthtype(getProperties());
+          JDBCSecurityImpl.getAuthType(getProperties());
 
       final String connectionUrl = appendProxyUserToURL(url, user, dbPrefix);
-
       JDBCSecurityImpl.createSecureConfiguration(getProperties(), authType);
-      switch (authType) {
-        case KERBEROS:
-          if (user == null || "false".equalsIgnoreCase(
-              getProperty("zeppelin.jdbc.auth.kerberos.proxy.enable"))) {
-            connection = getConnectionFromPool(connectionUrl, user, dbPrefix, properties);
-          } else {
-            if (basePropertiesMap.get(dbPrefix).containsKey("proxy.user.property")) {
-              connection = getConnectionFromPool(connectionUrl, user, dbPrefix, properties);
-            } else {
-              UserGroupInformation ugi = null;
-              try {
-                ugi = UserGroupInformation.createProxyUser(
-                    user, UserGroupInformation.getCurrentUser());
-              } catch (Exception e) {
-                LOGGER.error("Error in getCurrentUser", e);
-                throw new InterpreterException("Error in getCurrentUser", e);
-              }
 
-              final String poolKey = dbPrefix;
-              try {
-                connection = ugi.doAs(new PrivilegedExceptionAction<Connection>() {
-                  @Override
-                  public Connection run() throws Exception {
-                    return getConnectionFromPool(connectionUrl, user, poolKey, properties);
-                  }
-                });
-              } catch (Exception e) {
-                LOGGER.error("Error in doAs", e);
-                throw new InterpreterException("Error in doAs", e);
-              }
-            }
-          }
-          break;
+      if (basePropertiesMap.get(dbPrefix).containsKey("proxy.user.property")) {
+        connection = getConnectionFromPool(connectionUrl, user, dbPrefix, properties);
+      } else {
+        UserGroupInformation ugi = null;
+        try {
+          ugi = UserGroupInformation.createProxyUser(
+                  user, UserGroupInformation.getCurrentUser());
+        } catch (Exception e) {
+          LOGGER.error("Error in getCurrentUser", e);
+          throw new InterpreterException("Error in getCurrentUser", e);
+        }
 
-        default:
-          connection = getConnectionFromPool(connectionUrl, user, dbPrefix, properties);
+        final String poolKey = dbPrefix;
+        try {
+          connection = ugi.doAs((PrivilegedExceptionAction<Connection>) () ->
+                  getConnectionFromPool(connectionUrl, user, poolKey, properties));
+        } catch (Exception e) {
+          LOGGER.error("Error in doAs", e);
+          throw new InterpreterException("Error in doAs", e);
+        }
       }
     }
 
diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/hive/HiveUtils.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/hive/HiveUtils.java
index 5dc66ec..58fc2f0 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/hive/HiveUtils.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/hive/HiveUtils.java
@@ -97,12 +97,12 @@ public class HiveUtils {
           LOGGER.warn("Fail to write output", e);
         }
       }
-      LOGGER.debug("Hive monitor thread is finished");
+      LOGGER.info("HiveMonitor-Thread is finished");
     });
     thread.setName("HiveMonitor-Thread");
     thread.setDaemon(true);
     thread.start();
-    LOGGER.info("Start HiveMonitor-Thread for sql: " + stmt);
+    LOGGER.info("Start HiveMonitor-Thread for sql: " + hiveStmt);
 
     if (progressBar != null) {
       hiveStmt.setInPlaceUpdateStream(progressBar.getInPlaceUpdateStream(context.out));
diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/security/JDBCSecurityImpl.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/security/JDBCSecurityImpl.java
index b093ab2..9d83a1f 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/security/JDBCSecurityImpl.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/security/JDBCSecurityImpl.java
@@ -53,9 +53,10 @@ public class JDBCSecurityImpl {
           if (!UserGroupInformation.isSecurityEnabled()
               || UserGroupInformation.getCurrentUser().getAuthenticationMethod() != KERBEROS
               || !UserGroupInformation.isLoginKeytabBased()) {
-            UserGroupInformation.loginUserFromKeytab(
-                properties.getProperty("zeppelin.jdbc.principal"),
-                properties.getProperty("zeppelin.jdbc.keytab.location"));
+            String keytab = properties.getProperty("zeppelin.jdbc.keytab.location");
+            String principal = properties.getProperty("zeppelin.jdbc.principal");
+            UserGroupInformation.loginUserFromKeytab(principal, keytab);
+            LOGGER.info("Login successfully via keytab: {} and principal: {}", keytab, principal);
           } else {
             LOGGER.info("The user has already logged in using Keytab and principal, " +
                 "no action required");
@@ -67,7 +68,7 @@ public class JDBCSecurityImpl {
     }
   }
 
-  public static AuthenticationMethod getAuthtype(Properties properties) {
+  public static AuthenticationMethod getAuthType(Properties properties) {
     AuthenticationMethod authType;
     try {
       authType = AuthenticationMethod.valueOf(properties.getProperty("zeppelin.jdbc.auth.type")