You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/03/25 09:11:25 UTC

[GitHub] [cloudstack] davidjumani opened a new issue #4868: isrecursive not honoured for listAccounts, maybe others as well

davidjumani opened a new issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868


   
   ##### ISSUE TYPE
   
    * Bug Report
   
   ##### CLOUDSTACK VERSION
   
   4.14 + (maybe even lower versions)
   
   ##### SUMMARY
   
   isrecursive is not honoured when passed along with listall=true for listAccounts as a root admin
   It is also not honoured when allow.user.view.all.domain.accounts is true
   
   https://github.com/apache/cloudstack/blob/master/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java#L2198-L2205
   
   ##### EXPECTED RESULTS
   ```
   (qa) 🐱 > list accounts filter=name,domain domainid=e4874e10-5fdf-11ea-9a56-1e006800018c listall=true isrecursive=false 
   {
     "account": [
       {
         "domain": "ROOT",
         "name": "admin"
       },
       {
         "domain": "ROOT",
         "name": "baremetal-system-account"
       },
       ................
   
   ```
   
   ##### ACTUAL RESULTS
   ```
   (qa) 🐱 > list accounts filter=name,domain domainid=e4874e10-5fdf-11ea-9a56-1e006800018c listall=true isrecursive=false 
   {
     "account": [
       {
         "domain": "ROOT",
         "name": "admin"
       },
       {
         "domain": "ROOT",
         "name": "baremetal-system-account"
       },
       {
         "domain": "small",            <- from a sub domain
         "name": "rick"
       },
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-813934350


   I think that this is a deeper issue that could be across several elements. Need to look into whether this should be fixed. I personally think so and can raise a PR for the same but would like to get a wider feedback


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez closed issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
nvazquez closed issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-896740755


   +1 for me, @DaanHoogland @rhtyd any input?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-890697087


   @nvazquez Not just the definitions but the implementations as well


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rohityadavcloud commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-1077411114


   It appears David's PR was reverted. @nvazquez pl review with 4.17 team if this is worth fixing for 4.17, or move to next milestone(s).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] Pearl1594 commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-897526345


   If I could add a point here - as per my understanding, `isrecursive` is a parameter used in conjunction with domainid, where as when invoking a list api with `listall` it should list everything, irrespective of the value of `isrecursive`(which is what it does currently). In my opinion, the current behavior isn't actually wrong/buggy. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-909046869


   Moving this to next milestone.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] ravening commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
ravening commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-812517436


   Not with recursion but how the code works


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-806492553


   so basically teh defined functionalities of `listall` and `isrecursive` overlap and `listall` prefails. iniutitively I'd say `isrecursive` should prefail, so FWIW I agree. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-909046869


   Moving this to next milestone.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-909046869


   Moving this to next milestone.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-812515600


   > @davidjumani @DaanHoogland this issue exists for several api's if you call the api from the child domain
   > 
   > I have seen the same issue for listing service offering and disk offering as well. I have created a pr for it
   > 
   > Create the following domain relationship
   > 
   > ROOT -> test1 -> test11
   > 
   > Now login as test1 and create a service offering for test11 domain.
   > If you login as test11, you wont see it
   
   @ravening that sounds more like a problem with access than recursion, wouldn't you say so?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-891586681


   @nvazquez My idea is :
   - listall : Returns all resources the user can access. As of now when set to true, it fetches resources recursively even if isrecursive is set to false. Change it to respect isrecursive
   - isrecursive : Leave it as is


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] davidjumani commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-900050440


   @Pearl1594 If that is the case, isn't passing a `domainid` along with `isrecursive` as true the same as passing `domainid` with `listall` as true ? In that case is `isrecursive` redundant ?
   Also a user might want to list all accounts only in his / her domain without the need to explicitly pass the domainid. In that case it will return all users in subdomains as well.
   I believe that this could be misleading. Either it should be fixed or a note added that `listall` ignores or is the same as passing `isrecursive`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-812522745


   yes, well, domains have a concept of recursion but accounts do not. In the zone/pod/cluster/host there is another form of what could be called recursion. What you name above seems like a bug to me, but in general it is hard to define recursion consistently in cloudstack behaviour when it comes to these constructs.
   
   > Not with recursion but how the code works
   
   Do you agree with how @davidjumani proposes to implement this?
   And do you have other examples where it should behave the same?
   
   I think what you mention about creating a service offering merits an issue ticket of its own.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-896877954


   I agree that what @davidjumani describes is the intuitive way, but it might cause a regression like behaviour of user scripts so we need to document very well if we change this. (and possibly provide automated migration, though it seems not more than adding the `isrecursive=true` in scripts at first sight)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] ravening commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
ravening commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-811812119


   @davidjumani @DaanHoogland this issue exists for several api's if you call the api from the child domain
   
   I have seen the same issue for listing service offering and disk offering as well. I have created a pr for it
   
   Create the following domain relationship
   
   ROOT -> test1 -> test11
   
   Now login as test1 and create a service offering for test11 domain.
   If you login as test11, you wont see it


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-891096438


   @davidjumani sure, my question was if you could explain what's your idea on the new implementation. Maybe easier to explain the new definitions of both parameter


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
nvazquez commented on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-890690750


   @davidjumani reading the description of the parameters:
   - listall: If set to false, list only resources belonging to the command's caller; if set to true - list resources that the caller is authorized to see. Default value is false
   - isrecursive: defaults to false, but if true, lists all resources from the parent specified by the domainId till leaves.
   
   Seems like these definitions might need a change, have you thought about a new behaviour?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez edited a comment on issue #4868: isrecursive not honoured for listAccounts, maybe others as well

Posted by GitBox <gi...@apache.org>.
nvazquez edited a comment on issue #4868:
URL: https://github.com/apache/cloudstack/issues/4868#issuecomment-896740755


   +1 for me, @DaanHoogland @rhtyd @sureshanaparti any input?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org