> On April 19, 2018, 10:04 p.m., Na Li wrote: > > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java > > Lines 60 (patched) > > <https://reviews.apache.org/r/66590/diff/4/?file=2002482#file2002482line60> > > > > should we make this function thread-safe? It invoves two tables and > > they have to be consistent.
Not at this time, there should only be 1 thread that would be calling this at this time and for the next few revisions. > On April 19, 2018, 10:04 p.m., Na Li wrote: > > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java > > Lines 101 (patched) > > <https://reviews.apache.org/r/66590/diff/4/?file=2002482#file2002482line101> > > > > do we need to make this function thread-safe? Otherwise, those two > > tables may be in inconsistent state. Not at this time, there should only be 1 thread that would be calling this at this time and for the next few revisions. > On April 19, 2018, 10:04 p.m., Na Li wrote: > > sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java > > Lines 116 (patched) > > <https://reviews.apache.org/r/66590/diff/4/?file=2002486#file2002486line116> > > > > should each policy has an index? So you can print the index and it is > > easy to debug and correleate events. It can, once we add profiles it would go better here. > On April 19, 2018, 10:04 p.m., Na Li wrote: > > sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java > > Lines 119 (patched) > > <https://reviews.apache.org/r/66590/diff/4/?file=2002486#file2002486line119> > > > > 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? Yes, the policies WILL have priorities, for now if there is a label on the field it is redacted. This will be expanded upon in later versions. > On April 19, 2018, 10:04 p.m., Na Li wrote: > > sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java > > Lines 48 (patched) > > <https://reviews.apache.org/r/66590/diff/4/?file=2002488#file2002488line48> > > > > do you expect someone calls this function? > > > > It seems you should have a internal thread to periodically pull other > > places to get delta change. Yes, right now static ingestion is provided, this a placeholder for deltas in the next ingestion method that will poll for changes. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66590/#review201567 ----------------------------------------------------------- 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 > >
