diff --git a/swarm-controller/src/read_policy.rs b/swarm-controller/src/read_policy.rs index cc236ba4..5723b8b0 100644 --- a/swarm-controller/src/read_policy.rs +++ b/swarm-controller/src/read_policy.rs @@ -9,10 +9,13 @@ //! A pass runs at startup, and repeats until it reaches the store. The two //! start together, so the store being the slower one is the ordinary case //! rather than an edge one, and a pass that gave up there provisions nobody -//! until something restarts this daemon. Per-hive failures are different: they -//! are reported and skipped — one hive that cannot be provisioned should not -//! take down a daemon that serves everything else, and a store that answers is -//! answering the same way next time, so the next start is the retry for those. +//! until something restarts this daemon. A shared policy the store refuses is +//! the same kind of failure: the controller's own policy is rewritten by a unit +//! that is not ordered against this daemon, so the refusal can be a moment too. +//! Per-hive role failures are different: they are reported and skipped — one +//! hive that cannot be provisioned should not take down a daemon that serves +//! everything else, and a store that answers is answering the same way next +//! time, so the next start is the retry for those. //! //! The rule is written once for all hives, [`policy::SHARED_HIVE_POLICY`]: one //! stanza is the same everywhere, the other is scoped to the reader's own name @@ -38,7 +41,7 @@ use crate::store::AccessStore; /// `swarm-controller.nix` names it. pub const ENV_HIVE_CLIENT_CA: &str = "SWARM_CONTROLLER_HIVE_CLIENT_CA_FILE"; -/// How long to wait before re-attempting a pass that never reached the store. +/// How long to wait before re-attempting a pass that did not complete. /// /// `nix/host-modules/swarm-bao.nix`'s bootstrap unit retries the other side of /// this same race on the same cadence: the two halves of one race should not @@ -55,7 +58,9 @@ const RETRY_ATTEMPTS: u32 = 2_880; /// one of them can be edited alone. const RETRY_WINDOW: Duration = Duration::from_hours(24); -/// A pass that never got as far as the store, carrying what to say about it. +/// A pass that could not complete for a reason a later attempt may resolve: the +/// store was not reached, or it answered and refused (an accessor read or a +/// shared-policy write answered 403). Carries what to say about it. /// /// Returned rather than logged where it happens: the first attempt deserves a /// warning and the two-hundredth is noise, and only the caller knows which one @@ -65,14 +70,14 @@ struct Unreachable(String); /// Give every hive in `hives` the access its certificate needs. /// /// Returns once the first pass is done, so a store that is already up leaves -/// nothing deferred. When that pass cannot reach the store, it repeats in the +/// nothing deferred. When that pass cannot complete, it repeats in the /// background for up to [`RETRY_WINDOW`]. pub async fn ensure_hive_access(hives: Vec) { if let Err(Unreachable(reason)) = provision(&hives).await { tracing::warn!( reason, retry_in = ?RETRY_INTERVAL, - "hive provisioning could not reach the swarm secret store; retrying in the background" + "hive provisioning could not complete (the swarm secret store was unreachable or refused); retrying in the background" ); spawn_retry(hives); } @@ -86,7 +91,7 @@ fn spawn_retry(hives: Vec) { async move { let outcome = provision(&hives).await; if let Err(Unreachable(reason)) = &outcome { - tracing::debug!(reason, "hive provisioning: store still unreachable"); + tracing::debug!(reason, "hive provisioning: still not complete"); } outcome } @@ -138,8 +143,9 @@ where /// /// # Errors /// [`Unreachable`] for the states a later attempt can resolve: a store that is -/// not up, and an authority file that is not placed yet. A deployment that is -/// configured for neither is `Ok` — there is nothing to wait for. +/// not up or that refuses the shared hive policy, and an authority file that is +/// not placed yet. A deployment that is configured for neither is `Ok` — +/// there is nothing to wait for. async fn provision(hives: &[String]) -> Result<(), Unreachable> { if hives.is_empty() { return Ok(()); @@ -180,9 +186,7 @@ async fn provision_with( hives: &[String], read_authority: impl FnOnce() -> Result, Unreachable>, ) -> Result<(), Unreachable> { - if !write_shared_policy(store).await { - return Ok(()); - } + write_shared_policy(store).await?; let Some(ca) = read_authority()? else { tracing::info!("no hive certificate authority configured; hive roles are not managed here"); return Ok(()); @@ -196,48 +200,40 @@ async fn provision_with( } /// Write [`policy::SHARED_HIVE_POLICY`], rendered with the cert mount's -/// accessor. Answers whether it is in place: hive roles are written only then. +/// accessor. Hive roles are written only once it is in place. /// -/// A store that refuses the accessor read (the controller's policy may not -/// carry it yet) or the write leaves the roles already in the store as they are. -async fn write_shared_policy(store: &impl AccessStore) -> bool { - let accessor = match store.auth_mount_accessor(DEFAULT_CERT_MOUNT).await { - Ok(accessor) => accessor, - Err(e) => { - tracing::warn!( - error = %e, - "the shared hive policy is not managed: the cert auth mount's accessor could not be read; hive roles are left as they are" - ); - return false; - } - }; - let document = match policy::render_shared_hives(&accessor) { - Ok(document) => document, - Err(e) => { - tracing::warn!(error = %e, "the shared hive policy is not managed: the cert auth mount's accessor is not usable in a policy; hive roles are left as they are"); - return false; - } - }; - match store +/// # Errors +/// [`Unreachable`] when the store refuses the accessor read (the controller's +/// policy may not carry it yet) or the write, or when the accessor is not +/// usable in a policy: the roles already in the store are left as they are. +async fn write_shared_policy(store: &impl AccessStore) -> Result<(), Unreachable> { + let accessor = store + .auth_mount_accessor(DEFAULT_CERT_MOUNT) + .await + .map_err(|e| { + Unreachable(format!( + "the cert auth mount's accessor could not be read: {e}" + )) + })?; + let document = policy::render_shared_hives(&accessor).map_err(|e| { + Unreachable(format!( + "the cert auth mount's accessor is not usable in a policy: {e}" + )) + })?; + store .write_policy(policy::SHARED_HIVE_POLICY, &document) .await - { - Ok(()) => { - tracing::info!( - policy = policy::SHARED_HIVE_POLICY, - "shared hive policy in place" - ); - true - } - Err(e) => { - tracing::warn!( - policy = policy::SHARED_HIVE_POLICY, - error = %e, - "writing the shared hive policy failed; hive roles are left as they are" - ); - false - } - } + .map_err(|e| { + Unreachable(format!( + "writing the {} policy failed: {e}", + policy::SHARED_HIVE_POLICY + )) + })?; + tracing::info!( + policy = policy::SHARED_HIVE_POLICY, + "shared hive policy in place" + ); + Ok(()) } /// The policies a hive's role attaches: the shared one, and no policy named for @@ -269,20 +265,20 @@ async fn write_role(store: &impl AccessStore, hive: &str, ca: &str) -> Result<() /// Write the shared agent policy and give every agent its identity entity, with /// the retry [`ensure_hive_access`] has and for the same reason. /// -/// Returns once the first pass is done. When that pass cannot reach the store, +/// Returns once the first pass is done. When that pass cannot complete, /// it repeats in the background for up to [`RETRY_WINDOW`]. pub async fn ensure_agent_policies() { if let Err(Unreachable(reason)) = rewrite_agent_policies().await { tracing::warn!( reason, retry_in = ?RETRY_INTERVAL, - "agent policy rewrite could not reach the swarm secret store; retrying in the background" + "agent policy rewrite could not complete (the swarm secret store was unreachable or refused); retrying in the background" ); tokio::spawn(async { let reached = retry_until_ok(RETRY_INTERVAL, RETRY_ATTEMPTS, || async { let outcome = rewrite_agent_policies().await; if let Err(Unreachable(reason)) = &outcome { - tracing::debug!(reason, "agent policy rewrite: store still unreachable"); + tracing::debug!(reason, "agent policy rewrite: still not complete"); } outcome }) @@ -311,7 +307,9 @@ pub async fn ensure_agent_policies() { /// # Errors /// [`Unreachable`] when the store cannot be reached or will not list its /// roles: either way no agent was written, and a later attempt may get -/// further. A deployment with no store configured is `Ok`. +/// further. Also when it refuses the shared agent policy, after the agents +/// have still been given their entities and token lifetimes. A deployment +/// with no store configured is `Ok`. async fn rewrite_agent_policies() -> Result<(), Unreachable> { let store = match crate::store::connect().await { Ok(store) => store, @@ -328,24 +326,32 @@ async fn rewrite_agent_policies() -> Result<(), Unreachable> { .list_cert_roles(DEFAULT_CERT_MOUNT) .await .map_err(|e| Unreachable(format!("listing the store's cert-auth roles: {e}")))?; + write_agent_policy_and_entities(&store, &roles).await +} + +/// The part of [`rewrite_agent_policies`] after the roles are listed. +async fn write_agent_policy_and_entities( + store: &impl AccessStore, + roles: &[String], +) -> Result<(), Unreachable> { // First, so the cert-auth role that names it never points at nothing. - match store + let written = store .write_policy(policy::SHARED_AGENT_POLICY, &policy::render_shared_agent()) - .await - { - Ok(()) => tracing::info!( + .await; + if written.is_ok() { + tracing::info!( policy = policy::SHARED_AGENT_POLICY, "shared agent policy in place" - ), - Err(e) => tracing::warn!( - policy = policy::SHARED_AGENT_POLICY, - error = %e, - "writing the shared agent policy failed" - ), + ); } - crate::agent_entity::ensure_agents(&store, &policy::agents_from_role_names(&roles)).await; - crate::agent_role_ttl::bring_current(&store, &roles).await; - Ok(()) + crate::agent_entity::ensure_agents(store, &policy::agents_from_role_names(roles)).await; + crate::agent_role_ttl::bring_current(store, roles).await; + written.map_err(|e| { + Unreachable(format!( + "writing the {} policy failed: {e}", + policy::SHARED_AGENT_POLICY + )) + }) } #[cfg(test)] @@ -356,7 +362,7 @@ mod tests { use super::{ RETRY_ATTEMPTS, RETRY_INTERVAL, RETRY_WINDOW, Unreachable, attached_policies, policy, - provision_with, retry_until_ok, + provision_with, retry_until_ok, write_agent_policy_and_entities, }; #[test] @@ -442,9 +448,11 @@ mod tests { #[tokio::test] async fn an_unreadable_accessor_leaves_every_hive_role_as_it_was() { let store = FakeAccess::without_accessor(); - provision_with(&store, &hives(), || Ok(Some("HIVE-CA".to_owned()))) - .await - .unwrap_or_else(|_| panic!("a policy that cannot be written is not a retry")); + let outcome = provision_with(&store, &hives(), || Ok(Some("HIVE-CA".to_owned()))).await; + assert!( + outcome.is_err(), + "a policy that cannot be written is retried" + ); assert!(store.roles_written().is_empty(), "{:?}", store.calls()); // The control: the accessor read was attempted, and the same fake // writes roles when it works (the test above). @@ -454,9 +462,8 @@ mod tests { #[tokio::test] async fn a_refused_shared_policy_leaves_every_hive_role_as_it_was() { let store = FakeAccess::refusing_policies(); - provision_with(&store, &hives(), || Ok(Some("HIVE-CA".to_owned()))) - .await - .unwrap_or_else(|_| panic!("a refused write is not a retry")); + let outcome = provision_with(&store, &hives(), || Ok(Some("HIVE-CA".to_owned()))).await; + assert!(outcome.is_err(), "a refused write is retried"); assert!(store.roles_written().is_empty(), "{:?}", store.calls()); assert!( store @@ -489,4 +496,48 @@ mod tests { assert!(outcome.is_err()); assert!(store.roles_written().is_empty(), "{:?}", store.calls()); } + + /// The property this change is about: a pass that is refused the shared + /// policy is repeated, and the roles are written once the store accepts it. + #[tokio::test] + async fn a_refused_accessor_read_is_repeated_and_the_roles_follow() { + let store = FakeAccess::with_accessor_refused(1); + let hives = hives(); + let pass = || provision_with(&store, &hives, || Ok(Some("HIVE-CA".to_owned()))); + assert!(pass().await.is_err(), "the first pass is refused"); + assert!(store.roles_written().is_empty(), "{:?}", store.calls()); + let reached = retry_until_ok(Duration::from_millis(1), 10, pass).await; + assert_eq!(reached, Some(1), "the first retry succeeds"); + assert_eq!( + store.roles_written(), + [ + "write_cert_role hive-pr1ma hive-hives", + "write_cert_role hive-beta hive-hives" + ], + "{:?}", + store.calls() + ); + } + + #[tokio::test] + async fn a_refused_shared_agent_policy_is_retried_and_a_written_one_is_not() { + let refused = FakeAccess::refusing_policies(); + assert!( + write_agent_policy_and_entities(&refused, &[]) + .await + .is_err() + ); + // The control: the same pass against a store that accepts the write. + let accepting = FakeAccess::default(); + assert!( + write_agent_policy_and_entities(&accepting, &[]) + .await + .is_ok() + ); + assert!( + accepting + .calls() + .contains(&format!("write_policy {}", policy::SHARED_AGENT_POLICY)) + ); + } } diff --git a/swarm-controller/src/store.rs b/swarm-controller/src/store.rs index 210c7709..ecd598df 100644 --- a/swarm-controller/src/store.rs +++ b/swarm-controller/src/store.rs @@ -197,6 +197,8 @@ pub(crate) mod fake { unwritable_roles: Vec, /// The accessor read fails. no_accessor: bool, + /// How many more accessor reads fail before one succeeds. + accessor_refusals: Mutex, /// Every policy write is refused. refuse_policy: bool, /// The entity that owns each alias, as `(id, name)`. @@ -214,6 +216,14 @@ pub(crate) mod fake { } } + /// A store whose cert mount accessor cannot be read the first `n` times. + pub(crate) fn with_accessor_refused(n: u32) -> Self { + Self { + accessor_refusals: Mutex::new(n), + ..Self::default() + } + } + /// A store that refuses every policy write. pub(crate) fn refusing_policies() -> Self { Self { @@ -340,7 +350,9 @@ pub(crate) mod fake { async fn auth_mount_accessor(&self, _mount: &str) -> Result { self.log("auth_mount_accessor".to_owned()); - if self.no_accessor { + let mut refusals = self.accessor_refusals.lock().expect("not poisoned"); + if self.no_accessor || *refusals > 0 { + *refusals = refusals.saturating_sub(1); return Err(Self::denied()); } Ok("auth_cert_test".to_owned())