HDDS-1946. CertificateClient should not persist keys/certs to ozone.m… #1311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/label ozone |
|
@anuengineer @avijayanhwx @swagle @xiaoyuyao Please review when you find time |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
...c/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DNCertificateClient.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/OMCertificateClient.java
Outdated
Show resolved
Hide resolved
|
@bharatviswa504 Thanks for the review! I have updated the patch to include component name in the key location as well. |
|
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Over all LGTM.
Can we remove the KeyCodec constructor with only configs? I see this is being used only in tests. With this, we can eliminate the chances of not passing the component name to codec and face the issue provided in Jira. If you feel there is a need for this, we can leave it for now.
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @vivekratnavel for working on this. The changes look good to me overall with two issues:
|
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
The test failure in TestSecureContainerServer seems related. Can you take a look @vivekratnavel |
|
Hi @xiaoyuyao I checked the cause of failure for this integration test
|
|
Created https://issues.apache.org/jira/browse/HDDS-2040 to track the integration test failure. |
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Aug 27, 2019
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
Show resolved
Hide resolved
xiaoyuyao
reviewed
Aug 27, 2019
amahussein
pushed a commit
to amahussein/hadoop
that referenced
this pull request
Oct 29, 2019
RogPodge
pushed a commit
to RogPodge/hadoop
that referenced
this pull request
Mar 25, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…etadata.dir
The issue was that when OM and SCM are deployed on the same host with ozone.metadata.dir defined. SCM can start successfully but OM can not because the key/cert from OM will collide with SCM.
The solution implemented in this patch is to store certs in a sub directory inside ozone.metadata.dir based on the component. Ozone Manager will store its certs in
${ozone.metadata.dir}/om/certsand Datanode will store in${ozone.metadata.dir}/dn/certsto avoid conflicts. This solution was discussed with @anuengineer and I thank him for his guidance.Testing done:
I tested the patch in docker containers and verified that certs are now stored in
${ozone.metadata.dir}/${component}/certspath. I modified the unit tests and verified that all unit tests pass.