Re: [pdm-devel] [PATCH datacenter-manager 0/3] token secret shadow config

["Lukas Wagner" <l.wagner@proxmox.com>]

On Mon Dec 1, 2025 at 10:29 AM CET, Fabian Grünbichler wrote:
> this patch series splits out token secrets into their own file,
> replacing them with a placeholder value in the regular remotes.cfg
>
> Fabian Grünbichler (3):
>   remote config: let save_config take ownership
>   remote config: get token secret from shadow file if shadowed
>   remote config: shadow token secrets when saving
>
>  lib/pdm-api-types/src/remotes.rs | 50 +++++++++++++++++++++
>  lib/pdm-config/src/remotes.rs    | 76 +++++++++++++++++++++++++++++---
>  server/src/api/pve/lxc.rs        |  2 +-
>  server/src/api/pve/qemu.rs       |  2 +-
>  server/src/api/remotes.rs        |  6 +--
>  server/src/connection.rs         |  8 +++-
>  6 files changed, 130 insertions(+), 14 deletions(-)

In general, this looks good to me.

Two things came to mind when testing/reviewing this:
  - as noted in the individual patch, the token should be removed when
    deleting a remote

  - I'm not sure it's super elegant to have `token -` even for *new*
    remotes. I think it would be much cleaner to make the token field
    optional, but I can see how that would be quite a bit more work
    (could entail having to use distinct types for the API, the config
    file and internal usage) so I guess it can stay as is for now. Maybe
    we can see how we could achieve this in a follow-up. Worst case, we
    drop the token parameter in the next major release.

With this in mind,

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel