Skip to content

Filter dedicated and non-dedicated VLAN in API listPublicIPAddresses #5533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
GutoVeronezi wants to merge 3 commits into from
Closed

Filter dedicated and non-dedicated VLAN in API listPublicIPAddresses #5533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94919b4
Filter dedicated and non-dedicated VLAN in API listPublicIPAddresses
Sep 24, 2021
dc0b07c
Fix cherry-pick conflicts
Oct 5, 2021
e03015d
Fix failures
Oct 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 75 additions & 5 deletions server/src/main/java/com/cloud/server/ManagementServerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2177,9 +2178,9 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
Boolean isRecursive = null;
final List<Long> permittedAccounts = new ArrayList<>();
ListProjectResourcesCriteria listProjectResourcesCriteria = null;

if (isAllocated || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.ACCOUNT_TYPE_ADMIN || cmd.getDomainId() != null))) {
final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(),
null);
final Ternary<Long, Boolean, ListProjectResourcesCriteria> 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();
Expand Down Expand Up @@ -2273,7 +2274,7 @@ private void buildParameters(final SearchBuilder<IPAddressVO> 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);
Expand Down Expand Up @@ -2367,7 +2368,9 @@ private void setParameters(SearchCriteria<IPAddressVO> 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);
}

Expand All @@ -2390,6 +2393,73 @@ private void setParameters(SearchCriteria<IPAddressVO> 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 <b>true</b> for the account.<br/>
* 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 <b>listPublicIPAddresses</b>, it will retrieve all the VLANs for the zone, even the dedicated to another accounts.
*/
protected List<Long> retrieveAvailableVlanDbIdsForAccount(Long zoneId) {
List<Long> vlanDbIdsToRetrieve = new ArrayList<>();
List<VlanVO> 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 <b>true</b> for the account.
*/
protected List<VlanVO> retrieveZoneWideNonDedicatedVlans(long accountId, String accountDescription, long zoneId) {
List<VlanVO> 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<VlanVO> retrieveDedicatedVlans(long accountId, String accountDescription) {
List<VlanVO> 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<List<? extends GuestOS>, Integer> listGuestOSByCriteria(final ListGuestOsCmd cmd) {
final Filter searchFilter = new Filter(GuestOSVO.class, "displayName", true, cmd.getStartIndex(), cmd.getPageSizeVal());
Expand Down
163 changes: 145 additions & 18 deletions server/src/test/java/com/cloud/server/ManagementServerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,29 @@
// 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;
import org.mockito.Mockito;
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;
Expand All @@ -45,7 +55,7 @@ public class ManagementServerImplTest {
SSHKeyPairVO existingPair;

@Mock
Account account;
Account accountMock;

@Mock
SSHKeyPairDao sshKeyPairDao;
Expand All @@ -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
Expand All @@ -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<VlanVO> expectedResult = Arrays.asList(vlanVoMock1);

Mockito.doReturn(expectedResult).when(vlanDaoMock).listDedicatedVlans(Mockito.anyLong());

List<VlanVO> result = managementServerImplSpy.retrieveDedicatedVlans(0, "");

Assert.assertEquals(expectedResult, result);
Mockito.verify(vlanDaoMock).listDedicatedVlans(Mockito.anyLong());
}

@Test
public void validateRetrieveZoneWideNonDedicatedVlansAccountWithoutAccessToPublicIps() {
List<VlanVO> expectedResult = new ArrayList<>();

Mockito.doReturn(false).when(managementServerImplSpy).getUseSystemPublicIpsValueIn(Mockito.anyLong());

List<VlanVO> result = managementServerImplSpy.retrieveZoneWideNonDedicatedVlans(0, "", 0);

Assert.assertEquals(expectedResult, result);
Mockito.verify(vlanDaoMock, never()).listZoneWideNonDedicatedVlans(Mockito.anyLong());
}

@Test
public void validateRetrieveZoneWideNonDedicatedVlansAccountWithAccessToPublicIps() {
List<VlanVO> expectedResult = Arrays.asList(vlanVoMock2);

Mockito.doReturn(true).when(managementServerImplSpy).getUseSystemPublicIpsValueIn(Mockito.anyLong());
Mockito.doReturn(expectedResult).when(vlanDaoMock).listZoneWideNonDedicatedVlans(Mockito.anyLong());

List<VlanVO> result = managementServerImplSpy.retrieveZoneWideNonDedicatedVlans(0, "", 0);

Assert.assertEquals(expectedResult, result);
Mockito.verify(vlanDaoMock).listZoneWideNonDedicatedVlans(Mockito.anyLong());
}

@Test
public void validateRetrieveAvailableVlanDbIdsForAccountWithNonDedicatedAndDedicatedVlans() {
List<Long> 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<Long> 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<Long> 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<Long> 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<Long> 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<Long> 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<Long> 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<Long> 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());
}
}