Lukas Wagner [Tue, 21 May 2024 13:31:47 +0000 (15:31 +0200)]
tests: remove vzdump_notification test
With the upcoming changes in how we send notifications, this one
really becomes pretty annoying to keep working. The location where
templates are looked up are defined in the proxmox_notify crate, so
there is no easy way to mock this for testing.
The test itself seemed not super valuable, mainly testing if
the backup logs are shortened if they ware too long - so they are just
removed.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com> Tested-by: Max Carrara <m.carrara@proxmox.com> Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Thomas Lamprecht [Tue, 23 Apr 2024 17:51:26 +0000 (19:51 +0200)]
ui: importer: try to better convey what live-import does
It's hard to cram a easy to understandable meaning in the space we
have, to get a bit more space move the warning hint to a separate line
and use the box-label to show an always visible hint about the VM to
be stopped previously.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
fix #5251: login: set autocomplete on password and user
By default they have 'autocomplete=off'. From [1]:
> In most modern browsers, setting autocomplete to "off" will not
> prevent a password manager from asking the user if they would like to
> save username and password information, or from automatically filling
> in those values in a site's login form. See the autocomplete
> attribute and login fields [2].
Fiona Ebner [Fri, 9 Feb 2024 13:08:19 +0000 (14:08 +0100)]
ui: user edit: protect user's TFA settings again
Same rationale as in 5b25580d ("Protect the user's tfa key setting."):
it should not be possible to change the value when it's not an actual
secret but a reference to what TFA method is used or, in case of 'x',
whether TFA is used.
Dominik Csapak [Thu, 14 Dec 2023 09:55:17 +0000 (10:55 +0100)]
ui: mobile: enable subscription popup
not sure if this was lost at some point or never implemented, but we
want to be consistent with the remaining web ui and apps, so show
the subscription popup here too.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Thu, 14 Dec 2023 09:55:16 +0000 (10:55 +0100)]
ui: mobile: fix totp login
Log-in with TOTP enabled account on mobile was broken due to these two
commits:
- pve-manager: 509d7a20 ("mobile ui: implement dummy message box and
scrip loader")
- pve-access-control: cb64967 ("api: drop old verify_tfa api call")
The pve-manager one overwrote the Ext.MessageBox and Ext.Msg classes
and thus removed the Ext.MessageBox.OKCANCEL constant that represented
the buttons of popup messages (without those no buttons on message
boxes where shown).
This override did not work as intended, as we still showed the
message box by accident, because at that point the Ext.MessageBox was
already initialized (so it was overwritten), but Ext.Msg was not (this
happens later).
And the pve-access-control removed the old tfa verify api (which is
now done via the /access/ticket api)
So to fix that, we have to adapt to the api changes and restore the
stock Ext.MessageBox and Ext.Msg classes by removing the overrides
(i couldn't find where we would need those)
We still cannot handle u2f/WebAuthn or recovery methods though.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
ui: backup job editor: fix disable behavior for fleecing storage
commit 569b0388 (ui: fix reset behavior of backup job editor) disabled
the fleecing storage field by default (as that is the default state)
to fix the reset behavior. This broke editing the job when fleecing
was enabled and the user did not navigate to the advanced tab yet.
It seems that the 'bind' here only gets triggered once the panel is
rendered, but we actually need it before that.
To work around the issue for now, manually enable/disable the field
when toggling the fleecing checkbox. (Though this warrants a bit of
deeper investigation into this bind behavior)
when we `bind` we also have to set the initial value correctly,
otherwise the form dirty tracking is off (the initial bind set does not
reset the `originalValue`)
also the bandwidth selector auto transformed the value `null` to `0`
when there was no initial transformation. Since this is not a valid
value anyway, skip that.
Markus Frank [Mon, 15 Apr 2024 08:50:01 +0000 (10:50 +0200)]
ui: machine: add viommu ComboBox
Added a proxmoxKVComboBox for selecting a vIOMMU implementation for a VM.
If i440fx is selected, another ComboBox will be enabled/visible that does not
have the Intel option, as Intel-vIOMMU is not compatible with i440fx.
Uses the new machine property-string from the qemu-server's "config: define
machine schema as property-string" commit and the viommu option added in the
qemu-server's "fix #3784: config: Parameter for guest vIOMMU + test-cases"
commit.
acme: ui: handle missing meta field in directory response
When none of the meta fields is set by the directory, the whole
dictionary is missing from the response, leading to an exception
when testing for fields inside it.
Thomas Lamprecht [Mon, 22 Apr 2024 09:24:52 +0000 (11:24 +0200)]
ui: backup job: rework empty-text for advanced fields again
This partially reverts commit a32a5c4a6 ("ui: backup job: rework hint
about fallback config and make it less flashy"), i.e., the part about
the fallback values, as those was barely visible now.
Add the schema default to the end of the description and expand the
hint at the bottom to also mention that this is used as second level
fallback, if the vzdump.conf does not has the option set.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
ui: backup job: correctly align descriptions with fields in advanced options
Merges the column1/2/B into just single items so that the vertical
alignment is still correct even if a description wraps over multiple
lines.
Use the new pveTwoColumnContainer to achieve this without extra
boilerplate code and use a 1/3 of the width for the field and the 2/3
rest for the description.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: adapt to changes in prev. commit, reword message, fix eslint ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sun, 21 Apr 2024 11:01:53 +0000 (13:01 +0200)]
d/control: bump versioned dependency for widget-toolkit and common
To ensure that the lifting of the bridge name == vmbr\d+ restriction
works correctly and that the new notes view double-click editing
setting can work.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
ui: dc: backup: improve UX for the different 'notification-mode's
- Switch order of 'mailto' and 'mailnotification' field
- When mode is 'auto', disable 'mailtnotification' field
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com> Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
[ TL: drop the hint, not really explaining much as is so mostly
visible noise ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sat, 20 Apr 2024 18:15:03 +0000 (20:15 +0200)]
ui: guest stop: show overrule checkbox also if no task is active
The UI state about running tasks can be out of sync, especially for
situations where one quickly follows up with a stop, e.g. after
triggering a shutdown by mistake.
So, show the checkbox always for users that got Sys.Modify on (some)
node, but pre-check it still only if there where task detected on
component creation (we could watch the state though and show a hint,
but that's a bit over the top IMO).
Show it also when HA is enabled but explicitly disable it there,
hopefully this increases the chance that the users can understand that
this is done by design, and isn't a bug – ideally we would also show
an extra hint.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Friedrich Weber [Fri, 12 Apr 2024 14:15:53 +0000 (16:15 +0200)]
fix #4474: ui: guest stop: offer to overrule active shutdown tasks
Implement a new "guest stop" confirmation message box which first
checks if there is an active shutdown task for the same guest that is
visible to the logged-in user. If there is at least one, the dialog
displays an additional default-on checkbox for overruling active
shutdown tasks. If the user confirms and the checkbox is checked, the
UI sends a guest stop API request with the `overrule-shutdown`
parameter set to 1. If there are no active shutdown tasks, or the
checkbox is unchecked, the UI sends a guest stop API request without
`overrule-shutdown`.
To avoid an additional API request for querying active shutdown tasks,
check the UI's current view of cluster tasks instead, which is fetched
from the `pve-cluster-tasks` store.
As the UI might hold an outdated task list, there are some
opportunities for races, e.g., the UI may miss a new shutdown task or
consider a shutdown task active even though it has already terminated.
These races either result in a surviving shutdown task that the user
still needs to abort manually, or a superfluous `override-shutdown=1`
parameter that does not actually abort any tasks. Since "stop
overrules shutdown" is merely a convenience feature, both outcomes
seem bearable.
The confirmation message box is now always marked as dangerous (with a
warning sign icon), whereas previously it was only marked dangerous if
the stop issued from the guest panel, but not when issued from the
resource tree command menu.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: squash in some slightly opinionated code/style clean-ups ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Hanreich [Fri, 19 Apr 2024 09:42:37 +0000 (11:42 +0200)]
firewall: expose configuration option for new nftables firewall
There's a new firewall implementation available as `proxmox-firewall`
package, in contrast to the existing `pve-firewall` package it is
using nftables directly, not the legacy iptables, and can thus
leverage a modern stack with atomic updates, avoiding the need for
different tools (e.g., ebtables), and not requiring intermediate
firewall bridges to handle VM flow correctly. Additionally it's
written in rust, making it more efficient and safer to change.
The new implementation is using the same configuration file as source
and should be mostly the same in semantic behavior, it basically is a
drop-in replacement besides one known issue:
There is currently one major issue that we still need to solve:
REJECTing packets from the guest firewalls is currently not possible
for incoming traffic (it will instead be dropped).
This is due to the fact that we are using the postrouting hook of
nftables in a table with type bridge for incoming traffic. In the
bridge table in the postrouting hook we cannot tell whether the packet
has also been sent to other ports in the bridge (e.g. when a MAC has
not yet been learned and the packet then gets flooded to all bridge
ports). If we would then REJECT a packet in the postrouting hook this
can lead to a bug where the firewall rules for one guest REJECT a
packet and send a response (RST for TCP, ICMP port/host-unreachable
otherwise).
While this is being addressed, and the whole stack is better tested in
general, the new FW will be only enabled if the admin enables a
boolean configuration which this patch exposes on the UI.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
d/control bump versioned dependency for libpve-(guest-)common-perl and qemu-server
to make the backup fleecing feature available. The bump for
qemu-server is also required for moving unused disks of VMs.
The bump for libpve-common-perl is required because of pve-common
commit c302a28 ("json schema: add format description for
pve-storage-id standard option"), which is required for API
verification.
vzdump: have property string helpers always return the result
Previously, the result would only be returned implicitly and if not
already parsed. While callers do not strictly need the return value,
future callers might mistakenly rely on it and even work by chance in
some scenarios, because of the implicit return. Make the code more
future proof by explicitly returning the result in all cases.
There was an oversight with recent replication fixes that led to
attempting to remove snapshots that do not exist (in more scenarios).
While not an issue with real consequences, it's confusing to users.
This has since been fixed by pve-guest-common commit "replication:
snapshot cleanup: only attempt to remove snapshots that exist".
ui: acme: add External Account Binding (EAB) related fields
Adds fields for eab credentials. By default eab is optional, but if the
directory should report that eab is required, the eab credential fields
are marked as mandatory and prevent the form from being submittable
until credentials are provided.
Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This patch allows the user to set a custom ACME directory by providing
a 'Custom' option in the directory dropdown. This in turn reveals an
input for the url. When using a custom directory the directory has to
be manually queried via button press to prevent from spamming the
directory on every input.
Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
else this can break an upgrade for unrelated reasons (regular debhelper also
constructs the restart invocations like this, it even redirects output to
/dev/null)
Thomas Lamprecht [Wed, 17 Apr 2024 12:22:41 +0000 (14:22 +0200)]
ui: backup job: avoid calling max-workers VM workers
that could make some users (not reading the explanation on the right
closely) belief that this controls the amount of parallel VMs to be
backed up or the like.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
close #4513: ui: backup job: add tab for advanced options
pigz is not exposed, because it only works after manually installing
the pigz package.
ionice is not exposed, because it only works in combination with the
BFQ scheduler and even then not in all cases (only affects the
compressor when doing snapshot/suspend mode backup of a VM).
The pbs-entries-max performance option is not exposed. It is rather
niche and hard to understand. It serves as an escape hatch for
rare/extreme cases.
These can still be added with appropriate notes if there is enough
user demand.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vzdump: use per-property fallback for performance settings
Currently, fallback for the 'performance' option is done as a whole,
taking away flexibility from the user. It also means that when only
one of the two sub-properties is specified, the other one will default
to the backend (i.e. QEMU or proxmox-backup-client) default rather
than the schema default. For the latter point in particular, it can be
argued to be incorrect. These limitations will only get worse in the
future with more sub-properties.
Switch to a per-property fallback mechanism to improve the situation,
having each go through the usual preference order (CLI/job > node-wide
default > schema default).
Technically, this is a breaking change, but pbs-entries-max is rather
new and potential for breakage seems rather low. Requirements for
breakage:
* job (or CLI) that defines only one of the performance options
* job also covers a guest where the other performance option applies
* the other performance option is defined in the node-wide configuration
* the node-wide setting is worse for the job than the implicit backend
default (because this change will have the node-wide default win over
the implicit backend default).
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vzdump: actually honor schema defaults for performance
The 'performance' option itself defines no 'default' in the schema, so
what happened is that the defaults used by the backends (i.e. QEMU and
proxmox-backup-client) would be used. Luckily, they correspond to the
default values defined in the schema, i.e. in the 'backup-performance'
format. Make the code future-proof and use the actual defaults defined
in the schema instead of relying on that correspondence.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 17 Apr 2024 09:33:55 +0000 (11:33 +0200)]
ui: lxc: keep passthrough ID internal for now
this is not like mount points, where the order can make a difference,
but rather like the PCI passthrough for VMs, for which we do not
expose editing the ID either.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Hanreich [Tue, 16 Jan 2024 14:30:22 +0000 (15:30 +0100)]
firewall: properly detect changes when ip / cidr is used in rule
With the current implementation using queryDelay, this means that the
change event for the input never completes. This in turn leads to
the input panel never changing its dirty status. By using the
beforequery event we can simply cancel the query without resorting to
the queryDelay hack.
Reported-By: Mira Limbeck <m.limbeck@proxmox.com> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> Tested-by: Mira Limbeck <m.limbeck@proxmox.com> Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>
fall back to using v.ref as value when we do not have an alias or ipset
since scope and name are not set for ips / cidrs
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> Tested-by: Filip Schauer <f.schauer@proxmox.com> Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com> Tested-by: Mira Limbeck <m.limbeck@proxmox.com>
and guard it to only run on ceph-using systems (the regular 'inited' check
doesn't work as a guard for this, because it checks for new-style inits
including the dir existing).
Max Carrara [Tue, 2 Apr 2024 14:55:22 +0000 (16:55 +0200)]
fix #4759: ceph: configure ceph-crash.service and its key
Due to Ceph dropping privileges when running the 'ceph-crash' daemon
[0], it is necessary to allow the daemon to authenticate with its
cluster in a safe manner.
In order to avoid exposing sensitive keyrings or somehow escalating
its privileges again, 'ceph-crash' is therefore provided with its own
keyring in the '/etc/pve/ceph' directory. This directory, due to being
on 'pmxcfs', may be read by members of the 'www-data' group, which
'ceph-crash' is made part of [1].
Expected Configuration
----------------------
1. A keyring file named '/etc/pve/ceph/ceph.client.crash.keyring'
exists
2. A section named 'client.crash' exists in '/etc/pve/ceph.conf'
3. The 'client.crash' section has a key named 'keyring' which
references the keyring file as '/etc/pve/ceph/$cluster.$name.keyring'
4. The 'client.crash' section has *no* key named 'key'
New Clusters
------------
The keyring file is created and the conf file is updated after the first
monitor has been created (when calling `pveceph mon create`).
Existing Clusters
-----------------
A new helper script creates and configures the 'client.crash' keyring in
`postinst`, if:
* Ceph is installed
* Ceph is initialized ('/etc/pve/ceph.conf' and '/etc/pve/ceph' exist)
* Connection to RADOS is successful
If the above conditions are met, the helper script ensures that the
existing configuration matches the expected configuration mentioned
above.
The configuration is not changed if it is already as expected.
The helper script may be called again manually if the `postinst` hook
fails. It is installed to '/usr/share/pve-manager/helpers/pve-init-ceph-crash'.
Max Carrara [Tue, 2 Apr 2024 14:55:21 +0000 (16:55 +0200)]
ceph: introduce '/etc/pve/ceph'
This commit adds the '/etc/pve/ceph' directory to our overall expected
Ceph configuration.
This directory is meant to store cluster-wide, non-private
configuration files used by Ceph applications and services that are
executed with lower privileges, such as 'ceph-crash.service'.
The existence of the directory is now also checked for when checking
whether Ceph is configured correctly. This makes it easier for our
other tooling to rely on the directory's existence, reducing the
number of otherwise needless frequent checking.
* For new clusters: `pveceph init` now creates '/etc/pve/ceph' when
called.
* For existing clusters: The 'postinst' hook this commit adds ensures
that '/etc/pve/ceph' is created when updating.
Signed-off-by: Max Carrara <m.carrara@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com>