SENTRY-2471: Table rename should sync Sentry privilege even without location information (Hao Hao, reviewed by Na Li)
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
index 377c673..bbfed62 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
@@ -378,34 +378,18 @@
String oldTableName = alterTableMessage.getTable();
String newDbName = event.getDbName();
String newTableName = event.getTableName();
- String oldLocation = alterTableMessage.getOldLocation();
- String newLocation = alterTableMessage.getNewLocation();
- if ((oldDbName == null)
- || (oldTableName == null)
- || (newDbName == null)
- || (newTableName == null)
- || (oldLocation == null)
- || (newLocation == null)) {
+ if ((oldDbName == null) ||
+ (oldTableName == null) ||
+ (newDbName == null) ||
+ (newTableName == null)) {
LOGGER.warn(String.format("Alter table notification ignored since event "
- + "has incomplete information. oldDbName = %s, oldTableName = %s, oldLocation = %s, "
- + "newDbName = %s, newTableName = %s, newLocation = %s",
+ + "has incomplete information. oldDbName = %s, oldTableName = %s, "
+ + "newDbName = %s, newTableName = %s",
StringUtils.defaultIfBlank(oldDbName, "null"),
StringUtils.defaultIfBlank(oldTableName, "null"),
- StringUtils.defaultIfBlank(oldLocation, "null"),
StringUtils.defaultIfBlank(newDbName, "null"),
- StringUtils.defaultIfBlank(newTableName, "null"),
- StringUtils.defaultIfBlank(newLocation, "null")));
- return false;
- }
-
- if ((oldDbName.equals(newDbName))
- && (oldTableName.equals(newTableName))
- && (oldLocation.equals(newLocation))) {
- LOGGER.debug(String.format("Alter table notification ignored as neither name nor "
- + "location has changed: oldAuthzObj = %s, oldLocation = %s, newAuthzObj = %s, "
- + "newLocation = %s", oldDbName + "." + oldTableName, oldLocation,
- newDbName + "." + newTableName, newLocation));
+ StringUtils.defaultIfBlank(newTableName, "null")));
return false;
}
@@ -425,6 +409,27 @@
if (!hdfsSyncEnabled) {
return false;
}
+
+ String oldLocation = alterTableMessage.getOldLocation();
+ String newLocation = alterTableMessage.getNewLocation();
+ if (oldLocation == null || newLocation == null) {
+ LOGGER.warn(String.format("HMS path update ignored since event has incomplete "
+ + "path information. oldLocation = %s, newLocation = %s",
+ StringUtils.defaultIfBlank(oldLocation, "null"),
+ StringUtils.defaultIfBlank(newLocation, "null")));
+ return false;
+ }
+
+ if ((oldDbName.equals(newDbName)) &&
+ (oldTableName.equals(newTableName)) &&
+ (oldLocation.equals(newLocation))) {
+ LOGGER.debug(String.format("Alter table notification ignored as neither name nor "
+ + "location has changed: oldAuthzObj = %s, oldLocation = %s, newAuthzObj = %s, "
+ + "newLocation = %s", oldDbName + "." + oldTableName, oldLocation,
+ newDbName + "." + newTableName, newLocation));
+ return false;
+ }
+
String oldAuthzObj = oldDbName + "." + oldTableName;
String newAuthzObj = newDbName + "." + newTableName;
renameAuthzPath(oldAuthzObj, newAuthzObj, oldLocation, newLocation, event);
diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestNotificationProcessor.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestNotificationProcessor.java
index 18e5ddf..dda1c46 100644
--- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestNotificationProcessor.java
+++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestNotificationProcessor.java
@@ -340,6 +340,51 @@
}
@Test
+ // Makes sure that appropriate sentry store methods are invoked when process events that rename
+ // tables without location information.
+ public void testRenameTableWithoutLocation() throws Exception {
+ String dbName = "db1";
+ String tableName = "table1";
+
+ String newDbName = "db1";
+ String newTableName = "table2";
+
+ Configuration authConf = new Configuration();
+ // enable HDFS sync, so perm and path changes will be saved into DB
+ authConf.set(ServiceConstants.ServerConfig.PROCESSOR_FACTORIES, "org.apache.sentry.hdfs.SentryHDFSServiceProcessorFactory");
+ authConf.set(ServiceConstants.ServerConfig.SENTRY_POLICY_STORE_PLUGINS, "org.apache.sentry.hdfs.SentryPlugin");
+
+ notificationProcessor = new NotificationProcessor(sentryStore,
+ hiveInstance, authConf);
+
+ // Create notification event
+ StorageDescriptor sd = new StorageDescriptor();
+ NotificationEvent notificationEvent = new NotificationEvent(1, 0,
+ EventMessage.EventType.ALTER_TABLE.toString(),
+ messageFactory.buildAlterTableMessage(
+ new Table(tableName, dbName, null, 0, 0, 0, sd, null, null, null, null, null),
+ new Table(newTableName, newDbName, null, 0, 0, 0, sd, null, null, null, null, null))
+ .toString());
+ notificationEvent.setDbName(newDbName);
+ notificationEvent.setTableName(newTableName);
+
+ notificationProcessor.processNotificationEvent(notificationEvent);
+
+ TSentryAuthorizable authorizable = new TSentryAuthorizable(hiveInstance);
+ authorizable.setServer(hiveInstance);
+ authorizable.setDb(dbName);
+ authorizable.setTable(tableName);
+
+ TSentryAuthorizable newAuthorizable = new TSentryAuthorizable(hiveInstance);
+ authorizable.setServer(hiveInstance);
+ newAuthorizable.setDb(newDbName);
+ newAuthorizable.setTable(newTableName);
+
+ verify(sentryStore, times(1)).renamePrivilege(authorizable, newAuthorizable,
+ NotificationProcessor.getPermUpdatableOnRename(authorizable, newAuthorizable));
+ }
+
+ @Test
/*
Makes sure that appropriate sentry store methods are invoked when alter tables event is
processed.
