-
Notifications
You must be signed in to change notification settings - Fork 594
HDDS-4915. [SCM HA Security] Integrate CertClient. #2000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
PR based on #1988 |
6a8c957 to
f9c35cb
Compare
aafeead to
b4b9e8c
Compare
xiaoyuyao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bharatviswa504 thanks for working on this. Can you separate the patch into smaller ones? For example
separate the bootstrap of the non-primary from this patch and focus only on the init of primary SCM.
Otherwise, the review will take longer time.
...tainer-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java
Outdated
Show resolved
Hide resolved
|
Please hold on the merge util we finish merging master back to 2823. Thanks! |
@xiaoyuyao I understand it is a big change, sorry for the trouble. Bootstrap changes will be less than 100 LOC, but if it is still needed I can split it into a new Jira. Actually, I have planned to implement CertClient and Integration in two parts, later when integrating found issues and closed the other one. Let me see if I can split again, but 2nd PR integration one will be dependent on this. Can we also have a call for a review? |
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/HASecurityUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
Show resolved
Hide resolved
|
@xiaoyuyao Addressed/replied review comments these are addressed in HDDS-4897 under PR #2041 I will update this PR to use all the Jira's which went in and integrate it. |
8af70d3 to
40caf37
Compare
|
Re-opened pull request based on HDDS-4897. |
40caf37 to
f222d57
Compare
|
Rebased with latest HDDS-2823. |
GlenGeng-awx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just some general comments, most of them are typo.
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientRatis.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java
Outdated
Show resolved
Hide resolved
...tainer-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java
Outdated
Show resolved
Hide resolved
| this.scmVersion = RPC.getProtocolVersion(ScmBlockLocationProtocolPB.class); | ||
|
|
||
| try { | ||
| this.ugi = UserGroupInformation.getCurrentUser(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need change this file ? The logic is unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to make use of UGI during the creation of FailOverProxyProvider, otherwise, we will use UGI during create proxy time which might not be correct one.
Without this change, we shall see this error.
om1_1 | 2021-03-23 05:59:25,420 [IPC Server handler 7 on default port 9862] WARN ipc.Client: Exception encountered while connecting to the server
om1_1 | javax.security.sasl.SaslException: GSS initiate failed [Caused by GSSException: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)]
om1_1 | at jdk.security.jgss/com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:211)
om1_1 | at org.apache.hadoop.security.SaslRpcClient.saslConnect(SaslRpcClient.java:408)
om1_1 | at org.apache.hadoop.ipc.Client$Connection.setupSaslConnection(Client.java:622)
om1_1 | at org.apache.hadoop.ipc.Client$Connection.access$2300(Client.java:413)
om1_1 | at org.apache.hadoop.ipc.Client$Connection$2.run(Client.java:822)
om1_1 | at org.apache.hadoop.ipc.Client$Connection$2.run(Client.java:818)
om1_1 | at java.base/java.security.AccessController.doPrivileged(Native Method)
om1_1 | at java.base/javax.security.auth.Subject.doAs(Subject.java:423)
om1_1 | at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1762)
om1_1 | at org.apache.hadoop.ipc.Client$Connection.setupIOstreams(Client.java:818)
om1_1 | at org.apache.hadoop.ipc.Client$Connection.access$3800(Client.java:413)
om1_1 | at org.apache.hadoop.ipc.Client.getConnection(Client.java:1636)
om1_1 | at org.apache.hadoop.ipc.Client.call(Client.java:1452)
om1_1 | at org.apache.hadoop.ipc.Client.call(Client.java:1405)
om1_1 | at org.apache.hadoop.ipc.ProtobufRpcEngine$Invoker.invoke(ProtobufRpcEngine.java:233)
om1_1 | at org.apache.hadoop.ipc.ProtobufRpcEngine$Invoker.invoke(ProtobufRpcEngine.java:118)
om1_1 | at com.sun.proxy.$Proxy32.send(Unknown Source)
om1_1 | at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
om1_1 | at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
om1_1 | at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
om1_1 | at java.base/java.lang.reflect.Method.invoke(Method.java:566)
om1_1 | at org.apache.hadoop.io.retry.RetryInvocationHandler.invokeMethod(RetryInvocationHandler.java:422)
om1_1 | at org.apache.hadoop.io.retry.RetryInvocationHandler$Call.invokeMethod(RetryInvocationHandler.java:165)
om1_1 | at org.apache.hadoop.io.retry.RetryInvocationHandler$Call.invoke(RetryInvocationHandler.java:157)
om1_1 | at org.apache.hadoop.io.retry.RetryInvocationHandler$Call.invokeOnce(RetryInvocationHandler.java:95)
om1_1 | at org.apache.hadoop.io.retry.RetryInvocationHandler.invoke(RetryInvocationHandler.java:359)
om1_1 | at com.sun.proxy.$Proxy32.send(Unknown Source)
om1_1 | at org.apache.hadoop.hdds.scm.protocolPB.ScmBlockLocationProtocolClientSideTranslatorPB.submitRequest(ScmBlockLocationProtocolClientSideTranslatorPB.java:118)
om1_1 | at org.apache.hadoop.hdds.scm.protocolPB.ScmBlockLocationProtocolClientSideTranslatorPB.allocateBlock(ScmBlockLocationProtocolClientSideTranslatorPB.java:172)
om1_1 | at org.apache.hadoop.ozone.om.request.key.OMKeyRequest.allocateBlock(OMKeyRequest.java:128)
om1_1 | at org.apache.hadoop.ozone.om.request.key.OMKeyCreateRequest.preExecute(OMKeyCreateRequest.java:151)
om1_1 | at org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB.processRequest(OzoneManagerProtocolServerSideTranslatorPB.java:139)
om1_1 | at org.apache.hadoop.hdds.server.OzoneProtocolMessageDispatcher.processRequest(OzoneProtocolMessageDispatcher.java:87)
om1_1 | at org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB.submitRequest(OzoneManagerProtocolServerSideTranslatorPB.java:122)
om1_1 | at org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos$OzoneManagerService$2.callBlockingMethod(OzoneManagerProtocolProtos.java)
om1_1 | at org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:528)
om1_1 | at org.apache.hadoop.ipc.RPC$Server.call(RPC.java:1086)
om1_1 | at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:1029)
om1_1 | at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:957)
om1_1 | at java.base/java.security.AccessController.doPrivileged(Native Method)
om1_1 | at java.base/javax.security.auth.Subject.doAs(Subject.java:423)
om1_1 | at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1762)
om1_1 | at org.apache.hadoop.ipc.Server$Handler.run(Server.java:2957)
om1_1 | Caused by: GSSException: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)
om1_1 | at java.security.jgss/sun.security.jgss.krb5.Krb5InitCredential.getInstance(Krb5InitCredential.java:162)
om1_1 | at java.security.jgss/sun.security.jgss.krb5.Krb5MechFactory.getCredentialElement(Krb5MechFactory.java:126)
om1_1 | at java.security.jgss/sun.security.jgss.krb5.Krb5MechFactory.getMechanismContext(Krb5MechFactory.java:193)
om1_1 | at java.security.jgss/sun.security.jgss.GSSManagerImpl.getMechanismContext(GSSManagerImpl.java:218)
om1_1 | at java.security.jgss/sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:230)
om1_1 | at java.security.jgss/sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:196)
om1_1 | at jdk.security.jgss/com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:192)
om1_1 | ... 42 more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you enable the Debug log of UGI class to see which UGI is used if we don't cache the UGI at the time of provider creation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification!
...rc/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
Show resolved
Hide resolved
...n/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java
Outdated
Show resolved
Hide resolved
...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
Show resolved
Hide resolved
70f3f06 to
8b2ff29
Compare
...ds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we wrap line 623 and 641 into separate helper, this seems to be a long function already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log message can be improved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, Let me know if that looks okay to you?
xiaoyuyao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bharatviswa504 for the update. A few more comments. Let's discuss offline tomorrow.
21acf72 to
76d6a86
Compare
|
Thank You @xiaoyuyao for the review. I have addressed review comments.
Sure. |
xiaoyuyao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bharatviswa504 for the update. The latest change LGTM, +1 pending CI.
|
Thank You @xiaoyuyao and @GlenGeng for the review. |
* HDDS-3698-nonrolling-upgrade: (144 commits) fix project name in NOTICE.txt (apache#2112) HDDS-5066. Use fixed vesion from pnpm to build recon (apache#2115) HDDS-5014. Add non-rolling upgrade design docs. HDDS-5035. Use default config values to solve generated config file conflict (apache#2087) HDDS-5032. DN stopped to load containers on volume after a container load exception. (apache#2109) HDDS-4504. Datanode deletion config should be based on number of blocks (apache#1885) Fix ozone-ha acceptance test. HDDS-5058. Make getScmInfo retry for a duration. HDDS-4506. Support query parameter based v4 auth in S3g (apache#1628) HDDS-4553. ChunkInputStream should release buffer as soon as last byte in the buffer is read (apache#2062) HDDS-5022. SCM get roles command should provide Ratis Leader/Follower… (apache#2098) HDDS-5033. SCM may not be able to know full port list of Datanode after Datanode is started. (apache#2090) HDDS-3752. Fix o3fs list bucket contents issue when without tailing "/" (apache#2088) HDDS-4901. Remove OmOzoneAclMap from OmVolumeArgs to avoid OzoneAcl conversions (apache#1992) HDDS-4987. Import container should not delete container contents if container already exists (apache#2077) Checkstyle fix. Intialize DN layout version before security init. HDDS-4915. [SCM HA Security] Integrate CertClient. (apache#2000) HDDS-5049. Add timeout support for ratis requests in SCM HA. (apache#2099) trigger new CI check ...
What changes were proposed in this pull request?
This PR is to implement
If freshly installed cluster where init is performed, start rootCA and subCA. (Not implemented changes required upgrade from the old cluster to the new cluster and Ratis is enabled after upgrade)
a. Add API to store rootCA Certificate.
b. Expose API's to getRootCA Certificate and updateCAList which fetches list of CA and update it with in-memory.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4915
How was this patch tested?
Added docker test