Repository: incubator-sentry Updated Branches: refs/heads/master 8529f8e12 -> d96f95160
SENTRY-1002: PathsUpdate.parsePath(path) will throw an NPE when parsing relative paths (Hao Hao via Lenni Kuff) Change-Id: I8882078abeed37c17734b04d09f6fb2b298861b9 Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/d96f9516 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/d96f9516 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/d96f9516 Branch: refs/heads/master Commit: d96f95160fd3dfa30c27b82d09fb5cc2c348b483 Parents: 8529f8e Author: Lenni Kuff <[email protected]> Authored: Thu Jan 28 11:15:28 2016 -0800 Committer: Lenni Kuff <[email protected]> Committed: Thu Jan 28 11:15:28 2016 -0800 ---------------------------------------------------------------------- .../org/apache/sentry/hdfs/PathsUpdate.java | 23 +++++- .../tests/e2e/hdfs/TestHDFSIntegration.java | 78 ++++++++++++++++++-- 2 files changed, 92 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/d96f9516/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java index 1dcb75a..50ef112 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java +++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java @@ -23,13 +23,15 @@ import java.net.URISyntaxException; import java.util.LinkedList; import java.util.List; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; - import org.apache.sentry.hdfs.service.thrift.TPathChanges; import org.apache.sentry.hdfs.service.thrift.TPathsUpdate; import org.apache.commons.httpclient.util.URIUtil; import org.apache.commons.httpclient.URIException; import org.apache.commons.lang.StringUtils; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.conf.Configuration; import com.google.common.collect.Lists; @@ -42,7 +44,7 @@ import com.google.common.collect.Lists; public class PathsUpdate implements Updateable.Update { public static String ALL_PATHS = "__ALL_PATHS__"; - + private static final Configuration CONF = new Configuration(); private final TPathsUpdate tPathsUpdate; public PathsUpdate() { @@ -89,6 +91,10 @@ public class PathsUpdate implements Updateable.Update { return tPathsUpdate; } + @VisibleForTesting + public static Configuration getConfiguration() { + return CONF; + } /** * @@ -106,9 +112,18 @@ public class PathsUpdate implements Updateable.Update { return null; } - Preconditions.checkNotNull(uri.getScheme()); + String scheme = uri.getScheme(); + if (scheme == null) { + // Use the default URI scheme only if the paths has no scheme. + URI defaultUri = FileSystem.getDefaultUri(CONF); + scheme = defaultUri.getScheme(); + } + + // The paths without a scheme will be default to default scheme. + Preconditions.checkNotNull(scheme); - if(uri.getScheme().equalsIgnoreCase("hdfs")) { + // Non-HDFS paths will be skipped. + if(scheme.equalsIgnoreCase("hdfs")) { return Lists.newArrayList(uri.getPath().split("^/")[1] .split("/")); } else { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/d96f9516/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java index fc7f324..4d9e31c 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java @@ -51,13 +51,12 @@ import org.apache.hadoop.fs.permission.AclEntryType; import org.apache.hadoop.fs.permission.AclStatus; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; -import org.apache.hadoop.hdfs.DFSConfigKeys; -import org.apache.hadoop.hdfs.DFSTestUtil; -import org.apache.hadoop.hdfs.HdfsConfiguration; -import org.apache.hadoop.hdfs.MiniDFSCluster; +import org.apache.hadoop.hdfs.*; import org.apache.hadoop.hdfs.server.namenode.EditLogFileOutputStream; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; +import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.StorageDescriptor; import org.apache.hadoop.io.LongWritable; import org.apache.hadoop.io.Text; import org.apache.hadoop.mapred.FileInputFormat; @@ -76,6 +75,7 @@ import org.apache.hadoop.mapred.TextOutputFormat; import org.apache.hadoop.security.UserGroupInformation; import org.apache.sentry.binding.hive.SentryHiveAuthorizationTaskFactoryImpl; import org.apache.sentry.binding.hive.conf.HiveAuthzConf; +import org.apache.sentry.hdfs.PathsUpdate; import org.apache.sentry.hdfs.SentryAuthorizationProvider; import org.apache.sentry.provider.db.SentryAlreadyExistsException; import org.apache.sentry.provider.db.SimpleDBProviderBackend; @@ -101,6 +101,7 @@ import org.slf4j.LoggerFactory; import com.google.common.collect.Maps; import com.google.common.io.Files; import com.google.common.io.Resources; +import org.apache.hadoop.hive.metastore.api.Table; public class TestHDFSIntegration { @@ -140,6 +141,7 @@ public class TestHDFSIntegration { private static final int NUM_RETRIES = 10; private static final int RETRY_WAIT = 1000; + private static final String EXTERNAL_SENTRY_SERVICE = "sentry.e2etest.external.sentry"; private static final String DFS_NAMENODE_AUTHORIZATION_PROVIDER_KEY = "dfs.namenode.authorization.provider.class"; @@ -147,6 +149,7 @@ public class TestHDFSIntegration { private MiniMRClientCluster miniMR; private static InternalHiveServer hiveServer2; private static InternalMetastoreServer metastore; + private static HiveMetaStoreClient hmsClient; private static int sentryPort = -1; protected static SentrySrv sentryServer; @@ -304,6 +307,7 @@ public class TestHDFSIntegration { } }.start(); + hmsClient = new HiveMetaStoreClient(hiveConf); startHiveServer2(retries, hiveConf); return null; } @@ -1266,7 +1270,7 @@ public class TestHDFSIntegration { conn = hiveServer2.createConnection(StaticUserGroup.ADMIN1, StaticUserGroup.ADMIN1); stmt = conn.createStatement(); stmt.execute("create database " + dbName); - stmt.execute("use "+ dbName); + stmt.execute("use " + dbName); stmt.execute("create table p1 (c1 string, c2 string) partitioned by (month int, day int)"); stmt.execute("alter table p1 add partition (month=1, day=1)"); loadDataTwoCols(stmt); @@ -1591,6 +1595,70 @@ public class TestHDFSIntegration { } } + /** + * SENTRY-1002: + * Ensure the paths with no scheme will not cause NPE during paths update. + */ + @Test + public void testMissingScheme() throws Throwable { + + // In the local test environment, EXTERNAL_SENTRY_SERVICE is false, + // set the default URI scheme to be hdfs. + boolean testConfOff = new Boolean(System.getProperty(EXTERNAL_SENTRY_SERVICE, "false")); + if (!testConfOff) { + PathsUpdate.getConfiguration().set("fs.defaultFS", "hdfs:///"); + } + + tmpHDFSDir = new Path("/tmp/external"); + if (!miniDFS.getFileSystem().exists(tmpHDFSDir)) { + miniDFS.getFileSystem().mkdirs(tmpHDFSDir); + } + + Path partitionDir = new Path("/tmp/external/p1"); + if (!miniDFS.getFileSystem().exists(partitionDir)) { + miniDFS.getFileSystem().mkdirs(partitionDir); + } + + String dbName = "db1"; + String tblName = "tab1"; + dbNames = new String[]{dbName}; + roles = new String[]{"admin_role"}; + admin = StaticUserGroup.ADMIN1; + + Connection conn; + Statement stmt; + + conn = hiveServer2.createConnection("hive", "hive"); + stmt = conn.createStatement(); + stmt.execute("create role admin_role"); + stmt.execute("grant all on server server1 to role admin_role"); + stmt.execute("grant role admin_role to group " + StaticUserGroup.ADMINGROUP); + stmt.close(); + conn.close(); + + conn = hiveServer2.createConnection(StaticUserGroup.ADMIN1, StaticUserGroup.ADMIN1); + stmt = conn.createStatement(); + stmt.execute("create database " + dbName); + stmt.execute("create external table " + dbName + "." + tblName + "(s string) location '/tmp/external/p1'"); + + // Deep copy of table tab1 + Table tbCopy = hmsClient.getTable(dbName, tblName); + + // Change the location of the table to strip the scheme. + StorageDescriptor sd = hmsClient.getTable(dbName, tblName).getSd(); + sd.setLocation("/tmp/external"); + tbCopy.setSd(sd); + + // Alter table tab1 to be tbCopy which is at scheme-less location. + // And the corresponding path will be updated to sentry server. + hmsClient.alter_table(dbName, "tab1", tbCopy); + Assert.assertEquals(hmsClient.getTable(dbName, tblName).getSd().getLocation(), "/tmp/external"); + verifyOnPath("/tmp/external", FsAction.ALL, StaticUserGroup.HIVE, true); + + stmt.close(); + conn.close(); + } + private void loadData(Statement stmt) throws IOException, SQLException { FSDataOutputStream f1 = miniDFS.getFileSystem().create(new Path("/tmp/f1.txt")); f1.writeChars("m1d1_t1\n");
