----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66590/#review201567 -----------------------------------------------------------
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 60 (patched) <https://reviews.apache.org/r/66590/#comment282874> should we make this function thread-safe? It invoves two tables and they have to be consistent. sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 101 (patched) <https://reviews.apache.org/r/66590/#comment282871> do we need to make this function thread-safe? Otherwise, those two tables may be in inconsistent state. sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 207 (patched) <https://reviews.apache.org/r/66590/#comment282879> This function name is strange. It just returns the sentry object, not action sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java Lines 20 (patched) <https://reviews.apache.org/r/66590/#comment282884> Can you add google doc? sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java Lines 31 (patched) <https://reviews.apache.org/r/66590/#comment282885> can you add google doc for this class? sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java Lines 116 (patched) <https://reviews.apache.org/r/66590/#comment282890> should each policy has an index? So you can print the index and it is easy to debug and correleate events. sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java Lines 119 (patched) <https://reviews.apache.org/r/66590/#comment282891> This logic has some problem. will the policies have priorities? then they should be sorted before you check them and need to handle conflict. If you don't do that, what's the assumption the policies behave? sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java Lines 48 (patched) <https://reviews.apache.org/r/66590/#comment282896> do you expect someone calls this function? It seems you should have a internal thread to periodically pull other places to get delta change. - Na Li On April 19, 2018, 9:09 p.m., Steve Moist wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66590/ > ----------------------------------------------------------- > > (Updated April 19, 2018, 9:09 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > This is the inital draft of attribute based access control. > > > Diffs > ----- > > pom.xml 16a3838a > sentry-abac/example-definition.json PRE-CREATION > sentry-abac/example-delta.json PRE-CREATION > sentry-abac/notes.txt PRE-CREATION > sentry-abac/pom.xml PRE-CREATION > sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java > PRE-CREATION > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java > PRE-CREATION > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java > PRE-CREATION > > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java > PRE-CREATION > > sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java > PRE-CREATION > > sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java > PRE-CREATION > sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java > PRE-CREATION > > sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java > PRE-CREATION > sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java > PRE-CREATION > > sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java > PRE-CREATION > > sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java > PRE-CREATION > > sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java > PRE-CREATION > sentry-abac/src/test/resources/abac.props PRE-CREATION > sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java > 1ab5be35 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java > 86ff0cc2 > > > Diff: https://reviews.apache.org/r/66590/diff/5/ > > > Testing > ------- > > full build,added unit tests, tested code on a cluster. > > > Thanks, > > Steve Moist > >
