Skip to content

Commit 8f71ce6

Browse files
sudo87sudo87
committed
introduce 'unmanage' param in deleteDnsServerCmd to prevent deletion of zones from dns provider
1 parent 9391ea3 commit 8f71ce6

12 files changed

Lines changed: 124 additions & 55 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public class CreateDnsZoneCmd extends BaseAsyncCreateCmd {
6868

6969
@Parameter(name = ApiConstants.EXISTING, type = CommandType.BOOLEAN, entityType = DnsZoneResponse.class,
7070
description = "If true, imports an existing DNS zone from the DNS provider into CloudStack. " +
71-
"If false, creates the zone in the DNS provider and registers it in CloudStack.")
71+
"If false, creates the zone in the DNS provider and registers it in CloudStack. Default is false")
7272
private Boolean existing = false;
7373

7474
/////////////////////////////////////////////////////

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,15 @@ public class DeleteDnsServerCmd extends BaseAsyncCmd {
5353
private Long id;
5454

5555
@Parameter(name = ApiConstants.CLEANUP, type = CommandType.BOOLEAN,
56-
entityType = DnsZoneResponse.class, description = "True if all associated DNS zones have to be cleaned up with this server")
56+
entityType = DnsZoneResponse.class, description = "If true, all associated DNS zones will be cleaned up " +
57+
"when the server is removed. Default: true")
5758
private Boolean cleanup = true;
5859

60+
@Parameter(name = ApiConstants.UNMANAGE, type = CommandType.BOOLEAN, entityType = DnsZoneResponse.class,
61+
description = "If true, the DNS zone is only removed from CloudStack (unmanaged); if false, it is removed " +
62+
"from both CloudStack and the DNS provider. Default: false")
63+
private Boolean unmanage = false;
64+
5965
/////////////////////////////////////////////////////
6066
/////////////////// Accessors ///////////////////////
6167
/////////////////////////////////////////////////////
@@ -102,4 +108,8 @@ public long getEntityOwnerId() {
102108
public Boolean getCleanup() {
103109
return Boolean.TRUE.equals(cleanup);
104110
}
111+
112+
public Boolean isUnmanage() {
113+
return Boolean.TRUE.equals(unmanage);
114+
}
105115
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ public class DeleteDnsZoneCmd extends BaseAsyncCmd {
5252
private Long id;
5353

5454
@Parameter(name = ApiConstants.UNMANAGE, type = CommandType.BOOLEAN, entityType = DnsZoneResponse.class,
55-
description = "If true, removes the DNS zone from CloudStack; if false, " +
56-
"removes it from both CloudStack and the DNS provider.")
55+
description = "If true, imports an existing DNS zone from the DNS provider into CloudStack; " +
56+
"if false, creates the DNS zone in the provider and registers it with CloudStack. Default: false")
5757
private Boolean unmanage = false;
5858

5959
/////////////////////////////////////////////////////
@@ -79,7 +79,7 @@ public void execute() {
7979
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete DNS Zone");
8080
}
8181
} catch (Exception e) {
82-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete DNS Zone: " + e.getMessage());
82+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
8383
}
8484
}
8585

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
// under the License.
1717
package com.cloud.vm.dao;
1818

19-
import java.util.List;
19+
import java.util.Set;
2020

2121
import org.apache.cloudstack.resourcedetail.ResourceDetailsDao;
2222

@@ -25,5 +25,5 @@
2525

2626
public interface NicDetailsDao extends GenericDao<NicDetailVO, Long>, ResourceDetailsDao<NicDetailVO> {
2727

28-
void removeDetailsForValuesIn(String resourceName, List<String> values);
28+
void removeDetailsForNicIds(String resourceName, Set<Long> nicIds);
2929
}

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,27 @@
1717
package com.cloud.vm.dao;
1818

1919

20-
import java.util.List;
20+
import java.util.Set;
2121

2222
import org.apache.cloudstack.api.ApiConstants;
23+
import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
2324
import org.apache.commons.collections.CollectionUtils;
2425
import org.springframework.stereotype.Component;
2526

26-
import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
27-
2827
import com.cloud.utils.db.SearchBuilder;
2928
import com.cloud.utils.db.SearchCriteria;
3029
import com.cloud.vm.NicDetailVO;
3130

3231
@Component
3332
public class NicDetailsDaoImpl extends ResourceDetailsDaoBase<NicDetailVO> implements NicDetailsDao {
34-
private final SearchBuilder<NicDetailVO> NameValuesSearch;
33+
private final SearchBuilder<NicDetailVO> ResourceIdNameSearch;
3534

3635
public NicDetailsDaoImpl() {
3736
super();
38-
NameValuesSearch = createSearchBuilder();
39-
NameValuesSearch.and(ApiConstants.NAME, NameValuesSearch.entity().getName(), SearchCriteria.Op.EQ);
40-
NameValuesSearch.and(ApiConstants.VALUE, NameValuesSearch.entity().getValue(), SearchCriteria.Op.IN);
41-
NameValuesSearch.done();
37+
ResourceIdNameSearch = createSearchBuilder();
38+
ResourceIdNameSearch.and(ApiConstants.NAME, ResourceIdNameSearch.entity().getName(), SearchCriteria.Op.EQ);
39+
ResourceIdNameSearch.and(ApiConstants.RESOURCE_ID, ResourceIdNameSearch.entity().getResourceId(), SearchCriteria.Op.IN);
40+
ResourceIdNameSearch.done();
4241
}
4342

4443

@@ -48,13 +47,13 @@ public void addDetail(long resourceId, String key, String value, boolean display
4847
}
4948

5049
@Override
51-
public void removeDetailsForValuesIn(String resourceName, List<String> values) {
52-
if (CollectionUtils.isEmpty(values)) {
50+
public void removeDetailsForNicIds(String resourceName, Set<Long> nicIds) {
51+
if (CollectionUtils.isEmpty(nicIds)) {
5352
return;
5453
}
55-
SearchCriteria<NicDetailVO> sc = NameValuesSearch.create();
54+
SearchCriteria<NicDetailVO> sc = ResourceIdNameSearch.create();
5655
sc.setParameters(ApiConstants.NAME, resourceName);
57-
sc.setParameters(ApiConstants.VALUE, values.toArray());
56+
sc.setParameters(ApiConstants.RESOURCE_ID, nicIds.toArray());
5857
remove(sc);
5958
}
6059
}

‎engine/schema/src/main/resources/META-INF/db/views/nic_dns_view.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,4 @@ FROM
3737
LEFT JOIN
3838
`cloud`.`nic_details` nd ON n.id = nd.nic_id AND nd.name = 'nicdnsname'
3939
WHERE
40-
map.removed IS NULL;
40+
n.instance_id IS NOT NULL AND map.removed IS NULL;

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.HashSet;
2929
import java.util.List;
3030
import java.util.Map;
31-
import java.util.Objects;
3231
import java.util.Set;
3332
import java.util.stream.Collectors;
3433

@@ -316,7 +315,7 @@ public boolean deleteDnsServer(DeleteDnsServerCmd cmd) {
316315
List<DnsZoneVO> dnsZones = dnsZoneDao.findDnsZonesByServerId(dnsServerId);
317316
for (DnsZoneVO dnsZone : dnsZones) {
318317
try {
319-
deleteDnsZone(dnsZone.getId(), false);
318+
deleteDnsZone(dnsZone.getId(), cmd.isUnmanage());
320319
} catch (Exception ex) {
321320
logger.error("Error cleaning up DNS zone: {} during DNS server: {} deletion", dnsZone.getName(), dnsServer.getName());
322321
}
@@ -328,31 +327,24 @@ public boolean deleteDnsServer(DeleteDnsServerCmd cmd) {
328327

329328
@Override
330329
@ActionEvent(eventType = EventTypes.EVENT_DNS_ZONE_DELETE, eventDescription = "Deleting DNS Zone")
331-
public boolean deleteDnsZone(Long zoneId, boolean isUnmanage) {
330+
public boolean deleteDnsZone(Long zoneId, boolean retainInProvider) {
332331
DnsZoneVO dnsZone = dnsZoneDao.findById(zoneId);
333332
if (dnsZone == null) {
334333
throw new InvalidParameterValueException("DNS zone not found for the given ID.");
335334
}
336335
String dnsZoneName = dnsZone.getName();
337336
Account caller = CallContext.current().getCallingAccount();
338337
accountMgr.checkAccess(caller, null, true, dnsZone);
339-
DnsServerVO server = dnsServerDao.findById(dnsZone.getDnsServerId());
340-
if (server == null) {
341-
throw new CloudRuntimeException(String.format("The DNS server not found for DNS zone: %s", dnsZoneName));
342-
}
343338

344339
boolean dbResult = Transaction.execute((TransactionCallback<Boolean>) status -> {
345340
DnsZoneNetworkMapVO networkMapVO = dnsZoneNetworkMapDao.findByZoneId(zoneId);
346-
DnsProvider provider = getProviderByType(server.getProviderType());
347341
// Remove DNS records from nic_details if there are any
348342
if (networkMapVO != null) {
349343
try {
350-
List<DnsRecord> records = provider.listRecords(server, dnsZone);
351-
if (CollectionUtils.isNotEmpty(records)) {
352-
List<String> dnsRecordNames = records.stream().map(DnsRecord::getName).filter(Objects::nonNull)
353-
.map(name -> name.replaceAll("\\.+$", ""))
354-
.collect(Collectors.toList());
355-
nicDetailsDao.removeDetailsForValuesIn(ApiConstants.NIC_DNS_NAME, dnsRecordNames);
344+
List<NicDnsJoinVO> nicDnsJoinVOS = nicDnsJoinDao.listByZoneId(zoneId);
345+
if (CollectionUtils.isNotEmpty(nicDnsJoinVOS)) {
346+
Set<Long> nicIds = nicDnsJoinVOS.stream().map(NicDnsJoinVO::getId).collect(Collectors.toSet());
347+
nicDetailsDao.removeDetailsForNicIds(ApiConstants.NIC_DNS_NAME, nicIds);
356348
}
357349
} catch (Exception ex) {
358350
logger.warn("Failed to fetch DNS records for dnsZone: {}, perform manual cleanup.", dnsZoneName, ex);
@@ -361,14 +353,22 @@ public boolean deleteDnsZone(Long zoneId, boolean isUnmanage) {
361353
}
362354

363355
// Remove DNS zone from provider and cleanup DB
364-
if (!isUnmanage) {
356+
if (!retainInProvider) {
365357
try {
358+
DnsServerVO server = dnsServerDao.findById(dnsZone.getDnsServerId());
359+
if (server == null) {
360+
throw new CloudRuntimeException(String.format("The DNS server not found for DNS zone: %s", dnsZoneName));
361+
}
362+
DnsProvider provider = getProviderByType(server.getProviderType());
366363
provider.deleteZone(server, dnsZone);
367364
logger.debug("Deleted DNS zone: {} from provider", dnsZoneName);
368365
} catch (DnsNotFoundException ex) {
369366
logger.warn("DNS zone: {} is not present in the provider, proceeding with cleanup", dnsZoneName);
370367
} catch (Exception ex) {
371368
logger.error("Failed to delete DNS zone from provider", ex);
369+
if (ex instanceof CloudRuntimeException) {
370+
throw new CloudRuntimeException(ex.getMessage());
371+
}
372372
throw new CloudRuntimeException(String.format("Failed to delete DNS zone: %s.", dnsZoneName));
373373
}
374374
}
@@ -1151,7 +1151,7 @@ String prepareDnsRecordUrl(String hostName, String subDomain, String dnsZoneName
11511151
}
11521152

11531153
private boolean isDnsCollision(String dnsRecordUrl, long targetZoneId, long instanceId) {
1154-
NicDnsJoinVO existing = nicDnsJoinDao.findActiveByDnsRecordAndZone(dnsRecordUrl, targetZoneId);
1154+
NicDnsJoinVO existing = nicDnsJoinDao.findActiveByDnsRecordAndZone(targetZoneId, dnsRecordUrl);
11551155
if (existing != null && existing.getInstanceId() != instanceId) {
11561156
logger.error("DNS collision: cannot register DNS record: {}. Already owned by Instance: {}.",
11571157
dnsRecordUrl, existing.getInstanceId());

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ public boolean remove(Long dnsServerId) {
173173

174174
@Override
175175
public void loadDetails(DnsServer dnsServer) {
176+
if (dnsServer == null) {
177+
return;
178+
}
176179
Map<String, String> details = dnsServerDetailsDao.listDetailsKeyPairs(dnsServer.getId());
177180
dnsServer.setDetails(details);
178181
}

‎server/src/main/java/org/apache/cloudstack/dns/dao/NicDnsJoinDao.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,22 @@ public interface NicDnsJoinDao extends GenericDao<NicDnsJoinVO, Long> {
2727

2828
/**
2929
* Used for Collision Checks.
30-
* @param dnsRecordUrl
30+
*
3131
* @param dnsZoneId
32+
* @param nicDnsName
3233
* @return active records to see who currently owns the dnsRecordUrl.
3334
*/
34-
NicDnsJoinVO findActiveByDnsRecordAndZone(String dnsRecordUrl, long dnsZoneId);
35+
NicDnsJoinVO findActiveByDnsRecordAndZone(long dnsZoneId, String nicDnsName);
3536

3637
/**
3738
* Used to sync DNS record url based on available ips for vmId in the dnsZone
39+
*
3840
* @param vmId
3941
* @param dnsZoneId
40-
* @param dnsRecordUrl
42+
* @param nicDnsName
4143
* @return list of active nics using the dnsRecordUrl, supports null vmId for dnsZone wide query
4244
*/
43-
List<NicDnsJoinVO> listActiveByVmIdZoneAndDnsRecord(Long vmId, long dnsZoneId, String dnsRecordUrl);
45+
List<NicDnsJoinVO> listActiveByVmIdZoneAndDnsRecord(Long vmId, long dnsZoneId, String nicDnsName);
4446

4547
/**
4648
* Used for VM Start/Running
@@ -55,4 +57,11 @@ public interface NicDnsJoinDao extends GenericDao<NicDnsJoinVO, Long> {
5557
* @return records with soft-delete
5658
*/
5759
List<NicDnsJoinVO> listIncludingRemovedByVmId(long vmId);
60+
61+
/**
62+
* Find all records for dnsZoneId with valid nicDnsName
63+
* @param dnsZoneId
64+
* @return
65+
*/
66+
List<NicDnsJoinVO> listByZoneId(long dnsZoneId);
5867
}

‎server/src/main/java/org/apache/cloudstack/dns/dao/NicDnsJoinDaoImpl.java‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public class NicDnsJoinDaoImpl extends GenericDaoBase<NicDnsJoinVO, Long> implem
3030
private final SearchBuilder<NicDnsJoinVO> activeDnsRecordZoneSearch;
3131
private final SearchBuilder<NicDnsJoinVO> activeVmZoneDnsRecordSearch; // Route for null vmId
3232
private final SearchBuilder<NicDnsJoinVO> activeVmSearch;
33+
private final SearchBuilder<NicDnsJoinVO> activeDnsRecordsByZoneIdSearch;
3334

3435
public NicDnsJoinDaoImpl() {
3536

@@ -49,28 +50,36 @@ public NicDnsJoinDaoImpl() {
4950
activeVmSearch = createSearchBuilder();
5051
activeVmSearch.and(ApiConstants.INSTANCE_ID, activeVmSearch.entity().getInstanceId(), SearchCriteria.Op.EQ);
5152
activeVmSearch.done();
53+
54+
activeDnsRecordsByZoneIdSearch = createSearchBuilder();
55+
activeDnsRecordsByZoneIdSearch.selectFields(activeDnsRecordsByZoneIdSearch.entity().getId());
56+
activeDnsRecordsByZoneIdSearch.and(ApiConstants.DNS_ZONE_ID, activeDnsRecordsByZoneIdSearch.entity().getDnsZoneId(), SearchCriteria.Op.EQ);
57+
activeDnsRecordsByZoneIdSearch.and(ApiConstants.NIC_DNS_NAME, activeDnsRecordsByZoneIdSearch.entity().getNicDnsName(), SearchCriteria.Op.NNULL);
58+
activeDnsRecordsByZoneIdSearch.and(ApiConstants.REMOVED, activeDnsRecordsByZoneIdSearch.entity().getRemoved(), SearchCriteria.Op.NULL);
59+
activeDnsRecordsByZoneIdSearch.done();
60+
5261
}
5362

5463
@Override
55-
public NicDnsJoinVO findActiveByDnsRecordAndZone(String dnsRecordUrl, long dnsZoneId) {
64+
public NicDnsJoinVO findActiveByDnsRecordAndZone(long dnsZoneId, String nicDnsName) {
5665
SearchCriteria<NicDnsJoinVO> sc = activeDnsRecordZoneSearch.create();
57-
sc.setParameters(ApiConstants.NIC_DNS_NAME, dnsRecordUrl);
66+
sc.setParameters(ApiConstants.NIC_DNS_NAME, nicDnsName);
5867
sc.setParameters(ApiConstants.DNS_ZONE_ID, dnsZoneId);
5968
return findOneBy(sc);
6069
}
6170

6271
@Override
63-
public List<NicDnsJoinVO> listActiveByVmIdZoneAndDnsRecord(Long vmId, long dnsZoneId, String dnsRecordUrl) {
72+
public List<NicDnsJoinVO> listActiveByVmIdZoneAndDnsRecord(Long vmId, long dnsZoneId, String nicDnsName) {
6473
if (vmId != null) {
65-
SearchCriteria<NicDnsJoinVO> sc = activeDnsRecordZoneSearch.create();
74+
SearchCriteria<NicDnsJoinVO> sc = activeVmZoneDnsRecordSearch.create();
6675
sc.setParameters(ApiConstants.INSTANCE_ID, vmId);
6776
sc.setParameters(ApiConstants.DNS_ZONE_ID, dnsZoneId);
68-
sc.setParameters(ApiConstants.NIC_DNS_NAME, dnsRecordUrl);
77+
sc.setParameters(ApiConstants.NIC_DNS_NAME, nicDnsName);
6978
return listBy(sc);
7079
} else {
7180
SearchCriteria<NicDnsJoinVO> sc = activeDnsRecordZoneSearch.create();
72-
sc.setParameters(ApiConstants.NIC_DNS_NAME, dnsRecordUrl);
7381
sc.setParameters(ApiConstants.DNS_ZONE_ID, dnsZoneId);
82+
sc.setParameters(ApiConstants.NIC_DNS_NAME, nicDnsName);
7483
return listBy(sc);
7584
}
7685
}
@@ -88,4 +97,11 @@ public List<NicDnsJoinVO> listIncludingRemovedByVmId(long vmId) {
8897
sc.setParameters(ApiConstants.INSTANCE_ID, vmId);
8998
return listIncludingRemovedBy(sc);
9099
}
100+
101+
@Override
102+
public List<NicDnsJoinVO> listByZoneId(long dnsZoneId) {
103+
SearchCriteria<NicDnsJoinVO> sc = activeDnsRecordsByZoneIdSearch.create();
104+
sc.setParameters(ApiConstants.DNS_ZONE_ID, dnsZoneId);
105+
return listBy(sc);
106+
}
91107
}

0 commit comments

Comments
 (0)