[EC2 Query API] AuthorizeSecurityGroupIngress and RevokeSecurityGroupIngress 
return false even when the request is successful.

https://reviews.apache.org/r/8513

If the response is not null and if all the ruleid's(permissions) are present 
return true.


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

Branch: refs/heads/master
Commit: de517d95b0c3ef6feb57026bffb8b93240c04273
Parents: b514735
Author: Likitha Shetty <[email protected]>
Authored: Thu Jan 31 11:37:01 2013 -0800
Committer: Prachi Damle <[email protected]>
Committed: Thu Jan 31 12:00:30 2013 -0800

----------------------------------------------------------------------
 .../cloud/bridge/service/core/ec2/EC2Engine.java   |   15 +++++++++------
 awsapi/src/com/cloud/stack/CloudStackApi.java      |    9 +++++----
 2 files changed, 14 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/de517d95/awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java
----------------------------------------------------------------------
diff --git a/awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java 
b/awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java
index cd187a4..6f54d38 100644
--- a/awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java
+++ b/awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java
@@ -310,7 +310,7 @@ public class EC2Engine {
                                throw new 
EC2ServiceException(ClientError.InvalidGroup_NotFound, "Cannot find matching 
ruleid.");
 
                        CloudStackInfoResponse resp = 
getApi().revokeSecurityGroupIngress(ruleId);
-                       if (resp != null && resp.getId() != null) {
+            if (resp != null) {
                                return resp.getSuccess();
                        }
                        return false;
@@ -341,7 +341,7 @@ public class EC2Engine {
                                        pair.setKeyValue(group.getAccount(), 
group.getName());
                                        secGroupList.add(pair);
                                }
-                               CloudStackSecurityGroupIngress resp = null;
+                CloudStackSecurityGroup resp = null;
                                if 
(ipPerm.getProtocol().equalsIgnoreCase("icmp")) {
                                        resp = 
getApi().authorizeSecurityGroupIngress(null, 
constructList(ipPerm.getIpRangeSet()), null, null, 
                                                        ipPerm.getIcmpCode(), 
ipPerm.getIcmpType(), ipPerm.getProtocol(), null, 
@@ -351,10 +351,13 @@ public class EC2Engine {
                                                        
ipPerm.getToPort().longValue(), null, null, ipPerm.getProtocol(), null, 
request.getName(), 
                                                        
ipPerm.getFromPort().longValue(), secGroupList);
                                }
-                               if (resp != null && resp.getRuleId() != null) {
-                                       return true;
-                               }
-                               return false;
+                if (resp != null ){
+                    List<CloudStackIngressRule> ingressRules = 
resp.getIngressRules();
+                    for (CloudStackIngressRule ingressRule : ingressRules)
+                        if (ingressRule.getRuleId() == null) return false;
+                } else {
+                    return false;
+                }
                        }
                } catch(Exception e) {
                        logger.error( "EC2 AuthorizeSecurityGroupIngress - ", 
e);

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/de517d95/awsapi/src/com/cloud/stack/CloudStackApi.java
----------------------------------------------------------------------
diff --git a/awsapi/src/com/cloud/stack/CloudStackApi.java 
b/awsapi/src/com/cloud/stack/CloudStackApi.java
index 52c2459..5a30ef2 100644
--- a/awsapi/src/com/cloud/stack/CloudStackApi.java
+++ b/awsapi/src/com/cloud/stack/CloudStackApi.java
@@ -1115,9 +1115,9 @@ public class CloudStackApi {
         * @return
         * @throws Exception
         */
-       public CloudStackSecurityGroupIngress 
authorizeSecurityGroupIngress(String account, String cidrList, String domainId, 
Long endPort, 
-                       String icmpCode, String icmpType, String protocol, 
String securityGroupId, String securityGroupName, Long startPort, 
-                       List<CloudStackKeyValue> userSecurityGroupList) throws 
Exception {
+    public CloudStackSecurityGroup authorizeSecurityGroupIngress(String 
account, String cidrList, String domainId, Long endPort,
+            String icmpCode, String icmpType, String protocol, String 
securityGroupId, String securityGroupName, Long startPort,
+            List<CloudStackKeyValue> userSecurityGroupList) throws Exception {
                CloudStackCommand cmd = new 
CloudStackCommand(ApiConstants.AUTHORIZE_SECURITY_GROUP_INGRESS);
                if (cmd != null) {
                        if (account != null) cmd.setParam(ApiConstants.ACCOUNT, 
account);
@@ -1139,7 +1139,8 @@ public class CloudStackApi {
                                }                               
                        }
                }
-               return _client.call(cmd, apiKey, secretKey, true, 
ApiConstants.AUTHORIZE_SECURITY_GROUP_INGRESS_RESPONSE, 
ApiConstants.SECURITY_GROUP, CloudStackSecurityGroupIngress.class);
+        return _client.call(cmd, apiKey, secretKey, true, 
ApiConstants.AUTHORIZE_SECURITY_GROUP_INGRESS_RESPONSE,
+                ApiConstants.SECURITY_GROUP, CloudStackSecurityGroup.class);
        }
 
        /**

Reply via email to