You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by pr...@apache.org on 2015/02/26 21:01:53 UTC
incubator-sentry git commit: SENTRY-660: Support client principal and
keytab configuration properties for Sentry HA to work with secure zookeeper.
(Prasad Mujumdar, reviewed by Lenni Kuff)
Repository: incubator-sentry
Updated Branches:
refs/heads/master 0f8e5e452 -> fb783e930
SENTRY-660: Support client principal and keytab configuration properties for Sentry HA to work with secure zookeeper. (Prasad Mujumdar, reviewed by Lenni Kuff)
Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/fb783e93
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/fb783e93
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/fb783e93
Branch: refs/heads/master
Commit: fb783e930f12f5d57cecfcfcfd2ca5eb33502605
Parents: 0f8e5e4
Author: Prasad Mujumdar <pr...@apache.org>
Authored: Thu Feb 26 12:01:46 2015 -0800
Committer: Prasad Mujumdar <pr...@apache.org>
Committed: Thu Feb 26 12:01:46 2015 -0800
----------------------------------------------------------------------
.../org/apache/sentry/SentryUserException.java | 1 +
.../db/service/persistent/HAContext.java | 6 ++--
.../thrift/HAClientInvocationHandler.java | 30 +++++++++++++-------
.../sentry/service/thrift/ServiceConstants.java | 5 +++-
.../thrift/SentryServiceIntegrationBase.java | 4 +++
.../sentry/tests/e2e/ha/TestHaEnd2End.java | 5 ++--
.../apache/sentry/tests/e2e/hive/Context.java | 8 ++++--
7 files changed, 40 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryUserException.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryUserException.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryUserException.java
index 9e49a74..9a8f85d 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryUserException.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryUserException.java
@@ -29,6 +29,7 @@ public class SentryUserException extends Exception{
}
public SentryUserException(String msg, Throwable t) {
super(msg, t);
+ reason = t.getMessage();
}
public String getReason() {
return reason;
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java
index ed4da96..c3aa23c 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java
@@ -184,7 +184,7 @@ public class HAContext {
LOGGER.info("'sasl' ACLs not set; setting...");
List<String> children = curatorFramework.getZookeeperClient().getZooKeeper().getChildren(namespace, null);
for (String child : children) {
- checkAndSetACLs(child);
+ checkAndSetACLs(namespace + "/" + child);
}
curatorFramework.getZookeeperClient().getZooKeeper().setACL(namespace, saslACL, -1);
}
@@ -202,9 +202,9 @@ public class HAContext {
// This gets ignored during most tests, see ZKXTestCaseWithSecurity#setupZKServer()
private void setJaasConfiguration(Configuration conf) throws IOException {
- String keytabFile = conf.get(ServerConfig.KEY_TAB);
+ String keytabFile = conf.get(ServerConfig.SERVER_HA_ZOOKEEPER_CLIENT_KEYTAB);
Preconditions.checkArgument(keytabFile.length() != 0, "Keytab File is not right.");
- String principal = conf.get(ServerConfig.PRINCIPAL);
+ String principal = conf.get(ServerConfig.SERVER_HA_ZOOKEEPER_CLIENT_PRINCIPAL);
principal = SecurityUtil.getServerPrincipal(principal, conf.get(ServerConfig.RPC_ADDRESS));
Preconditions.checkArgument(principal.length() != 0, "Kerberos principal is not right.");
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
index 52be099..4947ad1 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
@@ -47,11 +47,11 @@ public class HAClientInvocationHandler implements InvocationHandler {
private SentryPolicyServiceClient client = null;
private static final String THRIFT_EXCEPTION_MESSAGE = "Thrift exception occured ";
+ public static final String SENTRY_HA_ERROR_MESSAGE = "No Sentry server available. Please ensure that at least one Sentry server is online";
public HAClientInvocationHandler(Configuration conf) throws Exception {
this.conf = conf;
checkClientConf();
- renewSentryClient();
}
@Override
@@ -63,6 +63,11 @@ public class HAClientInvocationHandler implements InvocationHandler {
if (!method.isAccessible()) {
method.setAccessible(true);
}
+ // The client is initialized in the first call instead of constructor.
+ // This way we can propagate the connection exception to caller cleanly
+ if (client == null) {
+ renewSentryClient();
+ }
result = method.invoke(client, args);
} catch (IllegalAccessException e) {
throw new SentryUserException(e.getMessage(), e.getCause());
@@ -72,18 +77,26 @@ public class HAClientInvocationHandler implements InvocationHandler {
} else {
LOGGER.warn(THRIFT_EXCEPTION_MESSAGE + ": Error in connect current" +
" service, will retry other service.", e);
- try {
- renewSentryClient();
- } catch (IOException e1) {
- throw new SentryUserException(e1.getMessage(), e1.getCause());
+ if (client != null) {
+ client.close();
+ client = null;
}
}
-
+ } catch (IOException e1) {
+ // close() doesn't throw exception we supress that in case of connection
+ // loss. Changing SentryPolicyServiceClient#close() to throw an
+ // exception would be a backward incompatible change for Sentry clients.
+ if ("close".equals(method.getName())) {
+ return null;
+ }
+ throw new SentryUserException("Error connecting to sentry service "
+ + e1.getMessage(), e1);
}
return result;
}
}
+ // Retrieve the new connection endpoint from ZK and connect to new server
private void renewSentryClient() throws IOException {
try {
manager = new ServiceManager(HAContext.getHAContext(conf));
@@ -93,12 +106,9 @@ public class HAClientInvocationHandler implements InvocationHandler {
try {
while (true) {
- if (client != null) {
- client.close();
- }
currentServiceInstance = manager.getServiceInstance();
if (currentServiceInstance == null) {
- throw new IOException("No avaiable node.");
+ throw new IOException(SENTRY_HA_ERROR_MESSAGE);
}
InetSocketAddress serverAddress =
ServiceManager.convertServiceInstance(currentServiceInstance);
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
index 8ef586e..c8f7450 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
@@ -122,7 +122,9 @@ public class ServiceConstants {
public static final int SENTRY_HA_ZOOKEEPER_SLEEP_BETWEEN_RETRIES_MS_DEFAULT = 100;
public static final String SENTRY_HA_ZOOKEEPER_NAMESPACE = SENTRY_HA_ZK_PROPERTY_PREFIX + "namespace";
public static final String SENTRY_HA_ZOOKEEPER_NAMESPACE_DEFAULT = "sentry";
-
+ // principal and keytab for client to be able to connect to secure ZK. Needed for Sentry HA with secure ZK
+ public static final String SERVER_HA_ZOOKEEPER_CLIENT_PRINCIPAL = "sentry.zookeeper.client.principal";
+ public static final String SERVER_HA_ZOOKEEPER_CLIENT_KEYTAB = "sentry.zookeeper.client.keytab";
public static final ImmutableMap<String, String> SENTRY_STORE_DEFAULTS =
ImmutableMap.<String, String>builder()
.put("datanucleus.connectionPoolingType", "BoneCP")
@@ -179,6 +181,7 @@ public class ServiceConstants {
public static final String SERVER_HA_ZOOKEEPER_QUORUM_DEFAULT = ServerConfig.SENTRY_HA_ZOOKEEPER_QUORUM_DEFAULT;
public static final String SENTRY_HA_ZOOKEEPER_NAMESPACE = ServerConfig.SENTRY_HA_ZOOKEEPER_NAMESPACE;
public static final String SERVER_HA_ZOOKEEPER_NAMESPACE_DEFAULT = ServerConfig.SENTRY_HA_ZOOKEEPER_NAMESPACE_DEFAULT;
+
}
/**
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
index a8da078..9a6f8c4 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
@@ -136,6 +136,10 @@ public abstract class SentryServiceIntegrationBase extends SentryMiniKdcTestcase
conf.set(ServerConfig.PRINCIPAL, getServerKerberosName());
conf.set(ServerConfig.KEY_TAB, serverKeytab.getPath());
conf.set(ServerConfig.ALLOW_CONNECT, CLIENT_KERBEROS_SHORT_NAME);
+ conf.set(ServerConfig.SERVER_HA_ZOOKEEPER_CLIENT_PRINCIPAL,
+ getServerKerberosName());
+ conf.set(ServerConfig.SERVER_HA_ZOOKEEPER_CLIENT_KEYTAB,
+ serverKeytab.getPath());
conf.set(ServerConfig.SECURITY_USE_UGI_TRANSPORT, "false");
clientSubject = new Subject(false, Sets.newHashSet(
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java
index 78894d1..70828da 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java
@@ -25,8 +25,8 @@ import java.sql.Statement;
import org.apache.sentry.provider.db.SentryAccessDeniedException;
import org.apache.sentry.provider.file.PolicyFile;
+import org.apache.sentry.service.thrift.HAClientInvocationHandler;
import org.apache.sentry.tests.e2e.hive.AbstractTestWithStaticConfiguration;
-import org.apache.sentry.tests.e2e.hive.StaticUserGroup;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -133,7 +133,8 @@ public class TestHaEnd2End extends AbstractTestWithStaticConfiguration {
// stop both servers and verify it fails
getSentrySrv().stop(0);
getSentrySrv().stop(1);
- context.assertAuthzException(adminStmt, "CREATE ROLE " + roleName3);
+ context.assertAuthzExecHookException(adminStmt, "CREATE ROLE " + roleName3,
+ HAClientInvocationHandler.SENTRY_HA_ERROR_MESSAGE);
getSentrySrv().start(0);
getSentrySrv().start(1);
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/fb783e93/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
index 69743bc..f600fdf 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
@@ -173,13 +173,15 @@ public class Context {
}
}
- public void assertAuthzExecHookException(Statement statement, String query)
- throws SQLException {
+ public void assertAuthzExecHookException(Statement statement, String query,
+ String expectedMsg) throws SQLException {
try {
statement.execute(query);
Assert.fail("Expected SQLException for '" + query + "'");
} catch (SQLException e) {
verifyAuthzExecHookException(e);
+ assertNotNull(e.getMessage());
+ assertTrue(e.getMessage().contains(expectedMsg));
}
}
@@ -283,7 +285,7 @@ public class Context {
/**
* Execute "set x" and extract value from key=val format result Verify the
* extracted value
- *
+ *
* @param stmt
* @return
* @throws Exception