From 26eaae78723727556aec7afbdd06e97006735443 Mon Sep 17 00:00:00 2001 From: Stephan Krug Date: Tue, 31 Jan 2023 04:42:57 -0300 Subject: [PATCH] Allow VPC offering creation only with active VR service offerings (#6957) --- .../com/cloud/network/NetworkService.java | 3 + .../ConfigurationManagerImpl.java | 8 +-- .../com/cloud/network/NetworkServiceImpl.java | 25 ++++++++ .../com/cloud/network/vpc/VpcManagerImpl.java | 4 ++ .../cloud/network/NetworkServiceImplTest.java | 63 ++++++++++++++++++- .../cloud/network/vpc/VpcManagerImplTest.java | 5 ++ .../com/cloud/vpc/MockNetworkManagerImpl.java | 4 ++ 7 files changed, 102 insertions(+), 10 deletions(-) diff --git a/api/src/main/java/com/cloud/network/NetworkService.java b/api/src/main/java/com/cloud/network/NetworkService.java index 85e25ba7732..e329f45acfb 100644 --- a/api/src/main/java/com/cloud/network/NetworkService.java +++ b/api/src/main/java/com/cloud/network/NetworkService.java @@ -41,6 +41,7 @@ import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.network.Network.IpAddresses; import com.cloud.network.Network.Service; import com.cloud.network.Networks.TrafficType; @@ -241,4 +242,6 @@ public interface NetworkService { boolean removeNetworkPermissions(RemoveNetworkPermissionsCmd removeNetworkPermissionsCmd); boolean resetNetworkPermissions(ResetNetworkPermissionsCmd resetNetworkPermissionsCmd); + + void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(final Long serviceOfferingId) throws InvalidParameterValueException; } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 5e684a07ad6..271f9ad17cf 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -5885,13 +5885,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati final Long serviceOfferingId = cmd.getServiceOfferingId(); if (serviceOfferingId != null) { - final ServiceOfferingVO offering = _serviceOfferingDao.findById(serviceOfferingId); - if (offering == null) { - throw new InvalidParameterValueException("Cannot find specified service offering: " + serviceOfferingId); - } - if (!VirtualMachine.Type.DomainRouter.toString().equalsIgnoreCase(offering.getSystemVmType())) { - throw new InvalidParameterValueException("The specified service offering " + serviceOfferingId + " cannot be used by virtual router!"); - } + _networkSvc.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(serviceOfferingId); } // configure service provider map diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index adfde495d3e..c868144a306 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -41,6 +41,8 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.offering.ServiceOffering; +import com.cloud.service.dao.ServiceOfferingDao; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.alert.AlertService; @@ -247,6 +249,7 @@ import com.cloud.vm.dao.NicSecondaryIpVO; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.googlecode.ipv6.IPv6Address; +import com.cloud.service.ServiceOfferingVO; /** * NetworkServiceImpl implements NetworkService. @@ -395,6 +398,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C CommandSetupHelper commandSetupHelper; @Inject AgentManager agentManager; + @Inject + ServiceOfferingDao serviceOfferingDao; @Autowired @Qualifier("networkHelper") @@ -4136,6 +4141,26 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C } + public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(final Long serviceOfferingId) { + s_logger.debug(String.format("Validating if service offering [%s] is active, and if system VM is of Domain Router type.", serviceOfferingId)); + final ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(serviceOfferingId); + + if (serviceOffering == null) { + throw new InvalidParameterValueException(String.format("Could not find specified service offering [%s].", serviceOfferingId)); + } + + if (serviceOffering.getState() == ServiceOffering.State.Inactive) { + throw new InvalidParameterValueException(String.format("The specified service offering [%s] is inactive.", serviceOffering)); + } + + final String virtualMachineDomainRouterType = VirtualMachine.Type.DomainRouter.toString(); + if (!virtualMachineDomainRouterType.equalsIgnoreCase(serviceOffering.getSystemVmType())) { + throw new InvalidParameterValueException(String.format("The specified service offering [%s] is of type [%s]. Virtual routers can only be created with service offering " + + "of type [%s].", serviceOffering, serviceOffering.getSystemVmType(), virtualMachineDomainRouterType.toLowerCase())); + } + } + + public String generateVnetString(List vnetList) { Collections.sort(vnetList, new Comparator() { @Override diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 30c4aa367f8..9222520602f 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -438,6 +438,10 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } } + if (serviceOfferingId != null) { + _ntwkSvc.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(serviceOfferingId); + } + return createVpcOffering(vpcOfferingName, displayText, supportedServices, serviceProviderList, serviceCapabilityList, internetProtocol, serviceOfferingId, domainIds, zoneIds, (enable ? State.Enabled : State.Disabled)); diff --git a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java index 1dfecd9680d..fcf7ad3665a 100644 --- a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java +++ b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doReturn; import java.lang.reflect.Field; import java.lang.reflect.Modifier; @@ -61,7 +62,6 @@ import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.exception.InsufficientCapacityException; -import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; @@ -96,7 +96,10 @@ import com.cloud.vm.NicVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.NicDao; - +import com.cloud.offering.ServiceOffering; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.exception.InvalidParameterValueException; @PowerMockIgnore("javax.management.*") @RunWith(PowerMockRunner.class) @@ -154,6 +157,10 @@ public class NetworkServiceImplTest { AccountService accountService; @Mock NetworkHelper networkHelper; + @Mock + ServiceOfferingDao serviceOfferingDaoMock; + @Mock + ServiceOfferingVO serviceOfferingVoMock; @InjectMocks AccountManagerImpl accountManagerImpl; @@ -171,7 +178,8 @@ public class NetworkServiceImplTest { @Mock private Account accountMock; @InjectMocks - private NetworkServiceImpl service = new NetworkServiceImpl(); + NetworkServiceImpl service = new NetworkServiceImpl(); + private static final String VLAN_ID_900 = "900"; private static final String VLAN_ID_901 = "901"; private static final String VLAN_ID_902 = "902"; @@ -200,6 +208,8 @@ public class NetworkServiceImplTest { CallContext.register(user, account); } + Class expectedException = InvalidParameterValueException.class; + @Before public void setup() throws Exception { MockitoAnnotations.initMocks(this); @@ -241,6 +251,7 @@ public class NetworkServiceImplTest { Mockito.lenient().doNothing().when(accountMgr).checkAccess(accountMock, networkOffering, dc); Mockito.when(accountMgr.isRootAdmin(accountMock.getId())).thenReturn(true); } + @Test public void testGetPrivateVlanPairNoVlans() { Pair pair = service.getPrivateVlanPair(null, null, null); @@ -713,4 +724,50 @@ public class NetworkServiceImplTest { Assert.assertNull(networkVO.getIp6Dns1()); Assert.assertNull(networkVO.getIp6Dns2()); } + @Test + public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMustThrowInvalidParameterValueExceptionWhenServiceOfferingIsNull() { + doReturn(null).when(serviceOfferingDaoMock).findById(anyLong()); + + String expectedMessage = String.format("Could not find specified service offering [%s].", 1l); + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedException, () -> { + service.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMustThrowInvalidParameterValueExceptionWhenServiceOfferingStateIsInactive() { + doReturn(serviceOfferingVoMock).when(serviceOfferingDaoMock).findById(anyLong()); + doReturn(ServiceOffering.State.Inactive).when(serviceOfferingVoMock).getState(); + + String expectedMessage = String.format("The specified service offering [%s] is inactive.", serviceOfferingVoMock); + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedException, () -> { + service.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMustThrowInvalidParameterValueExceptionWhenSystemVmTypeIsNotDomainRouter() { + doReturn(serviceOfferingVoMock).when(serviceOfferingDaoMock).findById(anyLong()); + doReturn(ServiceOffering.State.Active).when(serviceOfferingVoMock).getState(); + doReturn(VirtualMachine.Type.ElasticLoadBalancerVm.toString()).when(serviceOfferingVoMock).getSystemVmType(); + + String expectedMessage = String.format("The specified service offering [%s] is of type [%s]. Virtual routers can only be created with service offering of type [%s].", + serviceOfferingVoMock, serviceOfferingVoMock.getSystemVmType(), VirtualMachine.Type.DomainRouter.toString().toLowerCase()); + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedException, () -> { + service.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMustNotThrowInvalidParameterValueExceptionWhenSystemVmTypeIsDomainRouter() { + NetworkServiceImpl networkServiceImplMock = mock(NetworkServiceImpl.class); + + networkServiceImplMock.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l); + } } diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 03d4c1659b3..c820026309d 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -41,6 +41,7 @@ import java.util.UUID; import com.cloud.alert.AlertManager; +import com.cloud.network.NetworkService; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.alert.AlertService; import org.apache.cloudstack.context.CallContext; @@ -146,6 +147,8 @@ public class VpcManagerImplTest { NicDao nicDao; @Mock AlertManager alertManager; + @Mock + NetworkService networkServiceMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -196,6 +199,7 @@ public class VpcManagerImplTest { manager._resourceLimitMgr = resourceLimitService; manager._vpcOffDao = vpcOfferingDao; manager._dcDao = dataCenterDao; + manager._ntwkSvc = networkServiceMock; CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); registerCallContext(); } @@ -418,6 +422,7 @@ public class VpcManagerImplTest { public void testDisabledConfigCreateIpv6VpcOffering() { CreateVPCOfferingCmd cmd = Mockito.mock(CreateVPCOfferingCmd.class); Mockito.when(cmd.getInternetProtocol()).thenReturn(NetUtils.InternetProtocol.DualStack.toString()); + Mockito.doNothing().when(networkServiceMock).validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(Mockito.any()); manager.createVpcOffering(cmd); } diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index c7cf7ecf3e5..bd0c73f54cb 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -1050,4 +1050,8 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches public boolean resetNetworkPermissions(ResetNetworkPermissionsCmd resetNetworkPermissionsCmd) { return false; } + + @Override + public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(final Long serviceOfferingId) { + } }