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 2020/02/18 13:12:44 UTC

[GitHub] [cloudstack] Pearl1594 opened a new pull request #3894: Fixed count value in the list apis

Pearl1594 opened a new pull request #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894
 
 
   ## Description
   The count value returned in the APIs responses is incorrect
   
   Addresses: https://github.com/apache/cloudstack/issues/3890
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [X] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-589963511
 
 
   @DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590058558
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590838404
 
 
   Quick test, for example the listing of ips:
   
   Before: (domain admin sees ips from other projects)
   ```
   (qaprimate) šŸ± > list publicipaddresses  listall=true projectid=-1
   {
     "count": 3,
     "publicipaddress": [
       {
         "allocated": "2020-02-17T16:33:27+0100",
         "associatednetworkid": "0ac48c1b-e2bc-4462-8dfd-cbdb3bd71390",
         "associatednetworkname": "TestUserProjectNetwork",
         "domain": "ROOT",
         "domainid": "fa323836-4f04-11ea-b408-1e006800018c",
         "forvirtualnetwork": true,
         "id": "26e2d0f7-476d-4598-a2c8-4b59d29aaf8e",
         "ipaddress": "192.168.2.195",
         "isportable": false,
         "issourcenat": true,
         "isstaticnat": false,
         "issystem": false,
         "networkid": "ecd874e8-9c14-4a66-8528-c4a11960c7be",
         "physicalnetworkid": "08344756-9abc-41c0-b575-8ca1a0a7c77d",
         "project": "TestUserProject",
         "projectid": "5660c407-167c-480d-9825-5b54d63797b7",
         "state": "Allocated",
         "tags": [],
         "zoneid": "91b49be0-5a8b-4f3e-83b8-3bdfee4eba78",
         "zonename": "Sandbox-simulator"
       },
       {
         "allocated": "2020-02-19T13:32:44+0100",
         "associatednetworkid": "22272dc5-65fb-439e-af3d-c2884f3101c8",
         "associatednetworkname": "SamlUserProjectNet",
         "domain": "ROOT",
         "domainid": "fa323836-4f04-11ea-b408-1e006800018c",
         "forvirtualnetwork": true,
         "id": "3574b63a-5dd9-4d1d-8c79-91de2952203d",
         "ipaddress": "192.168.2.168",
         "isportable": false,
         "issourcenat": true,
         "isstaticnat": false,
         "issystem": false,
         "networkid": "ecd874e8-9c14-4a66-8528-c4a11960c7be",
         "physicalnetworkid": "08344756-9abc-41c0-b575-8ca1a0a7c77d",
         "project": "SamlUserProject",
         "projectid": "879a2fe0-639f-4709-a405-5d145d7625f7",
         "state": "Allocated",
         "tags": [],
         "zoneid": "91b49be0-5a8b-4f3e-83b8-3bdfee4eba78",
         "zonename": "Sandbox-simulator"
       },
       {
         "allocated": "2020-02-25T09:39:16+0100",
         "associatednetworkid": "493824ac-12d5-43c6-b6d6-2246270cf0a0",
         "associatednetworkname": "testdomain-project-net",
         "domain": "Domain",
         "domainid": "d56ba667-945e-46be-8829-e21df75af531",
         "forvirtualnetwork": true,
         "id": "aea77e18-99b8-4a66-8cb5-15e6848134f6",
         "ipaddress": "192.168.2.121",
         "isportable": false,
         "issourcenat": true,
         "isstaticnat": false,
         "issystem": false,
         "networkid": "ecd874e8-9c14-4a66-8528-c4a11960c7be",
         "physicalnetworkid": "08344756-9abc-41c0-b575-8ca1a0a7c77d",
         "project": "DomainAdminProject",
         "projectid": "03b2f678-f237-4804-a29d-88503a63817e",
         "state": "Allocated",
         "tags": [],
         "zoneid": "91b49be0-5a8b-4f3e-83b8-3bdfee4eba78",
         "zonename": "Sandbox-simulator"
       }
     ]
   }
   ```
   
   After the fix: (domain admin only sees ips from their own projects)
   ```
   (qaprimate) šŸ± > list publicipaddresses  projectid=-1 listall=true 
   {
     "count": 1,
     "publicipaddress": [
       {
         "allocated": "2020-02-25T09:39:16+0100",
         "associatednetworkid": "493824ac-12d5-43c6-b6d6-2246270cf0a0",
         "associatednetworkname": "testdomain-project-net",
         "domain": "Domain",
         "domainid": "d56ba667-945e-46be-8829-e21df75af531",
         "forvirtualnetwork": true,
         "id": "aea77e18-99b8-4a66-8cb5-15e6848134f6",
         "ipaddress": "192.168.2.121",
         "isportable": false,
         "issourcenat": true,
         "isstaticnat": false,
         "issystem": false,
         "networkid": "ecd874e8-9c14-4a66-8528-c4a11960c7be",
         "physicalnetworkid": "08344756-9abc-41c0-b575-8ca1a0a7c77d",
         "project": "DomainAdminProject",
         "projectid": "03b2f678-f237-4804-a29d-88503a63817e",
         "state": "Allocated",
         "tags": [],
         "zoneid": "91b49be0-5a8b-4f3e-83b8-3bdfee4eba78",
         "zonename": "Sandbox-simulator"
       }
     ]
   }
   ```
   
   ... and the root admin is able to see everything:
   ```
   (qaprimate) šŸ± > list publicipaddresses  projectid=-1 listall=true filter=id,ipaddress,domain
   {
     "count": 18,
     "publicipaddress": [
       {
         "domain": "ROOT",
         "id": "b3e30c2a-02e9-44b6-b9b2-cd46531d00d7",
         "ipaddress": "192.168.2.99"
       },
       {
         "domain": "ROOT",
         "id": "c70bb8c8-7d41-4686-9740-9626dc216bc9",
         "ipaddress": "192.168.2.98"
       },
       {
         "domain": "ROOT",
         "id": "a5f9da6a-f4cc-44b1-8f5a-4456863d17fb",
         "ipaddress": "192.168.2.97"
       },
       {
         "domain": "ROOT",
         "id": "df296484-5f4c-4b73-a9b9-c14c2900217f",
         "ipaddress": "192.168.2.95"
       },
       {
         "domain": "ROOT",
         "id": "7f53ee95-1ce8-4b02-b4ad-2f7d3c4428d5",
         "ipaddress": "192.168.2.94"
       },
       {
         "domain": "ROOT",
         "id": "28b51886-963c-41d2-9507-3863da658136",
         "ipaddress": "192.168.2.92"
       },
       {
         "domain": "ROOT",
         "id": "2ee8f2bb-c0b1-4a2d-868a-8c28fe9ada66",
         "ipaddress": "192.168.2.4"
       },
       {
         "domain": "ROOT",
         "id": "a1dcfa06-6df4-4710-ad45-83b93e2e6327",
         "ipaddress": "192.168.2.3"
       },
       {
         "domain": "ROOT",
         "id": "26e2d0f7-476d-4598-a2c8-4b59d29aaf8e",
         "ipaddress": "192.168.2.195"
       },
       {
         "domain": "ROOT",
         "id": "3ada9c24-b138-4dd4-ac8a-2260b1865944",
         "ipaddress": "192.168.2.192"
       },
       {
         "domain": "ROOT",
         "id": "b17cc857-e6bd-4cbc-a09e-9f3e4f5c5433",
         "ipaddress": "192.168.2.183"
       },
       {
         "domain": "ROOT",
         "id": "242d048c-fec4-4d02-be32-14e9a29375f5",
         "ipaddress": "192.168.2.181"
       },
       {
         "domain": "ROOT",
         "id": "3574b63a-5dd9-4d1d-8c79-91de2952203d",
         "ipaddress": "192.168.2.168"
       },
       {
         "domain": "ROOT",
         "id": "bdf26431-c879-4750-847f-7303b905f718",
         "ipaddress": "192.168.2.162"
       },
       {
         "domain": "ROOT",
         "id": "8f8fd0cf-5707-4677-ad38-18e3ad8912d1",
         "ipaddress": "192.168.2.155"
       },
       {
         "domain": "ROOT",
         "id": "6330bf8c-d668-413b-9fca-b0ae0111d58d",
         "ipaddress": "192.168.2.127"
       },
       {
         "domain": "Domain",
         "id": "aea77e18-99b8-4a66-8cb5-15e6848134f6",
         "ipaddress": "192.168.2.121"
       },
       {
         "domain": "ROOT",
         "id": "90f7a57f-3dfd-4244-ba08-973bc847d2c0",
         "ipaddress": "192.168.2.103"
       }
     ]
   }
   ```

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [ ] listInstanceGroups
   - [x] listVirtualMachines
   - [ ] listVolumes
   - [ ] listSecurityGroups
   - [ ] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] andrijapanicsb commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591394897
 
 
   If Rohit tested the security things, I'm fine with 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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [ ] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [ ] listSecurityGroups
   - [x] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-589963459
 
 
   @blueorangutan package

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [ ] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [ ] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [ ] listTags
   - [ ] listEvents
   - [ ] listInstanceGroups
   - [ ] listVirtualMachines
   - [ ] listVolumes
   - [ ] listSecurityGroups
   - [ ] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587530036
 
 
   @blueorangutan test

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588607984
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-911

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588608421
 
 
   @blueorangutan test 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591363038
 
 
   @andrijapanicsb, care to test?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [x] listPrivateGateways
   - [x] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587485251
 
 
   I'll test against Primate
   @blueorangutan package

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [x] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590774786
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-947

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587721425
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-897

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587493846
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-895

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588103138
 
 
   @rhtyd @Pearl1594 Are you testing on the old ui 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591305002
 
 
   @DaanHoogland @rhtyd I am ok with merging it.
   remember to fix the remaining issues

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587578959
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [x] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588107890
 
 
   @DaanHoogland see my test plan above, I've tested both. See here:
   Old UI: http://primate-qa.cloudstack.cloud:8080/client
   Primate: http://primate-qa.cloudstack.cloud:8080/client/master

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588107890
 
 
   @DaanHoogland here:
   Old UI: http://primate-qa.cloudstack.cloud:8080/client
   Primate: http://primate-qa.cloudstack.cloud:8080/client/master

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587598951
 
 
   @blueorangutan test

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591189659
 
 
   @DaanHoogland @andrijapanic can one of you review/test the final changes? 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588106816
 
 
   @weizhouapache I agree the code solves two different issues from the outside. The problem the primate dev's are solving is getting a single valid return from a single API call and both problems need addressing for them.
   Do you have a concern that makes you ask to split the code changes?
   My personal opinion is that it is just extra (test) work, if we split it, that's why i have to ask.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587586797
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-896

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [ ] findHostsForMigration
   - [x] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [ ] listUsers
   - [ ] listTags
   - [ ] listEvents
   - [ ] listInstanceGroups
   - [ ] listVirtualMachines
   - [ ] listVolumes
   - [ ] listSecurityGroups
   - [ ] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588180827
 
 
   > @rhtyd @DaanHoogland do we need to add a global setting like 'display.project.resources.in.default.view' ? project resources are not displayed in older versions. I am ok no matter the answer is yes or no.
   
   @weizhouapache I don't think so. this would be more of a user choice/setting/preference than a global setting. Would you 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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587530444
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590056739
 
 
   Support for listNetworks and other list APIs can be done in future https://github.com/apache/cloudstack/issues/3897
   
   @andrijapanicsb can you review/test again, the raised issue has been fixed; now only the root admin can see all resources. Wrt the old UI, we simply want to ensure that the views get all the resources that were previously returned (i.e. no change in before and after).
   
   Master+this PR is at http://primate-qa.cloudstack.cloud:8080/client/
   
   @blueorangutan package
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588535037
 
 
   <b>Trillian test result (tid-1033)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 57139 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1033-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 54 look OK, 23 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 173.01 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 168.75 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 229.23 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   test_01_sys_vm_start | `Failure` | 0.07 | test_secondary_storage.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.14 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.03 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.03 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.03 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_09_destroy_ssvm | `Failure` | 0.03 | test_ssvm.py
   test_10_destroy_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.38 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.37 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.38 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.35 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.20 | test_templates.py
   test_03_deploy_vm_wrong_checksum | `Error` | 1.24 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 16.60 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.86 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   test_01_cancel_host_maintenace_with_no_migration_jobs | `Failure` | 0.07 | test_host_maintenance.py
   test_02_cancel_host_maintenace_with_migration_jobs | `Error` | 3.35 | test_host_maintenance.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1512.08 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.53 | test_hostha_kvm.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588996130
 
 
   <b>Trillian test result (tid-1064)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 25964 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1064-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Smoke tests completed. 76 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 172.85 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 168.60 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 219.73 | test_privategw_acl.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590056802
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [ ] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [ ] listSecurityGroups
   - [x] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] andrijapanicsb commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-589285734
 
 
   Alright, so:
   
   @Pearl1594 could you please update the description with the list of list* APIs which actually ARE supported by listall=true + projectid=-1 to show everything - since i.e. listNetworks API is not supported and many others as it seems (or in other words, I could see in code just 7-8 or so...so let's list them as we only support handfull of all list* APIs)
   
   
   @rhtyd 
   Tested fine in general
   - the count is OK with page size = 5 (old UI is unusable, but that's another story - can't scroll and load additional pages anywhere, instances, volumes, settings. etc...)
   - listall=true + projectid=-1 does behave as expected and returns list of "really all" resources of a specific kind, for which this combination is supported (tested listVirtualMachines and listRouters)
   - listInfrastructure does return alerts and internal LBs.
   
   But we have a security issue @rhtyd @Pearl1594 
   - in the main ROOT domain, create resoruces outside project, create a projects, create resources inside the project
   - create subdomain and domain admin for that subdomain (no resources created)
   - a doman admin of a **subdomain** can list all resources of the ROOT domain when listall=true AND projectid=-1 (listing both non-project and project resources created by the main "admin" user in ROOT doman) 
   - only using listall=true does NOT return anything (does not return non-project resources from the ROOT domain)
   - using projectid=-1 will list all resources of the projects from the ROOT domain, although the user (subdomain domain-admin) is NOT participating in that project
   (localcloud) SBCM5> > list virtualmachines listall=true filter=name,domain
   (localcloud) SBCM5> > list virtualmachines listall=true projectid=-1 filter=name,domain
   {
     "count": **15,**
     "virtualmachine": [
       {
         "domain": "ROOT",
         "name": "VM-9b745dda-395c-4694-a364-f9813cd5bb8e"
       },
       {
         "domain": "ROOT",
         "name": "VM-522d3b9a-c98c-4afe-b600-835628739a34"
       },
       {
         "domain": "ROOT",
         "name": "VM-dea62692-9afd-43d9-beb7-afb4e2f13674"
       },
       {
         "domain": "ROOT",
         "name": "VM-43e25bf1-2902-4d51-87c1-eb9ac32b686b"
       },
       {
         "domain": "ROOT",
         "name": "VM-1252f8c1-393b-457c-ac9c-3a5f64cc8bfa"
       }
     ]
   }
   (localcloud) SBCM5> > list volumes projectid=-1 filter=name,project,domain
   {
     "count": **4**
     "volume": [
       {
         "domain": "ROOT",
         "name": "ROOT-17",
         "project": "project1"
       },
       {
         "domain": "ROOT",
         "name": "ROOT-15",
         "project": "project1"
       },
       {
         "domain": "ROOT",
         "name": "ROOT-19",
         "project": "project1"
       },
       {
         "domain": "ROOT",
         "name": "ROOT-18",
         "project": "project1"
       }
     ]
   }
   
   (localcloud) SBCM5> > exit
   [root@pr3894-t1068-kvm-centos7-mgmt1 ~]# grep -E "(domain|usernam)" .cmk/config
   username = andrija1
   domain   = /dom1

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590284220
 
 
   @weizhouapache let me back to you on your comments soon, did you test/compare against 4.13/master? Wrt the listall=true&project-id change, this restricted to the root admin here: https://github.com/apache/cloudstack/pull/3894/files#diff-44c2e394637af1d7c556dac5575ba8d3R2683 Given this fix, there shouldn't be any change any non-admin accounts 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-589965368
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-936

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [x] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] Pearl1594 commented on a change in pull request #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r380727019
 
 

 ##########
 File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
 ##########
 @@ -1248,7 +1248,7 @@ private HypervisorType getHypervisorType(VMInstanceVO vm, StoragePool srcVolumeP
         final Map<Host, Boolean> requiresStorageMotion = new HashMap<Host, Boolean>();
         DataCenterDeployment plan = null;
         if (canMigrateWithStorage) {
-            allHostsPair = searchForServers(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), null, null, null, keyword, null, null, srcHost.getHypervisorType(),
+            allHostsPair = searchForServersExcluding(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), null, null, srcHostId, keyword, null, null, srcHost.getHypervisorType(),
                     srcHost.getHypervisorVersion());
             allHosts = allHostsPair.first();
             allHosts.remove(srcHost);
 
 Review comment:
   @weizhouapache True, however, it was noticed that, say if there were 15 hosts, with a default pagesize of 10, the 1st page (considering it contained the src host) will have 9 objects returned and the 2nd page will have the remaining 5, as opposed to the ideal behavior of the 1st page containing 10 items and the remaining 4 being listed on page 2.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588129356
 
 
   Copy @rhtyd , i missed the mention of old UI

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590281598
 
 
   @rhtyd @DaanHoogland @Pearl1594 
   again, I would suggest you to split it to two PRs.
   The change to list all resource in projects is small, but it requires much more testing.
   unless you want to merge this into master now, and fix the issues afterwards.
   
   If possible, please add a component/smoke test.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on a change in pull request #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r380702071
 
 

 ##########
 File path: plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
 ##########
 @@ -140,7 +145,9 @@ public InfrastructureResponse listInfrastructure() {
         response.setStoragePools(storagePoolDao.listAll().size());
         response.setImageStores(imageStoreDao.listImageStores().size());
         response.setSystemvms(vmInstanceDao.listByTypes(VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm).size());
-        response.setRouters(domainRouterDao.listAll().size());
+        response.setRouters(domainRouterDao.listByRole(VirtualRouter.Role.VIRTUAL_ROUTER).size());
 
 Review comment:
   @Pearl1594 calling listAll() i.e. getting all the rows will make the APIs slow in a large env; this is fine but can you explore a way to simply get the count (instead of the rows).

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591476954
 
 
   merging, assuming #3897 is there to remind us of the remaining issues @weizhouapache is asking for.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [ ] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [ ] listClustersMetrics
   - [ ] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588604306
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588096118
 
 
   @rhtyd can you revert the commits related to list projects resource ?
   It seems to be different issue

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on a change in pull request #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r380780775
 
 

 ##########
 File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
 ##########
 @@ -1515,6 +1515,89 @@ private boolean hasSuitablePoolsForVolume(final VolumeVO volume, final Host host
         return suitablePools;
     }
 
+
 
 Review comment:
   @Pearl1594 curb adding newlines

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [ ] listVirtualMachinesMetrics
   - [ ] listVolumesMetrics
   - [ ] listHostsMetrics
   - [ ] listStoragePoolsMetrics
   - [ ] listZonesMetrics
   - [ ] listClustersMetrics
   - [ ] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-589498288
 
 
   @andrijapanicsb in old UI there shouldn't be any visible change before and after. For thr security concern I've advised Pearl the fix so listing of all resources is only allowed for the root admin or accounts of admin account type. 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [x] listPrivateGateways
   - [x] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [x] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588203883
 
 
   @weizhouapache I'll check and get back tomorrow. There is no need of a new global setting as the current UI (see changes in this PR) will keep showing the previous behaviour, the end user (domain admin or root admin won't see any difference).
   
   I've also verified the api works for:
   List VMs in Project and list Volumes in Project (in current UI, projectid=<some uuid> is passed and the correct list of items are returned as previously).
   
   It does not work for networks when both projectid=-1 and listall=true is passed through.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [x] listPrivateGateways
   - [x] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [x] listSnapshots
   - [x] listVMSnapshot
   - [x] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [ ] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588325146
 
 
   > @rhtyd are you testing those extras ?
   > @weizhouapache you say "and so on", how extensive do you want that list to be?
   
   @DaanHoogland everything in project view :-D

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590525641
 
 
   <b>Trillian test result (tid-1107)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 27885 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1107-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 76 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 182.66 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 171.22 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 260.92 | test_privategw_acl.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland merged pull request #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590765962
 
 
   @weizhouapache I tried to look at git history, there was an attempt to fix similar issue for listVpcs https://github.com/apache/cloudstack/pull/2352 but as a domain admin I'm still able to listVpcs of projects not owned/managed by the domain admin.
   
   I'll be updating http://primate-qa.cloudstack.cloud:8080/client/ shortly with this PR and carry tests on the APIs listed in previous comment (will tick those which passes excepted result).

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [ ] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [ ] listSecurityGroups
   - [ ] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] andrijapanicsb commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
andrijapanicsb commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-589287265
 
 
   @rhtyd can you also advise WHAT exactly needs to be tested in the old UI - I couldn't make conclusion myself after reading the PR/issue/L?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [ ] listSecurityGroups
   - [x] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [ ] listVolumesMetrics
   - [ ] listHostsMetrics
   - [ ] listStoragePoolsMetrics
   - [ ] listZonesMetrics
   - [ ] listClustersMetrics
   - [ ] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587599266
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588608457
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [ ] listInstanceGroups
   - [ ] listVirtualMachines
   - [ ] listVolumes
   - [ ] listSecurityGroups
   - [ ] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588107890
 
 
   @DaanHoogland see my test plan above (https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497), I've tested both. See here:
   Old UI: http://primate-qa.cloudstack.cloud:8080/client
   Primate: http://primate-qa.cloudstack.cloud:8080/client/master

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590807416
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590807230
 
 
   @blueorangutan test

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan removed a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590058558
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590279113
 
 
   tested with the following setting
   (1) login as root admin, create a network/vm
   (2) login as domain admin of domain test1, create a network/vm.
   (3) login as domain admin of domain test1, create a project, create a network/vm in project
   (4) login as user1 in test1, create a network/vm.
   (5) login as user1 in test1, create a project, create a network/vm in project
   
   verified following apis
   (1) list virtualmachines
   (2) list volumes
   (3) list nics
   (5) list publicipaddresses
   (6) list networks
   (7) list routers
   
   results
   (1) root admin is able to list resources in projects of sub-domains, with projectid=-1 (but listall=false, @rhtyd is this expected result ? )
   if listall =true and projectid=-1, it returns resources in projects and non-projects. seems good.
   
   (2) list networks as root admin,
   projectid=-1/listall=false, return nothing (need to be fixed)
   projectid=-1/listall=true, return isolated networks in projects, and shared network for projects
   listall=true, return networks not in/for projects.
   
   (3) list routers as domain admin, 
   projectid=-1, returns routers of isolated networks in projects. should it be allowed ? 
   
   (4) list resource as domain admin
   projectid=-1, returns vms/volumes in projects
   listall=true, returns vms/volumes in non-projects
   listall =true and projectid=-1, it returns vms/volumes in projects (vms in non-projects are not returned. @rhtyd it this expected result ? )
   
   (5) list networks as domain admin
   listall=true/false, returns all isolated/shared networks this domain admin can access.
   projectid=-1, returns all isolated/shared networks a project in the domain can access.
   projectid=-1 and listall=true, same as above (projectid=-1).
   
   (6) list resources as normal user (same as domain admin)
   projectid=-1, returns vms/volumes in projects
   listall=true, returns vms/volumes in non-projects
   listall =true and projectid=-1, it returns vms/volumes in projects (vms in non-projects are not returned. @rhtyd it this expected result ? )
   
   (7) list networks as normal user (same as domain admin)
   listall=true/false, returns all isolated/shared networks this user can access.
   projectid=-1, returns all isolated/shared networks a project this user manages can access.
   projectid=-1 and listall=true, same as above (projectid=-1).

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588604297
 
 
   @blueorangutan package 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [ ] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [ ] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r382921466
 
 

 ##########
 File path: engine/components-api/src/main/java/com/cloud/agent/AgentManager.java
 ##########
 @@ -152,4 +152,6 @@
     void notifyMonitorsOfHostAboutToBeRemoved(long hostId);
 
     void notifyMonitorsOfRemovedHost(long hostId, long clusterId);
+
 
 Review comment:
   Can you check the diff, do we need changes around agent propagation? 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan: cc @DaanHoogland @andrijapanicsb @borisstoyanov 
   
   Verified on http://primate-qa.cloudstack.cloud:8080/client/master (this runs latest master + this PR)
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [x] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [x] findHostsForMigration
   - [x] listServiceOfferings
   - [x] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [x] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd removed a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd removed a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590058541
 
 
   @blueorangutan test

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587697577
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [x] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [x] findHostsForMigration
   - [x] listServiceOfferings
   - [x] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on a change in pull request #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r380702333
 
 

 ##########
 File path: plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
 ##########
 @@ -140,7 +145,9 @@ public InfrastructureResponse listInfrastructure() {
         response.setStoragePools(storagePoolDao.listAll().size());
         response.setImageStores(imageStoreDao.listImageStores().size());
         response.setSystemvms(vmInstanceDao.listByTypes(VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm).size());
-        response.setRouters(domainRouterDao.listAll().size());
+        response.setRouters(domainRouterDao.listByRole(VirtualRouter.Role.VIRTUAL_ROUTER).size());
+        response.setInternalLbs(domainRouterDao.listByRole(VirtualRouter.Role.INTERNAL_LB_VM).size());
+        response.setAlerts(alertDao.listAll().size());
 
 Review comment:
   Same as above ^^ if it may be done quickly.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590058368
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-938

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590766534
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590160290
 
 
   <b>Trillian test result (tid-1090)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 58445 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1090-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 55 look OK, 22 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 173.18 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 173.45 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 240.25 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   test_01_sys_vm_start | `Failure` | 0.07 | test_secondary_storage.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.14 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.02 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.02 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_09_destroy_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_10_destroy_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.36 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.34 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.37 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.33 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.21 | test_templates.py
   test_03_deploy_vm_wrong_checksum | `Error` | 1.25 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 17.85 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_14_secure_to_secure_vm_migration | `Error` | 11.30 | test_vm_life_cycle.py
   test_15_secured_to_nonsecured_vm_migration | `Error` | 74.08 | test_vm_life_cycle.py
   test_16_nonsecured_to_secured_vm_migration | `Error` | 1.20 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.68 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1511.57 | test_hostha_kvm.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587696333
 
 
   @blueorangutan package
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588273416
 
 
   @rhtyd are you testing those extras ?
   @weizhouapache you say "and so on", how extensive do you want that list to be?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan removed a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588535037
 
 
   <b>Trillian test result (tid-1033)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 57139 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1033-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 54 look OK, 23 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 173.01 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 168.75 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 229.23 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   test_01_sys_vm_start | `Failure` | 0.07 | test_secondary_storage.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.14 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.03 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.03 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.03 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_09_destroy_ssvm | `Failure` | 0.03 | test_ssvm.py
   test_10_destroy_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.38 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.37 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.38 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.35 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.20 | test_templates.py
   test_03_deploy_vm_wrong_checksum | `Error` | 1.24 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 16.60 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.86 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   test_01_cancel_host_maintenace_with_no_migration_jobs | `Failure` | 0.07 | test_host_maintenance.py
   test_02_cancel_host_maintenace_with_migration_jobs | `Error` | 3.35 | test_host_maintenance.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1512.08 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.53 | test_hostha_kvm.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588259942
 
 
   @blueorangutan package

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [x] listPrivateGateways
   - [x] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [x] listSSHKeyPairs
   - [x] listSnapshots
   - [x] listVMSnapshot
   - [x] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [ ] listStoragePoolsMetrics
   - [ ] listZonesMetrics
   - [ ] listClustersMetrics
   - [ ] listClusters
   - [ ] listAffinityGroups
   - [ ] findHostsForMigration
   - [ ] listServiceOfferings
   - [ ] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [x] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [x] findHostsForMigration
   - [x] listServiceOfferings
   - [x] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [x] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] Pearl1594 commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r382920805
 
 

 ##########
 File path: engine/components-api/src/main/java/com/cloud/agent/AgentManager.java
 ##########
 @@ -152,4 +152,6 @@
     void notifyMonitorsOfHostAboutToBeRemoved(long hostId);
 
     void notifyMonitorsOfRemovedHost(long hostId, long clusterId);
+
 
 Review comment:
   @rhtyd I haven't merged master

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591090085
 
 
   @weizhouapache are you satisfied that we can merge this?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590766071
 
 
   @blueorangutan package

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587485715
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591482431
 
 
   Yes #3897 is to remind us to to system-wide survey/checks for all list APIs.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [x] findHostsForMigration
   - [x] listServiceOfferings
   - [x] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r382920487
 
 

 ##########
 File path: engine/components-api/src/main/java/com/cloud/agent/AgentManager.java
 ##########
 @@ -152,4 +152,6 @@
     void notifyMonitorsOfHostAboutToBeRemoved(long hostId);
 
     void notifyMonitorsOfRemovedHost(long hostId, long clusterId);
+
 
 Review comment:
   @Pearl1594 did you merge master? There are additional changes. 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588259984
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [x] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587578599
 
 
   @blueorangutan package

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590176262
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590176152
 
 
   @blueorangutan test 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [x] listPrivateGateways
   - [x] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [x] listSnapshots
   - [x] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-587486497
 
 
   Test plan:
   
   - [x] listVirtualMachinesMetrics
   - [x] listVolumesMetrics
   - [ ] listHostsMetrics
   - [x] listStoragePoolsMetrics
   - [x] listZonesMetrics
   - [x] listClustersMetrics
   - [x] listClusters
   - [x] listAffinityGroups
   - [ ] findHostsForMigration
   - [x] listServiceOfferings
   - [x] listDiskOfferings
   
   Alerts, ilbvm keys:
   - [ ] listInfrastructure 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [x] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [ ] listTags
   - [x] listEvents
   - [ ] listInstanceGroups
   - [ ] listVirtualMachines
   - [ ] listVolumes
   - [ ] listSecurityGroups
   - [ ] listRouters
   - [ ] listProjectInvitations
   - [ ] listAsyncJobs
   - [ ] listTemplates
   - [ ] listIsos
   - [ ] listAffinityGroups
   - [ ] listFirewallRules
   - [ ] listEgressFirewallRules
   - [ ] listLoadBalancerRules
   - [ ] listPortForwardingRules
   - [ ] listNetworkACLLists
   - [ ] listNetworkACLs
   - [ ] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [ ] listVpnUsers
   - [ ] listRemoteAccessVpns
   - [ ] listVpnCustomerGateways
   - [ ] listVpnGateways
   - [ ] listVpnConnections
   - [ ] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591006196
 
 
   <b>Trillian test result (tid-1117)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 27058 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1117-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Smoke tests completed. 76 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 175.66 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 176.90 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 238.35 | test_privategw_acl.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590056739
 
 
   Support for listNetworks and other list APIs can be done in future https://github.com/apache/cloudstack/issues/3897
   
   @andrijapanicsb can you review/test again, the raised issue has been fixed; now only the root admin can see all resources.
   
   @blueorangutan package
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281
 
 
   Thanks @weizhouapache I went through your test results, I've gathered a list of APIs that are only supported/affected by this PR for rest of the `list` APIs including `listNetworks` I've logged https://github.com/apache/cloudstack/issues/3897 to be done in future. Here are my comments:
   
   - The magic feature of `projectid=-1` (I think documented in one of the old docs) is to allow non-normal-accounts to be able to see all project resources (excluding non-project resources). Domain admins shouldn't be allowed to see all resources including project resources when listall=true and projectid=-1.
   
   - A `listall=false`, when combined with `projectid=-1`, will still return all the project-specific resources (for backward compatibility).
   
   - The `listNetworks` is not a supported API yet, see #3897 which is why you're not seeing the new behaviour, the PR initially only intended to fix it for `listRouters` for Primate to render pagination/list properly. If this is necessary for the milestone, I can raise another PR.
   
   - *BIG ONE* - For the `listRouters` as domain admin called with projectid=-1, I tested both this PR and 4.13.0.0 and indeed I'm able to list routers of other networks. I found this to be true for many other APIs (from the list below). This **should not be allowed** for domain admins, based on quick investigation I think this happens because when projectid=-1 is passed it allows listing of all project resources without enforcing the permittedAccounts check? cc @DaanHoogland @PaulAngus @andrijapanicsb  - if we agree I can enforce a check that only the root admin can see all other project resources.
   
   - I do see and verified the old behaviour to list shared networks for other domains/projects/accounts, no change in behaviour seen.
   
   Based on static code analysis and usages, only the following APIs are affected by the listall=true&projectid=-1 change and only for the root admin account type:
   
   - [x] listUsers
   - [x] listTags
   - [x] listEvents
   - [x] listInstanceGroups
   - [x] listVirtualMachines
   - [x] listVolumes
   - [x] listSecurityGroups
   - [x] listRouters
   - [x] listProjectInvitations
   - [x] listAsyncJobs
   - [x] listTemplates
   - [x] listIsos
   - [x] listAffinityGroups
   - [x] listFirewallRules
   - [x] listEgressFirewallRules
   - [x] listLoadBalancerRules
   - [x] listPortForwardingRules
   - [x] listNetworkACLLists
   - [x] listNetworkACLs
   - [x] listVpcs
   - [ ] listPrivateGateways
   - [ ] listStaticRoutes
   - [x] listVpnUsers
   - [x] listRemoteAccessVpns
   - [x] listVpnCustomerGateways
   - [x] listVpnGateways
   - [x] listVpnConnections
   - [x] listPublicIpAddresses
   - [ ] listSSHKeyPairs
   - [ ] listSnapshots
   - [ ] listVMSnapshot
   - [ ] listLoadBalancers

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan removed a comment on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590160290
 
 
   <b>Trillian test result (tid-1090)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 58445 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3894-t1090-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 55 look OK, 22 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_vpc_privategw_static_routes | `Failure` | 173.18 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 173.45 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 240.25 | test_privategw_acl.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   test_01_sys_vm_start | `Failure` | 0.07 | test_secondary_storage.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.14 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.02 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.02 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.03 | test_ssvm.py
   test_09_destroy_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_10_destroy_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.36 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.34 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.37 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.33 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.21 | test_templates.py
   test_03_deploy_vm_wrong_checksum | `Error` | 1.25 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 17.85 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_14_secure_to_secure_vm_migration | `Error` | 11.30 | test_vm_life_cycle.py
   test_15_secured_to_nonsecured_vm_migration | `Error` | 74.08 | test_vm_life_cycle.py
   test_16_nonsecured_to_secured_vm_migration | `Error` | 1.20 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.68 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1511.57 | test_hostha_kvm.py
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588620916
 
 
   @weizhouapache @DaanHoogland I've tested everything in the project view and in the default view as the root admin on the current/legacy UI (master+this PR): http://primate-qa.cloudstack.cloud:8080/client/
   
   ^^ + @andrijapanicsb @borisstoyanov @vladimirpetrov - do you guys want to test as well (I'm sure my tests shouldn't be considered as I'm the co-author on the PR)?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590771236
 
 
   @rhtyd thanks for your update and quick fix !
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r380717812
 
 

 ##########
 File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
 ##########
 @@ -1248,7 +1248,7 @@ private HypervisorType getHypervisorType(VMInstanceVO vm, StoragePool srcVolumeP
         final Map<Host, Boolean> requiresStorageMotion = new HashMap<Host, Boolean>();
         DataCenterDeployment plan = null;
         if (canMigrateWithStorage) {
-            allHostsPair = searchForServers(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), null, null, null, keyword, null, null, srcHost.getHypervisorType(),
+            allHostsPair = searchForServersExcluding(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), null, null, srcHostId, keyword, null, null, srcHost.getHypervisorType(),
                     srcHost.getHypervisorVersion());
             allHosts = allHostsPair.first();
             allHosts.remove(srcHost);
 
 Review comment:
   @Pearl1594 can you share why you add the method searchForServersExcluding ?
   
   source host is removed in line 
   ```
   allHosts.remove(srcHost);
   ```

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: Fixed count value in the list apis

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: Fixed count value in the list apis
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588102559
 
 
   @weizhouapache it's related to the issue of count or items not returned in the API that are expected per the API docs usage of `listall` parameter. See my email on dev@. Thanks.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588165313
 
 
   > @weizhouapache it's related to the issue of count or items not returned in the API that are expected per the API docs usage of `listall` parameter. See my email on dev@. Thanks.
   
   @rhtyd @DaanHoogland do we need to add a global setting like 'display.project.resources.in.default.view' ? project resources are not displayed in older versions. I am ok no matter the answer is yes or no.
   
   @rhtyd considering you have made changes on listing project resources, can you please pend you more testing result like you did for other query ?
   
   - [ ] List VMs in project
   - [ ] List Volumes in project
   - [ ] List Isolated networks in project
   - [ ] List Shared networks for project
   
   and so on
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-590058541
 
 
   @blueorangutan test

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-591371163
 
 
   @DaanHoogland @andrijapanicsb I've tested apis mentioned in https://github.com/apache/cloudstack/pull/3894#issuecomment-590756281 (see ticked)
   
   I think this is ready for merging once it passes one of your reviews/testings.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] rhtyd commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#discussion_r382396997
 
 

 ##########
 File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java
 ##########
 @@ -2680,6 +2680,9 @@ public void buildACLSearchParameters(Account caller, Long id, String accountName
                         }
                     } else {
                         domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.ListProjectResourcesOnly);
+                        if (listAll) {
 
 Review comment:
   @Pearl1594 can you add a check here that listall is true and the user is an admin (not a domain admin, not a user but an admin) 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3894: api: Fix count and item issues returned by list APIs
URL: https://github.com/apache/cloudstack/pull/3894#issuecomment-588271293
 
 
   Packaging result: āœ–centos6 āœ”centos7 āœ”debian. JID-908

----------------------------------------------------------------
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


With regards,
Apache Git Services