From 89ca2fe48e37dfec642c0b18f1b63b2f6767eb58 Mon Sep 17 00:00:00 2001 From: Sheng Yang Date: Thu, 5 Jan 2012 16:19:42 -0800 Subject: [PATCH] bug 12656: Add restriction for network update and new rules status 12656: resolved fixed --- api/src/com/cloud/network/NetworkService.java | 2 +- server/src/com/cloud/api/ApiDBUtils.java | 4 +- .../src/com/cloud/api/ApiResponseHelper.java | 16 +- .../src/com/cloud/network/NetworkManager.java | 4 +- .../com/cloud/network/NetworkManagerImpl.java | 176 +++++++++++++----- .../lb/LoadBalancingRulesManagerImpl.java | 13 ++ .../VirtualNetworkApplianceManagerImpl.java | 2 +- .../cloud/network/rules/RulesManagerImpl.java | 9 + .../vpn/RemoteAccessVpnManagerImpl.java | 6 +- .../cloud/network/MockNetworkManagerImpl.java | 2 +- 10 files changed, 168 insertions(+), 66 deletions(-) diff --git a/api/src/com/cloud/network/NetworkService.java b/api/src/com/cloud/network/NetworkService.java index 87eef8f07d1..ed451e51492 100644 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -84,7 +84,7 @@ public interface NetworkService { Network getSystemNetworkByZoneAndTrafficType(long zoneId, TrafficType trafficType); - Map> listNetworkOfferingServicesAndProviders(long networkOfferingId); + Map> getNetworkOfferingServiceProvidersMap(long networkOfferingId); PhysicalNetwork createPhysicalNetwork(Long zoneId, String vnetRange, String networkSpeed, List isolationMethods, String broadcastDomainRange, Long domainId, List tags); diff --git a/server/src/com/cloud/api/ApiDBUtils.java b/server/src/com/cloud/api/ApiDBUtils.java index 0d2982ed226..79b4cd9770f 100755 --- a/server/src/com/cloud/api/ApiDBUtils.java +++ b/server/src/com/cloud/api/ApiDBUtils.java @@ -698,8 +698,8 @@ public class ApiDBUtils { return details.isEmpty() ? null : details; } - public static Map> listNetworkOfferingServices(long networkOfferingId) { - return _networkMgr.listNetworkOfferingServicesAndProviders(networkOfferingId); + public static Map> listNetworkOfferingServices(long networkOfferingId) { + return _networkMgr.getNetworkOfferingServiceProvidersMap(networkOfferingId); } public static List getElementServices(Provider provider) { diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index beab81df62a..4ba7b9ed2a0 100755 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -2596,31 +2596,31 @@ public class ApiResponseHelper implements ResponseGenerator { response.setState(offering.getState().name()); - Map> serviceProviderMap = ApiDBUtils.listNetworkOfferingServices(offering.getId()); + Map> serviceProviderMap = ApiDBUtils.listNetworkOfferingServices(offering.getId()); List serviceResponses = new ArrayList(); - for (String service : serviceProviderMap.keySet()) { + for (Service service : serviceProviderMap.keySet()) { ServiceResponse svcRsp = new ServiceResponse(); //skip gateway service - if (service.equalsIgnoreCase(Service.Gateway.getName())) { + if (service == Service.Gateway) { continue; } - svcRsp.setName(service); + svcRsp.setName(service.getName()); List providers = new ArrayList(); - for (String provider : serviceProviderMap.get(service)) { + for (Provider provider : serviceProviderMap.get(service)) { ProviderResponse providerRsp = new ProviderResponse(); - providerRsp.setName(provider); + providerRsp.setName(provider.getName()); providers.add(providerRsp); } svcRsp.setProviders(providers); - if (Service.Lb.getName().equalsIgnoreCase(service)) { + if (Service.Lb == service) { List lbCapResponse = new ArrayList(); CapabilityResponse lbIsoaltion = new CapabilityResponse(); lbIsoaltion.setName(Capability.SupportedLBIsolation.getName()); lbIsoaltion.setValue(offering.getDedicatedLB()?"dedicated":"shared"); lbCapResponse.add(lbIsoaltion); svcRsp.setCapabilities(lbCapResponse); - } else if (Service.SourceNat.getName().equalsIgnoreCase(service)) { + } else if (Service.SourceNat == service) { List capabilities = new ArrayList(); CapabilityResponse sharedSourceNat = new CapabilityResponse(); sharedSourceNat.setName(Capability.SupportedSourceNatTypes.getName()); diff --git a/server/src/com/cloud/network/NetworkManager.java b/server/src/com/cloud/network/NetworkManager.java index 7192c9fcf48..7304ba70f3c 100644 --- a/server/src/com/cloud/network/NetworkManager.java +++ b/server/src/com/cloud/network/NetworkManager.java @@ -266,7 +266,9 @@ public interface NetworkManager extends NetworkService { boolean areServicesEnabledInZone(long zoneId, long networkOfferingId, String tags, List services); - public Map> getIpToServices(Network network, List publicIps, boolean rulesRevoked); + public Map> getIpToServices(List publicIps, boolean rulesRevoked); public Map> getProviderToIpList(Network network, Map> ipToServices); + + public boolean checkIpForService(IPAddressVO ip, Service service); } diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index a787e95ff35..3d13bf3bcf1 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -605,7 +605,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag return success; } - private Map> getProviderServiceMap(long networkId) { + private Map> getProviderServicesMap(long networkId) { Map> map = new HashMap>(); List nsms = _ntwkSrvcDao.getServicesInNetwork(networkId); for (NetworkServiceMapVO nsm : nsms) { @@ -619,7 +619,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag return map; } - private Map> getServiceProviderMap(long networkId) { + private Map> getServiceProvidersMap(long networkId) { Map> map = new HashMap>(); List nsms = _ntwkSrvcDao.getServicesInNetwork(networkId); for (NetworkServiceMapVO nsm : nsms) { @@ -635,7 +635,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag /* Get a list of IPs, classify them by service */ @Override - public Map> getIpToServices(Network network, List publicIps, boolean rulesRevoked) { + public Map> getIpToServices(List publicIps, boolean rulesRevoked) { Map> ipToServices = new HashMap>(); if (publicIps != null && !publicIps.isEmpty()) { @@ -650,10 +650,12 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag services.add(Service.SourceNat); gotSNAT = true; } else { - //TODO throw proper exception - throw new CloudRuntimeException("Multiply generic source NAT IPs in network " + network.getId() + "!"); + throw new CloudRuntimeException("Multiply generic source NAT IPs provided!"); } } + if (ip.isOneToOneNat()) { + services.add(Service.StaticNat); + } ipToServices.put(ip, services); //if IP in allocating state then it will not have any rules attached so skip IPAssoc to network service provider @@ -703,30 +705,77 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag return ipToServices; } - public boolean canIpUsedForService(Network network, PublicIp ip, Service service) { - // We need to check if it's non-conserve mode, then the new ip should not be used by any other services - NetworkOffering offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); - if (offering.isConserveMode()) { - List ipList = new ArrayList(); - ipList.add(ip); - Map> ipToServices = getIpToServices(network, ipList, false); - Set currentServices = ipToServices.get(ip); - // Not used currently, safe - if (currentServices == null || currentServices.isEmpty()) { - return true; - } + public boolean canIpUsedForNonConserveService(PublicIp ip, Service service) { + // If it's non-conserve mode, then the new ip should not be used by any other services + List ipList = new ArrayList(); + ipList.add(ip); + Map> ipToServices = getIpToServices(ipList, false); + Set currentServices = ipToServices.get(ip); + // Not used currently, safe + if (currentServices == null || currentServices.isEmpty()) { + return true; } - Map> serviceToProviders = getServiceProviderMap(network.getId()); - Set currentProviders = serviceToProviders.get(service); - if (currentProviders.size() > 1) { - throw new CloudRuntimeException("Can't support multiply providers for same service now!"); + // Since it's non-conserve mode, only one service should used for IP + if (currentServices.size() != 1) { + throw new CloudRuntimeException("There are multiply services used ip " + ip.getAddress() + "."); + } + if (service != null && (Service)currentServices.toArray()[0] != service) { + throw new CloudRuntimeException("The IP " + ip.getAddress() + " is already used as " + ((Service)currentServices.toArray()[0]).getName() + " rather than " + service.getName()); } - Provider currentProvider = (Provider)currentProviders.toArray()[0]; - return true; } - /* Return a mapping between NetworkElement in the network and the IP they should applied */ + protected boolean canIpsUsedForNonConserve(List publicIps) { + boolean result = true; + for (PublicIp ip : publicIps) { + result = canIpUsedForNonConserveService(ip, null); + if (!result) { + break; + } + } + return result; + } + + public boolean canIpsUseOffering(List publicIps, long offeringId) { + Map> ipToServices = getIpToServices(publicIps, false); + Map> serviceToProviders = getNetworkOfferingServiceProvidersMap(offeringId); + for (PublicIp ip : ipToServices.keySet()) { + Set services = ipToServices.get(ip); + Provider provider = null; + for (Service service : services) { + Provider curProvider = (Provider)serviceToProviders.get(service).toArray()[0]; + //We don't support multiply providers for one service now + if (provider == null) { + provider = curProvider; + continue; + } + if (!provider.equals(curProvider)) { + throw new CloudRuntimeException("There would be multiply providers for IP " + ip.getAddress() + " with the new network offering!"); + } + } + } + return true; + } + + public boolean canIpUsedForConserveService(PublicIp publicIp, Service service) { + Map> serviceToProviders = getServiceProvidersMap(publicIp.getAssociatedWithNetworkId()); + List ipList = new ArrayList(); + ipList.add(publicIp); + Map> ipToServices = getIpToServices(ipList, false); + Set services = ipToServices.get(publicIp); + if (services == null || services.isEmpty()) { + return true; + } + //We only support one provider for one service now + Provider oldProvider = (Provider)serviceToProviders.get((Service)services.toArray()[0]).toArray()[0]; + Provider newProvider = (Provider)serviceToProviders.get(service).toArray()[0]; + if (!oldProvider.equals(newProvider)) { + throw new CloudRuntimeException("There would be multiply providers for IP " + publicIp.getAddress() + "!"); + } + return true; + } + + /* Return a mapping between provider in the network and the IP they should applied */ @Override public Map> getProviderToIpList(Network network, Map> ipToServices) { NetworkOffering offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); @@ -734,8 +783,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag for (PublicIp ip : ipToServices.keySet()) { Set services = ipToServices.get(ip); if (services != null && services.size() > 1) { - //TODO throw proper exception - throw new CloudRuntimeException(""); + throw new CloudRuntimeException("Ip " + ip.getAddress() + " is used by multiply services!"); } } } @@ -751,11 +799,10 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } } //TODO Check different provider for same IP - Map> providerToServices = getProviderServiceMap(network.getId()); + Map> providerToServices = getProviderServicesMap(network.getId()); Map> providerToIpList = new HashMap>(); for (Provider provider: providerToServices.keySet()) { Set services = providerToServices.get(provider); - //TODO add checking for services, multiply services may bind to one provider, which is invalid in non-conserved mode ArrayList ipList = new ArrayList(); Set ipSet = new HashSet(); for (Service service: services) { @@ -779,7 +826,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag protected boolean applyIpAssociations(Network network, boolean rulesRevoked, boolean continueOnError, List publicIps) throws ResourceUnavailableException { boolean success = true; - Map> ipToServices = getIpToServices(network, publicIps, rulesRevoked); + Map> ipToServices = getIpToServices(publicIps, rulesRevoked); Map> providerToIpList = getProviderToIpList(network, ipToServices); for (Provider provider : providerToIpList.keySet()) { @@ -2817,7 +2864,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag boolean success = true; Network network = _networksDao.findById(rules.get(0).getNetworkId()); Purpose purpose = rules.get(0).getPurpose(); - + // get the list of public ip's owned by the network List userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null); List publicIps = new ArrayList(); @@ -3668,7 +3715,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag throw new InvalidParameterValueException("Network offering " + networkOffering + " contained external network elements, can't be upgraded from a CIDR specify network!"); } //check if the network is upgradable - if (!canUpgrade(oldNetworkOfferingId, networkOfferingId)) { + if (!canUpgrade(network, oldNetworkOfferingId, networkOfferingId)) { throw new InvalidParameterValueException("Can't upgrade from network offering " + oldNetworkOfferingId + " to " + networkOfferingId + "; check logs for more information"); } restartNetwork = true; @@ -3987,20 +4034,19 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } @Override - public Map> listNetworkOfferingServicesAndProviders(long networkOfferingId) { - Map> serviceProviderMap = new HashMap>(); + public Map> getNetworkOfferingServiceProvidersMap(long networkOfferingId) { + Map> serviceProviderMap = new HashMap>(); List map = _ntwkOfferingSrvcDao.listByNetworkOfferingId(networkOfferingId); for (NetworkOfferingServiceMapVO instance : map) { String service = instance.getService(); - Set providers; - if (serviceProviderMap.containsKey(service)) { - providers = serviceProviderMap.get(service); - } else { - providers = new HashSet(); + Set providers; + providers = serviceProviderMap.get(service); + if (providers == null) { + providers = new HashSet(); } - providers.add(instance.getProvider()); - serviceProviderMap.put(service, providers); + providers.add(Provider.getProvider(instance.getProvider())); + serviceProviderMap.put(Service.getService(service), providers); } return serviceProviderMap; @@ -4011,7 +4057,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag return _ntwkSrvcDao.canProviderSupportServiceInNetwork(networkId, service, provider); } - protected boolean canUpgrade(long oldNetworkOfferingId, long newNetworkOfferingId) { + protected boolean canUpgrade(Network network, long oldNetworkOfferingId, long newNetworkOfferingId) { NetworkOffering oldNetworkOffering = _networkOfferingDao.findByIdIncludingRemoved(oldNetworkOfferingId); NetworkOffering newNetworkOffering = _networkOfferingDao.findById(newNetworkOfferingId); @@ -4049,22 +4095,35 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag s_logger.debug("Network offerings " + newNetworkOfferingId + " and " + oldNetworkOfferingId + " have different traffic types, can't upgrade"); return false; } - - return true; + + //Check all ips + List userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null); + List publicIps = new ArrayList(); + if (userIps != null && !userIps.isEmpty()) { + for (IPAddressVO userIp : userIps) { + PublicIp publicIp = new PublicIp(userIp, _vlanDao.findById(userIp.getVlanId()), NetUtils.createSequenceBasedMacAddress(userIp.getMacAddress())); + publicIps.add(publicIp); + } + } + if (oldNetworkOffering.isConserveMode() && !newNetworkOffering.isConserveMode()) { + return canIpsUsedForNonConserve(publicIps); + } + + return canIpsUseOffering(publicIps, newNetworkOfferingId); } protected boolean canUpgradeProviders(long oldNetworkOfferingId, long newNetworkOfferingId) { //list of services and providers should be the same - Map> newServices = listNetworkOfferingServicesAndProviders(newNetworkOfferingId); - Map> oldServices = listNetworkOfferingServicesAndProviders(oldNetworkOfferingId); + Map> newServices = getNetworkOfferingServiceProvidersMap(newNetworkOfferingId); + Map> oldServices = getNetworkOfferingServiceProvidersMap(oldNetworkOfferingId); if (newServices.size() < oldServices.size()) { s_logger.debug("Network offering downgrade is not allowed: number of supported services for the new offering " + newNetworkOfferingId + " is less than the old offering " + oldNetworkOfferingId); return false; } - for (String service : oldServices.keySet()) { + for (Service service : oldServices.keySet()) { //1)check that all old services are present in the new network offering if (!newServices.containsKey(service)) { @@ -4072,18 +4131,18 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag return false; } - Set newProviders = newServices.get(service); - Set oldProviders = oldServices.get(service); + Set newProviders = newServices.get(service); + Set oldProviders = oldServices.get(service); //2) Can upgrade only from internal provider to external provider. Any other combinations are not allowed - for (String oldProvider : oldProviders) { + for (Provider oldProvider : oldProviders) { if (newProviders.contains(oldProvider)) { s_logger.trace("New list of providers contains provider " + oldProvider); continue; } //iterate through new providers and check that the old provider can upgrade - for (String newProvider : newProviders) { - if (!(!Provider.getProvider(oldProvider).isExternal() && Provider.getProvider(newProvider).isExternal())) { + for (Provider newProvider : newProviders) { + if (!(!oldProvider.isExternal() && newProvider.isExternal())) { s_logger.debug("Can't downgrade from network offering " + oldNetworkOfferingId + " to the new networkOffering " + newNetworkOfferingId); return false; } @@ -5562,4 +5621,19 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag return result; } + + @Override + public boolean checkIpForService(IPAddressVO userIp, Service service) { + Long networkId = userIp.getAssociatedWithNetworkId(); + NetworkVO network = _networksDao.findById(networkId); + NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); + if (offering.getGuestType() != GuestType.Isolated) { + return true; + } + PublicIp publicIp = new PublicIp(userIp, _vlanDao.findById(userIp.getVlanId()), NetUtils.createSequenceBasedMacAddress(userIp.getMacAddress())); + if (!offering.isConserveMode()) { + return canIpUsedForNonConserveService(publicIp, service); + } + return canIpUsedForConserveService(publicIp, service); + } } diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index bb50b73ba1b..9e5d79803c0 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -584,6 +584,19 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesMa throw new InvalidParameterValueException("Invalid algorithm: " + lb.getAlgorithm()); } + Long ipAddrId = lb.getSourceIpAddressId(); + + IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); + + // Validate ip address + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create load balance rule; ip id=" + ipAddrId + " doesn't exist in the system"); + } else if (ipAddress.isOneToOneNat()) { + throw new NetworkRuleConflictException("Can't do load balance on ip address: " + ipAddress.getAddress()); + } + + _networkMgr.checkIpForService(ipAddress, Service.Lb); + LoadBalancer result = _elbMgr.handleCreateLoadBalancerRule(lb, caller.getCaller()); if (result == null){ result = createLoadBalancer(lb, openFirewall); diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 4bc68607eda..e31b4c5c4a0 100755 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -1792,7 +1792,7 @@ public class VirtualNetworkApplianceManagerImpl implements VirtualNetworkApplian //Get public Ips that should be handled by router Network network = _networkDao.findById(networkId); - Map> ipToServices = _networkMgr.getIpToServices(network, allPublicIps, false); + Map> ipToServices = _networkMgr.getIpToServices(allPublicIps, false); Map> providerToIpList = _networkMgr.getProviderToIpList(network, ipToServices); // Only cover virtual router for now, if ELB use it this need to be modified ArrayList publicIps = providerToIpList.get(Provider.VirtualRouter); diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 3b119dfba1a..66388d28c45 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -45,12 +45,16 @@ import com.cloud.network.IpAddress; import com.cloud.network.Network; import com.cloud.network.Network.Service; import com.cloud.network.NetworkManager; +import com.cloud.network.PublicIpAddress; import com.cloud.network.dao.FirewallRulesCidrsDao; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.NetworkDao; import com.cloud.network.rules.FirewallRule.FirewallRuleType; import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.network.rules.dao.PortForwardingRulesDao; +import com.cloud.offering.NetworkOffering; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.DomainManager; @@ -190,6 +194,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } else { checkRuleAndUserVm(rule, vm, caller); } + + _networkMgr.checkIpForService(ipAddress, Service.PortForwarding); // Verify that vm has nic in the network Ip dstIp = rule.getDestinationIpAddress(); @@ -264,6 +270,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { Long accountId = ipAddress.getAccountId(); Long domainId = ipAddress.getDomainId(); + _networkMgr.checkIpForService(ipAddress, Service.StaticNat); String dstIp = _networkMgr.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); @@ -375,6 +382,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (ip != null) { throw new InvalidParameterValueException("Failed to enable static nat for the ip address id=" + ipId + " as vm id=" + vmId + " is already associated with ip id=" + ip.getId()); } + + _networkMgr.checkIpForService(ip, Service.StaticNat); ipAddress.setOneToOneNat(true); ipAddress.setAssociatedWithVmId(vmId); diff --git a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index bc92318cff9..54f5a6a8c6c 100755 --- a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -40,6 +40,7 @@ import com.cloud.exception.AccountLimitException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.network.IPAddressVO; import com.cloud.network.Network; import com.cloud.network.Network.Service; import com.cloud.network.NetworkManager; @@ -114,12 +115,15 @@ public class RemoteAccessVpnManagerImpl implements RemoteAccessVpnService, Manag if (ipAddr == null) { throw new InvalidParameterValueException("Unable to create remote access vpn, invalid public IP address id" + publicIpId); } - + _accountMgr.checkAccess(caller, null, ipAddr); if (!ipAddr.readyToUse() || ipAddr.getAssociatedWithNetworkId() == null) { throw new InvalidParameterValueException("The Ip address is not ready to be used yet: " + ipAddr.getAddress()); } + + IPAddressVO ipAddress = _ipAddressDao.findById(publicIpId); + _networkMgr.checkIpForService(ipAddress, Service.Vpn); RemoteAccessVpnVO vpnVO = _remoteAccessVpnDao.findByPublicIpAddress(publicIpId); diff --git a/server/test/com/cloud/network/MockNetworkManagerImpl.java b/server/test/com/cloud/network/MockNetworkManagerImpl.java index b0217a1819d..04d10c0947c 100755 --- a/server/test/com/cloud/network/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/network/MockNetworkManagerImpl.java @@ -429,7 +429,7 @@ public class MockNetworkManagerImpl implements NetworkManager, Manager, NetworkS } @Override - public Map> listNetworkOfferingServicesAndProviders(long networkOfferingId) { + public Map> getNetworkOfferingServiceProvidersMap(long networkOfferingId) { return null; }