You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by sr...@apache.org on 2016/10/27 23:36:18 UTC
[1/3] sentry git commit: SENTRY-1313: Database prefix is not honoured
when executing grant statement (Sravya Tirukkovalur,
Reviewed by: Hao Hao and Alexander Kolbasov)
Repository: sentry
Updated Branches:
refs/heads/sentry-ha-redesign 13c3305de -> 1024efb4d
SENTRY-1313: Database prefix is not honoured when executing grant statement (Sravya Tirukkovalur, Reviewed by: Hao Hao and Alexander Kolbasov)
Change-Id: I7c39724cd8b8f89d96499fdaed70fffad75728aa
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/caf924b5
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/caf924b5
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/caf924b5
Branch: refs/heads/sentry-ha-redesign
Commit: caf924b5fe7862932443d445bdf46de1bedd2a61
Parents: 13c3305
Author: Sravya Tirukkovalur <sr...@apache.org>
Authored: Thu Oct 27 12:21:37 2016 -0700
Committer: Sravya Tirukkovalur <sr...@apache.org>
Committed: Thu Oct 27 16:35:49 2016 -0700
----------------------------------------------------------------------
.../hive/SentryHiveAuthorizationTaskFactoryImpl.java | 4 ++--
.../hive/TestSentryHiveAuthorizationTaskFactory.java | 11 +++++++++++
.../tests/e2e/dbprovider/TestDatabaseProvider.java | 2 +-
3 files changed, 14 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/sentry/blob/caf924b5/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
index 25531af..ca6bb17 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
@@ -297,8 +297,8 @@ public class SentryHiveAuthorizationTaskFactoryImpl implements HiveAuthorization
subject.setServer(true);
} else if (astChild.getToken().getType() == HiveParser.TOK_TABLE_TYPE) {
subject.setTable(true);
- String[] qualified = BaseSemanticAnalyzer.getQualifiedTableName(gchild);
- subject.setObject(qualified[1]);
+ String qualified = BaseSemanticAnalyzer.getUnescapedName(gchild);
+ subject.setObject(qualified);
}
for (int i = 1; i < astChild.getChildCount(); i++) {
gchild = (ASTNode) astChild.getChild(i);
http://git-wip-us.apache.org/repos/asf/sentry/blob/caf924b5/sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java b/sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
index 4fcb71c..aed218e 100644
--- a/sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
+++ b/sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
@@ -465,6 +465,17 @@ public class TestSentryHiveAuthorizationTaskFactory {
Assert.assertEquals(SERVER, privilegeDesc.getObject());
}
+ /*
+ Db prefix in grant
+ */
+ @Test
+ public void testDBPrefixInGrant() throws Exception {
+ DDLWork work = analyze(parse("GRANT " + ALL + " ON TABLE " + "db1." + TABLE
+ + " TO ROLE " + ROLE));
+ GrantDesc grantDesc = work.getGrantDesc();
+ Assert.assertEquals("Fully qualified table name in Grant statement is resolved incorrectly", "db1." + TABLE,
+ grantDesc.getPrivilegeSubjectDesc().getObject());
+ }
private void expectSemanticException(String command, String msg) throws Exception {
try {
analyze(parse(command));
http://git-wip-us.apache.org/repos/asf/sentry/blob/caf924b5/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
index ff4eeaf..255e266 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
@@ -2200,7 +2200,7 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration {
resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
while ( resultSet.next()) {
- assertThat(resultSet.getString(1), equalToIgnoringCase("default"));//the value should be db1
+ assertThat(resultSet.getString(1), equalToIgnoringCase("db1"));//the value should be db1
assertThat(resultSet.getString(2), equalToIgnoringCase("tab1"));
assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
[3/3] sentry git commit: SENTRY-1260: Improve error handling -
ArrayIndexOutOfBoundsException in PathsUpdate.parsePath can cause
MetastoreCacheInitializer intialization to fail(Sravya Tirukkovalur,
Reviewed by: Hao Hao)
Posted by sr...@apache.org.
SENTRY-1260: Improve error handling - ArrayIndexOutOfBoundsException in PathsUpdate.parsePath can cause MetastoreCacheInitializer intialization to fail(Sravya Tirukkovalur, Reviewed by: Hao Hao)
Change-Id: I76ce7333518e8b1c2fe15677124281d3fa6972c5
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/1024efb4
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/1024efb4
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/1024efb4
Branch: refs/heads/sentry-ha-redesign
Commit: 1024efb4de784e8a6bc952633dd0a52ae1599176
Parents: a8c405a
Author: Sravya Tirukkovalur <sr...@apache.org>
Authored: Thu Oct 27 15:00:23 2016 -0700
Committer: Sravya Tirukkovalur <sr...@apache.org>
Committed: Thu Oct 27 16:36:01 2016 -0700
----------------------------------------------------------------------
.../org/apache/sentry/hdfs/PathsUpdate.java | 37 ++++--
.../hdfs/SentryMalformedPathException.java | 38 ++++++
.../org/apache/sentry/hdfs/TestPathsUpdate.java | 31 ++++-
.../sentry/hdfs/TestUpdateableAuthzPaths.java | 4 +-
.../sentry/hdfs/MetastoreCacheInitializer.java | 36 ++++--
.../org/apache/sentry/hdfs/MetastorePlugin.java | 40 +++++-
.../hdfs/TestMetastoreCacheInitializer.java | 122 ++++++++++++++++---
7 files changed, 260 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/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 5985756..7cfb3bf 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
@@ -24,7 +24,6 @@ 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;
@@ -101,43 +100,55 @@ public class PathsUpdate implements Updateable.Update {
/**
*
- * @param path : Needs to be a HDFS location with scheme
+ * @param path : Needs to be a HDFS location in the forms:
+ * - hdfs://hostname:port/path
+ * - hdfs:///path
+ * - /path, in which case, scheme will be constructed from FileSystem.getDefaultURI
+ * - URIs with non hdfs schemee will just be ignored
* @return Path in the form a list containing the path tree with scheme/ authority stripped off.
* Returns null if a non HDFS path or if path is null/empty
*/
- public static List<String> parsePath(String path) {
+ public static List<String> parsePath(String path) throws SentryMalformedPathException {
try {
LOGGER.debug("Parsing path " + path);
URI uri = null;
if (StringUtils.isNotEmpty(path)) {
uri = new URI(URIUtil.encodePath(path));
} else {
- return null;
+ String msg = "Input is empty";
+ throw new SentryMalformedPathException(msg);
}
String scheme = uri.getScheme();
if (scheme == null) {
- // Use the default URI scheme only if the paths has no scheme.
+ // Use the default URI scheme only if the path has no scheme.
URI defaultUri = FileSystem.getDefaultUri(CONF);
scheme = defaultUri.getScheme();
+ if(scheme == null) {
+ String msg = "Scheme is missing and could not be constructed from defaultURI=" + defaultUri;
+ throw new SentryMalformedPathException(msg);
+ }
}
- // The paths without a scheme will be default to default scheme.
- Preconditions.checkNotNull(scheme);
-
// Non-HDFS paths will be skipped.
if(scheme.equalsIgnoreCase("hdfs")) {
-
- return Lists.newArrayList(uri.getPath().split("^/")[1]
- .split("/"));
+ String uriPath = uri.getPath();
+ if(uriPath == null) {
+ throw new SentryMalformedPathException("Path is empty. uri=" + uri);
+ }
+ if(uriPath.split("^/").length < 2) {
+ throw new SentryMalformedPathException("Path part of uri does not seem right, was expecting a non empty path" +
+ ": path = " + uriPath + ", uri=" + uri);
+ }
+ return Lists.newArrayList(uriPath.split("^/")[1].split("/"));
} else {
LOGGER.warn("Invalid FS: " + scheme + "://; expected hdfs://");
return null;
}
} catch (URISyntaxException e) {
- throw new RuntimeException("Incomprehensible path [" + path + "]");
+ throw new SentryMalformedPathException("Incomprehensible path [" + path + "]", e);
} catch (URIException e){
- throw new RuntimeException("Unable to create URI: ",e);
+ throw new SentryMalformedPathException("Unable to create URI: ", e);
}
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryMalformedPathException.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryMalformedPathException.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryMalformedPathException.java
new file mode 100644
index 0000000..9e163ba
--- /dev/null
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryMalformedPathException.java
@@ -0,0 +1,38 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sentry.hdfs;
+
+public class SentryMalformedPathException extends Exception{
+ private static final long serialVersionUID = 433430558380655835L;
+ private String reason;
+ public SentryMalformedPathException(String msg) {
+ super(msg);
+ }
+ public SentryMalformedPathException(String msg, String reason) {
+ super(msg);
+ this.reason = reason;
+ }
+ public SentryMalformedPathException(String msg, Throwable t) {
+ super(msg, t);
+ reason = t.getMessage();
+ }
+ public String getReason() {
+ return reason;
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
index 71618ab..53243b4 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java
@@ -18,16 +18,45 @@
package org.apache.sentry.hdfs;
import java.util.List;
+
import org.junit.Test;
import org.junit.Assert;
public class TestPathsUpdate {
@Test
- public void testParsePath(){
+ public void testParsePathComplexCharacters() throws SentryMalformedPathException{
List<String> results = PathsUpdate.parsePath(
"hdfs://hostname.test.com:8020/user/hive/warehouse/break/b=all | ' & the spaces/c=in PartKeys/With fun chars *%!|"
);
System.out.println(results);
Assert.assertNotNull("Parse path without throwing exception",results);
}
+
+ @Test
+ public void testPositiveParsePath() throws SentryMalformedPathException {
+ List<String> results = PathsUpdate.parsePath("hdfs://hostname.test.com:8020/path");
+ Assert.assertTrue("Parsed path is unexpected", results.get(0).equals("path"));
+ Assert.assertTrue("Parsed path size is unexpected", results.size() == 1);
+
+ results = PathsUpdate.parsePath("hdfs://hostname.test.com/path");
+ Assert.assertTrue("Parsed path is unexpected", results.get(0).equals("path"));
+ Assert.assertTrue("Parsed path size is unexpected", results.size() == 1);
+
+ results = PathsUpdate.parsePath("hdfs:///path");
+ Assert.assertTrue("Parsed path is unexpected", results.get(0).equals("path"));
+ Assert.assertTrue("Parsed path size is unexpected", results.size() == 1);
+ }
+
+ @Test(expected = SentryMalformedPathException.class)
+ public void testMalformedPathFunny() throws SentryMalformedPathException{
+ PathsUpdate.parsePath("hdfs://hostname");
+ }
+
+ //if file:// - should return null
+ @Test
+ public void testMalformedPathFile() throws SentryMalformedPathException{
+ List<String> results = PathsUpdate.parsePath("file://hostname/path");
+ System.out.println(results);
+ Assert.assertNull("Parse path without throwing exception",results);
+ }
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java
index 909ff3a..e643e01 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java
@@ -68,7 +68,7 @@ public class TestUpdateableAuthzPaths {
}
@Test
- public void testPartialUpdateAddPath() {
+ public void testPartialUpdateAddPath() throws SentryMalformedPathException{
HMSPaths hmsPaths = createBaseHMSPaths(1, 1);
UpdateableAuthzPaths authzPaths = new UpdateableAuthzPaths(hmsPaths);
ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
@@ -115,7 +115,7 @@ public class TestUpdateableAuthzPaths {
}
@Test
- public void testPartialUpdateDelPath() {
+ public void testPartialUpdateDelPath() throws SentryMalformedPathException{
HMSPaths hmsPaths = createBaseHMSPaths(1, 1);
UpdateableAuthzPaths authzPaths = new UpdateableAuthzPaths(hmsPaths);
ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
index 807e4e0..517bba5 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
@@ -25,6 +25,7 @@ import org.apache.hadoop.hive.metastore.api.Database;
import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.Table;
import org.apache.sentry.hdfs.service.thrift.TPathChanges;
+import org.apache.thrift.TException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -154,7 +155,7 @@ class MetastoreCacheInitializer implements Closeable {
}
@Override
- public void doTask() throws Exception {
+ public void doTask() throws TException, SentryMalformedPathException {
List<Partition> tblParts =
hmsHandler.get_partitions_by_names(dbName, tblName, partNames);
if (LOGGER.isDebugEnabled()) {
@@ -162,8 +163,15 @@ class MetastoreCacheInitializer implements Closeable {
"[" + dbName + "." + tblName + "]" + "[" + partNames + "]");
}
for (Partition part : tblParts) {
- List<String> partPath = PathsUpdate.parsePath(part.getSd()
- .getLocation());
+ List<String> partPath = null;
+ try {
+ partPath = PathsUpdate.parsePath(part.getSd().getLocation());
+ } catch (SentryMalformedPathException e) {
+ String msg = "Unexpected path in partitionTask: dbName=" + dbName +
+ ", tblName=" + tblName + ", path=" + part.getSd().getLocation();
+ throw new SentryMalformedPathException(msg, e);
+ }
+
if (partPath != null) {
synchronized (tblPathChange) {
tblPathChange.addToAddPaths(partPath);
@@ -202,8 +210,14 @@ class MetastoreCacheInitializer implements Closeable {
tblPathChange = update.newPathChange(db.getName() + "." + tableName);
}
if (tbl.getSd().getLocation() != null) {
- List<String> tblPath =
- PathsUpdate.parsePath(tbl.getSd().getLocation());
+ List<String> tblPath = null;
+ try {
+ tblPath = PathsUpdate.parsePath(tbl.getSd().getLocation());
+ } catch (SentryMalformedPathException e) {
+ String msg = "Unexpected path in TableTask: dbName=" + tbl.getDbName() +
+ ", tblName=" + tbl.getTableName() + ", path=" + tbl.getSd().getLocation();
+ throw new SentryMalformedPathException(msg, e);
+ }
if (tblPath != null) {
tblPathChange.addToAddPaths(tblPath);
}
@@ -238,9 +252,15 @@ class MetastoreCacheInitializer implements Closeable {
}
@Override
- public void doTask() throws Exception {
+ public void doTask() throws TException, SentryMalformedPathException {
Database db = hmsHandler.get_database(dbName);
- List<String> dbPath = PathsUpdate.parsePath(db.getLocationUri());
+ List<String> dbPath = null;
+ try {
+ dbPath = PathsUpdate.parsePath(db.getLocationUri());
+ } catch (SentryMalformedPathException e) {
+ String msg = "Unexpected path in DbTask: DB=" + db.getName() + ", Path=" + db.getLocationUri();
+ throw new SentryMalformedPathException(msg, e);
+ }
if (dbPath != null) {
synchronized (update) {
Preconditions.checkArgument(dbName.equalsIgnoreCase(db.getName()));
@@ -328,7 +348,7 @@ class MetastoreCacheInitializer implements Closeable {
// Fail the HMS startup if tasks are not all successful and
// fail on partial updates flag is set in the config.
if (!callResult.getSuccessStatus() && failOnRetry) {
- throw new RuntimeException(callResult.getFailure());
+ throw callResult.getFailure();
}
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
index 2df9f45..085971b 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
@@ -197,7 +197,14 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
@Override
public void addPath(String authzObj, String path) {
- List<String> pathTree = PathsUpdate.parsePath(path);
+ List<String> pathTree = null;
+ try {
+ pathTree = PathsUpdate.parsePath(path);
+ } catch (SentryMalformedPathException e) {
+ LOGGER.error("Unexpected path in addPath: authzObj = " + authzObj + " , path = " + path);
+ e.printStackTrace();
+ return;
+ }
if(pathTree == null) {
return;
}
@@ -233,7 +240,14 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
if ("*".equals(path)) {
removeAllPaths(authzObj.toLowerCase(), null);
} else {
- List<String> pathTree = PathsUpdate.parsePath(path);
+ List<String> pathTree = null;
+ try {
+ pathTree = PathsUpdate.parsePath(path);
+ } catch (SentryMalformedPathException e) {
+ LOGGER.error("Unexpected path in removePath: authzObj = " + authzObj + " , path = " + path);
+ e.printStackTrace();
+ return;
+ }
if(pathTree == null) {
return;
}
@@ -259,11 +273,29 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
+ "oldPath : " + oldPath + ","
+ "newName : " + newNameLC + ","
+ "newPath : " + newPath + "]");
- List<String> newPathTree = PathsUpdate.parsePath(newPath);
+ List<String> newPathTree = null;
+ try {
+ newPathTree = PathsUpdate.parsePath(newPath);
+ } catch (SentryMalformedPathException e) {
+ LOGGER.error("Unexpected path in renameAuthzObject while parsing newPath: oldName=" + oldName + ", oldPath=" + oldPath +
+ ", newName=" + newName + ", newPath=" + newPath);
+ e.printStackTrace();
+ return;
+ }
+
if( newPathTree != null ) {
update.newPathChange(newNameLC).addToAddPaths(newPathTree);
}
- List<String> oldPathTree = PathsUpdate.parsePath(oldPath);
+ List<String> oldPathTree = null;
+ try {
+ oldPathTree = PathsUpdate.parsePath(oldPath);
+ } catch (SentryMalformedPathException e) {
+ LOGGER.error("Unexpected path in renameAuthzObject while parsing oldPath: oldName=" + oldName + ", oldPath=" + oldPath +
+ ", newName=" + newName + ", newPath=" + newPath);
+ e.printStackTrace();
+ return;
+ }
+
if( oldPathTree != null ) {
update.newPathChange(oldNameLC).addToDelPaths(oldPathTree);
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/1024efb4/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java b/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java
index 9e6072d..fbaaa2c 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java
@@ -31,9 +31,22 @@ import org.mockito.Mockito;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
+import java.util.List;
public class TestMetastoreCacheInitializer {
+ private Configuration setConf() {
+ Configuration conf = new Configuration();
+ conf.setInt(ServiceConstants.ServerConfig
+ .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_PART_PER_RPC, 1);
+ conf.setInt(ServiceConstants.ServerConfig
+ .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_TABLES_PER_RPC, 1);
+ conf.setInt(ServiceConstants.ServerConfig
+ .SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS, 1);
+ return conf;
+
+ }
+
@Test
public void testInitializer() throws Exception {
@@ -103,16 +116,8 @@ public class TestMetastoreCacheInitializer {
Lists.newArrayList("part312")))
.thenReturn(Lists.newArrayList(part312));
- Configuration conf = new Configuration();
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_PART_PER_RPC, 1);
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_TABLES_PER_RPC, 1);
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS, 1);
-
MetastoreCacheInitializer cacheInitializer = new
- MetastoreCacheInitializer(hmsHandler, conf);
+ MetastoreCacheInitializer(hmsHandler, setConf());
UpdateableAuthzPaths update = cacheInitializer.createInitialUpdate();
Assert.assertEquals(new HashSet<String>(Arrays.asList("db1")), update.findAuthzObjectExactMatches(new
@@ -156,19 +161,9 @@ public class TestMetastoreCacheInitializer {
Mockito.when(hmsHandler.get_all_tables("db1")).thenReturn(Lists
.newArrayList("tab1"));
- Configuration conf = new Configuration();
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_PART_PER_RPC, 1);
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_MAX_TABLES_PER_RPC, 1);
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_INIT_THREADS, 1);
- conf.setInt(ServiceConstants.ServerConfig
- .SENTRY_HDFS_SYNC_METASTORE_CACHE_RETRY_MAX_NUM, 2);
-
try {
MetastoreCacheInitializer cacheInitializer = new
- MetastoreCacheInitializer(hmsHandler, conf);
+ MetastoreCacheInitializer(hmsHandler, setConf());
cacheInitializer.createInitialUpdate();
Assert.fail("Expected cacheInitializer to fail");
} catch (Exception e) {
@@ -176,4 +171,91 @@ public class TestMetastoreCacheInitializer {
}
}
+
+ @Test(expected = SentryMalformedPathException.class)
+ public void testSentryMalFormedExceptionInDbTask() throws Exception {
+ //Set up mocks: db1 with malformed paths
+ Database db1 = Mockito.mock(Database.class);
+ Mockito.when(db1.getName()).thenReturn("db1");
+ Mockito.when(db1.getLocationUri()).thenReturn("hdfs://db1");
+
+ IHMSHandler hmsHandler = Mockito.mock(IHMSHandler.class);
+ Mockito.when(hmsHandler.get_all_databases()).thenReturn(Lists
+ .newArrayList("db1"));
+ Mockito.when(hmsHandler.get_database("db1")).thenReturn(db1);
+
+
+ MetastoreCacheInitializer cacheInitializer = new MetastoreCacheInitializer(hmsHandler, setConf());
+ cacheInitializer.createInitialUpdate();
+ Assert.fail("Expected cacheInitializer to fail");
+ }
+
+ @Test(expected = SentryMalformedPathException.class)
+ public void testSentryMalFormedExceptionInTableTask() throws Exception {
+ //Set up mocks: db1 and tb1 with wrong location
+ Database db1 = Mockito.mock(Database.class);
+ Mockito.when(db1.getName()).thenReturn("db1");
+ IHMSHandler hmsHandler = Mockito.mock(IHMSHandler.class);
+ Mockito.when(hmsHandler.get_all_databases()).thenReturn(Lists
+ .newArrayList("db1"));
+ Mockito.when(hmsHandler.get_database("db1")).thenReturn(db1);
+
+ Table tab1 = Mockito.mock(Table.class);
+ Mockito.when(tab1.getDbName()).thenReturn("db1");
+ Mockito.when(tab1.getTableName()).thenReturn("tab1");
+ StorageDescriptor sd = Mockito.mock(StorageDescriptor.class);
+ Mockito.when(tab1.getSd()).thenReturn(sd);
+ Mockito.when(tab1.getSd().getLocation()).thenReturn("hdfs://db1");
+
+ Mockito.when(hmsHandler.get_table_objects_by_name("db1",
+ Lists.newArrayList("tab1")))
+ .thenReturn(Lists.newArrayList(tab1));
+ Mockito.when(hmsHandler.get_all_tables("db1")).thenReturn(Lists
+ .newArrayList("tab1"));
+
+ MetastoreCacheInitializer cacheInitializer = new MetastoreCacheInitializer(hmsHandler, setConf());
+ cacheInitializer.createInitialUpdate();
+ Assert.fail("Expected cacheInitializer to fail");
+
+ }
+
+ @Test(expected = SentryMalformedPathException.class)
+ public void testSentryMalFormedExceptionInPartitionTask() throws Exception {
+ //Set up mocks: db1,tb1 and partition with wrong location
+ Database db1 = Mockito.mock(Database.class);
+ Mockito.when(db1.getName()).thenReturn("db1");
+ IHMSHandler hmsHandler = Mockito.mock(IHMSHandler.class);
+ Mockito.when(hmsHandler.get_all_databases()).thenReturn(Lists
+ .newArrayList("db1"));
+ Mockito.when(hmsHandler.get_database("db1")).thenReturn(db1);
+
+ Table tab1 = Mockito.mock(Table.class);
+ StorageDescriptor tableSd = Mockito.mock(StorageDescriptor.class);
+ Mockito.when(tab1.getDbName()).thenReturn("db1");
+ Mockito.when(tab1.getTableName()).thenReturn("tab1");
+ Mockito.when(tab1.getSd()).thenReturn(tableSd);
+ Mockito.when(tab1.getSd().getLocation()).thenReturn("hdfs://hostname/db1/tab1");
+
+ StorageDescriptor sd = Mockito.mock(StorageDescriptor.class);
+ Partition partition = Mockito.mock(Partition.class);
+ Mockito.when(partition.getSd()).thenReturn(sd);
+ Mockito.when(partition.getSd().getLocation()).thenReturn("hdfs://db1");
+
+ Mockito.when(hmsHandler.get_table_objects_by_name("db1",
+ Lists.newArrayList("tab1")))
+ .thenReturn(Lists.newArrayList(tab1));
+ Mockito.when(hmsHandler.get_all_tables("db1")).thenReturn(Lists
+ .newArrayList("tab1"));
+ List<String> partnames = new ArrayList<>();
+ partnames.add("part1");
+ List<Partition> partitions = new ArrayList<>();
+ partitions.add(partition);
+ Mockito.when(hmsHandler.get_partition_names("db1", "tab1", (short) -1)).thenReturn(partnames);
+ Mockito.when(hmsHandler.get_partitions_by_names("db1", "tab1", partnames)).thenReturn(partitions);
+
+ MetastoreCacheInitializer cacheInitializer = new MetastoreCacheInitializer(hmsHandler, setConf());
+ cacheInitializer.createInitialUpdate();
+ Assert.fail("Expected cacheInitializer to fail");
+
+ }
}
[2/3] sentry git commit: SENTRY-1270: Improve error handling -
Database with malformed URI causes NPE in HMS plugin during DDL (Sravya
Tirukkovalur, Reviewed by: Lenni Kuff)
Posted by sr...@apache.org.
SENTRY-1270: Improve error handling - Database with malformed URI causes NPE in HMS plugin during DDL (Sravya Tirukkovalur, Reviewed by: Lenni Kuff)
Change-Id: I11da6179300ff50e892b7d7c0d290d4213be55a9
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/a8c405a9
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/a8c405a9
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/a8c405a9
Branch: refs/heads/sentry-ha-redesign
Commit: a8c405a9ab6910fd6341588db76ccb78c5bcb535
Parents: caf924b
Author: Sravya Tirukkovalur <sr...@apache.org>
Authored: Thu Oct 27 13:25:11 2016 -0700
Committer: Sravya Tirukkovalur <sr...@apache.org>
Committed: Thu Oct 27 16:35:56 2016 -0700
----------------------------------------------------------------------
.../java/org/apache/sentry/hdfs/MetastorePlugin.java | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/sentry/blob/a8c405a9/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
index f37596d..2df9f45 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
@@ -53,6 +53,10 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
private static final Logger LOGGER = LoggerFactory.getLogger(MetastorePlugin.class);
+ private static final String initializationFailureMsg = "Cache failed to initialize, cannot send path updates to Sentry." +
+ " Please review HMS error logs during startup for additional information. If the initialization failure is due" +
+ " to SentryMalformedPathException, you will need to rectify the malformed path in HMS db and restart HMS";
+
class SyncTask implements Runnable {
@Override
public void run() {
@@ -61,8 +65,7 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
return;
}
if (MetastorePlugin.this.authzPaths == null) {
- LOGGER.info("#### Metastore Plugin cache has not finished" +
- "initialization.");
+ LOGGER.warn(initializationFailureMsg);
return;
}
try {
@@ -316,6 +319,10 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
protected void applyLocal(PathsUpdate update) {
final Timer.Context timerContext =
SentryHdfsMetricsUtil.getApplyLocalUpdateTimer.time();
+ if(authzPaths == null) {
+ LOGGER.error(initializationFailureMsg);
+ return;
+ }
authzPaths.updatePartial(Lists.newArrayList(update), new ReentrantReadWriteLock());
timerContext.stop();
SentryHdfsMetricsUtil.getApplyLocalUpdateHistogram.update(
@@ -323,6 +330,10 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin {
}
private void notifySentryAndApplyLocal(PathsUpdate update) {
+ if(authzPaths == null) {
+ LOGGER.error(initializationFailureMsg);
+ return;
+ }
if (initComplete) {
processUpdate(update);
} else {