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 EA9406456F for ; Mon, 20 Jul 2020 11:13:48 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D6579BE19 for ; Mon, 20 Jul 2020 11:13:48 +0200 (CEST) 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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id E0FABBE0F for ; Mon, 20 Jul 2020 11:13:46 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A1CB8430AE for ; Mon, 20 Jul 2020 11:13:46 +0200 (CEST) Date: Mon, 20 Jul 2020 11:13:36 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion , Stefan Reiter References: <20200714091304.15254-1-s.reiter@proxmox.com> <1594819058.q8bha0npe8.astroid@nora.none> In-Reply-To: MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1595236131.3nedn9btcy.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.324 Adjusted score from AWL reputation of From: address KAM_ASCII_DIVIDERS 0.8 Spam that uses ascii formatting tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 Subject: Re: [pbs-devel] [PATCH proxmox-backup] Add .../apt/update 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: Mon, 20 Jul 2020 09:13:49 -0000 On July 20, 2020 10:47 am, Stefan Reiter wrote: > thanks for taking a look! >=20 > On 7/15/20 3:34 PM, Fabian Gr=C3=BCnbichler wrote: >> On July 14, 2020 11:13 am, Stefan Reiter wrote: >>> Lists all available package updates via libapt-pkg. Output format is >>> taken from PVE to enable JS component reuse in the future. >>> >>> Depends on apt-pkg-native-rs. Changelog-URL detection is inspired by PV= E >>> perl code (but modified). >>> >>> list_installed_apt_packages iterates all packages and creates an >>> APTUpdateInfo with detailed information for every package matched by th= e >>> given filter Fn. >>> >>> libapt-pkg has some questionable design choices regarding their use of >>> 'iterators', which means quite a bit of nesting sadly... >>> >>> Signed-off-by: Stefan Reiter >>> --- >>> >>> apt-pkg-native-rs requires some custom patches on top of the current up= stream to >>> be able to access all required information. I sent a PR to upstream, bu= t it >>> hasn't been included as of yet. >>> >>> The package is not mentioned in Cargo.toml, since we don't have an inte= rnal >>> package for it, but for testing you can include my fork with the patche= s: >>> >>> apt-pkg-native =3D { git =3D "https://github.com/PiMaker/apt-pkg-nat= ive-rs" } >>> >>> The original is here: https://github.com/FauxFaux/apt-pkg-native-rs >>=20 >> this is now available with your patches in devel, probably warrants a >> comment that this is a special patched one when we add it to Cargo.toml >>=20 >=20 > I'll send the v2 with the Cargo.toml updated and a comment >=20 >>> Also, the changelog URL detection was initially just taken from Perl co= de, but >>> it turns out we have some slightly different information there, so I di= d my best >>> to rewrite it to be accurate. With this implementation I get a "200 OK"= on the >>> generated changelog URL of all default-installed packages on PBS, excep= t for two >>> with recent security updates (as mentioned in the code comment, they do= n't seem >>> to have changelogs at all). >>> >>> I'll probably take a look at the perl code in the future to see if it c= an be >>> improved as well, some Debian changelogs produce 404 URLs for me there. >>> >>> >>> src/api2/node.rs | 2 + >>> src/api2/node/apt.rs | 205 ++++++++++++++++++++++++++++++++++++++++++= + >>> src/api2/types.rs | 27 ++++++ >>> 3 files changed, 234 insertions(+) >>> create mode 100644 src/api2/node/apt.rs >>> >>> diff --git a/src/api2/node.rs b/src/api2/node.rs >>> index 13ff282c..7e70bc04 100644 >>> --- a/src/api2/node.rs >>> +++ b/src/api2/node.rs >>> @@ -11,8 +11,10 @@ mod services; >>> mod status; >>> pub(crate) mod rrd; >>> pub mod disks; >>> +mod apt; >>> =20 >>> pub const SUBDIRS: SubdirMap =3D &[ >>> + ("apt", &apt::ROUTER), >>> ("disks", &disks::ROUTER), >>> ("dns", &dns::ROUTER), >>> ("journal", &journal::ROUTER), >>> diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs >>> new file mode 100644 >>> index 00000000..8b1f2ecf >>> --- /dev/null >>> +++ b/src/api2/node/apt.rs >>> @@ -0,0 +1,205 @@ >>> +use apt_pkg_native::Cache; >>> +use anyhow::{Error, bail}; >>> +use serde_json::{json, Value}; >>> + >>> +use proxmox::{list_subdirs_api_method, const_regex}; >>> +use proxmox::api::{api, Router, Permission, SubdirMap}; >>> + >>> +use crate::config::acl::PRIV_SYS_AUDIT; >>> +use crate::api2::types::{APTUpdateInfo, NODE_SCHEMA}; >>> + >>> +const_regex! { >>> + VERSION_EPOCH_REGEX =3D r"^\d+:"; >>> + FILENAME_EXTRACT_REGEX =3D r"^.*/.*?_(.*)_Packages$"; >>> +} >>> + >>> +fn get_changelog_url( >>> + package: &str, >>> + filename: &str, >>> + source_pkg: &str, >>> + version: &str, >>> + source_version: &str, >>> + origin: &str, >>> + component: &str, >>> +) -> Result { >>> + if origin =3D=3D "" { >>> + bail!("no origin available for package {}", package); >>> + } >>=20 >> maybe we could just switch this out in favor of 'apt changelog'? or see >> what that does internally? >>=20 >=20 > AFAICT it does exactly this... Haven't looked at the source, but running=20 > "apt changelog samba" (where the latest update was a security one, thus=20 > without a changelog) displays the same behaviour we do (a 404 with the=20 > "metadata.*" url). >=20 > The issue with just running "apt changelog" here is that we want a URL=20 > and not the changelog itself (if we want to stay faithful to the PVE API=20 > anyway). Though I suppose we could implement the "changelog" API call=20 > here and put that into "ChangeLogURL"? AFAICT 'apt changelog' looks into the 'Changelogs' line of the=20 (In)Release file, and falls back to the local changelog if that is not=20 set.. so at least we should also look there if that is accessible (and=20 possibly also set it for our repos ;)) >=20 >>> + >>> + if origin =3D=3D "Debian" { >>> + let source_version =3D (VERSION_EPOCH_REGEX.regex_obj)().repla= ce_all(source_version, ""); >>> + >>> + let prefix =3D if source_pkg.starts_with("lib") { >>> + source_pkg.get(0..4) >>> + } else { >>> + source_pkg.get(0..1) >>> + }; >>> + >>> + let prefix =3D match prefix { >>> + Some(p) =3D> p, >>> + None =3D> bail!("cannot get starting characters of package= name '{}'", package) >>> + }; >>> + >>> + // note: security updates seem to not always upload a changelo= g for >>> + // their package version, so this only works *most* of the tim= e >>> + return Ok(format!("https://metadata.ftp-master.debian.org/chan= gelogs/main/{}/{}/{}_{}_changelog", >>> + prefix, source_pkg, source_pkg, source_versi= on)); >>> + >>> + } else if origin =3D=3D "Proxmox" && component.starts_with("pbs") = { >>=20 >> what about co-located PBS & PVE? and probably less important, what about >> the devel/Ceph repositories? >>=20 >=20 > Would just leaving out the starts_with fix this? I'll test around. maybe? ;) >=20 >>> + let version =3D (VERSION_EPOCH_REGEX.regex_obj)().replace_all(= version, ""); >>> + >>> + let base =3D match (FILENAME_EXTRACT_REGEX.regex_obj)().captur= es(filename) { >>> + Some(captures) =3D> { >>> + let base_capture =3D captures.get(1); >>> + match base_capture { >>> + Some(base_underscore) =3D> base_underscore.as_str(= ).replace("_", "/"), >>> + None =3D> bail!("incompatible filename, cannot fin= d regex group") >>> + } >>> + }, >>> + None =3D> bail!("incompatible filename, doesn't match rege= x") >>> + }; >>> + >>> + return Ok(format!("http://download.proxmox.com/{}/{}_{}.change= log", >>> + base, package, version)); >>> + } >>> + >>> + bail!("unknown origin ({}) or component ({})", origin, component) >>> +} >>> + >>> +fn list_installed_apt_packages bool>(filter= : F) >>> + -> Vec { >>> + >>> + let mut ret =3D Vec::new(); >>> + >>> + // note: this is not an 'apt update', it just re-reads the cache f= rom disk >>> + let mut cache =3D Cache::get_singleton(); >>> + cache.reload(); >>> + >>> + let mut cache_iter =3D cache.iter(); >>> + >>> + loop { >>> + let view =3D match cache_iter.next() { >>> + Some(view) =3D> view, >>> + None =3D> break >>> + }; >>> + >>> + let current_version =3D match view.current_version() { >>> + Some(vers) =3D> vers, >>> + None =3D> continue >>> + }; >>> + let candidate_version =3D match view.candidate_version() { >>> + Some(vers) =3D> vers, >>> + // if there's no candidate (i.e. no update) get info of cu= rrently >>> + // installed version instead >>> + None =3D> current_version.clone() >>> + }; >>> + >>> + let package =3D view.name(); >>> + if filter(&package, ¤t_version, &candidate_version) { >>> + let mut origin_res =3D "unknown".to_owned(); >>> + let mut section_res =3D "unknown".to_owned(); >>> + let mut priority_res =3D "unknown".to_owned(); >>> + let mut change_log_url =3D "".to_owned(); >>> + let mut short_desc =3D package.clone(); >>> + let mut long_desc =3D "".to_owned(); >>> + >>> + // get additional information via nested APT 'iterators' >>> + let mut view_iter =3D view.versions(); >>> + while let Some(ver) =3D view_iter.next() { >>> + if ver.version() =3D=3D candidate_version { >>> + if let Some(section) =3D ver.section() { >>> + section_res =3D section; >>> + } >>> + >>> + if let Some(prio) =3D ver.priority_type() { >>> + priority_res =3D prio; >>> + } >>> + >>> + // assume every package has only one origin and pa= ckage file >>=20 >> isn't this wrong though? e.g., we ship packages that are also shipped >> with origin Debian.. >>=20 >=20 > I'll have to check what happens with those. What is supposed to happen?=20 > Which package would that be for example? regular repos: lvm2, corosync + kronosnet, grub, frr, lxcfs, zfs, spice stu= ff, openvswitch, smartmontools optional repos: lots of rust packages in the devel repo, Ceph packages >=20 >>> + let mut origin_iter =3D ver.origin_iter(); >>> + let origin =3D origin_iter.next(); >>> + if let Some(origin) =3D origin { >>> + >>> + if let Some(sd) =3D origin.short_desc() { >>> + short_desc =3D sd; >>> + } >>> + >>> + if let Some(ld) =3D origin.long_desc() { >>> + long_desc =3D ld; >>> + } >>> + >>> + let mut pkg_iter =3D origin.file(); >>> + let pkg_file =3D pkg_iter.next(); >>> + if let Some(pkg_file) =3D pkg_file { >>> + if let Some(origin_name) =3D pkg_file.orig= in() { >>> + origin_res =3D origin_name; >>> + } >>> + >>> + let filename =3D pkg_file.file_name(); >>> + let source_pkg =3D ver.source_package(); >>> + let source_ver =3D ver.source_version(); >>> + let component =3D pkg_file.component(); >>> + >>> + // build changelog URL from gathered infor= mation >>> + // ignore errors, use empty changelog inst= ead >>> + let url =3D get_changelog_url(&package, &f= ilename, &source_pkg, >>> + &candidate_version, &source_ver, &orig= in_res, &component); >>> + if let Ok(url) =3D url { >>> + change_log_url =3D url; >>> + } >>> + } >>> + } >>> + >>> + break; >>> + } >>> + } >>> + >>> + let info =3D APTUpdateInfo { >>> + package, >>> + title: short_desc, >>> + arch: view.arch(), >>> + description: long_desc, >>> + change_log_url, >>> + origin: origin_res, >>> + version: candidate_version, >>> + old_version: current_version, >>> + priority: priority_res, >>> + section: section_res, >>> + }; >>> + ret.push(info); >>> + } >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +#[api( >>> + input: { >>> + properties: { >>> + node: { >>> + schema: NODE_SCHEMA, >>> + }, >>> + }, >>> + }, >>> + returns: { >>> + description: "A list of packages with available updates.", >>> + type: Array, >>> + items: { type: APTUpdateInfo }, >>> + }, >>> + access: { >>> + permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false)= , >>> + }, >>> +)] >>> +/// List available APT updates >>> +fn apt_update_available(_param: Value) -> Result { >>> + let ret =3D list_installed_apt_packages(|_pkg, cur_ver, can_ver| c= ur_ver !=3D can_ver); >>> + Ok(json!(ret)) >>> +} >>> + >>> +const SUBDIRS: SubdirMap =3D &[ >>> + ("update", &Router::new().get(&API_METHOD_APT_UPDATE_AVAILABLE)), >>> +]; >>> + >>> +pub const ROUTER: Router =3D Router::new() >>> + .get(&list_subdirs_api_method!(SUBDIRS)) >>> + .subdirs(SUBDIRS); >>> diff --git a/src/api2/types.rs b/src/api2/types.rs >>> index 0d0fab3b..da73f8db 100644 >>> --- a/src/api2/types.rs >>> +++ b/src/api2/types.rs >>> @@ -962,3 +962,30 @@ pub enum RRDTimeFrameResolution { >>> /// 1 week =3D> last 490 days >>> Year =3D 60*10080, >>> } >>> + >>> +#[api()] >>> +#[derive(Serialize, Deserialize)] >>> +#[serde(rename_all =3D "kebab-case")] >>=20 >> widget toolkit expects PascalCase, so either we fix it there to handle >> both (if that's possible), or we return ugly here :-/ >>=20 >=20 > Right, I wasn't sure how to handle this. I guess PascalCase here would=20 > be the easiest fix. Have we not encountered this issue anywhere else? >=20 >>> +/// Describes a package for which an update is available. >>> +pub struct APTUpdateInfo { >>> + /// Package name >>> + pub package: String, >>> + /// Package title >>> + pub title: String, >>> + /// Package architecture >>> + pub arch: String, >>> + /// Human readable package description >>> + pub description: String, >>> + /// New version to be updated to >>> + pub version: String, >>> + /// Old version currently installed >>> + pub old_version: String, >>> + /// Package origin >>> + pub origin: String, >>> + /// Package priority in human-readable form >>> + pub priority: String, >>> + /// Package section >>> + pub section: String, >>> + /// URL under which the package's changelog can be retrieved >>> + pub change_log_url: String, >>> +} >>> --=20 >>> 2.20.1 >>> >>> >>> >>> _______________________________________________ >>> pbs-devel mailing list >>> pbs-devel@lists.proxmox.com >>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >>> >>> >>> >>=20 >>=20 >> _______________________________________________ >> pbs-devel mailing list >> pbs-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >>=20 >>=20 >=20 =