rafaelweingartner commented on a change in pull request #2995: KVM:
Improvements on upload direct download certificates
URL: https://github.com/apache/cloudstack/pull/2995#discussion_r289654679
##########
File path:
server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java
##########
@@ -313,14 +322,64 @@ private DirectDownloadCommand
getDirectDownloadCommandFromProtocol(DownloadProto
.collect(Collectors.toList());
}
+ /**
+ * Return pretified PEM certificate
+ */
+ protected String getPretifiedCertificate(String certificateCer) {
+ String cert = certificateCer.replaceAll("(.{64})", "$1\n");
+ if (!cert.startsWith(BEGIN_CERT) && !cert.endsWith(END_CERT)) {
+ cert = BEGIN_CERT + LINE_SEPARATOR + cert + LINE_SEPARATOR +
END_CERT;
+ }
+ return cert;
+ }
+
+ /**
+ * Generate and return certificate from the string
+ * @throws CloudRuntimeException if the certificate is not well formed
+ */
+ private Certificate getCertificateFromString(String certificatePem) {
+ try {
+ return CertificateHelper.buildCertificate(certificatePem);
+ } catch (CertificateException e) {
+ e.printStackTrace();
+ throw new CloudRuntimeException("Cannot parse the certificate
provided, please provide a PEM certificate. Error: " + e.getMessage());
+ }
+ }
+
+ /**
+ * Perform sanity of string parsed certificate
+ */
+ protected void certificateSanity(String certificatePem) {
+ Certificate certificate = getCertificateFromString(certificatePem);
+
+ if (certificate instanceof X509CertImpl) {
+ X509CertImpl x509Cert = (X509CertImpl) certificate;
+ try {
+ x509Cert.checkValidity();
+ } catch (CertificateExpiredException |
CertificateNotYetValidException e) {
+ e.printStackTrace();
+ throw new CloudRuntimeException("Certificate is invalid.
Please provide a valid certificate. Error: " + e.getMessage());
+ }
+ if (x509Cert.getSubjectDN() != null) {
+ s_logger.info("Valid certificate for domain name: " +
x509Cert.getSubjectDN().getName());
+ }
+ }
+ }
+
@Override
- public boolean uploadCertificateToHosts(String certificateCer, String
certificateName, String hypervisor) {
+ public boolean uploadCertificateToHosts(String certificateCer, String
alias, String hypervisor) {
HypervisorType hypervisorType = HypervisorType.getType(hypervisor);
List<HostVO> hosts =
getRunningHostsToUploadCertificate(hypervisorType);
+
+ String certificatePem = getPretifiedCertificate(certificateCer);
+ certificateSanity(certificatePem);
+
+ s_logger.info("Attempting to upload certificate: " + alias + " to " +
hosts.size() + " hosts");
if (CollectionUtils.isNotEmpty(hosts)) {
for (HostVO host : hosts) {
- if (!uploadCertificate(certificateCer, certificateName,
host.getId())) {
- throw new CloudRuntimeException("Uploading certificate " +
certificateName + " failed on host: " + host.getId());
+ if (!uploadCertificate(certificatePem, alias, host.getId())) {
+ s_logger.error("Could not upload certificate on host: " +
host.getName() + " (" + host.getUuid() + ")");
+ throw new CloudRuntimeException("Uploading certificate " +
alias + " failed on host: " + host.getUuid());
Review comment:
What if you used the same message in the log "error" method, and in the
throw cloud runtime?
I think this message needs to contain, all information that are such as
hostname, host uuid, and certificate alias
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
