Integrate the zone signer with the new zone storage

crates/api/src/lib.rs

@@ -374,7 +374,6 @@ pub struct SigningInProgressReport {
     pub rrsig_count: Option<usize>,
     pub rrsig_reused_count: Option<usize>,
     pub rrsig_time: Option<Duration>,
-    pub insertion_time: Option<Duration>,
     pub total_time: Option<Duration>,
     pub threads_used: Option<usize>,
 }
@@ -392,7 +391,6 @@ pub struct SigningFinishedReport {
     pub rrsig_count: usize,
     pub rrsig_reused_count: usize,
     pub rrsig_time: Duration,
-    pub insertion_time: Duration,
     pub total_time: Duration,
     pub threads_used: usize,
     pub finished_at: SystemTime,

crates/cli/src/commands/zone.rs

@@ -916,15 +916,6 @@ impl Progress {
                             rrsig_count / (rrsig_time.as_secs() as usize)
                         );
                     }
-                    if let (Some(rrsig_count), Some(insertion_time)) =
-                        (r.rrsig_count, r.insertion_time)
-                    {
-                        println!(
-                            "  Inserted signatures in {} ({} sig/s)",
-                            format_duration(insertion_time),
-                            rrsig_count / (insertion_time.as_secs() as usize)
-                        );
-                    }
                     if let Some(threads_used) = r.threads_used {
                         println!("  Using {threads_used} threads to generate signatures");
                     }
@@ -961,13 +952,6 @@ impl Progress {
                             .checked_div(r.rrsig_time.as_secs() as usize)
                             .unwrap_or(r.rrsig_count),
                     );
-                    println!(
-                        "  Inserted signatures in {} ({} sig/s)",
-                        format_duration(r.insertion_time),
-                        r.rrsig_count
-                            .checked_div(r.insertion_time.as_secs() as usize)
-                            .unwrap_or(r.rrsig_count),
-                    );
                     println!(
                         "  Took {} in total, using {} threads",
                         format_duration(r.total_time),

src/signer/mod.rs

@@ -5,12 +5,11 @@
 use std::sync::Arc;
 
 use cascade_zonedata::SignedZoneBuilder;
-use tracing::{debug, error};
+use tracing::error;
 
 use crate::{
     center::{Center, halt_zone},
-    manager::record_zone_event,
-    zone::{HistoricalEvent, SigningTrigger, Zone},
+    zone::{HistoricalEvent, SigningTrigger, Zone, ZoneHandle},
 };
 
 pub mod zone;
@@ -30,38 +29,59 @@ pub mod zone;
 async fn sign(
     center: Arc<Center>,
     zone: Arc<Zone>,
-    builder: SignedZoneBuilder,
+    mut builder: SignedZoneBuilder,
     trigger: SigningTrigger,
 ) {
-    match center
-        .signer
-        .join_sign_zone_queue(&center, &zone.name, !builder.have_next_loaded(), trigger)
-        .await
-    {
-        Ok(()) => {}
-        Err(error) if error.is_benign() => {
-            // Ignore this benign case. It was probably caused by dnst keyset
-            // cron triggering resigning before we even signed the first time,
-            // either because the zone was large and slow to load and sign, or
-            // because the unsigned zone was pending review.
-            debug!("Ignoring probably benign failure: {error}");
+    let (status, _permits) = center.signer.wait_to_sign(&zone).await;
+
+    let (result, builder) = tokio::task::spawn_blocking({
+        let center = center.clone();
+        let zone = zone.clone();
+        let status = status.clone();
+        move || {
+            let result = center
+                .signer
+                .sign_zone(&center, &zone, &mut builder, trigger, status);
+            (result, builder)
         }
-        Err(error) => {
-            error!("Signing failed: {error}");
+    })
+    .await
+    .unwrap();
 
-            // TODO: Inline these methods and use a single 'ZoneState' lock.
+    let mut status = status.write().unwrap();
+    let mut state = zone.state.lock().unwrap();
+    let mut handle = ZoneHandle {
+        zone: &zone,
+        state: &mut state,
+        center: &center,
+    };
+    handle.state.signer.ongoing.finish();
 
-            halt_zone(&center, &zone.name, true, &error.to_string());
+    match result {
+        Ok(()) => {
+            let built = builder.finish().unwrap_or_else(|_| unreachable!());
+            handle.storage().finish_sign(built);
+            status.status.finish(true);
+            status.current_action = "Finished".to_string();
+        }
+        Err(error) => {
+            error!("Signing failed: {error}");
+            handle.storage().give_up_sign(builder);
+            status.status.finish(false);
+            status.current_action = "Aborted".to_string();
 
-            record_zone_event(
-                &center,
-                &zone.name,
+            handle.state.record_event(
                 HistoricalEvent::SigningFailed {
                     trigger,
                     reason: error.to_string(),
                 },
                 None, // TODO
             );
+
+            std::mem::drop(state);
+
+            // TODO: Inline.
+            halt_zone(&center, &zone.name, true, &error.to_string());
         }
     }
 }

src/signer/zone.rs

@@ -3,10 +3,11 @@
 use std::{sync::Arc, time::SystemTime};
 
 use cascade_zonedata::SignedZoneBuilder;
+use tracing::info;
 
 use crate::{
     center::Center,
-    util::AbortOnDrop,
+    util::BackgroundTasks,
     zone::{SigningTrigger, Zone, ZoneHandle, ZoneState},
 };
 
@@ -35,36 +36,51 @@ impl SignerZoneHandle<'_> {
     }
 
     /// Enqueue a signing operation for a newly loaded instance of the zone.
+    #[tracing::instrument(
+        level = "trace",
+        skip_all,
+        fields(zone = %self.zone.name)
+    )]
     pub fn enqueue_sign(&mut self, builder: SignedZoneBuilder) {
+        info!("Enqueuing a sign operation");
+
         // A zone can have at most one 'SignedZoneBuilder' at a time. Because
         // we have 'builder', we are guaranteed that no other signing operations
         // are ongoing right now. A re-signing operation may be enqueued, but it
         // has lower priority than this (for now).
 
         assert!(self.state.signer.enqueued_sign.is_none());
-        assert!(self.state.signer.ongoing.is_none());
 
         // TODO: Keep state for a queue of pending (re-)signing operations, so
         // that the number of simultaneous operations can be limited. At the
         // moment, this queue is opaque and is handled within the asynchronous
         // task.
 
-        let handle = tokio::task::spawn(super::sign(
-            self.center.clone(),
-            self.zone.clone(),
-            builder,
-            SigningTrigger::ZoneChangesApproved,
-        ));
-
-        self.state.signer.ongoing = Some(handle.into());
+        let span = tracing::Span::none();
+        self.state.signer.ongoing.spawn(
+            span,
+            super::sign(
+                self.center.clone(),
+                self.zone.clone(),
+                builder,
+                SigningTrigger::ZoneChangesApproved,
+            ),
+        );
     }
 
     /// Enqueue a re-signing operation for the zone.
     ///
     /// ## Panics
     ///
     /// Panics if `keys_changed` and `sigs_need_refresh` are both `false`.
+    #[tracing::instrument(
+        level = "trace",
+        skip_all,
+        fields(zone = %self.zone.name, keys_changed, sigs_need_refresh)
+    )]
     pub fn enqueue_resign(&mut self, keys_changed: bool, sigs_need_refresh: bool) {
+        info!("Enqueuing a re-sign operation");
+
         assert!(
             keys_changed || sigs_need_refresh,
             "a reason for re-signing was not specified"
@@ -93,7 +109,6 @@ impl SignerZoneHandle<'_> {
             // may be enqueued, but it has lower priority than this (for now).
 
             assert!(self.state.signer.enqueued_sign.is_none());
-            assert!(self.state.signer.ongoing.is_none());
 
             // TODO: 'SigningTrigger' can't express multiple reasons.
             let trigger = if keys_changed {
@@ -102,14 +117,11 @@ impl SignerZoneHandle<'_> {
                 SigningTrigger::SignatureExpiration
             };
 
-            let handle = tokio::task::spawn(super::sign(
-                self.center.clone(),
-                self.zone.clone(),
-                builder,
-                trigger,
-            ));
-
-            self.state.signer.ongoing = Some(handle.into());
+            let span = tracing::Span::none();
+            self.state.signer.ongoing.spawn(
+                span,
+                super::sign(self.center.clone(), self.zone.clone(), builder, trigger),
+            );
         } else {
             // TODO: Track expiration time in 'SignerState'.
             let expiration_time = self
@@ -147,7 +159,6 @@ impl SignerZoneHandle<'_> {
                 .as_ref()
                 .is_none_or(|o| o.builder.is_none())
         );
-        assert!(self.state.signer.ongoing.is_none());
 
         // Load the one enqueued re-sign operation, if it exists.
         let Some(resign) = self.state.signer.enqueued_resign.take() else {
@@ -179,14 +190,11 @@ impl SignerZoneHandle<'_> {
             SigningTrigger::SignatureExpiration
         };
 
-        let handle = tokio::task::spawn(super::sign(
-            self.center.clone(),
-            self.zone.clone(),
-            builder,
-            trigger,
-        ));
-
-        self.state.signer.ongoing = Some(handle.into());
+        let span = tracing::Span::none();
+        self.state.signer.ongoing.spawn(
+            span,
+            super::sign(self.center.clone(), self.zone.clone(), builder, trigger),
+        );
 
         true
     }
@@ -197,8 +205,8 @@ impl SignerZoneHandle<'_> {
 /// State for signing a zone.
 #[derive(Debug, Default)]
 pub struct SignerState {
-    /// A handle to an ongoing operation, if any.
-    pub ongoing: Option<AbortOnDrop>,
+    /// Ongoing (re-)signing operations.
+    pub ongoing: BackgroundTasks,
 
     /// An enqueued signing operation, if any.
     pub enqueued_sign: Option<EnqueuedSign>,

src/units/key_manager.rs

@@ -1,11 +1,11 @@
 use crate::api;
 use crate::api::{FileKeyImport, KeyImport, KmipKeyImport};
-use crate::center::{Center, ZoneAddError};
+use crate::center::{Center, ZoneAddError, get_zone};
 use crate::manager::record_zone_event;
 use crate::policy::{KeyParameters, PolicyVersion};
 use crate::units::http_server::KmipServerState;
 use crate::util::AbortOnDrop;
-use crate::zone::{HistoricalEvent, SigningTrigger};
+use crate::zone::{HistoricalEvent, ZoneHandle};
 use bytes::Bytes;
 use camino::{Utf8Path, Utf8PathBuf};
 use cascade_api::keyset::{KeyRollCommand, KeyRollVariant};
@@ -25,7 +25,7 @@ use std::process::Output;
 use std::sync::Arc;
 use tokio::sync::Mutex;
 use tokio::time::Instant;
-use tracing::{debug, error, info, warn};
+use tracing::{debug, error, warn};
 
 //------------ KeyManager ----------------------------------------------------
 
@@ -437,13 +437,15 @@ impl KeyManager {
                     }
                 };
                 let _ = ks_info.insert(apex_name, new_info);
-                info!("[CC]: Instructing zone signer to re-sign the zone");
-                center.signer.on_sign_zone(
+                let zone = get_zone(center, zone.apex_name()).unwrap();
+                let mut state = zone.state.lock().unwrap();
+                ZoneHandle {
+                    zone: &zone,
+                    state: &mut state,
                     center,
-                    zone.apex_name().clone(),
-                    None,
-                    SigningTrigger::ExternallyModifiedKeySetState,
-                );
+                }
+                .signer()
+                .enqueue_resign(true, false);
                 continue;
             }
 
@@ -482,13 +484,15 @@ impl KeyManager {
                         // signer.
                         // let new_info = get_keyset_info(&state_path);
                         let _ = ks_info.insert(apex_name, new_info);
-                        info!("[CC]: Instructing zone signer to re-sign the zone");
-                        center.signer.on_sign_zone(
+                        let zone = get_zone(center, zone.apex_name()).unwrap();
+                        let mut state = zone.state.lock().unwrap();
+                        ZoneHandle {
+                            zone: &zone,
+                            state: &mut state,
                             center,
-                            zone.apex_name().clone(),
-                            None,
-                            SigningTrigger::KeySetModifiedAfterCron,
-                        );
+                        }
+                        .signer()
+                        .enqueue_resign(true, false);
                         continue;
                     }
 

src/units/zone_server.rs

@@ -46,8 +46,8 @@ use crate::manager::Terminated;
 use crate::manager::record_zone_event;
 use crate::util::AbortOnDrop;
 use crate::zone::{
-    HistoricalEvent, SignedZoneVersionState, SigningTrigger, UnsignedZoneVersionState, Zone,
-    ZoneHandle, ZoneVersionReviewState,
+    HistoricalEvent, SignedZoneVersionState, UnsignedZoneVersionState, Zone, ZoneHandle,
+    ZoneVersionReviewState,
 };
 
 /// The source of a zone server.
@@ -529,29 +529,15 @@ impl ZoneServer {
         zone: &Arc<Zone>,
         zone_serial: Serial,
     ) {
-        {
-            let mut zone_state = zone.state.lock().unwrap();
-            zone_state.record_event(
-                HistoricalEvent::UnsignedZoneReview {
-                    status: crate::api::ZoneReviewStatus::Approved,
-                },
-                Some(zone_serial),
-            );
-            ZoneHandle {
-                zone,
-                state: &mut zone_state,
-                center,
-            }
-            .storage()
-            .approve_loaded();
-        }
-        info!("[CC]: Instructing zone signer to sign the approved zone");
-        center.signer.on_sign_zone(
+        let _ = zone_serial; // TODO
+        let mut state = zone.state.lock().unwrap();
+        ZoneHandle {
+            zone,
+            state: &mut state,
             center,
-            zone.name.clone(),
-            Some(zone_serial),
-            SigningTrigger::ZoneChangesApproved,
-        );
+        }
+        .storage()
+        .approve_loaded();
     }
 
     fn on_signed_zone_approved(