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 5EB501FF13B for ; Tue, 27 Jan 2026 10:16:42 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 404441D7A0; Tue, 27 Jan 2026 10:17:06 +0100 (CET) Date: Tue, 27 Jan 2026 10:17:01 +0100 Message-Id: From: "Lukas Wagner" To: "Proxmox Datacenter Manager development discussion" , "Gabriel Goller" , "Shan Shaji" Mime-Version: 1.0 X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260123172910.244121-1-s.shaji@proxmox.com> <20260123172910.244121-3-s.shaji@proxmox.com> In-Reply-To: <20260123172910.244121-3-s.shaji@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1769505357424 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.036 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 Subject: Re: [pdm-devel] [PATCH datacenter-manager 2/3] fix #7179: cli: admin: expose acme commands 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" Hi Shan, thanks for the patch! Some notes inline. On Fri Jan 23, 2026 at 6:29 PM CET, Shan Shaji wrote: > Previously, ACME commands were not exposed through the admin CLI. > Added the necessary functionality to manage ACME settings directly > via the command line. > > Signed-off-by: Shan Shaji > --- > cli/admin/Cargo.toml | 4 +- > cli/admin/src/acme.rs | 442 ++++++++++++++++++++++++++++++++++++++++++ > cli/admin/src/main.rs | 5 + > 3 files changed, 450 insertions(+), 1 deletion(-) > create mode 100644 cli/admin/src/acme.rs > > diff --git a/cli/admin/Cargo.toml b/cli/admin/Cargo.toml > index e566b39..01afc88 100644 > --- a/cli/admin/Cargo.toml > +++ b/cli/admin/Cargo.toml > @@ -22,7 +22,9 @@ proxmox-access-control.workspace = true > proxmox-rest-server.workspace = true > proxmox-sys.workspace = true > proxmox-daemon.workspace = true > - > +proxmox-acme.workspace = true > +proxmox-acme-api.workspace = true > +proxmox-base64.workspace = true > pdm-api-types.workspace = true > pdm-config.workspace = true > pdm-buildcfg.workspace = true > diff --git a/cli/admin/src/acme.rs b/cli/admin/src/acme.rs > new file mode 100644 > index 0000000..81c63f9 > --- /dev/null > +++ b/cli/admin/src/acme.rs > @@ -0,0 +1,442 @@ > +use std::io::Write; > + > +use anyhow::{bail, Error}; > +use serde_json::Value; > + > +use proxmox_acme::async_client::AcmeClient; > +use proxmox_acme_api::{completion::*, AcmeAccountName, DnsPluginCore, KNOWN_ACME_DIRECTORIES}; > +use proxmox_rest_server::wait_for_local_worker; > +use proxmox_router::{cli::*, ApiHandler, RpcEnvironment}; > +use proxmox_schema::api; > +use proxmox_sys::fs::file_get_contents; > + > +use server::api as dc_api; > + > +pub fn acme_mgmt_cli() -> CommandLineInterface { > + let cmd_def = CliCommandMap::new() > + .insert("account", account_cli()) > + .insert("plugin", plugin_cli()) > + .insert("certificate", cert_cli()); > + > + cmd_def.into() > +} > + > +#[api( > + input: { > + properties: { > + "output-format": { > + schema: OUTPUT_FORMAT, > + optional: true, > + } > + } > + } > +)] > +/// List ACME accounts. > +fn list_accounts(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> { > + let output_format = get_output_format(¶m); > + let info = &dc_api::config::acme::API_METHOD_LIST_ACCOUNTS; > + let mut data = match info.handler { > + ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, > + _ => unreachable!(), > + }; > + > + let options = default_table_format_options(); > + format_and_print_result_full(&mut data, &info.returns, &output_format, &options); > + > + Ok(()) > +} > + > +#[api( > + input: { > + properties: { > + name: { type: AcmeAccountName }, > + "output-format": { > + schema: OUTPUT_FORMAT, > + optional: true, > + }, > + } > + } > +)] > +/// Show ACME account information. > +async fn get_account(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> { > + let output_format = get_output_format(¶m); > + > + let info = &dc_api::config::acme::API_METHOD_GET_ACCOUNT; > + let mut data = match info.handler { > + ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?, > + _ => unreachable!(), > + }; > + > + let options = default_table_format_options() > + .column( > + ColumnConfig::new("account") > + .renderer(|value, _record| Ok(serde_json::to_string_pretty(value)?)), > + ) > + .column(ColumnConfig::new("directory")) > + .column(ColumnConfig::new("location")) > + .column(ColumnConfig::new("tos")); > + format_and_print_result_full(&mut data, &info.returns, &output_format, &options); > + > + Ok(()) > +} > + > +#[api( > + input: { > + properties: { > + name: { type: AcmeAccountName }, > + contact: { > + description: "List of email addresses.", > + }, > + directory: { > + type: String, > + description: "The ACME Directory.", > + optional: true, }, Formatting is a bit off here :) > + } > + } > +)] > +///Register an ACME account. > +async fn register_account( > + name: AcmeAccountName, > + contact: String, > + directory: Option, > +) -> Result<(), Error> { > + let (directory_url, custom_directory) = match directory { > + Some(directory) => (directory, true), > + None => { > + println!("Directory endpoints:"); > + for (i, dir) in KNOWN_ACME_DIRECTORIES.iter().enumerate() { > + println!("{}) {}", i, dir.url); > + } > + > + println!("{}) Custom", KNOWN_ACME_DIRECTORIES.len()); > + let mut attempt = 0; > + loop { > + print!("Enter selection: "); > + std::io::stdout().flush()?; > + > + let mut input = String::new(); > + std::io::stdin().read_line(&mut input)?; I know most of this function was copied from PBS, but maybe it would make sense to have a fn read_input(prompt: &str) -> Result and use it for all those prompts here? I think this could make this function much more readable. > + > + match input.trim().parse::() { > + Ok(n) if n < KNOWN_ACME_DIRECTORIES.len() => { > + break (KNOWN_ACME_DIRECTORIES[n].url.to_string(), false); > + } > + Ok(n) if n == KNOWN_ACME_DIRECTORIES.len() => { > + input.clear(); > + print!("Enter custom directory URI: "); > + std::io::stdout().flush()?; > + std::io::stdin().read_line(&mut input)?; > + break (input.trim().to_owned(), true); > + } > + _ => eprintln!("Invalid selection."), > + } > + > + attempt += 1; > + if attempt >= 3 { > + bail!("Aborting."); > + } > + } > + } > + }; > + > + println!("Attempting to fetch Terms of Service from {directory_url:?}"); > + let mut client = AcmeClient::new(directory_url.clone()); > + let directory = client.directory().await?; > + let tos_agreed = if let Some(tos_url) = directory.terms_of_service_url() { > + println!("Terms of Service: {tos_url}"); > + print!("Do you agree to the above terms? [y|N]: "); > + std::io::stdout().flush()?; > + let mut input = String::new(); > + std::io::stdin().read_line(&mut input)?; > + input.trim().eq_ignore_ascii_case("y") > + } else { > + println!("No Terms of Service found, proceeding."); > + true > + }; > + > + let mut eab_enabled = directory.external_account_binding_required(); > + if !eab_enabled && custom_directory { > + print!("Do you want to use external account binding? [y|N]: "); > + std::io::stdout().flush()?; > + let mut input = String::new(); > + std::io::stdin().read_line(&mut input)?; > + eab_enabled = input.trim().eq_ignore_ascii_case("y"); > + } else if eab_enabled { > + println!("The CA requires external account binding."); > + } > + > + let eab_creds = if eab_enabled { > + println!("You should have received a key id and a key from your CA."); > + > + print!("Enter EAB key id: "); > + std::io::stdout().flush()?; > + let mut eab_kid = String::new(); > + std::io::stdin().read_line(&mut eab_kid)?; > + > + print!("Enter EAB key: "); > + std::io::stdout().flush()?; > + let mut eab_hmac_key = String::new(); > + std::io::stdin().read_line(&mut eab_hmac_key)?; > + > + Some((eab_kid.trim().to_owned(), eab_hmac_key.trim().to_owned())) > + } else { > + None > + }; > + > + let tos_url = tos_agreed > + .then(|| directory.terms_of_service_url().map(str::to_owned)) > + .flatten(); > + > + println!("Registering ACME account '{}'...", &name,); No need to create a reference here. Also, name could be inlined into the format string. Also, there is a trailing comma here, which is a bit odd. > + let location = > + proxmox_acme_api::register_account(&name, contact, tos_url, Some(directory_url), eab_creds) > + .await?; I wonder if it would make sense to actually call the API handler here, same as for the other commands? This would entail running registration in a worker task, which would mean that it is visible in the task history together with a task log. But maybe I'm overlooking something here, I saw that PBS does this in the same way as here, and maybe there is a good reason why the registration is not done in a worker. > + println!("Registration successful, account URL: {}", location); > + > + Ok(()) > +} > + [...] > diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs > index bcaa0a6..15114c9 100644 > --- a/cli/admin/src/main.rs > +++ b/cli/admin/src/main.rs > @@ -9,6 +9,7 @@ use proxmox_router::RpcEnvironment; > use proxmox_schema::api; > use proxmox_sys::fs::CreateOptions; > > +mod acme; > mod remotes; > mod support_status; > > @@ -21,13 +22,17 @@ async fn run() -> Result<(), Error> { > &pdm_api_types::AccessControlConfig, > pdm_buildcfg::configdir!("/access"), > )?; > + proxmox_acme_api::init(pdm_buildcfg::configdir!("/acme"), false)?; > + > proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO) > + .stderr_on_no_workertask() > .stderr() > .init()?; Doing this actually prints all messages logged outside a workertask *twice*, since this adds two subscribers that will print to stdout. I think it would make more sense to: proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO) .tasklog_pbs() .stderr() .init()?; ... which should print *all* messages to stderr and messages from within a task log will also be stored in the task log. Maybe also ask Gabriel about these, he did a lot of work on logging. @Gabriel, would my proposal make sense here? > > server::context::init()?; > > let cmd_def = CliCommandMap::new() > + .insert("acme", acme::acme_mgmt_cli()) > .insert("remote", remotes::cli()) > .insert( > "report", _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel