----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66590/#review201522 -----------------------------------------------------------
sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java Lines 19 (patched) <https://reviews.apache.org/r/66590/#comment282806> Is Attribute persisted? sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 44 (patched) <https://reviews.apache.org/r/66590/#comment282813> If Attribute and SentryObject are tables, I believe that this should also be a table. Seems like it is a Many-To-Many mapping. If you had the database we won't need many of the below methods right? sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 60 (patched) <https://reviews.apache.org/r/66590/#comment282809> Thinking if there is a better way to do this Instead of addEntry, can we have addAttributeToObject, and addObjectToAttribute? Then we could have 2 methods as opposed to 3 separate addEntry methods sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 61 (patched) <https://reviews.apache.org/r/66590/#comment282811> Add Null checks? sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java Lines 121 (patched) <https://reviews.apache.org/r/66590/#comment282812> This check should happen before we call targetObjects.remove(targetObject). I don't think we should be removing the object, if we can't remove the attribute sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java Lines 25 (patched) <https://reviews.apache.org/r/66590/#comment282807> Can we have a description here, just like the one you had for Attribute? sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java Lines 27 (patched) <https://reviews.apache.org/r/66590/#comment282808> this class seems identical to Attrubute. Any reason why we have another one? Is this going to be persisted? - Arjun Mishra On April 16, 2018, 7:51 p.m., Steve Moist wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66590/ > ----------------------------------------------------------- > > (Updated April 16, 2018, 7:51 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/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/4/ > > > Testing > ------- > > full build,added unit tests, tested code on a cluster. > > > Thanks, > > Steve Moist > >
