From 7e73ae8e74e3d4c850faf3556ec00f8e06f58d1e Mon Sep 17 00:00:00 2001 From: Alena Prokharchyk Date: Thu, 12 Jul 2012 10:00:29 -0700 Subject: [PATCH] VPC: CS-15553 and CS-15549 - more checks during automatic ip assoc to VPC network --- .../commands/CreateLoadBalancerRuleCmd.java | 2 +- .../src/com/cloud/network/NetworkManager.java | 2 +- .../com/cloud/network/NetworkManagerImpl.java | 7 +- .../network/firewall/FirewallManagerImpl.java | 2 +- .../lb/LoadBalancingRulesManagerImpl.java | 29 +++++--- .../cloud/network/rules/RulesManagerImpl.java | 74 ++++++++++++------- .../vpn/RemoteAccessVpnManagerImpl.java | 2 +- .../cloud/network/MockNetworkManagerImpl.java | 2 +- 8 files changed, 77 insertions(+), 43 deletions(-) diff --git a/api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java b/api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java index dbad001b8f0..8e949c261ef 100644 --- a/api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java @@ -317,7 +317,7 @@ public class CreateLoadBalancerRuleCmd extends BaseAsyncCreateCmd /*implements throw new InvalidParameterValueException("Unable to find account " + account + " in domain with specified id", idList); } } else { - throw new InvalidParameterValueException("Can't define IP owner. Either specify account/domainId or ipAddressId", null); + throw new InvalidParameterValueException("Can't define IP owner. Either specify account/domainId or publicIpId", null); } } diff --git a/server/src/com/cloud/network/NetworkManager.java b/server/src/com/cloud/network/NetworkManager.java index b34d9ecdf41..923ae5abea8 100755 --- a/server/src/com/cloud/network/NetworkManager.java +++ b/server/src/com/cloud/network/NetworkManager.java @@ -266,7 +266,7 @@ public interface NetworkManager extends NetworkService { public Map> getProviderToIpList(Network network, Map> ipToServices); - public boolean checkIpForService(IPAddressVO ip, Service service); + public boolean checkIpForService(IPAddressVO ip, Service service, Long networkId); void checkVirtualNetworkCidrOverlap(Long zoneId, String cidr); diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 0e3b559a4a3..eb80d83b44d 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -6938,8 +6938,11 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } @Override - public boolean checkIpForService(IPAddressVO userIp, Service service) { - Long networkId = userIp.getAssociatedWithNetworkId(); + public boolean checkIpForService(IPAddressVO userIp, Service service, Long networkId) { + if (networkId == null) { + networkId = userIp.getAssociatedWithNetworkId(); + } + NetworkVO network = _networksDao.findById(networkId); NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); if (offering.getGuestType() != GuestType.Isolated) { diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index 3ea17832f93..8c7479a726f 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -157,7 +157,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma "couldn't locate IP address by id in the system", null); } - _networkMgr.checkIpForService(ipAddress, Service.Firewall); + _networkMgr.checkIpForService(ipAddress, Service.Firewall, null); validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall, type); diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 5d9386d31e7..c87bf5b5f6e 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -749,19 +749,28 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesMa } try { + Network network = _networkMgr.getNetwork(lb.getNetworkId()); if (ipVO != null) { if (ipVO.getAssociatedWithNetworkId() == null) { - //set networkId just for verification purposes - ipVO.setAssociatedWithNetworkId(lb.getNetworkId()); - _networkMgr.checkIpForService(ipVO, Service.Lb); + boolean assignToVpcNtwk = network.getVpcId() != null + && ipVO.getVpcId() != null && ipVO.getVpcId().longValue() == network.getVpcId(); + if (assignToVpcNtwk) { + //set networkId just for verification purposes + ipVO.setAssociatedWithNetworkId(lb.getNetworkId()); + _networkMgr.checkIpForService(ipVO, Service.Lb, lb.getNetworkId()); - s_logger.debug("The ip is not associated with the network id="+ lb.getNetworkId() + " so assigning"); - ipVO = _networkMgr.associateIPToGuestNetwork(ipAddrId, lb.getNetworkId()); - performedIpAssoc = true; - } else { - _networkMgr.checkIpForService(ipVO, Service.Lb); - } - } + s_logger.debug("The ip is not associated with the VPC network id="+ lb.getNetworkId() + " so assigning"); + ipVO = _networkMgr.associateIPToGuestNetwork(ipAddrId, lb.getNetworkId()); + performedIpAssoc = true; + } + } else { + _networkMgr.checkIpForService(ipVO, Service.Lb, null); + } + } + + if (ipVO.getAssociatedWithNetworkId() == null) { + throw new InvalidParameterValueException("Ip address " + ipVO + " is not assigned to the network " + network); + } if (lb.getSourceIpAddressId() == null) { throw new CloudRuntimeException("No ip address is defined to assign the LB to"); diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 01c71abd5df..e8e2bd08573 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -183,23 +183,32 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } Long networkId = rule.getNetworkId(); + Network network = _networkMgr.getNetwork(networkId); //associate ip address to network (if needed) boolean performedIpAssoc = false; if (ipAddress.getAssociatedWithNetworkId() == null) { - //set networkId just for verification purposes - ipAddress.setAssociatedWithNetworkId(networkId); - _networkMgr.checkIpForService(ipAddress, Service.PortForwarding); + boolean assignToVpcNtwk = network.getVpcId() != null + && ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == network.getVpcId(); + if (assignToVpcNtwk) { + //set networkId just for verification purposes + ipAddress.setAssociatedWithNetworkId(networkId); + _networkMgr.checkIpForService(ipAddress, Service.PortForwarding, networkId); - s_logger.debug("The ip is not associated with the network id="+ networkId + " so assigning"); - try { - ipAddress = _networkMgr.associateIPToGuestNetwork(ipAddrId, networkId); - performedIpAssoc = true; - } catch (Exception ex) { - throw new CloudRuntimeException("Failed to associate ip to network as " + - "a part of port forwarding rule creation"); + s_logger.debug("The ip is not associated with the VPC network id="+ networkId + ", so assigning"); + try { + ipAddress = _networkMgr.associateIPToGuestNetwork(ipAddrId, networkId); + performedIpAssoc = true; + } catch (Exception ex) { + throw new CloudRuntimeException("Failed to associate ip to VPC network as " + + "a part of port forwarding rule creation"); + } } } else { - _networkMgr.checkIpForService(ipAddress, Service.PortForwarding); + _networkMgr.checkIpForService(ipAddress, Service.PortForwarding, null); + } + + if (ipAddress.getAssociatedWithNetworkId() == null) { + throw new InvalidParameterValueException("Ip address " + ipAddress + " is not assigned to the network " + network); } try { @@ -318,7 +327,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { Long accountId = ipAddress.getAllocatedToAccountId(); Long domainId = ipAddress.getAllocatedInDomainId(); - _networkMgr.checkIpForService(ipAddress, Service.StaticNat); + _networkMgr.checkIpForService(ipAddress, Service.StaticNat, null); Network network = _networkMgr.getNetwork(networkId); NetworkOffering off = _configMgr.getNetworkOffering(network.getNetworkOfferingId()); @@ -393,21 +402,37 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // Verify input parameters boolean setNetworkId = false; + Network network = _networkMgr.getNetwork(networkId); + if (network == null) { + throw new InvalidParameterValueException("Unable to find network by id", null); + } + if (!isSystemVm) { //associate ip address to network (if needed) if (ipAddress.getAssociatedWithNetworkId() == null) { - s_logger.debug("The ip is not associated with the network id="+ networkId + " so assigning"); - try { - ipAddress = _networkMgr.associateIPToGuestNetwork(ipId, networkId); - } catch (Exception ex) { - s_logger.warn("Failed to associate ip id=" + ipId + " to network id=" + networkId + " as " + - "a part of enable static nat"); - return false; + boolean assignToVpcNtwk = network.getVpcId() != null + && ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == network.getVpcId(); + if (assignToVpcNtwk) { + _networkMgr.checkIpForService(ipAddress, Service.StaticNat, networkId); + + s_logger.debug("The ip is not associated with the VPC network id="+ networkId + ", so assigning"); + try { + ipAddress = _networkMgr.associateIPToGuestNetwork(ipId, networkId); + } catch (Exception ex) { + s_logger.warn("Failed to associate ip id=" + ipId + " to VPC network id=" + networkId + " as " + + "a part of enable static nat"); + return false; + } + setNetworkId = true; } - setNetworkId = true; + } else { + _networkMgr.checkIpForService(ipAddress, Service.StaticNat, null); + } + + + if (ipAddress.getAssociatedWithNetworkId() == null) { + throw new InvalidParameterValueException("Ip address " + ipAddress + " is not assigned to the network " + network); } - - _networkMgr.checkIpForService(ipAddress, Service.StaticNat); // Check permissions checkIpAndUserVm(ipAddress, vm, caller); @@ -421,10 +446,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { throw new InvalidParameterValueException("Vm doesn't belong to the network with specified id", idList); } - Network network = _networkMgr.getNetwork(networkId); - if (network == null) { - throw new InvalidParameterValueException("Unable to find network by id", null); - } + if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.StaticNat)) { List idList = new ArrayList(); idList.add(new IdentityProxy(network, networkId, "networkId")); diff --git a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index 9290044e324..d99564fd24f 100755 --- a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -120,7 +120,7 @@ public class RemoteAccessVpnManagerImpl implements RemoteAccessVpnService, Manag } IPAddressVO ipAddress = _ipAddressDao.findById(publicIpId); - _networkMgr.checkIpForService(ipAddress, Service.Vpn); + _networkMgr.checkIpForService(ipAddress, Service.Vpn, null); RemoteAccessVpnVO vpnVO = _remoteAccessVpnDao.findByPublicIpAddress(publicIpId); diff --git a/server/test/com/cloud/network/MockNetworkManagerImpl.java b/server/test/com/cloud/network/MockNetworkManagerImpl.java index 2b94ee7733f..7b375c8d5ec 100755 --- a/server/test/com/cloud/network/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/network/MockNetworkManagerImpl.java @@ -741,7 +741,7 @@ public class MockNetworkManagerImpl implements NetworkManager, Manager, NetworkS } @Override - public boolean checkIpForService(IPAddressVO ip, Service service) { + public boolean checkIpForService(IPAddressVO ip, Service service, Long networkId) { // TODO Auto-generated method stub return false; }