From b24c673a9db6a8c4f86ba5c72c8cacd33d1c330e Mon Sep 17 00:00:00 2001 From: Vijayendra Bhamidipati Date: Thu, 23 Aug 2012 15:42:25 -0700 Subject: [PATCH] CS-16145: InterVlan - Error Report to Regular Account has reference to ACL Rule ID Reviewed-by: Vijayendra Bhamidipati Description: Replacing db ids with uuids in all instances where NetworkRuleConflictException is thrown. --- .../NetworkRuleConflictException.java | 13 ++++++- .../network/firewall/FirewallManagerImpl.java | 17 +++++---- .../lb/ElasticLoadBalancerManagerImpl.java | 10 ++--- .../lb/LoadBalancingRulesManagerImpl.java | 8 ++-- .../cloud/network/rules/RulesManagerImpl.java | 17 +++++++-- .../network/vpc/NetworkACLManagerImpl.java | 38 ++++++++++--------- .../com/cloud/network/vpc/VpcManagerImpl.java | 16 ++++---- 7 files changed, 72 insertions(+), 47 deletions(-) diff --git a/api/src/com/cloud/exception/NetworkRuleConflictException.java b/api/src/com/cloud/exception/NetworkRuleConflictException.java index a6e0ef305a4..abdf4f867eb 100644 --- a/api/src/com/cloud/exception/NetworkRuleConflictException.java +++ b/api/src/com/cloud/exception/NetworkRuleConflictException.java @@ -12,13 +12,22 @@ // Automatically generated by addcopyright.py at 04/03/2012 package com.cloud.exception; +import java.util.List; + +import com.cloud.utils.IdentityProxy; + public class NetworkRuleConflictException extends - ManagementServerException { +ManagementServerException { private static final long serialVersionUID = -294905017911859479L; - public NetworkRuleConflictException(String message) { + public NetworkRuleConflictException(String message, List idProxyList) { super(message); + if (idProxyList != null) { + for (IdentityProxy id : idProxyList) { + this.addProxyObject(id.getTableName(), id.getValue(), id.getidFieldName()); + } + } } } diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index 7263854e92f..e7358a3502b 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -37,7 +37,6 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IPAddressVO; -import com.cloud.network.IpAddress; import com.cloud.network.Network; import com.cloud.network.Network.Capability; import com.cloud.network.Network.Service; @@ -314,18 +313,18 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma } if (!oneOfRulesIsFirewall) { + List idList = new ArrayList(); + idList.add(new IdentityProxy(newRule, newRule.getSourceIpAddressId(), "ipId")); if (rule.getPurpose() == Purpose.StaticNat && newRule.getPurpose() != Purpose.StaticNat) { - throw new NetworkRuleConflictException("There is 1 to 1 Nat rule specified for the ip address id=" - + newRule.getSourceIpAddressId()); + throw new NetworkRuleConflictException("There is 1 to 1 Nat rule specified for the ip address with specified id", idList); } else if (rule.getPurpose() != Purpose.StaticNat && newRule.getPurpose() == Purpose.StaticNat) { - throw new NetworkRuleConflictException("There is already firewall rule specified for the ip address id=" - + newRule.getSourceIpAddressId()); + throw new NetworkRuleConflictException("There is already firewall rule specified for the ip address with specified id", idList); } } if (rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) { throw new NetworkRuleConflictException("New rule is for a different network than what's specified in rule " - + rule.getXid()); + + rule.getXid(), null); } if (newRule.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO) && newRule.getProtocol().equalsIgnoreCase(rule.getProtocol())) { @@ -359,8 +358,10 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma && !newRule.getProtocol().equalsIgnoreCase(rule.getProtocol())); if (!(allowPf || allowStaticNat || oneOfRulesIsFirewall)) { - throw new NetworkRuleConflictException("The range specified, " + newRule.getSourcePortStart() + "-" + newRule.getSourcePortEnd() + ", conflicts with rule " + rule.getId() - + " which has " + rule.getSourcePortStart() + "-" + rule.getSourcePortEnd()); + List idList = new ArrayList(); + idList.add(new IdentityProxy(rule, rule.getId(), "ruleId")); + throw new NetworkRuleConflictException("The range specified, " + newRule.getSourcePortStart() + "-" + newRule.getSourcePortEnd() + ", conflicts with rule with specified id " + + " which has " + rule.getSourcePortStart() + "-" + rule.getSourcePortEnd(), idList); } } } diff --git a/server/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java b/server/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java index e5cdcd18fb7..020549727e8 100644 --- a/server/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java +++ b/server/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java @@ -133,7 +133,7 @@ import com.cloud.vm.dao.NicDao; @Local(value = { ElasticLoadBalancerManager.class }) public class ElasticLoadBalancerManagerImpl implements - ElasticLoadBalancerManager, Manager, VirtualMachineGuru { +ElasticLoadBalancerManager, Manager, VirtualMachineGuru { private static final Logger s_logger = Logger .getLogger(ElasticLoadBalancerManagerImpl.class); @@ -330,7 +330,7 @@ public class ElasticLoadBalancerManagerImpl implements @Override public boolean applyLoadBalancerRules(Network network, List rules) - throws ResourceUnavailableException { + throws ResourceUnavailableException { if (rules == null || rules.isEmpty()) { return true; } @@ -452,7 +452,7 @@ public class ElasticLoadBalancerManagerImpl implements } public DomainRouterVO deployELBVm(Network guestNetwork, DeployDestination dest, Account owner, Map params) throws - ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { + ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { long dcId = dest.getDataCenter().getId(); // lock guest network @@ -531,7 +531,7 @@ public class ElasticLoadBalancerManagerImpl implements } private DomainRouterVO start(DomainRouterVO elbVm, User user, Account caller, Map params) throws StorageUnavailableException, InsufficientCapacityException, - ConcurrentOperationException, ResourceUnavailableException { + ConcurrentOperationException, ResourceUnavailableException { s_logger.debug("Starting ELB VM " + elbVm); if (_itMgr.start(elbVm, params, user, caller) != null) { return _routerDao.findById(elbVm.getId()); @@ -649,7 +649,7 @@ public class ElasticLoadBalancerManagerImpl implements } } else { s_logger.warn("ELB: Found existing load balancers matching requested new LB"); - throw new NetworkRuleConflictException("ELB: Found existing load balancers matching requested new LB"); + throw new NetworkRuleConflictException("ELB: Found existing load balancers matching requested new LB", null); } Network network = _networkMgr.getNetwork(networkId); diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index c75548da715..3d7ee4c4ed5 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -235,7 +235,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesMa serviceResponse.setName(service.getName()); if ("Lb".equalsIgnoreCase(service.getName())) { Map serviceCapabilities = serviceCapabilitiesMap - .get(service); + .get(service); if (serviceCapabilities != null) { for (Capability capability : serviceCapabilities .keySet()) { @@ -964,13 +964,15 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesMa if (ipVO == null) { throw new InvalidParameterValueException("Unable to create load balance rule; can't find/allocate source IP", null); } else if (ipVO.isOneToOneNat()) { - throw new NetworkRuleConflictException("Can't do load balance on ip address: " + ipVO.getAddress()); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipVO, ipVO.getId(), "ipId")); + throw new NetworkRuleConflictException("Can't do load balance on ip address with specified id", idList); } try { if (ipVO.getAssociatedWithNetworkId() == null) { boolean assignToVpcNtwk = network.getVpcId() != null - && ipVO.getVpcId() != null && ipVO.getVpcId().longValue() == network.getVpcId(); + && ipVO.getVpcId() != null && ipVO.getVpcId().longValue() == network.getVpcId(); if (assignToVpcNtwk) { // set networkId just for verification purposes _networkMgr.checkIpForService(ipVO, Service.Lb, lb.getNetworkId()); diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 6fc8ae625f3..c3b77fc1c75 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -316,7 +316,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { idList.add(new IdentityProxy(ipAddress, ipAddrId, "ipId")); throw new InvalidParameterValueException("Unable to create static nat rule; ip with specified ipId doesn't exist in the system", idList); } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || ipAddress.getAssociatedWithVmId() == null) { - throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddress.getId(), "ipId")); + throw new NetworkRuleConflictException("Can't do static nat on ip address with specified id", idList); } _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat, FirewallRuleType.User); @@ -514,15 +516,22 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { if (!ipAddress.isOneToOneNat()) { // Dont allow to enable static nat if PF/LB rules exist for the IP List portForwardingRules = _firewallDao.listByIpAndPurposeAndNotRevoked(ipAddress.getId(), Purpose.PortForwarding); if (portForwardingRules != null && !portForwardingRules.isEmpty()) { - throw new NetworkRuleConflictException("Failed to enable static nat for the ip address " + ipAddress + " as it already has PortForwarding rules assigned"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddress.getId(), "ipId")); + throw new NetworkRuleConflictException("Failed to enable static nat for the ip address with specified id as it already has PortForwarding rules assigned", idList); } List loadBalancingRules = _firewallDao.listByIpAndPurposeAndNotRevoked(ipAddress.getId(), Purpose.LoadBalancing); if (loadBalancingRules != null && !loadBalancingRules.isEmpty()) { - throw new NetworkRuleConflictException("Failed to enable static nat for the ip address " + ipAddress + " as it already has LoadBalancing rules assigned"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddress.getId(), "ipId")); + throw new NetworkRuleConflictException("Failed to enable static nat for the ip address with specified id as it already has LoadBalancing rules assigned", idList); } } else if (ipAddress.getAssociatedWithVmId() != null && ipAddress.getAssociatedWithVmId().longValue() != vmId) { - throw new NetworkRuleConflictException("Failed to enable static for the ip address " + ipAddress + " and vm id=" + vmId + " as it's already assigned to antoher vm"); + List idList = new ArrayList(); + idList.add(new IdentityProxy(ipAddress, ipAddress.getId(), "ipId")); + idList.add(new IdentityProxy("vm_instance", vmId, "vmId")); + throw new NetworkRuleConflictException("Failed to enable static nat for the ip address with specified id on vm with specified id as it's already assigned to antoher vm", idList); } IPAddressVO oldIP = _ipAddressDao.findByAssociatedVmId(vmId); diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java index 1e5968b14f7..ba2d3d92f5c 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java @@ -152,7 +152,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) { throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP", null); } - + //validate icmp code and type if (icmpType != null) { if (icmpType.longValue() != -1 && !NetUtils.validateIcmpType(icmpType.longValue())) { @@ -209,7 +209,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ if (network.getTrafficType() != Networks.TrafficType.Guest) { throw new InvalidParameterValueException("Network ACL can be created just for networks of type " - + Networks.TrafficType.Guest, null); + + Networks.TrafficType.Guest, null); } // Verify that the network guru supports the protocol specified @@ -226,7 +226,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ } } - + protected void detectNetworkACLConflict(FirewallRuleVO newRule) throws NetworkRuleConflictException { if (newRule.getPurpose() != Purpose.NetworkACL) { return; @@ -284,16 +284,18 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ } else if (duplicatedCidrs && ((rule.getSourcePortStart().intValue() <= newRule.getSourcePortStart().intValue() && rule.getSourcePortEnd().intValue() >= newRule.getSourcePortStart().intValue()) - || (rule.getSourcePortStart().intValue() <= newRule.getSourcePortEnd().intValue() - && rule.getSourcePortEnd().intValue() >= newRule.getSourcePortEnd().intValue()) - || (newRule.getSourcePortStart().intValue() <= rule.getSourcePortStart().intValue() - && newRule.getSourcePortEnd().intValue() >= rule.getSourcePortStart().intValue()) - || (newRule.getSourcePortStart().intValue() <= rule.getSourcePortEnd().intValue() - && newRule.getSourcePortEnd().intValue() >= rule.getSourcePortEnd().intValue()))) { + || (rule.getSourcePortStart().intValue() <= newRule.getSourcePortEnd().intValue() + && rule.getSourcePortEnd().intValue() >= newRule.getSourcePortEnd().intValue()) + || (newRule.getSourcePortStart().intValue() <= rule.getSourcePortStart().intValue() + && newRule.getSourcePortEnd().intValue() >= rule.getSourcePortStart().intValue()) + || (newRule.getSourcePortStart().intValue() <= rule.getSourcePortEnd().intValue() + && newRule.getSourcePortEnd().intValue() >= rule.getSourcePortEnd().intValue()))) { + List idList = new ArrayList(); + idList.add(new IdentityProxy(rule, rule.getId(), "ruleId")); throw new NetworkRuleConflictException("The range specified, " + newRule.getSourcePortStart() + "-" - + newRule.getSourcePortEnd() + ", conflicts with rule " + rule.getId() - + " which has " + rule.getSourcePortStart() + "-" + rule.getSourcePortEnd()); + + newRule.getSourcePortEnd() + ", conflicts with rule with specified ruleId" + rule.getId() + + " which has " + rule.getSourcePortStart() + "-" + rule.getSourcePortEnd(), idList); } } @@ -335,7 +337,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ return success; } - + @Override public FirewallRule getNetworkACL(long ACLId) { FirewallRule rule = _firewallDao.findById(ACLId); @@ -345,7 +347,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ return null; } - + @Override public List listNetworkACLs(ListNetworkACLsCmd cmd) { Long networkId = cmd.getNetworkId(); @@ -421,17 +423,17 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ return _firewallDao.listByNetworkAndPurpose(guestNtwkId, Purpose.NetworkACL); } - + @Override public boolean revokeAllNetworkACLsForNetwork(long networkId, long userId, Account caller) throws ResourceUnavailableException { List ACLs = _firewallDao.listByNetworkAndPurpose(networkId, Purpose.NetworkACL); - + if (ACLs.isEmpty()) { s_logger.debug("Found no network ACLs for network id=" + networkId); return true; } - + if (s_logger.isDebugEnabled()) { s_logger.debug("Releasing " + ACLs.size() + " Network ACLs for network id=" + networkId); } @@ -441,7 +443,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ // need to send them one by one revokeNetworkACL(ACL.getId(), false, caller, Account.ACCOUNT_ID_SYSTEM); } - + List ACLsToRevoke = _firewallDao.listByNetworkAndPurpose(networkId, Purpose.NetworkACL); // now send everything to the backend @@ -449,7 +451,7 @@ public class NetworkACLManagerImpl implements Manager,NetworkACLManager{ if (s_logger.isDebugEnabled()) { s_logger.debug("Successfully released Network ACLs for network id=" + networkId + " and # of rules now = " - + ACLs.size()); + + ACLs.size()); } return success; diff --git a/server/src/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/com/cloud/network/vpc/VpcManagerImpl.java index 1831ce22a15..8124df233ca 100644 --- a/server/src/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/com/cloud/network/vpc/VpcManagerImpl.java @@ -579,7 +579,7 @@ public class VpcManagerImpl implements VpcManager, Manager{ if (!NetUtils.isValidCIDR(cidr)) { throw new InvalidParameterValueException("Invalid CIDR specified " + cidr, null); } - + //cidr has to be RFC 1918 complient if (!NetUtils.validateGuestCidr(cidr)) { throw new InvalidParameterValueException("Guest Cidr " + cidr + " is not RFC1918 compliant", null); @@ -942,7 +942,7 @@ public class VpcManagerImpl implements VpcManager, Manager{ Account networkOwner, Vpc vpc, Long networkId, String gateway) { NetworkOffering guestNtwkOff = _configMgr.getNetworkOffering(ntwkOffId); - + if (guestNtwkOff == null) { throw new InvalidParameterValueException("Can't find network offering by id specified", null); } @@ -999,7 +999,7 @@ public class VpcManagerImpl implements VpcManager, Manager{ + " is supported for network offering that can be used in VPC", null); } } - + //2) Only Isolated networks with Source nat service enabled can be added to vpc if (!(guestNtwkOff.getGuestType() == GuestType.Isolated && supportedSvcs.contains(Service.SourceNat))) { @@ -1105,7 +1105,7 @@ public class VpcManagerImpl implements VpcManager, Manager{ if (vpcElement == null) { throw new CloudRuntimeException("Failed to initialize vpc element"); } - + return vpcElement; } @@ -1586,13 +1586,13 @@ public class VpcManagerImpl implements VpcManager, Manager{ if (!NetUtils.isValidCIDR(cidr)){ throw new InvalidParameterValueException("Invalid format for cidr " + cidr, null); } - + //validate the cidr //1) CIDR should be outside of VPC cidr for guest networks if (NetUtils.isNetworksOverlap(vpc.getCidr(), cidr)) { throw new InvalidParameterValueException("CIDR should be outside of VPC cidr " + vpc.getCidr(), null); } - + //2) CIDR should be outside of link-local cidr if (NetUtils.isNetworksOverlap(vpc.getCidr(), NetUtils.getLinkLocalCIDR())) { throw new InvalidParameterValueException("CIDR should be outside of link local cidr " + NetUtils.getLinkLocalCIDR(), null); @@ -1698,7 +1698,9 @@ public class VpcManagerImpl implements VpcManager, Manager{ } if (NetUtils.isNetworksOverlap(route.getCidr(), newRoute.getCidr())) { - throw new NetworkRuleConflictException("New static route cidr conflicts with existing route " + route); + List idList = new ArrayList(); + idList.add(new IdentityProxy(route, route.getId(), "routeId")); + throw new NetworkRuleConflictException("New static route cidr conflicts with existing route with specified id", idList); } } }