[pbs-devel] [PATCH proxmox-backup] Add .../apt/update API call
Stefan Reiter
s.reiter at proxmox.com
Mon Jul 20 10:47:11 CEST 2020
thanks for taking a look!
On 7/15/20 3:34 PM, Fabian Grünbichler 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 PVE
>> perl code (but modified).
>>
>> list_installed_apt_packages iterates all packages and creates an
>> APTUpdateInfo with detailed information for every package matched by the
>> 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 <s.reiter at proxmox.com>
>> ---
>>
>> apt-pkg-native-rs requires some custom patches on top of the current upstream to
>> be able to access all required information. I sent a PR to upstream, but it
>> hasn't been included as of yet.
>>
>> The package is not mentioned in Cargo.toml, since we don't have an internal
>> package for it, but for testing you can include my fork with the patches:
>>
>> apt-pkg-native = { git = "https://github.com/PiMaker/apt-pkg-native-rs" }
>>
>> The original is here: https://github.com/FauxFaux/apt-pkg-native-rs
>
> 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
>
I'll send the v2 with the Cargo.toml updated and a comment
>> Also, the changelog URL detection was initially just taken from Perl code, but
>> it turns out we have some slightly different information there, so I did 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, except for two
>> with recent security updates (as mentioned in the code comment, they don't seem
>> to have changelogs at all).
>>
>> I'll probably take a look at the perl code in the future to see if it can 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;
>>
>> pub const SUBDIRS: SubdirMap = &[
>> + ("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 = r"^\d+:";
>> + FILENAME_EXTRACT_REGEX = r"^.*/.*?_(.*)_Packages$";
>> +}
>> +
>> +fn get_changelog_url(
>> + package: &str,
>> + filename: &str,
>> + source_pkg: &str,
>> + version: &str,
>> + source_version: &str,
>> + origin: &str,
>> + component: &str,
>> +) -> Result<String, Error> {
>> + if origin == "" {
>> + bail!("no origin available for package {}", package);
>> + }
>
> maybe we could just switch this out in favor of 'apt changelog'? or see
> what that does internally?
>
AFAICT it does exactly this... Haven't looked at the source, but running
"apt changelog samba" (where the latest update was a security one, thus
without a changelog) displays the same behaviour we do (a 404 with the
"metadata.*" url).
The issue with just running "apt changelog" here is that we want a URL
and not the changelog itself (if we want to stay faithful to the PVE API
anyway). Though I suppose we could implement the "changelog" API call
here and put that into "ChangeLogURL"?
>> +
>> + if origin == "Debian" {
>> + let source_version = (VERSION_EPOCH_REGEX.regex_obj)().replace_all(source_version, "");
>> +
>> + let prefix = if source_pkg.starts_with("lib") {
>> + source_pkg.get(0..4)
>> + } else {
>> + source_pkg.get(0..1)
>> + };
>> +
>> + let prefix = match prefix {
>> + Some(p) => p,
>> + None => bail!("cannot get starting characters of package name '{}'", package)
>> + };
>> +
>> + // note: security updates seem to not always upload a changelog for
>> + // their package version, so this only works *most* of the time
>> + return Ok(format!("https://metadata.ftp-master.debian.org/changelogs/main/{}/{}/{}_{}_changelog",
>> + prefix, source_pkg, source_pkg, source_version));
>> +
>> + } else if origin == "Proxmox" && component.starts_with("pbs") {
>
> what about co-located PBS & PVE? and probably less important, what about
> the devel/Ceph repositories?
>
Would just leaving out the starts_with fix this? I'll test around.
>> + let version = (VERSION_EPOCH_REGEX.regex_obj)().replace_all(version, "");
>> +
>> + let base = match (FILENAME_EXTRACT_REGEX.regex_obj)().captures(filename) {
>> + Some(captures) => {
>> + let base_capture = captures.get(1);
>> + match base_capture {
>> + Some(base_underscore) => base_underscore.as_str().replace("_", "/"),
>> + None => bail!("incompatible filename, cannot find regex group")
>> + }
>> + },
>> + None => bail!("incompatible filename, doesn't match regex")
>> + };
>> +
>> + return Ok(format!("http://download.proxmox.com/{}/{}_{}.changelog",
>> + base, package, version));
>> + }
>> +
>> + bail!("unknown origin ({}) or component ({})", origin, component)
>> +}
>> +
>> +fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
>> + -> Vec<APTUpdateInfo> {
>> +
>> + let mut ret = Vec::new();
>> +
>> + // note: this is not an 'apt update', it just re-reads the cache from disk
>> + let mut cache = Cache::get_singleton();
>> + cache.reload();
>> +
>> + let mut cache_iter = cache.iter();
>> +
>> + loop {
>> + let view = match cache_iter.next() {
>> + Some(view) => view,
>> + None => break
>> + };
>> +
>> + let current_version = match view.current_version() {
>> + Some(vers) => vers,
>> + None => continue
>> + };
>> + let candidate_version = match view.candidate_version() {
>> + Some(vers) => vers,
>> + // if there's no candidate (i.e. no update) get info of currently
>> + // installed version instead
>> + None => current_version.clone()
>> + };
>> +
>> + let package = view.name();
>> + if filter(&package, ¤t_version, &candidate_version) {
>> + let mut origin_res = "unknown".to_owned();
>> + let mut section_res = "unknown".to_owned();
>> + let mut priority_res = "unknown".to_owned();
>> + let mut change_log_url = "".to_owned();
>> + let mut short_desc = package.clone();
>> + let mut long_desc = "".to_owned();
>> +
>> + // get additional information via nested APT 'iterators'
>> + let mut view_iter = view.versions();
>> + while let Some(ver) = view_iter.next() {
>> + if ver.version() == candidate_version {
>> + if let Some(section) = ver.section() {
>> + section_res = section;
>> + }
>> +
>> + if let Some(prio) = ver.priority_type() {
>> + priority_res = prio;
>> + }
>> +
>> + // assume every package has only one origin and package file
>
> isn't this wrong though? e.g., we ship packages that are also shipped
> with origin Debian..
>
I'll have to check what happens with those. What is supposed to happen?
Which package would that be for example?
>> + let mut origin_iter = ver.origin_iter();
>> + let origin = origin_iter.next();
>> + if let Some(origin) = origin {
>> +
>> + if let Some(sd) = origin.short_desc() {
>> + short_desc = sd;
>> + }
>> +
>> + if let Some(ld) = origin.long_desc() {
>> + long_desc = ld;
>> + }
>> +
>> + let mut pkg_iter = origin.file();
>> + let pkg_file = pkg_iter.next();
>> + if let Some(pkg_file) = pkg_file {
>> + if let Some(origin_name) = pkg_file.origin() {
>> + origin_res = origin_name;
>> + }
>> +
>> + let filename = pkg_file.file_name();
>> + let source_pkg = ver.source_package();
>> + let source_ver = ver.source_version();
>> + let component = pkg_file.component();
>> +
>> + // build changelog URL from gathered information
>> + // ignore errors, use empty changelog instead
>> + let url = get_changelog_url(&package, &filename, &source_pkg,
>> + &candidate_version, &source_ver, &origin_res, &component);
>> + if let Ok(url) = url {
>> + change_log_url = url;
>> + }
>> + }
>> + }
>> +
>> + break;
>> + }
>> + }
>> +
>> + let info = 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<Value, Error> {
>> + let ret = list_installed_apt_packages(|_pkg, cur_ver, can_ver| cur_ver != can_ver);
>> + Ok(json!(ret))
>> +}
>> +
>> +const SUBDIRS: SubdirMap = &[
>> + ("update", &Router::new().get(&API_METHOD_APT_UPDATE_AVAILABLE)),
>> +];
>> +
>> +pub const ROUTER: Router = 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 => last 490 days
>> Year = 60*10080,
>> }
>> +
>> +#[api()]
>> +#[derive(Serialize, Deserialize)]
>> +#[serde(rename_all = "kebab-case")]
>
> widget toolkit expects PascalCase, so either we fix it there to handle
> both (if that's possible), or we return ugly here :-/
>
Right, I wasn't sure how to handle this. I guess PascalCase here would
be the easiest fix. Have we not encountered this issue anywhere else?
>> +/// 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,
>> +}
>> --
>> 2.20.1
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel at lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
More information about the pbs-devel
mailing list