re-read the declaration before a mint, and mint for a current certificate whose role is missing

swarm-controller/src/agent_renewal.rs

@@ -26,14 +26,14 @@
 //! **Only agents some hive is declared to run are renewed** ([`live_agents`]):
 //! a destroy is recorded in the wanted-state declarations, and an agent declared
 //! nowhere, or only as `Destroyed`, is skipped. A live agent with no stored
-//! certificate gets one minted, as a failed mint at creation is retried nowhere
-//! else, unless its mint is in flight. A missing queue secret stays missing.
+//! certificate, or a current one with no listed cert-auth role, gets one minted
+//! unless its mint is in flight. A missing queue secret stays missing.
 
 use std::collections::BTreeSet;
 use std::sync::Arc;
 
 use anyhow::{Context, Result, bail};
-use swarm_secret_client::{SecretStore, cert, mtls, queue};
+use swarm_secret_client::{SecretStore, cert, client::DEFAULT_CERT_MOUNT, mtls, policy, queue};
 use swarm_wire::wanted::{AgentState, HiveWanted};
 
 use crate::wanted::WantedWriter;
@@ -141,8 +141,9 @@ pub enum Observed {
     Unstamped,
     /// Stored and not yet due.
     Current,
-    /// Nothing stored. A certificate is minted for a live agent; a queue secret
-    /// is left alone (see the module doc).
+    /// Nothing stored (a certificate: or stored with no cert-auth role, see
+    /// [`cert_needing_role`]). A certificate is minted for a live agent; a queue
+    /// secret is left alone (see the module doc).
     Absent,
     /// The read or the decode failed. Nothing is known, so nothing is done.
     Unknown,
@@ -290,13 +291,13 @@ async fn observe_secret(store: &SecretStore, agent: &str, now: i64) -> Observed
     }
 }
 
-/// One pass: every live agent, observed. Fails as a whole when any hive's
-/// declaration cannot be read, since that hive may be the one declaring an
-/// agent destroyed.
-async fn observe_all(
+/// Every readable wanted-state declaration of `hives`. Fails as a whole when
+/// one cannot be read, since that hive may be the one declaring an agent
+/// destroyed.
+pub(crate) async fn read_declarations(
     wanted: &WantedWriter,
     hives: &[String],
-) -> Result<Vec<(String, AgentObserved)>> {
+) -> Result<Vec<HiveWanted>> {
     let mut declarations = Vec::with_capacity(hives.len());
     for hive in hives {
         if let Some(d) = wanted
@@ -307,14 +308,54 @@ async fn observe_all(
             declarations.push(d);
         }
     }
+    Ok(declarations)
+}
+
+/// A stored, current certificate whose cert-auth role is not in `roles` is as
+/// good as absent: the agent cannot log in with it, and the mint is the only
+/// thing that writes the role. `None` (the role listing failed) and every
+/// other observation come back unchanged.
+pub fn cert_needing_role(
+    cert: Observed,
+    roles: Option<&BTreeSet<String>>,
+    agent: &str,
+) -> Observed {
+    match (cert, roles) {
+        (Observed::Current, Some(roles)) if !roles.contains(agent) => Observed::Absent,
+        _ => cert,
+    }
+}
+
+/// One pass: every live agent, observed. Fails as a whole when any hive's
+/// declaration cannot be read.
+async fn observe_all(
+    wanted: &WantedWriter,
+    hives: &[String],
+) -> Result<Vec<(String, AgentObserved)>> {
+    let declarations = read_declarations(wanted, hives).await?;
     let store = crate::store::connect()
         .await
         .context("logging in to the swarm secret store")?;
     let now = unix_now();
+    let roles = match store.list_cert_roles(DEFAULT_CERT_MOUNT).await {
+        Ok(names) => Some(
+            policy::agents_from_role_names(&names)
+                .into_iter()
+                .collect::<BTreeSet<_>>(),
+        ),
+        Err(e) => {
+            tracing::warn!(error = %e, "agent certificate: listing the cert-auth roles failed");
+            None
+        }
+    };
     let mut observed = Vec::new();
     for agent in live_agents(&declarations) {
         let o = AgentObserved {
-            cert: observe_cert(&store, &agent, now).await,
+            cert: cert_needing_role(
+                observe_cert(&store, &agent, now).await,
+                roles.as_ref(),
+                &agent,
+            ),
             secret: observe_secret(&store, &agent, now).await,
         };
         if let Observed::Due(age) = o.cert {
@@ -323,7 +364,7 @@ async fn observe_all(
         if o.cert == Observed::Absent {
             tracing::info!(
                 agent,
-                "agent has no stored certificate; minting its store identity"
+                "agent has no usable certificate (none stored, or no cert-auth role); minting its store identity"
             );
         }
         if let Observed::Due(age) = o.secret {
@@ -596,6 +637,35 @@ mod tests {
         );
     }
 
+    /// A current certificate with no cert-auth role listed is minted for; the
+    /// controls are the same certificate with its role listed, a failed role
+    /// listing, and a certificate that is not `Current` anyway.
+    #[test]
+    fn a_current_certificate_without_its_role_is_minted_for() {
+        let roles: BTreeSet<String> = ["sock".to_owned()].into();
+        let planned = |cert: Observed, roles: Option<&BTreeSet<String>>, agent: &str| {
+            let observed = [(
+                agent.to_owned(),
+                AgentObserved {
+                    cert: cert_needing_role(cert, roles, agent),
+                    secret: Observed::Current,
+                },
+            )];
+            plan(&observed).iter().map(|r| r.cert).collect::<Vec<_>>()
+        };
+        assert_eq!(planned(Observed::Current, Some(&roles), "paused"), [true]);
+        assert_eq!(
+            planned(Observed::Current, Some(&roles), "sock"),
+            [] as [bool; 0]
+        );
+        assert_eq!(planned(Observed::Current, None, "paused"), [] as [bool; 0]);
+        assert_eq!(
+            planned(Observed::Unknown, Some(&roles), "paused"),
+            [] as [bool; 0]
+        );
+        assert_eq!(planned(Observed::Due(1), Some(&roles), "sock"), [true]);
+    }
+
     #[test]
     fn the_age_logged_is_whole_days() {
         assert_eq!(Age(46 * DAY + 5).to_string(), "46d");