You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Ashish Singh <as...@cloudera.com> on 2016/02/09 20:50:29 UTC
Re: Review Request 42924: SENTRY-1029: Remove support for * as kafka
action. Add Javadocs. Add negative tests for KafkaAction.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42924/
-----------------------------------------------------------
(Updated Feb. 9, 2016, 7:50 p.m.)
Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff.
Changes
-------
Fix bug id.
Summary (updated)
-----------------
SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
Bugs: SENTRY-1029
https://issues.apache.org/jira/browse/SENTRY-1029
Repository: sentry
Description (updated)
-------
SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
Diffs
-----
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java 9525aaff4f0b9b92212aa59919c2cb6078376b95
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java e0f4160269a652f2b4d0c1d7376b34ee80c0e10e
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 13421f9c0be31b372c910d304a081403bdea0998
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 2577406fb3d3a4bbaab0f5733e7054c61498a22c
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java 9e288b0c92b91964d2d8ab4e727d4f5257e47502
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java f22ebc03a27b5f08e501b40990c20fbf4372cddc
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 1abb116023e7a652684f981a6e8261abf8c4d8c5
Diff: https://reviews.apache.org/r/42924/diff/
Testing
-------
Added unit tests.
Thanks,
Ashish Singh
Re: Review Request 42924: SENTRY-1029: Remove support for * as kafka
action. Add Javadocs. Add negative tests for KafkaAction.
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/KafkaWildcardPrivilege.java, line 122
> > <https://reviews.apache.org/r/42924/diff/2/?file=1239150#file1239150line122>
> >
> > Why not make the Kafka action to be "*" instead of "ALL"? And here you probably want to use KafkaAuthorizable.ALL, so that no need extra checking for host?
'ALL' is defined as an action in Kafka, it is better to be inline with Kafka terminologies. I have refactored this logic a bit. Should make it easier to comprehend now.
> On Feb. 10, 2016, 3:04 a.m., Hao Hao wrote:
> > sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java, line 44
> > <https://reviews.apache.org/r/42924/diff/2/?file=1239146#file1239146line44>
> >
> > Can you add a comment on what "*" stands for?
This is some left over code. Removed.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42924/#review118530
-----------------------------------------------------------
On Feb. 10, 2016, 12:49 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42924/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 12:49 a.m.)
>
>
> Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff.
>
>
> Bugs: SENTRY-1029
> https://issues.apache.org/jira/browse/SENTRY-1029
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java ccbe60ee89f1ccf9f808a20f0d39db20f8bcbcd2
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java 9525aaff4f0b9b92212aa59919c2cb6078376b95
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java e0f4160269a652f2b4d0c1d7376b34ee80c0e10e
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 13421f9c0be31b372c910d304a081403bdea0998
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 2577406fb3d3a4bbaab0f5733e7054c61498a22c
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java 9e288b0c92b91964d2d8ab4e727d4f5257e47502
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java f22ebc03a27b5f08e501b40990c20fbf4372cddc
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 1abb116023e7a652684f981a6e8261abf8c4d8c5
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java e04aeb754c9cf9b7dc4fd7fec1ea45e479915f23
>
> Diff: https://reviews.apache.org/r/42924/diff/
>
>
> Testing
> -------
>
> Added unit tests.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 42924: SENTRY-1029: Remove support for * as kafka
action. Add Javadocs. Add negative tests for KafkaAction.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42924/#review118530
-----------------------------------------------------------
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java (line 44)
<https://reviews.apache.org/r/42924/#comment179795>
Can you add a comment on what "*" stands for?
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java (line 122)
<https://reviews.apache.org/r/42924/#comment179826>
Why not make the Kafka action to be "*" instead of "ALL"? And here you probably want to use KafkaAuthorizable.ALL, so that no need extra checking for host?
- Hao Hao
On Feb. 10, 2016, 12:49 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42924/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 12:49 a.m.)
>
>
> Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff.
>
>
> Bugs: SENTRY-1029
> https://issues.apache.org/jira/browse/SENTRY-1029
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java ccbe60ee89f1ccf9f808a20f0d39db20f8bcbcd2
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java 9525aaff4f0b9b92212aa59919c2cb6078376b95
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java e0f4160269a652f2b4d0c1d7376b34ee80c0e10e
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 13421f9c0be31b372c910d304a081403bdea0998
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 2577406fb3d3a4bbaab0f5733e7054c61498a22c
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java 9e288b0c92b91964d2d8ab4e727d4f5257e47502
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java f22ebc03a27b5f08e501b40990c20fbf4372cddc
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 1abb116023e7a652684f981a6e8261abf8c4d8c5
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java e04aeb754c9cf9b7dc4fd7fec1ea45e479915f23
>
> Diff: https://reviews.apache.org/r/42924/diff/
>
>
> Testing
> -------
>
> Added unit tests.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 42924: SENTRY-1029: Remove support for * as kafka
action. Add Javadocs. Add negative tests for KafkaAction.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42924/#review119111
-----------------------------------------------------------
Ship it!
Ship It!
- Hao Hao
On Feb. 11, 2016, 7:20 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42924/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 7:20 a.m.)
>
>
> Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff.
>
>
> Bugs: SENTRY-1029
> https://issues.apache.org/jira/browse/SENTRY-1029
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java ccbe60ee89f1ccf9f808a20f0d39db20f8bcbcd2
> sentry-binding/sentry-binding-kafka/src/test/resources/test-authz-provider.ini 5f853827d369646f13c93df4d2a3c64047569a8e
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java 9525aaff4f0b9b92212aa59919c2cb6078376b95
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java e0f4160269a652f2b4d0c1d7376b34ee80c0e10e
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 13421f9c0be31b372c910d304a081403bdea0998
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 2577406fb3d3a4bbaab0f5733e7054c61498a22c
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java 9e288b0c92b91964d2d8ab4e727d4f5257e47502
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java f22ebc03a27b5f08e501b40990c20fbf4372cddc
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 1abb116023e7a652684f981a6e8261abf8c4d8c5
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java ba9303653a82445dab30a75095a483d42a43b5cb
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaPrivilegeValidator.java ecad3553254adf63f93d0ceae5eb78cdf6b74a21
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java e04aeb754c9cf9b7dc4fd7fec1ea45e479915f23
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 46a00787728992ab4acbf1f90dfe1252f21f93fc
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java ba670f7b26ceaf955ccd16890c42fd6f8483160b
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaWildcardPrivilege.java 720c98fba6f3f1ffc2f31a7f69287d67c78bc6d6
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/AbstractTestKafkaPolicyEngine.java 4da506b1311e7e749c7b961d80a7b08d39dbc4e7
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaPolicyNegative.java 0186cc96d0340e7aa0b597fdffcdef4e938c1a1e
> sentry-policy/sentry-policy-kafka/src/test/resources/test-authz-provider.ini c533e690fe8ff5b55fd5ffae1752f8f9ae544bad
>
> Diff: https://reviews.apache.org/r/42924/diff/
>
>
> Testing
> -------
>
> Added unit tests.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 42924: SENTRY-1029: Remove support for * as kafka
action. Add Javadocs. Add negative tests for KafkaAction.
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42924/
-----------------------------------------------------------
(Updated Feb. 11, 2016, 7:20 a.m.)
Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff.
Changes
-------
Remove unwanted code and refactor imples logic a bit.
Bugs: SENTRY-1029
https://issues.apache.org/jira/browse/SENTRY-1029
Repository: sentry
Description
-------
SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
Diffs (updated)
-----
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java ccbe60ee89f1ccf9f808a20f0d39db20f8bcbcd2
sentry-binding/sentry-binding-kafka/src/test/resources/test-authz-provider.ini 5f853827d369646f13c93df4d2a3c64047569a8e
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java 9525aaff4f0b9b92212aa59919c2cb6078376b95
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java e0f4160269a652f2b4d0c1d7376b34ee80c0e10e
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 13421f9c0be31b372c910d304a081403bdea0998
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 2577406fb3d3a4bbaab0f5733e7054c61498a22c
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java 9e288b0c92b91964d2d8ab4e727d4f5257e47502
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java f22ebc03a27b5f08e501b40990c20fbf4372cddc
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 1abb116023e7a652684f981a6e8261abf8c4d8c5
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java ba9303653a82445dab30a75095a483d42a43b5cb
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaPrivilegeValidator.java ecad3553254adf63f93d0ceae5eb78cdf6b74a21
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java e04aeb754c9cf9b7dc4fd7fec1ea45e479915f23
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 46a00787728992ab4acbf1f90dfe1252f21f93fc
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaPrivilegeValidator.java ba670f7b26ceaf955ccd16890c42fd6f8483160b
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaWildcardPrivilege.java 720c98fba6f3f1ffc2f31a7f69287d67c78bc6d6
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/engine/AbstractTestKafkaPolicyEngine.java 4da506b1311e7e749c7b961d80a7b08d39dbc4e7
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaPolicyNegative.java 0186cc96d0340e7aa0b597fdffcdef4e938c1a1e
sentry-policy/sentry-policy-kafka/src/test/resources/test-authz-provider.ini c533e690fe8ff5b55fd5ffae1752f8f9ae544bad
Diff: https://reviews.apache.org/r/42924/diff/
Testing
-------
Added unit tests.
Thanks,
Ashish Singh
Re: Review Request 42924: SENTRY-1029: Remove support for * as kafka
action. Add Javadocs. Add negative tests for KafkaAction.
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42924/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 12:49 a.m.)
Review request for sentry, Dapeng Sun, Hao Hao, and Lenni Kuff.
Changes
-------
Rebased on latest kafka branch.
Bugs: SENTRY-1029
https://issues.apache.org/jira/browse/SENTRY-1029
Repository: sentry
Description
-------
SENTRY-1029: Remove support for * as kafka action. Add Javadocs. Add negative tests for KafkaAction.
Diffs (updated)
-----
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java ccbe60ee89f1ccf9f808a20f0d39db20f8bcbcd2
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java b1fc0637fe4ede82019c5cec9c1e82f5fba2fb8a
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/ConsumerGroup.java 9525aaff4f0b9b92212aa59919c2cb6078376b95
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Host.java e0f4160269a652f2b4d0c1d7376b34ee80c0e10e
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionConstant.java 13421f9c0be31b372c910d304a081403bdea0998
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 2577406fb3d3a4bbaab0f5733e7054c61498a22c
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaAuthorizable.java 0d2155e5eb2fe77e1254337ac49c6ca8593ca6ce
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Topic.java 9e288b0c92b91964d2d8ab4e727d4f5257e47502
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAction.java f22ebc03a27b5f08e501b40990c20fbf4372cddc
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 1abb116023e7a652684f981a6e8261abf8c4d8c5
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaWildcardPrivilege.java e04aeb754c9cf9b7dc4fd7fec1ea45e479915f23
Diff: https://reviews.apache.org/r/42924/diff/
Testing
-------
Added unit tests.
Thanks,
Ashish Singh