[pbs-devel] [RFC 0/4] Add GET /apt/changelog API call

[pbs-devel] [RFC 0/4] Add GET /apt/changelog API call

[Stefan Reiter]

Allows a user to retrieve changelogs for individual packages. Compatible with
the widget-toolkit's APT panel, works the same as in PVE.

Sent as RFC since it contains quite a bit of 'workaround' code. The initial idea
was to replicate the way PVE does it right now, but that turned out to be very
fragile with all the hardcoded URLs and whatnot (indicated by the fact that is
indeed currently broken in PVE, I'll look at that soon).

So instead, after Discussion with Fabian, I opted to simply use 'apt-get
changelog' - this works well for Debian packages, but not for ours, since our
repo doesn't contain the changelogs at the place APT expects.

These patches therefore implement a rather weird in-between solution, where a
HTTP GET to a fixed URL is used for our packages, and 'apt-get changelog
[--print-uris]' is used for Debian ones.

If we can make our repos work with 'apt-get changelog' soon, or don't mind the
changelogs for our packages unavailable in PBS for a while, I can send a v2
dropping the 'tools::http' module and using 'apt-get' everywhere, without any
new FIXMEs. If not, consider using it like this and I'll rework the code once
our Repos support it.

@Fabian: the idea that you can pre-download packages and get changelogs that way
appears moot, since as it turns out, packages only downloaded with 'apt-get -d
install' are not checked by 'apt-get changelog', returning the same 404 error as
before.


proxmox-backup: Stefan Reiter (4):
  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
  apt: add /changelog API call similar to PVE

 src/api2/node/apt.rs      | 157 ++++++++++++++++++++++++++------------
 src/client/http_client.rs |  64 +---------------
 src/tools.rs              |   1 +
 src/tools/http.rs         |  90 ++++++++++++++++++++++
 4 files changed, 202 insertions(+), 110 deletions(-)
 create mode 100644 src/tools/http.rs

-- 
2.20.1

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

[Stefan Reiter]

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

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

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 37bf48d4..f2c4ae7b 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -71,7 +71,7 @@ fn get_changelog_url(
     bail!("unknown origin ({}) or component ({})", origin, component)
 }
 
-fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
+fn list_installed_apt_packages<F: Fn(&str, &str, &str, &str) -> bool>(filter: F)
     -> Vec<APTUpdateInfo> {
 
     let mut ret = Vec::new();
@@ -99,8 +99,12 @@ fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
             None => current_version.clone()
         };
 
-        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,9 +112,7 @@ 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 filter(&package, &current_version, &candidate_version, &version) {
                 if ver.version() == candidate_version {
                     if let Some(section) = ver.section() {
                         section_res = section;
@@ -159,23 +161,21 @@ fn list_installed_apt_packages<F: Fn(&str, &str, &str) -> bool>(filter: F)
                         }
                     }
 
-                    break;
+                    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);
                 }
             }
-
-            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);
         }
     }
 
@@ -201,7 +201,7 @@ 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);
+    let ret = list_installed_apt_packages(|_pkg, cur_ver, can_ver, act_ver| cur_ver != can_ver && can_ver == act_ver);
     Ok(json!(ret))
 }
 
-- 
2.20.1

[pbs-devel] [RFC proxmox-backup 2/4] 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 | 64 +---------------------------
 src/tools.rs              |  1 +
 src/tools/http.rs         | 90 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 62 deletions(-)
 create mode 100644 src/tools/http.rs

diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index a930ea56..ee0cd623 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -1,5 +1,4 @@
 use std::io::Write;
-use std::task::{Context, Poll};
 use std::sync::{Arc, Mutex};
 
 use chrono::Utc;
@@ -24,8 +23,7 @@ use proxmox::{
 };
 
 use super::pipe_to_stream::PipeToSendStream;
-use crate::tools::async_io::EitherStream;
-use crate::tools::{self, BroadcastFuture, DEFAULT_ENCODE_SET};
+use crate::tools::{self, BroadcastFuture, DEFAULT_ENCODE_SET, http::HttpsConnector};
 
 #[derive(Clone)]
 pub struct AuthInfo {
@@ -286,7 +284,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.set_recv_buffer_size(Some(1024*1024)); //important for h2 download performance!
         httpc.enforce_http(false); // we want https...
@@ -861,61 +859,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?;
-            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 44db796d..c99f5120 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -35,6 +35,7 @@ pub mod timer;
 pub mod statistics;
 pub mod systemd;
 pub mod nom;
+pub mod http;
 
 mod wrapped_reader_stream;
 pub use wrapped_reader_stream::*;
diff --git a/src/tools/http.rs b/src/tools/http.rs
new file mode 100644
index 00000000..b4b1424c
--- /dev/null
+++ b/src/tools/http.rs
@@ -0,0 +1,90 @@
+use anyhow::{Error, format_err, bail};
+use lazy_static::lazy_static;
+use std::task::{Context, Poll};
+
+use hyper::{Uri, Body};
+use hyper::client::{Client, HttpConnector};
+use openssl::ssl::{SslConnector, SslMethod};
+use futures::*;
+
+use crate::tools::async_io::EitherStream;
+
+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?;
+            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] [RFC proxmox-backup 3/4] 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 f2c4ae7b..e47698d1 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) {
@@ -147,14 +145,12 @@ fn list_installed_apt_packages<F: Fn(&str, &str, &str, &str) -> 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,
-                                &candidate_version, &source_ver, &origin_res, &component);
+                            let url = get_changelog_url(&package, &filename,
+                                &candidate_version, &origin_res, &component);
                             if let Ok(url) = url {
                                 change_log_url = url;
                             }
-- 
2.20.1

[pbs-devel] [RFC proxmox-backup 4/4] 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 | 67 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index e47698d1..c7f93466 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -1,5 +1,5 @@
 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};
@@ -7,6 +7,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, UPID_SCHEMA};
@@ -252,7 +253,71 @@ 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(|pkg, _, can_ver, ver| {
+        if pkg == name {
+            match version {
+                Some(v) => v == ver,
+                None => ver == can_ver
+            }
+        } else {
+            false
+        }
+    });
+
+    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

Re: [pbs-devel] [RFC 0/4] Add GET /apt/changelog API call

[Stefan Reiter]

Long-buried ping...

I think we were waiting on some progress for updating our internal repos 
so we can use 'apt-get changelog' there too and don't require the 
workaround with HTTP retrieval. Any update?

(Definitely needs a rebase in any case)

On 7/28/20 10:58 AM, Stefan Reiter wrote:
> Allows a user to retrieve changelogs for individual packages. Compatible with
> the widget-toolkit's APT panel, works the same as in PVE.
> 
> Sent as RFC since it contains quite a bit of 'workaround' code. The initial idea
> was to replicate the way PVE does it right now, but that turned out to be very
> fragile with all the hardcoded URLs and whatnot (indicated by the fact that is
> indeed currently broken in PVE, I'll look at that soon).
> 
> So instead, after Discussion with Fabian, I opted to simply use 'apt-get
> changelog' - this works well for Debian packages, but not for ours, since our
> repo doesn't contain the changelogs at the place APT expects.
> 
> These patches therefore implement a rather weird in-between solution, where a
> HTTP GET to a fixed URL is used for our packages, and 'apt-get changelog
> [--print-uris]' is used for Debian ones.
> 
> If we can make our repos work with 'apt-get changelog' soon, or don't mind the
> changelogs for our packages unavailable in PBS for a while, I can send a v2
> dropping the 'tools::http' module and using 'apt-get' everywhere, without any
> new FIXMEs. If not, consider using it like this and I'll rework the code once
> our Repos support it.
> 
> @Fabian: the idea that you can pre-download packages and get changelogs that way
> appears moot, since as it turns out, packages only downloaded with 'apt-get -d
> install' are not checked by 'apt-get changelog', returning the same 404 error as
> before.
> 
> 
> proxmox-backup: Stefan Reiter (4):
>    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
>    apt: add /changelog API call similar to PVE
> 
>   src/api2/node/apt.rs      | 157 ++++++++++++++++++++++++++------------
>   src/client/http_client.rs |  64 +---------------
>   src/tools.rs              |   1 +
>   src/tools/http.rs         |  90 ++++++++++++++++++++++
>   4 files changed, 202 insertions(+), 110 deletions(-)
>   create mode 100644 src/tools/http.rs
>