> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote: > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > > Lines 220-221 (patched) > > <https://reviews.apache.org/r/69585/diff/3/?file=2115112#file2115112line220> > > > > These two variables are not necessary. To disable the filter, the admin > > can just remove the filter hooks from the configuration and voilà, filter > > disabled. This applies to both sides, server-side and client-side.
removing the filter hooks is more like a workaround. It is more straightforward to keep the hook configuration, and have specific configuration to enable/disable filtering at HMS client and server. > On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote: > > standalone-metastore/metastore-server/pom.xml > > Lines 242-246 (patched) > > <https://reviews.apache.org/r/69585/diff/3/?file=2115113#file2115113line242> > > > > Why is this dep necessary? I wonder if this conflicts with the > > standalone solution where all the Hive dependences were removed. It is not needed. I have removed it. > On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 1405 (patched) > > <https://reviews.apache.org/r/69585/diff/3/?file=2115114#file2115114line1405> > > > > I thought we were going to use the PreReadEvent instead. Also, I'm > > concerned the implementation of filterDatabase does not throw > > NoSuchObjectException, and instead returns null. We should check both cases > > to guarantee get_database and others methods will deny the access to this > > database. > > > > I think we should use the PreReadEvent which is meant for authorization > > hooks. The PreReadEvent calls sentry MetastoreAuthzBindingBase for authorization. As you can see in https://github.com/apache/sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java#L169, it does not do anything on reading metadata. Therefore, we have to use filter to prevent user from reading metadata it does not have privileges. I can only remove the filter for get_database() or get_table() if we change sentry's implementation for that file. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69585/#review211465 ----------------------------------------------------------- On Dec. 19, 2018, 10:50 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69585/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2018, 10:50 p.m.) > > > Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio > Pena, and Vihang Karajgaonkar. > > > Bugs: HIVE-20776 > https://issues.apache.org/jira/browse/HIVE-20776 > > > Repository: hive-git > > > Description > ------- > > add filtering to read result at HMS server, so user cannot see metadata > he/she has no privileges. Filtering is enabled/disabled based on > configuration. > > > Diffs > ----- > > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd > standalone-metastore/metastore-server/pom.xml > 895abfc423f00b121ee63e40904f5b3e57aea8ed > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/69585/diff/3/ > > > Testing > ------- > > Existing unit tests passed. > add new unit tests for filtering at HMS server and HMS client > add code to enabled/disable filtering at HMS client based on configuration > > > Thanks, > > Na Li > >
