Skip to content

Commit 2330cb2

Browse files
raveningravening
committed
Bug fix in displaying pulbic IP address of shared networks
Public IP address dedicated to one domain should not be accessed by other domains. Also root admin should be able to display all public ip address in system. Currently following issues exist 1. Public IP address assigned to one domain can be accessed by other sibling domains If use.system.public.ip is false then child domains should not see public ip of ROOT domain Before fix ``` (test1) mgt01 > list publicipaddresses listall=true fordisplay=true allocatedonly=false forvirtualnetwork=true filter=ipaddress, { "count": 59, "publicipaddress": [ ``` After fix ``` (test) mgt01 > list publicipaddresses listall=true fordisplay=true allocatedonly=false forvirtualnetwork=true filter=ipaddress, { "count": 10, ```
1 parent de7b131 commit 2330cb2

File tree

7 files changed

+1182
-87
lines changed

7 files changed

+1182
-87
lines changed

‎.travis.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ env:
164164

165165
- TESTS="component/test_project_usage
166166
component/test_protocol_number_security_group
167+
component/test_public_ip
167168
component/test_resource_limits"
168169

169170
- TESTS="component/test_regions_accounts

‎engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,5 +208,10 @@ void allocateNicValues(NicProfile nic, DataCenter dc, VirtualMachineProfile vm,
208208
void releasePodIp(Long id) throws CloudRuntimeException;
209209

210210
boolean isUsageHidden(IPAddressVO address);
211+
212+
List<IPAddressVO> listAvailablePublicIps(final long dcId, final Long podId, final List<Long> vlanDbIds, final Account owner, final VlanType vlanUse, final Long guestNetworkId,
213+
final boolean sourceNat, final boolean assign, final boolean allocate, final String requestedIp, final boolean isSystem,
214+
final Long vpcId, final Boolean displayIp, final boolean forSystemVms, final boolean lockOneRow)
215+
throws InsufficientAddressCapacityException;
211216
}
212217

‎server/src/main/java/com/cloud/api/ApiDBUtils.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import javax.annotation.PostConstruct;
2828
import javax.inject.Inject;
2929

30+
import com.cloud.vm.NicVO;
31+
import com.cloud.vm.dao.NicDao;
3032
import org.apache.cloudstack.acl.Role;
3133
import org.apache.cloudstack.acl.RoleService;
3234
import org.apache.cloudstack.affinity.AffinityGroup;
@@ -458,6 +460,7 @@ public class ApiDBUtils {
458460
static BackupDao s_backupDao;
459461
static BackupScheduleDao s_backupScheduleDao;
460462
static BackupOfferingDao s_backupOfferingDao;
463+
static NicDao s_nicDao;
461464

462465
@Inject
463466
private ManagementServer ms;
@@ -702,6 +705,8 @@ public class ApiDBUtils {
702705
private BackupOfferingDao backupOfferingDao;
703706
@Inject
704707
private BackupScheduleDao backupScheduleDao;
708+
@Inject
709+
private NicDao nicDao;
705710

706711
@PostConstruct
707712
void init() {
@@ -811,6 +816,7 @@ void init() {
811816
s_hostDetailsDao = hostDetailsDao;
812817
s_clusterDetailsDao = clusterDetailsDao;
813818
s_vmSnapshotDao = vmSnapshotDao;
819+
s_nicDao = nicDao;
814820
s_nicSecondaryIpDao = nicSecondaryIpDao;
815821
s_vpcProvSvc = vpcProvSvc;
816822
s_affinityGroupDao = affinityGroupDao;
@@ -2079,4 +2085,12 @@ public static BackupScheduleResponse newBackupScheduleResponse(BackupSchedule sc
20792085
public static BackupOfferingResponse newBackupOfferingResponse(BackupOffering policy) {
20802086
return s_backupOfferingDao.newBackupOfferingResponse(policy);
20812087
}
2088+
2089+
public static NicVO findByIp4AddressAndNetworkId(String ip4Address, long networkId) {
2090+
return s_nicDao.findByIp4AddressAndNetworkId(ip4Address, networkId);
2091+
}
2092+
2093+
public static NicSecondaryIpVO findSecondaryIpByIp4AddressAndNetworkId(String ip4Address, long networkId) {
2094+
return s_nicSecondaryIpDao.findByIp4AddressAndNetworkId(ip4Address, networkId);
2095+
}
20822096
}

‎server/src/main/java/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,41 @@ public IPAddressResponse createIPAddressResponse(ResponseView view, IpAddress ip
910910
}
911911
}
912912

913+
// show vm info for shared networks
914+
if (!forVirtualNetworks) {
915+
NicVO nic = ApiDBUtils.findByIp4AddressAndNetworkId(ipAddr.getAddress().toString(), ipAddr.getNetworkId());
916+
if (nic != null && nic.getVmType() == VirtualMachine.Type.User) {
917+
UserVm vm = ApiDBUtils.findUserVmById(nic.getInstanceId());
918+
if (vm != null) {
919+
ipResponse.setVirtualMachineId(vm.getUuid());
920+
ipResponse.setVirtualMachineName(vm.getHostName());
921+
if (vm.getDisplayName() != null) {
922+
ipResponse.setVirtualMachineDisplayName(vm.getDisplayName());
923+
} else {
924+
ipResponse.setVirtualMachineDisplayName(vm.getHostName());
925+
}
926+
}
927+
} else if (nic != null && nic.getVmType() == VirtualMachine.Type.DomainRouter) {
928+
ipResponse.setIsSystem(true);
929+
}
930+
if (nic == null) { // find in nic_secondary_ips, user vm only
931+
NicSecondaryIpVO secondaryIp =
932+
ApiDBUtils.findSecondaryIpByIp4AddressAndNetworkId(ipAddr.getAddress().toString(), ipAddr.getNetworkId());
933+
if (secondaryIp != null) {
934+
UserVm vm = ApiDBUtils.findUserVmById(secondaryIp.getVmId());
935+
if (vm != null) {
936+
ipResponse.setVirtualMachineId(vm.getUuid());
937+
ipResponse.setVirtualMachineName(vm.getHostName());
938+
if (vm.getDisplayName() != null) {
939+
ipResponse.setVirtualMachineDisplayName(vm.getDisplayName());
940+
} else {
941+
ipResponse.setVirtualMachineDisplayName(vm.getHostName());
942+
}
943+
}
944+
}
945+
}
946+
}
947+
913948
// show this info to full view only
914949
if (view == ResponseView.Full) {
915950
VlanVO vl = ApiDBUtils.findVlanById(ipAddr.getVlanId());

‎server/src/main/java/com/cloud/network/IpAddressManagerImpl.java‎

Lines changed: 65 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -322,31 +322,30 @@ private IPAddressVO assignAndAllocateIpAddressEntry(final Account owner, final V
322322
}
323323
}
324324

325-
for (final IPAddressVO possibleAddr : addressVOS) {
325+
for (IPAddressVO possibleAddr : addressVOS) {
326326
if (possibleAddr.getState() != State.Free) {
327327
continue;
328328
}
329-
final IPAddressVO addressVO = possibleAddr;
330-
addressVO.setSourceNat(sourceNat);
331-
addressVO.setAllocatedTime(new Date());
332-
addressVO.setAllocatedInDomainId(owner.getDomainId());
333-
addressVO.setAllocatedToAccountId(owner.getId());
334-
addressVO.setSystem(isSystem);
329+
possibleAddr.setSourceNat(sourceNat);
330+
possibleAddr.setAllocatedTime(new Date());
331+
possibleAddr.setAllocatedInDomainId(owner.getDomainId());
332+
possibleAddr.setAllocatedToAccountId(owner.getId());
333+
possibleAddr.setSystem(isSystem);
335334

336335
if (displayIp != null) {
337-
addressVO.setDisplay(displayIp);
336+
possibleAddr.setDisplay(displayIp);
338337
}
339338

340339
if (vlanUse != VlanType.DirectAttached) {
341-
addressVO.setAssociatedWithNetworkId(guestNetworkId);
342-
addressVO.setVpcId(vpcId);
340+
possibleAddr.setAssociatedWithNetworkId(guestNetworkId);
341+
possibleAddr.setVpcId(vpcId);
343342
}
344343
if (_ipAddressDao.lockRow(possibleAddr.getId(), true) != null) {
345-
final IPAddressVO userIp = _ipAddressDao.findById(addressVO.getId());
344+
final IPAddressVO userIp = _ipAddressDao.findById(possibleAddr.getId());
346345
if (userIp.getState() == State.Free) {
347-
addressVO.setState(State.Allocating);
348-
if (_ipAddressDao.update(addressVO.getId(), addressVO)) {
349-
finalAddress = addressVO;
346+
possibleAddr.setState(State.Allocating);
347+
if (_ipAddressDao.update(possibleAddr.getId(), possibleAddr)) {
348+
finalAddress = possibleAddr;
350349
break;
351350
}
352351
}
@@ -783,9 +782,22 @@ public PublicIp fetchNewPublicIp(final long dcId, final Long podId, final List<L
783782
public PublicIp fetchNewPublicIp(final long dcId, final Long podId, final List<Long> vlanDbIds, final Account owner, final VlanType vlanUse, final Long guestNetworkId,
784783
final boolean sourceNat, final boolean assign, final boolean allocate, final String requestedIp, final boolean isSystem, final Long vpcId, final Boolean displayIp, final boolean forSystemVms)
785784
throws InsufficientAddressCapacityException {
786-
IPAddressVO addr = Transaction.execute(new TransactionCallbackWithException<IPAddressVO, InsufficientAddressCapacityException>() {
785+
List<IPAddressVO> addrs = listAvailablePublicIps(dcId, podId, vlanDbIds, owner, vlanUse, guestNetworkId, sourceNat, assign, allocate, requestedIp, isSystem, vpcId, displayIp, forSystemVms, true);
786+
IPAddressVO addr = addrs.get(0);
787+
if (vlanUse == VlanType.VirtualNetwork) {
788+
_firewallMgr.addSystemFirewallRules(addr, owner);
789+
}
790+
791+
return PublicIp.createFromAddrAndVlan(addr, _vlanDao.findById(addr.getVlanId()));
792+
}
793+
794+
@Override
795+
public List<IPAddressVO> listAvailablePublicIps(final long dcId, final Long podId, final List<Long> vlanDbIds, final Account owner, final VlanType vlanUse, final Long guestNetworkId,
796+
final boolean sourceNat, final boolean assign, final boolean allocate, final String requestedIp, final boolean isSystem,
797+
final Long vpcId, final Boolean displayIp, final boolean forSystemVms, final boolean lockOneRow) throws InsufficientAddressCapacityException {
798+
return Transaction.execute(new TransactionCallbackWithException<List<IPAddressVO>, InsufficientAddressCapacityException>() {
787799
@Override
788-
public IPAddressVO doInTransaction(TransactionStatus status) throws InsufficientAddressCapacityException {
800+
public List<IPAddressVO> doInTransaction(TransactionStatus status) throws InsufficientAddressCapacityException {
789801
StringBuilder errorMessage = new StringBuilder("Unable to get ip address in ");
790802
boolean fetchFromDedicatedRange = false;
791803
List<Long> dedicatedVlanDbIds = new ArrayList<Long>();
@@ -823,23 +835,26 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
823835
if (vlanDbIds == null || vlanDbIds.contains(nonDedicatedVlan.getId()))
824836
nonDedicatedVlanDbIds.add(nonDedicatedVlan.getId());
825837
}
826-
if (dedicatedVlanDbIds != null && !dedicatedVlanDbIds.isEmpty()) {
827-
fetchFromDedicatedRange = true;
828-
sc.setParameters("vlanId", dedicatedVlanDbIds.toArray());
829-
errorMessage.append(", vlanId id=" + Arrays.toString(dedicatedVlanDbIds.toArray()));
830-
} else if (nonDedicatedVlanDbIds != null && !nonDedicatedVlanDbIds.isEmpty()) {
831-
sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray());
832-
errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray()));
833-
} else {
834-
if (podId != null) {
835-
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", Pod.class, podId);
836-
ex.addProxyObject(ApiDBUtils.findPodById(podId).getUuid());
838+
839+
if (vlanUse == VlanType.VirtualNetwork) {
840+
if (dedicatedVlanDbIds != null && !dedicatedVlanDbIds.isEmpty()) {
841+
fetchFromDedicatedRange = true;
842+
sc.setParameters("vlanId", dedicatedVlanDbIds.toArray());
843+
errorMessage.append(", vlanId id=" + Arrays.toString(dedicatedVlanDbIds.toArray()));
844+
} else if (nonDedicatedVlanDbIds != null && !nonDedicatedVlanDbIds.isEmpty()) {
845+
sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray());
846+
errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray()));
847+
} else {
848+
if (podId != null) {
849+
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", Pod.class, podId);
850+
ex.addProxyObject(ApiDBUtils.findPodById(podId).getUuid());
851+
throw ex;
852+
}
853+
s_logger.warn(errorMessage.toString());
854+
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", DataCenter.class, dcId);
855+
ex.addProxyObject(ApiDBUtils.findZoneById(dcId).getUuid());
837856
throw ex;
838857
}
839-
s_logger.warn(errorMessage.toString());
840-
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", DataCenter.class, dcId);
841-
ex.addProxyObject(ApiDBUtils.findZoneById(dcId).getUuid());
842-
throw ex;
843858
}
844859

845860
sc.setParameters("dc", dcId);
@@ -864,21 +879,31 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
864879

865880
filter.addOrderBy(IPAddressVO.class,"vlanId", true);
866881

867-
List<IPAddressVO> addrs = _ipAddressDao.search(sc, filter, false);
882+
List<IPAddressVO> addrs;
883+
884+
if (lockOneRow) {
885+
addrs = _ipAddressDao.lockRows(sc, filter, true);
886+
} else {
887+
addrs = new ArrayList<>(_ipAddressDao.search(sc, null));
888+
}
868889

869890
// If all the dedicated IPs of the owner are in use fetch an IP from the system pool
870-
if (addrs.size() == 0 && fetchFromDedicatedRange) {
891+
if ((!lockOneRow || (lockOneRow && addrs.size() == 0)) && fetchFromDedicatedRange && vlanUse == VlanType.VirtualNetwork) {
871892
// Verify if account is allowed to acquire IPs from the system
872893
boolean useSystemIps = UseSystemPublicIps.valueIn(owner.getId());
873894
if (useSystemIps && nonDedicatedVlanDbIds != null && !nonDedicatedVlanDbIds.isEmpty()) {
874895
fetchFromDedicatedRange = false;
875896
sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray());
876897
errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray()));
877-
addrs = _ipAddressDao.search(sc, filter, false);
898+
if (lockOneRow) {
899+
addrs = _ipAddressDao.lockRows(sc, filter, true);
900+
} else {
901+
addrs.addAll(_ipAddressDao.search(sc, null));
902+
}
878903
}
879904
}
880905

881-
if (addrs.size() == 0) {
906+
if (lockOneRow && addrs.size() == 0) {
882907
if (podId != null) {
883908
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", Pod.class, podId);
884909
// for now, we hardcode the table names, but we should ideally do a lookup for the tablename from the VO object.
@@ -891,23 +916,16 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
891916
throw ex;
892917
}
893918

894-
assert(addrs.size() == 1) : "Return size is incorrect: " + addrs.size();
895-
IPAddressVO finalAddr = null;
919+
if (lockOneRow) {
920+
assert (addrs.size() == 1) : "Return size is incorrect: " + addrs.size();
921+
}
896922
if (assign) {
897-
finalAddr = assignAndAllocateIpAddressEntry(owner, vlanUse, guestNetworkId, sourceNat, allocate,
923+
assignAndAllocateIpAddressEntry(owner, vlanUse, guestNetworkId, sourceNat, allocate,
898924
isSystem,vpcId, displayIp, fetchFromDedicatedRange, addrs);
899-
} else {
900-
finalAddr = addrs.get(0);
901925
}
902-
return finalAddr;
926+
return addrs;
903927
}
904928
});
905-
906-
if (vlanUse == VlanType.VirtualNetwork) {
907-
_firewallMgr.addSystemFirewallRules(addr, owner);
908-
}
909-
910-
return PublicIp.createFromAddrAndVlan(addr, _vlanDao.findById(addr.getVlanId()));
911929
}
912930

913931
@DB

0 commit comments

Comments
 (0)