diff --git a/CHANGELOG.md b/CHANGELOG.md index c4043775..bd736ecf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Plan retention is bounded per phase, so replan churn no longer evicts the record of what ran.** Terminal plans were trimmed as one pool of 10 by creation time. `Superseded` is generated churn — every replan supersedes its predecessor — so on an active policy it filled the pool and deleted the `Applied` plans, which are the audit record of what actually executed against the database. The least informative state was evicting the most informative one. The bounds are now `Applied` 25 (never fewer than 30 days' worth, hard ceiling 200), `Failed` and `Rejected` 10 shared, `Superseded` 3; `Pending`, `Approved` and `Applying` are live and never evicted. The age floor makes the retained span a stated period rather than a function of how often a policy applies, and the ceiling stops that promise becoming unbounded growth. `pgroles.io/keep=true` still exempts a plan from every bound. Each bound is operator-level configuration — `PLAN_RETENTION_APPLIED`, `PLAN_RETENTION_APPLIED_MIN_AGE`, `PLAN_RETENTION_APPLIED_CEILING`, `PLAN_RETENTION_DECIDED`, `PLAN_RETENTION_SUPERSEDED` on the operator environment, replacing the `max_plans` parameter that nothing could ever set — and an invalid value refuses operator startup with the variable named. Deliberately not a `PostgresPolicy` field: retention caps object growth in the cluster, and the per-object need is what the `keep` label is for. The `Applied` bounds measure — and order — by `status.appliedAt`, not object creation, so a plan that waited on a reviewer is not already outside its floor the moment it executes. They also govern terminal-candidate pruning: deleting a candidate cascades to the plan it owns, so a promoted candidate owning an `Applied` plan is held to the `Applied` bounds instead of the flat terminal-candidate bound, and `pgroles.io/keep=true` on either the candidate or its plan exempts the pair. (#194) + - **`spec.mode: plan` is renamed to `spec.mode: observe`, with a deprecation window.** "Plan" now names exactly one thing, the `PostgresPolicyPlan` resource; the `ApprovalIgnored` reason `PlanModeNeverExecutes` is now `ObserveModeNeverExecutes`. The old value keeps working: `mode: plan` stays an accepted schema value with identical behaviour, so a GitOps controller re-applying an existing manifest is unaffected by the upgrade. A policy using it reports a `ModeValueDeprecated` condition, warns in the operator log, and counts toward `pgroles.deprecated.mode_plan`. **Upgrade:** change `mode: plan` to `mode: observe` in your manifests at your convenience — a future release removes the `plan` value, and that removal will be the breaking change. diff --git a/charts/pgroles-operator/README.md b/charts/pgroles-operator/README.md index f99f975b..d90fa5e1 100644 --- a/charts/pgroles-operator/README.md +++ b/charts/pgroles-operator/README.md @@ -65,7 +65,7 @@ these policies, pinned to the `pgroles-system` namespace and the | fullnameOverride | string | `""` | Replaces the full generated name in resource names. | | nameOverride | string | `""` | Replaces the generated name in resource names, keeping the release prefix. | | operator.affinity | object | `{}` | Affinity rules for operator pod scheduling. | -| operator.env | list | `[{"name":"RUST_LOG","value":"info,pgroles_operator=debug"}]` | Additional environment variables for the operator container. This is also where operator-wide settings live, since the operator is configured by environment rather than by flags. Notable variables: `OTEL_EXPORTER_OTLP_ENDPOINT` enables OTLP metrics and logs, including structured ephemeral-access audit events; `OTEL_LOGS_EXPORTER=none` disables the log half when another agent already ships container logs off-cluster; `EPHEMERAL_ACCESS_MAXIMUM_DURATION` (default `24h`) and `EPHEMERAL_ACCESS_MAX_PENDING_TTL` (default `1h`) are the only cluster-wide ceilings on ephemeral access, and an access policy exceeding either is rejected with `Accepted=False`. | +| operator.env | list | `[{"name":"RUST_LOG","value":"info,pgroles_operator=debug"}]` | Additional environment variables for the operator container. This is also where operator-wide settings live, since the operator is configured by environment rather than by flags. Notable variables: `OTEL_EXPORTER_OTLP_ENDPOINT` enables OTLP metrics and logs, including structured ephemeral-access audit events; `OTEL_LOGS_EXPORTER=none` disables the log half when another agent already ships container logs off-cluster; `EPHEMERAL_ACCESS_MAXIMUM_DURATION` (default `24h`) and `EPHEMERAL_ACCESS_MAX_PENDING_TTL` (default `1h`) are the only cluster-wide ceilings on ephemeral access, and an access policy exceeding either is rejected with `Accepted=False`; the `PLAN_RETENTION_*` variables bound how many terminal plans are kept per policy — `PLAN_RETENTION_APPLIED` (default `25`), `PLAN_RETENTION_APPLIED_MIN_AGE` (default `720h`), `PLAN_RETENTION_APPLIED_CEILING` (default `200`), `PLAN_RETENTION_DECIDED` (default `10`, Failed and Rejected shared) and `PLAN_RETENTION_SUPERSEDED` (default `3`) — and an invalid value refuses operator startup with the variable named. | | operator.http.port | int | `8080` | Port serving the `/livez` and `/readyz` probes. | | operator.image.pullPolicy | string | `"IfNotPresent"` | Image pull policy for the operator container. | | operator.image.repository | string | `"ghcr.io/hardbyte/pgroles-operator"` | Operator container image repository. | diff --git a/charts/pgroles-operator/values.yaml b/charts/pgroles-operator/values.yaml index 38d26ae6..0f7def71 100644 --- a/charts/pgroles-operator/values.yaml +++ b/charts/pgroles-operator/values.yaml @@ -69,7 +69,13 @@ operator: # `EPHEMERAL_ACCESS_MAXIMUM_DURATION` (default `24h`) and # `EPHEMERAL_ACCESS_MAX_PENDING_TTL` (default `1h`) are the only cluster-wide # ceilings on ephemeral access, and an access policy exceeding either is - # rejected with `Accepted=False`. + # rejected with `Accepted=False`; the `PLAN_RETENTION_*` variables bound how + # many terminal plans are kept per policy — `PLAN_RETENTION_APPLIED` (default + # `25`), `PLAN_RETENTION_APPLIED_MIN_AGE` (default `720h`), + # `PLAN_RETENTION_APPLIED_CEILING` (default `200`), `PLAN_RETENTION_DECIDED` + # (default `10`, Failed and Rejected shared) and `PLAN_RETENTION_SUPERSEDED` + # (default `3`) — and an invalid value refuses operator startup with the + # variable named. env: - name: RUST_LOG value: "info,pgroles_operator=debug" @@ -83,6 +89,16 @@ operator: # value: 24h # - name: EPHEMERAL_ACCESS_MAX_PENDING_TTL # value: 1h + # - name: PLAN_RETENTION_APPLIED + # value: "25" + # - name: PLAN_RETENTION_APPLIED_MIN_AGE + # value: 720h + # - name: PLAN_RETENTION_APPLIED_CEILING + # value: "200" + # - name: PLAN_RETENTION_DECIDED + # value: "10" + # - name: PLAN_RETENTION_SUPERSEDED + # value: "3" # -- Annotations to add to the operator pod. podAnnotations: {} diff --git a/crates/pgroles-operator/src/candidate.rs b/crates/pgroles-operator/src/candidate.rs index 2a72f771..0e9ce7fa 100644 --- a/crates/pgroles-operator/src/candidate.rs +++ b/crates/pgroles-operator/src/candidate.rs @@ -69,7 +69,11 @@ use crate::reconciler::{ReconcileError, ResolvedPassword}; /// Maximum terminal candidates retained per policy before the oldest are /// pruned. Deleting a candidate cascades to the plan it owns and to that -/// plan's SQL ConfigMap. +/// plan's SQL ConfigMap — which is why this flat bound governs only +/// candidates whose plans never executed. A candidate owning an `Applied` +/// plan is held to [`crate::plan::PlanRetention`]'s applied bounds instead, +/// so proposal churn cannot delete execution history through the owner +/// object (see [`terminal_candidates_to_prune`]). const DEFAULT_MAX_TERMINAL_CANDIDATES: usize = 10; /// Maximum *open* candidates per policy that are planned in one pass. @@ -742,6 +746,7 @@ async fn plan_against_target( target.target_identity(), &summary, &password_source_versions, + ctx.plan_retention, Some(CandidatePlanBinding { candidate, content_digest: &content_digest, @@ -1434,33 +1439,57 @@ async fn supersede_candidate_plan( Ok(()) } -/// Prune terminal candidates beyond the retention bound. +/// Prune terminal candidates beyond the retention bounds. /// /// Plans cascade: each is owned by its candidate, so deleting the candidate -/// takes the plan and its SQL ConfigMap with it. `pgroles.io/keep=true` exempts -/// a candidate, and this is best-effort — retention must never block planning. +/// takes the plan and its SQL ConfigMap with it. That cascade is why pruning +/// reads the plans first — see [`terminal_candidates_to_prune`] for the +/// decision. Best-effort throughout: retention must never block planning, and +/// when the plans cannot be read this pass prunes nothing rather than prune +/// blind and cascade away an `Applied` plan retention promises to keep. async fn cleanup_terminal_candidates( ctx: &OperatorContext, namespace: &str, candidates: &[PostgresPolicyCandidate], ) { - let mut terminal: Vec<&PostgresPolicyCandidate> = candidates + let terminal: Vec<&PostgresPolicyCandidate> = candidates .iter() .filter(|candidate| candidate_phase(candidate).is_terminal()) - .filter(|candidate| !is_retention_exempt(*candidate)) .collect(); - if terminal.len() <= DEFAULT_MAX_TERMINAL_CANDIDATES { + let retention = ctx.plan_retention; + // The buckets below partition the terminal set, so when the whole set + // fits inside every count bound nothing can be pruned — skip the plan + // read on that common path. + if terminal.len() <= DEFAULT_MAX_TERMINAL_CANDIDATES && terminal.len() <= retention.applied { return; } - terminal.sort_by(|a, b| { - a.metadata - .creation_timestamp - .cmp(&b.metadata.creation_timestamp) - }); + + let plans: Vec = + match Api::::namespaced(ctx.kube_client.clone(), namespace) + .list(&ListParams::default()) + .await + { + Ok(list) => list.items, + Err(err) => { + tracing::warn!(%err, "could not read plans; skipping terminal-candidate pruning"); + return; + } + }; + let records: Vec<(&PostgresPolicyCandidate, Vec<&PostgresPolicyPlan>)> = terminal + .into_iter() + .map(|candidate| { + let uid = candidate.metadata.uid.clone().unwrap_or_default(); + let owned: Vec<&PostgresPolicyPlan> = plans + .iter() + .filter(|plan| !uid.is_empty() && crate::plan::is_owned_by_uid(*plan, &uid)) + .collect(); + (candidate, owned) + }) + .collect(); let api: Api = Api::namespaced(ctx.kube_client.clone(), namespace); - let excess = terminal.len() - DEFAULT_MAX_TERMINAL_CANDIDATES; - for candidate in terminal.into_iter().take(excess) { + let now_ts = Timestamp::now().as_second(); + for candidate in terminal_candidates_to_prune(&records, retention, now_ts) { let name = candidate.name_any(); info!(candidate = %name, "pruning terminal candidate"); if let Err(err) = api.delete(&name, &DeleteParams::default()).await { @@ -1469,6 +1498,75 @@ async fn cleanup_terminal_candidates( } } +/// Which terminal candidates to prune, given the plans each one owns. +/// +/// Deleting a candidate cascades to every plan it owns, so the decision is +/// made per candidate-and-plans pair: +/// +/// - `pgroles.io/keep=true` on the candidate **or on any plan it owns** +/// exempts the pair. The cascade cannot honour a keep on the child if the +/// parent goes, so a kept child must protect its parent. +/// - A candidate owning an `Applied` plan is the provenance of an execution +/// record, so it is held to the [`crate::plan::PlanRetention`] bounds for +/// `Applied` plans — same count, age floor and ceiling, ordered by when the plan +/// applied — not to the flat terminal bound, which proposal churn would +/// otherwise use to delete fresh execution history through the owner +/// object. +/// - Every other terminal candidate is proposal churn — nothing it owns ever +/// ran — bounded by [`DEFAULT_MAX_TERMINAL_CANDIDATES`], oldest first by +/// creation. +fn terminal_candidates_to_prune<'a>( + records: &[(&'a PostgresPolicyCandidate, Vec<&'a PostgresPolicyPlan>)], + retention: crate::plan::PlanRetention, + now_ts: i64, +) -> Vec<&'a PostgresPolicyCandidate> { + let mut churn: Vec<&PostgresPolicyCandidate> = Vec::new(); + let mut applied: Vec<(&PostgresPolicyCandidate, &PostgresPolicyPlan)> = Vec::new(); + for (candidate, plans) in records { + if is_retention_exempt(*candidate) || plans.iter().any(|plan| is_retention_exempt(*plan)) { + continue; + } + let applied_plan = plans.iter().find(|plan| { + plan.status + .as_ref() + .is_some_and(|status| status.phase == PlanPhase::Applied) + }); + match applied_plan { + Some(plan) => applied.push((candidate, plan)), + None => churn.push(candidate), + } + } + + let mut prune: Vec<&PostgresPolicyCandidate> = Vec::new(); + if churn.len() > DEFAULT_MAX_TERMINAL_CANDIDATES { + churn.sort_by(|a, b| { + a.metadata + .creation_timestamp + .cmp(&b.metadata.creation_timestamp) + }); + let excess = churn.len() - DEFAULT_MAX_TERMINAL_CANDIDATES; + prune.extend(churn.into_iter().take(excess)); + } + + // Plan names are unique within the namespace, so they key the way back + // from an evicted plan to the candidate that owns it. + let evicted_plans: BTreeSet = crate::plan::applied_plans_to_evict( + applied.iter().map(|(_, plan)| *plan).collect(), + retention, + now_ts, + ) + .into_iter() + .map(|plan| plan.name_any()) + .collect(); + prune.extend( + applied + .into_iter() + .filter(|(_, plan)| evicted_plans.contains(&plan.name_any())) + .map(|(candidate, _)| candidate), + ); + prune +} + /// Read the `Ready` condition reason from a candidate status, for tests and /// status consumers that only care about the current verdict. pub fn ready_reason(status: &PostgresPolicyCandidateStatus) -> Option<&str> { @@ -1535,6 +1633,172 @@ mod tests { /// The TTL in hours, so fixtures can sit either side of it precisely. const TTL_HOURS: i64 = DEFAULT_OPEN_CANDIDATE_TTL.as_hours(); + // ----------------------------------------------------------------- + // Terminal-candidate pruning + // ----------------------------------------------------------------- + + /// A terminal candidate created `age_secs` ago. + fn terminal_candidate( + name: &str, + phase: CandidatePhase, + age_secs: i64, + now_ts: i64, + ) -> PostgresPolicyCandidate { + let mut candidate = candidate(name, PolicyContent::default()); + candidate.metadata.uid = Some(format!("{name}-uid")); + candidate.metadata.creation_timestamp = + Some(k8s_openapi::apimachinery::pkg::apis::meta::v1::Time( + jiff::Timestamp::from_second(now_ts - age_secs).expect("epoch second in range"), + )); + candidate.status = Some(PostgresPolicyCandidateStatus { + phase, + ..Default::default() + }); + candidate + } + + /// An `Applied` plan that executed `applied_age_secs` ago. + fn applied_plan(name: &str, applied_age_secs: i64, now_ts: i64) -> PostgresPolicyPlan { + let spec = crate::crd::PostgresPolicyPlanSpec { + policy_ref: crate::crd::PolicyPlanRef { + name: "orders".to_string(), + }, + policy_generation: 1, + reconciliation_mode: crate::crd::CrdReconciliationMode::Authoritative, + owned_roles: Vec::new(), + owned_schemas: Vec::new(), + managed_database_identity: "default/db/DATABASE_URL".to_string(), + origin: None, + scope: None, + }; + let mut plan = PostgresPolicyPlan::new(name, spec); + plan.status = Some(crate::crd::PostgresPolicyPlanStatus { + phase: PlanPhase::Applied, + applied_at: Some( + jiff::Timestamp::from_second(now_ts - applied_age_secs) + .expect("epoch second in range") + .to_string(), + ), + ..Default::default() + }); + plan + } + + fn keep_plan(mut plan: PostgresPolicyPlan) -> PostgresPolicyPlan { + plan.metadata + .labels + .get_or_insert_with(Default::default) + .insert("pgroles.io/keep".to_string(), "true".to_string()); + plan + } + + fn pruned_names( + records: &[(&PostgresPolicyCandidate, Vec<&PostgresPolicyPlan>)], + retention: crate::plan::PlanRetention, + now_ts: i64, + ) -> Vec { + let mut names: Vec = terminal_candidates_to_prune(records, retention, now_ts) + .into_iter() + .map(|candidate| candidate.name_any()) + .collect(); + names.sort(); + names + } + + /// The recommended workflow's failure mode: every change lands as a + /// candidate, so terminal proposals accumulate fast, and under one flat + /// creation-time bound the eleventh proposal deletes the oldest terminal + /// candidate — cascading to the Applied plan it owns, straight through + /// the retention promise made for Applied plans. + #[test] + fn proposal_churn_cannot_prune_a_promoted_candidate_with_a_fresh_applied_plan() { + let now = 1_700_000_000; + let retention = crate::plan::PlanRetention::default(); + + // The promoted candidate is the oldest object in the set by a wide + // margin, and its plan applied a minute ago. + let promoted = terminal_candidate("promoted", CandidatePhase::Promoted, 100_000, now); + let promoted_plan = applied_plan("promoted-plan", 60, now); + + let churn: Vec = (0..DEFAULT_MAX_TERMINAL_CANDIDATES + 2) + .map(|i| { + terminal_candidate( + &format!("churn-{i:03}"), + CandidatePhase::Superseded, + 1_000 - i as i64, + now, + ) + }) + .collect(); + + let mut records: Vec<(&PostgresPolicyCandidate, Vec<&PostgresPolicyPlan>)> = + vec![(&promoted, vec![&promoted_plan])]; + records.extend( + churn + .iter() + .map(|candidate| (candidate, Vec::<&PostgresPolicyPlan>::new())), + ); + + // Guard against a vacuous pass: the promoted candidate must be the + // oldest by creation, so a flat creation-time bound over the whole + // set — the behaviour this test exists to reject — would prune it + // first. + assert!( + churn + .iter() + .all(|c| c.metadata.creation_timestamp > promoted.metadata.creation_timestamp), + "the fixture must make the promoted candidate the oldest object" + ); + + let pruned = pruned_names(&records, retention, now); + assert_eq!( + pruned, + vec!["churn-000".to_string(), "churn-001".to_string()], + "exactly the excess churn goes, oldest first" + ); + assert!( + !pruned.contains(&"promoted".to_string()), + "a promoted candidate with a fresh Applied plan is execution history, not churn" + ); + } + + /// Deleting the candidate cascades to its plans, so `pgroles.io/keep=true` + /// on the child plan has to protect the pair — a keep the cascade would + /// ignore is not a keep. + #[test] + fn a_keep_label_on_the_child_plan_protects_the_candidate() { + let now = 1_700_000_000; + // A floor of zero and a bound of one, so the applied bucket must + // evict — only exemptions can spare anything here. + let retention = crate::plan::PlanRetention { + applied: 1, + applied_ceiling: 1, + applied_min_age_secs: 0, + ..Default::default() + }; + + let oldest = terminal_candidate("p-old", CandidatePhase::Promoted, 900, now); + let oldest_plan = keep_plan(applied_plan("p-old-plan", 300, now)); + let middle = terminal_candidate("p-mid", CandidatePhase::Promoted, 800, now); + let middle_plan = applied_plan("p-mid-plan", 200, now); + let newest = terminal_candidate("p-new", CandidatePhase::Promoted, 700, now); + let newest_plan = applied_plan("p-new-plan", 100, now); + + let records: Vec<(&PostgresPolicyCandidate, Vec<&PostgresPolicyPlan>)> = vec![ + (&oldest, vec![&oldest_plan]), + (&middle, vec![&middle_plan]), + (&newest, vec![&newest_plan]), + ]; + + // p-old's plan is the earliest execution, so without the keep it is + // the first eviction; pruning the *younger* p-mid instead is what + // proves the child's label protected its parent. + assert_eq!( + pruned_names(&records, retention, now), + vec!["p-mid".to_string()] + ); + } + fn keep(mut candidate: PostgresPolicyCandidate) -> PostgresPolicyCandidate { candidate .metadata diff --git a/crates/pgroles-operator/src/context.rs b/crates/pgroles-operator/src/context.rs index 1fcc53a0..b00ec176 100644 --- a/crates/pgroles-operator/src/context.rs +++ b/crates/pgroles-operator/src/context.rs @@ -16,6 +16,7 @@ use tokio::sync::{Mutex, RwLock}; use crate::crd::{ConnectionAuth, ConnectionSpec, SecretKeySelector}; use crate::observability::OperatorObservability; +use crate::plan::PlanRetention; use crate::request_index::RequestIndex; /// Minimum pool size required for reconciliation. @@ -414,6 +415,9 @@ pub struct OperatorContext { /// Optional namespace which bounds every operator watch and list. pub watch_namespace: Option, + /// Terminal-plan retention bounds, shared by every policy reconcile. + pub plan_retention: PlanRetention, + /// Fetches short-lived provider-backed database passwords. gcp_token_provider: Arc, } @@ -435,11 +439,20 @@ impl OperatorContext { observability, request_index, watch_namespace, + plan_retention: PlanRetention::default(), database_locks: Arc::new(Mutex::new(HashMap::new())), gcp_token_provider: Arc::new(MetadataGcpAccessTokenProvider::default()), } } + /// Replace the default plan retention bounds — typically with the + /// environment-derived values resolved once at startup by + /// [`PlanRetention::from_env`]. + pub fn with_plan_retention(mut self, plan_retention: PlanRetention) -> Self { + self.plan_retention = plan_retention; + self + } + /// Try to acquire the in-process lock for the given database identity. /// /// Returns `Some(guard)` if no other reconcile is in progress for this diff --git a/crates/pgroles-operator/src/ephemeral.rs b/crates/pgroles-operator/src/ephemeral.rs index f3a0aed2..15e200f3 100644 --- a/crates/pgroles-operator/src/ephemeral.rs +++ b/crates/pgroles-operator/src/ephemeral.rs @@ -451,7 +451,7 @@ fn set_condition( conditions.push(condition); } -fn parse_duration(value: &str) -> Result { +pub(crate) fn parse_duration(value: &str) -> Result { let value = value.trim(); if value.is_empty() { return Err(EphemeralError::Invalid( diff --git a/crates/pgroles-operator/src/main.rs b/crates/pgroles-operator/src/main.rs index eedf8a0b..f576cc5a 100644 --- a/crates/pgroles-operator/src/main.rs +++ b/crates/pgroles-operator/src/main.rs @@ -27,6 +27,7 @@ use pgroles_operator::ephemeral::{ use pgroles_operator::observability::{ OperatorObservability, init_log_provider_from_env, serve_health, }; +use pgroles_operator::plan::PlanRetention; use pgroles_operator::reconciler::{error_policy, reconcile}; use pgroles_operator::request_index::RequestIndex; @@ -140,14 +141,28 @@ async fn main() -> anyhow::Result<()> { } let request_index = RequestIndex::default(); + // Resolved once here so a malformed value refuses startup with the exact + // variable named, instead of surfacing later as retention quietly running + // with bounds the environment did not ask for. + let plan_retention = PlanRetention::from_env()?; + if plan_retention != PlanRetention::default() { + info!( + ?plan_retention, + "plan retention bounds set from environment" + ); + } + // Create the shared operator context. - let ctx = Arc::new(OperatorContext::new_with_runtime_config( - client.clone(), - observability.clone(), - event_recorder, - request_index.clone(), - watch_namespace.clone(), - )); + let ctx = Arc::new( + OperatorContext::new_with_runtime_config( + client.clone(), + observability.clone(), + event_recorder, + request_index.clone(), + watch_namespace.clone(), + ) + .with_plan_retention(plan_retention), + ); // Watch all PostgresPolicy resources across all namespaces. let policies: Api = match &watch_namespace { diff --git a/crates/pgroles-operator/src/plan.rs b/crates/pgroles-operator/src/plan.rs index 378bfe88..f4ed73b1 100644 --- a/crates/pgroles-operator/src/plan.rs +++ b/crates/pgroles-operator/src/plan.rs @@ -84,8 +84,205 @@ const ORPHAN_GRACE_SECS: i64 = 60; /// Best-effort cleanup should never block a fresh reconcile for long. const CLEANUP_TIMEOUT_SECS: u64 = 5; -/// Default maximum number of historical plans to retain per policy. -const DEFAULT_MAX_PLANS: usize = 10; +/// How many terminal plans of each kind retention keeps for one policy. +/// +/// Split by phase rather than pooled, because the phases are not worth the +/// same and the cheapest one is the one generated fastest. `Superseded` is +/// churn: every replan supersedes its predecessor, so on an active policy a +/// single pool fills with records of plans that never ran and evicts the +/// `Applied` ones, which are the record of what did. +/// +/// The bounds are operator-level configuration: each field can be overridden +/// by its `PLAN_RETENTION_*` environment variable (see [`Self::from_env`]). +/// They are deliberately not per-policy CRD fields — retention caps object +/// growth in the cluster, it is not policy intent, and the per-object need is +/// served by the `pgroles.io/keep=true` label. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PlanRetention { + /// `Applied` plans kept once they are older than `applied_min_age_secs`. + pub applied: usize, + /// Hard ceiling on `Applied` plans, age floor notwithstanding. Without it + /// a policy applying continuously would grow the count without limit for + /// the whole floor period; the floor is a promise about history, not a + /// licence to keep everything. + pub applied_ceiling: usize, + /// An `Applied` plan younger than this is kept whatever `applied` says, + /// so the audit trail spans a stated period instead of however long the + /// policy's churn rate happens to make it. + pub applied_min_age_secs: i64, + /// `Failed` and `Rejected` plans. Both are decisions worth reading back — + /// why something did not run, and who declined it. + pub decided: usize, + /// `Superseded` plans. Deliberately small: it is the least informative + /// terminal state and the one generating the pressure. A couple is enough + /// to see what a replan replaced. + pub superseded: usize, +} + +impl Default for PlanRetention { + fn default() -> Self { + Self { + applied: 25, + applied_ceiling: 200, + applied_min_age_secs: 30 * 24 * 60 * 60, + decided: 10, + superseded: 3, + } + } +} + +impl PlanRetention { + /// Overrides [`PlanRetention::applied`]. + pub const ENV_APPLIED: &'static str = "PLAN_RETENTION_APPLIED"; + /// Overrides [`PlanRetention::applied_ceiling`]. + pub const ENV_APPLIED_CEILING: &'static str = "PLAN_RETENTION_APPLIED_CEILING"; + /// Overrides [`PlanRetention::applied_min_age_secs`]. Takes the same + /// `s`/`m`/`h` duration syntax as the `EPHEMERAL_ACCESS_*` ceilings. + pub const ENV_APPLIED_MIN_AGE: &'static str = "PLAN_RETENTION_APPLIED_MIN_AGE"; + /// Overrides [`PlanRetention::decided`]. + pub const ENV_DECIDED: &'static str = "PLAN_RETENTION_DECIDED"; + /// Overrides [`PlanRetention::superseded`]. + pub const ENV_SUPERSEDED: &'static str = "PLAN_RETENTION_SUPERSEDED"; + + /// Resolve the retention bounds from the process environment. + /// + /// Unset variables keep their defaults. An invalid value is an error, not + /// a fallback: this is meant to be called once at startup, where refusing + /// to start is the only failure mode an operator of the operator will + /// actually see. Retention that silently ran with different bounds than + /// the environment asked for would only be discovered when the plan + /// someone wanted was already deleted. + pub fn from_env() -> Result { + let mut resolved = std::collections::BTreeMap::new(); + for variable in [ + Self::ENV_APPLIED, + Self::ENV_APPLIED_CEILING, + Self::ENV_APPLIED_MIN_AGE, + Self::ENV_DECIDED, + Self::ENV_SUPERSEDED, + ] { + if let Some(value) = env_read(variable, std::env::var(variable))? { + resolved.insert(variable, value); + } + } + Self::from_lookup(|variable| resolved.get(variable).cloned()) + } + + /// [`Self::from_env`] over an arbitrary lookup, so parsing and validation + /// are testable without mutating process-global environment state. + fn from_lookup( + lookup: impl Fn(&str) -> Option, + ) -> Result { + let defaults = Self::default(); + let retention = Self { + applied: count_bound(&lookup, Self::ENV_APPLIED, defaults.applied)?, + applied_ceiling: count_bound( + &lookup, + Self::ENV_APPLIED_CEILING, + defaults.applied_ceiling, + )?, + applied_min_age_secs: age_bound( + &lookup, + Self::ENV_APPLIED_MIN_AGE, + defaults.applied_min_age_secs, + )?, + decided: count_bound(&lookup, Self::ENV_DECIDED, defaults.decided)?, + superseded: count_bound(&lookup, Self::ENV_SUPERSEDED, defaults.superseded)?, + }; + // A ceiling below the count bound could never take effect — eviction + // already stops at the count — so the configuration says one thing + // and the operator would do another. Reject it instead. + if retention.applied_ceiling < retention.applied { + return Err(PlanRetentionConfigError::CeilingBelowCount { + ceiling: retention.applied_ceiling, + count: retention.applied, + }); + } + Ok(retention) + } +} + +/// Classify one raw environment read: present, absent, or refused. +/// +/// `NotUnicode` is the one `VarError` that must not degrade to "unset": a +/// variable someone set, however malformed, silently taking the default is +/// exactly the failure mode this configuration's startup validation exists +/// to prevent. +fn env_read( + variable: &'static str, + read: Result, +) -> Result, PlanRetentionConfigError> { + match read { + Ok(value) => Ok(Some(value)), + Err(std::env::VarError::NotPresent) => Ok(None), + Err(std::env::VarError::NotUnicode(_)) => { + Err(PlanRetentionConfigError::NotUnicode { variable }) + } + } +} + +/// A count bound from `lookup`, or `default` when the variable is unset. +fn count_bound( + lookup: impl Fn(&str) -> Option, + variable: &'static str, + default: usize, +) -> Result { + match lookup(variable) { + None => Ok(default), + Some(value) => value + .trim() + .parse() + .map_err(|_| PlanRetentionConfigError::InvalidCount { variable, value }), + } +} + +/// An age bound in seconds from `lookup`, or `default` when the variable is +/// unset. The value uses the same duration syntax as the `EPHEMERAL_ACCESS_*` +/// variables. +fn age_bound( + lookup: impl Fn(&str) -> Option, + variable: &'static str, + default: i64, +) -> Result { + let Some(value) = lookup(variable) else { + return Ok(default); + }; + let invalid = |detail: String| PlanRetentionConfigError::InvalidDuration { + variable, + value: value.clone(), + detail, + }; + let duration = crate::ephemeral::parse_duration(&value).map_err(|error| { + invalid(match error { + crate::ephemeral::EphemeralError::Invalid(detail) => detail, + other => other.to_string(), + }) + })?; + i64::try_from(duration.as_secs()).map_err(|_| invalid("duration is too large".to_string())) +} + +/// Why [`PlanRetention::from_env`] refused the environment's values. +#[derive(Debug, thiserror::Error)] +pub enum PlanRetentionConfigError { + #[error("{variable} must be a non-negative integer, got {value:?}")] + InvalidCount { + variable: &'static str, + value: String, + }, + #[error("{variable} is not a valid duration ({detail}), got {value:?}")] + InvalidDuration { + variable: &'static str, + value: String, + detail: String, + }, + #[error( + "PLAN_RETENTION_APPLIED_CEILING ({ceiling}) is below PLAN_RETENTION_APPLIED ({count}); \ + a ceiling under the count bound can never take effect" + )] + CeilingBelowCount { ceiling: usize, count: usize }, + #[error("{variable} is set to a value that is not valid Unicode")] + NotUnicode { variable: &'static str }, +} /// How recently a Failed plan must have been created (in seconds) for the /// dedup check to consider it a match. Plans older than this are ignored so @@ -413,6 +610,7 @@ pub async fn create_or_update_plan( target_identity: &TargetIdentity, change_summary: &ChangeSummary, password_source_versions: &BTreeMap, + plan_retention: PlanRetention, candidate: Option>, ) -> Result { let namespace = policy.namespace().ok_or(ReconcileError::NoNamespace)?; @@ -469,7 +667,7 @@ pub async fn create_or_update_plan( // Candidate plans are pruned by candidate retention (their owner cascades), // never by the policy's plan retention, which would not see them anyway. if candidate.is_none() { - cleanup_old_plans_best_effort(client, policy, None).await; + cleanup_old_plans_best_effort(client, policy, plan_retention).await; } let plans_api: Api = Api::namespaced(client.clone(), &namespace); @@ -1286,11 +1484,11 @@ pub(crate) async fn execute_changes_in_transaction( pub async fn cleanup_old_plans_best_effort( client: &Client, policy: &PostgresPolicy, - max_plans: Option, + retention: PlanRetention, ) { match tokio::time::timeout( Duration::from_secs(CLEANUP_TIMEOUT_SECS), - cleanup_old_plans(client, policy, max_plans), + cleanup_old_plans(client, policy, retention), ) .await { @@ -1303,19 +1501,19 @@ pub async fn cleanup_old_plans_best_effort( } } -/// Clean up old plans for a policy, retaining at most `max_plans` terminal plans. +/// Clean up old plans for a policy under [`PlanRetention`]. /// -/// Terminal plans are those in Applied, Failed, Superseded, or Rejected phase; -/// Pending, Approved, and Applying plans are retained. Status-less plans and -/// SQL ConfigMaps older than a short grace period are treated as stale orphans. +/// Terminal plans — Applied, Failed, Superseded, Rejected — are bounded per +/// phase; Pending, Approved, and Applying plans are never evicted, because +/// they are still live. Status-less plans and SQL ConfigMaps older than a +/// short grace period are treated as stale orphans. pub async fn cleanup_old_plans( client: &Client, policy: &PostgresPolicy, - max_plans: Option, + retention: PlanRetention, ) -> Result<(), ReconcileError> { let namespace = policy.namespace().ok_or(ReconcileError::NoNamespace)?; let policy_name = policy.name_any(); - let max_plans = max_plans.unwrap_or(DEFAULT_MAX_PLANS); let plans_api: Api = Api::namespaced(client.clone(), &namespace); let selector = policy_selector(&policy_name); @@ -1346,51 +1544,20 @@ pub async fn cleanup_old_plans( } } - // Collect terminal plans sorted by creation timestamp (oldest first). - // `pgroles.io/keep=true` exempts an object from the bound: retention is a - // cap on unbounded growth, not a policy about what an operator may keep. - let mut terminal_plans: Vec<&PostgresPolicyPlan> = existing_plans - .iter() - .filter(|plan| !is_retention_exempt(*plan)) - .filter(|plan| { - plan.status - .as_ref() - .map(|s| { - matches!( - s.phase, - PlanPhase::Applied - | PlanPhase::Failed - | PlanPhase::Superseded - | PlanPhase::Rejected - ) - }) - .unwrap_or(false) - }) - .collect(); - - if terminal_plans.len() > max_plans { - // Sort by creation timestamp ascending (oldest first). - terminal_plans.sort_by(|a, b| { - let a_time = a.metadata.creation_timestamp.as_ref(); - let b_time = b.metadata.creation_timestamp.as_ref(); - a_time.cmp(&b_time) - }); - - let plans_to_delete = terminal_plans.len() - max_plans; - for plan in terminal_plans.into_iter().take(plans_to_delete) { - let plan_name = plan.name_any(); - info!( + for plan in plans_to_evict(&existing_plans, retention, now_ts) { + let plan_name = plan.name_any(); + info!( + plan = %plan_name, + policy = %policy_name, + phase = ?plan.status.as_ref().map(|status| &status.phase), + "cleaning up old plan" + ); + if let Err(err) = plans_api.delete(&plan_name, &DeleteParams::default()).await { + tracing::warn!( plan = %plan_name, - policy = %policy_name, - "cleaning up old plan" + %err, + "failed to delete old plan during cleanup" ); - if let Err(err) = plans_api.delete(&plan_name, &DeleteParams::default()).await { - tracing::warn!( - plan = %plan_name, - %err, - "failed to delete old plan during cleanup" - ); - } } } @@ -1491,6 +1658,135 @@ fn should_patch_existing_plan_status(plan: &PostgresPolicyPlan) -> bool { .unwrap_or(true) } +/// Which terminal plans retention evicts this pass. +/// +/// Pure and takes `now` as an argument, so the policy is testable without a +/// cluster — the bounds are the part worth pinning, and they are invisible in +/// an integration test that would have to create hundreds of objects to reach +/// them. +/// +/// `pgroles.io/keep=true` exempts an object from every bound: retention caps +/// unbounded growth, it is not a policy about what an operator may keep. +fn plans_to_evict( + plans: &[PostgresPolicyPlan], + retention: PlanRetention, + now_ts: i64, +) -> Vec<&PostgresPolicyPlan> { + let mut applied = Vec::new(); + let mut decided = Vec::new(); + let mut superseded = Vec::new(); + + for plan in plans.iter().filter(|plan| !is_retention_exempt(*plan)) { + let Some(phase) = plan.status.as_ref().map(|status| &status.phase) else { + continue; + }; + match phase { + PlanPhase::Applied => applied.push(plan), + PlanPhase::Failed | PlanPhase::Rejected => decided.push(plan), + PlanPhase::Superseded => superseded.push(plan), + // Still live. Never evicted, however old. + PlanPhase::Pending | PlanPhase::Approved | PlanPhase::Applying => {} + } + } + + let mut evict = oldest_beyond(&mut decided, retention.decided); + evict.extend(oldest_beyond(&mut superseded, retention.superseded)); + evict.extend(applied_plans_to_evict(applied, retention, now_ts)); + evict +} + +/// Which `Applied` plans the retention bounds evict from `applied`. +/// +/// Applied is the audit record of what actually ran, so its count bound +/// yields to the age floor. Walk oldest first and stop once the count bound +/// is met; spare anything still inside the floor along the way, unless the +/// survivors would breach the ceiling. Sparing is what makes the retained +/// span a promise rather than a function of how often the policy applies. +/// +/// Both the order and the floor measure from [`applied_epoch_secs`] — one +/// notion of "when did this apply" drives the whole walk. Ordering by +/// creation instead would evict a plan created before a long review but +/// applied moments ago, ahead of older executions whose objects happen to be +/// newer. +/// +/// Shared with terminal-candidate pruning: deleting a promoted candidate +/// cascades to the `Applied` plan it owns, so the candidate is held to these +/// same bounds (see `candidate::terminal_candidates_to_prune`). +pub(crate) fn applied_plans_to_evict( + mut applied: Vec<&PostgresPolicyPlan>, + retention: PlanRetention, + now_ts: i64, +) -> Vec<&PostgresPolicyPlan> { + applied.sort_by_key(|plan| applied_epoch_secs(plan)); + let mut evict = Vec::new(); + let mut remaining = applied.len(); + for plan in applied { + if remaining <= retention.applied { + break; + } + let inside_floor = applied_age_secs(plan, now_ts) < retention.applied_min_age_secs; + if inside_floor && remaining <= retention.applied_ceiling { + continue; + } + evict.push(plan); + remaining -= 1; + } + evict +} + +/// Sort oldest first and return everything past `keep`. +fn oldest_beyond<'a>( + plans: &mut Vec<&'a PostgresPolicyPlan>, + keep: usize, +) -> Vec<&'a PostgresPolicyPlan> { + if plans.len() <= keep { + return Vec::new(); + } + sort_oldest_first(plans); + plans[..plans.len() - keep].to_vec() +} + +fn sort_oldest_first(plans: &mut [&PostgresPolicyPlan]) { + plans.sort_by(|a, b| { + a.metadata + .creation_timestamp + .as_ref() + .cmp(&b.metadata.creation_timestamp.as_ref()) + }); +} + +/// When an `Applied` plan applied, as epoch seconds: `status.appliedAt`, +/// falling back to the creation timestamp when it is absent or unparseable so +/// plans recorded before it was set keep the creation-time behaviour rather +/// than reading as infinitely old. `None` when neither instant is known. +/// +/// This is the single notion of "when did this apply" behind both the +/// retention floor and the eviction order — the floor promises +/// post-application history, and a plan can sit `Pending` or `Approved` for +/// arbitrarily long before a reviewer decides it, so measuring from creation +/// would read a plan approved late as already outside that promise. +fn applied_epoch_secs(plan: &PostgresPolicyPlan) -> Option { + plan.status + .as_ref() + .and_then(|status| status.applied_at.as_deref()) + .and_then(parse_rfc3339_epoch_secs) + .or_else(|| { + plan.metadata + .creation_timestamp + .as_ref() + .map(|timestamp| timestamp.0.as_second()) + }) +} + +/// Age of an `Applied` plan for the retention floor — see +/// [`applied_epoch_secs`]. An unknown instant is age 0: brand new is the safe +/// direction for a deletion decision. +fn applied_age_secs(plan: &PostgresPolicyPlan, now_ts: i64) -> i64 { + applied_epoch_secs(plan) + .map(|applied_ts| now_ts.saturating_sub(applied_ts)) + .unwrap_or(0) +} + fn is_stale_statusless_plan(plan: &PostgresPolicyPlan, now_ts: i64) -> bool { plan.status.is_none() && is_stale_object(plan, now_ts) } @@ -2397,6 +2693,445 @@ mod tests { } /// Build a plan in `phase`, having recorded a failure `age_secs` ago. + /// A terminal plan of `phase`, created `age_secs` ago. + fn retained_plan( + name: &str, + phase: PlanPhase, + age_secs: i64, + now_ts: i64, + ) -> PostgresPolicyPlan { + let mut plan = PostgresPolicyPlan::new(name, test_plan_spec()); + plan.metadata.creation_timestamp = + Some(k8s_openapi::apimachinery::pkg::apis::meta::v1::Time( + jiff::Timestamp::from_second(now_ts - age_secs).expect("epoch second in range"), + )); + plan.status = Some(PostgresPolicyPlanStatus { + phase, + ..Default::default() + }); + plan + } + + fn evicted_names( + plans: &[PostgresPolicyPlan], + retention: PlanRetention, + now_ts: i64, + ) -> Vec { + let mut names: Vec = plans_to_evict(plans, retention, now_ts) + .into_iter() + .map(|plan| plan.name_any()) + .collect(); + names.sort(); + names + } + + /// The bug this split exists for: replan churn must not evict the record + /// of what actually ran. Under one pooled bound the Superseded plans, being + /// newer, kept every slot and the Applied ones were deleted. + #[test] + fn superseded_churn_does_not_evict_applied_plans() { + let now = 1_700_000_000; + let retention = PlanRetention::default(); + let year = 400 * 24 * 60 * 60; + + let mut plans = vec![retained_plan("applied-old", PlanPhase::Applied, year, now)]; + // Far more superseded plans than any pooled bound would have allowed, + // all newer than the applied one. + for i in 0..50 { + plans.push(retained_plan( + &format!("superseded-{i:03}"), + PlanPhase::Superseded, + 1000 - i, + now, + )); + } + + let evicted = evicted_names(&plans, retention, now); + assert!( + !evicted.contains(&"applied-old".to_string()), + "an applied plan must survive any amount of replan churn" + ); + assert_eq!( + evicted.len(), + 50 - retention.superseded, + "every superseded plan past the small bound is evicted" + ); + } + + #[test] + fn each_terminal_phase_is_bounded_separately() { + let now = 1_700_000_000; + let retention = PlanRetention::default(); + let year = 400 * 24 * 60 * 60; + + let mut plans = Vec::new(); + for (prefix, phase, count) in [ + ("applied", PlanPhase::Applied, retention.applied + 4), + ("failed", PlanPhase::Failed, retention.decided + 4), + ( + "superseded", + PlanPhase::Superseded, + retention.superseded + 4, + ), + ] { + for i in 0..count { + // Older than the age floor, so only the count bounds apply. + plans.push(retained_plan( + &format!("{prefix}-{i:03}"), + phase.clone(), + year + count as i64 - i as i64, + now, + )); + } + } + + let evicted = evicted_names(&plans, retention, now); + for prefix in ["applied", "failed", "superseded"] { + let count = evicted.iter().filter(|n| n.starts_with(prefix)).count(); + assert_eq!(count, 4, "{prefix} should lose exactly its 4 excess plans"); + } + } + + /// Failed and Rejected share one bound, so a policy that fails repeatedly + /// cannot bury the record of a plan a reviewer declined. + #[test] + fn failed_and_rejected_share_the_decided_bound() { + let now = 1_700_000_000; + let retention = PlanRetention::default(); + let year = 400 * 24 * 60 * 60; + + let mut plans = vec![retained_plan( + "rejected", + PlanPhase::Rejected, + year * 2, + now, + )]; + for i in 0..retention.decided { + plans.push(retained_plan( + &format!("failed-{i:03}"), + PlanPhase::Failed, + year - i as i64, + now, + )); + } + + // One over the shared bound, and the rejected plan is the oldest. + assert_eq!( + evicted_names(&plans, retention, now), + vec!["rejected".to_string()] + ); + } + + /// The age floor is what makes the retained span a promise rather than a + /// function of how often the policy applies. + #[test] + fn the_age_floor_keeps_applied_plans_past_the_count_bound() { + let now = 1_700_000_000; + let retention = PlanRetention::default(); + + let recent: Vec = (0..retention.applied + 10) + .map(|i| { + retained_plan( + &format!("applied-{i:03}"), + PlanPhase::Applied, + retention.applied_min_age_secs - 1 - i as i64, + now, + ) + }) + .collect(); + assert!( + evicted_names(&recent, retention, now).is_empty(), + "nothing inside the floor is evicted, even past the count bound" + ); + + // One second older and the same plan is outside the promise. + let stale: Vec = (0..retention.applied + 10) + .map(|i| { + retained_plan( + &format!("applied-{i:03}"), + PlanPhase::Applied, + retention.applied_min_age_secs + 1 + i as i64, + now, + ) + }) + .collect(); + assert_eq!(evicted_names(&stale, retention, now).len(), 10); + } + + /// The floor promises history; it does not license unbounded growth. + #[test] + fn the_ceiling_overrides_the_age_floor() { + let now = 1_700_000_000; + let retention = PlanRetention::default(); + + let plans: Vec = (0..retention.applied_ceiling + 40) + .map(|i| { + retained_plan( + &format!("applied-{i:04}"), + PlanPhase::Applied, + // All well inside the floor. + 60 + i as i64, + now, + ) + }) + .collect(); + + assert_eq!( + evicted_names(&plans, retention, now).len(), + 40, + "the ceiling trims back to itself, not to the count bound" + ); + } + + #[test] + fn live_plans_and_kept_plans_are_never_evicted() { + let now = 1_700_000_000; + let retention = PlanRetention::default(); + let year = 400 * 24 * 60 * 60; + + let mut plans = Vec::new(); + for (i, phase) in [PlanPhase::Pending, PlanPhase::Approved, PlanPhase::Applying] + .into_iter() + .enumerate() + { + plans.push(retained_plan(&format!("live-{i}"), phase, year * 2, now)); + } + // Enough superseded plans to blow past the bound many times over. + for i in 0..40 { + plans.push(retained_plan( + &format!("superseded-{i:03}"), + PlanPhase::Superseded, + year, + now, + )); + } + let mut kept = retained_plan("kept", PlanPhase::Superseded, year * 3, now); + kept.metadata + .labels + .get_or_insert_with(Default::default) + .insert("pgroles.io/keep".to_string(), "true".to_string()); + plans.push(kept); + + let evicted = evicted_names(&plans, retention, now); + assert!(!evicted.iter().any(|name| name.starts_with("live-"))); + assert!(!evicted.contains(&"kept".to_string())); + assert_eq!(evicted.len(), 40 - retention.superseded); + } + + /// [`PlanRetention::from_lookup`] over a fixed variable table. + fn retention_from(vars: &[(&str, &str)]) -> Result { + PlanRetention::from_lookup(|variable| { + vars.iter() + .find(|(name, _)| *name == variable) + .map(|(_, value)| value.to_string()) + }) + } + + #[test] + fn unset_retention_variables_keep_the_defaults() { + assert_eq!( + retention_from(&[]).expect("an empty environment is valid"), + PlanRetention::default() + ); + } + + #[test] + fn each_retention_variable_overrides_its_bound() { + let resolved = retention_from(&[ + (PlanRetention::ENV_APPLIED, "7"), + (PlanRetention::ENV_APPLIED_CEILING, "70"), + (PlanRetention::ENV_APPLIED_MIN_AGE, "36h"), + (PlanRetention::ENV_DECIDED, "4"), + (PlanRetention::ENV_SUPERSEDED, "1"), + ]) + .expect("all five values are valid"); + + let expected = PlanRetention { + applied: 7, + applied_ceiling: 70, + applied_min_age_secs: 36 * 60 * 60, + decided: 4, + superseded: 1, + }; + // Guard against this test going vacuous: if the fixture ever equalled + // the defaults, a lookup that ignored the environment entirely would + // still pass the assertion below. + assert_ne!(expected, PlanRetention::default()); + assert_eq!(resolved, expected); + } + + #[test] + fn an_invalid_retention_count_is_rejected_naming_the_variable() { + let error = retention_from(&[(PlanRetention::ENV_DECIDED, "many")]) + .expect_err("a non-numeric count must be rejected, not defaulted"); + assert!( + error.to_string().contains(PlanRetention::ENV_DECIDED), + "the error must name the variable to fix: {error}" + ); + } + + #[test] + fn an_invalid_retention_min_age_is_rejected_naming_the_variable() { + // Days are deliberately not a unit — this is the same syntax as the + // EPHEMERAL_ACCESS_* durations, and "30d" must fail loudly rather + // than resolve to something else. + let error = retention_from(&[(PlanRetention::ENV_APPLIED_MIN_AGE, "30d")]) + .expect_err("an unsupported duration unit must be rejected, not defaulted"); + assert!( + error + .to_string() + .contains(PlanRetention::ENV_APPLIED_MIN_AGE), + "the error must name the variable to fix: {error}" + ); + } + + #[test] + fn the_applied_floor_runs_from_when_the_plan_applied_not_when_it_was_created() { + // A plan can sit Pending or Approved for longer than the entire floor + // before a reviewer decides it. Measured from creation, such a plan is + // already outside the floor the moment it executes, and the cleanup + // that follows execution deletes it — the exact history the floor + // promises to keep. + let now = 1_700_000_000; + let retention = PlanRetention::default(); + + // Created far outside the floor, applied just now. Enough plans to + // exceed the count bound, but well under the ceiling, so only the + // floor can be what spares them. + let excess = 10; + assert!( + retention.applied + excess <= retention.applied_ceiling, + "this test must not lean on the ceiling to pass" + ); + let plans: Vec = (0..retention.applied + excess) + .map(|i| { + let mut plan = retained_plan( + &format!("applied-{i:03}"), + PlanPhase::Applied, + retention.applied_min_age_secs * 3 + i as i64, + now, + ); + plan.status + .as_mut() + .expect("retained_plan always sets a status") + .applied_at = Some( + jiff::Timestamp::from_second(now - 60 - i as i64) + .expect("epoch second in range") + .to_string(), + ); + plan + }) + .collect(); + + assert!( + evicted_names(&plans, retention, now).is_empty(), + "a plan applied inside the floor is kept, however old the object is" + ); + } + + #[test] + fn above_the_ceiling_eviction_order_follows_when_plans_applied_not_when_created() { + // Above the ceiling the floor is overridden and the walk evicts from + // the front, so the sort order decides who dies. Ordered by creation, + // a plan created before a long review but applied moments ago sorts + // first and is deleted by the cleanup right after it executes, while + // older executions whose objects happen to be newer survive. + let now = 1_700_000_000; + let retention = PlanRetention::default(); + let excess = 5; + let count = retention.applied_ceiling + excess; + + // Creation order is the exact reverse of applied order: plan 0 is the + // oldest object but the most recent execution. All applied inside the + // floor, so only the ceiling forces eviction. + let plans: Vec = (0..count) + .map(|i| { + let mut plan = retained_plan( + &format!("applied-{i:04}"), + PlanPhase::Applied, + retention.applied_min_age_secs * 3 + (count - i) as i64, + now, + ); + plan.status + .as_mut() + .expect("retained_plan always sets a status") + .applied_at = Some( + jiff::Timestamp::from_second(now - 100 - i as i64) + .expect("epoch second in range") + .to_string(), + ); + plan + }) + .collect(); + + let evicted = evicted_names(&plans, retention, now); + assert_eq!( + evicted.len(), + excess, + "the ceiling trims exactly the excess" + ); + for i in 0..excess { + let earliest_executed = format!("applied-{:04}", count - 1 - i); + assert!( + evicted.contains(&earliest_executed), + "the earliest executions go first, whatever their objects' creation order" + ); + } + assert!( + !evicted.contains(&"applied-0000".to_string()), + "the most recent execution must survive even as the oldest object by creation" + ); + } + + #[test] + fn a_non_unicode_environment_value_is_rejected_not_defaulted() { + use std::os::unix::ffi::OsStringExt; + + let read = Err(std::env::VarError::NotUnicode( + std::ffi::OsString::from_vec(vec![b'2', b'5', 0xff]), + )); + let error = env_read(PlanRetention::ENV_APPLIED, read) + .expect_err("a set-but-non-Unicode value must be rejected, not read as unset"); + assert!( + error.to_string().contains(PlanRetention::ENV_APPLIED), + "the error must name the variable to fix: {error}" + ); + + assert_eq!( + env_read( + PlanRetention::ENV_APPLIED, + Err(std::env::VarError::NotPresent) + ) + .expect("absent is not an error"), + None + ); + assert_eq!( + env_read(PlanRetention::ENV_APPLIED, Ok("25".to_string())) + .expect("a Unicode value is not an error"), + Some("25".to_string()) + ); + } + + #[test] + fn a_ceiling_below_the_applied_count_is_rejected() { + // Eviction stops at the count bound before the ceiling is consulted, + // so a smaller ceiling could never take effect; accepting it would + // mean running with different bounds than the environment states. + retention_from(&[ + (PlanRetention::ENV_APPLIED, "50"), + (PlanRetention::ENV_APPLIED_CEILING, "10"), + ]) + .expect_err("a ceiling below the count bound must be rejected"); + + // Equal is the degenerate-but-coherent form: the floor never spares + // anything, and that is exactly what the configuration says. + retention_from(&[ + (PlanRetention::ENV_APPLIED, "50"), + (PlanRetention::ENV_APPLIED_CEILING, "50"), + ]) + .expect("a ceiling equal to the count bound is valid"); + } + fn plan_failed_at(phase: PlanPhase, age_secs: i64, now_ts: i64) -> PostgresPolicyPlan { let mut plan = PostgresPolicyPlan::new("plan", test_plan_spec()); plan.status = Some(PostgresPolicyPlanStatus { diff --git a/crates/pgroles-operator/src/reconciler.rs b/crates/pgroles-operator/src/reconciler.rs index d63c5661..ee8cbdeb 100644 --- a/crates/pgroles-operator/src/reconciler.rs +++ b/crates/pgroles-operator/src/reconciler.rs @@ -1103,7 +1103,8 @@ async fn reconcile_apply_inner( // Release advisory lock (always, even on error). advisory_lock.release().await; - crate::plan::cleanup_old_plans_best_effort(&ctx.kube_client, resource, None).await; + crate::plan::cleanup_old_plans_best_effort(&ctx.kube_client, resource, ctx.plan_retention) + .await; result } @@ -1411,6 +1412,7 @@ async fn apply_under_lock( &target_identity, &summary, &applied_password_source_versions, + ctx.plan_retention, None, ) .await?; @@ -1530,6 +1532,7 @@ async fn apply_under_lock( &target_identity, &summary, &applied_password_source_versions, + ctx.plan_retention, None, ) .await?; @@ -1866,6 +1869,7 @@ async fn apply_under_lock( &target_identity, &summary, &applied_password_source_versions, + ctx.plan_retention, None, ) .await?; @@ -2197,6 +2201,7 @@ async fn apply_under_lock( &target_identity, &summary, &applied_password_source_versions, + ctx.plan_retention, None, ) .await?; @@ -2340,6 +2345,7 @@ async fn apply_under_lock( &target_identity, &summary, &applied_password_source_versions, + ctx.plan_retention, None, ) .await?; diff --git a/docs/src/pages/docs/operator-candidates.md b/docs/src/pages/docs/operator-candidates.md index 63c61fb8..44177fce 100644 --- a/docs/src/pages/docs/operator-candidates.md +++ b/docs/src/pages/docs/operator-candidates.md @@ -404,11 +404,66 @@ step one; it never performs the cutover itself. ## Retention Candidates carry an `ownerReference` to their parent policy, and each derived -plan is owned by its candidate, so pruning cascades. Terminal candidates — -`Promoted=True`, or `Superseded=True` for any reason including `PlanDenied` — -are pruned by the same bounded retention loop as plans; label a candidate -`pgroles.io/keep=true` to exempt it. Plans also expire after a TTL — an -approval is not an indefinite authorisation. +plan is owned by its candidate, so deleting a candidate cascades to its plan +and that plan's SQL ConfigMap. Pruning therefore decides on the pair, never +the candidate alone. A terminal candidate that owns no `Applied` plan — +superseded proposals of every kind, and promotions whose execution ran on +another plan — is proposal churn, bounded at 10 per policy, oldest first by +creation. A terminal candidate that owns an `Applied` plan is the provenance +of an execution record: it is held to the `Applied` bounds in the table below +— same count, age floor, and ceiling, ordered by when its plan applied — so +filing more proposals can never delete the record of what ran through the +owner object. +`pgroles.io/keep=true` on the candidate **or on its plan** exempts the pair. +Plans also expire after a TTL — an approval is not an indefinite +authorisation. + +### What plan retention keeps + +Terminal plans are bounded per phase, not as one pool, because the phases are +not worth the same and the cheapest one is generated fastest. Every replan +supersedes its predecessor, so under a single bound `Superseded` records — of +plans that never ran — would evict the `Applied` ones that record what did. + +| Phase | Retained | Notes | +| --- | --- | --- | +| `Applied` | 25, and never fewer than 30 days' worth | Hard ceiling of 200 | +| `Failed`, `Rejected` | 10, shared | Why something did not run, and who declined it | +| `Superseded` | 3 | Enough to see what a replan replaced | +| `Pending`, `Approved`, `Applying` | all | Still live; never evicted | + +The oldest go first within each bucket. For `Applied`, both the eviction +order and the age floor measure from `status.appliedAt` — not from creation, +since a plan can wait on a reviewer for arbitrarily long before it executes. +A plan applied inside the floor is kept even once the count is exceeded, so +the audit trail spans a stated period instead of however long the policy's +churn rate happens to make it. The ceiling overrides the floor — the floor is +a promise about history, not a licence to keep everything — and a policy applying hard +enough to reach 200 within the floor period will start losing its oldest. + +`pgroles.io/keep=true` exempts a plan from every one of these bounds. + +### Configuring the bounds + +Each bound is operator-level configuration, set by environment variable on the +operator Deployment (`operator.env` in the Helm chart) — the same mechanism as +the `EPHEMERAL_ACCESS_*` ceilings, and the same `s`/`m`/`h` duration syntax: + +| Variable | Default | Sets | +| --- | --- | --- | +| `PLAN_RETENTION_APPLIED` | `25` | `Applied` count bound | +| `PLAN_RETENTION_APPLIED_MIN_AGE` | `720h` (30 days) | `Applied` age floor | +| `PLAN_RETENTION_APPLIED_CEILING` | `200` | `Applied` hard ceiling; must be at least `PLAN_RETENTION_APPLIED` | +| `PLAN_RETENTION_DECIDED` | `10` | `Failed` + `Rejected` shared bound | +| `PLAN_RETENTION_SUPERSEDED` | `3` | `Superseded` bound | + +An invalid value refuses operator startup with the variable named, rather than +silently running with bounds the environment did not ask for. + +There is deliberately no per-policy retention field on `PostgresPolicy`. +Retention caps object growth in the cluster — an operational concern, like the +open-candidate budget and TTL above — not per-policy intent; the per-object +need ("this specific record matters") is what `pgroles.io/keep=true` is for. ## Bounding open candidates diff --git a/docs/src/pages/docs/operator-plan-approval.md b/docs/src/pages/docs/operator-plan-approval.md index aff9b12b..1a438f82 100644 --- a/docs/src/pages/docs/operator-plan-approval.md +++ b/docs/src/pages/docs/operator-plan-approval.md @@ -255,6 +255,11 @@ For `password.generate` roles the operator synthesizes material in memory while planning and creates the Kubernetes Secret only during post-approval execution — see [Passwords and planning](#passwords-and-planning). +Terminal plans are not kept forever: per-phase bounds prune the oldest, with +`Applied` plans kept the longest. See [what plan retention +keeps](/docs/operator-candidates#what-plan-retention-keeps) for the bounds, +how to configure them, and the `pgroles.io/keep=true` exemption. + ### Who may decide The trust model has two layers, and both matter: