Skip to content

Commit a1eca74

Browse files
sudo87sudo87
committed
work on updateDnsServer ui screen, fixed acl issue for dns server and zone and cleanup methods for dns server and zone
1 parent d8cca73 commit a1eca74

File tree

19 files changed

+282
-64
lines changed

19 files changed

+282
-64
lines changed

‎api/src/main/java/com/cloud/user/AccountService.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.apache.cloudstack.api.response.ListResponse;
4747
import org.apache.cloudstack.auth.UserTwoFactorAuthenticator;
4848
import org.apache.cloudstack.backup.BackupOffering;
49-
import org.apache.cloudstack.dns.DnsServer;
5049

5150
public interface AccountService {
5251

@@ -129,8 +128,6 @@ User createUser(String userName, String password, String firstName, String lastN
129128

130129
void checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException;
131130

132-
void checkAccess(Account account, DnsServer dnsServer) throws PermissionDeniedException;
133-
134131
void checkAccess(User user, ControlledEntity entity);
135132

136133
void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName, ControlledEntity... entities) throws PermissionDeniedException;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public class DeleteDnsServerCmd extends BaseAsyncCmd {
5454

5555
@Parameter(name = ApiConstants.CLEANUP, type = CommandType.BOOLEAN,
5656
entityType = DnsZoneResponse.class, description = "True if all associated DNS zones have to be cleaned up with this server")
57-
private Boolean cleanup;
57+
private Boolean cleanup = true;
5858

5959
/////////////////////////////////////////////////////
6060
/////////////////// Accessors ///////////////////////

‎api/src/main/java/org/apache/cloudstack/api/response/DnsServerResponse.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ public class DnsServerResponse extends BaseResponse {
7474
@Param(description = "the name of the domain associated with the DNS server")
7575
private String domainName;
7676

77+
@SerializedName(ApiConstants.STATE)
78+
@Param(description = "The state of the account")
79+
private String state;
80+
7781
public DnsServerResponse() {
7882
super();
7983

@@ -122,4 +126,8 @@ public void setDomainId(String domainId) {
122126
public void setDomainName(String domainName) {
123127
this.domainName = domainName;
124128
}
129+
130+
public void setState(String state) {
131+
this.state = state;
132+
}
125133
}

‎api/src/main/java/org/apache/cloudstack/dns/DnsProviderManager.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.apache.cloudstack.api.response.ListResponse;
3939

4040
import com.cloud.network.Network;
41+
import com.cloud.user.Account;
4142
import com.cloud.utils.component.Manager;
4243
import com.cloud.utils.component.PluggableService;
4344
import com.cloud.vm.Nic;
@@ -75,4 +76,8 @@ public interface DnsProviderManager extends Manager, PluggableService {
7576
boolean disassociateZoneFromNetwork(DisassociateDnsZoneFromNetworkCmd cmd);
7677

7778
String processDnsRecordForInstance(VirtualMachine instance, Network network, Nic nic, boolean isAdd);
79+
80+
void checkDnsServerPermission(Account caller, DnsServer dnsServer);
81+
82+
void checkDnsZonePermission(Account caller, DnsZone dnsZone);
7883
}

‎server/src/main/java/com/cloud/acl/DomainChecker.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,16 @@
2727
import org.apache.cloudstack.acl.RolePermissionEntity;
2828
import org.apache.cloudstack.acl.SecurityChecker;
2929
import org.apache.cloudstack.affinity.AffinityGroup;
30-
import org.apache.cloudstack.backup.BackupOffering;
31-
import org.apache.cloudstack.backup.dao.BackupOfferingDetailsDao;
3230
import org.apache.cloudstack.context.CallContext;
31+
import org.apache.cloudstack.dns.DnsProviderManager;
32+
import org.apache.cloudstack.dns.DnsServer;
33+
import org.apache.cloudstack.dns.DnsZone;
3334
import org.apache.cloudstack.query.QueryService;
3435
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
3536
import org.springframework.stereotype.Component;
3637

38+
import org.apache.cloudstack.backup.dao.BackupOfferingDetailsDao;
39+
import org.apache.cloudstack.backup.BackupOffering;
3740
import com.cloud.dc.DataCenter;
3841
import com.cloud.dc.DedicatedResourceVO;
3942
import com.cloud.dc.dao.DedicatedResourceDao;
@@ -101,6 +104,8 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
101104
private ProjectDao projectDao;
102105
@Inject
103106
private AccountService accountService;
107+
@Inject
108+
private DnsProviderManager dnsProviderManager;
104109

105110
protected DomainChecker() {
106111
super();
@@ -216,7 +221,11 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
216221
_networkMgr.checkRouterPermissions(caller, (VirtualRouter)entity);
217222
} else if (entity instanceof AffinityGroup) {
218223
return false;
219-
} else {
224+
} else if (entity instanceof DnsServer) {
225+
dnsProviderManager.checkDnsServerPermission(caller, (DnsServer) entity);
226+
} else if (entity instanceof DnsZone) {
227+
dnsProviderManager.checkDnsZonePermission(caller, (DnsZone) entity);
228+
} else {
220229
validateCallerHasAccessToEntityOwner(caller, entity, accessType);
221230
}
222231
return true;

‎server/src/main/java/com/cloud/event/ActionEventUtils.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.cloudstack.context.CallContext;
3535
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
3636
import org.apache.cloudstack.framework.events.EventDistributor;
37+
import org.apache.cloudstack.framework.messagebus.MessageBus;
3738
import org.apache.commons.lang3.ObjectUtils;
3839
import org.apache.commons.lang3.StringUtils;
3940
import org.apache.logging.log4j.LogManager;
@@ -85,6 +86,8 @@ public class ActionEventUtils {
8586
EntityManager entityMgr;
8687
@Inject
8788
ConfigurationDao configDao;
89+
@Inject
90+
MessageBus messageBus;
8891

8992
public ActionEventUtils() {
9093
}
@@ -97,6 +100,7 @@ void init() {
97100
s_projectDao = projectDao;
98101
s_entityMgr = entityMgr;
99102
s_configDao = configDao;
103+
messageBus = messageBus;
100104
}
101105

102106
public static Long onActionEvent(Long userId, Long accountId, Long domainId, String type, String description, Long resourceId, String resourceType) {

‎server/src/main/java/com/cloud/user/AccountManagerImpl.java‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
import org.apache.cloudstack.config.ApiServiceConfiguration;
9696
import org.apache.cloudstack.context.CallContext;
9797
import org.apache.cloudstack.dns.DnsServer;
98+
import org.apache.cloudstack.dns.DnsZone;
9899
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
99100
import org.apache.cloudstack.framework.config.ConfigKey;
100101
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -774,7 +775,8 @@ public void checkAccess(Account caller, AccessType accessType, boolean sameOwner
774775
}
775776
if (entity.getAccountId() != -1 && domainId != -1 && !(entity instanceof VirtualMachineTemplate)
776777
&& !(entity instanceof Network && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry))
777-
&& !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter)) {
778+
&& !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter)
779+
&& !(entity instanceof DnsServer) && !(entity instanceof DnsZone)) {
778780
List<ControlledEntity> toBeChecked = domains.get(entity.getDomainId());
779781
// for templates, we don't have to do cross domains check
780782
if (toBeChecked == null) {
@@ -3972,20 +3974,6 @@ public void checkAccess(Account account, BackupOffering bof) throws PermissionDe
39723974
throw new PermissionDeniedException("There's no way to confirm " + account + " has access to " + bof);
39733975
}
39743976

3975-
@Override
3976-
public void checkAccess(Account caller, DnsServer dnsServer) throws PermissionDeniedException {
3977-
if (caller.getId() == dnsServer.getAccountId()) {
3978-
return;
3979-
}
3980-
if (!dnsServer.getPublicServer()) {
3981-
throw new PermissionDeniedException(caller + "is not allowed to access the DNS server " + dnsServer.getName());
3982-
}
3983-
Account owner = getAccount(dnsServer.getAccountId());
3984-
if (!_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
3985-
throw new PermissionDeniedException(caller + "is not allowed to access the DNS server " + dnsServer.getName());
3986-
}
3987-
}
3988-
39893977
@Override
39903978
public void checkAccess(User user, ControlledEntity entity) throws PermissionDeniedException {
39913979
for (SecurityChecker checker : _securityCheckers) {

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

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,14 @@
7171
import com.cloud.network.dao.NetworkVO;
7272
import com.cloud.user.Account;
7373
import com.cloud.user.AccountManager;
74+
import com.cloud.user.dao.AccountDao;
7475
import com.cloud.utils.Pair;
7576
import com.cloud.utils.StringUtils;
7677
import com.cloud.utils.component.ManagerBase;
7778
import com.cloud.utils.component.PluggableService;
7879
import com.cloud.utils.db.Filter;
80+
import com.cloud.utils.db.Transaction;
81+
import com.cloud.utils.db.TransactionCallback;
7982
import com.cloud.utils.exception.CloudRuntimeException;
8083
import com.cloud.vm.Nic;
8184
import com.cloud.vm.VirtualMachine;
@@ -105,6 +108,8 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa
105108
DnsZoneJoinDao dnsZoneJoinDao;
106109
@Inject
107110
DnsServerJoinDao dnsServerJoinDao;
111+
@Inject
112+
AccountDao accountDao;
108113

109114
private DnsProvider getProviderByType(DnsProviderType type) {
110115
if (type == null) {
@@ -194,7 +199,7 @@ public DnsServer updateDnsServer(UpdateDnsServerCmd cmd) {
194199
}
195200

196201
Account caller = CallContext.current().getCallingAccount();
197-
accountMgr.checkAccess(caller, dnsServer);
202+
accountMgr.checkAccess(caller, null, true, dnsServer);
198203

199204
boolean validationRequired = false;
200205
String originalUrl = dnsServer.getUrl();
@@ -265,35 +270,55 @@ public boolean deleteDnsServer(DeleteDnsServerCmd cmd) {
265270
throw new InvalidParameterValueException(String.format("DNS server with ID: %s not found.", dnsServerId));
266271
}
267272
Account caller = CallContext.current().getCallingAccount();
268-
accountMgr.checkAccess(caller, dnsServer);
269-
if (cmd.getCleanup()) {
270-
// ToDo cleanup associated dnsZones
271-
}
272-
return dnsServerDao.remove(dnsServerId);
273+
accountMgr.checkAccess(caller, null, true, dnsServer);
274+
return Transaction.execute((TransactionCallback<Boolean>) status -> {
275+
if (cmd.getCleanup()) {
276+
List<DnsZoneVO> dnsZones = dnsZoneDao.findDnsZonesByServerId(dnsServerId);
277+
for (DnsZoneVO dnsZone : dnsZones) {
278+
long dnsZoneId = dnsZone.getId();
279+
dnsZoneNetworkMapDao.removeNetworkMappingByZoneId(dnsZoneId);
280+
// ToDo: delete nic_record_urls from vm_details if present before removing dnsZone
281+
dnsZoneDao.remove(dnsZoneId);
282+
}
283+
}
284+
return dnsServerDao.remove(dnsServerId);
285+
});
273286
}
274287

275288
@Override
276289
@ActionEvent(eventType = EventTypes.EVENT_DNS_ZONE_DELETE, eventDescription = "Deleting DNS Zone")
277290
public boolean deleteDnsZone(Long zoneId) {
278-
DnsZoneVO zone = dnsZoneDao.findById(zoneId);
279-
if (zone == null) {
291+
DnsZoneVO dnsZone = dnsZoneDao.findById(zoneId);
292+
if (dnsZone == null) {
280293
throw new InvalidParameterValueException("DNS zone not found for the given ID.");
281294
}
282-
295+
String dnsZoneName = dnsZone.getName();
283296
Account caller = CallContext.current().getCallingAccount();
284-
accountMgr.checkAccess(caller, null, true, zone);
285-
DnsServerVO server = dnsServerDao.findById(zone.getDnsServerId());
286-
if (server != null && zone.getState() == DnsZone.State.Active) {
287-
try {
288-
DnsProvider provider = getProviderByType(server.getProviderType());
289-
provider.deleteZone(server, zone);
290-
logger.debug("Deleted DNS zone: {}", zone.getName());
291-
} catch (Exception ex) {
292-
logger.error("Failed to delete DNS zone from provider", ex);
293-
throw new CloudRuntimeException("Failed to delete DNS zone.");
294-
}
297+
accountMgr.checkAccess(caller, null, true, dnsZone);
298+
DnsServerVO server = dnsServerDao.findById(dnsZone.getDnsServerId());
299+
if (server == null) {
300+
throw new CloudRuntimeException(String.format("The DNS server not found for DNS zone: %s", dnsZoneName));
295301
}
296-
return dnsZoneDao.remove(zoneId);
302+
try {
303+
DnsProvider provider = getProviderByType(server.getProviderType());
304+
provider.deleteZone(server, dnsZone);
305+
logger.debug("Deleted DNS zone: {} from provider", dnsZoneName);
306+
} catch (DnsNotFoundException ex) {
307+
logger.warn("DNS zone: {} is not present in the provider, proceeding with cleanup", dnsZoneName);
308+
} catch (Exception ex) {
309+
logger.error("Failed to delete DNS zone from provider", ex);
310+
throw new CloudRuntimeException(String.format("Failed to delete DNS zone: %s.", dnsZoneName));
311+
}
312+
313+
boolean dbResult = Transaction.execute((TransactionCallback<Boolean>) status -> {
314+
dnsZoneNetworkMapDao.removeNetworkMappingByZoneId(zoneId);
315+
// ToDo: delete nic_record_urls from vm_details if present before removing dnsZone
316+
return dnsZoneDao.remove(zoneId);
317+
});
318+
if (!dbResult) {
319+
logger.error("Failed to remove DNS zone {} from DB after provider deletion", dnsZoneName);
320+
}
321+
return dbResult;
297322
}
298323

299324
@Override
@@ -355,7 +380,7 @@ private Pair<List<DnsZoneVO>, Integer> searchForDnsZonesInternal(ListDnsZonesCmd
355380
Account caller = CallContext.current().getCallingAccount();
356381
if (cmd.getDnsServerId() != null) {
357382
DnsServer dnsServer = dnsServerDao.findById(cmd.getDnsServerId());
358-
accountMgr.checkAccess(caller, dnsServer);
383+
accountMgr.checkAccess(caller, null, true, dnsServer);
359384
}
360385
List<Long> ownDnsServerIds = dnsServerDao.listDnsServerIdsByAccountId(caller.getAccountId());
361386
String keyword = cmd.getKeyword();
@@ -538,6 +563,7 @@ DnsServerResponse createDnsServerResponse(DnsServerJoinVO server) {
538563
response.setAccountName(server.getAccountName());
539564
response.setDomainId(server.getDomainUuid()); // Note: APIs always return UUIDs, not internal DB IDs!
540565
response.setDomainName(server.getDomainName());
566+
response.setState(server.getState().name());
541567
response.setObjectName("dnsserver");
542568
return response;
543569
}
@@ -677,6 +703,31 @@ public String processDnsRecordForInstance(VirtualMachine instance, Network netwo
677703
return null;
678704
}
679705

706+
@Override
707+
public void checkDnsServerPermission(Account caller, DnsServer dnsServer) throws PermissionDeniedException {
708+
if (caller.getId() == dnsServer.getAccountId()) {
709+
return;
710+
}
711+
if (!dnsServer.getPublicServer()) {
712+
throw new PermissionDeniedException(caller + "is not allowed to access the DNS server " + dnsServer.getName());
713+
}
714+
Account owner = getAccount(dnsServer.getAccountId());
715+
if (!domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
716+
throw new PermissionDeniedException(caller + "is not allowed to access the DNS server " + dnsServer.getName());
717+
}
718+
}
719+
720+
@Override
721+
public void checkDnsZonePermission(Account caller, DnsZone zone) {
722+
if (caller.getId() != zone.getAccountId()) {
723+
throw new PermissionDeniedException(caller + "is not allowed to access the DNS Zone " + zone.getName());
724+
}
725+
}
726+
727+
public Account getAccount(long accountId) {
728+
return accountDao.findByIdIncludingRemoved(accountId);
729+
}
730+
680731
@Override
681732
public boolean start() {
682733
if (dnsProviders == null || dnsProviders.isEmpty()) {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ public boolean configure(final String name, final Map<String, Object> params) {
7777
eventBus.subscribe(new EventTopic(null, EventTypes.EVENT_VM_DESTROY, null, null, null), this);
7878
eventBus.subscribe(new EventTopic(null, EventTypes.EVENT_NIC_CREATE, null, null, null), this);
7979
eventBus.subscribe(new EventTopic(null, EventTypes.EVENT_NIC_DELETE, null, null, null), this);
80+
eventBus.subscribe(new EventTopic(null, EventTypes.EVENT_DNS_RECORD_DELETE, null, null, null), this);
8081
} catch (EventBusException ex) {
8182
logger.error("Failed to subscribe DnsVmLifecycleListener to EventBus", ex);
8283
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,6 @@ public interface DnsZoneDao extends GenericDao<DnsZoneVO, Long> {
3232

3333
Pair<List<DnsZoneVO>, Integer> searchZones(Long id, Long accountId, List<Long> ownDnsServerIds, Long targetDnsServerId,
3434
String keyword, Filter filter);
35+
36+
List<DnsZoneVO> findDnsZonesByServerId(long dnsServerId);
3537
}

0 commit comments

Comments
 (0)