[pbs-devel] [PATCH proxmox-backup v2 0/3] acme: add sleep for dns record propagation

[pbs-devel] [PATCH proxmox-backup v2 0/3] acme: add sleep for dns record propagation

[Stoiko Ivanov]

changes v1 -> v2 (based on Thomas' feedback - huge thx!):
* moved the sleep to only run for setup not for teardown
* added a check to only sleep and print the message if the delay is > 0
  for consistency, and to be able to skip it (e.g. for a simple
  powerdns-setup)
* while looking through the code found a small glitch in an error
  message (the fix is in patch 1/3)

forgot to mention that the patches were developed and tested on top of
the current stable-1 branch (they apply cleanly on master, but I did not
explicitly test on master)

cover-letter for v1:
The issue was brought up in our community forum:
https://forum.proxmox.com/threads/acme-configuration-via-gui.91381/

While I could not directly reproduce it (writing to a pdns-database is
faster than waiting on cloud-flare to propagate their records) I
compared the task-log to the one from PMG.

The use of tokio::time::sleep was taken from the other delays in
api2::node::certificate, for the rest I mostly trusted the compiler
warnings and rustfmt.


Stoiko Ivanov (3):
  acme: plugin: fix error message
  config: acme: make validation_delay crate public
  fix #3496: acme: plugin: add sleep for dns propagation

 src/acme/plugin.rs        | 19 +++++++++++++++++--
 src/config/acme/plugin.rs |  2 +-
 2 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup v2 1/3] acme: plugin: fix error message

[Stoiko Ivanov]

extract_challenge is used by both dns-01 and http-01 challenges.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/acme/plugin.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/acme/plugin.rs b/src/acme/plugin.rs
index 5e0e547a..0269b9ad 100644
--- a/src/acme/plugin.rs
+++ b/src/acme/plugin.rs
@@ -68,7 +68,7 @@ fn extract_challenge<'a>(
         .challenges
         .iter()
         .find(|ch| ch.ty == ty)
-        .ok_or_else(|| format_err!("no supported challenge type (dns-01) found"))
+        .ok_or_else(|| format_err!("no supported challenge type ({}) found", ty))
 }
 
 async fn pipe_to_tasklog<T: AsyncRead + Unpin>(
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup v2 2/3] config: acme: make validation_delay crate public

[Stoiko Ivanov]

we need the setting in acme::plugin.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/config/acme/plugin.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index 759b9a33..2e22ab77 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -72,7 +72,7 @@ pub struct DnsPluginCore {
     ///
     /// Allows to cope with long TTL of DNS records.
     #[serde(skip_serializing_if = "Option::is_none", default)]
-    validation_delay: Option<u32>,
+    pub(crate) validation_delay: Option<u32>,
 
     /// Flag to disable the config.
     #[serde(skip_serializing_if = "Option::is_none", default)]
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup v2 3/3] fix #3496: acme: plugin: add sleep for dns propagation

[Stoiko Ivanov]

the dns plugin config allow for a specified amount of time to wait for
the TXT record to be set and propagated through DNS.

This patch adds a sleep for this amount of time.
The log message was taken from the perl implementation in proxmox-acme
for consistency.

Tested with the powerdns plugin in my test setup.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/acme/plugin.rs | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/acme/plugin.rs b/src/acme/plugin.rs
index 0269b9ad..11939cf7 100644
--- a/src/acme/plugin.rs
+++ b/src/acme/plugin.rs
@@ -2,6 +2,7 @@ use std::future::Future;
 use std::pin::Pin;
 use std::process::Stdio;
 use std::sync::Arc;
+use std::time::Duration;
 
 use anyhow::{bail, format_err, Error};
 use hyper::{Body, Request, Response};
@@ -180,7 +181,21 @@ impl AcmePlugin for DnsPlugin {
         domain: &'d AcmeDomain,
         task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
-        Box::pin(self.action(client, authorization, domain, task, "setup"))
+        Box::pin(async move {
+            let result = self
+                .action(client, authorization, domain, task.clone(), "setup")
+                .await;
+            let validation_delay = self.core.validation_delay.unwrap_or(30) as u64;
+
+            if validation_delay > 0 {
+                task.log(format!(
+                    "Sleeping {} seconds to wait for TXT record propagation",
+                    validation_delay
+                ));
+                tokio::time::sleep(Duration::from_secs(validation_delay)).await;
+            }
+            result
+        })
     }
 
     fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
-- 
2.20.1

[pbs-devel] applied-series: [PATCH proxmox-backup v2 0/3] acme: add sleep for dns record propagation

[Thomas Lamprecht]

On 28.06.21 20:41, Stoiko Ivanov wrote:
> changes v1 -> v2 (based on Thomas' feedback - huge thx!):
> * moved the sleep to only run for setup not for teardown
> * added a check to only sleep and print the message if the delay is > 0
>   for consistency, and to be able to skip it (e.g. for a simple
>   powerdns-setup)
> * while looking through the code found a small glitch in an error
>   message (the fix is in patch 1/3)
> 
> forgot to mention that the patches were developed and tested on top of
> the current stable-1 branch (they apply cleanly on master, but I did not
> explicitly test on master)
> 
> cover-letter for v1:
> The issue was brought up in our community forum:
> https://forum.proxmox.com/threads/acme-configuration-via-gui.91381/
> 
> While I could not directly reproduce it (writing to a pdns-database is
> faster than waiting on cloud-flare to propagate their records) I
> compared the task-log to the one from PMG.
> 
> The use of tokio::time::sleep was taken from the other delays in
> api2::node::certificate, for the rest I mostly trusted the compiler
> warnings and rustfmt.
> 
> 
> Stoiko Ivanov (3):
>   acme: plugin: fix error message
>   config: acme: make validation_delay crate public
>   fix #3496: acme: plugin: add sleep for dns propagation
> 
>  src/acme/plugin.rs        | 19 +++++++++++++++++--
>  src/config/acme/plugin.rs |  2 +-
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 



applied series, thanks!