You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hao Hao <ha...@cloudera.com> on 2016/02/10 04:04:35 UTC
Re: Review Request 42778: SENTRY-1013: Add policy engine for Kafka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42778/#review118537
-----------------------------------------------------------
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java (line 30)
<https://reviews.apache.org/r/42778/#comment179802>
Is the resource value case sensitve? Can we do toLowerCase here if so?
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaPrivilegeValidator.java (line 49)
<https://reviews.apache.org/r/42778/#comment179828>
Other than checking whether privilege starts with host. Is it possible for a privilege be as "host=host1->topci=t1->cluster=c1->action=read"? If not possible, do we also need to validate the order of the resource?
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java (line 68)
<https://reviews.apache.org/r/42778/#comment179818>
Space after if.
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java (line 45)
<https://reviews.apache.org/r/42778/#comment179827>
Can you check the excption is as expected here?
- Hao Hao
On Jan. 27, 2016, 6:16 p.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42778/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2016, 6:16 p.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-1013
> https://issues.apache.org/jira/browse/SENTRY-1013
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1013: Add policy engine for Kafka
>
>
> Diffs
> -----
>
> pom.xml aa4a021eae93c3c817ec055e18868b95f7cba124
> sentry-policy/pom.xml ef938a6b52bb00042ea8ae32da3d245ef0697a00
> sentry-policy/sentry-policy-kafka/pom.xml PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaPrivilegeValidator.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/SimpleKafkaPolicyEngine.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/KafkaPolicyFileProviderBackend.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/MockGroupMappingServiceProvider.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaWildcardPrivilege.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/AbstractTestKafkaPolicyEngine.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/TestKafkaPolicyEngineDFS.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/TestKafkaPolicyEngineLocalFS.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderSpecialCases.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaPolicyNegative.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/resources/log4j.properties PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/resources/test-authz-provider.ini PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42778/diff/
>
>
> Testing
> -------
>
> Tested via unit tests and end-to-end tests that will be added as part of SENTRY-1014.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 42778: SENTRY-1013: Add policy engine for Kafka
Posted by Ashish Singh <as...@cloudera.com>.
> On Feb. 10, 2016, 3:04 a.m., Hao Hao wrote:
> > sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java, line 30
> > <https://reviews.apache.org/r/42778/diff/2/?file=1224068#file1224068line30>
> >
> > Is the resource value case sensitve? Can we do toLowerCase here if so?
Thanks for finding this, fixed and added tests to check the same.
> On Feb. 10, 2016, 3:04 a.m., Hao Hao wrote:
> > sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaPrivilegeValidator.java, line 49
> > <https://reviews.apache.org/r/42778/diff/2/?file=1224069#file1224069line49>
> >
> > Other than checking whether privilege starts with host. Is it possible for a privilege be as "host=host1->topci=t1->cluster=c1->action=read"? If not possible, do we also need to validate the order of the resource?
Yes, added,
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42778/#review118537
-----------------------------------------------------------
On Jan. 27, 2016, 6:16 p.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42778/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2016, 6:16 p.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-1013
> https://issues.apache.org/jira/browse/SENTRY-1013
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1013: Add policy engine for Kafka
>
>
> Diffs
> -----
>
> pom.xml aa4a021eae93c3c817ec055e18868b95f7cba124
> sentry-policy/pom.xml ef938a6b52bb00042ea8ae32da3d245ef0697a00
> sentry-policy/sentry-policy-kafka/pom.xml PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaPrivilegeValidator.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/SimpleKafkaPolicyEngine.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/KafkaPolicyFileProviderBackend.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/MockGroupMappingServiceProvider.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaWildcardPrivilege.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/AbstractTestKafkaPolicyEngine.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/TestKafkaPolicyEngineDFS.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/TestKafkaPolicyEngineLocalFS.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderSpecialCases.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaPolicyNegative.java PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/resources/log4j.properties PRE-CREATION
> sentry-policy/sentry-policy-kafka/src/test/resources/test-authz-provider.ini PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42778/diff/
>
>
> Testing
> -------
>
> Tested via unit tests and end-to-end tests that will be added as part of SENTRY-1014.
>
>
> Thanks,
>
> Ashish Singh
>
>