Re: [PATCH proxmox 1/3] system-report: add crate for shared report generation

[Christian Ebner <c.ebner@proxmox.com>]

On 7/14/26 10:31 AM, Erik Fastermann wrote:
> Factor the shared report generation from PBS and PDM into a new
> proxmox-system-report crate, so both generate their reports from a
> single implementation.
> 
> The crate defines the common section order (FILES, COMMANDS,
> FUNCTIONS) and the set of general commands run on every product;
> products pass their own additional files, commands and functions on
> top. It produces no report on its own, so the user-visible changes
> land in the respective server packages.
> 
> Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
> Suggested-by: Christian Ebner <c.ebner@proxmox.com>
> Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
> ---
>   Cargo.toml                                 |   2 +
>   proxmox-system-report/Cargo.toml           |  16 ++
>   proxmox-system-report/debian/copyright     |  18 ++
>   proxmox-system-report/debian/debcargo.toml |   7 +
>   proxmox-system-report/src/lib.rs           | 235 +++++++++++++++++++++
>   5 files changed, 278 insertions(+)
>   create mode 100644 proxmox-system-report/Cargo.toml
>   create mode 100644 proxmox-system-report/debian/copyright
>   create mode 100644 proxmox-system-report/debian/debcargo.toml
>   create mode 100644 proxmox-system-report/src/lib.rs
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index ef407166..1732444b 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -59,6 +59,7 @@ members = [
>       "proxmox-subscription",
>       "proxmox-sys",
>       "proxmox-syslog-api",
> +    "proxmox-system-report",
>       "proxmox-systemd",
>       "proxmox-tfa",
>       "proxmox-time",
> @@ -177,6 +178,7 @@ proxmox-io = { version = "1.2.1", path = "proxmox-io" }
>   proxmox-lang = { version = "1.5", path = "proxmox-lang" }
>   proxmox-log = { version = "1.0.0", path = "proxmox-log" }
>   proxmox-login = { version = "1.0.0", path = "proxmox-login" }
> +proxmox-network-api = { version = "1.0.5", path = "proxmox-network-api" }
>   proxmox-network-types = { version = "1.0.2", path = "proxmox-network-types" }
>   proxmox-parallel-handler = { version = "1.0.0", path = "proxmox-parallel-handler" }
>   proxmox-pgp = { version = "1.0.0", path = "proxmox-pgp" }
> diff --git a/proxmox-system-report/Cargo.toml b/proxmox-system-report/Cargo.toml
> new file mode 100644
> index 00000000..84a87c76
> --- /dev/null
> +++ b/proxmox-system-report/Cargo.toml
> @@ -0,0 +1,16 @@
> +[package]
> +name = "proxmox-system-report"
> +description = "Shared system report functionality"
> +version = "0.1.0"

nit: let's start with 1.0.0, otherwise this could be mis-interpreted as 
not production-ready.

> +
> +authors.workspace = true
> +edition.workspace = true
> +exclude.workspace = true
> +homepage.workspace = true
> +license.workspace = true
> +repository.workspace = true
> +rust-version.workspace = true
> +
> +[dependencies]
> +proxmox-network-api = { workspace = true, features = ["impl"] }
> +proxmox-sys.workspace = true

comment: these dependencies must also be declared in debian/control 
(still missing and should be included to build deb packages with their 
dependencies), so apt can resolve and install these in case no other 
package does.

> diff --git a/proxmox-system-report/debian/copyright b/proxmox-system-report/debian/copyright
> new file mode 100644
> index 00000000..77952eba
> --- /dev/null
> +++ b/proxmox-system-report/debian/copyright
> @@ -0,0 +1,18 @@
> +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> +
> +Files:
> + *
> +Copyright: 2019 - 2026 Proxmox Server Solutions GmbH <support@proxmox.com>

question: not sure about the copyright timespan here, but since the 
factored out code lived in proxmox-backup and that has 2019-2026 this is 
fine I guess?

> +License: AGPL-3.0-or-later
> + This program is free software: you can redistribute it and/or modify it under
> + the terms of the GNU Affero General Public License as published by the Free
> + Software Foundation, either version 3 of the License, or (at your option) any
> + later version.
> + .
> + This program is distributed in the hope that it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
> + details.
> + .
> + You should have received a copy of the GNU Affero General Public License along
> + with this program. If not, see <https://www.gnu.org/licenses/>.
> diff --git a/proxmox-system-report/debian/debcargo.toml b/proxmox-system-report/debian/debcargo.toml
> new file mode 100644
> index 00000000..b7864cdb
> --- /dev/null
> +++ b/proxmox-system-report/debian/debcargo.toml
> @@ -0,0 +1,7 @@
> +overlay = "."
> +crate_src_path = ".."
> +maintainer = "Proxmox Support Team <support@proxmox.com>"
> +
> +[source]
> +vcs_git = "git://git.proxmox.com/git/proxmox.git"
> +vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
> diff --git a/proxmox-system-report/src/lib.rs b/proxmox-system-report/src/lib.rs
> new file mode 100644
> index 00000000..92ca8e5d
> --- /dev/null
> +++ b/proxmox-system-report/src/lib.rs
> @@ -0,0 +1,235 @@
> +//! Shared functionality to generate system reports.
> +
> +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
> +#![deny(unsafe_code)]
> +#![deny(missing_docs)]
> +
> +use std::fmt::Write;
> +use std::path::Path;
> +use std::process::Command;
> +
> +use proxmox_network_api::NetworkInterfaceType;
> +
> +/// A group of files to include: `(group_name, [path, ...])`.
> +pub type FileGroup = (&'static str, Vec<&'static str>);
> +
> +/// A command to run: `(command, [arg, ...])`.
> +pub type CommandSpec = (&'static str, Vec<&'static str>);

nit: IMHO this should be StaticArgsCommandSpec, and the docstring state 
that this is for commands with static, compile time known arguments ...

> +
> +/// A command to run with owned arguments: `(command, [arg, ...])`.
> +type DynamicCommandSpec = (&'static str, Vec<String>);

... and this DynamicArgsCommmandSpec, the docstring mentioning that 
these commands are to be run with dynamically generated arguments 
produced on runtime.

> +
> +/// A function to run and its label: `(description, function)`.
> +pub type FunctionMapping = (&'static str, fn() -> String);
> +
> +fn get_top_processes() -> String {
> +    let (exe, args) = ("top", vec!["-b", "-c", "-w512", "-n", "1", "-o", "TIME"]);
> +    let output = Command::new(exe).args(&args).output();
> +    let output = match output {
> +        Ok(output) => String::from_utf8_lossy(&output.stdout).to_string(),
> +        Err(err) => err.to_string(),
> +    };
> +    let output = output.lines().take(30).collect::<Vec<&str>>().join("\n");
> +    format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" "))
> +}
> +
> +fn files() -> Vec<FileGroup> {

nit: in PBS and PDM, the files() helper generates also the list of 
additional, product specific file groups. So might be better to call 
this product_common_files() or product_agnostic_files()?

> +    vec![(
> +        "General System Info",
> +        vec![
> +            "/etc/hostname",
> +            "/etc/hosts",
> +            "/etc/network/interfaces",
> +            "/etc/apt/sources.list",
> +            "/etc/apt/sources.list.d/",
> +            "/proc/pressure/",
> +        ],
> +    )]
> +}
> +
> +fn commands() -> Vec<CommandSpec> {

nit: same as for files(), the command() helper should reflect that these 
are product common commands, so maybe rename this to 
product_common_commands()?

> +    vec![
> +        ("proxmox-boot-tool", vec!["status"]),
> +        ("df", vec!["-h", "-T"]),
> +        (
> +            "lsblk",
> +            vec![
> +                "--ascii",
> +                "-M",
> +                "-o",
> +                "+HOTPLUG,ROTA,PHY-SEC,FSTYPE,MODEL,TRAN",
> +            ],
> +        ),
> +        ("bash", vec!["-c", "ls -l /dev/disk/by-*/"]),
> +        ("zpool", vec!["status"]),
> +        ("zfs", vec!["list"]),
> +        ("zarcstat", vec![]),
> +        ("dmidecode", vec!["-t", "bios"]),
> +        ("lscpu", vec![]),
> +        ("lspci", vec!["-nnk"]),
> +        ("ip", vec!["-details", "-statistics", "a"]),
> +        ("ip", vec!["-4", "route", "show"]),
> +        ("ip", vec!["-6", "route", "show"]),

comment: these commands depend on external executables. So there should 
be a dependency in debian/control as well.

> +    ]
> +}
> +
> +fn dynamic_commands() -> Vec<DynamicCommandSpec> {

nit: same as files() and commands() above ...

> +    let mut commands = Vec::new();
> +
> +    match proxmox_network_api::config() {
> +        Ok((config, _)) => {
> +            for (name, iface) in config.interfaces {
> +                if iface.interface_type == NetworkInterfaceType::Eth {
> +                    commands.push(("ethtool", vec![name]));
> +                }
> +            }
> +        }
> +        Err(err) => {
> +            eprintln!("failed to query network interfaces: {err}");
> +        }
> +    }
> +
> +    commands
> +}
> +
> +fn function_calls() -> Vec<FunctionMapping> {

nit: same as for others above, this are the product_common_function_calls()

> +    vec![("System Load & Uptime", get_top_processes)]
> +}
> +
> +fn get_file_content(file: impl AsRef<Path>) -> String {
> +    use proxmox_sys::fs::file_read_optional_string;
> +    let content = match file_read_optional_string(&file) {
> +        Ok(Some(content)) => content,
> +        Ok(None) => String::from("# file does not exist"),
> +        Err(err) => err.to_string(),
> +    };
> +    let file_name = file.as_ref().display();
> +    format!("`$ cat '{file_name}'`\n```\n{}\n```", content.trim_end())
> +}
> +
> +fn get_directory_content(path: impl AsRef<Path>) -> String {
> +    let read_dir_iter = match std::fs::read_dir(&path) {
> +        Ok(iter) => iter,
> +        Err(err) => {
> +            return format!(
> +                "`$ cat '{}*'`\n```\n# read dir failed - {err}\n```",
> +                path.as_ref().display(),
> +            );
> +        }
> +    };
> +    let mut out = String::new();
> +    let mut first = true;
> +    for entry in read_dir_iter {
> +        let entry = match entry {
> +            Ok(entry) => entry,
> +            Err(err) => {
> +                let _ = writeln!(out, "error during read-dir - {err}");
> +                continue;
> +            }
> +        };
> +        let path = entry.path();
> +        if path.is_file() {
> +            if first {
> +                let _ = writeln!(out, "{}", get_file_content(path));
> +                first = false;
> +            } else {
> +                let _ = writeln!(out, "\n{}", get_file_content(path));
> +            }
> +        } else {
> +            let _ = writeln!(out, "skipping sub-directory `{}`", path.display());
> +        }
> +    }
> +    out
> +}
> +
> +fn get_command_output(exe: &str, args: &[&str]) -> String {
> +    let output = Command::new(exe)
> +        .env("PROXMOX_OUTPUT_NO_BORDER", "1")
> +        .args(args)
> +        .output();
> +    let output = match output {
> +        Ok(output) => {
> +            let mut out = String::from_utf8_lossy(&output.stdout)
> +                .trim_end()
> +                .to_string();
> +            let stderr = String::from_utf8_lossy(&output.stderr)
> +                .trim_end()
> +                .to_string();
> +            if !stderr.is_empty() {
> +                let _ = writeln!(out, "\n```\nSTDERR:\n```\n{stderr}");
> +            }
> +            out
> +        }
> +        Err(err) => err.to_string(),
> +    };
> +    format!("$ `{exe} {}`\n```\n{output}\n```", args.join(" "))
> +}
> +
> +/// Generate a system report as a Markdown-like document.
> +///
> +/// The report has three top-level sections, always emitted in this order:
> +/// `FILES`, `COMMANDS` and `FUNCTIONS`. The project-specific entries passed in
> +/// are merged with a set of built-in, general-purpose entries common to all
> +/// products.
> +///
> +/// Missing files, unreadable directories and commands that fail to spawn are
> +/// reported inline instead of aborting the report.
> +pub fn generate_report(
> +    project_files: Vec<FileGroup>,
> +    project_commands: Vec<CommandSpec>,
> +    project_function_calls: Vec<FunctionMapping>,
> +) -> String {
> +    // We deliberately output the shared files before the project specific files
> +    // to preserve the legacy report ordering.
> +    let file_contents = files()
> +        .iter()
> +        .chain(&project_files)
> +        .map(|group| {
> +            let (group, files) = group;
> +            let group_content = files
> +                .iter()
> +                .map(|file_name| {
> +                    let path = Path::new(file_name);
> +                    if path.is_dir() {
> +                        get_directory_content(path)
> +                    } else {
> +                        get_file_content(file_name)
> +                    }
> +                })
> +                .collect::<Vec<String>>()
> +                .join("\n\n");
> +
> +            format!("### {group}\n\n{group_content}")
> +        })
> +        .collect::<Vec<String>>()
> +        .join("\n\n");
> +
> +    let static_command_outputs = project_commands
> +        .into_iter()
> +        .chain(commands())
> +        .map(|(command, args)| get_command_output(command, &args));
> +
> +    let dynamic_command_outputs = dynamic_commands().into_iter().map(|(command, args)| {
> +        let args: Vec<_> = args.iter().map(String::as_str).collect();
> +        get_command_output(command, &args)
> +    });
> +
> +    let command_outputs = static_command_outputs
> +        .chain(dynamic_command_outputs)
> +        .collect::<Vec<String>>()
> +        .join("\n\n");
> +
> +    let function_outputs = project_function_calls
> +        .iter()
> +        .chain(&function_calls())
> +        .map(|(desc, function)| {
> +            let output = function();
> +            format!("#### {desc}\n{}\n", output.trim_end())
> +        })
> +        .collect::<Vec<String>>()
> +        .join("\n\n");
> +
> +    format!(
> +        "## FILES\n\n{file_contents}\n## COMMANDS\n\n{command_outputs}\n## FUNCTIONS\n\n{function_outputs}\n"
> +    )
> +}