You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rags22489664 <gi...@git.apache.org> on 2015/02/24 11:16:31 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
GitHub user rags22489664 opened a pull request:
https://github.com/apache/cloudstack/pull/87
CLOUDSTACK-8280: UI does not display source CIDR in VPC ACL
Since https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=6b0c34f, the source cidr for acl rules are not dislpayed in the UI. Since the entity was split into two tables network_acl_item and network_acl_item_cidrs, populating cidrs before returning the response,
A view could have been created but only one column from the network_acl_item_cidrs table is needed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rags22489664/cloudstack master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/87.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #87
----
commit 9f07a55f391f4928a6d16f930f142ad63884074d
Author: ramamurtis <ra...@citrix.com>
Date: 2015-02-24T10:12:12Z
CLOUDSTACK-8280: UI does not display source CIDR in VPC ACL
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/87#issuecomment-77809892
As per rest of the design, all the managers generally implement the response generators. The fix that was pushed follows same convention/design.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/87#issuecomment-77741942
I don't mind. I'm just worried about the rammifications for the API design.
I stole my implementation from somewhere else. It must have once been
decided that this was the way to go...
On Sun, Mar 8, 2015 at 9:57 AM, Rohit Yadav <no...@github.com>
wrote:
> Hi @DaanHoogland <https://github.com/DaanHoogland> your fix looks good as
> well, though the fix @rags22489664 <https://github.com/rags22489664>
> suggests or that one I did would solve for the create response method in
> the manager implementation such that it solves for its usage across all the
> list, update, create APIs. But now it's already fixed by that version,
> let's close this?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/87#issuecomment-77739908>.
>
--
Daan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/87#issuecomment-77739908
Hi @DaanHoogland your fix looks good as well, though the fix @rags22489664 suggests or that one I did would solve for the create response method in the manager implementation such that it solves for its usage across all the list, update, create APIs. But now it's already fixed by that version, let's close this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/87#issuecomment-77703465
I solved this privately and forgot to give back to the community. Please consider this solution @bhaisaab . It seems to be custom for these @transient-annotated fields:
From d39e0251f923620897bc408a58e4bfcfb0c6cdfa Mon Sep 17 00:00:00 2001
From: Daan Hoogland <da...@onecht.net>
Date: Mon, 15 Dec 2014 15:58:42 +0100
Subject: [PATCH 01/14] CLOUDSTACK-8073 load cidrs on create response
---
server/src/com/cloud/api/ApiDBUtils.java | 9 +++++++++
server/src/com/cloud/api/ApiResponseHelper.java | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/server/src/com/cloud/api/ApiDBUtils.java b/server/src/com/cloud/api/ApiDBUtils.java
index 90a09a0..4895d91 100755
--- a/server/src/com/cloud/api/ApiDBUtils.java
+++ b/server/src/com/cloud/api/ApiDBUtils.java
@@ -201,6 +201,7 @@ import com.cloud.network.security.SecurityGroupManager;
import com.cloud.network.security.SecurityGroupVO;
import com.cloud.network.security.dao.SecurityGroupDao;
import com.cloud.network.vpc.NetworkACL;
+import com.cloud.network.vpc.NetworkACLItemCidrsDao;
import com.cloud.network.vpc.StaticRouteVO;
import com.cloud.network.vpc.VpcGatewayVO;
import com.cloud.network.vpc.VpcManager;
@@ -357,6 +358,7 @@ public class ApiDBUtils {
static ConfigurationDao s_configDao;
static ConsoleProxyDao s_consoleProxyDao;
static FirewallRulesCidrsDao s_firewallCidrsDao;
+ static NetworkACLItemCidrsDao s_networkACLItemCidrsDao;
static VMInstanceDao s_vmDao;
static ResourceLimitService s_resourceLimitMgr;
static ProjectService s_projectMgr;
@@ -524,6 +526,8 @@ public class ApiDBUtils {
@Inject
private FirewallRulesCidrsDao firewallCidrsDao;
@Inject
+ private NetworkACLItemCidrsDao networkACLItemCidrsDao;
+ @Inject
private VMInstanceDao vmDao;
@Inject
private ResourceLimitService resourceLimitMgr;
@@ -692,6 +696,7 @@ public class ApiDBUtils {
s_configDao = configDao;
s_consoleProxyDao = consoleProxyDao;
s_firewallCidrsDao = firewallCidrsDao;
+ s_networkACLItemCidrsDao = networkACLItemCidrsDao;
s_vmDao = vmDao;
s_resourceLimitMgr = resourceLimitMgr;
s_projectMgr = projectMgr;
@@ -1241,6 +1246,10 @@ public class ApiDBUtils {
return s_firewallCidrsDao.getSourceCidrs(id);
}
+ public static List<String> findNetworkAclItemSourceCidrs(long id) {
+ return s_networkACLItemCidrsDao.getCidrs(id);
+ }
+
public static Account getProjectOwner(long projectId) {
return s_projectMgr.getProjectOwner(projectId);
}
diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java
index 37cb155..df4cca8 100755
--- a/server/src/com/cloud/api/ApiResponseHelper.java
+++ b/server/src/com/cloud/api/ApiResponseHelper.java
@@ -2349,7 +2349,8 @@ public class ApiResponseHelper implements ResponseGenerator {
response.setEndPort(Integer.toString(aclItem.getSourcePortEnd()));
}
- response.setCidrList(StringUtils.join(aclItem.getSourceCidrList(), ","));
+ List<String> cidrs = ApiDBUtils.findNetworkAclItemSourceCidrs(aclItem.getId());
+ response.setCidrList(StringUtils.join(cidrs, ","));
response.setTrafficType(aclItem.getTrafficType().toString());
--
2.3.0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/87#issuecomment-77596535
Scks that I missed this PR couple of days ago. I though @DaanHoogland was reviewing it. Okay, I made a similar fix thanks for the effort @rags22489664 but you may close it now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by rags22489664 <gi...@git.apache.org>.
Github user rags22489664 closed the pull request at:
https://github.com/apache/cloudstack/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8280: UI does not display sour...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/87#issuecomment-75833513
I recognize the problem and did some code to implement it in the response generation. might be this didn't make it to any release yet. Have a look at ListNetworkACLsCmd.execute and the call to _responseGenerator.createNetworkACLItemResponse(acl) in there. It caught me by surprise.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---