GlenGeng commented on a change in pull request #2000:
URL: https://github.com/apache/ozone/pull/2000#discussion_r599250302
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
##########
@@ -294,7 +294,7 @@ public static void
createRaftServerProperties(ConfigurationSource ozoneConf,
// For External gRPC client to server with gRPC TLS.
// No mTLS for external client as SCM CA does not issued certificates for
them
public static GrpcTlsConfig createTlsClientConfig(SecurityConfig conf,
- X509Certificate caCert) {
+ List<X509Certificate> caCert) {
Review comment:
caCerts
##########
File path:
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -96,7 +96,7 @@
* @param caCert - SCM ca certificate.
*/
public XceiverClientGrpc(Pipeline pipeline, ConfigurationSource config,
- X509Certificate caCert) {
+ List<X509Certificate> caCert) {
Review comment:
typo `caCerts`
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
##########
@@ -173,4 +173,15 @@ public static OzoneConfiguration removeSelfId(
}
return conf;
}
+
+ /**
+ * Get SCM Node Id list.
+ * @param configuration
+ * @return list of node ids.
+ */
+ public static Collection<String> getSCMNodeIds(
+ ConfigurationSource configuration) {
+ String scmServiceId = SCMHAUtils.getScmServiceId(configuration);
Review comment:
no need `SCMHAUtils.` prefix for `getScmServiceId` and `getSCMNodeIds`
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
##########
@@ -70,9 +70,18 @@
private final int maxRetryCount;
private final long retryInterval;
+ private final UserGroupInformation ugi;
+
public SCMContainerLocationFailoverProxyProvider(ConfigurationSource conf) {
this.conf = conf;
+
+ try {
+ this.ugi = UserGroupInformation.getCurrentUser();
Review comment:
same as block client.
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java
##########
@@ -818,29 +830,64 @@ public Logger getLogger() {
return logger;
}
- /**
- * Create a scm security client, used to get SCM signed certificate.
- *
- * @return {@link SCMSecurityProtocol}
- */
- private static SCMSecurityProtocol getScmSecurityClient(
- OzoneConfiguration conf) throws IOException {
- RPC.setProtocolEngine(conf, SCMSecurityProtocolPB.class,
- ProtobufRpcEngine.class);
- long scmVersion =
- RPC.getProtocolVersion(ScmBlockLocationProtocolPB.class);
- InetSocketAddress scmSecurityProtoAdd =
- HddsServerUtil.getScmAddressForSecurityProtocol(conf);
- SCMSecurityProtocolClientSideTranslatorPB scmSecurityClient =
- new SCMSecurityProtocolClientSideTranslatorPB(
- RPC.getProxy(SCMSecurityProtocolPB.class, scmVersion,
- scmSecurityProtoAdd, UserGroupInformation.getCurrentUser(),
- conf, NetUtils.getDefaultSocketFactory(conf),
- Client.getRpcTimeout(conf)));
- return scmSecurityClient;
+ public String getComponentName() {
+ return null;
}
- public String getComponentName() {
+ @Override
+ public X509Certificate getRootCACertificate() {
+ if (rootCaCertId != null) {
+ return certificateMap.get(rootCaCertId);
+ }
return null;
}
+
+ @Override
+ public void storeRootCACertificate(String pemEncodedCert, boolean force)
+ throws CertificateException {
+ CertificateCodec certificateCodec = new CertificateCodec(securityConfig,
+ component);
+ try {
+ Path basePath = securityConfig.getCertificateLocation(component);
+
+ X509Certificate cert =
+ CertificateCodec.getX509Certificate(pemEncodedCert);
+ String certName = String.format(CERT_FILE_NAME_FORMAT,
+ cert.getSerialNumber().toString());
+
+ certName = ROOT_CA_CERT_PREFIX + certName;
+ rootCaCertId = cert.getSerialNumber().toString();
+
+ certificateCodec.writeCertificate(basePath, certName,
+ pemEncodedCert, force);
+ certificateMap.putIfAbsent(cert.getSerialNumber().toString(), cert);
+ } catch (IOException | java.security.cert.CertificateException e) {
+ throw new CertificateException("Error while storing Root CA " +
+ "certificate.", e, CERTIFICATE_ERROR);
+ }
+ }
+
+ @Override
+ public List<String> listCA() throws IOException {
+ if (pemEncodedCACerts == null) {
+ updateCAList();
Review comment:
Will there be contention of the client ? Since there is no lock
protection here, need make sure the modifications are properly serialized.
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java
##########
@@ -246,4 +247,20 @@ public String getScmSecurityAddress() {
public String getScmDatanodeAddress() {
return scmDatanodeAddress;
}
+
+ public static SCMNodeInfo getScmNodeInfo(ConfigurationSource conf,
+ String nodeId) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Preconditions.checkState(scmNodeInfoList.size() >=1);
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getNodeId().equals(nodeId)) {
+ return scmNodeInfo;
+ }
+ }
Review comment:
remove line 264, return null here, and print a warn ?
##########
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java
##########
@@ -70,11 +70,20 @@
private final int maxRetryCount;
private final long retryInterval;
+ private final UserGroupInformation ugi;
+
public SCMBlockLocationFailoverProxyProvider(ConfigurationSource conf) {
this.conf = conf;
this.scmVersion = RPC.getProtocolVersion(ScmBlockLocationProtocolPB.class);
+ try {
+ this.ugi = UserGroupInformation.getCurrentUser();
Review comment:
why need change this file ? The logic is unchanged.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java
##########
@@ -167,8 +171,25 @@ public OzoneContainer(
blockDeletingService =
new BlockDeletingService(this, svcInterval.toMillis(), serviceTimeout,
TimeUnit.MILLISECONDS, config);
- tlsClientConfig = RatisHelper.createTlsClientConfig(
- secConf, certClient != null ? certClient.getCACertificate() : null);
+
+ List< X509Certificate > x509Certificates;
Review comment:
```
List< X509Certificate > x509Certificates = null;
if (certClient != null) {
///
}
```
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeInfo.java
##########
@@ -246,4 +247,20 @@ public String getScmSecurityAddress() {
public String getScmDatanodeAddress() {
return scmDatanodeAddress;
}
+
+ public static SCMNodeInfo getScmNodeInfo(ConfigurationSource conf,
Review comment:
This function seems not used.
##########
File path:
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientRatis.java
##########
@@ -81,7 +81,7 @@ public static XceiverClientRatis newXceiverClientRatis(
public static XceiverClientRatis newXceiverClientRatis(
org.apache.hadoop.hdds.scm.pipeline.Pipeline pipeline,
- ConfigurationSource ozoneConf, X509Certificate caCert) {
+ ConfigurationSource ozoneConf, List<X509Certificate> caCert) {
Review comment:
caCerts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
