From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id DB2BE1FF146 for ; Tue, 12 May 2026 11:51:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7A5EFC673; Tue, 12 May 2026 11:51:15 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 12 May 2026 11:51:06 +0200 Message-Id: Subject: Re: [PATCH datacenter-manager v2 3/8] subscription: add key pool data model and config layer From: "Lukas Wagner" To: "Thomas Lamprecht" , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260507082943.2749725-1-t.lamprecht@proxmox.com> <20260507082943.2749725-4-t.lamprecht@proxmox.com> In-Reply-To: <20260507082943.2749725-4-t.lamprecht@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778579354412 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [subscriptions.rs,lib.rs] Message-ID-Hash: X27UWY6MZ6OTTGZUMYKM4TPKTANTGO57 X-Message-ID-Hash: X27UWY6MZ6OTTGZUMYKM4TPKTANTGO57 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: Looking good mostly, some notes inline. Inline comments are mostly cosmetic, so in any way: Reviewed-by: Lukas Wagner On Thu May 7, 2026 at 10:26 AM CEST, Thomas Lamprecht wrote: [...] > +#[api( > + properties: { > + "key": { schema: SUBSCRIPTION_KEY_SCHEMA }, > + "level": { optional: true }, > + "status": { optional: true }, > + "source": { optional: true }, > + }, > +)] > +#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)] > +#[serde(rename_all =3D "kebab-case")] > +/// A subscription key entry in the PDM key pool. > +pub struct SubscriptionKeyEntry { > + /// The subscription key (for example, pve4b-1234567890). > + pub key: String, > + > + /// Product type derived from the key prefix. > + #[serde(rename =3D "product-type")] > + pub product_type: ProductType, > + > + /// Subscription level, derived from the key suffix. > + #[serde(default)] > + pub level: SubscriptionLevel, the subscription level is serialized uppercase, e.g. "Community" -- is this intentional? Not a big deal IMO, just wanted to make sure if this was on purpose or not. > + > + /// Where the key entry came from. Defaults to manual entry. > + #[serde(default)] > + pub source: SubscriptionKeySource, > + > + /// Remote this key is assigned to (if any). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub remote: Option, > + > + /// Node within the remote this key is assigned to (if any). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub node: Option, > + > + /// Server ID this key is bound to (from signed info, if available). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub serverid: Option, Same note as at (1) with regards to hyphenation, but can also stay as is to have consistency with the API in PVE and PBS... > + > + /// Subscription status from last check. > + #[serde(default)] > + pub status: SubscriptionStatus, > + > + /// Next due date. > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub nextduedate: Option, (1) Personally i'm not a big fan of these word amalgamations, I think I'd prefer 'next_due_date' (serialized as 'next-due-date'). That being said, it might still be fine to keep as is, since that is the exact spelling used in the PVE/PBS API ... so no hard feelings from my side. > + > + /// Product name. > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub productname: Option, Same as (1). Additionally, we have `product-type` above, so I think `product-name` would make the config make self-consistent. > + > + /// Epoch of last import or refresh of this key's data. > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub checktime: Option, > +} > + > +impl ApiSectionDataEntry for SubscriptionKeyEntry { > + const INTERNALLY_TAGGED: Option<&'static str> =3D Some("product-type= "); > + const SECION_CONFIG_USES_TYPE_KEY: bool =3D true; > + > + fn section_config() -> &'static SectionConfig { > + static CONFIG: OnceLock =3D OnceLock::new(); > + > + CONFIG.get_or_init(|| { > + let mut this =3D > + SectionConfig::new(&SUBSCRIPTION_KEY_SCHEMA).with_type_k= ey("product-type"); > + for ty in [ > + ProductType::Pve, > + ProductType::Pbs, > + ProductType::Pmg, > + ProductType::Pom, > + ] { > + this.register_plugin(SectionConfigPlugin::new( > + ty.as_section_type().to_string(), > + Some("key".to_string()), > + SubscriptionKeyEntry::API_SCHEMA.unwrap_object_schem= a(), > + )); > + } > + this > + }) > + } > + > + fn section_type(&self) -> &'static str { > + self.product_type.as_section_type() > + } > +} > + > +#[api( > + properties: { > + "key": { schema: SUBSCRIPTION_KEY_SCHEMA }, > + }, > +)] > +#[derive(Clone, Debug, Default, Deserialize, Serialize)] > +#[serde(rename_all =3D "kebab-case")] > +/// Shadow entry storing the signed subscription info blob for a key. > +/// > +/// Currently only populated by the future shop-bundle import flow; manu= ally-added keys leave > +/// this table empty. The data layer is in place so that adding the impo= rt path later does not > +/// require reshaping the on-disk config. > +pub struct SubscriptionKeyShadow { > + /// The subscription key. > + pub key: String, > + > + /// Product type (section type marker). > + #[serde(rename =3D "product-type")] > + pub product_type: ProductType, > + > + /// Base64-encoded signed SubscriptionInfo JSON. > + #[serde(default)] > + pub info: String, > +} > + > +impl ApiSectionDataEntry for SubscriptionKeyShadow { > + const INTERNALLY_TAGGED: Option<&'static str> =3D Some("product-type= "); > + const SECION_CONFIG_USES_TYPE_KEY: bool =3D true; > + > + fn section_config() -> &'static SectionConfig { > + static CONFIG: OnceLock =3D OnceLock::new(); > + > + CONFIG.get_or_init(|| { > + let mut this =3D > + SectionConfig::new(&SUBSCRIPTION_KEY_SCHEMA).with_type_k= ey("product-type"); > + for ty in [ > + ProductType::Pve, > + ProductType::Pbs, > + ProductType::Pmg, > + ProductType::Pom, > + ] { > + this.register_plugin(SectionConfigPlugin::new( > + ty.as_section_type().to_string(), > + Some("key".to_string()), > + SubscriptionKeyShadow::API_SCHEMA.unwrap_object_sche= ma(), > + )); > + } > + this > + }) > + } > + > + fn section_type(&self) -> &'static str { > + self.product_type.as_section_type() > + } > +} > + > +/// Decode a base64-encoded `SubscriptionInfo` JSON blob from the shadow= file. > +/// > +/// Forward-compat helper for the future shop-bundle import path. Return= s the parsed > +/// `SubscriptionInfo`; the caller is responsible for verifying the sign= ature against the shop's > +/// signing key. > +pub fn parse_signed_info_blob(b64: &str) -> Result { > + let bytes =3D proxmox_base64::decode(b64)?; > + let info =3D serde_json::from_slice(&bytes)?; > + Ok(info) > +} > + > +/// Cross-check the `serverid` of a shadowed entry against what the remo= te reports. > +/// > +/// Forward-compat helper for the future bundle-import and push flow: wh= en the shadow has a > +/// signed serverid binding, the operator should be warned if the remote= it is being pushed to > +/// has a different hardware id. Returns Ok(None) when there is nothing = to compare. > +pub fn verify_serverid( > + entry: &SubscriptionKeyEntry, > + remote_info: &SubscriptionInfo, > +) -> Result, Error> { > + let Some(expected) =3D entry.serverid.as_deref() else { > + return Ok(None); > + }; > + let Some(actual) =3D remote_info.serverid.as_deref() else { > + return Ok(None); > + }; > + if expected =3D=3D actual { > + Ok(None) > + } else { > + Ok(Some(ServeridMismatch { > + key: entry.key.clone(), > + expected: expected.to_string(), > + actual: actual.to_string(), > + })) > + } > +} > + > +/// Result of [`verify_serverid`] when the bound and observed server-ids= disagree. > +#[derive(Clone, Debug, PartialEq, Eq)] > +pub struct ServeridMismatch { > + pub key: String, > + pub expected: String, > + pub actual: String, > +} This could use some docstrings for the struct members, as to make it clearer what `expected` and `actual` acutally refer to. Alternatively or additionally, it could be {actual,expected}_server_id > + > +#[api] > +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] > +#[serde(rename_all =3D "kebab-case")] > +/// Subscription status of a single remote node, combining remote query = data with key pool > +/// assignment information. > +pub struct RemoteNodeStatus { > + /// Remote name. > + pub remote: String, > + /// Remote type (pve or pbs). > + #[serde(rename =3D "type")] > + pub ty: RemoteType, > + /// Node name. > + pub node: String, > + /// Number of CPU sockets (PVE only). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub sockets: Option, > + /// Current subscription status. > + #[serde(default)] > + pub status: SubscriptionStatus, > + /// Subscription level. > + #[serde(default)] > + pub level: SubscriptionLevel, > + /// Currently assigned key from the pool (if any). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub assigned_key: Option, > + /// Current key on the node (from remote query). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub current_key: Option, > +} > + > +#[api] > +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] > +#[serde(rename_all =3D "kebab-case")] > +/// A proposed key-to-node assignment from the auto-assign algorithm. > +pub struct ProposedAssignment { > + /// The subscription key to assign. > + pub key: String, > + /// Target remote. > + pub remote: String, > + /// Target node. > + pub node: String, > + /// Socket count of the key (PVE only). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub key_sockets: Option, > + /// Socket count of the node (PVE only). > + #[serde(skip_serializing_if =3D "Option::is_none")] > + pub node_sockets: Option, > +} [...] > diff --git a/lib/pdm-config/src/lib.rs b/lib/pdm-config/src/lib.rs > index 5b9bcca..46ad1a2 100644 > --- a/lib/pdm-config/src/lib.rs > +++ b/lib/pdm-config/src/lib.rs > @@ -8,6 +8,7 @@ pub mod domains; > pub mod node; > pub mod remotes; > pub mod setup; > +pub mod subscriptions; > pub mod views; > =20 > mod config_version_cache; > diff --git a/lib/pdm-config/src/subscriptions.rs b/lib/pdm-config/src/sub= scriptions.rs Doc-Strings for `pub` fn's in this file are missing. No big deal here since the usage is clear enough from the signature alone, but still mentioning it for completeness' sake. > new file mode 100644 > index 0000000..7e930ba > --- /dev/null > +++ b/lib/pdm-config/src/subscriptions.rs > @@ -0,0 +1,102 @@ > +//! Read/write subscription key pool configuration. > +//! > +//! Call [`init`] to inject a concrete `SubscriptionKeyConfig` instance = before using the > +//! module-level functions. > +//! > +//! The shadow-config functions stash signed `SubscriptionInfo` blobs al= ongside the plain key > +//! entries, which is intended as future proofing for a more automated (= shop) import without having > +//! to adapt the data layer. > + > +use std::sync::OnceLock; > + > +use anyhow::Error; > + > +use proxmox_config_digest::ConfigDigest; > +use proxmox_product_config::{open_api_lockfile, replace_config, ApiLockG= uard}; > +use proxmox_section_config::typed::{ApiSectionDataEntry, SectionConfigDa= ta}; > + > +use pdm_api_types::subscription::{SubscriptionKeyEntry, SubscriptionKeyS= hadow}; > +use pdm_buildcfg::configdir; > + > +pub const SUBSCRIPTIONS_CFG_FILENAME: &str =3D configdir!("/subscription= s.cfg"); > +const SUBSCRIPTIONS_SHADOW_FILENAME: &str =3D configdir!("/subscriptions= .shadow"); > +pub const SUBSCRIPTIONS_CFG_LOCKFILE: &str =3D configdir!("/.subscriptio= ns.lock"); > + > +static INSTANCE: OnceLock> = =3D OnceLock::new(); > + > +fn instance() -> &'static (dyn SubscriptionKeyConfig + Send + Sync) { > + INSTANCE > + .get() > + .expect("subscription key config not initialized") > + .as_ref() > +} > + > +pub fn lock_config() -> Result { > + instance().lock_config() > +} > + > +pub fn config() -> Result<(SectionConfigData, Conf= igDigest), Error> { > + instance().config() > +} > + > +pub fn shadow_config() -> Result, Error> { > + instance().shadow_config() > +} > + > +pub fn save_config(config: &SectionConfigData) -> = Result<(), Error> { > + instance().save_config(config) > +} > + > +pub fn save_shadow(shadow: &SectionConfigData) ->= Result<(), Error> { > + instance().save_shadow(shadow) > +} > + > +pub trait SubscriptionKeyConfig { > + fn config(&self) -> Result<(SectionConfigData,= ConfigDigest), Error>; > + fn shadow_config(&self) -> Result, Error>; > + fn lock_config(&self) -> Result; > + fn save_config(&self, config: &SectionConfigData) -> Result<(), Error>; > + fn save_shadow(&self, shadow: &SectionConfigData) -> Result<(), Error>; > +} > + > +pub struct DefaultSubscriptionKeyConfig; > + > +impl SubscriptionKeyConfig for DefaultSubscriptionKeyConfig { > + fn lock_config(&self) -> Result { > + open_api_lockfile(SUBSCRIPTIONS_CFG_LOCKFILE, None, true) > + } > + > + fn config(&self) -> Result<(SectionConfigData,= ConfigDigest), Error> { > + let content =3D proxmox_sys::fs::file_read_optional_string(SUBSC= RIPTIONS_CFG_FILENAME)? > + .unwrap_or_default(); > + > + let digest =3D openssl::sha::sha256(content.as_bytes()); > + let data =3D > + SubscriptionKeyEntry::parse_section_config(SUBSCRIPTIONS_CFG= _FILENAME, &content)?; > + > + Ok((data, digest.into())) > + } > + > + fn shadow_config(&self) -> Result, Error> { > + let content =3D proxmox_sys::fs::file_read_optional_string(SUBSC= RIPTIONS_SHADOW_FILENAME)? > + .unwrap_or_default(); > + SubscriptionKeyShadow::parse_section_config(SUBSCRIPTIONS_SHADOW= _FILENAME, &content) > + } > + > + fn save_config(&self, config: &SectionConfigData) -> Result<(), Error> { > + let raw =3D SubscriptionKeyEntry::write_section_config(SUBSCRIPT= IONS_CFG_FILENAME, config)?; > + replace_config(SUBSCRIPTIONS_CFG_FILENAME, raw.as_bytes()) > + } > + > + fn save_shadow(&self, shadow: &SectionConfigData) -> Result<(), Error> { > + let raw =3D > + SubscriptionKeyShadow::write_section_config(SUBSCRIPTIONS_SH= ADOW_FILENAME, shadow)?; > + replace_config(SUBSCRIPTIONS_SHADOW_FILENAME, raw.as_bytes()) > + } > +} > + > +pub fn init(instance: Box) { > + if INSTANCE.set(instance).is_err() { > + panic!("subscription key config instance already set"); > + } > +}