You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Szabolcs Vasas <va...@gmail.com> on 2017/08/15 18:56:13 UTC
Review Request 61669: Test HBase kerberized connectivity
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/
-----------------------------------------------------------
Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
Bugs: SQOOP-3222
https://issues.apache.org/jira/browse/SQOOP-3222
Repository: sqoop-trunk
Description
-------
In this patch I have changed the following:
- Added test dependency on hadoop-minikdc.
- Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
- Added kerberos handling logic to HBaseTestCase and refactored it a bit.
- Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
The changes are inspired by the following HBase test classes:
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
HBase security documentation:
http://hbase.apache.org/1.2/book.html#security
Diffs
-----
build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
Diff: https://reviews.apache.org/r/61669/diff/1/
Testing
-------
Ran unit tests and third party tests.
Thanks,
Szabolcs Vasas
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Zoltán Tóth <sw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/#review184139
-----------------------------------------------------------
Ship it!
Hey Szabolcs,
Thanks for your contribution. I ran unit and integration tests to make your changes run smoothly and they did.
Cheers, Zoli
- Zoltán Tóth
On Aug. 30, 2017, 8:17 a.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61669/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2017, 8:17 a.m.)
>
>
> Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
>
>
> Bugs: SQOOP-3222
> https://issues.apache.org/jira/browse/SQOOP-3222
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> In this patch I have changed the following:
> - Added test dependency on hadoop-minikdc.
> - Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
> - Added kerberos handling logic to HBaseTestCase and refactored it a bit.
> - Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
>
> The changes are inspired by the following HBase test classes:
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
>
>
> HBase security documentation:
> http://hbase.apache.org/1.2/book.html#security
>
>
> Diffs
> -----
>
> build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
> ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
> src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
> src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/61669/diff/5/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/
-----------------------------------------------------------
(Updated Aug. 30, 2017, 8:17 a.m.)
Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
Changes
-------
Fixing review findings.
Bugs: SQOOP-3222
https://issues.apache.org/jira/browse/SQOOP-3222
Repository: sqoop-trunk
Description
-------
In this patch I have changed the following:
- Added test dependency on hadoop-minikdc.
- Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
- Added kerberos handling logic to HBaseTestCase and refactored it a bit.
- Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
The changes are inspired by the following HBase test classes:
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
HBase security documentation:
http://hbase.apache.org/1.2/book.html#security
Diffs (updated)
-----
build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
Diff: https://reviews.apache.org/r/61669/diff/5/
Changes: https://reviews.apache.org/r/61669/diff/4-5/
Testing
-------
Ran unit tests and third party tests.
Thanks,
Szabolcs Vasas
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Boglarka Egyed <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/#review184135
-----------------------------------------------------------
Fix it, then Ship it!
Hi Szabi,
I have a very minor finding regarding your change otherwise it looks good and well organized.
Thanks for filling this test gap and especially for cleaning up the setUp in HBaseTestCase.
Regards,
Bogi
src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java
Lines 26 (patched)
<https://reviews.apache.org/r/61669/#comment260196>
Could you please remove the space from here between String and []?
- Boglarka Egyed
On Aug. 25, 2017, 10:06 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61669/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2017, 10:06 p.m.)
>
>
> Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
>
>
> Bugs: SQOOP-3222
> https://issues.apache.org/jira/browse/SQOOP-3222
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> In this patch I have changed the following:
> - Added test dependency on hadoop-minikdc.
> - Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
> - Added kerberos handling logic to HBaseTestCase and refactored it a bit.
> - Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
>
> The changes are inspired by the following HBase test classes:
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
>
>
> HBase security documentation:
> http://hbase.apache.org/1.2/book.html#security
>
>
> Diffs
> -----
>
> build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
> ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
> src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
> src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/61669/diff/4/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/
-----------------------------------------------------------
(Updated Aug. 25, 2017, 10:06 p.m.)
Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
Bugs: SQOOP-3222
https://issues.apache.org/jira/browse/SQOOP-3222
Repository: sqoop-trunk
Description
-------
In this patch I have changed the following:
- Added test dependency on hadoop-minikdc.
- Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
- Added kerberos handling logic to HBaseTestCase and refactored it a bit.
- Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
The changes are inspired by the following HBase test classes:
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
HBase security documentation:
http://hbase.apache.org/1.2/book.html#security
Diffs (updated)
-----
build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
Diff: https://reviews.apache.org/r/61669/diff/4/
Changes: https://reviews.apache.org/r/61669/diff/3-4/
Testing
-------
Ran unit tests and third party tests.
Thanks,
Szabolcs Vasas
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/
-----------------------------------------------------------
(Updated Aug. 25, 2017, 12:09 p.m.)
Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
Bugs: SQOOP-3222
https://issues.apache.org/jira/browse/SQOOP-3222
Repository: sqoop-trunk
Description
-------
In this patch I have changed the following:
- Added test dependency on hadoop-minikdc.
- Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
- Added kerberos handling logic to HBaseTestCase and refactored it a bit.
- Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
The changes are inspired by the following HBase test classes:
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
HBase security documentation:
http://hbase.apache.org/1.2/book.html#security
Diffs (updated)
-----
build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
Diff: https://reviews.apache.org/r/61669/diff/3/
Changes: https://reviews.apache.org/r/61669/diff/2-3/
Testing
-------
Ran unit tests and third party tests.
Thanks,
Szabolcs Vasas
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/
-----------------------------------------------------------
(Updated Aug. 21, 2017, 3:18 p.m.)
Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
Bugs: SQOOP-3222
https://issues.apache.org/jira/browse/SQOOP-3222
Repository: sqoop-trunk
Description
-------
In this patch I have changed the following:
- Added test dependency on hadoop-minikdc.
- Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
- Added kerberos handling logic to HBaseTestCase and refactored it a bit.
- Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
The changes are inspired by the following HBase test classes:
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
HBase security documentation:
http://hbase.apache.org/1.2/book.html#security
Diffs (updated)
-----
build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
Diff: https://reviews.apache.org/r/61669/diff/2/
Changes: https://reviews.apache.org/r/61669/diff/1-2/
Testing
-------
Ran unit tests and third party tests.
Thanks,
Szabolcs Vasas
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Zoltán Tóth <sw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/#review183203
-----------------------------------------------------------
src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java
Line 151 (original), 153 (patched)
<https://reviews.apache.org/r/61669/#comment259217>
I think if you would move the class variable at the beginning of the class then it would improve the readability.
- Zoltán Tóth
On Aug. 15, 2017, 6:56 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61669/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2017, 6:56 p.m.)
>
>
> Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
>
>
> Bugs: SQOOP-3222
> https://issues.apache.org/jira/browse/SQOOP-3222
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> In this patch I have changed the following:
> - Added test dependency on hadoop-minikdc.
> - Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
> - Added kerberos handling logic to HBaseTestCase and refactored it a bit.
> - Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
>
> The changes are inspired by the following HBase test classes:
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
>
>
> HBase security documentation:
> http://hbase.apache.org/1.2/book.html#security
>
>
> Diffs
> -----
>
> build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
> ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
> src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
> src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/61669/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 61669: Test HBase kerberized connectivity
Posted by Zoltán Tóth <sw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61669/#review183204
-----------------------------------------------------------
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java
Lines 46 (patched)
<https://reviews.apache.org/r/61669/#comment259220>
I think you can use Files.createFile(Path, ...) so you will use only NIO package. It also depends on your taste, so if you leave it like this it is also okay.
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java
Lines 61 (patched)
<https://reviews.apache.org/r/61669/#comment259218>
You used plural in the name of the method but the miniKdc.createPrincipal is singular. I think it would better if you would use singular instead.
src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java
Lines 71 (patched)
<https://reviews.apache.org/r/61669/#comment259219>
Maybe you can change it into one line or you can just put that into the createPrincipals method because you use the same try - catch block for any error.
It depends on your taste so feel free to choose your action.
- Zoltán Tóth
On Aug. 15, 2017, 6:56 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61669/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2017, 6:56 p.m.)
>
>
> Review request for Sqoop, Boglarka Egyed, Ferenc Szabo, and Zoltán Tóth.
>
>
> Bugs: SQOOP-3222
> https://issues.apache.org/jira/browse/SQOOP-3222
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> In this patch I have changed the following:
> - Added test dependency on hadoop-minikdc.
> - Added a JUnit rule which starts/stops kerberos MiniKdc before/after a test case/class.
> - Added kerberos handling logic to HBaseTestCase and refactored it a bit.
> - Removed the kerberos-related properties from the build.xml as they caused HBaseKerberizedConnectivityTest to fail.
>
> The changes are inspired by the following HBase test classes:
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/SecureTestCluster.java
> https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestGenerateDelegationToken.java
>
>
> HBase security documentation:
> http://hbase.apache.org/1.2/book.html#security
>
>
> Diffs
> -----
>
> build.xml 5f02dcf7759887d84d8cf0505cc1873c53f70a67
> ivy.xml e4b45bfd9ff6d984a1d1d1808855a07d8b090921
> src/test/com/cloudera/sqoop/hbase/HBaseKerberizedConnectivityTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java d9f74952e5f9dd9497e6e9e99789471bcd8f8930
> src/test/org/apache/sqoop/infrastructure/kerberos/KerberosConfigurationProvider.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructure.java PRE-CREATION
> src/test/org/apache/sqoop/infrastructure/kerberos/MiniKdcInfrastructureRule.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/61669/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>