Re: [pbs-devel] [PATCH proxmox v4 2/4] acme: reduce visibility of Request type

["Max R. Carrara" <m.carrara@proxmox.com>]

On Wed Dec 3, 2025 at 11:22 AM CET, Samuel Rufinatscha wrote:
> Currently, the low-level ACME Request type is publicly exposed, even
> though users are expected to go through AcmeClient and
> proxmox-acme-api handlers. This patch reduces visibility so that
> the Request type and related fields/methods are crate-internal only.
>
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---
>  proxmox-acme/src/account.rs       | 17 ++++++++++-------
>  proxmox-acme/src/async_client.rs  |  2 +-
>  proxmox-acme/src/authorization.rs |  2 +-
>  proxmox-acme/src/client.rs        |  6 +++---
>  proxmox-acme/src/lib.rs           |  4 ----
>  proxmox-acme/src/order.rs         |  2 +-
>  proxmox-acme/src/request.rs       | 12 ++++++------
>  7 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
> index 0bbf0027..081ca986 100644
> --- a/proxmox-acme/src/account.rs
> +++ b/proxmox-acme/src/account.rs
> @@ -92,7 +92,7 @@ impl Account {
>      }
>  
>      /// Prepare a "POST-as-GET" request to fetch data. Low level helper.
> -    pub fn get_request(&self, url: &str, nonce: &str) -> Result<Request, Error> {
> +    pub(crate) fn get_request(&self, url: &str, nonce: &str) -> Result<Request, Error> {
>          let key = PKey::private_key_from_pem(self.private_key.as_bytes())?;
>          let body = serde_json::to_string(&Jws::new_full(
>              &key,
> @@ -112,7 +112,7 @@ impl Account {
>      }
>  
>      /// Prepare a JSON POST request. Low level helper.
> -    pub fn post_request<T: Serialize>(
> +    pub(crate) fn post_request<T: Serialize>(
>          &self,
>          url: &str,
>          nonce: &str,
> @@ -179,7 +179,7 @@ impl Account {
>      /// Prepare a request to update account data.
>      ///
>      /// This is a rather low level interface. You should know what you're doing.
> -    pub fn update_account_request<T: Serialize>(
> +    pub(crate) fn update_account_request<T: Serialize>(

^ Regarding this function ...

>          &self,
>          nonce: &str,
>          data: &T,
> @@ -188,7 +188,10 @@ impl Account {
>      }
>  
>      /// Prepare a request to deactivate this account.
> -    pub fn deactivate_account_request<T: Serialize>(&self, nonce: &str) -> Result<Request, Error> {
> +    pub(crate) fn deactivate_account_request<T: Serialize>(
> +        &self,
> +        nonce: &str,
> +    ) -> Result<Request, Error> {

^ and this one ...

>          self.post_request_raw_payload(
>              &self.location,
>              nonce,
> @@ -220,7 +223,7 @@ impl Account {
>      ///
>      /// This returns a raw `Request` since validation takes some time and the `Authorization`
>      /// object has to be re-queried and its `status` inspected.
> -    pub fn validate_challenge(
> +    pub(crate) fn validate_challenge(

^ as well as this one here, I noticed that they aren't used anywhere in
our code, at least I couldn't find any references to them by grepping
through our sources. Since they're not used at all, we could just remove
them entirely here, IMO. If it's not used, there's not really any point
in keeping those methods around—and as you mentioned, users should be
using `AcmeClient` and `proxmox-acme-api` handlers anyway.

Note that `post_request_raw_payload()` then also becomes redundant,
since its used in `validate_challenge()` and
`deactivate_account_request()`.

>          &self,
>          authorization: &Authorization,
>          challenge_index: usize,
> @@ -274,7 +277,7 @@ pub struct CertificateRevocation<'a> {
>  
>  impl CertificateRevocation<'_> {
>      /// Create the revocation request using the specified nonce for the given directory.
> -    pub fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
> +    pub(crate) fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
>          let revoke_cert = directory.data.revoke_cert.as_ref().ok_or_else(|| {
>              Error::Custom("no 'revokeCert' URL specified by provider".to_string())
>          })?;
> @@ -364,7 +367,7 @@ impl AccountCreator {
>      /// the resulting request.
>      /// Changing the private key between using the request and passing the response to
>      /// [`response`](AccountCreator::response()) will render the account unusable!
> -    pub fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
> +    pub(crate) fn request(&self, directory: &Directory, nonce: &str) -> Result<Request, Error> {
>          let key = self.key.as_deref().ok_or(Error::MissingKey)?;
>          let url = directory.new_account_url().ok_or_else(|| {
>              Error::Custom("no 'newAccount' URL specified by provider".to_string())
> diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
> index dc755fb9..2ff3ba22 100644
> --- a/proxmox-acme/src/async_client.rs
> +++ b/proxmox-acme/src/async_client.rs
> @@ -10,7 +10,7 @@ use proxmox_http::{client::Client, Body};
>  
>  use crate::account::AccountCreator;
>  use crate::order::{Order, OrderData};
> -use crate::Request as AcmeRequest;
> +use crate::request::Request as AcmeRequest;
>  use crate::{Account, Authorization, Challenge, Directory, Error, ErrorResponse};
>  
>  /// A non-blocking Acme client using tokio/hyper.
> diff --git a/proxmox-acme/src/authorization.rs b/proxmox-acme/src/authorization.rs
> index 28bc1b4b..765714fc 100644
> --- a/proxmox-acme/src/authorization.rs
> +++ b/proxmox-acme/src/authorization.rs
> @@ -145,7 +145,7 @@ pub struct GetAuthorization {
>      /// this is guaranteed to be `Some`.
>      ///
>      /// The response should be passed to the the [`response`](GetAuthorization::response()) method.
> -    pub request: Option<Request>,
> +    pub(crate) request: Option<Request>,
>  }
>  
>  impl GetAuthorization {
> diff --git a/proxmox-acme/src/client.rs b/proxmox-acme/src/client.rs
> index 931f7245..5c812567 100644
> --- a/proxmox-acme/src/client.rs
> +++ b/proxmox-acme/src/client.rs
> @@ -7,8 +7,8 @@ use serde::{Deserialize, Serialize};
>  use crate::b64u;
>  use crate::error;
>  use crate::order::OrderData;
> -use crate::request::ErrorResponse;
> -use crate::{Account, Authorization, Challenge, Directory, Error, Order, Request};
> +use crate::request::{ErrorResponse, Request};
> +use crate::{Account, Authorization, Challenge, Directory, Error, Order};
>  
>  macro_rules! format_err {
>      ($($fmt:tt)*) => { Error::Client(format!($($fmt)*)) };
> @@ -564,7 +564,7 @@ impl Client {
>      }
>  
>      /// Low-level API to run an n API request. This automatically updates the current nonce!
> -    pub fn run_request(&mut self, request: Request) -> Result<HttpResponse, Error> {
> +    pub(crate) fn run_request(&mut self, request: Request) -> Result<HttpResponse, Error> {
>          self.inner.run_request(request)
>      }
>  
> diff --git a/proxmox-acme/src/lib.rs b/proxmox-acme/src/lib.rs
> index df722629..6722030c 100644
> --- a/proxmox-acme/src/lib.rs
> +++ b/proxmox-acme/src/lib.rs
> @@ -66,10 +66,6 @@ pub use error::Error;
>  #[doc(inline)]
>  pub use order::Order;
>  
> -#[cfg(feature = "impl")]
> -#[doc(inline)]
> -pub use request::Request;
> -
>  // we don't inline these:
>  #[cfg(feature = "impl")]
>  pub use order::NewOrder;
> diff --git a/proxmox-acme/src/order.rs b/proxmox-acme/src/order.rs
> index b6551004..432a81a4 100644
> --- a/proxmox-acme/src/order.rs
> +++ b/proxmox-acme/src/order.rs
> @@ -153,7 +153,7 @@ pub struct NewOrder {
>      //order: OrderData,
>      /// The request to execute to place the order. When creating a [`NewOrder`] via
>      /// [`Account::new_order`](crate::Account::new_order) this is guaranteed to be `Some`.
> -    pub request: Option<Request>,
> +    pub(crate) request: Option<Request>,
>  }
>  
>  impl NewOrder {
> diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
> index 78a90913..dadfc5af 100644
> --- a/proxmox-acme/src/request.rs
> +++ b/proxmox-acme/src/request.rs
> @@ -4,21 +4,21 @@ pub(crate) const JSON_CONTENT_TYPE: &str = "application/jose+json";
>  pub(crate) const CREATED: u16 = 201;
>  
>  /// A request which should be performed on the ACME provider.
> -pub struct Request {
> +pub(crate) struct Request {
>      /// The complete URL to send the request to.
> -    pub url: String,
> +    pub(crate) url: String,
>  
>      /// The HTTP method name to use.
> -    pub method: &'static str,
> +    pub(crate) method: &'static str,
>  
>      /// The `Content-Type` header to pass along.
> -    pub content_type: &'static str,
> +    pub(crate) content_type: &'static str,
>  
>      /// The body to pass along with request, or an empty string.
> -    pub body: String,
> +    pub(crate) body: String,
>  
>      /// The expected status code a compliant ACME provider will return on success.
> -    pub expected: u16,
> +    pub(crate) expected: u16,
>  }
>  
>  /// An ACME error response contains a specially formatted type string, and can optionally



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel