diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 08ae0ad13f43..7ec73ac8ac5f 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -576,6 +576,7 @@ import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao; import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO; import org.apache.cloudstack.utils.identity.ManagementServerNode; +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; @@ -783,7 +784,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe @Inject private DataCenterDao _dcDao; @Inject - private VlanDao _vlanDao; + protected VlanDao _vlanDao; @Inject private AccountVlanMapDao _accountVlanMapDao; @Inject @@ -2177,9 +2178,9 @@ public Pair, Integer> searchForIPAddresses(final ListP Boolean isRecursive = null; final List permittedAccounts = new ArrayList<>(); ListProjectResourcesCriteria listProjectResourcesCriteria = null; + if (isAllocated || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.ACCOUNT_TYPE_ADMIN || cmd.getDomainId() != null))) { - final Ternary domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(), - null); + final Ternary domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(), null); _accountMgr.buildACLSearchParameters(caller, cmd.getId(), cmd.getAccountName(), cmd.getProjectId(), permittedAccounts, domainIdRecursiveListProject, cmd.listAll(), false); domainId = domainIdRecursiveListProject.first(); isRecursive = domainIdRecursiveListProject.second(); @@ -2273,7 +2274,7 @@ private void buildParameters(final SearchBuilder sb, final ListPubl sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.EQ); sb.and("address", sb.entity().getAddress(), SearchCriteria.Op.EQ); - sb.and("vlanDbId", sb.entity().getVlanId(), SearchCriteria.Op.EQ); + sb.and("vlanDbId", sb.entity().getVlanId(), SearchCriteria.Op.IN); sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); sb.and("physicalNetworkId", sb.entity().getPhysicalNetworkId(), SearchCriteria.Op.EQ); sb.and("associatedNetworkIdEq", sb.entity().getAssociatedWithNetworkId(), SearchCriteria.Op.EQ); @@ -2367,7 +2368,9 @@ private void setParameters(SearchCriteria sc, final ListPublicIpAdd sc.setParameters("address", address); } - if (vlan != null) { + if (vlan == null && zone != null) { + sc.setParameters("vlanDbId", retrieveAvailableVlanDbIdsForAccount(zone).toArray()); + } else if (vlan != null){ sc.setParameters("vlanDbId", vlan); } @@ -2390,6 +2393,73 @@ private void setParameters(SearchCriteria sc, final ListPublicIpAdd sc.setParameters( "forsystemvms", false); } + /** + * Retrieves a list of dedicated VLANs for the account and zone wide non dedicated VLANS, if {@link IpAddressManager#UseSystemPublicIps} is true for the account.
+ * If the lists is empty, then it will return a list with a single value (-1), meaning that the account does not have access to any VLAN. This is needed because if we do not + * set a value to the vlanDbId parameter in the API listPublicIPAddresses, it will retrieve all the VLANs for the zone, even the dedicated to another accounts. + */ + protected List retrieveAvailableVlanDbIdsForAccount(Long zoneId) { + List vlanDbIdsToRetrieve = new ArrayList<>(); + List availableVlans = new ArrayList<>(); + + Account caller = getCaller(); + String accountDescription = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(caller, "accountName", "uuid"); + long accountId = caller.getId(); + + availableVlans.addAll(retrieveZoneWideNonDedicatedVlans(accountId, accountDescription, zoneId)); + availableVlans.addAll(retrieveDedicatedVlans(accountId, accountDescription)); + + for (VlanVO availableVlan: availableVlans) { + vlanDbIdsToRetrieve.add(availableVlan.getId()); + } + + if (CollectionUtils.isEmpty(vlanDbIdsToRetrieve)) { + s_logger.debug(String.format("There are no VLANs available for account [%s].", accountDescription)); + vlanDbIdsToRetrieve.add(-1l); + } + + return vlanDbIdsToRetrieve; + } + + /** + * Returns zone wide non dedicated VLANs, if {@link IpAddressManager#UseSystemPublicIps} is true for the account. + */ + protected List retrieveZoneWideNonDedicatedVlans(long accountId, String accountDescription, long zoneId) { + List zoneWideNonDedicatedVlans = new ArrayList<>(); + + if (!getUseSystemPublicIpsValueIn(accountId)) { + s_logger.trace(String.format("Account [%s] does not have access to system public IPs. Not retrieving zone wide non-dedicated VLANs.", accountDescription)); + return zoneWideNonDedicatedVlans; + } + + s_logger.trace(String.format("Listing non-dedicated VLANs on zone [%s] for account [%s].", zoneId, accountDescription)); + zoneWideNonDedicatedVlans = _vlanDao.listZoneWideNonDedicatedVlans(zoneId); + + s_logger.debug(String.format("Listed [%s] non-dedicated VLANs on zone [%s] for account [%s]. Non-dedicated VLANs: [%s].", zoneWideNonDedicatedVlans.size(), zoneId, + accountDescription, ReflectionToStringBuilderUtils.reflectOnlySelectedFields(zoneWideNonDedicatedVlans, "id", "vlanTag", "ipRange"))); + + return zoneWideNonDedicatedVlans; + } + + /** + * Returns dedicated VLANs for account. + */ + protected List retrieveDedicatedVlans(long accountId, String accountDescription) { + List dedicatedVlans = new ArrayList<>(); + + s_logger.trace(String.format("Listing dedicated VLANs for account [%s].", accountDescription)); + dedicatedVlans = _vlanDao.listDedicatedVlans(accountId); + + s_logger.debug(String.format("Listed [%s] dedicated VLANs for account [%s]. Dedicated VLANs: [%s].", dedicatedVlans.size(), accountDescription, + ReflectionToStringBuilderUtils.reflectOnlySelectedFields(dedicatedVlans, "id", "vlanTag", "ipRange"))); + + return dedicatedVlans; + } + + protected Boolean getUseSystemPublicIpsValueIn(long accountId) { + return IpAddressManager.UseSystemPublicIps.valueIn(accountId); + } + @Override public Pair, Integer> listGuestOSByCriteria(final ListGuestOsCmd cmd) { final Filter searchFilter = new Filter(GuestOSVO.class, "displayName", true, cmd.getStartIndex(), cmd.getPageSizeVal()); diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index 488c5f5717a0..608cbe63250f 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -16,12 +16,20 @@ // under the License. package com.cloud.server; +import static org.junit.Assert.assertArrayEquals; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.any; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.apache.cloudstack.api.command.user.ssh.RegisterSSHKeyPairCmd; +import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -29,6 +37,8 @@ import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; +import com.cloud.dc.VlanVO; +import com.cloud.dc.dao.VlanDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.user.Account; import com.cloud.user.SSHKeyPair; @@ -45,7 +55,7 @@ public class ManagementServerImplTest { SSHKeyPairVO existingPair; @Mock - Account account; + Account accountMock; @Mock SSHKeyPairDao sshKeyPairDao; @@ -55,43 +65,54 @@ public class ManagementServerImplTest { SSHKeyPair sshKeyPair; @Spy - ManagementServerImpl spy; + ManagementServerImpl managementServerImplSpy; + + @Mock + VlanDao vlanDaoMock; + + @Mock + VlanVO vlanVoMock1, vlanVoMock2; + + @Before + public void setup() { + managementServerImplSpy._vlanDao = vlanDaoMock; + } @Test(expected = InvalidParameterValueException.class) public void testDuplicateRegistraitons(){ String accountName = "account"; String publicKeyString = "ssh-rsa very public"; - String publicKeyMaterial = spy.getPublicKeyFromKeyKeyMaterial(publicKeyString); + String publicKeyMaterial = managementServerImplSpy.getPublicKeyFromKeyKeyMaterial(publicKeyString); - Mockito.lenient().doReturn(account).when(spy).getCaller(); - Mockito.lenient().doReturn(account).when(spy).getOwner(regCmd); + Mockito.lenient().doReturn(accountMock).when(managementServerImplSpy).getCaller(); + Mockito.lenient().doReturn(accountMock).when(managementServerImplSpy).getOwner(regCmd); - Mockito.doNothing().when(spy).checkForKeyByName(regCmd, account); + Mockito.doNothing().when(managementServerImplSpy).checkForKeyByName(regCmd, accountMock); Mockito.lenient().doReturn(accountName).when(regCmd).getAccountName(); Mockito.doReturn(publicKeyString).when(regCmd).getPublicKey(); Mockito.doReturn("name").when(regCmd).getName(); - spy._sshKeyPairDao = sshKeyPairDao; - Mockito.doReturn(1L).when(account).getAccountId(); - Mockito.doReturn(1L).when(account).getDomainId(); + managementServerImplSpy._sshKeyPairDao = sshKeyPairDao; + Mockito.doReturn(1L).when(accountMock).getAccountId(); + Mockito.doReturn(1L).when(accountMock).getDomainId(); Mockito.doReturn(Mockito.mock(SSHKeyPairVO.class)).when(sshKeyPairDao).persist(any(SSHKeyPairVO.class)); lenient().when(sshKeyPairDao.findByName(1L, 1L, "name")).thenReturn(null).thenReturn(null); when(sshKeyPairDao.findByPublicKey(1L, 1L, publicKeyMaterial)).thenReturn(null).thenReturn(existingPair); - spy.registerSSHKeyPair(regCmd); - spy.registerSSHKeyPair(regCmd); + managementServerImplSpy.registerSSHKeyPair(regCmd); + managementServerImplSpy.registerSSHKeyPair(regCmd); } @Test public void testSuccess(){ String accountName = "account"; String publicKeyString = "ssh-rsa very public"; - String publicKeyMaterial = spy.getPublicKeyFromKeyKeyMaterial(publicKeyString); + String publicKeyMaterial = managementServerImplSpy.getPublicKeyFromKeyKeyMaterial(publicKeyString); - Mockito.lenient().doReturn(1L).when(account).getAccountId(); - Mockito.doReturn(1L).when(account).getAccountId(); - spy._sshKeyPairDao = sshKeyPairDao; + Mockito.lenient().doReturn(1L).when(accountMock).getAccountId(); + Mockito.doReturn(1L).when(accountMock).getAccountId(); + managementServerImplSpy._sshKeyPairDao = sshKeyPairDao; //Mocking the DAO object functions - NO object found in DB @@ -102,9 +123,115 @@ public void testSuccess(){ //Mocking the User Params Mockito.doReturn(accountName).when(regCmd).getName(); Mockito.doReturn(publicKeyString).when(regCmd).getPublicKey(); - Mockito.doReturn(account).when(spy).getOwner(regCmd); + Mockito.doReturn(accountMock).when(managementServerImplSpy).getOwner(regCmd); + + managementServerImplSpy.registerSSHKeyPair(regCmd); + Mockito.verify(managementServerImplSpy, Mockito.times(3)).getPublicKeyFromKeyKeyMaterial(anyString()); + } + + @Test + public void validateRetrieveDedicatedVlans() { + List expectedResult = Arrays.asList(vlanVoMock1); + + Mockito.doReturn(expectedResult).when(vlanDaoMock).listDedicatedVlans(Mockito.anyLong()); + + List result = managementServerImplSpy.retrieveDedicatedVlans(0, ""); + + Assert.assertEquals(expectedResult, result); + Mockito.verify(vlanDaoMock).listDedicatedVlans(Mockito.anyLong()); + } + + @Test + public void validateRetrieveZoneWideNonDedicatedVlansAccountWithoutAccessToPublicIps() { + List expectedResult = new ArrayList<>(); + + Mockito.doReturn(false).when(managementServerImplSpy).getUseSystemPublicIpsValueIn(Mockito.anyLong()); + + List result = managementServerImplSpy.retrieveZoneWideNonDedicatedVlans(0, "", 0); + + Assert.assertEquals(expectedResult, result); + Mockito.verify(vlanDaoMock, never()).listZoneWideNonDedicatedVlans(Mockito.anyLong()); + } + + @Test + public void validateRetrieveZoneWideNonDedicatedVlansAccountWithAccessToPublicIps() { + List expectedResult = Arrays.asList(vlanVoMock2); + + Mockito.doReturn(true).when(managementServerImplSpy).getUseSystemPublicIpsValueIn(Mockito.anyLong()); + Mockito.doReturn(expectedResult).when(vlanDaoMock).listZoneWideNonDedicatedVlans(Mockito.anyLong()); + + List result = managementServerImplSpy.retrieveZoneWideNonDedicatedVlans(0, "", 0); + + Assert.assertEquals(expectedResult, result); + Mockito.verify(vlanDaoMock).listZoneWideNonDedicatedVlans(Mockito.anyLong()); + } + + @Test + public void validateRetrieveAvailableVlanDbIdsForAccountWithNonDedicatedAndDedicatedVlans() { + List expectedResult = Arrays.asList(1l, 2l); + Mockito.doReturn(expectedResult.get(0)).when(vlanVoMock1).getId(); + Mockito.doReturn(expectedResult.get(1)).when(vlanVoMock2).getId(); + Mockito.doReturn(1l).when(accountMock).getId(); + + Mockito.doReturn(accountMock).when(managementServerImplSpy).getCaller(); + Mockito.doReturn(Arrays.asList(vlanVoMock1)).when(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.doReturn(Arrays.asList(vlanVoMock2)).when(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + + List result = managementServerImplSpy.retrieveAvailableVlanDbIdsForAccount(1l); + + Mockito.verify(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.verify(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + assertArrayEquals(expectedResult.toArray(), result.toArray()); + } + + @Test + public void validateRetrieveAvailableVlanDbIdsForAccountWithNonDedicatedAndWithoutDedicatedVlans() { + List expectedResult = Arrays.asList(1l); + Mockito.doReturn(expectedResult.get(0)).when(vlanVoMock1).getId(); + Mockito.doReturn(1l).when(accountMock).getId(); + + Mockito.doReturn(accountMock).when(managementServerImplSpy).getCaller(); + Mockito.doReturn(Arrays.asList(vlanVoMock1)).when(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.doReturn(new ArrayList<>()).when(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + + List result = managementServerImplSpy.retrieveAvailableVlanDbIdsForAccount(1l); + + Mockito.verify(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.verify(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + assertArrayEquals(expectedResult.toArray(), result.toArray()); + } + + @Test + public void validateRetrieveAvailableVlanDbIdsForAccountWithoutNonDedicatedAndWithDedicatedVlans() { + List expectedResult = Arrays.asList(2l); + Mockito.doReturn(expectedResult.get(0)).when(vlanVoMock2).getId(); + Mockito.doReturn(1l).when(accountMock).getId(); + + + Mockito.doReturn(accountMock).when(managementServerImplSpy).getCaller(); + Mockito.doReturn(new ArrayList<>()).when(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.doReturn(Arrays.asList(vlanVoMock2)).when(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + + List result = managementServerImplSpy.retrieveAvailableVlanDbIdsForAccount(1l); + + Mockito.verify(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.verify(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + assertArrayEquals(expectedResult.toArray(), result.toArray()); + } + + @Test + public void validateRetrieveAvailableVlanDbIdsForAccountWithoutNonDedicatedAndDedicatedVlans() { + List expectedResult = Arrays.asList(-1l); + Mockito.doReturn(1l).when(accountMock).getId(); + + Mockito.doReturn(accountMock).when(managementServerImplSpy).getCaller(); + Mockito.doReturn(new ArrayList<>()).when(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.doReturn(new ArrayList<>()).when(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + + List result = managementServerImplSpy.retrieveAvailableVlanDbIdsForAccount(1l); - spy.registerSSHKeyPair(regCmd); - Mockito.verify(spy, Mockito.times(3)).getPublicKeyFromKeyKeyMaterial(anyString()); + Mockito.verify(managementServerImplSpy).retrieveZoneWideNonDedicatedVlans(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong()); + Mockito.verify(managementServerImplSpy).retrieveDedicatedVlans(Mockito.anyLong(), Mockito.anyString()); + assertArrayEquals(expectedResult.toArray(), result.toArray()); } }