From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 8045466228 for ; Thu, 5 Nov 2020 16:05:29 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6C16E1A1A0 for ; Thu, 5 Nov 2020 16:04:59 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 69CC11A195 for ; Thu, 5 Nov 2020 16:04:58 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 30BA246026 for ; Thu, 5 Nov 2020 16:04:58 +0100 (CET) To: Proxmox Backup Server development discussion , Mira Limbeck References: <20201105125646.17933-1-m.limbeck@proxmox.com> From: Thomas Lamprecht Message-ID: Date: Thu, 5 Nov 2020 16:04:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101 Thunderbird/83.0 MIME-Version: 1.0 In-Reply-To: <20201105125646.17933-1-m.limbeck@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.115 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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. [mod.rs, node.rs, apt.rs, proxmox-backup-manager.rs] Subject: Re: [pbs-devel] [PATCH v4 proxmox-backup] add versions API call X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Nov 2020 15:05:29 -0000 On 05.11.20 13:56, Mira Limbeck wrote: > Adds an API call that returns all relevant packages, same as the > versions API call in PVE. this is really far from having anything to do with PVE... 1. It's on a different API path, PVE: `/nodes//apt/versions` this uses `/nodes//versions` 2. It returns a *complete* different result type, PVE uses the same as it= 's update call, which Stefan then adopted for PBS as APTUpdateInfo type, = this patch invents some arbitrary new type, not compatible to existing pve-manager widgets. 3. The CLI commando does a connect to the API daemon, instead of just cal= ling that code directly like PVE does, I mean, this is my least concern, bu= t the version command is often required when there are issues, i.e., pos= sibly no functional API daemon to connect to, so it IMO makes sense to avoid= connecting locally for such a thing. =20 > In addition extends proxmox-backup-manager with the 'versions' command > that prints the packages in a format similar to pveversion. why not use the API result type, and the format CLI helpers to make it mo= re consistent with the remaining commands? >=20 > Signed-off-by: Mira Limbeck > --- > v4: > - incorporated wolfgang's suggestions: > - is_some() && unwrap() replaced with match > - removed unnecessary map() > - renamed fd to filter_data > - changed the static str array to a slice of strs > v3: > - fixed the commit message to contain the right API call > and command ('versions') > v2: > - renamed api call to 'versions' from 'version' > - incorporated stefan's suggestions: > - changed the filter to include all packages, but only installed > versions > - fixed looping over the cache multiple times > - fixed use of wrong version > - removed unnecessary version checks >=20 > src/api2/node.rs | 151 +++++++++++++++++++++++++++++-= > src/api2/types/mod.rs | 15 ++- > src/bin/proxmox-backup-manager.rs | 38 ++++++++ > src/tools/apt.rs | 3 + > 4 files changed, 205 insertions(+), 2 deletions(-) >=20 > diff --git a/src/api2/node.rs b/src/api2/node.rs > index a19bea7e..0c66af55 100644 > --- a/src/api2/node.rs > +++ b/src/api2/node.rs > @@ -19,7 +19,7 @@ use proxmox::tools::websocket::WebSocket; > use proxmox::{identity, sortable}; > =20 > use crate::api2::types::*; > -use crate::config::acl::PRIV_SYS_CONSOLE; > +use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_CONSOLE}; > use crate::server::WorkerTask; > use crate::tools; > use crate::tools::ticket::{self, Empty, Ticket}; > @@ -305,6 +305,154 @@ fn upgrade_to_websocket( > .boxed() > } > =20 > +#[api( > + input: { > + properties: { > + node: { > + schema: NODE_SCHEMA, > + }, > + verbose: { > + description: "Enable verbose output.", > + type: Boolean, > + optional: true, > + default: false, > + }, > + }, > + }, > + returns: { > + description: "List of packages and their installed version.", > + type: Array, > + items: { > + type: PackageInfo, > + }, > + }, > + access: { > + permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false)= , > + }, > +)] > +/// Get package information for important Proxmox packages. > +pub fn get_versions(verbose: bool) -> Result { > + const PACKAGES: &[&str] =3D &[ > + "ifupdown2", > + "libjs-extjs", > + "proxmox-backup", > + "proxmox-backup-docs", > + "proxmox-backup-client", > + "proxmox-backup-server", > + "proxmox-mini-journalreader", > + "proxmox-widget-toolkit", > + "pve-xtermjs", > + "smartmontools", > + "zfsutils-linux", > + ]; > + > + let running_kernel =3D nix::sys::utsname::uname().release().to_own= ed(); > + let mut packages: Vec =3D Vec::new(); > + let pbs_packages =3D tools::apt::list_installed_apt_packages( > + |filter_data| { > + (filter_data.installed_version =3D=3D Some(filter_data.act= ive_version)) > + && (filter_data.package.starts_with("pve-kernel-") > + || PACKAGES.contains(&filter_data.package)) > + }, > + None, > + ); > + if let Some(proxmox_backup) =3D pbs_packages > + .iter() > + .find(|pkg| pkg.package =3D=3D "proxmox-backup") > + { > + packages.push(PackageInfo { > + name: proxmox_backup.package.clone(), > + version: proxmox_backup.old_version.clone(), > + comment: Some(format!("running kernel: {}", &running_kerne= l)), > + }); > + } else { > + packages.push(PackageInfo { > + name: "proxmox-backup".to_owned(), > + version: "not correctly installed".to_owned(), > + comment: Some(format!("running kernel: {}", &running_kerne= l)), > + }); > + } > + > + if !verbose { > + return Ok(json!(packages)); > + } > + > + if let Some(proxmox_backup_server) =3D pbs_packages > + .iter() > + .find(|pkg| pkg.package =3D=3D "proxmox-backup-server") > + { > + packages.push(PackageInfo { > + name: proxmox_backup_server.package.clone(), > + version: proxmox_backup_server.old_version.clone(), > + comment: Some(format!( > + "running version: {}.{}", > + crate::api2::version::PROXMOX_PKG_VERSION, > + crate::api2::version::PROXMOX_PKG_RELEASE > + )), > + }); > + } else { > + packages.push(PackageInfo { > + name: "proxmox-backup-server".to_owned(), > + version: "not correctly installed".to_owned(), > + comment: Some(format!( > + "running version: {}.{}", > + crate::api2::version::PROXMOX_PKG_VERSION, > + crate::api2::version::PROXMOX_PKG_RELEASE > + )), > + }); > + } > + > + let mut pve_kernel_pkgs: Vec =3D pbs_packages > + .iter() > + .filter(|pkg| pkg.package.starts_with("pve-kernel-")) > + .cloned() > + .collect(); > + // make sure the cache mutex gets dropped before the next call to = list_installed_apt_packages > + { > + let cache =3D apt_pkg_native::Cache::get_singleton(); > + pve_kernel_pkgs.sort_by(|left, right| { > + cache > + .compare_versions(&left.old_version, &right.old_versio= n) > + .reverse() > + }); > + } > + > + for pkg in pve_kernel_pkgs.iter() { > + packages.push(PackageInfo { > + name: pkg.package.clone(), > + version: pkg.old_version.clone(), > + comment: None, > + }); > + } > + > + // add packages we're interested in, but are not installed > + // and the installed ones returned by list_installed_apt_packages > + for pkg in PACKAGES.iter() { > + if pkg =3D=3D &"proxmox-backup" || pkg =3D=3D &"proxmox-backup= -server" { > + continue; > + } > + let apt_pkg =3D pbs_packages.iter().find(|item| &item.package = =3D=3D pkg); > + match apt_pkg { > + Some(apt_pkg) =3D> { > + packages.push(PackageInfo { > + name: apt_pkg.package.clone(), > + version: apt_pkg.old_version.clone(), > + comment: None, > + }); > + } > + None =3D> { > + packages.push(PackageInfo { > + name: pkg.to_string(), > + version: "not installed".to_owned(), > + comment: None, > + }); > + } > + } > + } > + > + Ok(json!(packages)) > +} > + > pub const SUBDIRS: SubdirMap =3D &[ > ("apt", &apt::ROUTER), > ("disks", &disks::ROUTER), > @@ -320,6 +468,7 @@ pub const SUBDIRS: SubdirMap =3D &[ > ("tasks", &tasks::ROUTER), > ("termproxy", &Router::new().post(&API_METHOD_TERMPROXY)), > ("time", &time::ROUTER), > + ("versions", &Router::new().get(&API_METHOD_GET_VERSIONS)), > ( > "vncwebsocket", > &Router::new().upgrade(&API_METHOD_WEBSOCKET), > diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs > index 7ee89f57..707f1a9a 100644 > --- a/src/api2/types/mod.rs > +++ b/src/api2/types/mod.rs > @@ -1129,7 +1129,7 @@ pub enum RRDTimeFrameResolution { > } > =20 > #[api()] > -#[derive(Debug, Serialize, Deserialize)] > +#[derive(Debug, Clone, Serialize, Deserialize)] > #[serde(rename_all =3D "PascalCase")] > /// Describes a package for which an update is available. > pub struct APTUpdateInfo { > @@ -1155,6 +1155,19 @@ pub struct APTUpdateInfo { > pub change_log_url: String, > } > =20 > +#[api()] > +#[derive(Serialize, Deserialize)] > +#[serde(rename_all =3D "PascalCase")] > +/// Pair of package name and version with optional comment. > +pub struct PackageInfo { > + /// Package name > + pub name: String, > + /// Package version > + pub version: String, > + /// Optional comment > + pub comment: Option, > +} > + > #[api()] > #[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] > #[serde(rename_all =3D "lowercase")] > diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup= -manager.rs > index 7499446b..d4ad73fe 100644 > --- a/src/bin/proxmox-backup-manager.rs > +++ b/src/bin/proxmox-backup-manager.rs > @@ -363,6 +363,41 @@ async fn report() -> Result { > Ok(Value::Null) > } > =20 > +#[api( > + input: { > + properties: { > + verbose: { > + description: "Enable verbose output.", > + type: Boolean, > + optional: true, > + default: false, > + }, > + }, > + }, > +)] > +/// Get package information for important Proxmox packages. > +async fn get_versions(param: Value) -> Result { > + let client =3D connect()?; > + > + let node =3D proxmox::tools::nodename(); > + > + let path =3D format!("api2/json/nodes/{}/versions", node); > + > + let mut result =3D client.get(&path, Some(param)).await?; > + > + let data =3D result["data"].take(); > + let packages: Vec =3D serde_json::from_value(data)?; > + > + for pkg in packages { > + match pkg.comment { > + Some(comment) =3D> println!("{}: {} ({})", pkg.name, pkg.v= ersion, comment), > + None =3D> println!("{}: {}", pkg.name, pkg.version), > + } > + } > + > + Ok(Value::Null) > +} > + > fn main() { > =20 > proxmox_backup::tools::setup_safe_path_env(); > @@ -396,6 +431,9 @@ fn main() { > ) > .insert("report", > CliCommand::new(&API_METHOD_REPORT) > + ) > + .insert("versions", > + CliCommand::new(&API_METHOD_GET_VERSIONS) > ); > =20 > =20 > diff --git a/src/tools/apt.rs b/src/tools/apt.rs > index 5800e0a2..fab8f998 100644 > --- a/src/tools/apt.rs > +++ b/src/tools/apt.rs > @@ -136,6 +136,8 @@ fn get_changelog_url( > } > =20 > pub struct FilterData<'a> { > + // package name > + pub package: &'a str, > // this is version info returned by APT > pub installed_version: Option<&'a str>, > pub candidate_version: &'a str, > @@ -270,6 +272,7 @@ where > let mut long_desc =3D "".to_owned(); > =20 > let fd =3D FilterData { > + package: package.as_str(), > installed_version: current_version.as_deref(), > candidate_version: &candidate_version, > active_version: &version, >=20