diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index c4c358874dd..308c12ca9dc 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -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); diff --git a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java index 3c663dde92d..681cb9a62ca 100644 --- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java +++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java @@ -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 ruleList = new ArrayList(); - 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 createExistingFirewallListRulesList(long existingNetworkId) { + List 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 sString = Arrays.asList("10.1.1.1/24","192.168.1.1/24"); List dString1 = Arrays.asList("10.1.1.1/25"); - List 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 createNewRuleList(long newNetworkId) { + List sString = Arrays.asList("10.1.1.1/24","192.168.1.1/24"); + List 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 ruleList = createExistingFirewallListRulesList(1L); + when(_firewallMgr._firewallDao.listByIpAndPurposeAndNotRevoked(3,null)).thenReturn(ruleList); + + List 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 ruleList = createExistingFirewallListRulesList(existingNetworkId); + when(_firewallMgr._firewallDao.listByIpAndPurposeAndNotRevoked(3,null)).thenReturn(ruleList); + + List 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() { diff --git a/test/integration/smoke/test_vpc_conserve_mode.py b/test/integration/smoke/test_vpc_conserve_mode.py new file mode 100644 index 00000000000..35bb471ea38 --- /dev/null +++ b/test/integration/smoke/test_vpc_conserve_mode.py @@ -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 + ) diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index bbec2027bdc..43b44d0d392 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -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):