From 4aa8f783cfe34505aac3a91da697c851aee901ea Mon Sep 17 00:00:00 2001
From: Tim Bruijnzeels <tim@nlnetlabs.nl>
Date: Tue, 2 Nov 2021 10:09:15 +0100
Subject: [PATCH] Report correct next update times (#701)

---
 src/commons/api/ca.rs       | 54 ++++++++++++++++++-----------
 src/daemon/ca/manager.rs    | 16 +++++++--
 src/daemon/ca/publishing.rs | 69 +++++++++++++++++++++++++------------
 src/daemon/ca/status.rs     | 15 +++++---
 4 files changed, 104 insertions(+), 50 deletions(-)

diff --git a/src/commons/api/ca.rs b/src/commons/api/ca.rs
index 6a9b8692..da7fead9 100644
--- a/src/commons/api/ca.rs
+++ b/src/commons/api/ca.rs
@@ -1412,17 +1412,17 @@ impl ParentStatus {
         self.last_exchange.as_ref().map(|e| e.to_failure_opt()).flatten()
     }
 
-    fn set_next_exchange_plus_seconds(&mut self, next_seconds: i64) {
-        self.next_exchange_before += Duration::seconds(next_seconds);
+    fn set_next_exchange(&mut self, next_run_seconds: i64) {
+        self.next_exchange_before = Timestamp::now_plus_seconds(next_run_seconds);
     }
 
-    fn set_failure(&mut self, uri: ServiceUri, error: ErrorResponse, next_seconds: i64) {
+    fn set_failure(&mut self, uri: ServiceUri, error: ErrorResponse, next_run_seconds: i64) {
         self.last_exchange = Some(ParentExchange {
             timestamp: Timestamp::now(),
             uri,
             result: ExchangeResult::Failure(error),
         });
-        self.set_next_exchange_plus_seconds(next_seconds);
+        self.set_next_exchange(next_run_seconds);
     }
 
     fn set_entitlements(&mut self, uri: ServiceUri, entitlements: &Entitlements, next_run_seconds: i64) {
@@ -1444,7 +1444,7 @@ impl ParentStatus {
         }
 
         self.all_resources = all_resources;
-        self.set_next_exchange_plus_seconds(next_run_seconds);
+        self.set_next_exchange(next_run_seconds);
     }
 
     fn set_last_updated(&mut self, uri: ServiceUri, next_run_seconds: i64) {
@@ -1455,7 +1455,7 @@ impl ParentStatus {
             result: ExchangeResult::Success,
         });
         self.last_success = Some(timestamp);
-        self.set_next_exchange_plus_seconds(next_run_seconds);
+        self.set_next_exchange(next_run_seconds);
     }
 }
 
@@ -1521,7 +1521,7 @@ impl RepoStatus {
         self.next_exchange_before = timestamp.plus_minutes(5);
     }
 
-    pub fn set_published(&mut self, uri: ServiceUri, published: Vec<PublishElement>, next_hours: i64) {
+    pub fn set_published(&mut self, uri: ServiceUri, published: Vec<PublishElement>, next_update: Timestamp) {
         let timestamp = Timestamp::now();
         self.last_exchange = Some(ParentExchange {
             timestamp,
@@ -1530,10 +1530,10 @@ impl RepoStatus {
         });
         self.published = published;
         self.last_success = Some(timestamp);
-        self.next_exchange_before = timestamp.plus_hours(next_hours);
+        self.next_exchange_before = next_update;
     }
 
-    pub fn set_last_updated(&mut self, uri: ServiceUri, next_hours: i64) {
+    pub fn set_last_updated(&mut self, uri: ServiceUri, next_update: Timestamp) {
         let timestamp = Timestamp::now();
         self.last_exchange = Some(ParentExchange {
             timestamp,
@@ -1541,7 +1541,7 @@ impl RepoStatus {
             result: ExchangeResult::Success,
         });
         self.last_success = Some(timestamp);
-        self.next_exchange_before = timestamp.plus_hours(next_hours);
+        self.next_exchange_before = next_update;
     }
 }
 
@@ -1844,18 +1844,10 @@ impl Timestamp {
         Timestamp::now().minus_hours(hours)
     }
 
-    pub fn now_minus_seconds(seconds: i64) -> Self {
-        Timestamp::now().minus_seconds(seconds)
-    }
-
     pub fn minus_hours(self, hours: i64) -> Self {
         self - Duration::hours(hours)
     }
 
-    pub fn minus_seconds(self, seconds: i64) -> Self {
-        self - Duration::seconds(seconds)
-    }
-
     pub fn now_plus_minutes(minutes: i64) -> Self {
         Timestamp::now().plus_minutes(minutes)
     }
@@ -1864,8 +1856,24 @@ impl Timestamp {
         self + Duration::minutes(minutes)
     }
 
-    pub fn to_rfc3339(&self) -> String {
-        Time::from(*self).to_rfc3339()
+    pub fn minus_seconds(self, seconds: i64) -> Self {
+        self - Duration::seconds(seconds)
+    }
+
+    pub fn plus_seconds(self, seconds: i64) -> Self {
+        self - Duration::seconds(seconds)
+    }
+
+    pub fn now_minus_seconds(seconds: i64) -> Self {
+        Timestamp::now().minus_seconds(seconds)
+    }
+
+    pub fn now_plus_seconds(seconds: i64) -> Self {
+        Timestamp::now().plus_seconds(seconds)
+    }
+
+    pub fn to_rfc3339(self) -> String {
+        Time::from(self).to_rfc3339()
     }
 }
 
@@ -1875,6 +1883,12 @@ impl From<Timestamp> for Time {
     }
 }
 
+impl From<Time> for Timestamp {
+    fn from(time: Time) -> Self {
+        Timestamp(time.timestamp())
+    }
+}
+
 //--- Display
 
 impl fmt::Display for Timestamp {
diff --git a/src/daemon/ca/manager.rs b/src/daemon/ca/manager.rs
index 6c5dc2a2..a7b938a8 100644
--- a/src/daemon/ca/manager.rs
+++ b/src/daemon/ca/manager.rs
@@ -1592,12 +1592,18 @@ impl CaManager {
             Ok(reply) => reply,
         };
 
+        let next_update = self
+            .ca_objects_store
+            .ca_objects(ca_handle)?
+            .closest_next_update()
+            .unwrap_or_else(|| Timestamp::now_plus_hours(self.config.republish_hours()));
+
         match reply {
             rfc8181::ReplyMessage::ListReply(list_reply) => {
                 self.status_store
                     .lock()
                     .await
-                    .set_status_repo_success(ca_handle, uri.clone(), self.config.republish_hours())
+                    .set_status_repo_success(ca_handle, uri.clone(), next_update)
                     .await?;
                 Ok(list_reply)
             }
@@ -1652,12 +1658,16 @@ impl CaManager {
                 // TODO: reflect the status for each REPO in the API / UI?
                 // We probably should.. though it should be extremely rare and short-lived to
                 // have more than one repository.
-                let published = self.ca_objects_store.ca_objects(ca_handle)?.all_publish_elements();
+                let ca_objects = self.ca_objects_store.ca_objects(ca_handle)?;
+                let published = ca_objects.all_publish_elements();
+                let next_update = ca_objects
+                    .closest_next_update()
+                    .unwrap_or_else(|| Timestamp::now_plus_hours(self.config.republish_hours()));
 
                 self.status_store
                     .lock()
                     .await
-                    .set_status_repo_published(ca_handle, uri.clone(), published, self.config.republish_hours())
+                    .set_status_repo_published(ca_handle, uri.clone(), published, next_update)
                     .await?;
                 Ok(())
             }
diff --git a/src/daemon/ca/publishing.rs b/src/daemon/ca/publishing.rs
index 27ead900..7f97df99 100644
--- a/src/daemon/ca/publishing.rs
+++ b/src/daemon/ca/publishing.rs
@@ -25,7 +25,7 @@ use crate::{
     commons::{
         api::{
             rrdp::PublishElement, Base64, Handle, IssuedCert, ObjectName, RcvdCert, RepositoryContact,
-            ResourceClassName, Revocation, Revocations,
+            ResourceClassName, Revocation, Revocations, Timestamp,
         },
         crypto::KrillSigner,
         error::Error,
@@ -335,10 +335,10 @@ impl CaObjects {
         if let Some(repo) = &self.repo {
             res.insert(repo.clone(), vec![]);
 
-            for rco in self.classes.values() {
+            for resource_class_objects in self.classes.values() {
                 // Note the map 'res' will get entries for other (old) repositories
                 // if there are any keys with such repositories.
-                rco.add_elements(&mut res, repo);
+                resource_class_objects.add_elements(&mut res, repo);
             }
         }
 
@@ -357,6 +357,24 @@ impl CaObjects {
         all_elements
     }
 
+    /// Returns the closest next update time from among manifests held by this CA
+    pub fn closest_next_update(&self) -> Option<Timestamp> {
+        let mut closest = None;
+
+        for resource_class_objects in self.classes.values() {
+            let rco_time = Timestamp::from(resource_class_objects.next_update_time());
+            if let Some(current_closest) = closest {
+                if current_closest > rco_time {
+                    closest = Some(rco_time);
+                }
+            } else {
+                closest = Some(rco_time);
+            }
+        }
+
+        closest
+    }
+
     pub fn deprecated_repos(&self) -> &Vec<DeprecatedRepository> {
         &self.deprecated_repos
     }
@@ -420,8 +438,7 @@ impl CaObjects {
         timing: &IssuanceTimingConfig,
         signer: &KrillSigner,
     ) -> KrillResult<()> {
-        let rco = self.get_class_mut(rcn)?;
-        rco.keyroll_stage(key, timing, signer)
+        self.get_class_mut(rcn)?.keyroll_stage(key, timing, signer)
     }
 
     // Activates the keyset by retiring the current set, and promoting
@@ -432,17 +449,16 @@ impl CaObjects {
         timing: &IssuanceTimingConfig,
         signer: &KrillSigner,
     ) -> KrillResult<()> {
-        let rco = self.get_class_mut(rcn)?;
-        rco.keyroll_activate(timing, signer)
+        self.get_class_mut(rcn)?.keyroll_activate(timing, signer)
     }
 
     // Finish a keyroll
     fn keyroll_finish(&mut self, rcn: &ResourceClassName) -> KrillResult<()> {
-        let rco = self.get_class_mut(rcn)?;
+        let resource_class_objects = self.get_class_mut(rcn)?;
 
-        // finish the key roll for this rco. This will remove the old key, and return
-        // an old_repo if there was one.
-        if let Some(old_repo) = rco.keyroll_finish()? {
+        // finish the key roll for this resource class objects. This will remove the old
+        // key, and return an old_repo if there was one.
+        if let Some(old_repo) = resource_class_objects.keyroll_finish()? {
             self.deprecate_repo_if_no_longer_used(old_repo);
         }
 
@@ -457,8 +473,7 @@ impl CaObjects {
         timing: &IssuanceTimingConfig,
         signer: &KrillSigner,
     ) -> KrillResult<()> {
-        let rco = self.get_class_mut(rcn)?;
-        rco.update_roas(roa_updates, timing, signer)
+        self.get_class_mut(rcn)?.update_roas(roa_updates, timing, signer)
     }
 
     // Update the delegated certificates in the current set
@@ -469,14 +484,12 @@ impl CaObjects {
         timing: &IssuanceTimingConfig,
         signer: &KrillSigner,
     ) -> KrillResult<()> {
-        let rco = self.get_class_mut(rcn)?;
-        rco.update_certs(cert_updates, timing, signer)
+        self.get_class_mut(rcn)?.update_certs(cert_updates, timing, signer)
     }
 
     // Update the received certificate.
     fn update_received_cert(&mut self, rcn: &ResourceClassName, cert: &RcvdCert) -> KrillResult<()> {
-        let rco = self.get_class_mut(rcn)?;
-        rco.update_received_cert(cert)
+        self.get_class_mut(rcn)?.update_received_cert(cert)
     }
 
     /// Reissue the MFT and CRL in this set if needed, i.e. if it's close to the next
@@ -486,10 +499,10 @@ impl CaObjects {
         let hours = timing.timing_publish_hours_before_next;
         let mut required = false;
 
-        for (_, rco) in self.classes.iter_mut() {
-            if rco.requires_re_issuance(hours) {
+        for (_, resource_class_objects) in self.classes.iter_mut() {
+            if resource_class_objects.requires_re_issuance(hours) {
                 required = true;
-                rco.reissue(timing, signer)?;
+                resource_class_objects.reissue(timing, signer)?;
             }
         }
 
@@ -503,8 +516,8 @@ impl CaObjects {
     // existing keys.
     fn update_repo(&mut self, repo: &RepositoryContact) {
         if let Some(old) = &self.repo {
-            for rco in self.classes.values_mut() {
-                rco.set_old_repo(old);
+            for resource_class_objects in self.classes.values_mut() {
+                resource_class_objects.set_old_repo(old);
             }
         }
         self.repo = Some(repo.clone());
@@ -645,6 +658,14 @@ impl ResourceClassObjects {
         }
     }
 
+    fn next_update_time(&self) -> Time {
+        match &self.keys {
+            ResourceClassKeyState::Current(state) => state.current_set.next_update_time(),
+            ResourceClassKeyState::Old(state) => state.current_set.next_update_time(),
+            ResourceClassKeyState::Staging(state) => state.current_set.next_update_time(),
+        }
+    }
+
     fn reissue(&mut self, timing: &IssuanceTimingConfig, signer: &KrillSigner) -> KrillResult<()> {
         match self.keys.borrow_mut() {
             ResourceClassKeyState::Current(state) => state.current_set.reissue(timing, signer),
@@ -1086,6 +1107,10 @@ impl BasicKeyObjectSet {
             || Some(self.signing_cert.uri()) != self.manifest.cert().ca_issuer()
     }
 
+    pub fn next_update_time(&self) -> Time {
+        self.manifest.next_update()
+    }
+
     fn next(&self) -> u64 {
         self.number + 1
     }
diff --git a/src/daemon/ca/status.rs b/src/daemon/ca/status.rs
index d1bcaf1e..114704d6 100644
--- a/src/daemon/ca/status.rs
+++ b/src/daemon/ca/status.rs
@@ -5,7 +5,7 @@ use tokio::sync::RwLock;
 use crate::commons::{
     api::{
         rrdp::PublishElement, ChildConnectionStats, ChildHandle, ChildStatus, ChildrenConnectionStats, Entitlements,
-        ErrorResponse, Handle, ParentHandle, ParentStatuses, RepoStatus,
+        ErrorResponse, Handle, ParentHandle, ParentStatuses, RepoStatus, Timestamp,
     },
     error::Error,
     eventsourcing::{KeyStoreKey, KeyValueStore},
@@ -227,8 +227,13 @@ impl StatusStore {
             .await
     }
 
-    pub async fn set_status_repo_success(&self, ca: &Handle, uri: ServiceUri, next_hours: i64) -> KrillResult<()> {
-        self.update_ca_status(ca, |status| status.repo.set_last_updated(uri, next_hours))
+    pub async fn set_status_repo_success(
+        &self,
+        ca: &Handle,
+        uri: ServiceUri,
+        next_update: Timestamp,
+    ) -> KrillResult<()> {
+        self.update_ca_status(ca, |status| status.repo.set_last_updated(uri, next_update))
             .await
     }
 
@@ -237,9 +242,9 @@ impl StatusStore {
         ca: &Handle,
         uri: ServiceUri,
         published: Vec<PublishElement>,
-        next_hours: i64,
+        next_update: Timestamp,
     ) -> KrillResult<()> {
-        self.update_ca_status(ca, |status| status.repo.set_published(uri, published, next_hours))
+        self.update_ca_status(ca, |status| status.repo.set_published(uri, published, next_update))
             .await
     }