[pdm-devel] [PATCH datacenter-manager] cli: fix adding remotes

[pdm-devel] [PATCH datacenter-manager] cli: fix adding remotes

[Dominik Csapak]

three things needed fixing here:
* type is already included in the remote type, so no need to have it
  externally
* the api call uses `async` so change the handler matching
* the api call needs the proxmox_product_config to be initialized

noticed by a user in the forum:
https://forum.proxmox.com/threads/167483/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 cli/admin/Cargo.toml     |  2 ++
 cli/admin/src/main.rs    |  4 ++++
 cli/admin/src/remotes.rs | 12 ++----------
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/cli/admin/Cargo.toml b/cli/admin/Cargo.toml
index 7ac8965..17155a8 100644
--- a/cli/admin/Cargo.toml
+++ b/cli/admin/Cargo.toml
@@ -15,8 +15,10 @@ serde_json.workspace = true
 
 proxmox-async.workspace = true
 proxmox-log.workspace = true
+proxmox-product-config.workspace = true
 proxmox-router = { workspace = true, features = [ "cli" ], default-features = false }
 proxmox-schema = { workspace = true, features = [ "api-macro" ] }
 
 pdm-api-types.workspace = true
+pdm-config.workspace = true
 server.workspace = true
diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs
index 91b93f3..7170471 100644
--- a/cli/admin/src/main.rs
+++ b/cli/admin/src/main.rs
@@ -5,6 +5,10 @@ mod remotes;
 fn main() {
     //pbs_tools::setup_libc_malloc_opts(); // TODO: move from PBS to proxmox-sys and uncomment
 
+    let api_user = pdm_config::api_user().expect("cannot get api user");
+    let priv_user = pdm_config::priv_user().expect("cannot get privileged user");
+    proxmox_product_config::init(api_user, priv_user);
+
     proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO)
         .stderr()
         .init()
diff --git a/cli/admin/src/remotes.rs b/cli/admin/src/remotes.rs
index 2ef77b7..fc61f04 100644
--- a/cli/admin/src/remotes.rs
+++ b/cli/admin/src/remotes.rs
@@ -92,7 +92,6 @@ fn list_remotes(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Err
 #[api(
     input: {
         properties: {
-            type: { type: RemoteType },
             remote: {
                 flatten: true,
                 type: Remote,
@@ -101,17 +100,10 @@ fn list_remotes(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Err
     }
 )]
 /// Add a new remote.
-fn add_remote(
-    r#type: RemoteType,
-    remote: pdm_api_types::remotes::Remote,
-    rpcenv: &mut dyn RpcEnvironment,
-) -> Result<(), Error> {
-    let mut param = serde_json::to_value(remote)?;
-    param["type"] = serde_json::to_value(r#type)?;
-
+async fn add_remote(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
     let info = &dc_api::remotes::API_METHOD_ADD_REMOTE;
     match info.handler {
-        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv).map(drop),
+        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await.map(drop),
         _ => unreachable!(),
     }
 }
-- 
2.39.5



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

Re: [pdm-devel] [PATCH datacenter-manager] cli: fix adding remotes

[Michael Köppl]

I also stumbled upon this on the forum and attempted to fix it, but you
clearly beat me to it ;)

So instead I tested this using `proxmox-datacenter-manager-admin remote
add --nodes <PVE VM IP> --token root@pam\!pdm-admin --authid root@pam
--id node1 --type pve` on my PDM host. The command had previously failed
as reported in the forum, not recognizing the `--type` param even when
it was supplied. With the patch applied, I can successfully add remotes
of type `pve` again, both possible types (pve, pbs) are recognized
correctly. Also checked that the validation still works; omitting the
`--type` param reports missing parameter 'type', any other value than
pve or pbs will result in an error. The command seems to work as
expected, did not notice anything off.

Also had a look at the changes in the code to the best of my knowledge
and the changes look good to me.

Please consider this:
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>

On 6/16/25 15:58, Dominik Csapak wrote:
> three things needed fixing here:
> * type is already included in the remote type, so no need to have it
>   externally
> * the api call uses `async` so change the handler matching
> * the api call needs the proxmox_product_config to be initialized
> 
> noticed by a user in the forum:
> https://forum.proxmox.com/threads/167483/
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  cli/admin/Cargo.toml     |  2 ++
>  cli/admin/src/main.rs    |  4 ++++
>  cli/admin/src/remotes.rs | 12 ++----------
>  3 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/cli/admin/Cargo.toml b/cli/admin/Cargo.toml
> index 7ac8965..17155a8 100644
> --- a/cli/admin/Cargo.toml
> +++ b/cli/admin/Cargo.toml
> @@ -15,8 +15,10 @@ serde_json.workspace = true
>  
>  proxmox-async.workspace = true
>  proxmox-log.workspace = true
> +proxmox-product-config.workspace = true
>  proxmox-router = { workspace = true, features = [ "cli" ], default-features = false }
>  proxmox-schema = { workspace = true, features = [ "api-macro" ] }
>  
>  pdm-api-types.workspace = true
> +pdm-config.workspace = true
>  server.workspace = true
> diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs
> index 91b93f3..7170471 100644
> --- a/cli/admin/src/main.rs
> +++ b/cli/admin/src/main.rs
> @@ -5,6 +5,10 @@ mod remotes;
>  fn main() {
>      //pbs_tools::setup_libc_malloc_opts(); // TODO: move from PBS to proxmox-sys and uncomment
>  
> +    let api_user = pdm_config::api_user().expect("cannot get api user");
> +    let priv_user = pdm_config::priv_user().expect("cannot get privileged user");
> +    proxmox_product_config::init(api_user, priv_user);
> +
>      proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::INFO)
>          .stderr()
>          .init()
> diff --git a/cli/admin/src/remotes.rs b/cli/admin/src/remotes.rs
> index 2ef77b7..fc61f04 100644
> --- a/cli/admin/src/remotes.rs
> +++ b/cli/admin/src/remotes.rs
> @@ -92,7 +92,6 @@ fn list_remotes(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Err
>  #[api(
>      input: {
>          properties: {
> -            type: { type: RemoteType },
>              remote: {
>                  flatten: true,
>                  type: Remote,
> @@ -101,17 +100,10 @@ fn list_remotes(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Err
>      }
>  )]
>  /// Add a new remote.
> -fn add_remote(
> -    r#type: RemoteType,
> -    remote: pdm_api_types::remotes::Remote,
> -    rpcenv: &mut dyn RpcEnvironment,
> -) -> Result<(), Error> {
> -    let mut param = serde_json::to_value(remote)?;
> -    param["type"] = serde_json::to_value(r#type)?;
> -
> +async fn add_remote(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
>      let info = &dc_api::remotes::API_METHOD_ADD_REMOTE;
>      match info.handler {
> -        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv).map(drop),
> +        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await.map(drop),
>          _ => unreachable!(),
>      }
>  }



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel