You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by resmo <gi...@git.apache.org> on 2015/06/22 16:58:49 UTC
[GitHub] cloudstack pull request: [WIP] CLOUDSTACK-6276: project support in...
GitHub user resmo opened a pull request:
https://github.com/apache/cloudstack/pull/508
[WIP] CLOUDSTACK-6276: project support in affinitygroups
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/resmo/cloudstack feature/6276-project-affinitygroup
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/508.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 #508
----
commit 2e129b3ef7b55fc79584305d667b59ae839c2ff2
Author: Rene Moser <re...@apache.org>
Date: 2015-06-22T14:46:33Z
CLOUDSTACK-6276: add project support to AffinityGroupJoinVO and API
commit c6b91fef14739ab4373e1637d74847180d0d1e0f
Author: Rene Moser <re...@apache.org>
Date: 2015-06-22T14:49:16Z
CLOUDSTACK-6276: implement searchForAffinityGroups in QueryService
commit 45a88be1d4f0c2b59e81c5e904679dab388ad88f
Author: Rene Moser <re...@apache.org>
Date: 2015-06-22T14:53:24Z
CLOUDSTACK-6276: use query service for listAffinityGroups
commit 7b322969ab47d943931eda876d54418980adf817
Author: Rene Moser <re...@apache.org>
Date: 2015-06-22T14:55:41Z
CLOUDSTACK-6276: add project support in AffinityGroupService in create and delete
----
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121914103
Hi @wilderrodrigues that would be awesome!
It is not supposed to break backward compatibility. In my opinion the functionality is implemented and should work, but I hadn't a change to really test it and the current tests are not green as well. So adding unit tests and looking at the current unit tests is needed.
As it is my first serious work in cloudstack, don't hesitate to correct any mistake I made. :)
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121829695
ping, any update on this? still WIP?
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-157853386
@pdube you can create a pull request on github when you finish the coding and testing.
This PR have lots of issues, so it will not be merged of course.
I have similar fix for cloudstack 4.2.1.
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-157754858
Hey @wilderrodrigues @resmo,
I was looking to fixing this as well. Is there any way I can contribute? What is the status of this?
Thanks,
pdube
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-157854104
@ustcweizhou I have checked it out into another branch. I am working on it. And will create a PR when I am done.
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158006677
@pdube If you are almost done, I will wait for your PR. Otherwise, I can work on porting it.
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158450166
@pdube
you are right.
system/root admin can access every entity, but root admin cannot create a vm using other account' affinity groups.
Affinity Group AffinityGroup[6f164261-99b7-4333-a9b4-0103888e9d94] does not belong to the VM's account
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158200709
@ustcweizhou Is that actually necessary?
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-127280785
@wilderrodrigues ok. thx!
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158434572
@ustcweizhou Thanks for the response. I understand the code. I was wondering why there was a need for a special access checker for AffinityGroups. After more investigation, it seems like the DomainChecker explicitly fails for affinity groups, which seems a bit strange.
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158164720
@pdube
try this change:
```
diff --git a/server/src/com/cloud/acl/AffinityGroupAccessChecker.java b/server/src/com/cloud/acl/AffinityGroupAccessChecker.java
index 57f7b37..c6e44d6 100644
--- a/server/src/com/cloud/acl/AffinityGroupAccessChecker.java
+++ b/server/src/com/cloud/acl/AffinityGroupAccessChecker.java
@@ -30,6 +30,9 @@ import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao;
import com.cloud.domain.DomainVO;
import com.cloud.exception.PermissionDeniedException;
+import com.cloud.projects.ProjectVO;
+import com.cloud.projects.dao.ProjectAccountDao;
+import com.cloud.projects.dao.ProjectDao;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.utils.exception.CloudRuntimeException;
@@ -44,6 +47,10 @@ public class AffinityGroupAccessChecker extends DomainChecker {
AccountManager _accountMgr;
@Inject
AffinityGroupDomainMapDao _affinityGroupDomainMapDao;
+ @Inject
+ ProjectDao _projectDao;
+ @Inject
+ ProjectAccountDao _projectAccountDao;
@Override
public boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType) throws PermissionDeniedException {
@@ -72,6 +79,15 @@ public class AffinityGroupAccessChecker extends DomainChecker {
} else {
//acl_type account
if (caller.getId() != group.getAccountId()) {
+ //check if the group belongs to a project
+ ProjectVO project = _projectDao.findByProjectAccountId(group.getAccountId());
+ if (project != null) {
+ if (AccessType.ModifyProject.equals(accessType) && _projectAccountDao.canModifyProjectAccount(caller.getId(), group.getAccountId())) {
+ return true;
+ } else if (!AccessType.ModifyProject.equals(accessType) && _projectAccountDao.canAccessProjectAccount(caller.getId(), group.getAccountId())) {
+ return true;
+ }
+ }
throw new PermissionDeniedException(caller + " does not have permission to operate with resource " + entity);
} else {
return true;
```
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121849880
Hi @resmo and @bhaisaab
I will check with @remibergsma / @DaanHoogland / @snuf if that's something we could get involved in. I'm okay with helping, as long as I have some time dedicated in our sprints.
Would you have some more details on how far you are, @resmo ? Will it break existing functionality if we merge? If not and if I get the time, we can get it merge and I will continue from where you left.
Cheers,
Wilder
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121846292
hey @bhaisaab, still WIP. I ask for some help to finish it on the ML, because I will not have time to work on it the next months.
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-127274573
@wilderrodrigues did you take it over? can I close this PR?
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-157874321
@ustcweizhou Also if you have a fix for it, why not port it forward?
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158075714
@ustcweizhou I am working on it now, but have a few questions about resource ownership
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158342455
@pdube
you may notice this code in UserVmManagerImpl.java
_accountMgr.checkAccess(caller, null, true, owner, ag);
normally it will check
(1) caller can access owner
(2) caller can access/modify ag (affinity group)
(3) The account of owner and ag are the same.
I guess you have modified the account of ag (when create an affinity group from project view) from account to project account of project. If yes, the (2) will fail, because affinity group has special AccessChecker that the caller can only be the account of the affinity group. However, when we log in as Account A and go to Project view, the call is A, not the project account of the project.
Do you need my help to fix the issues or test the changes ?
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121931572
@wilderrodrigues yes, please. Feel free.
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121921520
Hi @resmo
Can I then get your changes, from your PR, work on it and push via another PR? Would that be okay with you?
If so, I think we could close this PR. What do you think? @bhaisaab @DaanHoogland your thoughts on that...?
Cheers,
Wilder
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-158200813
Why is that access control done differently?
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-121931545
@wilderrodrigues as long as you don't squash them and keep the attribution intact, i'm fine with that. otherwise you can also make pull requests to resmo's fork and let him add it to his PR
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/508#issuecomment-127275589
I will start on it this week, hopefully! Just keep the PR open, I will get it and create a PR on top of your with the tests. In this way we keep track of everything
Cheers,
Wilder
---
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: [WIP] CLOUDSTACK-6276: project support in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/508
---
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.
---