From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 76E8C1FF16F for ; Tue, 19 Aug 2025 14:08:55 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D474A1374F; Tue, 19 Aug 2025 14:10:39 +0200 (CEST) Mime-Version: 1.0 Date: Tue, 19 Aug 2025 14:10:35 +0200 Message-Id: To: "Proxmox Datacenter Manager development discussion" , "Dominik Csapak" From: "Lukas Wagner" X-Mailer: aerc 0.20.1-0-g2ecb8770224a References: <20250516133611.3499075-1-d.csapak@proxmox.com> In-Reply-To: <20250516133611.3499075-1-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1755605393206 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.022 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [remotes.rs, mod.rs, connection.rs, lib.rs] Subject: Re: [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel