Repository: sentry Updated Branches: refs/heads/master ed965d871 -> 5acaa8ee2
SENTRY-1471: TestHDFSIntegrationBase.java implements HDFS ACL checking and query verification incorrectly. Also disable testAuthzObjOnPartitionMultipleTables until it gets fixed. ( Vadim Spector, reviewed by Anne Yu) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/5acaa8ee Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/5acaa8ee Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/5acaa8ee Branch: refs/heads/master Commit: 5acaa8ee265e8c83926013b208b786aa4083f5f5 Parents: ed965d8 Author: Anne Yu <[email protected]> Authored: Thu Sep 15 21:40:14 2016 -0700 Committer: Anne Yu <[email protected]> Committed: Thu Sep 15 21:40:14 2016 -0700 ---------------------------------------------------------------------- .../e2e/hdfs/TestHDFSIntegrationAdvanced.java | 15 +++++--- .../tests/e2e/hdfs/TestHDFSIntegrationBase.java | 37 +++++++++----------- 2 files changed, 27 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/5acaa8ee/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 8a425c9..1b5eb53 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 @@ -29,6 +29,7 @@ import org.apache.sentry.hdfs.PathsUpdate; import org.apache.sentry.tests.e2e.hive.StaticUserGroup; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.slf4j.Logger; @@ -130,8 +131,9 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { //HDFS to local file system, also make sure does not specifying scheme still works stmt.execute("create external table tab3(a int) location '/tmp/external/tab3_loc'"); // SENTRY-546 - // verifyOnAllSubDirs("/tmp/external/tab3_loc", FsAction.ALL, StaticUserGroup.USERGROUP1, true); - verifyOnAllSubDirs("/tmp/external/tab3_loc", null, StaticUserGroup.USERGROUP1, true); + // SENTRY-1471 - fixing the validation logic revealed that FsAction.ALL is the right value. + verifyOnAllSubDirs("/tmp/external/tab3_loc", FsAction.ALL, StaticUserGroup.USERGROUP1, true); + // verifyOnAllSubDirs("/tmp/external/tab3_loc", null, StaticUserGroup.USERGROUP1, true); stmt.execute("alter table tab3 set location 'file:///tmp/external/tab3_loc'"); verifyOnAllSubDirs("/tmp/external/tab3_loc", null, StaticUserGroup.USERGROUP1, false); @@ -140,8 +142,9 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { stmt.execute("alter table tab4 set location 'hdfs:///tmp/external/tab4_loc'"); miniDFS.getFileSystem().mkdirs(new Path("/tmp/external/tab4_loc")); // SENTRY-546 - // verifyOnAllSubDirs("/tmp/external/tab4_loc", FsAction.ALL, StaticUserGroup.USERGROUP1, true); - verifyOnAllSubDirs("/tmp/external/tab4_loc", null, StaticUserGroup.USERGROUP1, true); + // SENTRY-1471 - fixing the validation logic revealed that FsAction.ALL is the right value. + verifyOnAllSubDirs("/tmp/external/tab4_loc", FsAction.ALL, StaticUserGroup.USERGROUP1, true); + // verifyOnAllSubDirs("/tmp/external/tab4_loc", null, StaticUserGroup.USERGROUP1, true); stmt.close(); conn.close(); } @@ -557,6 +560,10 @@ public class TestHDFSIntegrationAdvanced extends TestHDFSIntegrationBase { } /* SENTRY-953 */ + /* SENTRY-1471 - fixing the validation logic revealed that this test is broken. + * Disabling this test for now; to be fixed in a separate JIRA. + */ + @Ignore @Test public void testAuthzObjOnPartitionMultipleTables() throws Throwable { String dbName = "db1"; http://git-wip-us.apache.org/repos/asf/sentry/blob/5acaa8ee/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 0cf018a..f52f9f9 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 @@ -205,14 +205,17 @@ public abstract class TestHDFSIntegrationBase { } protected void verifyOnAllSubDirs(Path p, FsAction fsAction, String group, boolean groupShouldExist, boolean recurse, int retry) throws Throwable { - Assert.assertTrue("Failed to verify ACLs on path and its children: " + p.getName(), - verifyOnAllSubDirsHelper(p, fsAction, group, groupShouldExist, recurse, retry)); + verifyOnAllSubDirsHelper(p, fsAction, group, groupShouldExist, recurse, retry); } - private boolean verifyOnAllSubDirsHelper(Path p, FsAction fsAction, String group, + /* SENTRY-1471 - fixing the validation logic. + * a) When the number of retries exceeds the limit, propagate the Assert exception to the caller. + * b) Throw an exception instead of returning false, to pass valuable debugging info up the stack + * - expected vs. found permissions. + */ + private void verifyOnAllSubDirsHelper(Path p, FsAction fsAction, String group, boolean groupShouldExist, boolean recurse, int retry) throws Throwable { FileStatus fStatus = null; - boolean hasSucceeded = false; // validate parent dir's acls try { fStatus = miniDFS.getFileSystem().getFileStatus(p); @@ -223,32 +226,22 @@ public abstract class TestHDFSIntegrationBase { " group : " + group + " ;", getAcls(p).containsKey(group)); } LOGGER.info("Successfully found acls for path = " + p.getName()); - hasSucceeded = true; } catch (Throwable th) { if (retry > 0) { LOGGER.info("Retry: " + retry); Thread.sleep(RETRY_WAIT); - hasSucceeded = verifyOnAllSubDirsHelper(p, fsAction, group, groupShouldExist, recurse, retry - 1); + verifyOnAllSubDirsHelper(p, fsAction, group, groupShouldExist, recurse, retry - 1); } else { - LOGGER.info("Successfully found ACLs for path = " + p.getName()); - hasSucceeded = true; + throw th; } } - if (!hasSucceeded) { - LOGGER.error("Failed to validate ACLs for path = " + p.getName()); - return false; - } // validate children dirs if (recurse && fStatus.isDirectory()) { FileStatus[] children = miniDFS.getFileSystem().listStatus(p); for (FileStatus fs : children) { - if (!verifyOnAllSubDirsHelper(fs.getPath(), fsAction, group, groupShouldExist, recurse, NUM_RETRIES)) { - LOGGER.error("Failed to validate ACLs for child path = " + fs.getPath().getName()); - return false; - } + verifyOnAllSubDirsHelper(fs.getPath(), fsAction, group, groupShouldExist, recurse, NUM_RETRIES); } } - return true; } protected Map<String, FsAction> getAcls(Path path) throws Exception { @@ -296,25 +289,27 @@ public abstract class TestHDFSIntegrationBase { verifyQuery(stmt, table, n, NUM_RETRIES); } + /* SENTRY-1471 - fixing the validation logic. + * a) When the number of retries exceeds the limit, propagate the Assert exception to the caller. + * b) Throw an exception immediately, instead of using boolean variable, to pass valuable debugging + * info up the stack - expected vs. found number of rows. + */ protected void verifyQuery(Statement stmt, String table, int n, int retry) throws Throwable { ResultSet rs = null; - boolean isSucceeded = false; try { rs = stmt.executeQuery("select * from " + table); int numRows = 0; while (rs.next()) { numRows++; } Assert.assertEquals(n, numRows); - isSucceeded = true; } catch (Throwable th) { if (retry > 0) { LOGGER.info("Retry: " + retry); Thread.sleep(RETRY_WAIT); verifyQuery(stmt, table, n, retry - 1); } else { - isSucceeded = true; + throw th; } } - Assert.assertTrue(isSucceeded); } protected void verifyAccessToPath(String user, String group, String path, boolean hasPermission) throws Exception{
