SENTRY-1757: Avoid using local hive meta store with wrong configuration (Na Li, reviewed by Alex Kolbasov and Vamsee Yarlagadda)
Change-Id: I40905325c1bc10c980df87fe8750b0a9563a440f Reviewed-on: http://gerrit.sjc.cloudera.com:8080/22812 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/20381607 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/20381607 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/20381607 Branch: refs/for/cdh5-1.5.1_ha Commit: 2038160725c52585ca751927336142146003d3af Parents: 71a1214 Author: Alexander Kolbasov <[email protected]> Authored: Wed May 17 15:39:30 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Wed May 17 22:41:15 2017 -0700 ---------------------------------------------------------------------- .../sentry/service/thrift/HMSFollower.java | 29 ++--- .../e2e/hdfs/TestHDFSIntegrationAdvanced.java | 20 +-- .../tests/e2e/hdfs/TestHDFSIntegrationBase.java | 122 ++++++++++++++++--- .../e2e/hdfs/TestHDFSIntegrationEnd2End.java | 2 +- 4 files changed, 130 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/20381607/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 7f9b706..211b22d 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,6 +57,7 @@ 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; @@ -71,8 +72,6 @@ import static org.apache.sentry.hdfs.Updateable.Update; public class HMSFollower implements Runnable, AutoCloseable { private static final Logger LOGGER = LoggerFactory.getLogger(HMSFollower.class); private long currentEventID; - // Copy from Hive - public static String CONF_METASTORE_URI = "hive.metastore.uris"; // Track the latest eventId of the event that has been logged. So we don't log the same message private long lastLoggedEventId = SentryStore.EMPTY_CHANGE_ID; private static boolean connectedToHMS = false; @@ -130,21 +129,19 @@ public class HMSFollower implements Runnable, AutoCloseable { } final HiveConf hiveConf = new HiveConf(); - // Do not create client if the metastre URI in the configuration is missing - if (hiveConf.get(CONF_METASTORE_URI, "").isEmpty()) { - // HDFS e2e tests add metastore URI to the sentry config, so it might be there - String metastoreUri = conf.get(CONF_METASTORE_URI); - if (metastoreUri == null) { - // Come back later with real Hive URI - return null; - } - // Ok, we found metastore URI in Sentry config (thanks to the tests setup), copy it to - // the Hive config - hiveConf.set(CONF_METASTORE_URI, metastoreUri); - } - 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? @@ -204,7 +201,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"); + LOGGER.info("Non secure connection established with HMS at {}", metaStoreUri); } return client; } http://git-wip-us.apache.org/repos/asf/sentry/blob/20381607/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java index 57d6cf4..9e56315 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java @@ -550,7 +550,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("create role table_role"); stmt.execute("grant all on table tb1 to role table_role"); stmt.execute("grant role table_role to group " + StaticUserGroup.USERGROUP1); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode //Verify user1 is able to access table directory verifyAccessToPath(StaticUserGroup.USER1_1, StaticUserGroup.USERGROUP1, "/user/hive/warehouse/db1.db/tb1", true); @@ -586,7 +586,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("create role tab1_role"); stmt.execute("grant insert on table tab1 to role tab1_role"); stmt.execute("grant role tab1_role to group " + StaticUserGroup.USERGROUP1); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode // Verify that user_group1 has insert(write_execute) permission on '/tmp/external/p1'. verifyOnAllSubDirs("/tmp/external/p1", FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP1, true); @@ -598,7 +598,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("create role tab2_role"); stmt.execute("grant select on table tab2 to role tab2_role"); stmt.execute("grant role tab2_role to group " + StaticUserGroup.USERGROUP2); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode // Verify that user_group2 have select(read_execute) permission on both paths. verifyOnAllSubDirs("/user/hive/warehouse/" + dbName + ".db/tab2", FsAction.READ_EXECUTE, StaticUserGroup.USERGROUP2, true); @@ -611,7 +611,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("create role tab3_role"); stmt.execute("grant insert on table tab3 to role tab3_role"); stmt.execute("grant role tab3_role to group " + StaticUserGroup.USERGROUP3); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode // When two partitions of different tables pointing to the same location with different grants, // ACLs should have union (no duplicates) of both rules. @@ -621,21 +621,21 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { // When alter the table name (tab2 to be tabx), ACLs should remain the same. stmt.execute("alter table tab2 rename to tabx"); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode verifyOnPath(tmpHDFSDirStr, FsAction.READ_EXECUTE, StaticUserGroup.USERGROUP2, true); verifyOnPath(tmpHDFSDirStr, FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP3, true); // When drop a partition that shares the same location with other partition belonging to // other table, should still have the other table permissions. stmt.execute("ALTER TABLE tabx DROP PARTITION (month = 1)"); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode verifyOnAllSubDirs("/user/hive/warehouse/" + dbName + ".db/tab3", FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP3, true); verifyOnPath(tmpHDFSDirStr, FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP3, true); // When drop a table that has a partition shares the same location with other partition // belonging to other table, should still have the other table permissions. stmt.execute("DROP TABLE IF EXISTS tabx"); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode verifyOnAllSubDirs("/user/hive/warehouse/" + dbName + ".db/tab3", FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP3, true); verifyOnPath(tmpHDFSDirStr, FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP3, true); @@ -674,7 +674,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("create role tab1_role"); stmt.execute("grant insert on table tab1 to role tab1_role"); stmt.execute("grant role tab1_role to group " + StaticUserGroup.USERGROUP1); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode // Verify that user_group1 has insert(write_execute) permission on '/tmp/external/p1'. verifyOnAllSubDirs("/tmp/external/p1", FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP1, true); @@ -716,7 +716,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("create role tab1_role"); stmt.execute("grant insert on table tab1 to role tab1_role"); stmt.execute("grant role tab1_role to group " + StaticUserGroup.USERGROUP1); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode // Verify that user_group1 has insert(write_execute) permission on '/tmp/external/p1'. verifyOnAllSubDirs("/tmp/external/p1", FsAction.WRITE_EXECUTE, StaticUserGroup.USERGROUP1, true); @@ -734,7 +734,7 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { // When drop table tab1, ACLs of tab2 still remain. stmt.execute("DROP TABLE IF EXISTS tab1"); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode verifyOnPath("/tmp/external/p1", FsAction.READ_EXECUTE, StaticUserGroup.USERGROUP1, true); stmt.close(); http://git-wip-us.apache.org/repos/asf/sentry/blob/20381607/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java index eb290a9..4bec45b 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java @@ -149,10 +149,26 @@ public abstract class TestHDFSIntegrationBase { protected boolean ignoreCleanUp = false; protected static final long STALE_THRESHOLD = 5000; - // we want to make sure the cache is updated in our tests so that changes reflect soon - protected static final long CACHE_REFRESH = + // It is the interval in milliseconds that hdfs uses to get acl from sentry. Default is 500, but + // we want it to be low in our tests so that changes reflect soon + protected static final long CACHE_REFRESH = 100; + + // It is Used to wait before verifying result in test. + // We want to make sure the cache is updated in our tests so that changes reflect soon. The unit is milliseconds + // It takes at most (ServerConfig.SENTRY_HMSFOLLOWER_INIT_DELAY_MILLS_DEFAULT + ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS_DEFAULT) + // for sentry to get the path configured in Hive, adding ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS_DEFAULT to be safe. + // And then it takes at most CACHE_REFRESH for HDFS to get this from sentry, adding CACHE_REFRESH to be sure + protected static final long WAIT_BEFORE_TESTVERIFY = ServerConfig.SENTRY_HMSFOLLOWER_INIT_DELAY_MILLS_DEFAULT + - ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS_DEFAULT * 2; + ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS_DEFAULT * 2 + CACHE_REFRESH * 2; + + // Time to wait before running next tests. The unit is milliseconds. + // Deleting HDFS may finish, but HDFS may not be ready for creating the same file again. + // We need to to make sure that creating the same file in the next test will succeed + // If we don't wait, next test may get exception similar to + // "org.apache.hadoop.security.AccessControlException Permission denied: user=hive, access=EXECUTE, + // inode="/tmp/external/p1":hdfs:hdfs:drwxrwx---" + protected static final long WAIT_BEFORE_NEXTTEST = 50; protected static String fsURI; protected static int hmsPort; @@ -428,19 +444,33 @@ public abstract class TestHDFSIntegrationBase { hiveUgi = UserGroupInformation.createUserForTesting( "hive", new String[] { "hive" }); - // Start Sentry + // Create SentryService and its internal objects. + // Set Sentry port + createSentry(); + + // Create hive-site.xml that contains the metastore uri + // it is used by HMSFollower + configureHiveAndMetastoreForSentry(); + + // Start SentryService after Hive configuration hive-site.xml is available + // So HMSFollower can contact metastore using its URI startSentry(); - // Start HDFS and MR + // Start HDFS and MR with Sentry Port. Set fsURI startDFSandYARN(); - // Start HiveServer2 and Metastore - startHiveAndMetastore(); + // Configure Hive and Metastore with Sentry Port and fsURI + // Read src/test/resources/sentry-site.xml. + // Create hive-site.xml and sentry-site.xml used by Hive. + HiveConf hiveConf = configureHiveAndMetastore(); + // Start Hive and Metastore after SentryService is started + startHiveAndMetastore(hiveConf); } @Before public void setUpTempDir() throws IOException { + LOGGER.debug("setUpTempDir starts"); tmpHDFSDirStr = "/tmp/external"; tmpHDFSPartitionStr = tmpHDFSDirStr + "/p1"; tmpHDFSDir = new Path(tmpHDFSDirStr); @@ -455,17 +485,50 @@ public abstract class TestHDFSIntegrationBase { miniDFS.getFileSystem().delete(partitionDir, true); } Assert.assertTrue(miniDFS.getFileSystem().mkdirs(partitionDir)); + LOGGER.debug("setUpTempDir ends"); } - private static void startHiveAndMetastore() throws IOException, InterruptedException { - startHiveAndMetastore(NUM_RETRIES); + private static HiveConf configureHiveAndMetastoreForSentry() throws IOException, InterruptedException { + final HiveConf hiveConfiguration = new HiveConf(); + hiveUgi.doAs(new PrivilegedExceptionAction<Void>() { + @Override + public Void run() throws Exception { + HiveConf hiveConf = hiveConfiguration; + 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 null; + } + }); + + return hiveConfiguration; } - private static void startHiveAndMetastore(final int retries) throws IOException, InterruptedException { + private static HiveConf configureHiveAndMetastore() throws IOException, InterruptedException { + final HiveConf hiveConfiguration = new HiveConf(); hiveUgi.doAs(new PrivilegedExceptionAction<Void>() { @Override public Void run() throws Exception { - HiveConf hiveConf = new HiveConf(); + HiveConf hiveConf = hiveConfiguration; hiveConf.set("sentry.metastore.plugins", "org.apache.sentry.hdfs.MetastorePlugin"); hiveConf.set("sentry.service.client.server.rpc-address", "localhost"); hiveConf.set("sentry.hdfs.service.client.server.rpc-address", "localhost"); @@ -496,8 +559,6 @@ public abstract class TestHDFSIntegrationBase { hiveConf.set("datanucleus.autoCreateSchema", "true"); hiveConf.set("datanucleus.fixedDatastore", "false"); hiveConf.set("datanucleus.autoStartMechanism", "SchemaTable"); - 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 @@ -537,7 +598,21 @@ public abstract class TestHDFSIntegrationBase { .ofType(URL.class) .in(HiveConf.class) .set(hiveSite.toURI().toURL()); + return null; + } + }); + + return hiveConfiguration; + } + private static void startHiveAndMetastore(HiveConf hiveConfig) throws IOException, InterruptedException { + startHiveAndMetastore(hiveConfig, NUM_RETRIES); + } + + private static void startHiveAndMetastore(final HiveConf hiveConf, final int retries) throws IOException, InterruptedException { + hiveUgi.doAs(new PrivilegedExceptionAction<Void>() { + @Override + public Void run() throws Exception { metastore = new InternalMetastoreServer(hiveConf); try { metastore.start(); @@ -668,6 +743,22 @@ public abstract class TestHDFSIntegrationBase { private static void startSentry() throws Exception { try { + hiveUgi.doAs(new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + sentryServer.startAll(); + LOGGER.info("\n\n Sentry service started \n\n"); + return null; + } + }); + } catch (Exception ex) { + //An exception happening in above block will result in a wrapped UndeclaredThrowableException. + throw new Exception(ex.getCause()); + } + } + + private static void createSentry() throws Exception { + try { hiveUgi.doAs(new PrivilegedExceptionAction<Void>() { @Override @@ -680,7 +771,6 @@ public abstract class TestHDFSIntegrationBase { SentryHiveAuthorizationTaskFactoryImpl.class.getName()); properties .put(ConfVars.HIVE_SERVER2_THRIFT_MIN_WORKER_THREADS.varname, "2"); - properties.put("hive.metastore.uris", "thrift://localhost:" + hmsPort); properties.put("hive.exec.local.scratchdir", Files.createTempDir().getAbsolutePath()); properties.put(ServerConfig.SECURITY_MODE, ServerConfig.SECURITY_MODE_NONE); // properties.put("sentry.service.server.compact.transport", "true"); @@ -710,8 +800,7 @@ public abstract class TestHDFSIntegrationBase { sentryServer = SentrySrvFactory.create(SentrySrvFactory.SentrySrvType.INTERNAL_SERVER, sentryConf, testSentryHA ? 2 : 1); sentryPort = sentryServer.get(0).getAddress().getPort(); - sentryServer.startAll(); - LOGGER.info("\n\n Sentry service started \n\n"); + LOGGER.info("Sentry service is created on port {}", sentryPort); return null; } }); @@ -757,6 +846,7 @@ public abstract class TestHDFSIntegrationBase { dbNames = null; roles = null; admin = null; + Thread.sleep(WAIT_BEFORE_NEXTTEST); // make sure the clean up is done before next test starts. otherwise, the next test may fail } @AfterClass http://git-wip-us.apache.org/repos/asf/sentry/blob/20381607/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java index b7a9109..09d75f7 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java @@ -504,7 +504,7 @@ TODO:SENTRY-819 stmt.execute("grant role col_role to group " + StaticUserGroup.ADMINGROUP); - Thread.sleep(CACHE_REFRESH);//Wait till sentry cache is updated in Namenode + Thread.sleep(WAIT_BEFORE_TESTVERIFY);//Wait till sentry cache is updated in Namenode //User with just column level privileges cannot read HDFS verifyOnAllSubDirs("/user/hive/warehouse/" + dbName + ".db/p1", null, StaticUserGroup.USERGROUP1, false);
