Description
Attachments
Attachments
- HDDS-548-HDDS-4.006.patch
- 54 kB
- Anu Engineer
- HDDS-548-HDDS-4.005.patch
- 52 kB
- Anu Engineer
- HDDS-548-HDDS-4.004.patch
- 51 kB
- Anu Engineer
- HDDS-548-HDDS-4.003.patch
- 52 kB
- Anu Engineer
- HDDS-548-HDDS-4.002.patch
- 44 kB
- Anu Engineer
- HDDS-548-HDDS-4.001.patch
- 44 kB
- Anu Engineer
- HDDS-548.001.patch
- 44 kB
- Anu Engineer
Activity
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 24s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
|
|||
-1 | mvninstall | 27m 31s | root in |
+1 | compile | 0m 41s | |
+1 | checkstyle | 0m 26s | |
+1 | mvnsite | 0m 42s | |
+1 | shadedclient | 12m 44s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 30s | |
+1 | javadoc | 1m 4s | |
Patch Compile Tests | |||
+1 | mvninstall | 0m 47s | the patch passed |
+1 | compile | 0m 45s | the patch passed |
+1 | javac | 0m 45s | the patch passed |
-0 | checkstyle | 0m 18s | hadoop-hdds/common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
+1 | mvnsite | 0m 42s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 2s | The patch has no ill-formed XML file. |
+1 | shadedclient | 13m 5s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 32s | the patch passed |
-1 | javadoc | 1m 0s | hadoop-hdds_common generated 56 new + 0 unchanged - 0 fixed = 56 total (was 0) |
Other Tests | |||
+1 | unit | 2m 22s | common in the patch passed. |
+1 | asflicense | 0m 33s | The patch does not generate ASF License warnings. |
66m 31s |
Subsystem | Report/Notes |
---|---|
Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:4b8c2b1 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12941135/HDDS-548-HDDS-4.002.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
uname | Linux 7221869737ed 4.4.0-133-generic #159-Ubuntu SMP Fri Aug 10 07:31:43 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_181 |
mvninstall | https://builds.apache.org/job/PreCommit-HDDS-Build/1205/artifact/out/branch-mvninstall-root.txt |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDDS-Build/1205/artifact/out/diff-checkstyle-hadoop-hdds_common.txt |
javadoc | https://builds.apache.org/job/PreCommit-HDDS-Build/1205/artifact/out/diff-javadoc-javadoc-hadoop-hdds_common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDDS-Build/1205/testReport/ |
Max. process+thread count | 407 (vs. ulimit of 10000) |
modules | C: hadoop-hdds/common U: hadoop-hdds/common |
Console output | https://builds.apache.org/job/PreCommit-HDDS-Build/1205/console |
Powered by | Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Patch v3 fixes the javadoc issues and some other minor cleanup discovered while fixing the javadocs. I am not able to repro the mvn install failure. I tried the command used by jenkins.
mvn clean install -Phdds -DskipTests -fae -DskipTests=true -Dmaven.javadoc.skip=true -Dcheckstyle.skip=true -Dfindbugs.skip=true
It works well on my local machine, not sure why it got the conflict on BC packages. The issue seems to be that we bring in BC via Ratis too. We can exclude that dependency and use the version that we bring in via Ozone if needed.
anu, the mvn build issue is caused by the bc JAR conflict which failed hdds-common build and all the others on Jenkins. Let's fix that with Ajay's patch on HDDS-546.
anu, the mvn build issue is caused by the bc JAR conflict which failed hdds-common build and all the others on Jenkins. Let's fix that with Ajay's patch on
HDDS-546.
I saw that, but I am not able to repro that issue locally. You can apply this patch and try to do an mvn clean install -Phdds and see if it repros.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 33s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
|
|||
-1 | mvninstall | 20m 36s | root in |
+1 | compile | 0m 32s | |
+1 | checkstyle | 0m 17s | |
+1 | mvnsite | 0m 40s | |
+1 | shadedclient | 11m 57s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 1s | |
+1 | javadoc | 0m 54s | |
Patch Compile Tests | |||
+1 | mvninstall | 0m 39s | the patch passed |
+1 | compile | 0m 31s | the patch passed |
+1 | javac | 0m 31s | the patch passed |
-0 | checkstyle | 0m 14s | hadoop-hdds/common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
+1 | mvnsite | 0m 31s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 1s | The patch has no ill-formed XML file. |
+1 | shadedclient | 12m 14s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 12s | the patch passed |
+1 | javadoc | 0m 53s | the patch passed |
Other Tests | |||
+1 | unit | 1m 0s | common in the patch passed. |
+1 | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
54m 49s |
Subsystem | Report/Notes |
---|---|
Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:4b8c2b1 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12941255/HDDS-548-HDDS-4.003.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
uname | Linux 753a1af72b0e 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_181 |
mvninstall | https://builds.apache.org/job/PreCommit-HDDS-Build/1212/artifact/out/branch-mvninstall-root.txt |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDDS-Build/1212/artifact/out/diff-checkstyle-hadoop-hdds_common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDDS-Build/1212/testReport/ |
Max. process+thread count | 335 (vs. ulimit of 10000) |
modules | C: hadoop-hdds/common U: hadoop-hdds/common |
Console output | https://builds.apache.org/job/PreCommit-HDDS-Build/1212/console |
Powered by | Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 40s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 6 new or modified test files. |
|
|||
+1 | mvninstall | 28m 2s | |
+1 | compile | 0m 37s | |
+1 | checkstyle | 0m 20s | |
+1 | mvnsite | 0m 37s | |
+1 | shadedclient | 12m 21s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 5s | |
+1 | javadoc | 0m 58s | |
Patch Compile Tests | |||
+1 | mvninstall | 0m 34s | the patch passed |
+1 | compile | 0m 28s | the patch passed |
+1 | javac | 0m 28s | the patch passed |
-0 | checkstyle | 0m 13s | hadoop-hdds/common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
+1 | mvnsite | 0m 31s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | shadedclient | 12m 50s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 19s | the patch passed |
+1 | javadoc | 0m 56s | the patch passed |
Other Tests | |||
+1 | unit | 0m 57s | common in the patch passed. |
+1 | asflicense | 0m 28s | The patch does not generate ASF License warnings. |
63m 15s |
Subsystem | Report/Notes |
---|---|
Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:4b8c2b1 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12941292/HDDS-548-HDDS-4.004.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
uname | Linux 0396b788d663 3.13.0-153-generic #203-Ubuntu SMP Thu Jun 14 08:52:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_181 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDDS-Build/1218/artifact/out/diff-checkstyle-hadoop-hdds_common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDDS-Build/1218/testReport/ |
Max. process+thread count | 300 (vs. ulimit of 10000) |
modules | C: hadoop-hdds/common U: hadoop-hdds/common |
Console output | https://builds.apache.org/job/PreCommit-HDDS-Build/1218/console |
Powered by | Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
The one checkstyle issue is something that I prefer not to fix. That is warning for 8 arguments to a private constructor that is accessed via a builder. I think this patch is ready for review now.
Thanks anu for working on this. The patch looks good to me. Here are few comments:
We need to move HDDSKeyGenerator.java and HDDSKeyPEMWriter.java to under the o/a/h/h/security/x509/keys to make package path and dir path match?
SelfSignedCertificate.java
Line 60: NIT: do we support self-signed certificate for ozone in non CA?
Line 132: can we leverage the build-in X509v3CertificateBuilder/X500NameBuilder to build DN and self-signed certificate for SCM? This way, we can have a simpler builder or don't have to maintain our own builder class.
Line 213: we will need API to persist and load the self-signed certificate. Do you want to add that in a separate JIRA?
Thanks for the review. I have updated the patch v5. Please see more responses below.
We need to move HDDSKeyGenerator.java and HDDSKeyPEMWriter.java to
Good catch, fixed this issue.
Line 60: NIT: do we support self-signed certificate for ozone in non CA?
Probably not; but it is useful to have that ability if someone decides to create a SSL certificate via SCM. I am hoping that eventually we will be able to move these classes to Hadoop common; so handling the generic cases too.
Line 132: can we leverage the build-in X509v3CertificateBuilder/X500NameBuilder to build DN and self-signed certificate for SCM
I actually started with that, but then I found that testing becomes too complex. The RDN format needs to be encoded and matched. X500Name that we pass to X509v3CertificateBuilder, gets parsed into RDNs and gets encoded in exactly same format. With the current approach we are able construct the expected name from NAME_FORMAT. Hence this approach.
Line 213: we will need API to persist and load the self-signed certificate. Do you want to add that in a separate JIRA?
Yes, Absolutely that is the plan.
anu thanks for working on this important functionality. Patch looks good. Few questions and suggestions:
- SelfSignedCertificate#generateCertificate creates a self signed certificate. This should be good for SCM but for other components we need a certificate with different subject names. Since this root certificate is private to this class don't we need another api to handle this?
- CertficateException: Typo in classname and constructor.
- SecurityConfig
- L69: Rename privateKeyName to privateKeyFileName?
- L70: Rename publicKeyName to pubicKeyFileName?
- L128: rename api getPublicKeyFileName to getPublicKeyFileName?
- L138: rename getPrivateKeyName to getPrivateKeyFileName?
ajayydv Thanks for the review. Please see specific comments below.
Since this root certificate is private to this class don't we need another api to handle this?
Well, The root certificate is used to kick start the CA or a simple stand alone system, like a SSL node. We will be creating a CA class that can take a CSR and sign the certificates for the users. But that CA will need a self-signed or a Root CA certificate provided. This patch is first of many that will create such a system, hence the private signatures.
CertficateException: Typo in classname and constructor.
Good catch, Fixed.
L69: Rename privateKeyName to privateKeyFileName?
Done
L70: Rename publicKeyName to pubicKeyFileName?
Done
L128: rename api getPublicKeyFileName to getPublicKeyFileName?
Done
L138: rename getPrivateKeyName to getPrivateKeyFileName?
Done.
The patch v6 takes care of all these issues.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 13s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
|
|||
+1 | mvninstall | 22m 36s | |
+1 | compile | 0m 33s | |
+1 | checkstyle | 0m 18s | |
+1 | mvnsite | 0m 35s | |
+1 | shadedclient | 11m 53s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 7s | |
+1 | javadoc | 0m 58s | |
Patch Compile Tests | |||
+1 | mvninstall | 0m 41s | the patch passed |
+1 | compile | 0m 35s | the patch passed |
+1 | javac | 0m 35s | the patch passed |
-0 | checkstyle | 0m 15s | hadoop-hdds/common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
+1 | mvnsite | 0m 37s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | shadedclient | 13m 5s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 17s | the patch passed |
+1 | javadoc | 0m 56s | the patch passed |
Other Tests | |||
+1 | unit | 1m 2s | common in the patch passed. |
+1 | asflicense | 0m 26s | The patch does not generate ASF License warnings. |
57m 34s |
Subsystem | Report/Notes |
---|---|
Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:4b8c2b1 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12941572/HDDS-548-HDDS-4.006.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
uname | Linux 6f24e14fa649 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_181 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDDS-Build/1239/artifact/out/diff-checkstyle-hadoop-hdds_common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDDS-Build/1239/testReport/ |
Max. process+thread count | 304 (vs. ulimit of 10000) |
modules | C: hadoop-hdds/common U: hadoop-hdds/common |
Console output | https://builds.apache.org/job/PreCommit-HDDS-Build/1239/console |
Powered by | Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 33s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 8 new or modified test files. |
|
|||
+1 | mvninstall | 24m 55s | |
+1 | compile | 0m 28s | |
+1 | checkstyle | 0m 16s | |
+1 | mvnsite | 0m 32s | |
+1 | shadedclient | 10m 43s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 1s | |
+1 | javadoc | 0m 56s | |
Patch Compile Tests | |||
+1 | mvninstall | 0m 31s | the patch passed |
+1 | compile | 0m 25s | the patch passed |
+1 | javac | 0m 25s | the patch passed |
-0 | checkstyle | 0m 12s | hadoop-hdds/common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
+1 | mvnsite | 0m 28s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | shadedclient | 11m 11s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 1m 5s | the patch passed |
+1 | javadoc | 0m 48s | the patch passed |
Other Tests | |||
+1 | unit | 1m 4s | common in the patch passed. |
+1 | asflicense | 0m 24s | The patch does not generate ASF License warnings. |
55m 54s |
Subsystem | Report/Notes |
---|---|
Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:4b8c2b1 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12941572/HDDS-548-HDDS-4.006.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
uname | Linux caefbf456822 4.4.0-133-generic #159-Ubuntu SMP Fri Aug 10 07:31:43 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_181 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDDS-Build/1241/artifact/out/diff-checkstyle-hadoop-hdds_common.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDDS-Build/1241/testReport/ |
Max. process+thread count | 441 (vs. ulimit of 10000) |
modules | C: hadoop-hdds/common U: hadoop-hdds/common |
Console output | https://builds.apache.org/job/PreCommit-HDDS-Build/1241/console |
Powered by | Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #15791 (See https://builds.apache.org/job/Hadoop-trunk-Commit/15791/)
HDDS-548. Create a Self-Signed Certificate. Contributed by Anu Engineer. (xyao: rev 2d269440b0ac9a1cd3076b8b1b20493cdf0d80f3)
- (add) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/TestHDDSKeyGenerator.java
- (delete) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/TestHDDSKeyPEMWriter.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificates/package-info.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/exceptions/CertificateException.java
- (delete) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/HDDSKeyPEMWriter.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/HDDSKeyPEMWriter.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/package-info.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/exceptions/SCMSecurityException.java
- (add) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/package-info.java
- (delete) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/HDDSKeyGenerator.java
- (add) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificates/TestRootCertificate.java
- (edit) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsConfigKeys.java
- (edit) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/SecurityConfig.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificates/SelfSignedCertificate.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/exceptions/package-info.java
- (add) hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/keys/HDDSKeyGenerator.java
- (add) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/certificates/package-info.java
- (delete) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/TestHDDSKeyGenerator.java
- (add) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/package-info.java
- (add) hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/security/x509/keys/TestHDDSKeyPEMWriter.java
HDDS-548does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.HDDS-548This message was automatically generated.