Fix marvin test and revoke certificate

Author: nvazquez

engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDao.java

@@ -22,7 +22,6 @@
 import java.util.List;
 
 public interface DirectDownloadCertificateDao extends GenericDao<DirectDownloadCertificateVO, Long> {
-    DirectDownloadCertificateVO findByAlias(String alias);
-    List<DirectDownloadCertificateVO> listByHypervisorType(Hypervisor.HypervisorType hypervisorType);
+    DirectDownloadCertificateVO findByAlias(String alias, Hypervisor.HypervisorType hypervisorType, long zoneId);
     List<DirectDownloadCertificateVO> listByZone(long zoneId);
 }

engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateDaoImpl.java

@@ -36,17 +36,12 @@ public DirectDownloadCertificateDaoImpl() {
     }
 
     @Override
-    public DirectDownloadCertificateVO findByAlias(String alias) {
+    public DirectDownloadCertificateVO findByAlias(String alias, Hypervisor.HypervisorType hypervisorType, long zoneId) {
         SearchCriteria<DirectDownloadCertificateVO> sc = certificateSearchBuilder.create();
         sc.setParameters("alias", alias);
-        return findOneBy(sc);
-    }
-
-    @Override
-    public List<DirectDownloadCertificateVO> listByHypervisorType(Hypervisor.HypervisorType hypervisorType) {
-        SearchCriteria<DirectDownloadCertificateVO> sc = certificateSearchBuilder.create();
         sc.setParameters("hypervisor_type", hypervisorType);
-        return listBy(sc);
+        sc.setParameters("zone_id", zoneId);
+        return findOneBy(sc);
     }
 
     @Override

engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDao.java

@@ -18,6 +18,9 @@
 
 import com.cloud.utils.db.GenericDao;
 
+import java.util.List;
+
 public interface DirectDownloadCertificateHostMapDao extends GenericDao<DirectDownloadCertificateHostMapVO, Long> {
     DirectDownloadCertificateHostMapVO findByCertificateAndHost(long certificateId, long hostId);
+    List<DirectDownloadCertificateHostMapVO> listByCertificateId(long certificateId);
 }

engine/schema/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadCertificateHostMapDaoImpl.java

@@ -20,6 +20,8 @@
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 
+import java.util.List;
+
 public class DirectDownloadCertificateHostMapDaoImpl extends GenericDaoBase<DirectDownloadCertificateHostMapVO, Long> implements DirectDownloadCertificateHostMapDao {
     private final SearchBuilder<DirectDownloadCertificateHostMapVO> mapSearchBuilder;
 
@@ -36,4 +38,11 @@ public DirectDownloadCertificateHostMapVO findByCertificateAndHost(long certific
         sc.setParameters("host_id", hostId);
         return findOneBy(sc);
     }
+
+    @Override
+    public List<DirectDownloadCertificateHostMapVO> listByCertificateId(long certificateId) {
+        SearchCriteria<DirectDownloadCertificateHostMapVO> sc = mapSearchBuilder.create();
+        sc.setParameters("certificate_id", certificateId);
+        return listBy(sc);
+    }
 }

server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java

@@ -391,7 +391,7 @@ public boolean uploadCertificateToHosts(String certificateCer, String alias, Str
         String certificatePem = getPretifiedCertificate(certificateCer);
         certificateSanity(certificatePem);
 
-        DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findByAlias(alias);
+        DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findByAlias(alias, hypervisorType, zoneId);
         if (certificateVO != null) {
             throw new CloudRuntimeException("Certificate alias " + alias + " has been already created");
         }
@@ -453,15 +453,22 @@ public boolean uploadCertificate(long certificateId, long hostId) {
     @Override
     public boolean revokeCertificateAlias(String certificateAlias, String hypervisor, Long zoneId) {
         HypervisorType hypervisorType = HypervisorType.getType(hypervisor);
-        List<HostVO> hosts = getRunningHostsToUploadCertificate(zoneId, hypervisorType);
-        s_logger.info("Attempting to revoke certificate alias: " + certificateAlias + " from " + hosts.size() + " hosts");
-        if (CollectionUtils.isNotEmpty(hosts)) {
-            for (HostVO host : hosts) {
-                if (!revokeCertificateAliasFromHost(certificateAlias, host.getId())) {
-                    String msg = "Could not revoke certificate from host: " + host.getName() + " (" + host.getUuid() + ")";
+        DirectDownloadCertificateVO certificateVO = directDownloadCertificateDao.findByAlias(certificateAlias, hypervisorType, zoneId);
+        if (certificateVO == null) {
+            throw new CloudRuntimeException("Certificate alias " + certificateAlias + " does not exist");
+        }
+
+        List<DirectDownloadCertificateHostMapVO> maps = directDownloadCertificateHostMapDao.listByCertificateId(certificateVO.getId());
+        s_logger.info("Attempting to revoke certificate alias: " + certificateAlias + " from " + maps.size() + " hosts");
+        if (CollectionUtils.isNotEmpty(maps)) {
+            for (DirectDownloadCertificateHostMapVO map : maps) {
+                Long hostId = map.getHostId();
+                if (!revokeCertificateAliasFromHost(certificateAlias, hostId)) {
+                    String msg = "Could not revoke certificate from host: " + hostId;
                     s_logger.error(msg);
                     throw new CloudRuntimeException(msg);
                 }
+                directDownloadCertificateHostMapDao.remove(map.getId());
             }
         }
         return true;

test/integration/smoke/test_direct_download.py

@@ -92,6 +92,7 @@ def test_01_sanity_check_on_certificates(self):
         cmd.hypervisor = self.hypervisor
         cmd.name = "marvin-test-verify-certs"
         cmd.certificate = self.certificates["invalid"]
+        cmd.zoneid = self.zone.id
 
         invalid_cert_uploadFails = False
         expired_cert_upload_fails = False
@@ -126,6 +127,7 @@ def test_02_upload_direct_download_certificates(self):
         cmd.hypervisor = self.hypervisor
         cmd.name = "marvin-test-verify-certs"
         cmd.certificate = self.certificates["valid"]
+        cmd.zoneid = self.zone.id
 
         try:
             self.apiclient.uploadTemplateDirectDownloadCertificate(cmd)
@@ -135,9 +137,10 @@ def test_02_upload_direct_download_certificates(self):
         revokecmd = revokeTemplateDirectDownloadCertificate.revokeTemplateDirectDownloadCertificateCmd()
         revokecmd.hypervisor = self.hypervisor
         revokecmd.name = cmd.name
+        revokecmd.zoneid = self.zone.id
 
         try:
-            self.apiclient.revokeTemplateDirectDownloadCertificate(cmd)
+            self.apiclient.revokeTemplateDirectDownloadCertificate(revokecmd)
         except Exception as e:
             self.fail("Uploaded certificates should be revoked when needed")