> On Oct. 6, 2012, 4:42 p.m., Rohit Yadav wrote:
> > Abhinav suggests on the issue the deleting VR still fails.
> > This is expected as the commit e1dc0024ea5d869dd9945b920ba85dd2a24a51c1
> > only fixes issues on the API layer.
> > The deletion fails on ACL. Please see diff again on this review and let's
> > work out a solution.
Rohit, I looked at the exception, it's a completely different error now. I
reviewed your patch, the solution still will break the current ACL concepts. We
can't do "includingRemoved" in the domain checker. Domain checker is used by
the API layer, and with the fix proposed, the following case would fail (I
mentioned it earlier):
* have removed account. The removed account has some detached volume and user
vm that weren't cleaned up yet
* As ROOT admin, attach account's volume to account's vm. The patch makes it
possible while we should allow just LISTING the resources belonging to the
removed account, but never allow to manipulate/create/delete them.
The fix in this case should be this: when deleteRouter/deleteNetwork call is
being made by periodic thread that cleans up VRs/Networks belonging to the
project/account, no ACL check should be made at all because the caller in this
case is System user. This part of the code shouldn't have been even hit:
if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL) {
Account account = _accountDao.findById(entity.getAccountId());
.......
You have to find out why caller of the destroyAccount call in
VirtualNetworkApplianceImpl (executed as a part of cleanup project) is not
System user, but Acct[3-user1] and fix this particular problem:
com.cloud.exception.PermissionDeniedException: Acct[3-user1] does not have
permission to operate with resource VM[DomainRouter|r-6-VM]
After the caller passed to destroyCommand is system user, no check will be
made, and the router will be deleted.
- Alena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7168/#review12205
-----------------------------------------------------------
On Sept. 19, 2012, 3:38 p.m., Rohit Yadav wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7168/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2012, 3:38 p.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek, Kishan Kavala, Nitin
> Mehta, Alena Prokharchyk, and Alex Huang.
>
>
> Description
> -------
>
> Domain ACL information should be valid even if account entry is marked
> removed. Patch fixes how account is obtained based on accountId, it
> finds among those entries which are marked deleted.
>
> In case of project deletion, the project is marked removed first and
> then each of its elements are cleared/cleaned/deleted. While deleting
> network and router it failed because ACL only checks accounts which are
> not marked deleted.
>
> Download original patch and git am <patch>:
> http://patchbin.baagi.org/p?id=40pdym
>
>
> This addresses bug CLOUDSTACK-84.
>
>
> Diffs
> -----
>
> server/src/com/cloud/acl/DomainChecker.java 6bc2cd3
> server/src/com/cloud/user/dao/AccountDao.java 3b7fa66
> server/src/com/cloud/user/dao/AccountDaoImpl.java 7300bb1
>
> Diff: https://reviews.apache.org/r/7168/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rohit Yadav
>
>
