Re: [PATCH proxmox-backup 3/7] api: add /nodes/localhost/instance-id

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

On 4/15/26 11:20 AM, Lukas Wagner wrote:
> Thanks for the feedback, Chris!
> 
> 
> On Tue Apr 14, 2026 at 4:23 PM CEST, Christian Ebner wrote:
>> On 4/13/26 2:11 PM, Lukas Wagner wrote:
>>>    }
>>>    
>>> +///  Static key used for deriving a unique instance id from `/etc/machine-id` using
>>> +///  [`proxmox_systemd::sd_id128::get_app_specific_id`].
>>> +///
>>> +///  This key was generated `systemd-id128 new`.
>>
>> nit: ... generated by `systemd-id128 new`
> 
> Fixed for v2, thanks :)
> 
>>
>>> +///
>>> +const INSTANCE_ID_KEY: [u8; 16] = 0xe5415d9999c146399b8bdae1260e19d2_u128.to_le_bytes();
>>> +
>>> +#[api(
>>> +    protected: false,
>>
>> nit: no need to set this explicitly.
> 
> ack!
> 
>>
>>> +    input: {
>>> +        properties: {
>>> +            node: {
>>> +                schema: NODE_SCHEMA,
>>> +                optional: true,
>>> +            },
>>> +        }
>>> +    },
>>> +    returns: {
>>> +        type: InstanceId,
>>> +    },
>>> +    access: {
>>> +        permission: &Permission::Anybody,
>>> +    }
>>> +)]
>>> +/// Returns a unique ID for this PBS node. The ID is derived from `/etc/machine-id`.
>>> +pub fn get_instance_id() -> Result<InstanceId, Error> {
>>> +    let machine_id_derived_app_id =
>>> +        proxmox_systemd::sd_id128::get_app_specific_id(INSTANCE_ID_KEY)?;
>>> +    let machine_id_derived_app_id = hex::encode(machine_id_derived_app_id);
>>> +
>>> +    Ok(InstanceId {
>>> +        instance_id: machine_id_derived_app_id,
>>
>> nit: Could make sense to derive `From` for this on the `InstanceId` itself?
>>
>> But do not want to pollute the api-types with dependencies on
>> proxmox-systemd, so this would only cover the plain id to string
>> conversion... so not sure if worth it.
>>
> 
> 
> I created a struct for this response instead of just returning a string
> so that it's extensible in the future, in case we want to return more
> information. With that in mind, the `From` impl does not make that much
> sense to me, if I'm honest.
> 
> I think I'd prefer this as is, if that is alright with you? I don't
> think the `From` helps much when it comes to maintainability nor
> readability here.

Yes, fine by me. No hard feelings was just wondering if we do want to 
protect from creating a unexpected instance id from a plain string.