[pve-devel] [PATCH common 1/2] remove read_password_func from cli handler
Dominik Csapak
d.csapak at proxmox.com
Tue Jun 12 12:33:46 CEST 2018
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.csapak at proxmox.com>
---
src/PVE/CLIHandler.pm | 18 ++++++++----------
src/PVE/JSONSchema.pm | 15 +--------------
src/PVE/RESTHandler.pm | 26 +++++++++++++-------------
3 files changed, 22 insertions(+), 37 deletions(-)
diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
index 8c911b2..b398f12 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');
@@ -149,7 +148,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')) {
@@ -173,7 +172,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 5e70503..44af354 100644
--- a/src/PVE/RESTHandler.pm
+++ b/src/PVE/RESTHandler.pm
@@ -429,7 +429,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 = '';
@@ -446,7 +446,7 @@ my $get_property_description = sub {
my $type_text = PVE::JSONSchema::schema_get_type_text($phash, $style);
- if ($hidepw && $name eq 'password') {
+ if ($mapdef && $name eq 'password') {
$type_text = '';
}
@@ -573,10 +573,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;
@@ -609,7 +608,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+
@@ -621,7 +620,10 @@ sub usage_str {
my $type_text = $prop->{$k}->{type} || 'string';
- next if $hidepw && ($k eq 'password') && !$prop->{$k}->{optional};
+ my $param_mapping_hash = $compute_param_mapping_hash->(&$param_mapping_func($name))
+ if $param_mapping_func;
+
+ next if $param_mapping_hash->{password} && ($k eq 'password') && !$prop->{$k}->{optional};
my $base = $k;
if ($k =~ m/^([a-z]+)(\d+)$/) {
@@ -633,11 +635,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});
+ $param_mapping_hash->{$k});
if (!$prop->{$k}->{optional}) {
$args .= " " if $args;
@@ -700,7 +700,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';
@@ -733,14 +733,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);
@@ -753,7 +753,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
More information about the pve-devel
mailing list