[pbs-devel] [PATCH 0/7] apt: add changelog API and fix #2934 (list new packages)

[pbs-devel] [PATCH 0/7] apt: add changelog API and fix #2934 (list new packages)

[Stefan Reiter]

(Note: This is technically a successor to this series [0], but since I changed a
lot around and also added onto it, this is not a v2 but a seperate series
entirely.)

Bring the "Updates" panel up to par with PVE by adding two features:
* A /changelog API call
* A fix for #2934, so new packages that will be installed on upgrade are
  shown too (i.e. new kernels)

The former also requires extracting a new 'http' module out of 'api', so we can
make simple HTTP requests from 'tools'.

Before that, some cleanup is done. Even with that, the code is not quite a prime
example of readability, but with the weirdness that is the libapt-pkg lib and
binding, this was the best I could manage.

Patch 4 is a dependency bump on a new version of my forked libapt-pkg binding
(apt-pkg-native(-rs)). As before, the changes I made to that can be found on
GitHub [1]. Patches 5 and later require the new version to compile.


[0] https://lists.proxmox.com/pipermail/pbs-devel/2020-July/000045.html
[1] https://github.com/PiMaker/apt-pkg-native-rs/


proxmox-backup: Stefan Reiter (7):
  apt: allow filter to select different package version
  add tools::http for generic HTTP GET and move HttpsConnector there
  apt: use 'apt-get changelog --print-uris' in get_changelog_url
  bump apt-pkg-native dependency to 0.3.2
  apt: refactor package detail reading into function
  fix #2934: list to-be-installed packages in updates
  apt: add /changelog API call similar to PVE

 Cargo.toml                |   2 +-
 src/api2/node/apt.rs      | 384 ++++++++++++++++++++++++++++----------
 src/client/http_client.rs |  75 +-------
 src/tools.rs              |   1 +
 src/tools/http.rs         | 100 ++++++++++
 5 files changed, 386 insertions(+), 176 deletions(-)
 create mode 100644 src/tools/http.rs

-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 1/7] apt: allow filter to select different package version

[Stefan Reiter]

To get package details for a specific version instead of only the
candidate.

Also cleanup filter function with extra struct instead of unnamed &str
parameters.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/node/apt.rs | 179 +++++++++++++++++++++++--------------------
 1 file changed, 98 insertions(+), 81 deletions(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 04beaa4d..40791126 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -71,7 +71,16 @@ fn get_changelog_url(
     bail!("unknown origin ({}) or component ({})", origin, component)
 }
 
-fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
+struct FilterData<'a> {
+    // this is version info returned by APT
+    installed_version: &'a str,
+    candidate_version: &'a str,
+
+    // this is the version info the filter is supposed to check
+    active_version: &'a str,
+}
+
+fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(filter: F)
     -> Vec<APTUpdateInfo> {
 
     let mut ret = Vec::new();
@@ -88,19 +97,22 @@ fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
             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 current_version = view.current_version();
+        let candidate_version = view.candidate_version();
+
+        let (current_version, candidate_version) = match (current_version, candidate_version) {
+            (Some(cur), Some(can)) => (cur, can), // package installed and there is an update
+            (Some(cur), None) => (cur.clone(), cur), // package installed and up-to-date
+            (None, Some(_)) => continue, // package could be installed
+            (None, None) => continue, // broken
         };
 
-        let package = view.name();
-        if filter(&package, &current_version, &candidate_version) {
+        // get additional information via nested APT 'iterators'
+        let mut view_iter = view.versions();
+        while let Some(ver) = view_iter.next() {
+
+            let package = view.name();
+            let version = ver.version();
             let mut origin_res = "unknown".to_owned();
             let mut section_res = "unknown".to_owned();
             let mut priority_res = "unknown".to_owned();
@@ -108,74 +120,76 @@ fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
             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 file (not
-                    // origin, but origin *file*, for some reason those seem to
-                    // be different concepts in APT)
-                    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;
-                        }
-
-                        // the package files appear in priority order, meaning
-                        // the one for the candidate version is first
-                        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,
+            let fd = FilterData {
+                installed_version: &current_version,
+                candidate_version: &candidate_version,
+                active_version: &version,
             };
-            ret.push(info);
+
+            if filter(fd) {
+                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 file (not
+                // origin, but origin *file*, for some reason those seem to
+                // be different concepts in APT)
+                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;
+                    }
+
+                    // the package files appear in priority order, meaning
+                    // the one for the candidate version is first - this is fine
+                    // however, as the source package should be the same for all
+                    // versions anyway
+                    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,
+                            &version, &source_ver, &origin_res, &component);
+                        if let Ok(url) = url {
+                            change_log_url = url;
+                        }
+                    }
+                }
+
+                let info = APTUpdateInfo {
+                    package,
+                    title: short_desc,
+                    arch: view.arch(),
+                    description: long_desc,
+                    change_log_url,
+                    origin: origin_res,
+                    version: candidate_version.clone(),
+                    old_version: current_version.clone(),
+                    priority: priority_res,
+                    section: section_res,
+                };
+                ret.push(info);
+            }
         }
     }
 
@@ -201,8 +215,11 @@ fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
 )]
 /// 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))
+    let all_upgradeable = list_installed_apt_packages(|data|
+        data.candidate_version == data.active_version &&
+        data.installed_version != data.candidate_version
+    );
+    Ok(json!(all_upgradeable))
 }
 
 #[api(
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/7] add tools::http for generic HTTP GET and move HttpsConnector there

[Stefan Reiter]

...to avoid having the tools:: module depend on api2.

The get_string function is based directly on hyper and thus relatively
simple, not supporting redirects for example.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/client/http_client.rs |  75 ++--------------------------
 src/tools.rs              |   1 +
 src/tools/http.rs         | 100 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+), 72 deletions(-)
 create mode 100644 src/tools/http.rs

diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index e92d4b18..b57630f8 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -1,8 +1,6 @@
 use std::io::Write;
-use std::task::{Context, Poll};
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::Duration;
-use std::os::unix::io::AsRawFd;
 
 use anyhow::{bail, format_err, Error};
 use futures::*;
@@ -19,22 +17,16 @@ use xdg::BaseDirectories;
 use proxmox::{
     api::error::HttpError,
     sys::linux::tty,
-    tools::{
-        fs::{file_get_json, replace_file, CreateOptions},
-    }
+    tools::fs::{file_get_json, replace_file, CreateOptions},
 };
 
 use super::pipe_to_stream::PipeToSendStream;
 use crate::api2::types::Userid;
-use crate::tools::async_io::EitherStream;
 use crate::tools::{
     self,
     BroadcastFuture,
     DEFAULT_ENCODE_SET,
-    socket::{
-        set_tcp_keepalive,
-        PROXMOX_BACKUP_TCP_KEEPALIVE_TIME,
-    },
+    http::HttpsConnector,
 };
 
 #[derive(Clone)]
@@ -301,7 +293,7 @@ impl HttpClient {
             ssl_connector_builder.set_verify(openssl::ssl::SslVerifyMode::NONE);
         }
 
-        let mut httpc = hyper::client::HttpConnector::new();
+        let mut httpc = HttpConnector::new();
         httpc.set_nodelay(true); // important for h2 download performance!
         httpc.enforce_http(false); // we want https...
 
@@ -929,64 +921,3 @@ impl H2Client {
         }
     }
 }
-
-#[derive(Clone)]
-pub struct HttpsConnector {
-    http: HttpConnector,
-    ssl_connector: std::sync::Arc<SslConnector>,
-}
-
-impl HttpsConnector {
-    pub fn with_connector(mut http: HttpConnector, ssl_connector: SslConnector) -> Self {
-        http.enforce_http(false);
-
-        Self {
-            http,
-            ssl_connector: std::sync::Arc::new(ssl_connector),
-        }
-    }
-}
-
-type MaybeTlsStream = EitherStream<
-    tokio::net::TcpStream,
-    tokio_openssl::SslStream<tokio::net::TcpStream>,
->;
-
-impl hyper::service::Service<Uri> for HttpsConnector {
-    type Response = MaybeTlsStream;
-    type Error = Error;
-    type Future = std::pin::Pin<Box<
-        dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static
-    >>;
-
-    fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        // This connector is always ready, but others might not be.
-        Poll::Ready(Ok(()))
-    }
-
-    fn call(&mut self, dst: Uri) -> Self::Future {
-        let mut this = self.clone();
-        async move {
-            let is_https = dst
-                .scheme()
-                .ok_or_else(|| format_err!("missing URL scheme"))?
-                == "https";
-            let host = dst
-                .host()
-                .ok_or_else(|| format_err!("missing hostname in destination url?"))?
-                .to_string();
-
-            let config = this.ssl_connector.configure();
-            let conn = this.http.call(dst).await?;
-
-            let _ = set_tcp_keepalive(conn.as_raw_fd(), PROXMOX_BACKUP_TCP_KEEPALIVE_TIME);
-
-            if is_https {
-                let conn = tokio_openssl::connect(config?, &host, conn).await?;
-                Ok(MaybeTlsStream::Right(conn))
-            } else {
-                Ok(MaybeTlsStream::Left(conn))
-            }
-        }.boxed()
-    }
-}
diff --git a/src/tools.rs b/src/tools.rs
index 22d6c344..0e4a65c7 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -37,6 +37,7 @@ pub mod loopdev;
 pub mod fuse_loop;
 pub mod socket;
 pub mod zip;
+pub mod http;
 
 mod parallel_handler;
 pub use parallel_handler::*;
diff --git a/src/tools/http.rs b/src/tools/http.rs
new file mode 100644
index 00000000..fe432806
--- /dev/null
+++ b/src/tools/http.rs
@@ -0,0 +1,100 @@
+use anyhow::{Error, format_err, bail};
+use lazy_static::lazy_static;
+use std::task::{Context, Poll};
+use std::os::unix::io::AsRawFd;
+
+use hyper::{Uri, Body};
+use hyper::client::{Client, HttpConnector};
+use openssl::ssl::{SslConnector, SslMethod};
+use futures::*;
+
+use crate::tools::{
+    async_io::EitherStream,
+    socket::{
+        set_tcp_keepalive,
+        PROXMOX_BACKUP_TCP_KEEPALIVE_TIME,
+    },
+};
+
+lazy_static! {
+    static ref HTTP_CLIENT: Client<HttpsConnector, Body> = {
+        let connector = SslConnector::builder(SslMethod::tls()).unwrap().build();
+        let httpc = HttpConnector::new();
+        let https = HttpsConnector::with_connector(httpc, connector);
+        Client::builder().build(https)
+    };
+}
+
+pub async fn get_string<U: AsRef<str>>(uri: U) -> Result<String, Error> {
+    let res = HTTP_CLIENT.get(uri.as_ref().parse()?).await?;
+
+    let status = res.status();
+    if !status.is_success() {
+        bail!("Got bad status '{}' from server", status)
+    }
+
+    let buf = hyper::body::to_bytes(res).await?;
+    String::from_utf8(buf.to_vec())
+        .map_err(|err| format_err!("Error converting HTTP result data: {}", err))
+}
+
+#[derive(Clone)]
+pub struct HttpsConnector {
+    http: HttpConnector,
+    ssl_connector: std::sync::Arc<SslConnector>,
+}
+
+impl HttpsConnector {
+    pub fn with_connector(mut http: HttpConnector, ssl_connector: SslConnector) -> Self {
+        http.enforce_http(false);
+
+        Self {
+            http,
+            ssl_connector: std::sync::Arc::new(ssl_connector),
+        }
+    }
+}
+
+type MaybeTlsStream = EitherStream<
+    tokio::net::TcpStream,
+    tokio_openssl::SslStream<tokio::net::TcpStream>,
+>;
+
+impl hyper::service::Service<Uri> for HttpsConnector {
+    type Response = MaybeTlsStream;
+    type Error = Error;
+    type Future = std::pin::Pin<Box<
+        dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static
+    >>;
+
+    fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
+        // This connector is always ready, but others might not be.
+        Poll::Ready(Ok(()))
+    }
+
+    fn call(&mut self, dst: Uri) -> Self::Future {
+        let mut this = self.clone();
+        async move {
+            let is_https = dst
+                .scheme()
+                .ok_or_else(|| format_err!("missing URL scheme"))?
+                == "https";
+            let host = dst
+                .host()
+                .ok_or_else(|| format_err!("missing hostname in destination url?"))?
+                .to_string();
+
+            let config = this.ssl_connector.configure();
+            let conn = this.http.call(dst).await?;
+
+            let _ = set_tcp_keepalive(conn.as_raw_fd(), PROXMOX_BACKUP_TCP_KEEPALIVE_TIME);
+
+            if is_https {
+                let conn = tokio_openssl::connect(config?, &host, conn).await?;
+                Ok(MaybeTlsStream::Right(conn))
+            } else {
+                Ok(MaybeTlsStream::Left(conn))
+            }
+        }.boxed()
+    }
+}
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 3/7] apt: use 'apt-get changelog --print-uris' in get_changelog_url

[Stefan Reiter]

Avoids custom hardcoded logic, but can only be used for debian packages
as of now. Adds a FIXME to switch over to use --print-uris only once our
package repos support that changelog format.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/node/apt.rs | 46 ++++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 40791126..b120eef5 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -16,14 +16,13 @@ const_regex! {
     FILENAME_EXTRACT_REGEX = r"^.*/.*?_(.*)_Packages$";
 }
 
-// FIXME: Replace with call to 'apt changelog <pkg> --print-uris'. Currently
-// not possible as our packages do not have a URI set in their Release file
+// 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,
-    source_pkg: &str,
     version: &str,
-    source_version: &str,
     origin: &str,
     component: &str,
 ) -> Result<String, Error> {
@@ -32,25 +31,24 @@ fn get_changelog_url(
     }
 
     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 mut command = std::process::Command::new("apt-get");
+        command.arg("changelog");
+        command.arg("--print-uris");
+        command.arg(package);
+        let output = crate::tools::run_command(command, None)?; // format: 'http://foo/bar' package.changelog
+        let output = match output.splitn(2, ' ').next() {
+            Some(output) => {
+                if output.len() < 2 {
+                    bail!("invalid output (URI part too short) from 'apt-get changelog --print-uris: {}", output)
+                }
+                output[1..output.len()-1].to_owned()
+            },
+            None => bail!("invalid output from 'apt-get changelog --print-uris': {}", output)
         };
-
-        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));
-
+        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) {
@@ -162,14 +160,12 @@ fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(filter: F)
                         }
 
                         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,
-                            &version, &source_ver, &origin_res, &component);
+                        let url = get_changelog_url(&package, &filename,
+                            &version, &origin_res, &component);
                         if let Ok(url) = url {
                             change_log_url = url;
                         }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 4/7] bump apt-pkg-native dependency to 0.3.2

[Stefan Reiter]

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

As before, this requires a custom version. My fork is still available at
https://github.com/PiMaker/apt-pkg-native-rs/ , the PR to the upstream has not
received an update.

 Cargo.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Cargo.toml b/Cargo.toml
index 88295d16..f7b52f13 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -14,7 +14,7 @@ name = "proxmox_backup"
 path = "src/lib.rs"
 
 [dependencies]
-apt-pkg-native = "0.3.1" # custom patched version
+apt-pkg-native = "0.3.2" # custom patched version
 base64 = "0.12"
 bitflags = "1.2.1"
 bytes = "0.5"
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 5/7] apt: refactor package detail reading into function

[Stefan Reiter]

No functional change intended.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/node/apt.rs | 204 +++++++++++++++++++++++--------------------
 1 file changed, 111 insertions(+), 93 deletions(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index b120eef5..d8a2e824 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -90,101 +90,16 @@ fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(filter: F)
     let mut cache_iter = cache.iter();
 
     loop {
-        let view = match cache_iter.next() {
-            Some(view) => view,
-            None => break
-        };
 
-        let current_version = view.current_version();
-        let candidate_version = view.candidate_version();
-
-        let (current_version, candidate_version) = match (current_version, candidate_version) {
-            (Some(cur), Some(can)) => (cur, can), // package installed and there is an update
-            (Some(cur), None) => (cur.clone(), cur), // package installed and up-to-date
-            (None, Some(_)) => continue, // package could be installed
-            (None, None) => continue, // broken
-        };
-
-        // get additional information via nested APT 'iterators'
-        let mut view_iter = view.versions();
-        while let Some(ver) = view_iter.next() {
-
-            let package = view.name();
-            let version = ver.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();
-
-            let fd = FilterData {
-                installed_version: &current_version,
-                candidate_version: &candidate_version,
-                active_version: &version,
-            };
-
-            if filter(fd) {
-                if let Some(section) = ver.section() {
-                    section_res = section;
+        match cache_iter.next() {
+            Some(view) => {
+                let di = query_detailed_info(&filter, view);
+                if let Some(info) = di {
+                    ret.push(info);
                 }
-
-                if let Some(prio) = ver.priority_type() {
-                    priority_res = prio;
-                }
-
-                // assume every package has only one origin file (not
-                // origin, but origin *file*, for some reason those seem to
-                // be different concepts in APT)
-                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;
-                    }
-
-                    // the package files appear in priority order, meaning
-                    // the one for the candidate version is first - this is fine
-                    // however, as the source package should be the same for all
-                    // versions anyway
-                    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 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;
-                        }
-                    }
-                }
-
-                let info = APTUpdateInfo {
-                    package,
-                    title: short_desc,
-                    arch: view.arch(),
-                    description: long_desc,
-                    change_log_url,
-                    origin: origin_res,
-                    version: candidate_version.clone(),
-                    old_version: current_version.clone(),
-                    priority: priority_res,
-                    section: section_res,
-                };
-                ret.push(info);
+            },
+            None => {
+                break;
             }
         }
     }
@@ -192,6 +107,109 @@ fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(filter: F)
     return ret;
 }
 
+fn query_detailed_info<'a, F, V>(
+    filter: F,
+    view: V,
+) -> Option<APTUpdateInfo>
+where
+    F: Fn(FilterData) -> bool,
+    V: std::ops::Deref<Target = apt_pkg_native::sane::PkgView<'a>>
+{
+    let current_version = view.current_version();
+    let candidate_version = view.candidate_version();
+
+    let (current_version, candidate_version) = match (current_version, candidate_version) {
+        (Some(cur), Some(can)) => (cur, can), // package installed and there is an update
+        (Some(cur), None) => (cur.clone(), cur), // package installed and up-to-date
+        (None, Some(_)) => return None, // package could be installed
+        (None, None) => return None, // broken
+    };
+
+    // get additional information via nested APT 'iterators'
+    let mut view_iter = view.versions();
+    while let Some(ver) = view_iter.next() {
+
+        let package = view.name();
+        let version = ver.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();
+
+        let fd = FilterData {
+            installed_version: &current_version,
+            candidate_version: &candidate_version,
+            active_version: &version,
+        };
+
+        if filter(fd) {
+            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 file (not
+            // origin, but origin *file*, for some reason those seem to
+            // be different concepts in APT)
+            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;
+                }
+
+                // the package files appear in priority order, meaning
+                // the one for the candidate version is first - this is fine
+                // however, as the source package should be the same for all
+                // versions anyway
+                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 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;
+                    }
+                }
+            }
+
+            return Some(APTUpdateInfo {
+                package,
+                title: short_desc,
+                arch: view.arch(),
+                description: long_desc,
+                change_log_url,
+                origin: origin_res,
+                version: candidate_version.clone(),
+                old_version: current_version.clone(),
+                priority: priority_res,
+                section: section_res,
+            });
+        }
+    }
+
+    return None;
+}
+
 #[api(
     input: {
         properties: {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 6/7] fix #2934: list to-be-installed packages in updates

[Stefan Reiter]

As always, libapt is mocking us with complexity, but we can get the
approximate result we want by retrieving dependencies of all
to-be-updated packages and then seeing if they are missing.

If they are, we assume they will be installed.

For this, query_detailed_info is extended to allow reading details for
non-installed packages, and this is also exposed in
list_installed_apt_packages via 'all_versions_for'. This is necessary so
we can retrieve changelogs for such packages.

Note that we cannot retrieve all that information all the time, as
querying details for packages that aren't installed takes a rather long
time.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/node/apt.rs | 116 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 101 insertions(+), 15 deletions(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index d8a2e824..20a738c0 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use apt_pkg_native::Cache;
 use anyhow::{Error, bail};
 use serde_json::{json, Value};
@@ -71,34 +73,83 @@ fn get_changelog_url(
 
 struct FilterData<'a> {
     // this is version info returned by APT
-    installed_version: &'a str,
+    installed_version: Option<&'a str>,
     candidate_version: &'a str,
 
     // this is the version info the filter is supposed to check
     active_version: &'a str,
 }
 
-fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(filter: F)
-    -> Vec<APTUpdateInfo> {
+enum PackagePreSelect {
+    OnlyInstalled,
+    OnlyNew,
+    All,
+}
+
+fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(
+    filter: F,
+    only_versions_for: Option<&str>,
+) -> Vec<APTUpdateInfo> {
 
     let mut ret = Vec::new();
+    let mut depends = HashSet::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();
+    let mut cache_iter = match only_versions_for {
+        Some(name) => cache.find_by_name(name),
+        None => cache.iter()
+    };
 
     loop {
 
         match cache_iter.next() {
             Some(view) => {
-                let di = query_detailed_info(&filter, view);
+                let di = if only_versions_for.is_some() {
+                    query_detailed_info(
+                        PackagePreSelect::All,
+                        &filter,
+                        view,
+                        None
+                    )
+                } else {
+                    query_detailed_info(
+                        PackagePreSelect::OnlyInstalled,
+                        &filter,
+                        view,
+                        Some(&mut depends)
+                    )
+                };
                 if let Some(info) = di {
                     ret.push(info);
                 }
+
+                if only_versions_for.is_some() {
+                    break;
+                }
             },
             None => {
+                drop(cache_iter);
+                // also loop through missing dependencies, as they would be installed
+                for pkg in depends.iter() {
+                    let mut iter = cache.find_by_name(&pkg);
+                    let view = match iter.next() {
+                        Some(view) => view,
+                        None => continue // package not found, ignore
+                    };
+
+                    let di = query_detailed_info(
+                        PackagePreSelect::OnlyNew,
+                        &filter,
+                        view,
+                        None
+                    );
+                    if let Some(info) = di {
+                        ret.push(info);
+                    }
+                }
                 break;
             }
         }
@@ -108,8 +159,10 @@ fn list_installed_apt_packages<F: Fn(FilterData) -> bool>(filter: F)
 }
 
 fn query_detailed_info<'a, F, V>(
+    pre_select: PackagePreSelect,
     filter: F,
     view: V,
+    depends: Option<&mut HashSet<String>>,
 ) -> Option<APTUpdateInfo>
 where
     F: Fn(FilterData) -> bool,
@@ -118,11 +171,25 @@ where
     let current_version = view.current_version();
     let candidate_version = view.candidate_version();
 
-    let (current_version, candidate_version) = match (current_version, candidate_version) {
-        (Some(cur), Some(can)) => (cur, can), // package installed and there is an update
-        (Some(cur), None) => (cur.clone(), cur), // package installed and up-to-date
-        (None, Some(_)) => return None, // package could be installed
-        (None, None) => return None, // broken
+    let (current_version, candidate_version) = match pre_select {
+        PackagePreSelect::OnlyInstalled => match (current_version, candidate_version) {
+            (Some(cur), Some(can)) => (Some(cur), can), // package installed and there is an update
+            (Some(cur), None) => (Some(cur.clone()), cur), // package installed and up-to-date
+            (None, Some(_)) => return None, // package could be installed
+            (None, None) => return None, // broken
+        },
+        PackagePreSelect::OnlyNew => match (current_version, candidate_version) {
+            (Some(_), Some(_)) => return None,
+            (Some(_), None) => return None,
+            (None, Some(can)) => (None, can),
+            (None, None) => return None,
+        },
+        PackagePreSelect::All => match (current_version, candidate_version) {
+            (Some(cur), Some(can)) => (Some(cur), can),
+            (Some(cur), None) => (Some(cur.clone()), cur),
+            (None, Some(can)) => (None, can),
+            (None, None) => return None,
+        },
     };
 
     // get additional information via nested APT 'iterators'
@@ -139,7 +206,7 @@ where
         let mut long_desc = "".to_owned();
 
         let fd = FilterData {
-            installed_version: &current_version,
+            installed_version: current_version.as_deref(),
             candidate_version: &candidate_version,
             active_version: &version,
         };
@@ -192,6 +259,22 @@ where
                 }
             }
 
+            if let Some(depends) = depends {
+                let mut dep_iter = ver.dep_iter();
+                loop {
+                    let dep = match dep_iter.next() {
+                        Some(dep) if dep.dep_type() != "Depends" => continue,
+                        Some(dep) => dep,
+                        None => break
+                    };
+
+                    let dep_pkg = dep.target_pkg();
+                    let name = dep_pkg.name();
+
+                    depends.insert(name);
+                }
+            }
+
             return Some(APTUpdateInfo {
                 package,
                 title: short_desc,
@@ -200,7 +283,10 @@ where
                 change_log_url,
                 origin: origin_res,
                 version: candidate_version.clone(),
-                old_version: current_version.clone(),
+                old_version: match current_version {
+                    Some(vers) => vers,
+                    None => "".to_owned()
+                },
                 priority: priority_res,
                 section: section_res,
             });
@@ -229,10 +315,10 @@ where
 )]
 /// List available APT updates
 fn apt_update_available(_param: Value) -> Result<Value, Error> {
-    let all_upgradeable = list_installed_apt_packages(|data|
+    let all_upgradeable = list_installed_apt_packages(|data| {
         data.candidate_version == data.active_version &&
-        data.installed_version != data.candidate_version
-    );
+        data.installed_version != Some(data.candidate_version)
+    }, None);
     Ok(json!(all_upgradeable))
 }
 
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 7/7] apt: add /changelog API call similar to PVE

[Stefan Reiter]

For proxmox packages it works the same way as PVE, by retrieving the
changelog URL and issuing a HTTP GET to it, forwarding the output to the
client. As this is only supposed to be a workaround removed in the
future, a simple block_on is used to avoid async.

For debian packages we can simply call 'apt-get changelog' and forward
it's output.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/node/apt.rs | 63 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 20a738c0..1c5b7db9 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -1,7 +1,7 @@
 use std::collections::HashSet;
 
 use apt_pkg_native::Cache;
-use anyhow::{Error, bail};
+use anyhow::{Error, bail, format_err};
 use serde_json::{json, Value};
 
 use proxmox::{list_subdirs_api_method, const_regex};
@@ -9,6 +9,7 @@ use proxmox::api::{api, RpcEnvironment, RpcEnvironmentType, Permission};
 use proxmox::api::router::{Router, SubdirMap};
 
 use crate::server::WorkerTask;
+use crate::tools::http;
 
 use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY};
 use crate::api2::types::{APTUpdateInfo, NODE_SCHEMA, Userid, UPID_SCHEMA};
@@ -373,7 +374,67 @@ pub fn apt_update_database(
     Ok(upid_str)
 }
 
+#[api(
+    input: {
+        properties: {
+            node: {
+                schema: NODE_SCHEMA,
+            },
+            name: {
+                description: "Package name to get changelog of.",
+                type: String,
+            },
+            version: {
+                description: "Package version to get changelog of. Omit to use candidate version.",
+                type: String,
+                optional: true,
+            },
+        },
+    },
+    returns: {
+        schema: UPID_SCHEMA,
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Retrieve the changelog of the specified package.
+fn apt_get_changelog(
+    param: Value,
+) -> Result<Value, Error> {
+
+    let name = crate::tools::required_string_param(&param, "name")?.to_owned();
+    let version = param["version"].as_str();
+
+    let pkg_info = 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.len() == 0 {
+        bail!("Package '{}' not found", name);
+    }
+
+    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 = crate::tools::runtime::block_on(http::get_string(changelog_url))
+            .map_err(|err| format_err!("Error downloading changelog: {}", err))?;
+        return Ok(json!(changelog));
+    } 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 = crate::tools::run_command(command, None)?;
+        return Ok(json!(output));
+    }
+}
+
 const SUBDIRS: SubdirMap = &[
+    ("changelog", &Router::new().get(&API_METHOD_APT_GET_CHANGELOG)),
     ("update", &Router::new()
         .get(&API_METHOD_APT_UPDATE_AVAILABLE)
         .post(&API_METHOD_APT_UPDATE_DATABASE)
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup 2/7] add tools::http for generic HTTP GET and move HttpsConnector there

[Thomas Lamprecht]

On 21.10.20 11:41, Stefan Reiter wrote:
> ...to avoid having the tools:: module depend on api2.
> 
> The get_string function is based directly on hyper and thus relatively
> simple, not supporting redirects for example.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  src/client/http_client.rs |  75 ++--------------------------
>  src/tools.rs              |   1 +
>  src/tools/http.rs         | 100 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+), 72 deletions(-)
>  create mode 100644 src/tools/http.rs
> 
>

applied this one already, as I need it myself, thanks!

[pbs-devel] applied-series: [PATCH 0/7] apt: add changelog API and fix #2934 (list new packages)

[Fabian Grünbichler]

I dropped the apt-pkg-native version bump (both from this series, and 
the patches to the binding crate) since I dislike bumping versions 
before upstream those (it's very confusing down the line).

added the following as follow-up as well:

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 1c5b7db9..e8d4094b 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -42,7 +42,7 @@ fn get_changelog_url(
         let output = match output.splitn(2, ' ').next() {
             Some(output) => {
                 if output.len() < 2 {
-                    bail!("invalid output (URI part too short) from 'apt-get changelog --print-uris: {}", output)
+                    bail!("invalid output (URI part too short) from 'apt-get changelog --print-uris': {}", output)
                 }
                 output[1..output.len()-1].to_owned()
             },
@@ -421,7 +421,7 @@ fn apt_get_changelog(
     // 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 = crate::tools::runtime::block_on(http::get_string(changelog_url))
-            .map_err(|err| format_err!("Error downloading changelog: {}", err))?;
+            .map_err(|err| format_err!("Error downloading changelog from '{}': {}", changelog_url, err))?;
         return Ok(json!(changelog));
     } else {
         let mut command = std::process::Command::new("apt-get");

On October 21, 2020 11:41 am, Stefan Reiter wrote:
> (Note: This is technically a successor to this series [0], but since I changed a
> lot around and also added onto it, this is not a v2 but a seperate series
> entirely.)
> 
> Bring the "Updates" panel up to par with PVE by adding two features:
> * A /changelog API call
> * A fix for #2934, so new packages that will be installed on upgrade are
>   shown too (i.e. new kernels)
> 
> The former also requires extracting a new 'http' module out of 'api', so we can
> make simple HTTP requests from 'tools'.
> 
> Before that, some cleanup is done. Even with that, the code is not quite a prime
> example of readability, but with the weirdness that is the libapt-pkg lib and
> binding, this was the best I could manage.
> 
> Patch 4 is a dependency bump on a new version of my forked libapt-pkg binding
> (apt-pkg-native(-rs)). As before, the changes I made to that can be found on
> GitHub [1]. Patches 5 and later require the new version to compile.
> 
> 
> [0] https://lists.proxmox.com/pipermail/pbs-devel/2020-July/000045.html
> [1] https://github.com/PiMaker/apt-pkg-native-rs/
> 
> 
> proxmox-backup: Stefan Reiter (7):
>   apt: allow filter to select different package version
>   add tools::http for generic HTTP GET and move HttpsConnector there
>   apt: use 'apt-get changelog --print-uris' in get_changelog_url
>   bump apt-pkg-native dependency to 0.3.2
>   apt: refactor package detail reading into function
>   fix #2934: list to-be-installed packages in updates
>   apt: add /changelog API call similar to PVE
> 
>  Cargo.toml                |   2 +-
>  src/api2/node/apt.rs      | 384 ++++++++++++++++++++++++++++----------
>  src/client/http_client.rs |  75 +-------
>  src/tools.rs              |   1 +
>  src/tools/http.rs         | 100 ++++++++++
>  5 files changed, 386 insertions(+), 176 deletions(-)
>  create mode 100644 src/tools/http.rs
> 
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>