From ffce2d601eecab625376fd54f23d7741b9a64e7e Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Sat, 26 Dec 2015 09:00:31 +0100 Subject: [PATCH 1/5] remove autogenerated pydev files --- .gitignore | 2 ++ plugins/hypervisors/kvm/.pydevproject | 25 ------------------------ scripts/.pydevproject | 28 --------------------------- 3 files changed, 2 insertions(+), 53 deletions(-) delete mode 100644 plugins/hypervisors/kvm/.pydevproject delete mode 100644 scripts/.pydevproject diff --git a/.gitignore b/.gitignore index 58eafaf68d07..48aa29b69e30 100644 --- a/.gitignore +++ b/.gitignore @@ -96,3 +96,5 @@ tools/appliance/box/ .pydevproject systemvm/.pydevproject test/.pydevprojec +plugins/hypervisors/kvm/.pydevproject +scripts/.pydevproject diff --git a/plugins/hypervisors/kvm/.pydevproject b/plugins/hypervisors/kvm/.pydevproject deleted file mode 100644 index d4a984a39095..000000000000 --- a/plugins/hypervisors/kvm/.pydevproject +++ /dev/null @@ -1,25 +0,0 @@ - - - - - -Default -python 2.7 - diff --git a/scripts/.pydevproject b/scripts/.pydevproject deleted file mode 100644 index 43efe469ed20..000000000000 --- a/scripts/.pydevproject +++ /dev/null @@ -1,28 +0,0 @@ - - - - - -Default -python 2.6 - -/scripts - - From 1ead444cca7c11181899627698b7a0779812c76c Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 25 Dec 2015 13:51:50 +0100 Subject: [PATCH 2/5] security rules test --- .../agent/api/SecurityGroupRulesCmd.java | 2 + .../agent/api/SecurityGroupRulesCmdTest.java | 101 ++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java diff --git a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java index b9bdef564694..098d82c83474 100644 --- a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java +++ b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java @@ -32,6 +32,8 @@ import com.cloud.utils.net.NetUtils; public class SecurityGroupRulesCmd extends Command { + static final String EGRESS_RULE = "E:"; + static final String INGRESS_RULE = "I:"; private static Logger s_logger = Logger.getLogger(SecurityGroupRulesCmd.class); public static class IpPortAndProto { diff --git a/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java b/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java new file mode 100644 index 000000000000..2b094c5a404f --- /dev/null +++ b/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java @@ -0,0 +1,101 @@ +// +// 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. +// + +package com.cloud.agent.api; + +import static org.junit.Assert.assertTrue; + +import java.util.List; +import java.util.Vector; + +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; + +import com.cloud.agent.api.SecurityGroupRulesCmd.IpPortAndProto; + +/** + * @author daan + * + */ +@RunWith(MockitoJUnitRunner.class) +public class SecurityGroupRulesCmdTest { + private SecurityGroupRulesCmd securityGroupRulesCmd; + + /** + * @throws java.lang.Exception + */ + @BeforeClass + public static void setUpBeforeClass() throws Exception { + } + + /** + * @throws java.lang.Exception + */ + @Before + public void setUp() throws Exception { + String guestIp = "10.10.10.10"; + String guestMac = "aa:aa:aa:aa:aa:aa"; + String vmName = "vm"; + Long vmId = 1L; + String signature = "sig"; + Long seqNum = 0L; + String proto = "abc"; + int startPort = 1; + int endPort = 2; + String[] allowedCidrs = new String[] {"1.2.3.4/5","6.7.8.9/0"}; + IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{new IpPortAndProto(proto, startPort, endPort, allowedCidrs)}; + IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{new IpPortAndProto(proto, startPort, endPort, allowedCidrs)}; + List secIps = new Vector(); + securityGroupRulesCmd = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); + } + + /** + * Test method for {@link com.cloud.agent.api.SecurityGroupRulesCmd#stringifyRules()}. + */ + @Test + public void testStringifyRules() throws Exception { + String a = securityGroupRulesCmd.stringifyRules(); +// do verification on a + assertTrue(a.contains(SecurityGroupRulesCmd.EGRESS_RULE)); + } + + /** + * Test method for {@link com.cloud.agent.api.SecurityGroupRulesCmd#stringifyCompressedRules()}. + */ + @Test + public void testStringifyCompressedRules() throws Exception { + String a = securityGroupRulesCmd.stringifyCompressedRules(); +// do verification on a + assertTrue(a.contains(SecurityGroupRulesCmd.EGRESS_RULE)); + } + + /** + * Test method for {@link com.cloud.agent.api.SecurityGroupRulesCmd#compressStringifiedRules()}. + */ + @Test + public void testCompressStringifiedRules() throws Exception { + String compressed = "eJzztEpMSrYytDKyMtQz0jPWM9E31THTM9ez0LPUN9Dxc40IUXAlrAQAPdoP3Q=="; + String a = securityGroupRulesCmd.compressStringifiedRules(); + assertTrue(compressed.equals(a)); + } + +} From d1c1bde01bda6e2aa01fd1fa5226b4067ff517ce Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 25 Dec 2015 13:04:11 +0100 Subject: [PATCH 3/5] code cleanup --- .../agent/api/SecurityGroupRulesCmd.java | 94 +++++-------------- 1 file changed, 26 insertions(+), 68 deletions(-) diff --git a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java index 098d82c83474..fbc3835d43b9 100644 --- a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java +++ b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java @@ -47,7 +47,7 @@ public IpPortAndProto() { } public IpPortAndProto(String proto, int startPort, int endPort, String[] allowedCidrs) { - super(); + this(); this.proto = proto; this.startPort = startPort; this.endPort = endPort; @@ -93,7 +93,7 @@ public SecurityGroupRulesCmd() { public SecurityGroupRulesCmd(String guestIp, String guestMac, String vmName, Long vmId, String signature, Long seqNum, IpPortAndProto[] ingressRuleSet, IpPortAndProto[] egressRuleSet) { - super(); + this(); this.guestIp = guestIp; this.vmName = vmName; this.ingressRuleSet = ingressRuleSet; @@ -110,19 +110,7 @@ public SecurityGroupRulesCmd(String guestIp, String guestMac, String vmName, Lon public SecurityGroupRulesCmd(String guestIp, String guestMac, String vmName, Long vmId, String signature, Long seqNum, IpPortAndProto[] ingressRuleSet, IpPortAndProto[] egressRuleSet, List secIps) { - super(); - this.guestIp = guestIp; - this.vmName = vmName; - this.ingressRuleSet = ingressRuleSet; - this.egressRuleSet = egressRuleSet; - this.guestMac = guestMac; - this.signature = signature; - this.seqNum = seqNum; - this.vmId = vmId; - if (signature == null) { - String stringified = stringifyRules(); - this.signature = DigestUtils.md5Hex(stringified); - } + this(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet); this.secIps = secIps; } @@ -159,27 +147,6 @@ public String getVmName() { return vmName; } - public String stringifyRules() { - StringBuilder ruleBuilder = new StringBuilder(); - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : getIngressRuleSet()) { - ruleBuilder.append("I:").append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); - for (String cidr : ipPandP.getAllowedCidrs()) { - ruleBuilder.append(cidr).append(","); - } - ruleBuilder.append("NEXT"); - ruleBuilder.append(" "); - } - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : getEgressRuleSet()) { - ruleBuilder.append("E:").append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); - for (String cidr : ipPandP.getAllowedCidrs()) { - ruleBuilder.append(cidr).append(","); - } - ruleBuilder.append("NEXT"); - ruleBuilder.append(" "); - } - return ruleBuilder.toString(); - } - //convert cidrs in the form "a.b.c.d/e" to "hexvalue of 32bit ip/e" private String compressCidr(String cidr) { String[] toks = cidr.split("/"); @@ -200,27 +167,35 @@ public String getSecIpsString() { return sb.toString(); } + public String stringifyRules() { + StringBuilder ruleBuilder = new StringBuilder(); + stringifyRulesFor(getIngressRuleSet(), INGRESS_RULE, false, ruleBuilder); + stringifyRulesFor(getEgressRuleSet(), EGRESS_RULE, false, ruleBuilder); + return ruleBuilder.toString(); + } + public String stringifyCompressedRules() { StringBuilder ruleBuilder = new StringBuilder(); - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : getIngressRuleSet()) { - ruleBuilder.append("I:").append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); - for (String cidr : ipPandP.getAllowedCidrs()) { - //convert cidrs in the form "a.b.c.d/e" to "hexvalue of 32bit ip/e" - ruleBuilder.append(compressCidr(cidr)).append(","); - } - ruleBuilder.append("NEXT"); - ruleBuilder.append(" "); - } - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : getEgressRuleSet()) { - ruleBuilder.append("E:").append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); + stringifyRulesFor(getEgressRuleSet(), INGRESS_RULE, true, ruleBuilder); + stringifyRulesFor(getEgressRuleSet(), EGRESS_RULE, true, ruleBuilder); + return ruleBuilder.toString(); + } + + /** + * @param ipPandPs + * @param gression + * @param compress + * @param ruleBuilder + */ + void stringifyRulesFor(SecurityGroupRulesCmd.IpPortAndProto[] ipPandPs, String gression, boolean compress, StringBuilder ruleBuilder) { + for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : ipPandPs) { + ruleBuilder.append(gression).append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); for (String cidr : ipPandP.getAllowedCidrs()) { - //convert cidrs in the form "a.b.c.d/e" to "hexvalue of 32bit ip/e" - ruleBuilder.append(compressCidr(cidr)).append(","); + ruleBuilder.append(compress?compressCidr(cidr):cidr).append(","); } ruleBuilder.append("NEXT"); ruleBuilder.append(" "); } - return ruleBuilder.toString(); } /* @@ -228,24 +203,7 @@ public String stringifyCompressedRules() { * to scale beyond 8k cidrs. */ public String compressStringifiedRules() { - StringBuilder ruleBuilder = new StringBuilder(); - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : getIngressRuleSet()) { - ruleBuilder.append("I:").append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); - for (String cidr : ipPandP.getAllowedCidrs()) { - ruleBuilder.append(cidr).append(","); - } - ruleBuilder.append("NEXT"); - ruleBuilder.append(" "); - } - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : getEgressRuleSet()) { - ruleBuilder.append("E:").append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); - for (String cidr : ipPandP.getAllowedCidrs()) { - ruleBuilder.append(cidr).append(","); - } - ruleBuilder.append("NEXT"); - ruleBuilder.append(" "); - } - String stringified = ruleBuilder.toString(); + String stringified = stringifyRules(); ByteArrayOutputStream out = new ByteArrayOutputStream(); try { //Note : not using GZipOutputStream since that is for files From d39182f9d432ff477b7021ce805980c35ba39408 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 25 Dec 2015 16:47:14 +0100 Subject: [PATCH 4/5] deal with PMD warnings --- .../agent/api/SecurityGroupRulesCmd.java | 168 +++---- .../SecurityGroupHttpClient.java | 410 +++++++++--------- ...bvirtSecurityGroupRulesCommandWrapper.java | 2 +- .../LibvirtComputingResourceTest.java | 22 +- .../cloud/ovm/hypervisor/OvmResourceBase.java | 4 +- .../agent/manager/MockVmManagerImpl.java | 2 +- ...itrixSecurityGroupRulesCommandWrapper.java | 2 +- .../xenbase/CitrixRequestWrapperTest.java | 14 +- .../security/SecurityGroupManagerImpl2.java | 4 +- 9 files changed, 329 insertions(+), 299 deletions(-) diff --git a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java index fbc3835d43b9..a7f44251ddcf 100644 --- a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java +++ b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java @@ -21,6 +21,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.zip.DeflaterOutputStream; @@ -32,34 +33,47 @@ import com.cloud.utils.net.NetUtils; public class SecurityGroupRulesCmd extends Command { - static final String EGRESS_RULE = "E:"; - static final String INGRESS_RULE = "I:"; - private static Logger s_logger = Logger.getLogger(SecurityGroupRulesCmd.class); + private static final char RULE_TARGET_SEPARATOR = ','; + private static final char RULE_COMMAND_SEPARATOR = ':'; + protected static final String EGRESS_RULE = "E:"; + protected static final String INGRESS_RULE = "I:"; + private static final Logger LOGGER = Logger.getLogger(SecurityGroupRulesCmd.class); + + private final String guestIp; + private final String vmName; + private final String guestMac; + private final String signature; + private final Long seqNum; + private final Long vmId; + private Long msId; + private List ingressRuleSet; + private List egressRuleSet; + private final List secIps; public static class IpPortAndProto { - private String proto; - private int startPort; - private int endPort; + private final String proto; + private final int startPort; + private final int endPort; @LogLevel(Log4jLevel.Trace) - private String[] allowedCidrs; + private List allowedCidrs; - public IpPortAndProto() { - } - - public IpPortAndProto(String proto, int startPort, int endPort, String[] allowedCidrs) { - this(); + public IpPortAndProto(final String proto, final int startPort, final int endPort, final String... allowedCidrs) { + super(); this.proto = proto; this.startPort = startPort; this.endPort = endPort; - this.allowedCidrs = allowedCidrs; + setAllowedCidrs(allowedCidrs); } - public String[] getAllowedCidrs() { + public List getAllowedCidrs() { return allowedCidrs; } - public void setAllowedCidrs(String[] allowedCidrs) { - this.allowedCidrs = allowedCidrs; + public void setAllowedCidrs(final String... allowedCidrs) { + this.allowedCidrs = new ArrayList(); + for (final String allowedCidr : allowedCidrs) { + this.allowedCidrs.add(allowedCidr); + } } public String getProto() { @@ -76,41 +90,29 @@ public int getEndPort() { } - String guestIp; - String vmName; - String guestMac; - String signature; - Long seqNum; - Long vmId; - Long msId; - IpPortAndProto[] ingressRuleSet; - IpPortAndProto[] egressRuleSet; - private List secIps; - - public SecurityGroupRulesCmd() { - super(); - } - - public SecurityGroupRulesCmd(String guestIp, String guestMac, String vmName, Long vmId, String signature, Long seqNum, IpPortAndProto[] ingressRuleSet, - IpPortAndProto[] egressRuleSet) { - this(); + public SecurityGroupRulesCmd( + final String guestIp, + final String guestMac, + final String vmName, + final Long vmId, + final String signature, + final Long seqNum, + final IpPortAndProto[] ingressRuleSet, + final IpPortAndProto[] egressRuleSet, + final List secIps) { this.guestIp = guestIp; this.vmName = vmName; - this.ingressRuleSet = ingressRuleSet; - this.egressRuleSet = egressRuleSet; + setIngressRuleSet(ingressRuleSet); + this.setEgressRuleSet(egressRuleSet); this.guestMac = guestMac; - this.signature = signature; this.seqNum = seqNum; this.vmId = vmId; if (signature == null) { - String stringified = stringifyRules(); + final String stringified = stringifyRules(); this.signature = DigestUtils.md5Hex(stringified); + } else { + this.signature = signature; } - } - - public SecurityGroupRulesCmd(String guestIp, String guestMac, String vmName, Long vmId, String signature, Long seqNum, IpPortAndProto[] ingressRuleSet, - IpPortAndProto[] egressRuleSet, List secIps) { - this(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet); this.secIps = secIps; } @@ -119,20 +121,26 @@ public boolean executeInSequence() { return true; } - public IpPortAndProto[] getIngressRuleSet() { + public List getIngressRuleSet() { return ingressRuleSet; } - public void setIngressRuleSet(IpPortAndProto[] ingressRuleSet) { - this.ingressRuleSet = ingressRuleSet; + public void setIngressRuleSet(final IpPortAndProto... ingressRuleSet) { + this.ingressRuleSet = new ArrayList(); + for(final IpPortAndProto rule: ingressRuleSet) { + this.ingressRuleSet.add(rule); + } } - public IpPortAndProto[] getEgressRuleSet() { + public List getEgressRuleSet() { return egressRuleSet; } - public void setEgressRuleSet(IpPortAndProto[] egressRuleSet) { - this.egressRuleSet = egressRuleSet; + public void setEgressRuleSet(final IpPortAndProto... egressRuleSet) { + this.egressRuleSet = new ArrayList(); + for(final IpPortAndProto rule: egressRuleSet) { + this.egressRuleSet.add(rule); + } } public String getGuestIp() { @@ -148,35 +156,35 @@ public String getVmName() { } //convert cidrs in the form "a.b.c.d/e" to "hexvalue of 32bit ip/e" - private String compressCidr(String cidr) { - String[] toks = cidr.split("/"); - long ipnum = NetUtils.ip2Long(toks[0]); + private String compressCidr(final String cidr) { + final String[] toks = cidr.split("/"); + final long ipnum = NetUtils.ip2Long(toks[0]); return Long.toHexString(ipnum) + "/" + toks[1]; } public String getSecIpsString() { - StringBuilder sb = new StringBuilder(); - List ips = getSecIps(); + final StringBuilder sb = new StringBuilder(); + final List ips = getSecIps(); if (ips == null) { - return "0:"; + sb.append("0:"); } else { - for (String ip : ips) { - sb.append(ip).append(":"); + for (final String ip : ips) { + sb.append(ip).append(RULE_COMMAND_SEPARATOR); } } return sb.toString(); } public String stringifyRules() { - StringBuilder ruleBuilder = new StringBuilder(); + final StringBuilder ruleBuilder = new StringBuilder(); stringifyRulesFor(getIngressRuleSet(), INGRESS_RULE, false, ruleBuilder); stringifyRulesFor(getEgressRuleSet(), EGRESS_RULE, false, ruleBuilder); return ruleBuilder.toString(); } public String stringifyCompressedRules() { - StringBuilder ruleBuilder = new StringBuilder(); - stringifyRulesFor(getEgressRuleSet(), INGRESS_RULE, true, ruleBuilder); + final StringBuilder ruleBuilder = new StringBuilder(); + stringifyRulesFor(getIngressRuleSet(), INGRESS_RULE, true, ruleBuilder); stringifyRulesFor(getEgressRuleSet(), EGRESS_RULE, true, ruleBuilder); return ruleBuilder.toString(); } @@ -187,14 +195,17 @@ public String stringifyCompressedRules() { * @param compress * @param ruleBuilder */ - void stringifyRulesFor(SecurityGroupRulesCmd.IpPortAndProto[] ipPandPs, String gression, boolean compress, StringBuilder ruleBuilder) { - for (SecurityGroupRulesCmd.IpPortAndProto ipPandP : ipPandPs) { - ruleBuilder.append(gression).append(ipPandP.getProto()).append(":").append(ipPandP.getStartPort()).append(":").append(ipPandP.getEndPort()).append(":"); - for (String cidr : ipPandP.getAllowedCidrs()) { - ruleBuilder.append(compress?compressCidr(cidr):cidr).append(","); + private void stringifyRulesFor( + final List ipPandPs, + final String gression, + final boolean compress, + final StringBuilder ruleBuilder) { + for (final IpPortAndProto ipPandP : ipPandPs) { + ruleBuilder.append(gression).append(ipPandP.getProto()).append(RULE_COMMAND_SEPARATOR).append(ipPandP.getStartPort()).append(RULE_COMMAND_SEPARATOR).append(ipPandP.getEndPort()).append(RULE_COMMAND_SEPARATOR); + for (final String cidr : ipPandP.getAllowedCidrs()) { + ruleBuilder.append(compress?compressCidr(cidr):cidr).append(RULE_TARGET_SEPARATOR); } - ruleBuilder.append("NEXT"); - ruleBuilder.append(" "); + ruleBuilder.append("NEXT "); } } @@ -203,19 +214,20 @@ void stringifyRulesFor(SecurityGroupRulesCmd.IpPortAndProto[] ipPandPs, String g * to scale beyond 8k cidrs. */ public String compressStringifiedRules() { - String stringified = stringifyRules(); - ByteArrayOutputStream out = new ByteArrayOutputStream(); + final String stringified = stringifyRules(); + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + String encodedResult = null; try { //Note : not using GZipOutputStream since that is for files //GZipOutputStream gives a different header, although the compression is the same - DeflaterOutputStream dzip = new DeflaterOutputStream(out); + final DeflaterOutputStream dzip = new DeflaterOutputStream(out); dzip.write(stringified.getBytes()); dzip.close(); + encodedResult = Base64.encodeBase64String(out.toByteArray()); } catch (IOException e) { - s_logger.warn("Exception while compressing security group rules"); - return null; + LOGGER.warn("Exception while compressing security group rules"); } - return Base64.encodeBase64String(out.toByteArray()); + return encodedResult; } public String getSignature() { @@ -237,16 +249,16 @@ public Long getVmId() { public int getTotalNumCidrs() { //useful for logging int count = 0; - for (IpPortAndProto i : ingressRuleSet) { - count += i.allowedCidrs.length; + for (final IpPortAndProto i : ingressRuleSet) { + count += i.allowedCidrs.size(); } - for (IpPortAndProto i : egressRuleSet) { - count += i.allowedCidrs.length; + for (final IpPortAndProto i : egressRuleSet) { + count += i.allowedCidrs.size(); } return count; } - public void setMsId(long msId) { + public void setMsId(final long msId) { this.msId = msId; } diff --git a/plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/SecurityGroupHttpClient.java b/plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/SecurityGroupHttpClient.java index b9e485889507..b100929da961 100644 --- a/plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/SecurityGroupHttpClient.java +++ b/plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/SecurityGroupHttpClient.java @@ -1,205 +1,205 @@ -// 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. -// -// Automatically generated by addcopyright.py at 01/29/2013 -// Apache License, Version 2.0 (the "License"); you may not use this -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// -// Automatically generated by addcopyright.py at 04/03/2012 - -package com.cloud.baremetal.networkservice; - -import com.cloud.agent.api.SecurityGroupRuleAnswer; -import com.cloud.agent.api.SecurityGroupRulesCmd; -import com.cloud.agent.api.SecurityGroupRulesCmd.IpPortAndProto; -import com.cloud.baremetal.networkservice.schema.SecurityGroupRule; -import com.cloud.baremetal.networkservice.schema.SecurityGroupVmRuleSet; -import com.cloud.utils.Pair; -import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; -import org.apache.commons.httpclient.methods.PostMethod; -import org.apache.commons.httpclient.methods.StringRequestEntity; -import org.apache.log4j.Logger; - -import javax.xml.bind.JAXBContext; -import javax.xml.bind.Marshaller; -import java.io.StringWriter; -import java.net.SocketTimeoutException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.concurrent.TimeUnit; - -public class SecurityGroupHttpClient { - private static final Logger logger = Logger.getLogger(SecurityGroupHttpClient.class); - private static final String ARG_NAME = "args"; - private static final String COMMAND = "command"; - private JAXBContext context; - private int port; - private static HttpClient httpClient; - static { - MultiThreadedHttpConnectionManager connman = new MultiThreadedHttpConnectionManager(); - httpClient = new HttpClient(connman); - httpClient.setConnectionTimeout(5000); - } - - private enum OpConstant { - setRules, echo, - } - - public SecurityGroupHttpClient() { - try { - context = JAXBContext.newInstance(SecurityGroupRule.class, SecurityGroupVmRuleSet.class); - port = 9988; - } catch (Exception e) { - throw new CloudRuntimeException( - "Unable to create JAXBContext for security group", e); - } - } - - private List generateRules(IpPortAndProto[] ipps) { - List rules = new ArrayList( - ipps.length); - for (SecurityGroupRulesCmd.IpPortAndProto ipp : ipps) { - SecurityGroupRule r = new SecurityGroupRule(); - r.setProtocol(ipp.getProto()); - r.setStartPort(ipp.getStartPort()); - r.setEndPort(ipp.getEndPort()); - for (String cidr : ipp.getAllowedCidrs()) { - r.getIp().add(cidr); - } - rules.add(r); - } - return rules; - } - - public HashMap> sync(String vmName, Long vmId, String agentIp) { - HashMap> states = new HashMap>(); - PostMethod post = new PostMethod(String.format("http://%s:%s/", agentIp, getPort())); - try { - post.addRequestHeader("command", "sync"); - if (httpClient.executeMethod(post) != 200) { - logger.debug(String.format("echoing baremetal security group agent on %s got error: %s", agentIp, post.getResponseBodyAsString())); - } else { - String res = post.getResponseBodyAsString(); - // res = ';'.join([vmName, vmId, seqno]) - String[] rulelogs = res.split(","); - if (rulelogs.length != 6) { - logger.debug(String.format("host[%s] returns invalid security group sync document[%s], reset rules", agentIp, res)); - states.put(vmName, new Pair(vmId, -1L)); - return states; - } - Pair p = new Pair(Long.valueOf(rulelogs[1]), Long.valueOf(rulelogs[5])); - states.put(rulelogs[0], p); - return states; - } - } catch (SocketTimeoutException se) { - logger.warn(String.format("unable to sync security group rules on host[%s], %s", agentIp, se.getMessage())); - } catch (Exception e) { - logger.warn(String.format("unable to sync security group rules on host[%s]", agentIp), e); - } finally { - if (post != null) { - post.releaseConnection(); - } - } - return states; - } - - - public boolean echo(String agentIp, long l, long m) { - boolean ret = false; - int count = 1; - while (true) { - try { - Thread.sleep(m); - count++; - } catch (InterruptedException e1) { - logger.warn("", e1); - break; - } - PostMethod post = new PostMethod(String.format("http://%s:%s/", agentIp, getPort())); - try { - post.addRequestHeader("command", "echo"); - if (httpClient.executeMethod(post) != 200) { - logger.debug(String.format("echoing baremetal security group agent on %s got error: %s", agentIp, post.getResponseBodyAsString())); - } else { - ret = true; - } - break; - } catch (Exception e) { - if (count*m >= l) { - logger.debug(String.format("ping security group agent on vm[%s] timeout after %s minutes, starting vm failed, count=%s", agentIp, TimeUnit.MILLISECONDS.toSeconds(l), count)); - break; - } else { - logger.debug(String.format("Having pinged security group agent on vm[%s] %s times, continue to wait...", agentIp, count)); - } - } finally { - if (post != null) { - post.releaseConnection(); - } - } - } - return ret; - } - - public SecurityGroupRuleAnswer call(String agentIp, SecurityGroupRulesCmd cmd) { - PostMethod post = new PostMethod(String.format( - "http://%s:%s", agentIp, getPort())); - try { - SecurityGroupVmRuleSet rset = new SecurityGroupVmRuleSet(); - rset.getEgressRules().addAll(generateRules(cmd.getEgressRuleSet())); - rset.getIngressRules().addAll( - generateRules(cmd.getIngressRuleSet())); - rset.setVmName(cmd.getVmName()); - rset.setVmIp(cmd.getGuestIp()); - rset.setVmMac(cmd.getGuestMac()); - rset.setVmId(cmd.getVmId()); - rset.setSignature(cmd.getSignature()); - rset.setSequenceNumber(cmd.getSeqNum()); - Marshaller marshaller = context.createMarshaller(); - StringWriter writer = new StringWriter(); - marshaller.marshal(rset, writer); - String xmlContents = writer.toString(); - logger.debug(xmlContents); - - post.addRequestHeader("command", "set_rules"); - StringRequestEntity entity = new StringRequestEntity(xmlContents); - post.setRequestEntity(entity); - if (httpClient.executeMethod(post) != 200) { - return new SecurityGroupRuleAnswer(cmd, false, - post.getResponseBodyAsString()); - } else { - return new SecurityGroupRuleAnswer(cmd); - } - } catch (Exception e) { - return new SecurityGroupRuleAnswer(cmd, false, e.getMessage()); - } finally { - if (post != null) { - post.releaseConnection(); - } - } - } - - public int getPort() { - return port; - } - - public void setPort(int port) { - this.port = port; - } -} +// 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. +// +// Automatically generated by addcopyright.py at 01/29/2013 +// Apache License, Version 2.0 (the "License"); you may not use this +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// Automatically generated by addcopyright.py at 04/03/2012 + +package com.cloud.baremetal.networkservice; + +import com.cloud.agent.api.SecurityGroupRuleAnswer; +import com.cloud.agent.api.SecurityGroupRulesCmd; +import com.cloud.agent.api.SecurityGroupRulesCmd.IpPortAndProto; +import com.cloud.baremetal.networkservice.schema.SecurityGroupRule; +import com.cloud.baremetal.networkservice.schema.SecurityGroupVmRuleSet; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.commons.httpclient.HttpClient; +import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; +import org.apache.commons.httpclient.methods.PostMethod; +import org.apache.commons.httpclient.methods.StringRequestEntity; +import org.apache.log4j.Logger; + +import javax.xml.bind.JAXBContext; +import javax.xml.bind.Marshaller; +import java.io.StringWriter; +import java.net.SocketTimeoutException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.concurrent.TimeUnit; + +public class SecurityGroupHttpClient { + private static final Logger logger = Logger.getLogger(SecurityGroupHttpClient.class); + private static final String ARG_NAME = "args"; + private static final String COMMAND = "command"; + private JAXBContext context; + private int port; + private static HttpClient httpClient; + static { + MultiThreadedHttpConnectionManager connman = new MultiThreadedHttpConnectionManager(); + httpClient = new HttpClient(connman); + httpClient.setConnectionTimeout(5000); + } + + private enum OpConstant { + setRules, echo, + } + + public SecurityGroupHttpClient() { + try { + context = JAXBContext.newInstance(SecurityGroupRule.class, SecurityGroupVmRuleSet.class); + port = 9988; + } catch (Exception e) { + throw new CloudRuntimeException( + "Unable to create JAXBContext for security group", e); + } + } + + private List generateRules(final List ipps) { + List rules = new ArrayList( + ipps.size()); + for (SecurityGroupRulesCmd.IpPortAndProto ipp : ipps) { + SecurityGroupRule r = new SecurityGroupRule(); + r.setProtocol(ipp.getProto()); + r.setStartPort(ipp.getStartPort()); + r.setEndPort(ipp.getEndPort()); + for (String cidr : ipp.getAllowedCidrs()) { + r.getIp().add(cidr); + } + rules.add(r); + } + return rules; + } + + public HashMap> sync(String vmName, Long vmId, String agentIp) { + HashMap> states = new HashMap>(); + PostMethod post = new PostMethod(String.format("http://%s:%s/", agentIp, getPort())); + try { + post.addRequestHeader("command", "sync"); + if (httpClient.executeMethod(post) != 200) { + logger.debug(String.format("echoing baremetal security group agent on %s got error: %s", agentIp, post.getResponseBodyAsString())); + } else { + String res = post.getResponseBodyAsString(); + // res = ';'.join([vmName, vmId, seqno]) + String[] rulelogs = res.split(","); + if (rulelogs.length != 6) { + logger.debug(String.format("host[%s] returns invalid security group sync document[%s], reset rules", agentIp, res)); + states.put(vmName, new Pair(vmId, -1L)); + return states; + } + Pair p = new Pair(Long.valueOf(rulelogs[1]), Long.valueOf(rulelogs[5])); + states.put(rulelogs[0], p); + return states; + } + } catch (SocketTimeoutException se) { + logger.warn(String.format("unable to sync security group rules on host[%s], %s", agentIp, se.getMessage())); + } catch (Exception e) { + logger.warn(String.format("unable to sync security group rules on host[%s]", agentIp), e); + } finally { + if (post != null) { + post.releaseConnection(); + } + } + return states; + } + + + public boolean echo(String agentIp, long l, long m) { + boolean ret = false; + int count = 1; + while (true) { + try { + Thread.sleep(m); + count++; + } catch (InterruptedException e1) { + logger.warn("", e1); + break; + } + PostMethod post = new PostMethod(String.format("http://%s:%s/", agentIp, getPort())); + try { + post.addRequestHeader("command", "echo"); + if (httpClient.executeMethod(post) != 200) { + logger.debug(String.format("echoing baremetal security group agent on %s got error: %s", agentIp, post.getResponseBodyAsString())); + } else { + ret = true; + } + break; + } catch (Exception e) { + if (count*m >= l) { + logger.debug(String.format("ping security group agent on vm[%s] timeout after %s minutes, starting vm failed, count=%s", agentIp, TimeUnit.MILLISECONDS.toSeconds(l), count)); + break; + } else { + logger.debug(String.format("Having pinged security group agent on vm[%s] %s times, continue to wait...", agentIp, count)); + } + } finally { + if (post != null) { + post.releaseConnection(); + } + } + } + return ret; + } + + public SecurityGroupRuleAnswer call(String agentIp, SecurityGroupRulesCmd cmd) { + PostMethod post = new PostMethod(String.format( + "http://%s:%s", agentIp, getPort())); + try { + SecurityGroupVmRuleSet rset = new SecurityGroupVmRuleSet(); + rset.getEgressRules().addAll(generateRules(cmd.getEgressRuleSet())); + rset.getIngressRules().addAll( + generateRules(cmd.getIngressRuleSet())); + rset.setVmName(cmd.getVmName()); + rset.setVmIp(cmd.getGuestIp()); + rset.setVmMac(cmd.getGuestMac()); + rset.setVmId(cmd.getVmId()); + rset.setSignature(cmd.getSignature()); + rset.setSequenceNumber(cmd.getSeqNum()); + Marshaller marshaller = context.createMarshaller(); + StringWriter writer = new StringWriter(); + marshaller.marshal(rset, writer); + String xmlContents = writer.toString(); + logger.debug(xmlContents); + + post.addRequestHeader("command", "set_rules"); + StringRequestEntity entity = new StringRequestEntity(xmlContents); + post.setRequestEntity(entity); + if (httpClient.executeMethod(post) != 200) { + return new SecurityGroupRuleAnswer(cmd, false, + post.getResponseBodyAsString()); + } else { + return new SecurityGroupRuleAnswer(cmd); + } + } catch (Exception e) { + return new SecurityGroupRuleAnswer(cmd, false, e.getMessage()); + } finally { + if (post != null) { + post.releaseConnection(); + } + } + } + + public int getPort() { + return port; + } + + public void setPort(int port) { + this.port = port; + } +} diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSecurityGroupRulesCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSecurityGroupRulesCommandWrapper.java index f1a2e02dea75..ef9fd896dd1b 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSecurityGroupRulesCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSecurityGroupRulesCommandWrapper.java @@ -62,7 +62,7 @@ public Answer execute(final SecurityGroupRulesCmd command, final LibvirtComputin return new SecurityGroupRuleAnswer(command, false, "programming network rules failed"); } else { s_logger.debug("Programmed network rules for vm " + command.getVmName() + " guestIp=" + command.getGuestIp() + ",ingress numrules=" - + command.getIngressRuleSet().length + ",egress numrules=" + command.getEgressRuleSet().length); + + command.getIngressRuleSet().size() + ",egress numrules=" + command.getEgressRuleSet().size()); return new SecurityGroupRuleAnswer(command); } } diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 04a27f3d8c70..66efdf29ec94 100644 --- a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -39,6 +39,7 @@ import java.util.List; import java.util.Random; import java.util.UUID; +import java.util.Vector; import javax.naming.ConfigurationException; import javax.xml.parsers.DocumentBuilderFactory; @@ -2893,8 +2894,11 @@ public void testSecurityGroupRulesCmdFalse() { final Long seqNum = 1l; final IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; final IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; + final List secIps = new Vector(); + final List cidrs = new Vector(); + cidrs.add("0.0.0.0/0"); - final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet); + final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class); final Connect conn = Mockito.mock(Connect.class); @@ -2914,12 +2918,12 @@ public void testSecurityGroupRulesCmdFalse() { when(ingressRuleSet[0].getProto()).thenReturn("tcp"); when(ingressRuleSet[0].getStartPort()).thenReturn(22); when(ingressRuleSet[0].getEndPort()).thenReturn(22); - when(ingressRuleSet[0].getAllowedCidrs()).thenReturn(new String[]{"0.0.0.0/0"}); + when(ingressRuleSet[0].getAllowedCidrs()).thenReturn(cidrs); when(egressRuleSet[0].getProto()).thenReturn("tcp"); when(egressRuleSet[0].getStartPort()).thenReturn(22); when(egressRuleSet[0].getEndPort()).thenReturn(22); - when(egressRuleSet[0].getAllowedCidrs()).thenReturn(new String[]{"0.0.0.0/0"}); + when(egressRuleSet[0].getAllowedCidrs()).thenReturn(cidrs); final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); @@ -2945,8 +2949,11 @@ public void testSecurityGroupRulesCmdTrue() { final Long seqNum = 1l; final IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; final IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; + final List secIps = new Vector(); + final List cidrs = new Vector(); + cidrs.add("0.0.0.0/0"); - final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet); + final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class); final Connect conn = Mockito.mock(Connect.class); @@ -2972,12 +2979,12 @@ public void testSecurityGroupRulesCmdTrue() { when(ingressRuleSet[0].getProto()).thenReturn("tcp"); when(ingressRuleSet[0].getStartPort()).thenReturn(22); when(ingressRuleSet[0].getEndPort()).thenReturn(22); - when(ingressRuleSet[0].getAllowedCidrs()).thenReturn(new String[]{"0.0.0.0/0"}); + when(ingressRuleSet[0].getAllowedCidrs()).thenReturn(cidrs); when(egressRuleSet[0].getProto()).thenReturn("tcp"); when(egressRuleSet[0].getStartPort()).thenReturn(22); when(egressRuleSet[0].getEndPort()).thenReturn(22); - when(egressRuleSet[0].getAllowedCidrs()).thenReturn(new String[]{"0.0.0.0/0"}); + when(egressRuleSet[0].getAllowedCidrs()).thenReturn(cidrs); when(libvirtComputingResource.addNetworkRules(command.getVmName(), Long.toString(command.getVmId()), command.getGuestIp(), command.getSignature(), Long.toString(command.getSeqNum()), command.getGuestMac(), command.stringifyRules(), vif, brname, command.getSecIpsString())).thenReturn(true); @@ -3007,8 +3014,9 @@ public void testSecurityGroupRulesCmdException() { final Long seqNum = 1l; final IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; final IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; + final List secIps = new Vector(); - final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet); + final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class); final Connect conn = Mockito.mock(Connect.class); diff --git a/plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java b/plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java index 0541b7e61a2f..648bf7fc39e4 100644 --- a/plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java +++ b/plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java @@ -940,8 +940,8 @@ private Answer execute(SecurityGroupRulesCmd cmd) { s_logger.warn("Failed to program network rules for vm " + cmd.getVmName()); return new SecurityGroupRuleAnswer(cmd, false, "programming network rules failed"); } else { - s_logger.info("Programmed network rules for vm " + cmd.getVmName() + " guestIp=" + cmd.getGuestIp() + ":ingress num rules=" + cmd.getIngressRuleSet().length + - ":egress num rules=" + cmd.getEgressRuleSet().length); + s_logger.info("Programmed network rules for vm " + cmd.getVmName() + " guestIp=" + cmd.getGuestIp() + ":ingress num rules=" + cmd.getIngressRuleSet().size() + + ":egress num rules=" + cmd.getEgressRuleSet().size()); return new SecurityGroupRuleAnswer(cmd); } } diff --git a/plugins/hypervisors/simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java b/plugins/hypervisors/simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java index 6602e1f1ad07..5efcd3b57e68 100644 --- a/plugins/hypervisors/simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java +++ b/plugins/hypervisors/simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java @@ -626,7 +626,7 @@ private boolean logSecurityGroupAction(final SecurityGroupRulesCmd cmd, final Te reason = ", seqno_new"; } s_logger.info("Programmed network rules for vm " + cmd.getVmName() + " seqno=" + cmd.getSeqNum() + " signature=" + cmd.getSignature() + " guestIp=" + - cmd.getGuestIp() + ", numIngressRules=" + cmd.getIngressRuleSet().length + ", numEgressRules=" + cmd.getEgressRuleSet().length + " total cidrs=" + + cmd.getGuestIp() + ", numIngressRules=" + cmd.getIngressRuleSet().size() + ", numEgressRules=" + cmd.getEgressRuleSet().size() + " total cidrs=" + cmd.getTotalNumCidrs() + action + reason); return updateSeqnoAndSig; } diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSecurityGroupRulesCommandWrapper.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSecurityGroupRulesCommandWrapper.java index 0f8e6b5ba891..b189ba076fb7 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSecurityGroupRulesCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSecurityGroupRulesCommandWrapper.java @@ -56,7 +56,7 @@ public Answer execute(final SecurityGroupRulesCmd command, final CitrixResourceB return new SecurityGroupRuleAnswer(command, false, "programming network rules failed"); } else { s_logger.info("Programmed network rules for vm " + command.getVmName() + " guestIp=" + command.getGuestIp() + ", ingress numrules=" - + command.getIngressRuleSet().length + ", egress numrules=" + command.getEgressRuleSet().length); + + command.getIngressRuleSet().size() + ", egress numrules=" + command.getEgressRuleSet().size()); return new SecurityGroupRuleAnswer(command); } } diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixRequestWrapperTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixRequestWrapperTest.java index ca58f355336c..0e512fb94010 100644 --- a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixRequestWrapperTest.java +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixRequestWrapperTest.java @@ -33,6 +33,7 @@ import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.Vector; import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; @@ -101,6 +102,7 @@ import com.cloud.agent.api.UpdateHostPasswordCommand; import com.cloud.agent.api.UpgradeSnapshotCommand; import com.cloud.agent.api.VMSnapshotTO; +import com.cloud.agent.api.SecurityGroupRulesCmd.IpPortAndProto; import com.cloud.agent.api.check.CheckSshCommand; import com.cloud.agent.api.proxy.CheckConsoleProxyLoadCommand; import com.cloud.agent.api.proxy.WatchConsoleProxyLoadCommand; @@ -740,7 +742,17 @@ public void testSecurityGroupRulesCommand() { final Connection conn = Mockito.mock(Connection.class); final XsHost xsHost = Mockito.mock(XsHost.class); - final SecurityGroupRulesCmd sshCommand = new SecurityGroupRulesCmd(); + final String guestIp = "127.0.0.1"; + final String guestMac = "00:00:00:00"; + final String vmName = "Test"; + final Long vmId = 1l; + final String signature = "signature"; + final Long seqNum = 1l; + final IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; + final IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{Mockito.mock(IpPortAndProto.class)}; + final List secIps = new Vector(); + + final SecurityGroupRulesCmd sshCommand = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); assertNotNull(wrapper); diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java index 64204fe9a7af..64bbb75faaa9 100644 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl2.java @@ -182,8 +182,6 @@ public void sendRulesetUpdates(SecurityGroupWork work) { List nicSecIps = null; if (nic != null) { if (nic.getSecondaryIp()) { - //get secondary ips of the vm - long networkId = nic.getNetworkId(); nicSecIps = _nicSecIpDao.getSecondaryIpAddressesForNic(nic.getId()); } } @@ -193,7 +191,7 @@ public void sendRulesetUpdates(SecurityGroupWork work) { cmd.setMsId(_serverId); if (s_logger.isDebugEnabled()) { s_logger.debug("SecurityGroupManager v2: sending ruleset update for vm " + vm.getInstanceName() + ":ingress num rules=" + - cmd.getIngressRuleSet().length + ":egress num rules=" + cmd.getEgressRuleSet().length + " num cidrs=" + cmd.getTotalNumCidrs() + " sig=" + + cmd.getIngressRuleSet().size() + ":egress num rules=" + cmd.getEgressRuleSet().size() + " num cidrs=" + cmd.getTotalNumCidrs() + " sig=" + cmd.getSignature()); } Commands cmds = new Commands(cmd); From b9b5967d6bbe7fd351dbc80499672e2c2edb1105 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Sun, 17 Jan 2016 16:24:54 +0100 Subject: [PATCH 5/5] SecurityGroupRulesCmd code cleanup review comments handled --- .../agent/api/SecurityGroupRulesCmd.java | 48 ++++++++++--------- .../agent/api/SecurityGroupRulesCmdTest.java | 42 +++++++--------- 2 files changed, 42 insertions(+), 48 deletions(-) diff --git a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java index a7f44251ddcf..09f926666985 100644 --- a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java +++ b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java @@ -33,6 +33,7 @@ import com.cloud.utils.net.NetUtils; public class SecurityGroupRulesCmd extends Command { + private static final String CIDR_LENGTH_SEPARATOR = "/"; private static final char RULE_TARGET_SEPARATOR = ','; private static final char RULE_COMMAND_SEPARATOR = ':'; protected static final String EGRESS_RULE = "E:"; @@ -155,11 +156,10 @@ public String getVmName() { return vmName; } - //convert cidrs in the form "a.b.c.d/e" to "hexvalue of 32bit ip/e" - private String compressCidr(final String cidr) { - final String[] toks = cidr.split("/"); + private String compressCidrToHexRepresentation(final String cidr) { + final String[] toks = cidr.split(CIDR_LENGTH_SEPARATOR); final long ipnum = NetUtils.ip2Long(toks[0]); - return Long.toHexString(ipnum) + "/" + toks[1]; + return Long.toHexString(ipnum) + CIDR_LENGTH_SEPARATOR + toks[1]; } public String getSecIpsString() { @@ -189,42 +189,41 @@ public String stringifyCompressedRules() { return ruleBuilder.toString(); } - /** - * @param ipPandPs - * @param gression - * @param compress - * @param ruleBuilder - */ - private void stringifyRulesFor( - final List ipPandPs, - final String gression, - final boolean compress, - final StringBuilder ruleBuilder) { - for (final IpPortAndProto ipPandP : ipPandPs) { - ruleBuilder.append(gression).append(ipPandP.getProto()).append(RULE_COMMAND_SEPARATOR).append(ipPandP.getStartPort()).append(RULE_COMMAND_SEPARATOR).append(ipPandP.getEndPort()).append(RULE_COMMAND_SEPARATOR); + private void stringifyRulesFor(final List ipPortAndProtocols, final String inOrEgress, final boolean compressed, final StringBuilder ruleBuilder) { + for (final IpPortAndProto ipPandP : ipPortAndProtocols) { + ruleBuilder.append(inOrEgress).append(ipPandP.getProto()).append(RULE_COMMAND_SEPARATOR).append(ipPandP.getStartPort()).append(RULE_COMMAND_SEPARATOR) + .append(ipPandP.getEndPort()).append(RULE_COMMAND_SEPARATOR); for (final String cidr : ipPandP.getAllowedCidrs()) { - ruleBuilder.append(compress?compressCidr(cidr):cidr).append(RULE_TARGET_SEPARATOR); + ruleBuilder.append(represent(cidr, compressed)).append(RULE_TARGET_SEPARATOR); } ruleBuilder.append("NEXT "); } } - /* + private String represent(final String cidr, final boolean compressed) { + if (compressed) { + return compressCidrToHexRepresentation(cidr); + } else { + return cidr; + } + } + + /** * Compress the security group rules using zlib compression to allow the call to the hypervisor * to scale beyond 8k cidrs. + * Note : not using {@see GZipOutputStream} since that is for files, using {@see DeflaterOutputStream} instead. + * {@see GZipOutputStream} gives a different header, although the compression is the same */ public String compressStringifiedRules() { final String stringified = stringifyRules(); final ByteArrayOutputStream out = new ByteArrayOutputStream(); String encodedResult = null; try { - //Note : not using GZipOutputStream since that is for files - //GZipOutputStream gives a different header, although the compression is the same final DeflaterOutputStream dzip = new DeflaterOutputStream(out); dzip.write(stringified.getBytes()); dzip.close(); encodedResult = Base64.encodeBase64String(out.toByteArray()); - } catch (IOException e) { + } catch (final IOException e) { LOGGER.warn("Exception while compressing security group rules"); } return encodedResult; @@ -246,8 +245,11 @@ public Long getVmId() { return vmId; } + /** + * used for logging + * @return the number of Cidrs in the in and egress rule sets for this security group rules command. + */ public int getTotalNumCidrs() { - //useful for logging int count = 0; for (final IpPortAndProto i : ingressRuleSet) { count += i.allowedCidrs.size(); diff --git a/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java b/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java index 2b094c5a404f..37b15ebb7281 100644 --- a/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java +++ b/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java @@ -25,7 +25,6 @@ import java.util.Vector; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; @@ -40,31 +39,24 @@ public class SecurityGroupRulesCmdTest { private SecurityGroupRulesCmd securityGroupRulesCmd; - /** - * @throws java.lang.Exception - */ - @BeforeClass - public static void setUpBeforeClass() throws Exception { - } - /** * @throws java.lang.Exception */ @Before public void setUp() throws Exception { - String guestIp = "10.10.10.10"; - String guestMac = "aa:aa:aa:aa:aa:aa"; - String vmName = "vm"; - Long vmId = 1L; - String signature = "sig"; - Long seqNum = 0L; - String proto = "abc"; - int startPort = 1; - int endPort = 2; - String[] allowedCidrs = new String[] {"1.2.3.4/5","6.7.8.9/0"}; - IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{new IpPortAndProto(proto, startPort, endPort, allowedCidrs)}; - IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{new IpPortAndProto(proto, startPort, endPort, allowedCidrs)}; - List secIps = new Vector(); + final String guestIp = "10.10.10.10"; + final String guestMac = "aa:aa:aa:aa:aa:aa"; + final String vmName = "vm"; + final Long vmId = 1L; + final String signature = "sig"; + final Long seqNum = 0L; + final String proto = "abc"; + final int startPort = 1; + final int endPort = 2; + final String[] allowedCidrs = new String[] {"1.2.3.4/5","6.7.8.9/0"}; + final IpPortAndProto[] ingressRuleSet = new IpPortAndProto[]{new IpPortAndProto(proto, startPort, endPort, allowedCidrs)}; + final IpPortAndProto[] egressRuleSet = new IpPortAndProto[]{new IpPortAndProto(proto, startPort, endPort, allowedCidrs)}; + final List secIps = new Vector(); securityGroupRulesCmd = new SecurityGroupRulesCmd(guestIp, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); } @@ -73,7 +65,7 @@ public void setUp() throws Exception { */ @Test public void testStringifyRules() throws Exception { - String a = securityGroupRulesCmd.stringifyRules(); + final String a = securityGroupRulesCmd.stringifyRules(); // do verification on a assertTrue(a.contains(SecurityGroupRulesCmd.EGRESS_RULE)); } @@ -83,7 +75,7 @@ public void testStringifyRules() throws Exception { */ @Test public void testStringifyCompressedRules() throws Exception { - String a = securityGroupRulesCmd.stringifyCompressedRules(); + final String a = securityGroupRulesCmd.stringifyCompressedRules(); // do verification on a assertTrue(a.contains(SecurityGroupRulesCmd.EGRESS_RULE)); } @@ -93,8 +85,8 @@ public void testStringifyCompressedRules() throws Exception { */ @Test public void testCompressStringifiedRules() throws Exception { - String compressed = "eJzztEpMSrYytDKyMtQz0jPWM9E31THTM9ez0LPUN9Dxc40IUXAlrAQAPdoP3Q=="; - String a = securityGroupRulesCmd.compressStringifiedRules(); + final String compressed = "eJzztEpMSrYytDKyMtQz0jPWM9E31THTM9ez0LPUN9Dxc40IUXAlrAQAPdoP3Q=="; + final String a = securityGroupRulesCmd.compressStringifiedRules(); assertTrue(compressed.equals(a)); }