cli: more generic interactive parameter definition
Instead of hardcoding 'password' as a special case in the
JSONSchema's getopt handling, extend the new parameter
mapping to allow defining a parameters as 'interactive'.
They also take an optional argument on the command line
directly.
This effectively deprecates the password special case which
should be replaced in pct/pveum/... and then dropped in
pve-common.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
track our command string, i.e. everything which cannot be an argument
for a specific command, in resolve_cmd, as we go through the commando
definition there anyway and know if a ARGV element is part of the
command itself or its arguments.
Fixes a problem where a invalid command had all the passed parameter
attached in the resulting USAGE output.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
the timegm(gmtime()) and timelocal(localtime(()) constructs are
problematic in the following case: - $last is such that $year gets set
to a two-digit value (e.g., the referred to timestamp is somewhere in
the range of 1900-1999) - the current date is such that the value of
$year gets interpreted wrongly (e.g., anything other than 1950).
the exact breakage depends on the actual current date AND value of
$last, since localtime/gmtime will interpret two-digit years as (perldoc
Time::Local):
[...] shorthand for years in the rolling "current century," defined
as 50 years on either side of the current year. Thus, today, in
1999, 0 would refer to 2000, and 45 to 2045, but 55 would refer to
1955. Twenty years from now, 55 would instead refer to 2055.
fix it by adding 1900 to force 4-digit $year values, as the localtime
documentation suggests.
cli: allow specifying sub commands through $cmddef
allow to use sub commands alá
# pveum user add
The new resolve_cmd traverses $cmddef, resolves one level of aliases
and returns the respective sub command, its cmddef, arguments and if
it was expanded (e.g., pveum u d ... => pveum user delete ...) which
allows quite easy integration in the usage/synopsis generator, bash
completion helper and command handler.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
fork_worker: use correct handle type for POSIX::write
$resfh can be a pipe from POSIX::pipe() or the upid output
handle, which is an IO::File, so we need to take its
fileno().
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Fixes: ed52a8435a6d ("fork_worker: use separate pipe for status messages") Reviewed-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Tue, 19 Dec 2017 10:50:16 +0000 (11:50 +0100)]
run_command: add 'quiet' parameter for omitting STD* prints
Without this patch we printed to STDOUT and STDERR, respectively, if
no $outfunc or $errfunc was passed.
Sometimes it's useful if one, or even both, of those prints can
be suppressed, currently this can only be done by either using an array
of arrays or a whole string for the command and redirecting STDOUT
and STDERR.
Add a 'quiet' option which allows to do this in an easier way.
It allows to silent STDERR or STDOUT or both.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 15 Dec 2017 16:00:31 +0000 (17:00 +0100)]
fork_worker: factor out synced worker output mirroring
When running in sync (CLI environment) we mirror the workers output
to both, STDOUT and th task log file, a similar function as the unix
comand line tool tee provides, thus we borrow its name for the
factored out sub method.
This moves ~60 lines of code out of the big fork_worker sub and makes
it easier to read track what happens there.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 15 Dec 2017 16:00:30 +0000 (17:00 +0100)]
fork_worker: use separate pipe for status messages
We forced line wise flushing of the workers STDOUT and STDERR to
capture the final status (TASK OK/TASK ERROR).
Thus, if the code executed in the worker wanted to flush explicitly,
e.g., when the last output wasn't new line terminated but needed to
reach the users eyes, the parent just ignored that.
This leads to confusing results in CLI handlers using fork_workers.
So remove the buffering logic completely and introduce a separate
pipe for sending the final status.
Said pipe gets once read after the child closes (EOF) its STDOUT.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 15 Dec 2017 16:00:29 +0000 (17:00 +0100)]
fork_worker: refactor passing $upid to parent for sync
STDOUT and $psync[1] are the same here, so no need to differ.
Also we do this only for letting the parent know tha we're ready, the
parent knows the UPID already as it was generated before forking.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 15 Dec 2017 05:41:49 +0000 (06:41 +0100)]
ticket: raise UNAUTHORIZED not FORBIDDEN in verify subs
In the ticket and CSRF prevention token verification methods we used
a raise_perm exception to tell our caller about a failure of such a
verification. raise_perm uses HTTP_FORBIDDEN (403) as code.
Earlier, all such exceptions or die's where caught when the anyevent
http server called the auth_handler method and transformed to
HTTP_UNAUTHORIZED (401).
With commit d8327719e353198a1dffad88c246fee065054a6b from
pve-http-server we gained the ability to tell a client about a server
internal 5XX error, so that clients do not get wrongly logged out if
we have a internal error.
This resulted also in the effect that the exceptions of the
verify_rsa_ticket and verify_csrf_prevention_token sub methods where
passed to the client.
If an old, now invalid, ticket was sent to the server a client got
403 (FORBIDDEN) instead of the 401 (UNAUTHORIZED) - which he was used
to, and thus meant that he did some wrong doing, instead of knowing
that he just needs to login.
As we are not yet logged in here, and thus cannot possibly know if
the call is forbidden or not, HTTP_FORBIDDEN seems the wrong code.
Change it to HTTP_UNAUTHORIZED, which restores it to the code we told
API clients since ever and is the correct one here.
Also RFC 2068 section 10.4.4 [1] defines that for the afformentioned
verify methods FORBIDDEN was not really correct:
> 403 Forbidden
>
> The server understood the request, but is refusing to fulfill it.
> Authorization will not help and the request SHOULD NOT be
> repeated. [...]
With a invalid ticket or CSRF prevention token we have a
authorization problem for the current call, not a permission problem
(we may have, but we can't tell yet).
* Cancel on Ctrl+C (die())
* Finish on Ctrl+D (eof/eot) without appending a newline
* Also finish on \n to be sure.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Reviewed-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Most times a port was requested for a specified IP family (v4, v6)
only. Thus also ensure that the port from the respective family got
ready, else we may return on a false positive.
As we had no user setting the $timeout param we can add the $family
param as second one, it'll get used more often, so no need to put it
at the back.
As we do nothing if not defined this does not changes the behavior of
our users yet.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 10 Nov 2017 11:09:27 +0000 (12:09 +0100)]
daemon: don't send SIGTERM before restart on leave_children_open_on_reload
Else this options is not really useful. First, sending a SIGTERM lets
the children exit, not quite what "leave_children_open_on_reload"
promises.
The problem this causes is that we may get a time window where no
worker is active and thus, for example, our API daemon would not
accept connections during a restart (or better said, reload).
So, don't request termination of any child worker, if this option is
set, but rather just restart (re-exec) ourself, startup a new set of
workers and only then request the termination of the old ones,
allowing a fully seamless reload.
This is only done on `$daemon-exe restart` and thus on
`systemctl reload $daemon`, systemctl restart or any other stop start
cycles always exit all other workers first.
This expects that the worker can do a graceful termination on
SIGTERM, which is already the case for anything using our AnyEvent
based class (which is base of our HTTPServer module).
With graceful termination is meant the following: the worker accepts
no new work and exits immediately after the current queued work is
done.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 10 Nov 2017 09:24:25 +0000 (10:24 +0100)]
lock_file_full: add missing trailing newline
When we do not instantly get the lock we print a respective message
to stderr. This shows also up in the task logs, and if it's the last
message before a 'Task OK' the UI gets confused an shows the task as
erroneous.
Keep the message as its a good feedback for the user to see why an op
seems to do nothing, so simply add a trailing newline.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Use double hyphens when prefixing command options in the documentation
This makes our man pages follow the GNU long option recommandations
where non-single character options are prefixed with a double hyphen
(https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html)
The benefit for PVE is that our documentation looks more similar to what
a user with previous Linux knowledge is used to.
Our bash autocompletion helper only completes options using double hyphens too.
Thomas Lamprecht [Mon, 11 Sep 2017 08:41:34 +0000 (10:41 +0200)]
Tools: add `convert_size` for generic byte conversion
We often need to convert between file sizes, for formatting output,
but also code-internal. Some methods expect kilobytes, some gigabytes
and sometimes we need bytes.
While conversion from smaller to bigger units can be simply done with
a left-shift, the opposite conversion may need more attention -
depending on the used context.
If we allocate disks this is quite critical. For example, if we need
to allocate a disk with size 1023 bytes using the
PVE::Storage::vdisk_alloc method (which expects kilobytes) a
right shift by 10 (<=> division by 1024) would result in "0", which
obviously fails.
Thus we round up the converted value if a remainder was lost on the
transformation in this new method. This behaviour is opt-out, to be
on the safe side.
The method can be used in a clear way, as it gives information about
the source and target unit size, unlike "$var *= 1024", which doesn't
gives direct information at all, if not commented or derived
somewhere from its context.
For example:
> my $size = convert_unit($value, 'gb' => 'kb');
is more clear than:
> my $size = $value*1024*1024;
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
run_fork_with_timeout: do not overwrite global signal handlers
perls 'local' must be either used in front of each $SIG{...}
assignments or they must be put in a list, else it affects only the
first variable and the rest are *not* in local context.
This may cause weird behaviour where daemons seemingly do not get
terminating signals delivered correctly and thus may not shutdown
gracefully anymore.
As we only send SIGINT to processes if a manual stop action gets
triggered just catch this one here.
As this is a general method which allows to pass an arbitrary code
payload we cannot sanely handle all signals here, so remove trapping
all other besides SIGINT, if those need to be trapped that should be
done by the caller on a case by case basis.
Fixes: #1495 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Don't die because the tasklist could not be broadcasted, just log the
error.
Else we may hinder all task to run with a quite confusing error (i.e.
"ipcc_send_rec: file to big").
This may happen if there are a lot currently running tasks at once.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Tue, 27 Jun 2017 09:12:04 +0000 (11:12 +0200)]
use more reliable checks in wait_for_vnc_port
We run into problems where this method returned to early, even if the
port wasn't actually ready yet. The reason for this is that we
checked /proc/net/tcp which does not guarantees and always up to date
state of only those ports which are actuall available, i.e. a port
could linger around (time-wait state) or appear even if it wasn't
accepting connections yet (as stated in the kernel docs:
/proc/net/tcp is seen as obsolete by the kernel devs).
Use the `ss` tool from the iproute2 package, it uses netlink to get
the current state and has switches where we can direct it to really
only get the state of those sockets which interest us currently.
I.e., we tell it to get only listening TCP sockets from the requested
port.
The only drawback is that we loop on a run_command, which is slower
than just reading a file. A single loop needs about 1ms here vs the
60µs on the /proc/net/tcp read. But this isn't a api call which is
used highly frequently but rather once per noVNC console open.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Tue, 13 Jun 2017 09:25:33 +0000 (11:25 +0200)]
trim event and check if empty
give a meaningful error if it is empty and disallow it instead of having
an implicit default (the default should be set by the component using
the calendarevent, not the calendarevent itself)
projects / pve-common.git / log