Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign 81eed11a9 -> 71de208d8
SENTRY-1705: Do not start HMSFollower if Hive isn't configured (Na Li, reviewed by Alex Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/71de208d Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/71de208d Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/71de208d Branch: refs/heads/sentry-ha-redesign Commit: 71de208d8eed1f3a949530877c3778985245f2d3 Parents: 81eed11 Author: Alexander Kolbasov <[email protected]> Authored: Thu May 18 23:27:48 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Thu May 18 23:27:48 2017 -0700 ---------------------------------------------------------------------- .../sentry/service/thrift/HMSFollower.java | 14 +------ .../sentry/service/thrift/SentryService.java | 16 ++++++++ .../service/thrift/SentryServiceUtil.java | 7 ++++ .../AbstractTestWithStaticConfiguration.java | 41 ++++++++++++++++++++ .../e2e/hive/hiveserver/HiveServerFactory.java | 10 +++-- 5 files changed, 72 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/71de208d/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 59eda52..58e8881 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 @@ -59,7 +59,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; -import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.METASTOREURIS; import static org.apache.sentry.binding.hive.conf.HiveAuthzConf.AuthzConfVars.AUTHZ_SYNC_CREATE_WITH_POLICY_STORE; import static org.apache.sentry.binding.hive.conf.HiveAuthzConf.AuthzConfVars.AUTHZ_SYNC_DROP_WITH_POLICY_STORE; import static org.apache.sentry.hdfs.Updateable.Update; @@ -140,17 +139,6 @@ public class HMSFollower implements Runnable, AutoCloseable { final HiveConf hiveConf = new HiveConf(); hiveInstance = hiveConf.get(HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME.getVar()); - // Do not create client if the metastore URI in the Hive configuration is missing. - // E2e Hive tests start Sentry first and then configure Hive, so eventually the - // metastore URI will appear in the config in such cases. - String metaStoreUri = hiveConf.get(METASTOREURIS.varname); - if (metaStoreUri == null) { - LOGGER.error("Metastore uri is not configured in hive config."); - return null; - } else { - LOGGER.info("Connecting to HMS {}", metaStoreUri); - } - String principal, keytab; //TODO: Is this the right(standard) way to create a HMS client? HiveMetastoreClientFactoryImpl? @@ -210,7 +198,7 @@ public class HMSFollower implements Runnable, AutoCloseable { } else { //This is only for testing purposes. Sentry strongly recommends strong authentication client = new HiveMetaStoreClient(hiveConf); - LOGGER.info("Non secure connection established with HMS at {}", metaStoreUri); + LOGGER.info("Established non-secure connection with HMS"); } return client; } http://git-wip-us.apache.org/repos/asf/sentry/blob/71de208d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java index 35289aa..9beb07b 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java @@ -286,6 +286,14 @@ public class SentryService implements Callable, SigUtils.SigListener { return; } + String metastoreURI = SentryServiceUtil.getHiveMetastoreURI(); + if (metastoreURI == null) { + LOGGER.info("Metastore uri is not configured. Do not start HMSFollower"); + return; + } + + LOGGER.info("Starting HMSFollower to HMS {}", metastoreURI); + Preconditions.checkState(hmsFollower == null); Preconditions.checkState(hmsFollowerExecutor == null); @@ -315,6 +323,14 @@ public class SentryService implements Callable, SigUtils.SigListener { return; } + if ((hmsFollowerExecutor == null) || (hmsFollower == null)) { + Preconditions.checkState(hmsFollower == null); + Preconditions.checkState(hmsFollowerExecutor == null); + + LOGGER.debug("Skip shuting down hmsFollowerExecutor and closing hmsFollower because they are not created"); + return; + } + Preconditions.checkNotNull(hmsFollowerExecutor); Preconditions.checkNotNull(hmsFollower); http://git-wip-us.apache.org/repos/asf/sentry/blob/71de208d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java index 9470437..215f7d5 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java @@ -28,6 +28,8 @@ import java.util.concurrent.TimeUnit; import com.google.common.base.Preconditions; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.conf.Configuration; +import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.METASTOREURIS; +import org.apache.hadoop.hive.conf.HiveConf; import org.apache.sentry.core.common.utils.SentryConstants; import org.apache.sentry.core.common.utils.KeyValue; import org.apache.sentry.core.common.utils.PolicyFileConstants; @@ -212,6 +214,11 @@ public final class SentryServiceUtil { && policyStorePlugins.contains("org.apache.sentry.hdfs.SentryPlugin"); } + static String getHiveMetastoreURI() { + HiveConf hiveConf = new HiveConf(); + return hiveConf.get(METASTOREURIS.varname); + } + private SentryServiceUtil() { // Make constructor private to avoid instantiation } http://git-wip-us.apache.org/repos/asf/sentry/blob/71de208d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java index e2fb36a..fe68f49 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java @@ -17,7 +17,11 @@ package org.apache.sentry.tests.e2e.hive; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; +import java.net.ServerSocket; +import java.net.URL; import java.security.PrivilegedExceptionAction; import java.sql.Connection; import java.sql.ResultSet; @@ -31,6 +35,7 @@ import java.util.HashSet; import com.google.common.collect.Sets; import org.apache.sentry.tests.e2e.hive.fs.TestFSContants; +import org.fest.reflect.core.Reflection; import org.junit.After; import org.junit.AfterClass; import org.junit.Assert; @@ -295,6 +300,7 @@ public abstract class AbstractTestWithStaticConfiguration extends RulesForE2ETes "org.apache.hadoop.hive.ql.lockmgr.EmbeddedLockManager"); } if (useSentryService && (!startSentry)) { + configureHiveAndMetastoreForSentry(); setupSentryService(); } @@ -443,6 +449,41 @@ public abstract class AbstractTestWithStaticConfiguration extends RulesForE2ETes } } + private static int findPort() throws IOException { + ServerSocket socket = new ServerSocket(0); + int port = socket.getLocalPort(); + socket.close(); + return port; + } + + private static HiveConf configureHiveAndMetastoreForSentry() throws IOException, InterruptedException { + HiveConf hiveConf = new HiveConf(); + int hmsPort = findPort(); + LOGGER.info("\n\n HMS port : " + hmsPort + "\n\n"); + + // Sets hive.metastore.authorization.storage.checks to true, so that + // disallow the operations such as drop-partition if the user in question + // doesn't have permissions to delete the corresponding directory + // on the storage. + hiveConf.set("hive.metastore.authorization.storage.checks", "true"); + hiveConf.set("hive.metastore.uris", "thrift://localhost:" + hmsPort); + hiveConf.set("sentry.metastore.service.users", "hive");// queries made by hive user (beeline) skip meta store check + + File confDir = assertCreateDir(new File(baseDir, "etc")); + File hiveSite = new File(confDir, "hive-site.xml"); + hiveConf.set("hive.server2.enable.doAs", "false"); + OutputStream out = new FileOutputStream(hiveSite); + hiveConf.writeXml(out); + out.close(); + + Reflection.staticField("hiveSiteURL") + .ofType(URL.class) + .in(HiveConf.class) + .set(hiveSite.toURI().toURL()); + + return hiveConf; + } + private static void setupSentryService() throws Exception { sentryConf = new Configuration(false); http://git-wip-us.apache.org/repos/asf/sentry/blob/71de208d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java index 0ca7704..f3f58f6 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java @@ -163,12 +163,16 @@ public class HiveServerFactory { properties.put(METASTORE_RAW_STORE_IMPL, "org.apache.sentry.binding.metastore.AuthorizingObjectStore"); - if (!properties.containsKey(METASTORE_URI) && HiveServer2Type.InternalMetastore.equals(type)) { + if (HiveServer2Type.InternalMetastore.equals(type)) { // The configuration sentry.metastore.service.users is for the user who // has all access to get the metadata. properties.put(METASTORE_BYPASS, "accessAllMetaUser"); - properties.put(METASTORE_URI, - "thrift://localhost:" + String.valueOf(findPort())); + + if (!properties.containsKey(METASTORE_URI)) { + properties.put(METASTORE_URI, + "thrift://localhost:" + String.valueOf(findPort())); + } + if (!properties.containsKey(METASTORE_HOOK)) { properties.put(METASTORE_HOOK, "org.apache.sentry.binding.metastore.MetastoreAuthzBinding");
