From 23d746e396ad5f2eb56d31829840eae1a7a01e48 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Fri, 6 Nov 2026 17:54:00 -0700 Subject: [PATCH] Tasks: a renewal created over the API replaces the domain's queued one Saving a domain already clears its queued certificate renewals before queuing a new one (#290), because each renewal queues the next and two would keep renewing for good. Creating an AcmeRenewal task, or a DNS task that renews the certificate after, through x:Task/set skipped that, so a client could start a second chain. It now clears them the same way, and refuses a second renewal for the same domain in one request. The test also covers telling a queued renewal to run now by setting its status to Pending, which the console's Order again uses (inbuxa-admin#167). --- crates/jmap/src/registry/mapping/task.rs | 35 +++++++++++-- tests/src/automation/acme.rs | 65 +++++++++++++++++++++++- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/crates/jmap/src/registry/mapping/task.rs b/crates/jmap/src/registry/mapping/task.rs index 8c7c2844..9dd00f16 100644 --- a/crates/jmap/src/registry/mapping/task.rs +++ b/crates/jmap/src/registry/mapping/task.rs @@ -25,7 +25,7 @@ use registry::{ schema::{ enums::{TaskStatusType, TaskType}, prelude::Property, - structs::Task, + structs::{Task, TaskDnsManagement}, }, types::{ EnumImpl, ObjectImpl, @@ -33,8 +33,8 @@ use registry::{ index::{IndexBuilder, IndexKey}, }, }; -use services::task_manager::lock::TaskLockManager; -use std::str::FromStr; +use services::task_manager::{domain_check::clear_queued_renewals, lock::TaskLockManager}; +use std::{collections::HashSet, str::FromStr}; use store::{ IterateParams, SerializeInfallible, U64_LEN, ValueKey, registry::RegistryFilterOp, @@ -48,6 +48,7 @@ pub(crate) async fn task_set( ) -> trc::Result> { let mut batch = BatchBuilder::new(); let mut locked_tasks = Vec::new(); + let mut renewing_domains = HashSet::new(); // Process creations 'outer: for (id, value) in set.create.drain() { @@ -125,6 +126,34 @@ pub(crate) async fn task_set( } } + // inbuxa: a renewal created here replaces the domain's queued + // one, the way saving the domain does, or both would keep + // renewing from here on (#290) + let renews = match &task { + Task::AcmeRenewal(task) => Some(task.domain_id), + Task::DnsManagement(TaskDnsManagement { + domain_id, + on_success_renew_certificate: true, + .. + }) => Some(*domain_id), + _ => None, + }; + if let Some(domain_id) = renews { + if !renewing_domains.insert(domain_id) { + set.response.not_created.append( + id, + SetError::invalid_properties() + .with_property(Property::DomainId) + .with_description( + "Only one certificate renewal per domain can be created at a time", + ), + ); + continue 'outer; + } + locked_tasks + .extend(clear_queued_renewals(set.server, domain_id, &mut batch).await?); + } + let task_id = set.server.registry().assign_id(); batch.schedule_task_with_id(task_id, task).commit_point(); set.response.created(id, task_id); diff --git a/tests/src/automation/acme.rs b/tests/src/automation/acme.rs index 8a48463b..ab3597f7 100644 --- a/tests/src/automation/acme.rs +++ b/tests/src/automation/acme.rs @@ -7,7 +7,7 @@ * Modified by Coffey Labs in 2026 for INBUXA. */ -use crate::utils::server::TestServer; +use crate::utils::{jmap::JmapUtils, server::TestServer}; use ahash::AHashSet; use common::network::dns::update::DNS_RECORDS; use dns_update::DnsRecord; @@ -21,7 +21,7 @@ use registry::{ CertificateManagementProperties, DkimManagement, DnsManagement, DnsManagementProperties, DnsServer, DnsServerCloudflare, Domain, PublicText, PublicTextValue, SecretKey, SecretKeyValue, SecretText, SecretTextValue, Task, - TaskDomainManagement, + TaskDomainManagement, TaskStatus, }, }, types::{datetime::UTCDateTime, id::ObjectId, map::Map}, @@ -575,6 +575,67 @@ pub async fn test(test: &TestServer) { test.wait_for_tasks_skip_not_due().await; assert_one_renewal(account, names_domain_id).await; + // inbuxa: a renewal created over the API replaces the queued one as well, + // and only one can be created per domain in a request + let renewal = || { + Task::AcmeRenewal(TaskDomainManagement { + domain_id: names_domain_id, + status: TaskStatus::now(), + }) + }; + account.registry_create_object(renewal()).await; + assert_one_renewal(account, names_domain_id).await; + test.wait_for_tasks_skip_not_due().await; + assert_one_renewal(account, names_domain_id).await; + let response = account.registry_create([renewal(), renewal()]).await; + response.created_id(0); + response + .not_created(1) + .to_set_error() + .assert_type(SetErrorType::InvalidProperties) + .assert_description_contains("Only one certificate renewal per domain"); + test.wait_for_tasks_skip_not_due().await; + assert_one_renewal(account, names_domain_id).await; + + // A queued renewal can be told to run now, as the console's Order again + // does. The certificate is still valid, so it only reschedules itself. + let (renewal_id, queued) = account + .registry_get_all::() + .await + .into_iter() + .find(|(_, task)| { + matches!(task, Task::AcmeRenewal(TaskDomainManagement { domain_id, .. }) if *domain_id == names_domain_id) + }) + .unwrap(); + let due_before = queued.due_timestamp(); + let now_text = UTCDateTime::now().to_string(); + account + .registry_update_object( + ObjectType::Task, + renewal_id, + json!({ "status": { "@type": "Pending", "due": now_text } }), + ) + .await; + test.wait_for_tasks_skip_not_due().await; + assert_one_renewal(account, names_domain_id).await; + let due_after = account + .registry_get_all::() + .await + .into_iter() + .find_map(|(_, task)| match task { + Task::AcmeRenewal(TaskDomainManagement { domain_id, .. }) + if domain_id == names_domain_id => + { + Some(task.due_timestamp()) + } + _ => None, + }) + .unwrap(); + assert_eq!( + due_after, due_before, + "the run now should reschedule for when the certificate falls due" + ); + // DNS-PERSIST-01 only allows a wildcard if its record says so. Going from // named hosts to the wildcard needs that record republished first. let names_persist_id = account -- 2.54.0