You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by sh...@apache.org on 2023/11/28 08:13:26 UTC
(cloudstack) branch main updated: Fix create private gateway rollback (#8244)
This is an automated email from the ASF dual-hosted git repository.
shwstppr 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 60b399f8758 Fix create private gateway rollback (#8244)
60b399f8758 is described below
commit 60b399f8758dd6e057fa887317e44c3b0791a0a1
Author: sato03 <he...@gmail.com>
AuthorDate: Tue Nov 28 05:13:18 2023 -0300
Fix create private gateway rollback (#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.
---
.../java/com/cloud/network/vpc/VpcManagerImpl.java | 39 +++++++++++++++-------
.../com/cloud/network/vpc/VpcManagerImplTest.java | 23 +++++++++++++
2 files changed, 50 insertions(+), 12 deletions(-)
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 b4cb73f035d..6bc70a9cef4 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 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
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 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
_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 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
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 b802d1fb355..deffb165f29 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.element.NetworkElement;
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 class VpcManagerImplTest {
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 class VpcManagerImplTest {
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));
+ }
+
}