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/25 02:53:58 UTC
Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/
-----------------------------------------------------------
Review request for sentry, Dapeng Sun and Hao Hao.
Bugs: SENTRY-1014
https://issues.apache.org/jira/browse/SENTRY-1014
Repository: sentry
Description
-------
SENTRY-1014: Add E2E tests for Kafka plugin.
Diffs
-----
pom.xml ca2c92a26e3e42fe036c974235db47255d1465de
sentry-binding/sentry-binding-kafka/pom.xml bd24c20edcbffb5caee69aedddb93cb5a6c8c112
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 9e72d7890e0db3710ab0971ccbb71a32d3d36e87
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java 92e50e6452a89e676eb559a3657b17ba563bfddc
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java e75ec7eddaa1a48b731d551736859a94c08af593
sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java eafe0f0ee5482fadee43ddec99bdc5da3f42e30f
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 7b8b5187e3d95a49304512e238e778da885fd27d
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 20d5e8e518cb91b32fad930e9d39400764f40b55
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java f1ed0001883e5d4b362389c258f4f72697a1e5a4
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 513c2719424f994e8eb773e72073401b465b7d58
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java bcc119860860d928d092ed3ddc0a5a28e841c9aa
sentry-provider/sentry-provider-db/pom.xml 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a
sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb
sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
Diff: https://reviews.apache.org/r/43977/diff/
Testing
-------
Added E2E tests.
Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
Thanks,
Ashish Singh
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
> On Feb. 29, 2016, 9:38 a.m., Dapeng Sun wrote:
> > Thank Ashish, I left some comments.
Dapeng thanks for the review. Addressed your concerns.
> On Feb. 29, 2016, 9:38 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java, line 68
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270528#file1270528line68>
> >
> > need sync?
Good point. Fixed.
> On Feb. 29, 2016, 9:38 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java, line 44
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270532#file1270532line44>
> >
> > Is sentryKafkaAuthorizer depended on AbstractKafkaSentryTestBase? If not, we should move the tests to "SENTRY-1057 Add implementations for acls' CRUD"
Makes sense/ moving.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review121214
-----------------------------------------------------------
On Feb. 25, 2016, 1:53 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2016, 1:53 a.m.)
>
>
> Review request for sentry, Dapeng Sun and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> pom.xml ca2c92a26e3e42fe036c974235db47255d1465de
> sentry-binding/sentry-binding-kafka/pom.xml bd24c20edcbffb5caee69aedddb93cb5a6c8c112
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 9e72d7890e0db3710ab0971ccbb71a32d3d36e87
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java 92e50e6452a89e676eb559a3657b17ba563bfddc
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java e75ec7eddaa1a48b731d551736859a94c08af593
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java eafe0f0ee5482fadee43ddec99bdc5da3f42e30f
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 7b8b5187e3d95a49304512e238e778da885fd27d
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 20d5e8e518cb91b32fad930e9d39400764f40b55
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java f1ed0001883e5d4b362389c258f4f72697a1e5a4
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 513c2719424f994e8eb773e72073401b465b7d58
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java bcc119860860d928d092ed3ddc0a5a28e841c9aa
> sentry-provider/sentry-provider-db/pom.xml 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a
> sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb
> sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review121214
-----------------------------------------------------------
Thank Ashish, I left some comments.
sentry-tests/sentry-tests-kafka/pom.xml (line 32)
<https://reviews.apache.org/r/43977/#comment182884>
The scope of some dependences should be "test"
sentry-tests/sentry-tests-kafka/pom.xml (line 40)
<https://reviews.apache.org/r/43977/#comment182881>
Move the version to the top-level pom.xml
sentry-tests/sentry-tests-kafka/pom.xml (line 49)
<https://reviews.apache.org/r/43977/#comment182882>
Move the version to the top-level pom.xml
sentry-tests/sentry-tests-kafka/pom.xml (line 54)
<https://reviews.apache.org/r/43977/#comment182883>
Move the version to the top-level pom.xml
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 56)
<https://reviews.apache.org/r/43977/#comment182889>
need to check if kafkaServer is null
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 61)
<https://reviews.apache.org/r/43977/#comment182890>
need to check if zkServer is null
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 68)
<https://reviews.apache.org/r/43977/#comment182886>
need sync?
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 69)
<https://reviews.apache.org/r/43977/#comment182887>
zkServerSocket should be renamed
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 73)
<https://reviews.apache.org/r/43977/#comment182888>
Should return the exception, otherwise there will be a NullPointerException
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 83)
<https://reviews.apache.org/r/43977/#comment182891>
Throw a runtime exception here?
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 109)
<https://reviews.apache.org/r/43977/#comment182885>
CumtomePrincipalBuilder is unused? should we delete it?
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 114)
<https://reviews.apache.org/r/43977/#comment182893>
How about change generate to setup?
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java (line 123)
<https://reviews.apache.org/r/43977/#comment182892>
Throw exception?
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java (line 21)
<https://reviews.apache.org/r/43977/#comment182894>
Can we use FileUtils at apache common?
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java (line 21)
<https://reviews.apache.org/r/43977/#comment182895>
Can we use FileUtils at apache common?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java (line 18)
<https://reviews.apache.org/r/43977/#comment182896>
The comments need to move before the class name
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java (line 59)
<https://reviews.apache.org/r/43977/#comment182897>
Is **getHostname()** okay? and it should not use port 80.
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java (line 44)
<https://reviews.apache.org/r/43977/#comment182901>
Is sentryKafkaAuthorizer depended on AbstractKafkaSentryTestBase? If not, we should move the tests to "SENTRY-1057 Add implementations for acls' CRUD"
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java (line 52)
<https://reviews.apache.org/r/43977/#comment182900>
The indentation should be two Spaces
- Dapeng Sun
On 二月 25, 2016, 9:53 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated 二月 25, 2016, 9:53 a.m.)
>
>
> Review request for sentry, Dapeng Sun and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> pom.xml ca2c92a26e3e42fe036c974235db47255d1465de
> sentry-binding/sentry-binding-kafka/pom.xml bd24c20edcbffb5caee69aedddb93cb5a6c8c112
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 9e72d7890e0db3710ab0971ccbb71a32d3d36e87
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java 92e50e6452a89e676eb559a3657b17ba563bfddc
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java e75ec7eddaa1a48b731d551736859a94c08af593
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java eafe0f0ee5482fadee43ddec99bdc5da3f42e30f
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 7b8b5187e3d95a49304512e238e778da885fd27d
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 20d5e8e518cb91b32fad930e9d39400764f40b55
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java f1ed0001883e5d4b362389c258f4f72697a1e5a4
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 513c2719424f994e8eb773e72073401b465b7d58
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java bcc119860860d928d092ed3ddc0a5a28e841c9aa
> sentry-provider/sentry-provider-db/pom.xml 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a
> sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb
> sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
> On Feb. 26, 2016, 10:41 p.m., Anne Yu wrote:
> >
Anne, thanks for the review.
> On Feb. 26, 2016, 10:41 p.m., Anne Yu wrote:
> > sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java, line 347
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270512#file1270512line347>
> >
> > Wondering if these lines are still needed? If so, why comment them out?
Removed in SENTRY-1057.
> On Feb. 26, 2016, 10:41 p.m., Anne Yu wrote:
> > sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java, line 63
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270520#file1270520line63>
> >
> > Does this change mean for cluster name, can't be case-insensitive?
No, it can't. It is discussed more on SENTRY-1030.
> On Feb. 26, 2016, 10:41 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java, line 17
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270532#file1270532line17>
> >
> > High level question: can these e2e tests be run on a real cluster? sentry dbProvider tests can run both locally and on a real cluster. If these kafka tests can also be run for real, we can effectively test generical model + clients, even server/client compatibility.
Not sure. I have not tried that, I think it should be doable with a few tweaks. However, this JIRA is not going to address that issue. If required, we will file a separate JIRA to track that.
> On Feb. 26, 2016, 10:41 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java, line 134
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270532#file1270532line134>
> >
> > Wondering why catch then rethrow?
For better error message. Also, I think that it makes the test readable.
> On Feb. 26, 2016, 10:41 p.m., Anne Yu wrote:
> > sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java, line 17
> > <https://reviews.apache.org/r/43977/diff/1/?file=1270518#file1270518line17>
> >
> > High level design question: could these kafka tests be run parallelly (on class level)? We want to reduce unit tests running time.
Good question, have not tested it, but should be doable. We should file a separate JIRA if we want to track that. Moreover, those tests are not part of this JIRA.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review120974
-----------------------------------------------------------
On Feb. 25, 2016, 1:53 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2016, 1:53 a.m.)
>
>
> Review request for sentry, Dapeng Sun and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> pom.xml ca2c92a26e3e42fe036c974235db47255d1465de
> sentry-binding/sentry-binding-kafka/pom.xml bd24c20edcbffb5caee69aedddb93cb5a6c8c112
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 9e72d7890e0db3710ab0971ccbb71a32d3d36e87
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java 92e50e6452a89e676eb559a3657b17ba563bfddc
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java e75ec7eddaa1a48b731d551736859a94c08af593
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java eafe0f0ee5482fadee43ddec99bdc5da3f42e30f
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 7b8b5187e3d95a49304512e238e778da885fd27d
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 20d5e8e518cb91b32fad930e9d39400764f40b55
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java f1ed0001883e5d4b362389c258f4f72697a1e5a4
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 513c2719424f994e8eb773e72073401b465b7d58
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java bcc119860860d928d092ed3ddc0a5a28e841c9aa
> sentry-provider/sentry-provider-db/pom.xml 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a
> sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb
> sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review120974
-----------------------------------------------------------
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java (line 345)
<https://reviews.apache.org/r/43977/#comment182529>
Wondering if these lines are still needed? If so, why comment them out?
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java (line 17)
<https://reviews.apache.org/r/43977/#comment182539>
High level design question: could these kafka tests be run parallelly (on class level)? We want to reduce unit tests running time.
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java (line 63)
<https://reviews.apache.org/r/43977/#comment182530>
Does this change mean for cluster name, can't be case-insensitive?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java (line 17)
<https://reviews.apache.org/r/43977/#comment182535>
High level question: can these e2e tests be run on a real cluster? sentry dbProvider tests can run both locally and on a real cluster. If these kafka tests can also be run for real, we can effectively test generical model + clients, even server/client compatibility.
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java (line 134)
<https://reviews.apache.org/r/43977/#comment182538>
Wondering why catch then rethrow?
- Anne Yu
On Feb. 25, 2016, 1:53 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2016, 1:53 a.m.)
>
>
> Review request for sentry, Dapeng Sun and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> pom.xml ca2c92a26e3e42fe036c974235db47255d1465de
> sentry-binding/sentry-binding-kafka/pom.xml bd24c20edcbffb5caee69aedddb93cb5a6c8c112
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 9e72d7890e0db3710ab0971ccbb71a32d3d36e87
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java 92e50e6452a89e676eb559a3657b17ba563bfddc
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java e75ec7eddaa1a48b731d551736859a94c08af593
> sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java eafe0f0ee5482fadee43ddec99bdc5da3f42e30f
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1
> sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 7b8b5187e3d95a49304512e238e778da885fd27d
> sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 20d5e8e518cb91b32fad930e9d39400764f40b55
> sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java f1ed0001883e5d4b362389c258f4f72697a1e5a4
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 513c2719424f994e8eb773e72073401b465b7d58
> sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java bcc119860860d928d092ed3ddc0a5a28e841c9aa
> sentry-provider/sentry-provider-db/pom.xml 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a
> sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb
> sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review122783
-----------------------------------------------------------
Ship it!
- Hao Hao
On March 9, 2016, 7:25 p.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated March 9, 2016, 7:25 p.m.)
>
>
> Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review122854
-----------------------------------------------------------
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java (line 52)
<https://reviews.apache.org/r/43977/#comment184945>
Thank Ashish for your contribution, I didn't find AbstractKafkaSentryTestBase, is it missing? https://github.com/apache/incubator-sentry/tree/kafka/sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka
Thank Ashish for your contribution, I have only one comment.
- Dapeng Sun
On 三月 10, 2016, 3:25 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated 三月 10, 2016, 3:25 a.m.)
>
>
> Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/
-----------------------------------------------------------
(Updated March 9, 2016, 7:25 p.m.)
Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
Changes
-------
Update error messages.
Bugs: SENTRY-1014
https://issues.apache.org/jira/browse/SENTRY-1014
Repository: sentry
Description
-------
SENTRY-1014: Add E2E tests for Kafka plugin.
Diffs (updated)
-----
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
Diff: https://reviews.apache.org/r/43977/diff/
Testing
-------
Added E2E tests.
Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
Thanks,
Ashish Singh
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/
-----------------------------------------------------------
(Updated March 9, 2016, 5:32 p.m.)
Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
Changes
-------
Address review comments.
Bugs: SENTRY-1014
https://issues.apache.org/jira/browse/SENTRY-1014
Repository: sentry
Description
-------
SENTRY-1014: Add E2E tests for Kafka plugin.
Diffs (updated)
-----
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
Diff: https://reviews.apache.org/r/43977/diff/
Testing
-------
Added E2E tests.
Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
Thanks,
Ashish Singh
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
> On March 8, 2016, 2 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java, line 58
> > <https://reviews.apache.org/r/43977/diff/3/?file=1287043#file1287043line58>
> >
> > Should it be 'admin' instead of 'test' to be more accurate?
This will require changing the generated keystore and truststore files to just rename it. I have used a variable `final String SuperuserName = "test";` too allevaite your concern. Hope it is OK with you.
> On March 8, 2016, 2 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java, line 102
> > <https://reviews.apache.org/r/43977/diff/3/?file=1287043#file1287043line102>
> >
> > "create topic" instead of "describe topic"?
Added more info.
> On March 8, 2016, 2 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java, line 111
> > <https://reviews.apache.org/r/43977/diff/3/?file=1287043#file1287043line111>
> >
> > A little confused here. Isn't addPermission grant the needed permission there?
Here user is trying to produce/write to a topic. So, the user should have write/produce permissions on the topic. Did I answer your question?
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review122434
-----------------------------------------------------------
On March 8, 2016, 12:23 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 12:23 a.m.)
>
>
> Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/#review122434
-----------------------------------------------------------
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java (line 58)
<https://reviews.apache.org/r/43977/#comment184431>
Should it be 'admin' instead of 'test' to be more accurate?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java (line 102)
<https://reviews.apache.org/r/43977/#comment184446>
"create topic" instead of "describe topic"?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java (line 111)
<https://reviews.apache.org/r/43977/#comment184452>
A little confused here. Isn't addPermission grant the needed permission there?
- Hao Hao
On March 8, 2016, 12:23 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43977/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 12:23 a.m.)
>
>
> Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
>
>
> Bugs: SENTRY-1014
> https://issues.apache.org/jira/browse/SENTRY-1014
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1014: Add E2E tests for Kafka plugin.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
> sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43977/diff/
>
>
> Testing
> -------
>
> Added E2E tests.
>
> Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/
-----------------------------------------------------------
(Updated March 8, 2016, 12:23 a.m.)
Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
Changes
-------
Rebased.
Bugs: SENTRY-1014
https://issues.apache.org/jira/browse/SENTRY-1014
Repository: sentry
Description
-------
SENTRY-1014: Add E2E tests for Kafka plugin.
Diffs (updated)
-----
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java 129191ae6f9fa4cc6794117fddb693a3cd1ab815
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
Diff: https://reviews.apache.org/r/43977/diff/
Testing
-------
Added E2E tests.
Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
Thanks,
Ashish Singh
Re: Review Request 43977: SENTRY-1014: Add E2E tests for Kafka plugin.
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43977/
-----------------------------------------------------------
(Updated Feb. 29, 2016, 9:06 p.m.)
Review request for sentry, Anne Yu, Dapeng Sun, and Hao Hao.
Changes
-------
Move acls crudtests to SENTRY-1057 and address review comments.
Bugs: SENTRY-1014
https://issues.apache.org/jira/browse/SENTRY-1014
Repository: sentry
Description
-------
SENTRY-1014: Add E2E tests for Kafka plugin.
Diffs (updated)
-----
.gitignore a89bad852812015695f373d18f8d9f72a3acce0e
pom.xml ca2c92a26e3e42fe036c974235db47255d1465de
sentry-binding/sentry-binding-kafka/pom.xml bd24c20edcbffb5caee69aedddb93cb5a6c8c112
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizer.java 9ffb971d8da51b716ece3ff5a5bbeebcc7f74e8d
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 9e72d7890e0db3710ab0971ccbb71a32d3d36e87
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBindingSingleton.java 92e50e6452a89e676eb559a3657b17ba563bfddc
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/conf/KafkaAuthConf.java e75ec7eddaa1a48b731d551736859a94c08af593
sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/kafka/authorizer/SentryKafkaAuthorizerTest.java eafe0f0ee5482fadee43ddec99bdc5da3f42e30f
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/Cluster.java bb30b1b78d4dccaf8ea5fe22eeed496b608cfcb1
sentry-core/sentry-core-model-kafka/src/main/java/org/apache/sentry/core/model/kafka/KafkaActionFactory.java 7b8b5187e3d95a49304512e238e778da885fd27d
sentry-core/sentry-core-model-kafka/src/test/java/org/apache/sentry/core/model/kafka/TestKafkaAuthorizable.java 20d5e8e518cb91b32fad930e9d39400764f40b55
sentry-policy/sentry-policy-kafka/src/main/java/org/apache/sentry/policy/kafka/KafkaModelAuthorizables.java f1ed0001883e5d4b362389c258f4f72697a1e5a4
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/TestKafkaModelAuthorizables.java 513c2719424f994e8eb773e72073401b465b7d58
sentry-policy/sentry-policy-kafka/src/test/java/org/apache/sentry/policy/kafka/provider/TestKafkaAuthorizationProviderGeneralCases.java bcc119860860d928d092ed3ddc0a5a28e841c9aa
sentry-provider/sentry-provider-db/pom.xml 7514a7cdfcc7934f2dd0386996fdaf88c0ccbb14
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java c3b0be8694c746cb09797425f98578b8faef8b4a
sentry-tests/pom.xml 3294335e95fb7dbb2da4151041269392004426fb
sentry-tests/sentry-tests-kafka/pom.xml PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/CustomPrincipalBuilder.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/EmbeddedZkServer.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/StaticUserGroupRole.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/log4j.properties PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/test.crt PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/test.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/test.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.crt PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user1.truststore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.crt PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.keystore.jks PRE-CREATION
sentry-tests/sentry-tests-kafka/src/test/resources/user2.truststore.jks PRE-CREATION
Diff: https://reviews.apache.org/r/43977/diff/
Testing
-------
Added E2E tests.
Note that this contains changes from SENTRY-1098, SENTRY-1056, SENTRY-1030 and SENTRY-1057, and will have to be rebased once they get it.
Thanks,
Ashish Singh