You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by va...@apache.org on 2017/02/03 02:12:07 UTC

sentry git commit: SENTRY-1619: Fix the secure HMS connection code in HMSFollower (Vamsee Yarlagadda, Reviewed by: Alexander Kolbasov, Kalyan Kumar Kalvagadda, Hao Hao)

Repository: sentry
Updated Branches:
  refs/heads/sentry-ha-redesign d5176b2ea -> b98b587e9


SENTRY-1619: Fix the secure HMS connection code in HMSFollower (Vamsee
Yarlagadda, Reviewed by: Alexander Kolbasov, Kalyan Kumar Kalvagadda,
Hao Hao)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/b98b587e
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/b98b587e
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/b98b587e

Branch: refs/heads/sentry-ha-redesign
Commit: b98b587e9e3d8a4598108d1c4887a74e5b116aae
Parents: d5176b2
Author: Vamsee Yarlagadda <va...@cloudera.com>
Authored: Wed Feb 1 20:56:04 2017 -0800
Committer: Vamsee Yarlagadda <va...@cloudera.com>
Committed: Thu Feb 2 18:09:05 2017 -0800

----------------------------------------------------------------------
 .../sentry/service/thrift/HMSFollower.java      | 80 ++++++++++----------
 1 file changed, 39 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/b98b587e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
index 749c2ce..1f05ba9 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
@@ -26,7 +26,9 @@ import org.apache.hadoop.hive.metastore.api.CurrentNotificationEventId;
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.hadoop.hive.metastore.api.NotificationEvent;
 import org.apache.hadoop.hive.metastore.api.NotificationEventResponse;
+import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.security.SaslRpcServer;
+import org.apache.hadoop.security.SecurityUtil;
 import org.apache.hive.hcatalog.messaging.HCatEventMessage;
 import org.apache.sentry.binding.hive.conf.HiveAuthzConf;
 import org.apache.sentry.core.common.exception.*;
@@ -42,6 +44,7 @@ import org.apache.sentry.binding.metastore.messaging.json.*;
 import javax.security.auth.Subject;
 import javax.security.auth.login.LoginException;
 import java.io.File;
+import java.io.IOException;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.List;
@@ -67,8 +70,6 @@ public class HMSFollower implements Runnable {
   private boolean kerberos;
   private final SentryStore sentryStore;
   private String hiveInstance;
-  private static final int maxRetriesForLogin = 3;
-  private static final int maxRetriesForConnection = 3;
 
   private volatile UpdateableAuthzPaths authzPaths;
   private boolean needHiveSnapshot = true;
@@ -103,7 +104,7 @@ public class HMSFollower implements Runnable {
    * Throws @MetaException if there was a problem on creating an HMSClient
    */
   private HiveMetaStoreClient getMetaStoreClient(Configuration conf)
-      throws LoginException, MetaException {
+      throws LoginException, MetaException, PrivilegedActionException {
     if(client != null) {
       return client;
     }
@@ -121,15 +122,20 @@ public class HMSFollower implements Runnable {
 
     //TODO: Is this the right(standard) way to create a HMS client? HiveMetastoreClientFactoryImpl?
     //TODO: Check if HMS is using kerberos instead of relying on Sentry conf
-    //TODO: Handle TGT renewals
     kerberos = ServiceConstants.ServerConfig.SECURITY_MODE_KERBEROS.equalsIgnoreCase(
         conf.get(ServiceConstants.ServerConfig.SECURITY_MODE, ServiceConstants.ServerConfig.SECURITY_MODE_KERBEROS).trim());
     if (kerberos) {
       LOGGER.info("Making a kerberos connection to HMS");
-      //TODO: Is this needed? Use Hadoop libraries to translate the _HOST placeholder with actual hostname
-      //Validate principal
-      principal = Preconditions.checkNotNull(ServiceConstants.ServerConfig.PRINCIPAL,
-          ServiceConstants.ServerConfig.PRINCIPAL + " is required");
+      try {
+        int port = conf.getInt(ServiceConstants.ServerConfig.RPC_PORT, ServiceConstants.ServerConfig.RPC_PORT_DEFAULT);
+        String rawPrincipal = Preconditions.checkNotNull(conf.get(ServiceConstants.ServerConfig.PRINCIPAL),
+            ServiceConstants.ServerConfig.PRINCIPAL + " is required");
+        principal = SecurityUtil.getServerPrincipal(rawPrincipal, NetUtils.createSocketAddr(
+            conf.get(ServiceConstants.ServerConfig.RPC_ADDRESS, ServiceConstants.ServerConfig.RPC_ADDRESS_DEFAULT), port).getAddress());
+      } catch(IOException io) {
+        throw new RuntimeException("Can't translate kerberos principal'", io);
+      }
+
       LOGGER.info("Using kerberos principal: " + principal);
       final String[] principalParts = SaslRpcServer.splitKerberosName(principal);
       Preconditions.checkArgument(principalParts.length == 3,
@@ -140,39 +146,31 @@ public class HMSFollower implements Runnable {
       File keytabFile = new File(keytab);
       Preconditions.checkState(keytabFile.isFile() && keytabFile.canRead(),
           "Keytab " + keytab + " does not exist or is not readable.");
-      boolean establishedKerberosContext = false;
-      int attempt = 1;
-      while(establishedKerberosContext) {
-        try {
-          kerberosContext = new SentryKerberosContext(principal, keytab, true);
-          establishedKerberosContext = true;
-          LOGGER.info("Established kerberos context, will now connect to HMS");
-        } catch (LoginException e) {
-          //Kerberos login failed
-          if( attempt > maxRetriesForLogin ) {
-            throw e;
-          }
-          attempt++;
-        }
-      }
-      boolean establishedConnection = false;
-      attempt = 1;
-      while(establishedConnection) {
-        try {
-          client = Subject.doAs(kerberosContext.getSubject(), new PrivilegedExceptionAction<HiveMetaStoreClient>() {
-            @Override
-            public HiveMetaStoreClient run() throws Exception {
-              return new HiveMetaStoreClient(hiveConf);
-            }
-          });
-          LOGGER.info("Secure connection established with HMS");
-        } catch (PrivilegedActionException e) {
-          if( attempt > maxRetriesForConnection ) {
-            //We should just retry as it is possible that HMS is not ready yet to receive requests
-            //TODO: How do we differentiate between kerberos problem versus HMS not being up?
-            LOGGER.error("Cannot connect to HMS", e);
+
+      try {
+        // Instantiating SentryKerberosContext in non-server mode handles the ticket renewal.
+        kerberosContext = new SentryKerberosContext(principal, keytab, false);
+
+        // HiveMetaStoreClient handles the connection retry logic to HMS and can be configured using properties:
+        // hive.metastore.connect.retries, hive.metastore.client.connect.retry.delay
+        client = Subject.doAs(kerberosContext.getSubject(), new PrivilegedExceptionAction<HiveMetaStoreClient>() {
+          @Override
+          public HiveMetaStoreClient run() throws Exception {
+            return new HiveMetaStoreClient(hiveConf);
           }
-          attempt++;
+        });
+        LOGGER.info("Secure connection established with HMS");
+      } catch (LoginException e) {
+        // Kerberos login failed
+        LOGGER.error("Failed to setup kerberos context.");
+        throw e;
+      } catch (PrivilegedActionException e) {
+        LOGGER.error("Failed to setup secure connection to HMS.");
+        throw e;
+      } finally {
+        // Shutdown kerberos context if HMS connection failed to setup to avoid thread leaks.
+        if (kerberosContext != null && client == null) {
+          kerberosContext.shutDown();
         }
       }
     } else {
@@ -196,7 +194,7 @@ public class HMSFollower implements Runnable {
           LOGGER.info("HMSFollower of Sentry successfully connected to HMS");
         }
       } catch (Exception e) {
-        LOGGER.error("HMSFollower cannot connect to HMS!!");
+        LOGGER.error("HMSFollower cannot connect to HMS!!", e);
         return;
       }
     }