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 33FB21FF136 for ; Mon, 18 May 2026 15:23:41 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9B4E112A0E; Mon, 18 May 2026 15:23:40 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 18 May 2026 15:23:35 +0200 Message-Id: Subject: Re: [PATCH datacenter-manager v3 00/12] subscription key pool registry From: "Lukas Wagner" To: "Thomas Lamprecht" , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260515074623.766766-1-t.lamprecht@proxmox.com> In-Reply-To: <20260515074623.766766-1-t.lamprecht@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1779110603571 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.054 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: IW7QNQQ4WSNCFBTU4PQ234FKL4USRTRJ X-Message-ID-Hash: IW7QNQQ4WSNCFBTU4PQ234FKL4USRTRJ X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri May 15, 2026 at 9:43 AM CEST, Thomas Lamprecht wrote: > v3 of the Subscription Registry - many thanks @Lukas for the review! > The notable shape change vs v2: the single Reissue Key patch is split > into the four discrete actions PDM can drive today (Clear Key, Adopt > Key, Adopt All, Check Subscription); the on-disk layout split moves to > its own commit; each user-visible action ships with its api / cli / ui / > docs change so a reviewer reads one patch end-to-end. > > Check Subscription uses the canonical UpdateSubscription from the > recently uploaded proxmox-subscription 1.0.2; the matching PBS-side > adoption is posted as a separate patch on proxmox-backup. > > Notable v2 -> v3: > * Reissue Key renamed to Clear Key (v2 reissue did not actually round- > trip through the shop); "Reissue" stays reserved for the future > shop-side action. > * Clear Pending renamed to Discard Pending; now also cancels queued > clears, not just pending pushes. > * New Adopt Key / Adopt All paths import a foreign live key into the > pool without touching the remote, for fleet onboarding. > * New Check Subscription action drives update_subscription(force=3Dtrue) > and invalidates the PDM cache, so a stale Invalid / Expired verdict > can be promoted without waiting for the periodic check. > * Pool grid columns are sortable; new Source column (hidden by > default) distinguishes manually-added from adopted entries. > * ESC dismisses every confirmation dialog on the registry view. > * Invalid keys land with a clear error instead of staying queued with > a misleading pending badge. > * Per-node Revert can drop a single queued clear without the global > Discard Pending. > > Open follow-ups, not in this series: > * Shop-side full reissue, so PDM can drive the actual key rotation > rather than just Clear Key on its side. > * Atomic clear-and-assign so swapping a key on a node gets reduced from > doing four (Clear / Apply / Assign / Apply) steps to one queued > change (+ apply). > * Shop-bundle import path; the on-disk shadow file plumbing already > accommodates the signed SubscriptionInfo blob. > * Per-row Auto-Assign overrides for pinning a specific key to a node. > * Status column filter on the node-status tree. > > The trailing wizard commit (v3-0012) is sent as RFC and should be > probably skipped; see its diffstat note for details. > Gave this another go on the latest master. I did not dig through the code yet, but focused on the UI and UX for now. I'll try to at least skim over the code again, probably tomorrow. Looks great so far, only found very minor things that could be improved in the UI: - "Assign Key to Remote" dialog - Maybe hide remotes from the dropdown where all nodes already have a subscription? - "Node" dropdown has a "Memory" column -> does not seem useful in this context - "Assign Key to ...", as triggered from the right-side panel has some quite weird padding on the bottom - "Add Subscription Keys" dialog: The text on the bottom ("One key per line= ...") does not have enough contrast to be clearly legible (in both light and dark mode) - In the "Auto-Assign Proposal" dialog, the "Key" column does seem to use a slightly smaller font, 12px instead of 13px -- this looks a bit odd But those are just very minor complaints which could also be fixed as follow-ups. Tested-by: Lukas Wagner