we have a param_mapping now, with which we can impement that e.g. instead of using:
sub read_password { # read password } we can do: sub param_mapping { my ($name) = @_; my $password_map = ['password', sub{ # read password }, '<password', 1]; my $mapping = { 'apicall1' => [$password_map], 'apicall2' => [$password_map], ... }; return $mapping->{$name}; } this has the advantage that we can use different subs for different api calls (e.g. pveum passwd vs pveum ticket) Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> --- changes from v1: * changed &$sub() to $sub->() where we touch lines * removed if(mapdef && name eq password) check (unecessary because of code below it) * fixed condiditional my statement src/PVE/CLIHandler.pm | 18 ++++++++---------- src/PVE/JSONSchema.pm | 15 +-------------- src/PVE/RESTHandler.pm | 32 +++++++++++++++----------------- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm index 9bbc156..cb51d92 100644 --- a/src/PVE/CLIHandler.pm +++ b/src/PVE/CLIHandler.pm @@ -127,7 +127,6 @@ sub generate_usage_str { $separator //= ''; $indent //= ''; - my $read_password_func = $cli_handler_class->can('read_password'); my $param_mapping_func = $cli_handler_class->can('param_mapping') || $cli_handler_class->can('string_param_file_mapping'); @@ -150,7 +149,7 @@ sub generate_usage_str { $str .= $indent; $str .= $class->usage_str($name, "$prefix $cmd", $arg_param, $fixed_param, $format, - $read_password_func, $param_mapping_func); + $param_mapping_func); $oldclass = $class; } elsif (defined($def->{$cmd}->{alias}) && ($format eq 'asciidoc')) { @@ -174,7 +173,7 @@ sub generate_usage_str { $str .= $indent; $str .= $class->usage_str($name, $prefix, $arg_param, $fixed_param, $format, - $read_password_func, $param_mapping_func); + $param_mapping_func); } return $str; }; @@ -466,7 +465,7 @@ sub setup_environment { } my $handle_cmd = sub { - my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_; + my ($args, $preparefunc, $param_mapping_func) = @_; $cmddef->{help} = [ __PACKAGE__, 'help', ['extra-args'] ]; @@ -496,13 +495,13 @@ my $handle_cmd = sub { my ($class, $name, $arg_param, $uri_param, $outsub) = @{$def || []}; $abort->("unknown command '$cmd_str'") if !$class; - my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, $uri_param, $read_password_func, $param_mapping_func); + my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, $uri_param, $param_mapping_func); &$outsub($res) if $outsub; }; my $handle_simple_cmd = sub { - my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_; + my ($args, $preparefunc, $param_mapping_func) = @_; my ($class, $name, $arg_param, $uri_param, $outsub) = @{$cmddef}; die "no class specified" if !$class; @@ -531,7 +530,7 @@ my $handle_simple_cmd = sub { &$preparefunc() if $preparefunc; - my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, $uri_param, $read_password_func, $param_mapping_func); + my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, $uri_param, $param_mapping_func); &$outsub($res) if $outsub; }; @@ -552,7 +551,6 @@ sub run_cli_handler { my $preparefunc = $params{prepare}; - my $read_password_func = $class->can('read_password'); my $param_mapping_func = $cli_handler_class->can('param_mapping') || $class->can('string_param_file_mapping'); @@ -564,9 +562,9 @@ sub run_cli_handler { $cmddef = ${"${class}::cmddef"}; if (ref($cmddef) eq 'ARRAY') { - &$handle_simple_cmd(\@ARGV, $read_password_func, $preparefunc, $param_mapping_func); + $handle_simple_cmd->(\@ARGV, $preparefunc, $param_mapping_func); } else { - &$handle_cmd(\@ARGV, $read_password_func, $preparefunc, $param_mapping_func); + $handle_cmd->(\@ARGV, $preparefunc, $param_mapping_func); } exit 0; diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm index f014dc3..1259195 100644 --- a/src/PVE/JSONSchema.pm +++ b/src/PVE/JSONSchema.pm @@ -1333,7 +1333,7 @@ sub method_get_child_link { # a way to parse command line parameters, using a # schema to configure Getopt::Long sub get_options { - my ($schema, $args, $arg_param, $fixed_param, $pwcallback, $param_mapping_hash) = @_; + my ($schema, $args, $arg_param, $fixed_param, $param_mapping_hash) = @_; if (!$schema || !$schema->{properties}) { raise("too many arguments\n", code => HTTP_BAD_REQUEST) @@ -1362,11 +1362,6 @@ sub get_options { # optional and call the mapping function afterwards. push @getopt, "$prop:s"; push @interactive, [$prop, $mapping->{func}]; - } elsif ($prop eq 'password' && $pwcallback) { - # we do not accept plain password on input line, instead - # we turn this into a boolean option and ask for password below - # using $pwcallback() (for security reasons). - push @getopt, "$prop"; } elsif ($pd->{type} eq 'boolean') { push @getopt, "$prop:s"; } else { @@ -1418,14 +1413,6 @@ sub get_options { } } - if (my $pd = $schema->{properties}->{password}) { - if ($pd->{type} ne 'boolean' && $pwcallback) { - if ($opts->{password} || !$pd->{optional}) { - $opts->{password} = &$pwcallback(); - } - } - } - foreach my $entry (@interactive) { my ($opt, $func) = @$entry; my $pd = $schema->{properties}->{$opt}; diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm index 50c37c2..7732157 100644 --- a/src/PVE/RESTHandler.pm +++ b/src/PVE/RESTHandler.pm @@ -432,7 +432,7 @@ sub handle { # $style: 'config', 'config-sub', 'arg' or 'fixed' # $mapdef: parameter mapping ({ desc => XXX, func => sub {...} }) my $get_property_description = sub { - my ($name, $style, $phash, $format, $hidepw, $mapdef) = @_; + my ($name, $style, $phash, $format, $mapdef) = @_; my $res = ''; @@ -449,10 +449,6 @@ my $get_property_description = sub { my $type_text = PVE::JSONSchema::schema_get_type_text($phash, $style); - if ($hidepw && $name eq 'password') { - $type_text = ''; - } - if ($mapdef && $phash->{type} eq 'string') { $type_text = $mapdef->{desc}; } @@ -576,10 +572,9 @@ my $compute_param_mapping_hash = sub { # 'short' ... command line only (text, one line) # 'full' ... text, include description # 'asciidoc' ... generate asciidoc for man pages (like 'full') -# $hidepw ... hide password option (use this if you provide a read passwork callback) # $param_mapping_func ... mapping for string parameters to file path parameters sub usage_str { - my ($self, $name, $prefix, $arg_param, $fixed_param, $format, $hidepw, $param_mapping_func) = @_; + my ($self, $name, $prefix, $arg_param, $fixed_param, $format, $param_mapping_func) = @_; $format = 'long' if !$format; @@ -612,7 +607,7 @@ sub usage_str { foreach my $k (@$arg_param) { next if defined($fixed_param->{$k}); # just to be sure next if !$prop->{$k}; # just to be sure - $argdescr .= &$get_property_description($k, 'fixed', $prop->{$k}, $format, 0); + $argdescr .= $get_property_description->($k, 'fixed', $prop->{$k}, $format); } my $idx_param = {}; # -vlan\d+ -scsi\d+ @@ -624,7 +619,12 @@ sub usage_str { my $type_text = $prop->{$k}->{type} || 'string'; - next if $hidepw && ($k eq 'password') && !$prop->{$k}->{optional}; + my $param_mapping_hash = {}; + + if ($param_mapping_func) { + $param_mapping_hash = $compute_param_mapping_hash->(&$param_mapping_func($name)); + next if $param_mapping_hash->{password} && ($k eq 'password') && !$prop->{$k}->{optional}; + } my $base = $k; if ($k =~ m/^([a-z]+)(\d+)$/) { @@ -636,11 +636,9 @@ sub usage_str { } } - my $param_mapping_hash = $compute_param_mapping_hash->(&$param_mapping_func($name)) - if $param_mapping_func; - $opts .= &$get_property_description($base, 'arg', $prop->{$k}, $format, - $hidepw, $param_mapping_hash->{$k}); + $opts .= $get_property_description->($base, 'arg', $prop->{$k}, $format, + $param_mapping_hash->{$k}); if (!$prop->{$k}->{optional}) { $args .= " " if $args; @@ -703,7 +701,7 @@ sub dump_properties { } } - $raw .= &$get_property_description($base, $style, $phash, $format, 0); + $raw .= $get_property_description->($base, $style, $phash, $format); next if $style ne 'config'; @@ -736,14 +734,14 @@ my $replace_file_names_with_contents = sub { }; sub cli_handler { - my ($self, $prefix, $name, $args, $arg_param, $fixed_param, $read_password_func, $param_mapping_func) = @_; + my ($self, $prefix, $name, $args, $arg_param, $fixed_param, $param_mapping_func) = @_; my $info = $self->map_method_by_name($name); my $res; eval { my $param_mapping_hash = $compute_param_mapping_hash->($param_mapping_func->($name)) if $param_mapping_func; - my $param = PVE::JSONSchema::get_options($info->{parameters}, $args, $arg_param, $fixed_param, $read_password_func, $param_mapping_hash); + my $param = PVE::JSONSchema::get_options($info->{parameters}, $args, $arg_param, $fixed_param, $param_mapping_hash); if (defined($param_mapping_hash)) { &$replace_file_names_with_contents($param, $param_mapping_hash); @@ -756,7 +754,7 @@ sub cli_handler { die $err if !$ec || $ec ne "PVE::Exception" || !$err->is_param_exc(); - $err->{usage} = $self->usage_str($name, $prefix, $arg_param, $fixed_param, 'short', $read_password_func, $param_mapping_func); + $err->{usage} = $self->usage_str($name, $prefix, $arg_param, $fixed_param, 'short', $param_mapping_func); die $err; } -- 2.11.0 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel