From a2a4968f51e08bba3a80af2dda402ff994144835 Mon Sep 17 00:00:00 2001 From: Rakesh Date: Fri, 31 Jan 2020 11:22:42 +0100 Subject: [PATCH] server: Allow creating network with duplicate name (#3807) Add a global setting to disable creating networks with same name in an account Add a global setting to disable creating network without mentioning the start and end IPv4 or IPv6 address By default we can create networks with the same name in the account. Sometimes we should not create the networks with same name. This change adds a global setting which prevents creating the network with same name. The default value is true and set it to false to prevent creating network with same names. Also its possible to create a shared network without mentioning the start and the end IPv4 or IPv6 address. This change adds a global setting which prevents creating a shared network without specifying the start and the end IPv4 or IPv6 address --- .travis.yml | 1 + .../com/cloud/network/dao/NetworkDao.java | 2 + .../com/cloud/network/dao/NetworkDaoImpl.java | 10 + .../com/cloud/network/NetworkServiceImpl.java | 43 ++- .../com/cloud/vpc/dao/MockNetworkDaoImpl.java | 5 + test/integration/smoke/test_create_network.py | 291 ++++++++++++++++++ 6 files changed, 351 insertions(+), 1 deletion(-) create mode 100644 test/integration/smoke/test_create_network.py diff --git a/.travis.yml b/.travis.yml index a7429fe2ed5..f1b8b7afea1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -44,6 +44,7 @@ env: smoke/test_affinity_groups_projects smoke/test_async_job smoke/test_create_list_domain_account_project + smoke/test_create_network smoke/test_deploy_vgpu_enabled_vm smoke/test_deploy_vm_iso smoke/test_deploy_vm_root_resize diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java index 5091ebd75df..7da76c43206 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java @@ -120,4 +120,6 @@ public interface NetworkDao extends GenericDao, StateDao listNetworkVO(List idset); + + List listByAccountIdNetworkName(long accountId, String name); } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java index 23936cbdeab..889bbc08467 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java @@ -104,6 +104,7 @@ public class NetworkDaoImpl extends GenericDaoBaseimplements Ne @PostConstruct protected void init() { AllFieldsSearch = createSearchBuilder(); + AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ); AllFieldsSearch.and("trafficType", AllFieldsSearch.entity().getTrafficType(), Op.EQ); AllFieldsSearch.and("cidr", AllFieldsSearch.entity().getCidr(), Op.EQ); AllFieldsSearch.and("broadcastType", AllFieldsSearch.entity().getBroadcastDomainType(), Op.EQ); @@ -697,4 +698,13 @@ public class NetworkDaoImpl extends GenericDaoBaseimplements Ne sc_2.addAnd("removed", SearchCriteria.Op.EQ, null); return this.search(sc_2, searchFilter_2); } + + @Override + public List listByAccountIdNetworkName(final long accountId, final String name) { + final SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("account", accountId); + sc.setParameters("name", name); + + return listBy(sc, null); + } } diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 7761ce69ef3..869fe67f5c8 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -55,6 +55,8 @@ import org.apache.cloudstack.api.command.user.vm.ListNicsCmd; import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.PublishScope; @@ -107,6 +109,7 @@ import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerDao; +import com.cloud.network.dao.NetworkAccountDao; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkDetailVO; import com.cloud.network.dao.NetworkDetailsDao; @@ -200,9 +203,14 @@ import com.cloud.vm.dao.VMInstanceDao; /** * NetworkServiceImpl implements NetworkService. */ -public class NetworkServiceImpl extends ManagerBase implements NetworkService { +public class NetworkServiceImpl extends ManagerBase implements NetworkService, Configurable { private static final Logger s_logger = Logger.getLogger(NetworkServiceImpl.class); + private static final ConfigKey AllowDuplicateNetworkName = new ConfigKey("Advanced", Boolean.class, + "allow.duplicate.networkname", "true", "Allow creating networks with same name in account", true, ConfigKey.Scope.Account); + private static final ConfigKey AllowEmptyStartEndIpAddress = new ConfigKey("Advanced", Boolean.class, + "allow.empty.start.end.ipaddress", "true", "Allow creating network without mentioning start and end IP address", + true, ConfigKey.Scope.Account); private static final long MIN_VLAN_ID = 0L; private static final long MAX_VLAN_ID = 4095L; // 2^12 - 1 private static final long MIN_GRE_KEY = 0L; @@ -313,6 +321,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { VpcOfferingDao _vpcOfferingDao; @Inject AccountService _accountService; + @Inject + NetworkAccountDao _networkAccountDao; int _cidrLimit; boolean _allowSubdomainNetworkAccess; @@ -1164,6 +1174,18 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { throw new InvalidParameterValueException("Parameter subDomainAccess can be specified only with aclType=Domain"); } + if (aclType == ACLType.Domain) { + owner = _accountDao.findById(Account.ACCOUNT_ID_SYSTEM); + } + + // The network name is unique under the account + if (!AllowDuplicateNetworkName.valueIn(owner.getAccountId())) { + List existingNetwork = _networksDao.listByAccountIdNetworkName(owner.getId(), name); + if (!existingNetwork.isEmpty()) { + throw new InvalidParameterValueException("Another network with same name already exists within account: " + owner.getAccountName()); + } + } + boolean ipv4 = true, ipv6 = false; if (startIP != null) { ipv4 = true; @@ -1188,6 +1210,15 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { } } + // Start and end IP address are mandatory for shared networks. + if (ntwkOff.getGuestType() == GuestType.Shared && vpcId == null) { + if (!AllowEmptyStartEndIpAddress.valueIn(owner.getAccountId()) && + (startIP == null && endIP == null) && + (startIPv6 == null && endIPv6 == null)) { + throw new InvalidParameterValueException("Either IPv4 or IPv6 start and end address are mandatory"); + } + } + String cidr = null; if (ipv4) { // if end ip is not specified, default it to startIp @@ -4537,4 +4568,14 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { return true; } + @Override + public String getConfigComponentName() { + return NetworkService.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {AllowDuplicateNetworkName, AllowEmptyStartEndIpAddress}; + } + } diff --git a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java index 11f3f81f357..e65b3270b42 100644 --- a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java +++ b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java @@ -235,4 +235,9 @@ public class MockNetworkDaoImpl extends GenericDaoBase implemen public List listNetworkVO(List idset) { return null; } + + @Override + public List listByAccountIdNetworkName(final long accountId, final String name) { + return null; + } } \ No newline at end of file diff --git a/test/integration/smoke/test_create_network.py b/test/integration/smoke/test_create_network.py new file mode 100644 index 00000000000..7fe7cbbc53a --- /dev/null +++ b/test/integration/smoke/test_create_network.py @@ -0,0 +1,291 @@ +# 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. + +# Import Local Modules +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase, unittest +from marvin.sshClient import SshClient +from marvin.lib.utils import (cleanup_resources, + random_gen) +from marvin.lib.base import (Account, + Configurations, + Domain, + Network, + NetworkOffering, + PhysicalNetwork, + ServiceOffering, + Zone) +from marvin.lib.common import (get_domain, + get_zone, + get_free_vlan) +import logging +import random + +class TestNetworkManagement(cloudstackTestCase): + @classmethod + def setUpClass(cls): + cls.testClient = super( + TestNetworkManagement, + cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.dbclient = cls.testClient.getDbConnection() + cls.testdata = cls.testClient.getParsedTestDataConfig() + cls.services = cls.testClient.getParsedTestDataConfig() + zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.zone = Zone(zone.__dict__) + cls._cleanup = [] + + cls.logger = logging.getLogger("TestNetworkManagement") + cls.stream_handler = logging.StreamHandler() + cls.logger.setLevel(logging.DEBUG) + cls.logger.addHandler(cls.stream_handler) + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + testClient = super(TestNetworkManagement, cls).getClsTestClient() + cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests()) + cls.services['mode'] = cls.zone.networktype + # Create new domain, account, network and VM + cls.user_domain = Domain.create( + cls.apiclient, + services=cls.testdata["acl"]["domain2"], + parentdomainid=cls.domain.id) + + # Create account + cls.account = Account.create( + cls.apiclient, + cls.testdata["acl"]["accountD2"], + admin=True, + domainid=cls.user_domain.id + ) + + # Create small service offering + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.testdata["service_offerings"]["small"] + ) + + cls._cleanup.append(cls.service_offering) + cls._cleanup.append(cls.account) + cls._cleanup.append(cls.user_domain) + + @classmethod + def tearDownClass(self): + try: + cleanup_resources(self.apiclient, self._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + return + + def tearDown(self): + try: + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @attr(tags=["adeancedsg"], required_hardware="false") + def test_01_create_network_without_start_end_ip(self): + """Create Shared network without start and end ip + + Steps: + # 1. Update the global setting allow.empty.start.end.ipaddress to true + # 2. Create a shared network without specifying start or end ip + # 3. This should create the network + # 4. Now Update the global setting allow.empty.start.end.ipaddress to false + # 5. Create a shared network without specifying start or end ip + # 6. Exception should be thrown since start and end ip are not specified + :return: + """ + # Create network offering + self.network_offering = NetworkOffering.create( + self.apiclient, + self.testdata["network_offering_shared"] + ) + + NetworkOffering.update( + self.network_offering, + self.apiclient, + id=self.network_offering.id, + state="enabled" + ) + + physical_network, vlan = get_free_vlan(self.apiclient, self.zone.id) + self.testdata["shared_network_sg"]["physicalnetworkid"] = physical_network.id + + random_subnet_number = random.randrange(100, 199) + self.testdata["shared_network_sg"]["specifyVlan"] = 'True' + self.testdata["shared_network_sg"]["specifyIpRanges"] = 'True' + self.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + self.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + self.testdata["shared_network_sg"]["startip"] = None + self.testdata["shared_network_sg"]["endip"] = None + self.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254" + self.testdata["shared_network_sg"]["netmask"] = "255.255.255.0" + self.testdata["shared_network_sg"]["acltype"] = "account" + + # Update the global setting to true + Configurations.update(self.apiclient, + name="allow.empty.start.end.ipaddress", + value="true" + ) + + # Create network + network = Network.create( + self.apiclient, + self.testdata["shared_network_sg"], + networkofferingid=self.network_offering.id, + zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid + ) + + self.logger.info("network id is %s" % network.id) + self.cleanup.append(network) + + # Update the global setting to false + Configurations.update(self.apiclient, + name="allow.empty.start.end.ipaddress", + value="false" + ) + + # Exception should be thrown + with self.assertRaises(Exception): + self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + network2 = Network.create( + self.apiclient, + self.testdata["shared_network_sg"], + networkofferingid=self.network_offering.id, + zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid + ) + + # Restore the setting to default value which is true + Configurations.update(self.apiclient, + name="allow.empty.start.end.ipaddress", + value="true" + ) + + self.cleanup.append(self.network_offering) + + @attr(tags=["adeancedsg"], required_hardware="false") + def test_02_create_network_with_same_name(self): + """Create Shared network with same name in same account + + Steps: + # 1. Update the global setting allow.duplicate.networkname to true + # 2. Create a shared network in an account + # 3. Try to create another shared network with same name in the same account + # 4. No exception should be thrown as multiple networks with same name can be created + # 5. Now update the global setting allow.duplicate.networkname to false + # 6. Try to create another shared network with same name in the same account + # 7. Exception should be thrown as network with same name cant be created in the same account + :return: + """ + # Update the global setting to true + Configurations.update(self.apiclient, + name="allow.duplicate.networkname", + value="true" + ) + + # Create network offering + self.network_offering = NetworkOffering.create( + self.apiclient, + self.testdata["network_offering_shared"] + ) + + NetworkOffering.update( + self.network_offering, + self.apiclient, + id=self.network_offering.id, + state="enabled" + ) + + physical_network, vlan = get_free_vlan(self.apiclient, self.zone.id) + self.testdata["shared_network_sg"]["physicalnetworkid"] = physical_network.id + + random_subnet_number = random.randrange(100, 199) + self.testdata["shared_network_sg"]["specifyVlan"] = 'True' + self.testdata["shared_network_sg"]["specifyIpRanges"] = 'True' + self.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan-1" + self.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan-1" + self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + self.testdata["shared_network_sg"]["startip"] = "192.168." + str(random_subnet_number) + ".1" + self.testdata["shared_network_sg"]["endip"] = "192.168." + str(random_subnet_number) + ".10" + self.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254" + self.testdata["shared_network_sg"]["netmask"] = "255.255.255.0" + self.testdata["shared_network_sg"]["acltype"] = "account" + + # Create the first network + network3 = Network.create( + self.apiclient, + self.testdata["shared_network_sg"], + networkofferingid=self.network_offering.id, + zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid + ) + self.cleanup.append(network3) + + # Create the second network with same name. No exception should be thrown + random_subnet_number = random.randrange(100, 199) + self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + network4 = Network.create( + self.apiclient, + self.testdata["shared_network_sg"], + networkofferingid=self.network_offering.id, + zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid + ) + + self.cleanup.append(network4) + + # Update the global setting to true + Configurations.update(self.apiclient, + name="allow.duplicate.networkname", + value="false" + ) + + # Exception should be thrown while creating another network with same name + with self.assertRaises(Exception): + random_subnet_number = random.randrange(100, 199) + self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + network5 = Network.create( + self.apiclient, + self.testdata["shared_network_sg"], + networkofferingid=self.network_offering.id, + zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid + ) + + # Update the global setting to original value + Configurations.update(self.apiclient, + name="allow.duplicate.networkname", + value="true" + ) + + self.cleanup.append(self.network_offering) +