Re: [PATCH installer v3 36/38] fetch-answer: send auto-installer HTTP authorization token if set

["Lukas Wagner" <l.wagner@proxmox.com>]

On Fri Apr 3, 2026 at 6:54 PM CEST, Christoph Heiss wrote:
> If an authorization token is present in the internal auto-installer
> HTTP configuration, add it as
>
>   Authorization: ProxmoxInstallerToken <token>
>

As mentioned in the other patch, *maybe* this should just be a `Bearer`
token, but no hard feelings.

> header to the POST HTTP request when retrieving the answer.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v2 -> v3:
>   * new patch
>
>  .../src/fetch_plugins/http.rs                 |  8 +++++++
>  proxmox-fetch-answer/src/main.rs              | 22 ++++++++++++-------
>  2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
> index 6508721..121b620 100644
> --- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
> +++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
> @@ -95,6 +95,14 @@ impl FetchFromHTTP {
>              HeaderValue::from_str("application/json, application/toml;q=0.5")?,
>          );
>  
> +        if let Some(token) = &settings.token {
> +            info!("Authentication token provided through ISO.");
> +            headers.insert(
> +                http::header::AUTHORIZATION,
> +                HeaderValue::from_str(&format!("ProxmoxInstallerToken {token}"))?,
> +            );
> +        }
> +
>          let (body, content_type) =
>              http::post(&answer_url, fingerprint.as_deref(), headers, payload)?;
>  
> diff --git a/proxmox-fetch-answer/src/main.rs b/proxmox-fetch-answer/src/main.rs
> index 18b27e7..2e399d1 100644
> --- a/proxmox-fetch-answer/src/main.rs
> +++ b/proxmox-fetch-answer/src/main.rs
> @@ -23,8 +23,13 @@ const CLI_USAGE_HELPTEXT: &str = concat!(
>  
>  Commands:
>    iso         Fetch the builtin answer file from the ISO
> +
>    http        Fetch the answer file via HTTP(S)
> -              Additional parameters: [<http-url>] [<tls-cert-fingerprint>]
> +              Additional parameters: [<http-url>] [<tls-cert-fingerprint>] [<auth-token>]
> +
> +              To provide an authentication token without a certificate fingerprint, pass an
> +              empty string to <tls-cert-fingerprint>.
> +
>    partition   Fetch the answer file from a mountable partition
>                Additional parameters: [<partition-label>]
>  
> @@ -47,18 +52,18 @@ fn fetch_answer(install_settings: &AutoInstSettings) -> Result<String> {
>              let answer_path = PathBuf::from("/cdrom/answer.toml");
>              match fs::read_to_string(answer_path) {
>                  Ok(answer) => return Ok(answer),
> -                Err(err) => info!("Fetching answer file from ISO failed: {err}"),
> +                Err(err) => info!("Fetching answer file from ISO failed: {err:#}"),
>              }
>          }
>          FetchAnswerFrom::Partition => {
>              match FetchFromPartition::get_answer(&install_settings.partition_label) {
>                  Ok(answer) => return Ok(answer),
> -                Err(err) => info!("Fetching answer file from partition failed: {err}"),
> +                Err(err) => info!("Fetching answer file from partition failed: {err:#}"),
>              }
>          }
>          FetchAnswerFrom::Http => match FetchFromHTTP::get_answer(&install_settings.http) {
>              Ok(answer) => return Ok(answer),
> -            Err(err) => info!("Fetching answer file via HTTP failed: {err}"),
> +            Err(err) => info!("Fetching answer file via HTTP failed: {err:#}"),

It's only a pretty minor change, but it seems like these should be split
out, especially since you also touch the CDROM/Partition paths

>          },
>      }
>      bail!("Could not find any answer file!");
> @@ -80,8 +85,8 @@ fn settings_from_cli_args(args: &[String]) -> Result<AutoInstSettings> {
>          FetchAnswerFrom::Iso if args.len() > 2 => {
>              bail!("'iso' mode does not take any additional arguments")
>          }
> -        FetchAnswerFrom::Http if args.len() > 4 => {
> -            bail!("'http' mode takes at most 2 additional arguments")
> +        FetchAnswerFrom::Http if args.len() > 5 => {
> +            bail!("'http' mode takes at most 3 additional arguments")
>          }
>          FetchAnswerFrom::Partition if args.len() > 3 => {
>              bail!("'partition' mode takes at most 1 additional argument")
> @@ -97,8 +102,9 @@ fn settings_from_cli_args(args: &[String]) -> Result<AutoInstSettings> {
>              .cloned()?,
>          http: HttpOptions {
>              url: args.get(2).cloned(),
> -            cert_fingerprint: args.get(3).cloned(),
> -            token: None,
> +            // treat empty value as not existing
> +            cert_fingerprint: args.get(3).cloned().filter(|s| !s.is_empty()),
> +            token: args.get(4).cloned(),
>          },
>      })
>  }


Code-wise this looks okay, maybe split out the logging out changes into
a separate patch.

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>