Re: [pbs-devel] [PATCH proxmox-backup] Add .../apt/update API call

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On July 20, 2020 10:47 am, Stefan Reiter wrote:
> 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@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"?

AFAICT 'apt changelog' looks into the 'Changelogs' line of the 
(In)Release file, and falls back to the local changelog if that is not 
set.. so at least we should also look there if that is accessible (and 
possibly also set it for our repos ;))

> 
>>> +
>>> +    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.

maybe? ;)

> 
>>> +        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, &current_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?

regular repos: lvm2, corosync + kronosnet, grub, frr, lxcfs, zfs, spice stuff, openvswitch, smartmontools
optional repos: lots of rust packages in the devel repo, Ceph packages

> 
>>> +                    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@lists.proxmox.com
>>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>>
>>>
>>>
>> 
>> 
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>> 
>> 
>