You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by ls...@apache.org on 2016/01/28 20:17:10 UTC
incubator-sentry git commit: SENTRY-1002: PathsUpdate.parsePath(path)
will throw an NPE when parsing relative paths (Hao Hao via Lenni Kuff)
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 <ls...@cloudera.com>
Authored: Thu Jan 28 11:15:28 2016 -0800
Committer: Lenni Kuff <ls...@cloudera.com>
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");