[pve-devel] [PATCH access-control v5 0/5] refactor API and introduce list commands

[Thomas Lamprecht]

On 6/21/18 2:31 PM, Stoiko Ivanov wrote:
> The last two patches rely on the already applied changes to pve-common, maybe
> we want to raise the dependency version upon packaging.
> 
> Changes from v4:
> * split up commits into hopefully sensible chunks
> * leave the userid option in PVE::Auth::Plugin, and add userid-completed in
>   PVE::AccessControl, which then gets used in the API2 modules
> * fix a bug in v4, which broke ticket generation/login
>   (thanks for catching it @Thomas!)
> * I ran some very rudimentary tests on each commit (create user via pvesh,
>   get login ticket, try to recreate the user, delete the user) for every commit
> 
> 
> Stoiko Ivanov (5):
>   fix PVE::AccessControl::role_is_special
>   PVE::AccessControl: register userid with completion
>   refactor API using get/register_standard_option
>   Add title and print_width fields to properties
>   pveum: add list and dump commands
> 
>  PVE/API2/ACL.pm           |  36 +++++++------
>  PVE/API2/AccessControl.pm |  12 ++---
>  PVE/API2/Group.pm         |  37 +++++++------
>  PVE/API2/Role.pm          |  54 ++++++++++++-------
>  PVE/API2/User.pm          | 135 +++++++++++++++++++++++-----------------------
>  PVE/AccessControl.pm      |   8 ++-
>  PVE/Auth/Plugin.pm        |   2 +-
>  PVE/CLI/pveum.pm          |  20 +++++++
>  8 files changed, 175 insertions(+), 129 deletions(-)
> 

applied all (newest version, respectively) but 5/5 for now, much thanks!

on 5/5 there's not much to say against, two nits maybe:
* I'd output the comment to in the list, at least for groups
* I do not like 'dump' as a command name to much, user not coming
  from an non-programming environment (or one with no Data::Dumper ;))
  may confuse this with the 'throw away' meaning of dump, maybe just
  'show' ?

But these are needs and could be fixed-up. I delayed applying because
this is the first use of the net print api/table methods and I get the
feeling that they may belong in their own perl module, PVE::Format as
example. Looking at the rate code and changes enter CLIHandler regarding
this stuff I have a feeling that we could have more formatting and pretty
table printing code there than "actual" CLIHandler stuff.
This is no must have but as it is really easy to move now, with no user of
those methods, it's should be worth a thought.

cheers and much thanks for the api cleanup again!