improve cleanup logic for DNS zone and server

Author: sudo87

api/src/main/java/org/apache/cloudstack/api/command/user/dns/AssociateDnsZoneToNetworkCmd.java

@@ -29,13 +29,13 @@
 import org.apache.cloudstack.api.response.DnsZoneNetworkMapResponse;
 import org.apache.cloudstack.api.response.DnsZoneResponse;
 import org.apache.cloudstack.api.response.NetworkResponse;
-import org.apache.cloudstack.dns.DnsZone;
 
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.InsufficientCapacityException;
 import com.cloud.exception.NetworkRuleConflictException;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.Network;
 import com.cloud.user.Account;
 
 @APICommand(name = "associateDnsZoneToNetwork",
@@ -73,9 +73,9 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
 
     @Override
     public long getEntityOwnerId() {
-        DnsZone zone = _entityMgr.findById(DnsZone.class, dnsZoneId);
-        if (zone != null) {
-            return zone.getAccountId();
+        Network network = _entityMgr.findById(Network.class, networkId);
+        if (network != null) {
+            return network.getAccountId();
         }
         return Account.ACCOUNT_ID_SYSTEM;
     }

engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDao.java

@@ -16,10 +16,14 @@
 // under the License.
 package com.cloud.vm.dao;
 
+import java.util.List;
+
 import org.apache.cloudstack.resourcedetail.ResourceDetailsDao;
 
 import com.cloud.utils.db.GenericDao;
 import com.cloud.vm.NicDetailVO;
 
 public interface NicDetailsDao extends GenericDao<NicDetailVO, Long>, ResourceDetailsDao<NicDetailVO> {
+
+    void removeDetailsForValuesIn(String resourceName, List<String> values);
 }

engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDaoImpl.java

@@ -17,17 +17,40 @@
 package com.cloud.vm.dao;
 
 
+import java.util.List;
+
+import org.apache.cloudstack.api.ApiConstants;
 import org.springframework.stereotype.Component;
 
 import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
 
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
 import com.cloud.vm.NicDetailVO;
 
 @Component
 public class NicDetailsDaoImpl extends ResourceDetailsDaoBase<NicDetailVO> implements NicDetailsDao {
+    private final SearchBuilder<NicDetailVO> NameValuesSearch;
+
+    public NicDetailsDaoImpl() {
+        super();
+        NameValuesSearch = createSearchBuilder();
+        NameValuesSearch.and(ApiConstants.NAME, NameValuesSearch.entity().getName(), SearchCriteria.Op.EQ);
+        NameValuesSearch.and(ApiConstants.VALUE, NameValuesSearch.entity().getValue(), SearchCriteria.Op.IN);
+        NameValuesSearch.done();
+    }
+
 
     @Override
     public void addDetail(long resourceId, String key, String value, boolean display) {
         super.addDetail(new NicDetailVO(resourceId, key, value, display));
     }
+
+    @Override
+    public void removeDetailsForValuesIn(String resourceName, List<String> values) {
+        SearchCriteria<NicDetailVO> sc = NameValuesSearch.create();
+        sc.setParameters(ApiConstants.NAME, resourceName);
+        sc.setParameters(ApiConstants.VALUE, values.toArray());
+        remove(sc);
+    }
 }

server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java

@@ -20,6 +20,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -59,6 +60,7 @@
 import org.apache.cloudstack.dns.vo.DnsZoneJoinVO;
 import org.apache.cloudstack.dns.vo.DnsZoneNetworkMapVO;
 import org.apache.cloudstack.dns.vo.DnsZoneVO;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.logging.log4j.util.Strings;
 import org.springframework.stereotype.Component;
 
@@ -280,10 +282,11 @@ public boolean deleteDnsServer(DeleteDnsServerCmd cmd) {
             if (cmd.getCleanup()) {
                 List<DnsZoneVO> dnsZones = dnsZoneDao.findDnsZonesByServerId(dnsServerId);
                 for (DnsZoneVO dnsZone : dnsZones) {
-                    long dnsZoneId = dnsZone.getId();
-                    dnsZoneNetworkMapDao.removeNetworkMappingByZoneId(dnsZoneId);
-                    // ToDo: delete nic_record_urls from vm_details if present before removing dnsZone
-                    dnsZoneDao.remove(dnsZoneId);
+                    try {
+                        deleteDnsZone(dnsZone.getId());
+                    } catch (Exception ex) {
+                        logger.error("Error cleaning up DNS zone: {} during DNS server: {} deletion", dnsZone.getName(), dnsServer.getName());
+                    }
                 }
             }
             return dnsServerDao.remove(dnsServerId);
@@ -304,22 +307,39 @@ public boolean deleteDnsZone(Long zoneId) {
         if (server == null) {
             throw new CloudRuntimeException(String.format("The DNS server not found for DNS zone: %s", dnsZoneName));
         }
-        try {
-            DnsProvider provider = getProviderByType(server.getProviderType());
-            provider.deleteZone(server, dnsZone);
-            logger.debug("Deleted DNS zone: {} from provider", dnsZoneName);
-        } catch (DnsNotFoundException ex) {
-            logger.warn("DNS zone: {} is not present in the provider, proceeding with cleanup", dnsZoneName);
-        } catch (Exception ex) {
-            logger.error("Failed to delete DNS zone from provider", ex);
-            throw new CloudRuntimeException(String.format("Failed to delete DNS zone: %s.", dnsZoneName));
-        }
 
         boolean dbResult = Transaction.execute((TransactionCallback<Boolean>) status -> {
-            dnsZoneNetworkMapDao.removeNetworkMappingByZoneId(zoneId);
-            // ToDo: delete nic_record_urls from vm_details if present before removing dnsZone
+            DnsZoneNetworkMapVO networkMapVO = dnsZoneNetworkMapDao.findByZoneId(zoneId);
+            if (networkMapVO != null) {
+                // Remove DNS records from nic_details if there are any
+                try {
+                    DnsProvider provider = getProviderByType(server.getProviderType());
+                    List<DnsRecord> records = provider.listRecords(server, dnsZone);
+                    if (CollectionUtils.isNotEmpty(records)) {
+                        List<String> dnsRecordNames = records.stream().map(DnsRecord::getName).filter(Objects::nonNull)
+                                .map(name -> name.replaceAll("\\.+$", ""))
+                                .collect(Collectors.toList());
+                        nicDetailsDao.removeDetailsForValuesIn(ApiConstants.NIC_DNS_RECORD, dnsRecordNames);
+                    }
+                } catch (Exception ex) {
+                    logger.warn("Failed to fetch DNS records for dnsZone: {}, perform manual cleanup.", dnsZoneName, ex);
+                }
+                // Remove DNS zone from provider and cleanup DB
+                try {
+                    DnsProvider provider = getProviderByType(server.getProviderType());
+                    provider.deleteZone(server, dnsZone);
+                    logger.debug("Deleted DNS zone: {} from provider", dnsZoneName);
+                } catch (DnsNotFoundException ex) {
+                    logger.warn("DNS zone: {} is not present in the provider, proceeding with cleanup", dnsZoneName);
+                } catch (Exception ex) {
+                    logger.error("Failed to delete DNS zone from provider", ex);
+                    throw new CloudRuntimeException(String.format("Failed to delete DNS zone: %s.", dnsZoneName));
+                }
+                dnsZoneNetworkMapDao.removeNetworkMappingByZoneId(zoneId);
+            }
             return dnsZoneDao.remove(zoneId);
         });
+
         if (!dbResult) {
             logger.error("Failed to remove DNS zone {} from DB after provider deletion", dnsZoneName);
         }
@@ -612,7 +632,10 @@ public DnsZoneNetworkMapResponse associateZoneToNetwork(AssociateDnsZoneToNetwor
             throw new InvalidParameterValueException("DNS zone not found.");
         }
         accountMgr.checkAccess(caller, null, true, dnsZone);
-
+        DnsServerVO dnsServer = dnsServerDao.findById(dnsZone.getDnsServerId());
+        if (dnsServer == null) {
+            throw new CloudRuntimeException("The underlying DNS server for this DNS zone is missing.");
+        }
         NetworkVO network = networkDao.findById(cmd.getNetworkId());
         if (network == null) {
             throw new InvalidParameterValueException("Network not found.");
@@ -621,11 +644,11 @@ public DnsZoneNetworkMapResponse associateZoneToNetwork(AssociateDnsZoneToNetwor
             throw new CloudRuntimeException(String.format("Operation is not permitted for network type: %s", network.getGuestType()));
         }
         accountMgr.checkAccess(caller, null, true, network);
-
         DnsZoneNetworkMapVO existing = dnsZoneNetworkMapDao.findByNetworkId(network.getId());
         if (existing != null) {
             throw new InvalidParameterValueException("Network has existing DNS zone associated to it.");
         }
+
         DnsZoneNetworkMapVO mapping = new DnsZoneNetworkMapVO(dnsZone.getId(), network.getId(), cmd.getSubDomain());
         dnsZoneNetworkMapDao.persist(mapping);
         DnsZoneNetworkMapResponse response = new DnsZoneNetworkMapResponse();

server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDao.java

@@ -33,7 +33,5 @@ public interface DnsServerDao extends GenericDao<DnsServerVO, Long> {
 
     List<Long> listDnsServerIdsByAccountId(Long accountId);
 
-    Pair<List<DnsServerVO>, Integer> searchDnsServers(Long id, String keyword, String provider, Long accountId, Filter filter);
-
     Pair<List<DnsServerVO>, Integer> searchDnsServer(Long dnsServerId, Long accountId, Set<Long> domainIds, DnsProviderType providerType, String keyword, Filter filter);
 }

server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDaoImpl.java

@@ -82,24 +82,6 @@ public List<Long> listDnsServerIdsByAccountId(Long accountId) {
         return customSearch(sc, null);
     }
 
-    @Override
-    public Pair<List<DnsServerVO>, Integer> searchDnsServers(Long id, String keyword, String provider, Long accountId, Filter filter) {
-        SearchCriteria<DnsServerVO> sc = AllFieldsSearch.create();
-        if (id != null) {
-            sc.setParameters(ApiConstants.ID, id);
-        }
-        if (keyword != null) {
-            sc.setParameters(ApiConstants.NAME, "%" + keyword + "%");
-        }
-        if (provider != null) {
-            sc.setParameters(ApiConstants.PROVIDER_TYPE, provider);
-        }
-        if (accountId != null) {
-            sc.setParameters(ApiConstants.ACCOUNT_ID, accountId);
-        }
-        return searchAndCount(sc, filter);
-    }
-
     @Override
     public Pair<List<DnsServerVO>, Integer> searchDnsServer(Long dnsServerId, Long accountId, Set<Long> domainIds, DnsProviderType providerType,
                                                             String keyword, Filter filter) {

server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java

@@ -41,7 +41,7 @@ public DnsZoneDaoImpl() {
         super();
 
         DnsServerSearch  = createSearchBuilder();
-        DnsServerSearch.selectFields(DnsServerSearch.entity().getDnsServerId());
+        DnsServerSearch.selectFields(DnsServerSearch.entity().getId());
         DnsServerSearch.and(ApiConstants.DNS_SERVER_ID, DnsServerSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ);
         DnsServerSearch.done();
 

server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneNetworkMapDao.java

@@ -24,4 +24,6 @@
 public interface DnsZoneNetworkMapDao extends GenericDao<DnsZoneNetworkMapVO, Long> {
     void removeNetworkMappingByZoneId(long dnsZoneId);
     DnsZoneNetworkMapVO findByNetworkId(long networkId);
+
+    DnsZoneNetworkMapVO findByZoneId(long networkId);
 }

server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneNetworkMapDaoImpl.java

@@ -54,4 +54,11 @@ public DnsZoneNetworkMapVO findByNetworkId(long networkId) {
         sc.setParameters(ApiConstants.NETWORK_ID, networkId);
         return findOneBy(sc);
     }
+
+    @Override
+    public DnsZoneNetworkMapVO findByZoneId(long dnsZoneId) {
+        SearchCriteria<DnsZoneNetworkMapVO> sc = ZoneNetworkSearch.create();
+        sc.setParameters(ApiConstants.DNS_ZONE_ID, dnsZoneId);
+        return findOneBy(sc);
+    }
 }

ui/src/config/section/network.js

@@ -1528,7 +1528,7 @@ export default {
     {
       name: 'dnsserver',
       title: 'label.dns.server',
-      icon: 'global-outlined',
+      icon: 'cloud-server-outlined',
       permission: ['listDnsServers'],
       columns: ['name', 'url', 'provider', 'ispublic', 'port', 'nameservers', 'publicdomainsuffix'],
       details: ['name', 'url', 'provider', 'ispublic', 'port', 'nameservers', 'publicdomainsuffix', 'domain', 'account'],
@@ -1555,6 +1555,7 @@ export default {
           label: 'label.dns.update.server',
           dataView: true,
           popup: true,
+          show: (record, store) => { return record.account === store.userInfo.account || isAdminOrDomainAdmin(store.userInfo.roletype) },
           component: shallowRef(defineAsyncComponent(() => import('@/views/network/dns/UpdateDnsServer.vue')))
         },
         {
@@ -1564,6 +1565,7 @@ export default {
           message: 'message.action.delete.dns.server',
           dataView: true,
           popup: true,
+          show: (record, store) => { return record.account === store.userInfo.account || isAdminOrDomainAdmin(store.userInfo.roletype) },
           groupAction: false,
           groupMap: (selection) => { return selection.map(x => { return { id: x } }) }
         }
@@ -1572,7 +1574,7 @@ export default {
     {
       name: 'dnszone',
       title: 'label.dns.zones',
-      icon: 'global-outlined',
+      icon: 'apartment-outlined',
       permission: ['listDnsZones'],
       columns: ['name', 'state', 'dnsservername', 'account', 'description'],
       details: ['name', 'id', 'state', 'dnsservername', 'dnsserverid', 'account', 'domainpath', 'description'],
@@ -1603,8 +1605,8 @@ export default {
           label: 'label.dns.update.zone',
           dataView: true,
           popup: true,
-          component: shallowRef(defineAsyncComponent(() => import('@/views/network/dns/UpdateDnsZone.vue'))),
-          show: (record) => { return true }
+          show: (record, store) => { return record.account === store.userInfo.account || isAdminOrDomainAdmin(store.userInfo.roletype) },
+          component: shallowRef(defineAsyncComponent(() => import('@/views/network/dns/UpdateDnsZone.vue')))
         },
         {
           api: 'deleteDnsZone',
@@ -1614,8 +1616,7 @@ export default {
           dataView: true,
           popup: true,
           groupAction: false,
-          disabled: (record) => false,
-          show: (record) => { return true },
+          show: (record, store) => { return record.account === store.userInfo.account || isAdminOrDomainAdmin(store.userInfo.roletype) },
           groupMap: (selection) => { return selection.map(x => { return { id: x } }) }
         }
       ]