api: Remove redundant API parameters

[Pearl1594]

Description

This PR removes the redundant api parameters listed under listVirtualMachines API. The parameters are already defined in ListVMsCmdByAdmin class

(localcloud) 🐱 > list virtualmachines -h
listVirtualMachines: List the virtual machines owned by the account.
API Params               Type     Description
==========               ====     ===========
account                  string   list resources by account. Must be used w
                                  ith the domainId parameter.
...
hostid                   uuid     the host ID
hostid                   uuid     the host ID
...
podid                    uuid     the pod ID
podid                    uuid     the pod ID
...
storageid                uuid     the storage ID where vm's volumes belong 
                                  to
storageid                uuid     the storage ID where vm's volumes belong 
                                  to
...

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• 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)

Feature/Enhancement Scale or Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

[weizhouapache]

code lgtm

[weizhouapache]

@blueorangutan package

[blueorangutan]

@weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 2942

[Pearl1594]

@blueorangutan package

[GabrielBrascher]

Thanks for the PR, @Pearl1594.

I would like to double-check if those parameters should be aimed only at Admins.
I ask this because it is not clear if that's the goal of this PR as well, or if it is aiming just at removing the redundant/duplicated parameters.

If the goal is to remove duplicated parameters but keep the current API behavior (Users & Admins), then I believe it should instead be removed at the ListVMsCmdAdmin which extends ListVMsCmd.

[Pearl1594]

Thanks for reviewing @GabrielBrascher , the following params: pod_id, storage_id, host_id, seem to be used only applicable only for root admins - https://github.com/apache/cloudstack/blob/main/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java#L1004-L1011 , hence I took the following approach. It removes the redundant params and also ensures to behave the same way as before.

[DaanHoogland]

@Pearl1594 shouldn't a domain- or resource admin be allowed to list by host, pod, zone ?

[GabrielBrascher]

Sorry @Pearl1594, I raised the question regarding "Admins & Users".
However, my concerns are regarding what @DaanHoogland mentioned, Admins & Domain Admins.

[Pearl1594]

It seems that only listing zones is allowed for all roles - listing storage pools, hosts, clusters and pods, are restricted to root admins only. So, this change shouldn't effect any behavior.

[Pearl1594]

@blueorangutan package

[blueorangutan]

@Pearl1594 a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[weizhouapache]

code lgtm

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2943

[Pearl1594]

@blueorangutan test

[blueorangutan]

@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[GabrielBrascher]

LGTM

[blueorangutan]

Trillian test result (tid-3683)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31833 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6143-t3683-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[nvazquez]

Code LGTM, checked the UI codebase and could not find any of the params being passed to the listVirtualMachines API - would be good to test the different roles to verify the behaviour is not affected

[nvazquez]

Have tested different roles and as advised by @Pearl1594 the removed parameters only apply for ROOT admin