Add unit tests for detectConflicts method and smoke tests

This commit is contained in:
nvazquez 2026-02-24 19:39:00 -03:00
parent a0755a143e
commit 6d33ef1221
No known key found for this signature in database
GPG Key ID: 656E1BCC8CB54F84
4 changed files with 381 additions and 36 deletions

View File

@ -400,17 +400,9 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
assert (rules.size() >= 1);
}
NetworkVO newRuleNetwork = _networkDao.findById(newRule.getNetworkId());
if (newRuleNetwork == null) {
throw new InvalidParameterValueException("Unable to create firewall rule as cannot find network by id=" + newRule.getNetworkId());
}
boolean isNewRuleOnVpcNetwork = newRuleNetwork.getVpcId() != null;
boolean isVpcConserveModeEnabled = false;
if (isNewRuleOnVpcNetwork) {
Vpc vpc = _vpcMgr.getActiveVpc(newRuleNetwork.getVpcId());
VpcOfferingVO vpcOffering = vpc != null ? vpcOfferingDao.findById(vpc.getVpcOfferingId()) : null;
isVpcConserveModeEnabled = vpcOffering != null && vpcOffering.isConserveMode();
}
NetworkVO newRuleNetwork = getNewRuleNetwork(newRule);
boolean newRuleIsOnVpcNetwork = isNewRuleOnVpcNetwork(newRuleNetwork);
boolean vpcConserveModeEnabled = isVpcConserveModeEnabled(newRuleNetwork);
for (FirewallRuleVO rule : rules) {
if (rule.getId() == newRule.getId()) {
@ -461,10 +453,10 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
// Checking if the rule applied is to the same network that is passed in the rule.
// (except for VPCs with conserve mode = true)
if ((!isNewRuleOnVpcNetwork || !isVpcConserveModeEnabled)
if ((!newRuleIsOnVpcNetwork || !vpcConserveModeEnabled)
&& rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) {
String errMsg = String.format("New rule is for a different network than what's specified in rule %s", rule.getXid());
if (isNewRuleOnVpcNetwork) {
if (newRuleIsOnVpcNetwork) {
errMsg += String.format(" - VPC id=%s is not using conserve mode", newRuleNetwork.getVpcId());
}
throw new NetworkRuleConflictException(errMsg);
@ -516,6 +508,27 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
}
}
protected boolean isVpcConserveModeEnabled(NetworkVO newRuleNetwork) {
if (isNewRuleOnVpcNetwork(newRuleNetwork)) {
Vpc vpc = _vpcMgr.getActiveVpc(newRuleNetwork.getVpcId());
VpcOfferingVO vpcOffering = vpc != null ? vpcOfferingDao.findById(vpc.getVpcOfferingId()) : null;
return vpcOffering != null && vpcOffering.isConserveMode();
}
return false;
}
protected boolean isNewRuleOnVpcNetwork(NetworkVO newRuleNetwork) {
return newRuleNetwork.getVpcId() != null;
}
protected NetworkVO getNewRuleNetwork(FirewallRule newRule) {
NetworkVO newRuleNetwork = _networkDao.findById(newRule.getNetworkId());
if (newRuleNetwork == null) {
throw new InvalidParameterValueException("Unable to create firewall rule as cannot find network by id=" + newRule.getNetworkId());
}
return newRuleNetwork;
}
protected boolean checkIfRulesHaveConflictingPortRanges(FirewallRule newRule, FirewallRule rule, boolean oneOfRulesIsFirewall, boolean bothRulesFirewall, boolean bothRulesPortForwarding, boolean duplicatedCidrs) {
String rulesAsString = String.format("[%s] and [%s]", rule, newRule);

View File

@ -32,7 +32,10 @@ import com.cloud.network.element.VpcVirtualRouterElement;
import com.cloud.network.rules.FirewallRule;
import com.cloud.network.rules.FirewallRule.Purpose;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.network.vpc.Vpc;
import com.cloud.network.vpc.VpcManager;
import com.cloud.network.vpc.VpcOfferingVO;
import com.cloud.network.vpc.dao.VpcOfferingDao;
import com.cloud.user.AccountManager;
import com.cloud.user.DomainManager;
import com.cloud.utils.component.ComponentContext;
@ -81,6 +84,8 @@ public class FirewallManagerTest {
FirewallRulesDao _firewallDao;
@Mock
NetworkDao _networkDao;
@Mock
VpcOfferingDao vpcOfferingDao;
@Spy
@InjectMocks
@ -168,54 +173,102 @@ public class FirewallManagerTest {
}
}
@Test
public void testDetectRulesConflict() {
List<FirewallRuleVO> ruleList = new ArrayList<FirewallRuleVO>();
FirewallRuleVO rule1 = spy(new FirewallRuleVO("rule1", 3, 500, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null));
FirewallRuleVO rule2 = spy(new FirewallRuleVO("rule2", 3, 1701, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null));
FirewallRuleVO rule3 = spy(new FirewallRuleVO("rule3", 3, 4500, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null));
private List<FirewallRuleVO> createExistingFirewallListRulesList(long existingNetworkId) {
List<FirewallRuleVO> ruleList = new ArrayList<>();
FirewallRuleVO rule1 = spy(new FirewallRuleVO("rule1", 3, 500, "UDP", existingNetworkId, 2, 1, Purpose.Vpn, null, null, null, null));
FirewallRuleVO rule2 = spy(new FirewallRuleVO("rule2", 3, 1701, "UDP", existingNetworkId, 2, 1, Purpose.Vpn, null, null, null, null));
FirewallRuleVO rule3 = spy(new FirewallRuleVO("rule3", 3, 4500, "UDP", existingNetworkId, 2, 1, Purpose.Vpn, null, null, null, null));
List<String> sString = Arrays.asList("10.1.1.1/24","192.168.1.1/24");
List<String> dString1 = Arrays.asList("10.1.1.1/25");
List<String> dString2 = Arrays.asList("10.1.1.128/25");
FirewallRuleVO rule4 = spy(new FirewallRuleVO("rule4", 3L, 10, 20, "TCP", 1, 2, 1, Purpose.Firewall, sString, dString1, null, null,
FirewallRuleVO rule4 = spy(new FirewallRuleVO("rule4", 3L, 10, 20, "TCP", existingNetworkId, 2, 1, Purpose.Firewall, sString, dString1, null, null,
null, FirewallRule.TrafficType.Egress));
when(rule1.getId()).thenReturn(1L);
when(rule2.getId()).thenReturn(2L);
when(rule3.getId()).thenReturn(3L);
when(rule4.getId()).thenReturn(4L);
ruleList.add(rule1);
ruleList.add(rule2);
ruleList.add(rule3);
ruleList.add(rule4);
FirewallManagerImpl firewallMgr = (FirewallManagerImpl)_firewallMgr;
return ruleList;
}
when(firewallMgr._firewallDao.listByIpAndPurposeAndNotRevoked(3,null)).thenReturn(ruleList);
when(rule1.getId()).thenReturn(1L);
when(rule2.getId()).thenReturn(2L);
when(rule3.getId()).thenReturn(3L);
when(rule4.getId()).thenReturn(4L);
private List<FirewallRule> createNewRuleList(long newNetworkId) {
List<String> sString = Arrays.asList("10.1.1.1/24","192.168.1.1/24");
List<String> dString2 = Arrays.asList("10.1.1.128/25");
FirewallRule newRule1 = new FirewallRuleVO("newRule1", 3, 500, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule2 = new FirewallRuleVO("newRule2", 3, 1701, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule3 = new FirewallRuleVO("newRule3", 3, 4500, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule4 = new FirewallRuleVO("newRule4", 3L, 15, 25, "TCP", 1, 2, 1, Purpose.Firewall, sString, dString2, null, null,
FirewallRule newRule1 = new FirewallRuleVO("newRule1", 3, 500, "TCP", newNetworkId, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule2 = new FirewallRuleVO("newRule2", 3, 1701, "TCP", newNetworkId, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule3 = new FirewallRuleVO("newRule3", 3, 4500, "TCP", newNetworkId, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule4 = new FirewallRuleVO("newRule4", 3L, 15, 25, "TCP", newNetworkId, 2, 1, Purpose.Firewall, sString, dString2, null, null,
null, FirewallRule.TrafficType.Egress);
return Arrays.asList(newRule1, newRule2, newRule3, newRule4);
}
@Test
public void testDetectRulesConflictIsolatedNetwork() {
List<FirewallRuleVO> ruleList = createExistingFirewallListRulesList(1L);
when(_firewallMgr._firewallDao.listByIpAndPurposeAndNotRevoked(3,null)).thenReturn(ruleList);
List<FirewallRule> newRuleList = createNewRuleList(1L);
NetworkVO networkVO = Mockito.mock(NetworkVO.class);
when(firewallMgr._networkDao.findById(1L)).thenReturn(networkVO);
when(_firewallMgr._networkDao.findById(1L)).thenReturn(networkVO);
when(networkVO.getVpcId()).thenReturn(null);
try {
firewallMgr.detectRulesConflict(newRule1);
firewallMgr.detectRulesConflict(newRule2);
firewallMgr.detectRulesConflict(newRule3);
firewallMgr.detectRulesConflict(newRule4);
for (FirewallRule newRule : newRuleList) {
_firewallMgr.detectRulesConflict(newRule);
}
}
catch (NetworkRuleConflictException ex) {
Assert.fail();
}
}
private void testDetectRulesConflictVpcBase(boolean vpcConserveMode) throws NetworkRuleConflictException {
long existingNetworkId = 1L;
long newNetworkId = 2L;
long vpcId = 10L;
List<FirewallRuleVO> ruleList = createExistingFirewallListRulesList(existingNetworkId);
when(_firewallMgr._firewallDao.listByIpAndPurposeAndNotRevoked(3,null)).thenReturn(ruleList);
List<FirewallRule> newRuleList = createNewRuleList(newNetworkId);
NetworkVO newNetworkVO = Mockito.mock(NetworkVO.class);
Vpc vpc = Mockito.mock(Vpc.class);
VpcOfferingVO vpcOffering = Mockito.mock(VpcOfferingVO.class);
when(_firewallMgr._networkDao.findById(2L)).thenReturn(newNetworkVO);
when(newNetworkVO.getVpcId()).thenReturn(vpcId);
when(_vpcMgr.getActiveVpc(vpcId)).thenReturn(vpc);
when(vpc.getVpcOfferingId()).thenReturn(1L);
when(vpcOfferingDao.findById(1L)).thenReturn(vpcOffering);
when(vpcOffering.isConserveMode()).thenReturn(vpcConserveMode);
for (FirewallRule newRule : newRuleList) {
_firewallMgr.detectRulesConflict(newRule);
}
}
@Test
public void testDetectRulesConflictVpcConserveMode() throws NetworkRuleConflictException {
// When VPC conserve mode is enabled, rules can be created for multiple network tiers
testDetectRulesConflictVpcBase(true);
}
@Test(expected = NetworkRuleConflictException.class)
public void testDetectRulesConflictVpcConserveModeFalse() throws NetworkRuleConflictException {
// When VPC conserve mode is disabled, an exception should be thrown when attempting to create rules on different network tiers
testDetectRulesConflictVpcBase(false);
}
@Test
public void checkIfRulesHaveConflictingPortRangesTestOnlyOneRuleIsFirewallReturnsFalse()
{

View File

@ -0,0 +1,277 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Tests for VPC Conserve Mode (since 4.23.0)
Conserve mode allows public IP services (LB, Port Forwarding, Static NAT) to be
shared across multiple VPC tiers using the same public IP address.
When conserve mode is ON:
- A single public IP can have rules targeting VMs in different VPC tiers
- FirewallManagerImpl skips the cross-network conflict check for that VPC
When conserve mode is OFF (default before 4.23.0):
- Rules on a given public IP must all belong to the same VPC tier (network)
- Attempting to create a rule on a different tier than an existing rule raises
a NetworkRuleConflictException
"""
from marvin.cloudstackException import CloudstackAPIException
from marvin.cloudstackTestCase import cloudstackTestCase
from marvin.codes import FAILED
from marvin.lib.base import (
Account,
LoadBalancerRule,
NATRule,
Network,
NetworkOffering,
PublicIPAddress,
ServiceOffering,
VirtualMachine,
VPC,
VpcOffering,
)
from marvin.lib.common import (
get_domain,
get_test_template,
get_zone,
list_publicIP
)
from marvin.lib.utils import cleanup_resources
from nose.plugins.attrib import attr
import logging
class TestVPCConserveModeRules(cloudstackTestCase):
"""Tests that conserve mode for VPC controls whether rules on the same public IP are allowed in multiple VPC tiers.
"""
@classmethod
def setUpClass(cls):
cls.testClient = super(TestVPCConserveModeRules, cls).getClsTestClient()
cls.apiclient = cls.testClient.getApiClient()
cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
cls.domain = get_domain(cls.apiclient)
cls.hypervisor = cls.testClient.getHypervisorInfo()
cls.logger = logging.getLogger("TestVPCConserveModeRules")
cls.account = Account.create(
cls.apiclient,
cls.services["account"],
admin=True,
domainid=cls.domain.id)
cls._cleanup.append(cls.account)
cls.template = get_test_template(
cls.apiclient,
cls.zone.id,
cls.hypervisor)
if cls.template == FAILED:
assert False, "get_test_template() failed to return template"
cls.service_offering = ServiceOffering.create(
cls.apiclient,
cls.services["service_offerings"]["tiny"]
)
cls._cleanup.append(cls.service_offering)
cls.vpc_offering_conserve_mode = VpcOffering.create(
cls.apiclient,
cls.services["vpc_offering"],
conservemode=True,
)
cls._cleanup.append(cls.vpc_offering_conserve_mode)
cls.vpc_offering_conserve_mode.update(cls.apiclient, state="Enabled")
cls.network_offering = NetworkOffering.create(
cls.apiclient,
cls.services["network_offering"],
conservemode=True
)
cls.network_offering.update(cls.apiclient, state="Enabled")
cls.services["vpc"]["cidr"] = "10.10.20.0/24"
cls.vpc = VPC.create(
cls.apiclient,
cls.services["vpc"],
vpcofferingid=cls.vpc_offering_conserve_mode.id,
zoneid=cls.zone.id,
account=cls.account.name,
domainid=cls.account.domainid,
)
gateway_tier1 = "10.10.20.1"
netmask_tiers = "255.255.255.240"
cls.services["network_offering"]["name"] = "tier1-" + cls.vpc.id
cls.services["network_offering"]["displayname"] = "tier1-" + cls.vpc.id
cls.tier1 = Network.create(
cls.self.apiclient,
services=cls.services["network_offering"],
accountid=cls.account.name,
domainid=cls.account.domainid,
networkofferingid=cls.network_offering.id,
zoneid=cls.zone.id,
vpcid=cls.vpc.id,
gateway=gateway_tier1,
netmask=netmask_tiers,
)
gateway_tier2 = "10.10.20.17"
cls.services["network_offering"]["name"] = "tier2-" + cls.vpc.id
cls.services["network_offering"]["displayname"] = "tier2-" + cls.vpc.id
cls.tier2 = Network.create(
cls.apiclient,
services=cls.services["network_offering"],
accountid=cls.account.name,
domainid=cls.account.domainid,
networkofferingid=cls.network_offering.id,
zoneid=cls.zone.id,
vpcid=cls.vpc.id,
gateway=gateway_tier2,
netmask=netmask_tiers,
)
cls.services["virtual_machine"]["displayname"] = "vm1" + cls.vpc.id
cls.vm1 = VirtualMachine.create(
cls.apiclient,
services=cls.services["virtual_machine"],
templateid=cls.template.id,
zoneid=cls.zone.id,
accountid=cls.account.name,
domainid=cls.account.domainid,
serviceofferingid=cls.service_offering.id,
networkids=[cls.tier1.id],
)
cls.services["virtual_machine"]["displayname"] = "vm2" + cls.vpc.id
cls.vm2 = VirtualMachine.create(
cls.apiclient,
services=cls.services["virtual_machine"],
templateid=cls.template.id,
zoneid=cls.zone.id,
accountid=cls.account.name,
domainid=cls.account.domainid,
serviceofferingid=cls.service_offering.id,
networkids=[cls.tier2.id],
)
@classmethod
def tearDownClass(cls):
super(TestVPCConserveModeRules, cls).tearDownClass()
def setUp(self):
self.apiclient = self.testClient.getApiClient()
self.cleanup = []
def tearDown(self):
super(TestVPCConserveModeRules, self).tearDown()
@attr(tags=["advanced"], required_hardware="true")
def test_01_vpc_conserve_mode_cross_tier_rules_allowed(self):
"""With conserveMode=True, LB rule on VPC Tier 1 and Port Forwarding rule on VPC Tier 2 can
share the same public IP without a NetworkRuleConflictException.
"""
public_ip = PublicIPAddress.create(
self.apiclient,
zoneid=self.zone.id,
accountid=self.account.name,
domainid=self.account.domainid,
vpcid=self.vpc.id,
)
self.logger.debug(
"Creating LB rule on tier-1 (networkid=%s) using public IP %s",
self.tier1.id,
public_ip.ipaddress.ipaddress,
)
lb_rule_tier1 = LoadBalancerRule.create(
self.apiclient,
self.services["lbrule"],
ipaddressid=public_ip.ipaddress.id,
accountid=self.account.name,
vpcid=self.vpc.id,
networkid=self.tier1.id,
domainid=self.account.domainid,
)
self.assertIsNotNone(lb_rule_tier1, "LB rule creation on tier-1 failed")
lb_rule_tier1.assign(self.apiclient, [self.vm1])
self.logger.debug(
"Creating Port Forwarding rule on tier-2 (networkid=%s) "
"using the same public IP %s should succeed with conserve mode",
self.tier2.id,
public_ip.ipaddress.ipaddress,
)
try:
nat_rule = NATRule.create(
self.apiclient,
self.vm2,
self.services["natrule"],
ipaddressid=public_ip.ipaddress.id,
vpcid=self.vpc.id,
networkid=self.tier2.id,
)
self.assertIsNotNone(
nat_rule,
"Port Forwarding rule creation on tier-2 failed unexpectedly",
)
except CloudstackAPIException as e:
self.fail(
"Expected cross-tier Port Forwarding rule to succeed with "
"conserveMode=True, but got exception: %s" % e
)
@attr(tags=["advanced"], required_hardware="true")
def test_02_vpc_conserve_mode_reuse_source_nat_ip_address(self):
"""With VPC conserve mode enabled, a NAT rule can be created on a VPC tier (conserve mode enabled)
with a source NAT IP address
"""
source_nat_ip_resp = list_publicIP(
self.apiclient,
vpcid=self.vpc.id,
listall=True,
issourcenat=True
)
source_nat_ip = source_nat_ip_resp[0]
self.logger.debug(
"Creating Port Forwarding rule on tier-1 (networkid=%s) "
"using the source NAT public IP %s should succeed with conserve mode",
self.tier1.id,
source_nat_ip.ipaddress.ipaddress,
)
try:
nat_rule = NATRule.create(
self.apiclient,
self.vm2,
self.services["natrule"],
ipaddressid=source_nat_ip.ipaddress.id,
vpcid=self.vpc.id,
networkid=self.tier2.id,
)
self.assertIsNotNone(
nat_rule,
"Port Forwarding rule creation on tier-2 failed unexpectedly",
)
except CloudstackAPIException as e:
self.fail(
"Expected cross-tier Port Forwarding rule to succeed with "
"conserveMode=True, but got exception: %s" % e
)

View File

@ -5225,6 +5225,8 @@ class VpcOffering:
cmd.networkmode = services["networkmode"]
if "routingmode" in services:
cmd.routingmode = services["routingmode"]
if "conservemode" in services:
cmd.conservemode = services["conservemode"]
return VpcOffering(apiclient.createVPCOffering(cmd).__dict__)
def update(self, apiclient, name=None, displaytext=None, state=None):