[pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard

[Lukas Wagner]

On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
> # Summary
>
> This series improves the remote wizard in various points:
> * improved wording and texts
> * moved seperate buttons on pages into the next button
> * probing of entered nodes
> * confirmation dialog for missing fingerprints
> * better realm selector
>
> # Notes
>
> I'm not super sure about the API call structure. I think two seperate
> API calls (one for tls probing, one for scanning) might make more sense.
> If that's wanted, I'll do so in a v2 of the series.

Yeah, I think two separate API endpoints make a lot of sense, the `scan`
endpoint is a bit weird right now.

>
> ## Probing on the server side
>
> I tried to determine if we need a fingerprint for nodes inside the
> API call, by probing each node. Since we currently only get the
> nodenames and not FQDNs in the nodelist, this will currently not
> result in a valid connection in most cases and return the fingerprint.
>
> My plan would be to include FQDNs of the nodes on the PVE side API call,
> so we can return here a list of nodename, ip and FQDNs, which the user
> then can select from. (which of those I'd probe on first check is yet
> to be determined)
>
> ## Fingerprint confirmation dialogs
>
> Not sure if we want to be able to let the user confirm the fingerprint
> so easily. On one hand it's very convenient, but maybe leads to users
> simply clicking yes without understanding what's happening.
>
> If it's deemed too dangerous, I'd rework the series without this.

Good question. IMO something like this can be okay for a bit more
convenience, but maybe some other people have an opinion on this as
well.

One thing I'm not sure about is the fact that we spawn a dialog from a
dialog here, which (I think) is a first for us. 


Some other minor things I've noticed while testing this:

- Entering the remote name on the second stage of the wizard seemed a
  bit odd to me (can't really explain why, just a gut feeling I had when
  the second stage was shown). But I guess it could make sense in case
  we want to autofill the clustername there.

- When you go from "Summary" from "Probe Remote" and then to "Settings"
  (all via clicking the tab bar), the 'Trust fingerprint' dialog is
  shown again. Maybe we can detect that the IP/host has not been changed
  and/or cache which fingerprints we already marked as trusted?

- 'Realm' field should be greyed out when "Use existing Token" has been
  selected

All just minor things, all it all these changes looked quite solid to
me!

>
> # Future work
>
> The next step for the wizard is to have some kind of quick copy&paste
> info. After discussing off-list with Fabian a bit, I think it would be
> best for this to contain the hostname (FQDN?) + fingerprint (if just a
> self-signed certificate) + a list of nodes with their respective
> nodename + FQDNs (maybe requires api change on PVE side to generate
> this). The user would then still have to do most of the steps currently
> necessary in the wizard, except the manual copy & pasting of
> fingerprints and maybe entering of FQDNs.
>
>
> Dominik Csapak (21):
>   server/ui: pve: change 'realm list' api call to GET
>   api types: RemoteType: put default port info to the type
>   server: connection: add probe_tls_connection helper
>   server/ui: pve api: extend 'scan' so it can probe the tls connection
>   pdm-client: add scan_remote and probe_tls methods
>   ui: remotes: node url list: add placeholder and clear trigger
>   ui: rmeotes: node url list: make column header clearer
>   ui: remotes: node url list: handle changing default
>   ui: pve wizard: rename 'realm' variable to 'info'
>   ui: pve wizard: summary: add default text for fingerprint
>   ui: pve wizard: nodes: improve info text
>   ui: pve wizard: nodes: probe hosts to verify fingerprint settings
>   ui: pve wizard: info: use pdm_client for scanning
>   ui: pve wizard: info: detect hostname and fingerprint
>   ui: pve wizard: info: remove manual scan button
>   ui: widget: add pve realm selector
>   ui: pve wizard: info: use pve realm selector
>   ui: pve wizard: connect: factor out normalize_hostname
>   ui: pve wizard: connect: move connection logic to next button
>   ui: pve wizard: connect: use scan api endpoint instead of realms
>   ui: pve wizard: connect: add certificate confirmation dialog
>
>  lib/pdm-api-types/Cargo.toml          |   1 +
>  lib/pdm-api-types/src/lib.rs          |   2 +
>  lib/pdm-api-types/src/remotes.rs      |  16 ++
>  lib/pdm-client/src/lib.rs             |  45 ++++
>  server/src/api/pve/mod.rs             |  62 ++++--
>  server/src/connection.rs              |  87 +++++++-
>  ui/Cargo.toml                         |   1 +
>  ui/src/remotes/add_wizard.rs          |   8 +-
>  ui/src/remotes/node_url_list.rs       |  33 ++-
>  ui/src/remotes/wizard_page_connect.rs | 308 ++++++++++++++++----------
>  ui/src/remotes/wizard_page_info.rs    | 127 ++++++-----
>  ui/src/remotes/wizard_page_nodes.rs   | 241 +++++++++++++++++++-
>  ui/src/remotes/wizard_page_summary.rs |   5 +-
>  ui/src/widget/mod.rs                  |   3 +
>  ui/src/widget/pve_realm_selector.rs   | 125 +++++++++++
>  15 files changed, 853 insertions(+), 211 deletions(-)
>  create mode 100644 ui/src/widget/pve_realm_selector.rs