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 b4cb73f035d7..6bc70a9cef4f 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.network.vpc; +import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -2161,6 +2162,8 @@ private PrivateGateway createVpcPrivateGateway(final long vpcId, Long physicalNe final PhysicalNetwork physNetFinal = physNet; VpcGatewayVO gatewayVO = null; try { + validateVpcPrivateGatewayAclId(vpcId, aclId); + s_logger.debug("Creating Private gateway for VPC " + vpc); // 1) create private network unless it is existing and // lswitch'd @@ -2200,18 +2203,7 @@ private PrivateGateway createVpcPrivateGateway(final long vpcId, Long physicalNe _dcDao.update(dc.getId(), dc); } - long networkAclId = NetworkACL.DEFAULT_DENY; - if (aclId != null) { - final NetworkACLVO aclVO = _networkAclDao.findById(aclId); - if (aclVO == null) { - throw new InvalidParameterValueException("Invalid network acl id passed "); - } - if (aclVO.getVpcId() != vpcId && !(aclId == NetworkACL.DEFAULT_DENY || aclId == NetworkACL.DEFAULT_ALLOW)) { - throw new InvalidParameterValueException("Private gateway and network acl are not in the same vpc"); - } - - networkAclId = aclId; - } + Long networkAclId = ObjectUtils.defaultIfNull(aclId, NetworkACL.DEFAULT_DENY); { // experimental block, this is a hack // set vpc id in network to null @@ -2244,6 +2236,29 @@ private PrivateGateway createVpcPrivateGateway(final long vpcId, Long physicalNe return getVpcPrivateGateway(gatewayVO.getId()); } + /** + * This method checks if the ACL that is being used to create the private gateway is valid. First, the aclId is used to search for a {@link NetworkACLVO} object + * by calling the {@link NetworkACLDao#findById(Serializable)} method. If the object is null, an {@link InvalidParameterValueException} exception is thrown. + * Secondly, we check if the ACL and the private gateway are in the same VPC and an {@link InvalidParameterValueException} is thrown if they are not. + * + * @param vpcId Private gateway VPC ID. + * @param aclId Private gateway ACL ID. + * @throws InvalidParameterValueException + */ + protected void validateVpcPrivateGatewayAclId(long vpcId, Long aclId) { + if (aclId == null) { + return; + } + + final NetworkACLVO aclVO = _networkAclDao.findById(aclId); + if (aclVO == null) { + throw new InvalidParameterValueException("Invalid network acl id passed."); + } + if (aclVO.getVpcId() != vpcId && !(aclId == NetworkACL.DEFAULT_DENY || aclId == NetworkACL.DEFAULT_ALLOW)) { + throw new InvalidParameterValueException("Private gateway and network acl are not in the same vpc."); + } + } + private void validateVpcPrivateGatewayAssociateNetworkId(NetworkOfferingVO ntwkOff, String broadcastUri, Long associatedNetworkId, Boolean bypassVlanOverlapCheck) { // Validate vlanId and associatedNetworkId if (broadcastUri == null && associatedNetworkId == null) { 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 b802d1fb3554..deffb165f29f 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -47,6 +47,7 @@ import com.cloud.network.router.CommandSetupHelper; import com.cloud.network.router.NetworkHelper; import com.cloud.network.router.VirtualRouter; +import com.cloud.network.vpc.dao.NetworkACLDao; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; @@ -127,6 +128,8 @@ public class VpcManagerImplTest { @Mock NetworkDao networkDao; @Mock + NetworkACLDao networkACLDaoMock; + @Mock NetworkModel networkModel; @Mock NetworkOfferingServiceMapDao networkOfferingServiceMapDao; @@ -150,6 +153,8 @@ public class VpcManagerImplTest { NetworkService networkServiceMock; @Mock FirewallRulesDao firewallDao; + @Mock + NetworkACLVO networkACLVOMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -168,6 +173,9 @@ public class VpcManagerImplTest { final Long vpcOwnerId = 1L; final String vpcName = "Test-VPC"; final String vpcDomain = "domain"; + final Long aclId = 1L; + final Long differentVpcAclId = 3L; + final Long vpcId = 1L; private AutoCloseable closeable; @@ -203,6 +211,7 @@ public void setup() throws NoSuchFieldException, IllegalAccessException { manager._dcDao = dataCenterDao; manager._ntwkSvc = networkServiceMock; manager._firewallDao = firewallDao; + manager._networkAclDao = networkACLDaoMock; CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); registerCallContext(); overrideDefaultConfigValue(NetworkService.AllowUsersToSpecifyVRMtu, "_defaultValue", "false"); @@ -487,4 +496,18 @@ public void testCreateVpcDnsIpv6OfferingFailure() { Assert.fail(String.format("failure with exception: %s", e.getMessage())); } } + + @Test + public void validateVpcPrivateGatewayAclIdTestNullAclVoThrowsInvalidParameterValueException() { + Mockito.doReturn(null).when(networkACLDaoMock).findById(aclId); + Assert.assertThrows(InvalidParameterValueException.class, () -> manager.validateVpcPrivateGatewayAclId(vpcId, aclId)); + } + + @Test + public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParameterValueException() { + Mockito.doReturn(2L).when(networkACLVOMock).getVpcId(); + Mockito.doReturn(networkACLVOMock).when(networkACLDaoMock).findById(differentVpcAclId); + Assert.assertThrows(InvalidParameterValueException.class, () -> manager.validateVpcPrivateGatewayAclId(vpcId, differentVpcAclId)); + } + }