Fix create private gateway rollback (apache#8244)

When creating a private gateway, if an ACL verification error occurs, the changes made up to that point are not rolled back, resulting in inconsistent records in the database.

This PR intends to fix this bug and, if an error occurs during the creation of the private gateway, the changes will be rolled back.

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) {

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));
+    }
+
 }