SENTRY-1705: Do not start HMSFollower if Hive isn't configured (Na Li, reviewed by Alex Kolbasov)
Change-Id: Iaec465993fd51faa3f199a0cfdb9f410881f04a1 Reviewed-on: http://gerrit.sjc.cloudera.com:8080/22861 Reviewed-by: Sergio Pena <[email protected]> Tested-by: Jenkins User Reviewed-by: Alexander Kolbasov <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/a0981fcf Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/a0981fcf Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/a0981fcf Branch: refs/for/cdh5-1.5.1_ha Commit: a0981fcfff0506a16ca1cb008a8341f2ea41f770 Parents: f241918 Author: Alexander Kolbasov <[email protected]> Authored: Thu May 18 23:27:48 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Fri May 19 13:08:34 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 | 24 ++++++------ 5 files changed, 78 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/a0981fcf/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 e11bead..2044bd3 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 @@ -57,7 +57,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; @@ -131,17 +130,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? @@ -201,7 +189,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/a0981fcf/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 3afc06f..8dfdf1f 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 @@ -285,6 +285,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); @@ -314,6 +322,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/a0981fcf/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 50e413c..524cccc 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 @@ -26,6 +26,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.slf4j.Logger; public final class SentryServiceUtil { @@ -84,6 +86,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/a0981fcf/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 a025009..eb4df2a 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 @@ -21,7 +21,11 @@ import static org.apache.sentry.provider.common.ProviderConstants.PRIVILEGE_PREF import static org.apache.sentry.provider.common.ProviderConstants.ROLE_SPLITTER; import static org.junit.Assert.assertTrue; 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; @@ -37,6 +41,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; @@ -296,6 +301,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/a0981fcf/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 d551bb2..f7194c0 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,18 +163,20 @@ public class HiveServerFactory { properties.put(METASTORE_RAW_STORE_IMPL, "org.apache.sentry.binding.metastore.AuthorizingObjectStore"); - if (!properties.containsKey(METASTORE_URI)) { - 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"); + + 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"); + + 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"); - } - properties.put(ConfVars.METASTORESERVERMINTHREADS.varname, "5"); + "thrift://localhost:" + String.valueOf(findPort())); + } + + if (!properties.containsKey(METASTORE_HOOK)) { + properties.put(METASTORE_HOOK, + "org.apache.sentry.binding.metastore.MetastoreAuthzBinding"); } }
