[pve-devel] [PATCH proxmox-backup 1/1] apt: use `apt changelog` for changelog fetching

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

support for it got added to Proxmox repositories, so there is no need to use
custom logic and manual fetching for this anymore.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires versioned depends on proxmox-widget-toolkit.

 pbs-api-types/src/lib.rs |  2 -
 src/api2/node/apt.rs     | 85 +++++-----------------------------------
 src/tools/apt.rs         | 77 ------------------------------------
 3 files changed, 9 insertions(+), 155 deletions(-)

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 4764c51a..b663f2d7 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -395,8 +395,6 @@ pub struct APTUpdateInfo {
     pub priority: String,
     /// Package section
     pub section: String,
-    /// URL under which the package's changelog can be retrieved
-    pub change_log_url: String,
     /// Custom extra field for additional package information
     #[serde(skip_serializing_if = "Option::is_none")]
     pub extra_info: Option<String>,
diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index f7328b81..491b8cc9 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -1,6 +1,5 @@
 use anyhow::{bail, format_err, Error};
 use serde_json::{json, Value};
-use std::collections::HashMap;
 use std::os::unix::prelude::OsStrExt;
 
 use proxmox_router::{
@@ -19,10 +18,9 @@ use pbs_api_types::{
     APTUpdateInfo, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA,
     UPID_SCHEMA,
 };
-use pbs_buildcfg::PROXMOX_BACKUP_SUBSCRIPTION_FN;
 
 use crate::config::node;
-use crate::tools::{apt, pbs_simple_http};
+use crate::tools::apt;
 use proxmox_rest_server::WorkerTask;
 
 #[api(
@@ -224,81 +222,17 @@ pub fn apt_update_database(
     },
 )]
 /// Retrieve the changelog of the specified package.
-fn apt_get_changelog(param: Value) -> Result<Value, Error> {
-    let name = pbs_tools::json::required_string_param(&param, "name")?.to_owned();
-    let version = param["version"].as_str();
-
-    let pkg_info = apt::list_installed_apt_packages(
-        |data| match version {
-            Some(version) => version == data.active_version,
-            None => data.active_version == data.candidate_version,
-        },
-        Some(&name),
-    );
-
-    if pkg_info.is_empty() {
-        bail!("Package '{}' not found", name);
-    }
-
-    let proxy_config = read_and_update_proxy_config()?;
-    let client = pbs_simple_http(proxy_config);
-
-    let changelog_url = &pkg_info[0].change_log_url;
-    // FIXME: use 'apt-get changelog' for proxmox packages as well, once repo supports it
-    if changelog_url.starts_with("http://download.proxmox.com/") {
-        let changelog = proxmox_async::runtime::block_on(client.get_string(changelog_url, None))
-            .map_err(|err| {
-                format_err!(
-                    "Error downloading changelog from '{}': {}",
-                    changelog_url,
-                    err
-                )
-            })?;
-        Ok(json!(changelog))
-    } else if changelog_url.starts_with("https://enterprise.proxmox.com/") {
-        let sub = match proxmox_subscription::files::read_subscription(
-            PROXMOX_BACKUP_SUBSCRIPTION_FN,
-            &[proxmox_subscription::files::DEFAULT_SIGNING_KEY],
-        )? {
-            Some(sub) => sub,
-            None => {
-                bail!("cannot retrieve changelog from enterprise repo: no subscription info found")
-            }
-        };
-        let (key, id) = match sub.key {
-            Some(key) => match sub.serverid {
-                Some(id) => (key, id),
-                None => bail!("cannot retrieve changelog from enterprise repo: no server id found"),
-            },
-            None => {
-                bail!("cannot retrieve changelog from enterprise repo: no subscription key found")
-            }
-        };
-
-        let mut auth_header = HashMap::new();
-        auth_header.insert(
-            "Authorization".to_owned(),
-            format!("Basic {}", base64::encode(format!("{}:{}", key, id))),
-        );
-
-        let changelog =
-            proxmox_async::runtime::block_on(client.get_string(changelog_url, Some(&auth_header)))
-                .map_err(|err| {
-                    format_err!(
-                        "Error downloading changelog from '{}': {}",
-                        changelog_url,
-                        err
-                    )
-                })?;
-        Ok(json!(changelog))
+fn apt_get_changelog(name: String, version: Option<String>) -> Result<Value, Error> {
+    let mut command = std::process::Command::new("apt-get");
+    command.arg("changelog");
+    command.arg("-qq"); // don't display download progress
+    if let Some(ver) = version {
+        command.arg(format!("{name}={ver}"));
     } else {
-        let mut command = std::process::Command::new("apt-get");
-        command.arg("changelog");
-        command.arg("-qq"); // don't display download progress
         command.arg(name);
-        let output = proxmox_sys::command::run_command(command, None)?;
-        Ok(json!(output))
     }
+    let output = proxmox_sys::command::run_command(command, None)?;
+    Ok(json!(output))
 }
 
 #[api(
@@ -349,7 +283,6 @@ pub fn get_versions() -> Result<Vec<APTUpdateInfo>, Error> {
             origin: "unknown".into(),
             priority: "unknown".into(),
             section: "unknown".into(),
-            change_log_url: "unknown".into(),
             extra_info,
         }
     }
diff --git a/src/tools/apt.rs b/src/tools/apt.rs
index 5a8cd136..900843aa 100644
--- a/src/tools/apt.rs
+++ b/src/tools/apt.rs
@@ -90,70 +90,6 @@ const_regex! {
     FILENAME_EXTRACT_REGEX = r"^.*/.*?_(.*)_Packages$";
 }
 
-// FIXME: once the 'changelog' API call switches over to 'apt-get changelog' only,
-// consider removing this function entirely, as it's value is never used anywhere
-// then (widget-toolkit doesn't use the value either)
-fn get_changelog_url(
-    package: &str,
-    filename: &str,
-    version: &str,
-    origin: &str,
-    component: &str,
-) -> Result<String, Error> {
-    if origin.is_empty() {
-        bail!("no origin available for package {}", package);
-    }
-
-    if origin == "Debian" {
-        let mut command = std::process::Command::new("apt-get");
-        command.arg("changelog");
-        command.arg("--print-uris");
-        command.arg(package);
-        let output = proxmox_sys::command::run_command(command, None)?; // format: 'http://foo/bar' package.changelog
-        let output = match output.split_once(' ') {
-            Some((uri, _file_name)) if uri.len() > 2 => uri[1..uri.len() - 1].to_owned(),
-            Some((uri, _file_name)) => bail!(
-                "invalid output (URI part too short) from 'apt-get changelog --print-uris': {}",
-                uri
-            ),
-            None => bail!(
-                "invalid output from 'apt-get changelog --print-uris': {}",
-                output
-            ),
-        };
-        return Ok(output);
-    } else if origin == "Proxmox" {
-        // FIXME: Use above call to 'apt changelog <pkg> --print-uris' as well.
-        // Currently not possible as our packages do not have a URI set in their Release file.
-        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"),
-        };
-
-        if component == "pbs-enterprise" {
-            return Ok(format!(
-                "https://enterprise.proxmox.com/{}/{}_{}.changelog",
-                base, package, version
-            ));
-        } else {
-            return Ok(format!(
-                "http://download.proxmox.com/{}/{}_{}.changelog",
-                base, package, version
-            ));
-        }
-    }
-
-    bail!("unknown origin ({}) or component ({})", origin, component)
-}
-
 pub struct FilterData<'a> {
     /// package name
     pub package: &'a str,
@@ -273,7 +209,6 @@ where
         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();
 
@@ -317,17 +252,6 @@ where
                     if let Some(origin_name) = pkg_file.origin() {
                         origin_res = origin_name;
                     }
-
-                    let filename = pkg_file.file_name();
-                    let component = pkg_file.component();
-
-                    // build changelog URL from gathered information
-                    // ignore errors, use empty changelog instead
-                    let url =
-                        get_changelog_url(&package, &filename, &version, &origin_res, &component);
-                    if let Ok(url) = url {
-                        change_log_url = url;
-                    }
                 }
             }
 
@@ -352,7 +276,6 @@ where
                 title: short_desc,
                 arch: view.arch(),
                 description: long_desc,
-                change_log_url,
                 origin: origin_res,
                 version: candidate_version.clone(),
                 old_version: match current_version {
-- 
2.39.2