diff --git a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java index a7f44251ddc..09f92666698 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.agent.api.LogLevel.Log4jLevel; 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 class SecurityGroupRulesCmd extends Command { 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 class SecurityGroupRulesCmd extends Command { 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 class SecurityGroupRulesCmd extends Command { 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 2b094c5a404..37b15ebb728 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.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; @@ -40,31 +39,24 @@ import com.cloud.agent.api.SecurityGroupRulesCmd.IpPortAndProto; 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 class SecurityGroupRulesCmdTest { */ @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 class SecurityGroupRulesCmdTest { */ @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 class SecurityGroupRulesCmdTest { */ @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)); }