fapifta commented on PR #3948: URL: https://github.com/apache/ozone/pull/3948#issuecomment-1314521457
Hi @xBis7, I don't think we can merge this change implemented this way... If you look at the usages of getCertificateLocation, and getKeyLocation in SecurityConfig, then you will find that beside the CA server code, the DefaultCertificateClient also uses it to determine where to store the certificate of an entity, and where to store the key material for the certificate. The implementation of these certificate client methods will have an effect for SCM, Ozone Manager, Ozone Recon, and Ozone DataNodes, as none of the component specific certificate client overrides these methods from the DefaultCertificateClient. The approach in the current code is bad, as it seems to rely on what config option is set in the configuration of which service, and it may cause problems, but this fix would make DataNodes to fail to find their correct location even if it is the only thing set in their config, at least as I understand the code looking at it quickly. Did I misinterpret the code and the change? Also it is an interesting philosophical question whether HDDS_METADATA_DIR or OZONE_METADATA_DIR should take precedence for OM and for Recon, as those are Ozone services, while which should take precedence for SCM and DN as those are HDDS services... :) -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
