----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27290/#review58842 -----------------------------------------------------------
Ship it! If Hive doesn't have these checks you might consider filing a JIRA against them make the HMS more secure. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java <https://reviews.apache.org/r/27290/#comment99996> I could see how this could grow in the future, consider splitting this out to its own function (with a comment on the purpose) so you just have: if (isBannedConfigVal(attr)) { throw ... } sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java <https://reviews.apache.org/r/27290/#comment99997> nit: rename to checkBannedConfigVal() - Lenni Kuff On Oct. 28, 2014, 5:35 p.m., Mike Yoder wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27290/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2014, 5:35 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > SENTRY-507 adding two more items to banned config vals in getConfigVal() > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 6de9992556d176aec546b93d928e33e8aa2f2ab4 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 23bd7653c94f6337b5cbbc927b39fc811f275482 > > Diff: https://reviews.apache.org/r/27290/diff/ > > > Testing > ------- > > Unit testing; added new tests for additional items. > > > Thanks, > > Mike Yoder > >
