You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ko...@apache.org on 2016/04/11 18:55:03 UTC

[5/6] git commit: updated refs/heads/master to d1def0a

SecurityGroupRulesCmd code cleanup review comments handled


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/b9b5967d
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/b9b5967d
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/b9b5967d

Branch: refs/heads/master
Commit: b9b5967d6bbe7fd351dbc80499672e2c2edb1105
Parents: d39182f
Author: Daan Hoogland <da...@onecht.net>
Authored: Sun Jan 17 16:24:54 2016 +0100
Committer: Daan Hoogland <da...@onecht.net>
Committed: Sun Jan 17 16:24:54 2016 +0100

----------------------------------------------------------------------
 .../cloud/agent/api/SecurityGroupRulesCmd.java  | 48 ++++++++++----------
 .../agent/api/SecurityGroupRulesCmdTest.java    | 42 +++++++----------
 2 files changed, 42 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b9b5967d/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/com/cloud/agent/api/SecurityGroupRulesCmd.java
index a7f4425..09f9266 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<IpPortAndProto> 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<IpPortAndProto> 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();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/b9b5967d/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java
----------------------------------------------------------------------
diff --git a/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java b/core/test/com/cloud/agent/api/SecurityGroupRulesCmdTest.java
index 2b094c5..37b15eb 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;
@@ -43,28 +42,21 @@ public class SecurityGroupRulesCmdTest {
     /**
      * @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<String> secIps = new Vector<String>();
+        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<String> secIps = new Vector<String>();
         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));
     }