From d817f3c364ad8f95ca755eafedde5a349c4ca5ee Mon Sep 17 00:00:00 2001 From: Chiradeep Vittal Date: Mon, 12 Sep 2011 12:17:42 -0700 Subject: [PATCH] Revert "bug 10617: Added Egress rules to Security groups." revert pending review This reverts commit a19212703b9734ebd44ebf55cfdd81ebdc9d7fe4. --- .../computing/LibvirtComputingResource.java | 8 ++--- .../xen/resource/CitrixResourceBase.java | 2 -- scripts/vm/network/security_group.py | 35 +++---------------- .../response/SecurityGroupResultObject.java | 8 +++-- .../security/SecurityGroupManagerImpl.java | 10 +++--- 5 files changed, 19 insertions(+), 44 deletions(-) diff --git a/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java b/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java index d4eb27762a1..900174e2e24 100644 --- a/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java +++ b/agent/src/com/cloud/agent/resource/computing/LibvirtComputingResource.java @@ -929,7 +929,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } else if (cmd instanceof CopyVolumeCommand) { return execute((CopyVolumeCommand)cmd); } else { - s_logger.warn("Unsupported command :"+cmd.toString()); + s_logger.warn("Unsupported command "); return Answer.createUnsupportedCommandAnswer(cmd); } } catch (final IllegalArgumentException e) { @@ -1622,7 +1622,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv cmd.stringifyRules(), vif, brname); if (!result) { - s_logger.warn("Failed to program Ingress network rules for vm " + cmd.getVmName()); + s_logger.warn("Failed to program network rules for vm " + cmd.getVmName()); return new SecurityIngressRuleAnswer(cmd, false, "programming network rules failed"); } else { s_logger.debug("Programmed network rules for vm " + cmd.getVmName() + " guestIp=" + cmd.getGuestIp() + ", numrules=" + cmd.getRuleSet().length); @@ -1650,7 +1650,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv cmd.stringifyRules(), vif, brname); if (!result) { - s_logger.warn("Failed to program Egress network rules for vm " + cmd.getVmName()); + s_logger.warn("Failed to program network rules for vm " + cmd.getVmName()); return new SecurityEgressRuleAnswer(cmd, false, "programming network rules failed"); } else { s_logger.debug("Programmed network rules for vm " + cmd.getVmName() + " guestIp=" + cmd.getGuestIp() + ", numrules=" + cmd.getRuleSet().length); @@ -3516,7 +3516,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv cmd.add("--vmid", vmId); cmd.add("--vmip", guestIP); /* type of the rule : ingress or egress */ - cmd.add("--ruletype", type); + cmd.add("--type", type); cmd.add("--sig", sig); cmd.add("--seq", seq); cmd.add("--vmmac", mac); diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index 712b5e32ba3..027710a4e8b 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -482,8 +482,6 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return execute((CheckSshCommand)cmd); } else if (clazz == SecurityIngressRulesCmd.class) { return execute((SecurityIngressRulesCmd) cmd); - } else if (clazz == SecurityEgressRulesCmd.class) { - return execute((SecurityEgressRulesCmd) cmd); } else if (clazz == OvsCreateGreTunnelCommand.class) { return execute((OvsCreateGreTunnelCommand)cmd); } else if (clazz == OvsSetTagAndFlowCommand.class) { diff --git a/scripts/vm/network/security_group.py b/scripts/vm/network/security_group.py index 37f94f9cdcf..de59df698c6 100755 --- a/scripts/vm/network/security_group.py +++ b/scripts/vm/network/security_group.py @@ -82,7 +82,6 @@ def ipset(ipsetname, proto, start, end, ips): def destroy_network_rules_for_vm(vm_name, vif=None): vmchain = vm_name - vmchain_egress = vm_name + "-egress" vmchain_default = None delete_rules_for_vm_in_bridge_firewall_chain(vm_name) @@ -112,19 +111,7 @@ def destroy_network_rules_for_vm(vm_name, vif=None): execute("iptables -X " + vmchain) except: logging.debug("Ignoring failure to delete chain " + vmchain) - - - try: - execute("iptables -F " + vmchain_egress) - except: - logging.debug("Ignoring failure to delete chain " + vmchain_egress) - try: - execute("iptables -X " + vmchain_egress) - except: - logging.debug("Ignoring failure to delete chain " + vmchain_egress) - - if vif is not None: try: dnats = execute("iptables -t nat -S | grep " + vif + " | sed 's/-A/-D/'").split("\n") @@ -259,7 +246,6 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_mac, vif, brname): domID = getvmId(vm_name) delete_rules_for_vm_in_bridge_firewall_chain(vmName) vmchain = vm_name - vmchain_egress = vm_name +"-egress" vmchain_default = '-'.join(vmchain.split('-')[:-1]) + "-def" destroy_ebtables_rules(vmName, vif) @@ -268,12 +254,7 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_mac, vif, brname): execute("iptables -N " + vmchain) except: execute("iptables -F " + vmchain) - - try: - execute("iptables -N " + vmchain_egress) - except: - execute("iptables -F " + vmchain_egress) - + try: execute("iptables -N " + vmchain_default) except: @@ -289,7 +270,7 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_mac, vif, brname): #don't let vm spoof its ip address if vm_ip is not None: - execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " --source " + vm_ip + " -j " + vmchain_egress) + execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " --source " + vm_ip + " -j ACCEPT") execute("iptables -A " + vmchain_default + " -j " + vmchain) execute("iptables -A " + vmchain + " -j DROP") except: @@ -571,17 +552,12 @@ def remove_rule_log_for_vm(vmName): return result -def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif, brname,ruletype): +def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif, brname): try: vmName = vm_name domId = getvmId(vmName) + vmchain = vm_name - if ruletype == 'egress': - vmchain = vm_name + "-egress" - else: - vmchain = vm_name - - changes = [] changes = check_rule_log_for_vm(vmName, vm_id, vm_ip, domId, signature, seqno) @@ -728,7 +704,6 @@ if __name__ == '__main__': parser.add_option("--vmid", dest="vmID") parser.add_option("--vmmac", dest="vmMAC") parser.add_option("--vif", dest="vif") - parser.add_option("--ruletype", dest="ruletype") parser.add_option("--sig", dest="sig") parser.add_option("--seq", dest="seq") parser.add_option("--rules", dest="rules") @@ -749,7 +724,7 @@ if __name__ == '__main__': elif cmd == "get_rule_logs_for_vms": get_rule_logs_for_vms() elif cmd == "add_network_rules": - add_network_rules(option.vmName, option.vmID, option.vmIP, option.sig, option.seq, option.vmMAC, option.rules, option.vif, option.brname,option.ruletype) + add_network_rules(option.vmName, option.vmID, option.vmIP, option.sig, option.seq, option.vmMAC, option.rules, option.vif, option.brname) elif cmd == "cleanup_rules": cleanup_rules() elif cmd == "post_default_network_rules": diff --git a/server/src/com/cloud/api/response/SecurityGroupResultObject.java b/server/src/com/cloud/api/response/SecurityGroupResultObject.java index 6b25382101f..221955b4238 100644 --- a/server/src/com/cloud/api/response/SecurityGroupResultObject.java +++ b/server/src/com/cloud/api/response/SecurityGroupResultObject.java @@ -175,6 +175,11 @@ public class SecurityGroupResultObject { currentGroup = groupResult; } + SecurityGroupRulesVO dummyIngressobj=new SecurityGroupRulesVO(); + SecurityGroupEgressRulesVO dummyEgressobj=new SecurityGroupEgressRulesVO() ; +String str=dummyIngressobj.getClass().getName(); + +String s1=netGroupRule.getClass().getSimpleName(); if (netGroupRule.getRuleId() != null && netGroupRule.getClass().getSimpleName().indexOf("SecurityGroupRulesVO") != -1) { // there's at least one ingress rule for this network group, add the ingress rule data @@ -241,9 +246,6 @@ public class SecurityGroupResultObject { if (!ingressDataList.isEmpty()) { currentGroup.setIngressRules(ingressDataList); } - if (!egressDataList.isEmpty()) { - currentGroup.setEgressRules(egressDataList); - } resultObjects.add(currentGroup); } } diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index 90a6de68893..789d6e99639 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -988,10 +988,10 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG Account caller = UserContext.current().getCaller(); Long id = cmd.getId(); - EgressRuleVO rule = _egressRuleDao.findById(id); + IngressRuleVO rule = _ingressRuleDao.findById(id); if (rule == null) { - s_logger.debug("Unable to find egress rule with id " + id); - throw new InvalidParameterValueException("Unable to find egress rule with id " + id); + s_logger.debug("Unable to find ingress rule with id " + id); + throw new InvalidParameterValueException("Unable to find ingress rule with id " + id); } // Check permissions @@ -1010,8 +1010,8 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG return false; } - _egressRuleDao.remove(id); - s_logger.debug("revokeSecurityGroupEgress succeeded for ingress rule id: " + id); + _ingressRuleDao.remove(id); + s_logger.debug("revokeSecurityGroupIngress succeeded for ingress rule id: " + id); final Set affectedVms = new HashSet(); affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(groupHandle.getId()));