----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67506/#review204543 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java Lines 70-74 (patched) <https://reviews.apache.org/r/67506/#comment287098> I don't feel necessary to do this loop to translate OWNER privileges prior to sending them to HDFS. HDFS can know what to do with an OWNER privilege the same it does with the ALL privilege in the UpdateableAuthzPermissions used in the binding side (that's one single line of code). I thought about what you said what if in the future we add a configuration for what OWNER will mean. If we do that, which I don't think will happen because we're behaving like other DBs where OWNER = ALL, but if we do, then this change in HDFS will not be affected because but that time Sentry will not send the OWNER anymore, but the new privilege used in the configuration. I prefer to translate the OWNER privilege in the NN side instead of adding another loop that walks through all the privileges send to the NN. This is adding an extra overhead on the updates for the NN. - Sergio Pena On June 8, 2018, 12:26 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67506/ > ----------------------------------------------------------- > > (Updated June 8, 2018, 12:26 p.m.) > > > Review request for sentry, Na Li and Sergio Pena. > > > Bugs: SENTRY-2260 > https://issues.apache.org/jira/browse/SENTRY-2260 > > > Repository: sentry > > > Description > ------- > > When owner privileges are implicitly granted/revoked, sentry ACL's in > Namenode plug-in should be updated accordingly. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > 71ef5f9251f182825177c00a8456ca3166b2d095 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java > 6974d37aac9cdcbc21edfe66b1794dd1df6597fb > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > 10d52b43729426c53c0168fc0b7d0cdf0e307b57 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDeltaRetriever.java > 60696cccc0d679dfa6b3e6c06cb5120942facc14 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java > d2d5391182d0da395562ad23f742d704d9a0ceb5 > > > Diff: https://reviews.apache.org/r/67506/diff/1/ > > > Testing > ------- > > Added new unit tests to verify the functionality. > > > Thanks, > > kalyan kumar kalvagadda > >
