public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Thomas Lamprecht" <t.lamprecht@proxmox.com>,
	<pdm-devel@lists.proxmox.com>
Subject: Re: [PATCH datacenter-manager v3 04/12] subscription: api: add key pool and node status endpoints
Date: Mon, 18 May 2026 16:27:15 +0200	[thread overview]
Message-ID: <DILVFKDDW5LS.100FZDO3ITSY6@proxmox.com> (raw)
In-Reply-To: <20260515074623.766766-5-t.lamprecht@proxmox.com>

On Fri May 15, 2026 at 9:43 AM CEST, Thomas Lamprecht wrote:
> +/// Compute a proposed mapping of unused pool keys to nodes without an active subscription.
> +///
> +/// Returns the plan plus snapshots of the inputs (pool digest and a hash of the consulted
> +/// node-status). The plan is committed by `bulk_assign` and rejected there if either snapshot no
> +/// longer matches the live state, so an operator never silently applies a plan that drifted
> +/// between preview and commit.
> +///
> +/// `PRIV_SYS_MODIFY` is required to *preview* the plan; the actual commit performed by
> +/// `bulk_assign` additionally drops proposals on any remote the caller cannot
> +/// `PRIV_RESOURCE_MODIFY`, so an audit-only-on-a-remote operator can see the suggestion but the
> +/// write never lands there.
> +async fn auto_assign(rpcenv: &mut dyn RpcEnvironment) -> Result<AutoAssignProposal, Error> {
> +    let node_statuses = collect_node_status(FRESH_NODE_STATUS_MAX_AGE, rpcenv).await?;
> +    let (config, keys_digest) = pdm_config::subscriptions::config()?;
> +    let assignments = compute_proposals(&config, &node_statuses);
> +    Ok(AutoAssignProposal {
> +        assignments,
> +        keys_digest,
> +        node_status_digest: hash_node_status(&node_statuses),
> +    })
> +}
> +
> +#[api(
> +    input: {
> +        properties: {
> +            proposal: { type: AutoAssignProposal },
> +        },

Brief comment, since I spotted this in the API viewer:

We should probably document that this API endpoint only works when
submitting the payload as `application/json`.

Our `application/x-www-form-urlencoded` code paths do not support
submitting complex data structures, as far as I know we only support
flat key-value pairs and basic arrays.

This is a limitation that we really should lift at some point, either by
adding support for parsing urlencoded nested objects, or by phasing it
out completely, only allowing JSON. As far as I know it is relatively
rare to support both, so I can sympathize with dropping support at some
point, but I'm well aware that this is a huge breaking change for API
users, which would need to be done very carefully after a long
deprecation period.

Parsing complex data structures from urlencoded bodies is IIRC a bit
awkward, since one has to flatten everything into key-value pairs, for
which there is no clear standard/convention (different frameworks use
different approaches -- so this is also not *that* nice for API users).

At some point before the PDM release we briefly talked about (can't
remember if you were part of the discussion) not supporting the
x-www-form-urlencoded API at all in PDM in the first place, and IIRC
there was a consensus that this could be a good idea, but unfortunately
it didn't really go anywhere. Maybe a topic for PDM 2.0?


> +    },
> +    returns: {
> +        type: Array,
> +        description: "Assignments that were actually persisted.",
> +        items: { type: ProposedAssignment },
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["system"], PRIV_SYS_MODIFY, false),
> +    },
> +)]
> +/// Apply a proposal previously returned by `auto_assign`.
> +///
> +/// Rejects with 409 if the pool config digest has moved or the live node-status hash differs
> +/// from what the proposal was computed against; the caller is expected to refresh the proposal
> +/// and retry. Per-remote `PRIV_RESOURCE_MODIFY` is checked inside the handler so an audit-only
> +/// caller's previously-rendered preview cannot be applied on their behalf.
> +async fn bulk_assign(
> +    proposal: AutoAssignProposal,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Vec<ProposedAssignment>, Error> {
> +    let auth_id: Authid = rpcenv
> +        .get_auth_id()
> +        .context("no authid available")?
> +        .parse()?;
> +




  parent reply	other threads:[~2026-05-18 14:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  7:43 [PATCH datacenter-manager v3 00/12] subscription key pool registry Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 01/12] api types: subscription level: render full names Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 02/12] pdm-client: add wait_for_local_task helper Thomas Lamprecht
2026-05-15 15:21   ` Wolfgang Bumiller
2026-05-15  7:43 ` [PATCH datacenter-manager v3 03/12] subscription: pool: add data model and config layer Thomas Lamprecht
2026-05-15 15:21   ` Wolfgang Bumiller
2026-05-15  7:43 ` [PATCH datacenter-manager v3 04/12] subscription: api: add key pool and node status endpoints Thomas Lamprecht
2026-05-15 15:21   ` Wolfgang Bumiller
2026-05-18 14:27   ` Lukas Wagner [this message]
2026-05-15  7:43 ` [PATCH datacenter-manager v3 05/12] ui: registry: add view with key pool and node status Thomas Lamprecht
2026-05-19 15:08   ` Wolfgang Bumiller
2026-05-15  7:43 ` [PATCH datacenter-manager v3 06/12] cli: client: add subscription key pool management subcommands Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 07/12] docs: add subscription registry chapter Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 08/12] subscription: add Clear Key action and per-node revert Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 09/12] subscription: add Adopt Key action for foreign live subscriptions Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 10/12] subscription: add Adopt All bulk action Thomas Lamprecht
2026-05-15  7:43 ` [PATCH datacenter-manager v3 11/12] subscription: add Check Subscription action Thomas Lamprecht
2026-05-15  7:43 ` [RFC PATCH datacenter-manager v3 12/12] ui: registry: add Add-and-Assign wizard from Assign Key dialog Thomas Lamprecht
2026-05-18 13:41   ` Lukas Wagner
2026-05-18 13:23 ` [PATCH datacenter-manager v3 00/12] subscription key pool registry Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DILVFKDDW5LS.100FZDO3ITSY6@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal