Re: [PATCH proxmox-backup v2 19/27] ui: define and expose encryption key management menu item and windows

Thomas Lamprecht
2026-04-11T08:51:56Z

Am 10.04.26 um 18:54 schrieb Christian Ebner:
> diff --git a/www/config/EncryptionKeysView.js b/www/config/EncryptionKeysView.js
> @@ -0,0 +1,324 @@
>
> + archiveEncryptionKey: function () {
> + ...
> + if (selection[0].data.type === 'tape') {
> + Ext.Msg.alert(gettext('Error'), gettext('cannot archive tape key'));

Missing `return;` here - the code falls through to the API2Request below even
after showing the error alert for tape keys. The enableFn on the button should
prevent reaching this in practice, but it is still wrong.

> + }

> + reload: async function () {
> + let [syncKeys, tapeKeys] = await Promise.all([syncKeysFuture, tapeKeysFuture]);

Might profit from wrapping this in a try/catch - if the user lacks permissions
for one of the two endpoints the entire panel dies with an unhandled promise
rejection instead of showing partial results.

nit: Type column with width 50 px might be rather narrow for icon + label, but
did not checked out the result yet, so YMMV.

nit: the synthetic tape key IDs are not super nice/telling on their own. The
tape keys have a hint field that would be more meaningful to display (in
addition?).

> diff --git a/www/window/EncryptionKeysEdit.js b/www/window/EncryptionKeysEdit.js
> @@ -0,0 +1,383 @@
>
> + if (key.data === undefined) {
> + return 'Does not seems like a valid Proxmox Backup key!';

nit: missing gettect for both validator strings, also: s/seems/seem/

> + {
> + xtype: 'displayfield',
> + name: 'crypt-key-fp',
> + fieldLabel: gettext('Key Source'),
> + padding: '2 0',
> + },

Is this set anywhere? I could not find a matching field name in the API
response nor this getting manually set. In fact, `rg -i 'crypt.?key.?fp'` shows
only this lone instance here.