[pve-devel] [PATCH proxmox-offline-mirror 1/2] fix #4445: mirror: subscription: add proxy support

[pve-devel] [PATCH proxmox-offline-mirror 1/2] fix #4445: mirror: subscription: add proxy support

[Lukas Wagner]

This commit adds support for HTTP proxies, configurable via the
ALL_PROXY environment variable.

For example:
  $ ALL_PROXY="localhost:3128" proxmox-offline-mirror mirror <...>

Note: `ureq` seems to use HTTP CONNECT for *all* connections, including
HTTP on port 80. Proxies need to be configured to allow that - Squid by
default allows CONNECT only for HTTPS on port 443.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/mirror.rs       |  3 ++-
 src/subscription.rs | 13 +++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/mirror.rs b/src/mirror.rs
index 86974b7..787e223 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -9,7 +9,7 @@ use anyhow::{bail, format_err, Error};
 use flate2::bufread::GzDecoder;
 use globset::{Glob, GlobSet, GlobSetBuilder};
 use nix::libc;
-use proxmox_http::{client::sync::Client, HttpClient, HttpOptions};
+use proxmox_http::{client::sync::Client, HttpClient, HttpOptions, ProxyConfig};
 use proxmox_sys::fs::file_get_contents;
 
 use crate::{
@@ -64,6 +64,7 @@ impl TryInto<ParsedMirrorConfig> for MirrorConfig {
 
         let options = HttpOptions {
             user_agent: Some("proxmox-offline-mirror 0.1".to_string()),
+            proxy_config: ProxyConfig::from_proxy_env()?,
             ..Default::default()
         }; // TODO actually read version ;)
 
diff --git a/src/subscription.rs b/src/subscription.rs
index 42794fe..d186a95 100644
--- a/src/subscription.rs
+++ b/src/subscription.rs
@@ -1,7 +1,7 @@
 use anyhow::{bail, format_err, Error};
 
 use proxmox_http::client::sync::Client;
-use proxmox_http::{HttpClient, HttpOptions};
+use proxmox_http::{HttpClient, HttpOptions, ProxyConfig};
 use proxmox_subscription::SubscriptionStatus;
 use proxmox_subscription::{
     sign::{SignRequest, SignedResponse},
@@ -15,12 +15,13 @@ const PRODUCT_URL: &str = "-";
 // TODO add version?
 const USER_AGENT: &str = "proxmox-offline-mirror";
 
-fn client() -> Client {
+fn client() -> Result<Client, Error> {
     let options = HttpOptions {
         user_agent: Some(USER_AGENT.to_string()),
+        proxy_config: ProxyConfig::from_proxy_env()?,
         ..Default::default()
     };
-    Client::new(options)
+    Ok(Client::new(options))
 }
 
 pub fn extract_mirror_key(keys: &[SubscriptionKey]) -> Result<SubscriptionKey, Error> {
@@ -61,7 +62,7 @@ pub fn refresh_offline_keys(
             key.key.clone(),
             key.server_id.clone(),
             PRODUCT_URL.to_string(),
-            client(),
+            client()?,
         ) {
             errors = true;
             eprintln!("Failed to refresh subscription key {} - {}", key.key, err);
@@ -74,7 +75,7 @@ pub fn refresh_offline_keys(
         mirror_key: mirror_key.into(),
         blobs: offline_keys.into_iter().map(|k| k.into()).collect(),
     };
-    let res = client().post(
+    let res = client()?.post(
         "https://shop.proxmox.com/proxmox-subscription/sign",
         Some(serde_json::to_vec(&request)?.as_slice()),
         Some("text/json"),
@@ -98,6 +99,6 @@ pub fn refresh_mirror_key(mirror_key: SubscriptionKey) -> Result<SubscriptionInf
         mirror_key.key,
         mirror_key.server_id,
         PRODUCT_URL.to_string(),
-        client(),
+        client()?,
     )
 }
-- 
2.30.2

[pve-devel] [PATCH proxmox-offline-mirror 2/2] docs: document `ALL_PROXY` environment variable

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 docs/command-syntax.rst |  3 +++
 docs/offline-mirror.rst | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/docs/command-syntax.rst b/docs/command-syntax.rst
index bd91d46..272e6bd 100644
--- a/docs/command-syntax.rst
+++ b/docs/command-syntax.rst
@@ -4,6 +4,9 @@ Command Syntax
 ``proxmox-offline-mirror``
 --------------------------
 
+For supported environment variables please refer to
+:ref:`env_vars` .
+
 .. include:: proxmox-offline-mirror/synopsis.rst
 
 
diff --git a/docs/offline-mirror.rst b/docs/offline-mirror.rst
index fde27ed..aa4780e 100644
--- a/docs/offline-mirror.rst
+++ b/docs/offline-mirror.rst
@@ -86,3 +86,27 @@ Space Management
 After removing a snapshot with ``proxmox-offline-mirror mirror snapshot remove``, a
 ``proxmox-offline-mirror mirror gc`` invocation is needed to trigger the garbage collection to
 actually remove any contents from the underlying hard link pool that are no longer needed.
+
+.. _env_vars :
+
+Environment Variables
+---------------------
+
+
+``ALL_PROXY``
+  When set, the client uses the specified HTTP proxy for all connections to the
+  backup server. Currently only HTTP proxies are supported. Valid proxy
+  configurations have the following format:
+  `[http://][user:password@]<host>[:port]`. Default `port` is 1080, if not
+  otherwise specified.
+
+.. Note:: The proxy server must allow ``HTTP CONNECT`` for all ports that are used
+   to connect to mirrors (e.g. port 80 for HTTP mirrors). For Squid,
+   the appropriate configuration parameter is ``http_access allow CONNECT <acl>``
+   (http://www.squid-cache.org/Doc/config/http_access/). By default, Squid only
+   allows ``HTTP CONNECT`` for port 443.
+
+
+.. Note:: Passwords must be valid UTF-8 and may not contain newlines. For your
+   convenience, Proxmox Backup Server only uses the first line as password, so
+   you can add arbitrary comments after the first newline.
-- 
2.30.2

[pve-devel] applied-series: [PATCH proxmox-offline-mirror 1/2] fix #4445: mirror: subscription: add proxy support

[Fabian Grünbichler]

On January 19, 2023 11:40 am, Lukas Wagner wrote:
> This commit adds support for HTTP proxies, configurable via the
> ALL_PROXY environment variable.
> 
> For example:
>   $ ALL_PROXY="localhost:3128" proxmox-offline-mirror mirror <...>
> 
> Note: `ureq` seems to use HTTP CONNECT for *all* connections, including
> HTTP on port 80. Proxies need to be configured to allow that - Squid by
> default allows CONNECT only for HTTPS on port 443.

I wonder how much work it would be to change that upstream? IIRC we also
contributed HTTPS_PROXY support to ureq, and this is not the only place where we
use it and want proper proxy support..

> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  src/mirror.rs       |  3 ++-
>  src/subscription.rs | 13 +++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mirror.rs b/src/mirror.rs
> index 86974b7..787e223 100644
> --- a/src/mirror.rs
> +++ b/src/mirror.rs
> @@ -9,7 +9,7 @@ use anyhow::{bail, format_err, Error};
>  use flate2::bufread::GzDecoder;
>  use globset::{Glob, GlobSet, GlobSetBuilder};
>  use nix::libc;
> -use proxmox_http::{client::sync::Client, HttpClient, HttpOptions};
> +use proxmox_http::{client::sync::Client, HttpClient, HttpOptions, ProxyConfig};
>  use proxmox_sys::fs::file_get_contents;
>  
>  use crate::{
> @@ -64,6 +64,7 @@ impl TryInto<ParsedMirrorConfig> for MirrorConfig {
>  
>          let options = HttpOptions {
>              user_agent: Some("proxmox-offline-mirror 0.1".to_string()),
> +            proxy_config: ProxyConfig::from_proxy_env()?,
>              ..Default::default()
>          }; // TODO actually read version ;)
>  
> diff --git a/src/subscription.rs b/src/subscription.rs
> index 42794fe..d186a95 100644
> --- a/src/subscription.rs
> +++ b/src/subscription.rs
> @@ -1,7 +1,7 @@
>  use anyhow::{bail, format_err, Error};
>  
>  use proxmox_http::client::sync::Client;
> -use proxmox_http::{HttpClient, HttpOptions};
> +use proxmox_http::{HttpClient, HttpOptions, ProxyConfig};
>  use proxmox_subscription::SubscriptionStatus;
>  use proxmox_subscription::{
>      sign::{SignRequest, SignedResponse},
> @@ -15,12 +15,13 @@ const PRODUCT_URL: &str = "-";
>  // TODO add version?
>  const USER_AGENT: &str = "proxmox-offline-mirror";
>  
> -fn client() -> Client {
> +fn client() -> Result<Client, Error> {
>      let options = HttpOptions {
>          user_agent: Some(USER_AGENT.to_string()),
> +        proxy_config: ProxyConfig::from_proxy_env()?,
>          ..Default::default()
>      };
> -    Client::new(options)
> +    Ok(Client::new(options))
>  }
>  
>  pub fn extract_mirror_key(keys: &[SubscriptionKey]) -> Result<SubscriptionKey, Error> {
> @@ -61,7 +62,7 @@ pub fn refresh_offline_keys(
>              key.key.clone(),
>              key.server_id.clone(),
>              PRODUCT_URL.to_string(),
> -            client(),
> +            client()?,
>          ) {
>              errors = true;
>              eprintln!("Failed to refresh subscription key {} - {}", key.key, err);
> @@ -74,7 +75,7 @@ pub fn refresh_offline_keys(
>          mirror_key: mirror_key.into(),
>          blobs: offline_keys.into_iter().map(|k| k.into()).collect(),
>      };
> -    let res = client().post(
> +    let res = client()?.post(
>          "https://shop.proxmox.com/proxmox-subscription/sign",
>          Some(serde_json::to_vec(&request)?.as_slice()),
>          Some("text/json"),
> @@ -98,6 +99,6 @@ pub fn refresh_mirror_key(mirror_key: SubscriptionKey) -> Result<SubscriptionInf
>          mirror_key.key,
>          mirror_key.server_id,
>          PRODUCT_URL.to_string(),
> -        client(),
> +        client()?,
>      )
>  }
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] applied-series: [PATCH proxmox-offline-mirror 1/2] fix #4445: mirror: subscription: add proxy support

[Lukas Wagner]

Thanks for reviewing my patch!

On 1/27/23 11:25, Fabian Grünbichler wrote:
> I wonder how much work it would be to change that upstream? IIRC we also
> contributed HTTPS_PROXY support to ureq, and this is not the only place where we
> use it and want proper proxy support..
> 

I'll check once I find the time for it.


-- 
- Lukas

Re: [pve-devel] applied-series: [PATCH proxmox-offline-mirror 1/2] fix #4445: mirror: subscription: add proxy support

[Lukas Wagner]

On 1/27/23 12:04, Lukas Wagner wrote:
> 
> I'll check once I find the time for it.

For the record:
https://github.com/algesten/ureq/pull/587
  

-- 
- Lukas