Re: [PATCH proxmox-backup v2 12/27] api: config: allow encryption key manipulation for sync job

[Christian Ebner <c.ebner@proxmox.com>]

On 4/11/26 10:50 AM, Thomas Lamprecht wrote:
> Am 10.04.26 um 18:54 schrieb Christian Ebner:
>> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
>> @@ -490,6 +511,16 @@ pub fn update_sync_job(
>>
>> +                DeletableProperty::ActiveEncryptionKey => {
>> +                    allow_remove_associated = data.active_encryption_key.is_none();
>> +                    keep_previous_key_as_associated(&data, &mut update.associated_key);
>> +                    data.active_encryption_key = None;
>> +                }
>> +                DeletableProperty::AssociatedKey => {
>> +                    if allow_remove_associated {
>> +                        data.associated_key = None;
>> +                    }
>> +                }
> 
> nit: the silent drop of AssociatedKey deletion when ActiveEncryptionKey is also
> being deleted in the same request could trip people up. At least worth a code
> comment explaining the rationale.
> 
> btw/nit: nice idea that the previous active key gets auto-promoted to
> associated on rotation, but the user receives zero feedback about that - maybe
> log it so users can more easily see what happened.

Actually, after revisiting this I think one can do better here: Instead 
of not dropping the associated keys as requested, drop them and only 
re-add the previous active encryption key, which can temporarily be 
store instead of the allow_remove_associated flag, therefore also being 
independent from the loop order. So it can simply be associated below, 
if required.