You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by we...@apache.org on 2022/04/19 21:15:22 UTC
[cloudstack] branch main updated: Allow users to view reserved System VM IPs, if they're already allocated to user (#5902)
This is an automated email from the ASF dual-hosted git repository.
weizhou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 4313c3def7e Allow users to view reserved System VM IPs, if they're already allocated to user (#5902)
4313c3def7e is described below
commit 4313c3def7edb456110254f271d9ca60c563ed7d
Author: SadiJr <sa...@gmail.com>
AuthorDate: Tue Apr 19 18:15:15 2022 -0300
Allow users to view reserved System VM IPs, if they're already allocated to user (#5902)
* Allow users to view reserved system VM IPs, if this IPs are already allocated to any user VM
* Fix checkstyle
* Address reviews
* Address reviews
* Apply @weizhouapache changes
Credits to @weizhouapache, and my sincere thanks for the help.
Co-authored-by: SadiJr <sa...@scclouds.com.br>
Co-authored-by: SadiJr <17...@firemailbox.club>
---
.../com/cloud/network/IpAddressManagerImpl.java | 6 +-
.../com/cloud/server/ManagementServerImpl.java | 7 +-
.../com/cloud/server/ManagementServerImplTest.java | 114 +++++++++++++++++++++
3 files changed, 124 insertions(+), 3 deletions(-)
diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
index b1186148578..dcd89fa5698 100644
--- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
@@ -306,7 +306,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
private static final ConfigKey<Boolean> SystemVmPublicIpReservationModeStrictness = new ConfigKey<Boolean>("Advanced",
Boolean.class, "system.vm.public.ip.reservation.mode.strictness", "false",
- "If enabled, the use of System VMs public IP reservation is strict, preferred if not.", false, ConfigKey.Scope.Global);
+ "If enabled, the use of System VMs public IP reservation is strict, preferred if not.", true, ConfigKey.Scope.Global);
private Random rand = new Random(System.currentTimeMillis());
@@ -2306,4 +2306,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
NetworkDetailVO networkDetail = _networkDetailsDao.findDetail(networkId, Network.hideIpAddressUsage);
return networkDetail != null && "true".equals(networkDetail.getValue());
}
+
+ public static ConfigKey<Boolean> getSystemvmpublicipreservationmodestrictness() {
+ return SystemVmPublicIpReservationModeStrictness;
+ }
}
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index f2e70e67b91..26d11eafb9b 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -666,6 +666,7 @@ import com.cloud.hypervisor.kvm.dpdk.DpdkHelper;
import com.cloud.info.ConsoleProxyInfo;
import com.cloud.network.IpAddress;
import com.cloud.network.IpAddressManager;
+import com.cloud.network.IpAddressManagerImpl;
import com.cloud.network.Network;
import com.cloud.network.NetworkModel;
import com.cloud.network.dao.IPAddressDao;
@@ -2395,7 +2396,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
}
}
- private void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
+ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
final Object keyword = cmd.getKeyword();
final Long physicalNetworkId = cmd.getPhysicalNetworkId();
final Long sourceNetworkId = cmd.getNetworkId();
@@ -2467,7 +2468,9 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
sc.setParameters("state", IpAddress.State.Allocated);
}
- sc.setParameters( "forsystemvms", false);
+ if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) {
+ sc.setParameters("forsystemvms", false);
+ }
}
@Override
diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java
index 488c5f5717a..2d6969fe966 100644
--- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java
+++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java
@@ -21,23 +21,35 @@ import static org.mockito.Mockito.any;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.when;
+import org.apache.cloudstack.api.command.user.address.ListPublicIpAddressesCmd;
import org.apache.cloudstack.api.command.user.ssh.RegisterSSHKeyPairCmd;
+import org.apache.cloudstack.framework.config.ConfigKey;
+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 org.powermock.reflect.Whitebox;
+import com.cloud.dc.Vlan.VlanType;
import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.network.IpAddress;
+import com.cloud.network.IpAddressManagerImpl;
+import com.cloud.network.dao.IPAddressVO;
import com.cloud.user.Account;
import com.cloud.user.SSHKeyPair;
import com.cloud.user.SSHKeyPairVO;
import com.cloud.user.dao.SSHKeyPairDao;
+import com.cloud.utils.db.SearchCriteria;
@RunWith(MockitoJUnitRunner.class)
public class ManagementServerImplTest {
+ @Mock
+ SearchCriteria<IPAddressVO> sc;
+
@Mock
RegisterSSHKeyPairCmd regCmd;
@@ -54,9 +66,20 @@ public class ManagementServerImplTest {
@Mock
SSHKeyPair sshKeyPair;
+ @Mock
+ IpAddressManagerImpl ipAddressManagerImpl;
+
@Spy
ManagementServerImpl spy;
+ ConfigKey mockConfig;
+
+ @Before
+ public void setup() {
+ mockConfig = Mockito.mock(ConfigKey.class);
+ Whitebox.setInternalState(ipAddressManagerImpl.getClass(), "SystemVmPublicIpReservationModeStrictness", mockConfig);
+ }
+
@Test(expected = InvalidParameterValueException.class)
public void testDuplicateRegistraitons(){
String accountName = "account";
@@ -107,4 +130,95 @@ public class ManagementServerImplTest {
spy.registerSSHKeyPair(regCmd);
Mockito.verify(spy, Mockito.times(3)).getPublicKeyFromKeyKeyMaterial(anyString());
}
+
+ @Test
+ public void setParametersTestWhenStateIsFreeAndSystemVmPublicIsTrue() throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException {
+ Mockito.when(mockConfig.value()).thenReturn(Boolean.TRUE);
+
+ ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
+ Mockito.when(cmd.getNetworkId()).thenReturn(10L);
+ Mockito.when(cmd.getZoneId()).thenReturn(null);
+ Mockito.when(cmd.getIpAddress()).thenReturn(null);
+ Mockito.when(cmd.getPhysicalNetworkId()).thenReturn(null);
+ Mockito.when(cmd.getVlanId()).thenReturn(null);
+ Mockito.when(cmd.getId()).thenReturn(null);
+ Mockito.when(cmd.isSourceNat()).thenReturn(null);
+ Mockito.when(cmd.isStaticNat()).thenReturn(null);
+ Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name());
+ Mockito.when(cmd.getTags()).thenReturn(null);
+ spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE);
+
+ Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free");
+ Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
+ }
+
+ @Test
+ public void setParametersTestWhenStateIsFreeAndSystemVmPublicIsFalse() {
+ Mockito.when(mockConfig.value()).thenReturn(Boolean.FALSE);
+ ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
+ Mockito.when(cmd.getNetworkId()).thenReturn(10L);
+ Mockito.when(cmd.getZoneId()).thenReturn(null);
+ Mockito.when(cmd.getIpAddress()).thenReturn(null);
+ Mockito.when(cmd.getPhysicalNetworkId()).thenReturn(null);
+ Mockito.when(cmd.getVlanId()).thenReturn(null);
+ Mockito.when(cmd.getId()).thenReturn(null);
+ Mockito.when(cmd.isSourceNat()).thenReturn(null);
+ Mockito.when(cmd.isStaticNat()).thenReturn(null);
+ Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name());
+ Mockito.when(cmd.getTags()).thenReturn(null);
+ spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE);
+
+ Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free");
+ Mockito.verify(sc, Mockito.never()).setParameters("forsystemvms", false);
+ }
+
+ @Test
+ public void setParametersTestWhenStateIsNullAndSystemVmPublicIsFalse() {
+ Mockito.when(mockConfig.value()).thenReturn(Boolean.FALSE);
+ ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
+ Mockito.when(cmd.getNetworkId()).thenReturn(10L);
+ Mockito.when(cmd.getZoneId()).thenReturn(null);
+ Mockito.when(cmd.getIpAddress()).thenReturn(null);
+ Mockito.when(cmd.getPhysicalNetworkId()).thenReturn(null);
+ Mockito.when(cmd.getVlanId()).thenReturn(null);
+ Mockito.when(cmd.getId()).thenReturn(null);
+ Mockito.when(cmd.isSourceNat()).thenReturn(null);
+ Mockito.when(cmd.isStaticNat()).thenReturn(null);
+ Mockito.when(cmd.getState()).thenReturn(null);
+ Mockito.when(cmd.getTags()).thenReturn(null);
+ spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE);
+
+ Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
+ Mockito.verify(sc, Mockito.never()).setParameters("forsystemvms", false);
+ }
+
+ @Test
+ public void setParametersTestWhenStateIsNullAndSystemVmPublicIsTrue() {
+ Mockito.when(mockConfig.value()).thenReturn(Boolean.TRUE);
+ ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
+ Mockito.when(cmd.getNetworkId()).thenReturn(10L);
+ Mockito.when(cmd.getZoneId()).thenReturn(null);
+ Mockito.when(cmd.getIpAddress()).thenReturn(null);
+ Mockito.when(cmd.getPhysicalNetworkId()).thenReturn(null);
+ Mockito.when(cmd.getVlanId()).thenReturn(null);
+ Mockito.when(cmd.getId()).thenReturn(null);
+ Mockito.when(cmd.isSourceNat()).thenReturn(null);
+ Mockito.when(cmd.isStaticNat()).thenReturn(null);
+ Mockito.when(cmd.getState()).thenReturn(null);
+ Mockito.when(cmd.getTags()).thenReturn(null);
+ spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE);
+
+ Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
+ Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
+ Mockito.verify(sc, Mockito.never()).setParameters("forsystemvms", false);
+ }
}