From aa85df76073fcc0214a90875572c1fb66f819e71 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:33:07 +0000 Subject: [PATCH 1/4] Make removal::mark own every statement over config_removal_mark The 'no live removal names this Environment' condition was pasted into six queries in pull_request, follow and proposal, IDLE_FACTS carried the Project-aware variant, and the merge undo deleted the mark with raw SQL. mark.rs now exposes unmarked, marks_holding and drop_live, and the state literals live there only. --- core/crates/ployz-store/src/branch/follow.rs | 9 +- .../crates/ployz-store/src/branch/proposal.rs | 37 +++++---- .../ployz-store/src/pull_request/mod.rs | 82 +++++++++++-------- core/crates/ployz-store/src/removal/mark.rs | 71 ++++++++++++++-- 4 files changed, 134 insertions(+), 65 deletions(-) diff --git a/core/crates/ployz-store/src/branch/follow.rs b/core/crates/ployz-store/src/branch/follow.rs index 338136869c..6facdd6d1c 100644 --- a/core/crates/ployz-store/src/branch/follow.rs +++ b/core/crates/ployz-store/src/branch/follow.rs @@ -5,6 +5,7 @@ use super::*; use crate::Trusted; +use crate::removal::mark; use ployz_core::RpcErrorCode; /// A Parent's deployed value that followed into its Branch but isn't staged there: @@ -52,9 +53,11 @@ pub(crate) fn follow( ) -> Result<(), RpcError> { let trusted = &Trusted::default(); let branches = tx.query( - "SELECT environment_id, organization_id FROM config_environment_branch b \ - WHERE parent_id = ?1 AND NOT EXISTS (SELECT 1 FROM config_removal_mark m \ - WHERE m.environment_id = b.environment_id AND m.state = 'removing') ORDER BY environment_id", + &format!( + "SELECT environment_id, organization_id FROM config_environment_branch b \ + WHERE parent_id = ?1 AND {} ORDER BY environment_id", + mark::unmarked("b.environment_id") + ), &[parent.as_str().into()], )?; if branches.is_empty() || lineages.is_empty() { diff --git a/core/crates/ployz-store/src/branch/proposal.rs b/core/crates/ployz-store/src/branch/proposal.rs index 100b4e3f09..651ccef984 100644 --- a/core/crates/ployz-store/src/branch/proposal.rs +++ b/core/crates/ployz-store/src/branch/proposal.rs @@ -10,6 +10,7 @@ use super::*; use crate::Trusted; use crate::id::{ProposalId, RepositoryId}; use crate::pull_request::PullRequestRef; +use crate::removal::mark; use crate::storage::Param; /// Remove a proposal from a draft: what it still owns there goes back to what the @@ -903,23 +904,25 @@ pub(crate) fn included( environment: &EnvironmentId, ) -> Result, RpcError> { let rows = tx.query( - "SELECT p.id, p.source_environment_id, p.source_name, p.source_revision, p.last_sync, \ - p.repository_id, p.number, \ - (SELECT COUNT(*) FROM config_sync_arrival a \ - WHERE a.environment_id = p.environment_id AND a.proposal_id = p.id), \ - COALESCE(r.name, e.name), COALESCE(r.working_revision, e.working_revision), \ - r.id \ - FROM config_proposal p JOIN config_environment d ON d.id = p.environment_id \ - LEFT JOIN config_environment e ON e.id = p.source_environment_id \ - LEFT JOIN (SELECT q.environment_id AS id, q.repository_id, q.number, \ - v.project_id, v.name, v.working_revision \ - FROM config_pr_environment q JOIN config_environment v ON v.id = q.environment_id \ - JOIN config_environment_branch b ON b.environment_id = q.environment_id \ - WHERE NOT EXISTS (SELECT 1 FROM config_removal_mark m \ - WHERE m.environment_id = b.environment_id AND m.state = 'removing')) r \ - ON r.id <> p.source_environment_id AND r.repository_id = p.repository_id \ - AND r.number = p.number AND r.project_id = d.project_id \ - WHERE p.environment_id = ?1 ORDER BY p.source_name, p.id", + &format!( + "SELECT p.id, p.source_environment_id, p.source_name, p.source_revision, p.last_sync, \ + p.repository_id, p.number, \ + (SELECT COUNT(*) FROM config_sync_arrival a \ + WHERE a.environment_id = p.environment_id AND a.proposal_id = p.id), \ + COALESCE(r.name, e.name), COALESCE(r.working_revision, e.working_revision), \ + r.id \ + FROM config_proposal p JOIN config_environment d ON d.id = p.environment_id \ + LEFT JOIN config_environment e ON e.id = p.source_environment_id \ + LEFT JOIN (SELECT q.environment_id AS id, q.repository_id, q.number, \ + v.project_id, v.name, v.working_revision \ + FROM config_pr_environment q JOIN config_environment v ON v.id = q.environment_id \ + JOIN config_environment_branch b ON b.environment_id = q.environment_id \ + WHERE {}) r \ + ON r.id <> p.source_environment_id AND r.repository_id = p.repository_id \ + AND r.number = p.number AND r.project_id = d.project_id \ + WHERE p.environment_id = ?1 ORDER BY p.source_name, p.id", + mark::unmarked("b.environment_id") + ), &[environment.as_str().into()], )?; let mut included = Vec::with_capacity(rows.len()); diff --git a/core/crates/ployz-store/src/pull_request/mod.rs b/core/crates/ployz-store/src/pull_request/mod.rs index 5d22685871..c2bca3a09f 100644 --- a/core/crates/ployz-store/src/pull_request/mod.rs +++ b/core/crates/ployz-store/src/pull_request/mod.rs @@ -46,6 +46,7 @@ use crate::deployment::{self, DeploymentSummary}; use crate::error; use crate::id::{EnvironmentId, EnvironmentName, ProjectName}; use crate::project::ProjectSummary; +use crate::removal::mark; use crate::review::history::NotSetVariable; use crate::scope::{self, Environment, EnvironmentRef, EnvironmentSummary}; use crate::settings::NodeName; @@ -503,10 +504,13 @@ fn open_in( repository_id: RepositoryId, ) -> Result, RpcError> { let rows = tx.query( - "SELECT p.number, e.name FROM config_pr_environment p \ - JOIN config_environment e ON e.id = p.environment_id \ - JOIN config_environment_branch b ON b.environment_id = p.environment_id \ - WHERE e.project_id = ?1 AND p.repository_id = ?2 AND NOT EXISTS (SELECT 1 FROM config_removal_mark m WHERE m.environment_id = b.environment_id AND m.state = 'removing') ORDER BY p.number", + &format!( + "SELECT p.number, e.name FROM config_pr_environment p \ + JOIN config_environment e ON e.id = p.environment_id \ + JOIN config_environment_branch b ON b.environment_id = p.environment_id \ + WHERE e.project_id = ?1 AND p.repository_id = ?2 AND {} ORDER BY p.number", + mark::unmarked("b.environment_id") + ), &[project.as_str().into(), repository_id.into()], )?; let mut open = Vec::new(); @@ -804,11 +808,14 @@ pub(crate) fn project_checks( environment: &EnvironmentId, ) -> Result, RpcError> { let rows = tx.query( - "SELECT DISTINCT p.repository_id, p.number FROM config_pr_environment p \ - JOIN config_environment e ON e.id = p.environment_id \ - JOIN config_environment s ON s.project_id = e.project_id \ - JOIN config_environment_branch b ON b.environment_id = p.environment_id \ - WHERE s.id = ?1 AND NOT EXISTS (SELECT 1 FROM config_removal_mark m WHERE m.environment_id = b.environment_id AND m.state = 'removing') ORDER BY p.repository_id, p.number", + &format!( + "SELECT DISTINCT p.repository_id, p.number FROM config_pr_environment p \ + JOIN config_environment e ON e.id = p.environment_id \ + JOIN config_environment s ON s.project_id = e.project_id \ + JOIN config_environment_branch b ON b.environment_id = p.environment_id \ + WHERE s.id = ?1 AND {} ORDER BY p.repository_id, p.number", + mark::unmarked("b.environment_id") + ), &[environment.as_str().into()], )?; rows.iter().map(pull_request_ref).collect() @@ -1021,10 +1028,7 @@ pub(crate) fn pull_request( if !merged.all { kept.insert(environment.clone()); // A close that came before the merge is undone: it stays open. - tx.execute( - "DELETE FROM config_removal_mark WHERE environment_id = ?1 AND state = 'removing'", - &[environment.as_str().into()], - )?; + mark::drop_live(tx, environment)?; } } on_merge::record_unlanded(tx, who, &reported, &unlanded)?; @@ -1210,10 +1214,9 @@ fn current( closing: bool, ) -> Result, RpcError> { let open = if closing { - "" + String::new() } else { - "AND NOT EXISTS (SELECT 1 FROM config_removal_mark m \ - WHERE m.environment_id = b.environment_id AND m.state = 'removing')" + format!("AND {}", mark::unmarked("b.environment_id")) }; let rows = tx.query( &format!( @@ -1548,15 +1551,18 @@ fn shut_down( /// What the idle rule reads of each Branch: `environment_id`, kept, whether a /// removal names it (or a cancelled one did), its latest admission (-1 for none), /// its Branches and whether it is a Default. -const IDLE_FACTS: &str = "SELECT b.environment_id, b.kept, \ - (SELECT COUNT(*) FROM config_removal_mark m WHERE (m.environment_id = b.environment_id AND m.state <> 'removed') \ - OR (m.state = 'removing' AND m.project_id = (SELECT e.project_id FROM config_environment e WHERE e.id = b.environment_id))), \ - (SELECT COALESCE(MAX(d.admitted), -1) FROM config_deployment d WHERE d.environment_id = b.environment_id), \ - (SELECT COUNT(*) FROM config_environment_branch c WHERE c.parent_id = b.environment_id), \ - (SELECT COUNT(*) FROM config_project p WHERE p.default_environment_id = b.environment_id) \ - FROM config_environment_branch b"; - -/// When the sweep closes a Branch for sitting idle, from an [`IDLE_FACTS`] row: a +fn idle_facts() -> String { + format!( + "SELECT b.environment_id, b.kept, {}, \ + (SELECT COALESCE(MAX(d.admitted), -1) FROM config_deployment d WHERE d.environment_id = b.environment_id), \ + (SELECT COUNT(*) FROM config_environment_branch c WHERE c.parent_id = b.environment_id), \ + (SELECT COUNT(*) FROM config_project p WHERE p.default_environment_id = b.environment_id) \ + FROM config_environment_branch b", + mark::marks_holding("b.environment_id") + ) +} + +/// When the sweep closes a Branch for sitting idle, from an [`idle_facts`] row: a /// week after its latest Deployment. Never when it is kept, being removed or its /// removal was cancelled, never /// deployed, a Parent or the Default Environment. @@ -1569,7 +1575,7 @@ fn idle_close(row: &crate::storage::Row) -> Result, RpcError> { /// When a Branch closes for sitting idle, in seconds since the Unix epoch. pub(crate) fn closes_at(tx: &mut dyn Tx, id: &EnvironmentId) -> Result, RpcError> { let rows = tx.query( - &format!("{IDLE_FACTS} WHERE b.environment_id = ?1"), + &format!("{} WHERE b.environment_id = ?1", idle_facts()), &[id.as_str().into()], )?; rows.first().map_or(Ok(None), idle_close) @@ -1578,7 +1584,10 @@ pub(crate) fn closes_at(tx: &mut dyn Tx, id: &EnvironmentId) -> Result Result { let rows = tx.query( - &format!("{IDLE_FACTS} WHERE b.organization_id = ?1 ORDER BY b.environment_id"), + &format!( + "{} WHERE b.organization_id = ?1 ORDER BY b.environment_id", + idle_facts() + ), &[who.organization.as_str().into()], )?; // Closing would lose what it holds for its merge: an idle PR Environment that @@ -1636,15 +1645,16 @@ pub(crate) fn awaiting_merge( branch: &BranchName, ) -> Result, RpcError> { let rows = tx.query( - "SELECT DISTINCT p.number, r.facts, r.merged, r.unlanded, r.standing, r.phase \ - FROM config_pr_environment p \ - JOIN config_environment_branch b ON b.environment_id = p.environment_id \ - JOIN config_pull_request r ON r.organization_id = p.organization_id \ - AND r.repository_id = p.repository_id AND r.number = p.number \ - WHERE p.organization_id = ?1 AND p.repository_id = ?2 \ - AND NOT EXISTS (SELECT 1 FROM config_removal_mark m \ - WHERE m.environment_id = b.environment_id AND m.state = 'removing') \ - AND r.merged IS NULL ORDER BY p.number", + &format!( + "SELECT DISTINCT p.number, r.facts, r.merged, r.unlanded, r.standing, r.phase \ + FROM config_pr_environment p \ + JOIN config_environment_branch b ON b.environment_id = p.environment_id \ + JOIN config_pull_request r ON r.organization_id = p.organization_id \ + AND r.repository_id = p.repository_id AND r.number = p.number \ + WHERE p.organization_id = ?1 AND p.repository_id = ?2 \ + AND {} AND r.merged IS NULL ORDER BY p.number", + mark::unmarked("b.environment_id") + ), &[who.organization.as_str().into(), repository_id.into()], )?; let mut awaiting = Vec::new(); diff --git a/core/crates/ployz-store/src/removal/mark.rs b/core/crates/ployz-store/src/removal/mark.rs index 3c5e9751f0..40bf2aa6b1 100644 --- a/core/crates/ployz-store/src/removal/mark.rs +++ b/core/crates/ployz-store/src/removal/mark.rs @@ -132,6 +132,36 @@ fn state_text(state: State) -> &'static str { } } +/// What [`finish`] writes. It is no [`State`]: no query loads it back as a [`Mark`]. +const REMOVED: &str = "removed"; + +/// A SQL condition that holds when no live removal mark names the Environment whose ID +/// `environment_column` reads, as its own target. A Project's mark does not count: the +/// callers that splice this in leave Branches of a removing Project be. Splice it into +/// a `WHERE` and keep the alias `m` free. +pub(crate) fn unmarked(environment_column: &str) -> String { + format!( + "NOT EXISTS (SELECT 1 FROM config_removal_mark m \ + WHERE m.environment_id = {environment_column} AND m.state = '{}')", + state_text(State::Removing) + ) +} + +/// A SQL scalar counting the marks that hold the Environment whose ID +/// `environment_column` reads: its own mark in any state but `removed`, a tombstone +/// included, and the live mark of its Project. The idle rule reads it where +/// [`unmarked`] would not do, because a tombstone and a Project removal also keep a +/// Branch from closing for sitting idle. Keep the aliases `m` and `e` free. +pub(crate) fn marks_holding(environment_column: &str) -> String { + format!( + "(SELECT COUNT(*) FROM config_removal_mark m \ + WHERE (m.environment_id = {environment_column} AND m.state <> '{REMOVED}') \ + OR (m.state = '{removing}' AND m.project_id = \ + (SELECT e.project_id FROM config_environment e WHERE e.id = {environment_column})))", + removing = state_text(State::Removing) + ) +} + /// `target`'s live mark or its tombstone. pub(crate) fn of(tx: &mut dyn Tx, target: &Target) -> Result, RpcError> { let (column, id) = match target { @@ -140,7 +170,7 @@ pub(crate) fn of(tx: &mut dyn Tx, target: &Target) -> Result, RpcEr }; tx.query( &format!( - "SELECT {COLUMNS} FROM config_removal_mark WHERE {column} = ?1 AND state <> 'removed'" + "SELECT {COLUMNS} FROM config_removal_mark WHERE {column} = ?1 AND state <> '{REMOVED}'" ), &[id.into()], )? @@ -156,9 +186,10 @@ pub(crate) fn naming( ) -> Result, RpcError> { tx.query( &format!( - "SELECT {COLUMNS} FROM config_removal_mark WHERE state = 'removing' AND \ + "SELECT {COLUMNS} FROM config_removal_mark WHERE state = '{}' AND \ (environment_id = ?1 OR project_id = \ - (SELECT project_id FROM config_environment WHERE id = ?1))" + (SELECT project_id FROM config_environment WHERE id = ?1))", + state_text(State::Removing) ), &[environment.as_str().into()], )? @@ -172,7 +203,8 @@ pub(crate) fn live(tx: &mut dyn Tx, who: &Actor) -> Result, RpcError> tx.query( &format!( "SELECT {COLUMNS} FROM config_removal_mark \ - WHERE organization_id = ?1 AND state = 'removing' ORDER BY at, id" + WHERE organization_id = ?1 AND state = '{}' ORDER BY at, id", + state_text(State::Removing) ), &[who.organization.as_str().into()], )? @@ -188,8 +220,10 @@ pub(crate) fn residuals( id: &RemovalId, ) -> Result, RpcError> { tx.query( - "SELECT residuals FROM config_removal_mark \ - WHERE organization_id = ?1 AND id = ?2 AND state = 'removed'", + &format!( + "SELECT residuals FROM config_removal_mark \ + WHERE organization_id = ?1 AND id = ?2 AND state = '{REMOVED}'" + ), &[who.organization.as_str().into(), id.as_str().into()], )? .first() @@ -247,8 +281,11 @@ pub(crate) fn finish( residuals: &[Residual], ) -> Result { let moved = tx.execute( - "UPDATE config_removal_mark SET state = 'removed', residuals = ?2 \ - WHERE id = ?1 AND state = 'removing'", + &format!( + "UPDATE config_removal_mark SET state = '{REMOVED}', residuals = ?2 \ + WHERE id = ?1 AND state = '{}'", + state_text(State::Removing) + ), &[id.as_str().into(), text(&residuals).as_str().into()], )?; Ok(if moved == 0 { @@ -265,12 +302,28 @@ pub(crate) fn clear_tombstone(tx: &mut dyn Tx, target: &Target) -> Result<(), Rp Target::Project(id) => ("project_id", id.as_str()), }; tx.execute( - &format!("DELETE FROM config_removal_mark WHERE {column} = ?1 AND state = 'cancelled'"), + &format!( + "DELETE FROM config_removal_mark WHERE {column} = ?1 AND state = '{}'", + state_text(State::Cancelled) + ), &[id.into()], )?; Ok(()) } +/// Delete `environment`'s live mark, leaving no tombstone: a close that came before a +/// merge is undone and the Environment stays open. A mark the sweep finished stays. +pub(crate) fn drop_live(tx: &mut dyn Tx, environment: &EnvironmentId) -> Result<(), RpcError> { + tx.execute( + &format!( + "DELETE FROM config_removal_mark WHERE environment_id = ?1 AND state = '{}'", + state_text(State::Removing) + ), + &[environment.as_str().into()], + )?; + Ok(()) +} + fn text(value: &impl Serialize) -> String { serde_json::to_string(value).expect("stored documents are JSON") } From cc5099bea92caa3ca7a11471a3a0a903f92b6c6e Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:53:26 +0000 Subject: [PATCH 2/4] refactor(store): type the saved Service slug as ServiceName The saved document owns its node vocabulary: SavedServiceIntent.slug is a ServiceName, decoded once with the document, so the store stops re-parsing the same string at each use. The serialized form is unchanged. --- .claude/skills/verify-removal-marks/SKILL.md | 5 ++-- .../ployz-core/src/config/environment.rs | 4 +-- .../src/config/environment_compile.rs | 2 +- .../ployz-core/tests/environment_intent.rs | 7 +++++ core/crates/ployz-core/tests/sync.rs | 2 +- core/crates/ployz-store/src/automation.rs | 5 ++-- core/crates/ployz-store/src/branch/create.rs | 10 +++---- core/crates/ployz-store/src/branch/live.rs | 4 +-- core/crates/ployz-store/src/branch/mod.rs | 18 +++++------- core/crates/ployz-store/src/branch/pair.rs | 12 ++------ core/crates/ployz-store/src/branch/setup.rs | 7 ++--- .../crates/ployz-store/src/config_item/mod.rs | 8 ++---- .../ployz-store/src/config_item/query.rs | 28 ++++++++----------- .../ployz-store/src/deployment/admit.rs | 6 ++-- .../ployz-store/src/deployment/lowering.rs | 26 +++++++---------- core/crates/ployz-store/src/deployment/mod.rs | 3 +- .../ployz-store/src/deployment/query.rs | 7 ++--- .../src/deployment/retire_tests.rs | 2 +- core/crates/ployz-store/src/domain/mod.rs | 4 +-- core/crates/ployz-store/src/domain/status.rs | 2 +- core/crates/ployz-store/src/git.rs | 2 +- core/crates/ployz-store/src/names.rs | 18 ++++++------ .../ployz-store/src/pull_request/checks.rs | 2 +- core/crates/ployz-store/src/removal/due.rs | 10 +++---- core/crates/ployz-store/src/reseal.rs | 12 ++++---- .../crates/ployz-store/src/review/approval.rs | 12 +++++--- .../crates/ployz-store/src/review/authored.rs | 15 +++++----- core/crates/ployz-store/src/review/history.rs | 2 +- core/crates/ployz-store/src/review/mod.rs | 28 +++++++++---------- core/crates/ployz-store/src/rules.rs | 18 ++++++------ core/crates/ployz-store/src/scope.rs | 10 +++---- .../crates/ployz-store/src/sealing_targets.rs | 11 +++----- core/crates/ployz-store/src/service/mod.rs | 8 +++--- core/crates/ployz-store/src/settings/query.rs | 5 ++-- core/crates/ployz-store/src/typed_address.rs | 15 ++++------ core/crates/ployz-store/src/variables.rs | 10 +++---- core/crates/ployz-store/src/volume/mod.rs | 5 ++-- core/crates/ployz-store/src/volume/query.rs | 21 ++++++-------- core/crates/ployz-store/tests/review.rs | 4 +-- 39 files changed, 167 insertions(+), 203 deletions(-) diff --git a/.claude/skills/verify-removal-marks/SKILL.md b/.claude/skills/verify-removal-marks/SKILL.md index f9b50eabe7..1dc9f9a492 100644 --- a/.claude/skills/verify-removal-marks/SKILL.md +++ b/.claude/skills/verify-removal-marks/SKILL.md @@ -74,14 +74,15 @@ Out of scope: a Project-target mark and its member Environments, two sweeps side Each model step is a copy of code at the sync commit. Reread these when a diff touches them: - `core/crates/ployz-store/src/removal/due.rs`: `sweep`, `settle`, `finish`, `close` and its `Cancelled` early return. -- `core/crates/ployz-store/src/removal/mark.rs`: `live`, `of`, `save` and its `ON CONFLICT` update, `finish` and its `WHERE state = 'removing'`. +- `core/crates/ployz-store/src/removal/mark.rs`: `live`, `of`, `save` and its `ON CONFLICT` update, `finish` and its `WHERE state = 'removing'`, and the SQL other modules splice in: `unmarked`, `marks_holding`, `drop_live`. - `core/crates/ployz-store/src/removal/mod.rs`: `cancel_removal`, `stop`, `lock_project`, `on_servers`, `ran`, `purge`. - `core/crates/ployz-store/src/scope.rs`: `environment`, `lock`, `lock_all`, `lock_id`, `owned`, `load_by_id`. - `core/crates/ployz-store/src/deployment/lifecycle.rs`: `cancel`, `record` and its `Applied`/`Failed` choice, `end` and its `(Cancelling, Failed)` to `Cancelled` mapping. - `core/crates/ployz-store/src/deployment/history.rs`: `in_flight`, `last_ran`. - `core/crates/ployz-store/src/storage/pg.rs`: `begin` and its READ COMMITTED level, `TIMEOUT`. - `core/crates/ployz-store/src/storage/sqlite.rs`: `transaction` and `BEGIN IMMEDIATE`. -- `core/crates/ployz-store/src/pull_request/mod.rs`: the calls to `removal::close` and `removal::sweep`. +- `core/crates/ployz-store/src/pull_request/mod.rs`: the calls to `removal::close` and `removal::sweep`, `mark::unmarked`, `mark::drop_live` at the merge undo, and `idle_facts`. +- `core/crates/ployz-store/src/branch/follow.rs` and `branch/proposal.rs`: `mark::unmarked`. - `dashboard/src/modules/config-store/store-github.inngest.ts` and `store-deployment.inngest.ts`: when Cloud sweeps. Sync commit: `379d927a88e00ad82aafa355f828f3b4a0bf5451`, the commit that made `finish` conditional, checked against it. Code in the list above that changed after it has not been rechecked. diff --git a/core/crates/ployz-core/src/config/environment.rs b/core/crates/ployz-core/src/config/environment.rs index ccb93b2449..de235eb3bf 100644 --- a/core/crates/ployz-core/src/config/environment.rs +++ b/core/crates/ployz-core/src/config/environment.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use ts_rs::TS; -use crate::{ConfigFileName, ConfigName, ContainerPath}; +use crate::{ConfigFileName, ConfigName, ContainerPath, ServiceName}; use super::{ AWAITING_FINGERPRINT, AuthoredServiceConfig, ConfigError, Generator, SecretCiphertext, @@ -35,7 +35,7 @@ pub struct SavedEnvironmentIntent { pub struct SavedServiceIntent { pub id: String, pub lineage_id: String, - pub slug: String, + pub slug: ServiceName, pub config: AuthoredServiceConfig, pub variables: Vec, pub volume_attachments: Vec, diff --git a/core/crates/ployz-core/src/config/environment_compile.rs b/core/crates/ployz-core/src/config/environment_compile.rs index 63ef1e2e74..6f63917b9c 100644 --- a/core/crates/ployz-core/src/config/environment_compile.rs +++ b/core/crates/ployz-core/src/config/environment_compile.rs @@ -233,7 +233,7 @@ pub fn compile_environment_intent( let slugs = intent .services .iter() - .map(|s| (s.lineage_id.clone(), s.slug.clone())) + .map(|s| (s.lineage_id.clone(), s.slug.to_string())) .collect(); let mut node_snapshots = Vec::new(); let mut variable_producers = Vec::new(); diff --git a/core/crates/ployz-core/tests/environment_intent.rs b/core/crates/ployz-core/tests/environment_intent.rs index cc93d25dbb..92f7775c53 100644 --- a/core/crates/ployz-core/tests/environment_intent.rs +++ b/core/crates/ployz-core/tests/environment_intent.rs @@ -236,3 +236,10 @@ fn configs_refuse_bare_references_and_colliding_mounts() { assert!(parse_environment_intent(value).is_err(), "{mode}"); } } + +#[test] +fn a_service_slug_that_is_not_a_service_name_is_refused_when_the_document_is_read() { + let mut document = intent(); + *document.pointer_mut("/services/0/slug").unwrap() = json!("Not A Name"); + assert!(parse_environment_intent(document).is_err()); +} diff --git a/core/crates/ployz-core/tests/sync.rs b/core/crates/ployz-core/tests/sync.rs index 84ba8effd9..510c517c20 100644 --- a/core/crates/ployz-core/tests/sync.rs +++ b/core/crates/ployz-core/tests/sync.rs @@ -1790,7 +1790,7 @@ fn name_of_names_a_mount_by_its_volume() { let parent = intent(&parent()); let at = |row: String| row.parse::().unwrap(); let (node, place) = name_of(&parent, &at(format!("{API}:mounts.{DATA}"))).unwrap(); - assert!(matches!(node, NodeRef::Service(service) if service.slug == "api")); + assert!(matches!(node, NodeRef::Service(service) if service.slug.as_str() == "api")); assert_eq!(place, "mounts.data"); let (node, place) = name_of(&parent, &at(format!("{DATA}:storage"))).unwrap(); assert!(matches!(node, NodeRef::Volume(volume) if volume.name == "data")); diff --git a/core/crates/ployz-store/src/automation.rs b/core/crates/ployz-store/src/automation.rs index d40b2cb641..af730f6d6b 100644 --- a/core/crates/ployz-store/src/automation.rs +++ b/core/crates/ployz-store/src/automation.rs @@ -757,9 +757,8 @@ fn admit( true => Vec::new(), false => selected .iter() - .map(|(service, _)| ServiceName::parse(service.slug.as_str())) - .collect::, _>>() - .map_err(|_| error::corrupt("Service name"))?, + .map(|(service, _)| service.slug.clone()) + .collect(), }; // A push of a live rule's branch deploys only the settings the rule approves: any // other Deployment would fail on every Server, so it waits for a person instead. diff --git a/core/crates/ployz-store/src/branch/create.rs b/core/crates/ployz-store/src/branch/create.rs index 718d84376c..522361c524 100644 --- a/core/crates/ployz-store/src/branch/create.rs +++ b/core/crates/ployz-store/src/branch/create.rs @@ -90,7 +90,7 @@ pub(crate) fn create_branch( .into_iter() .filter(|setup| { working.services.iter().any(|service| { - service.slug == setup.service.as_str() && own.contains(&service.lineage_id) + service.slug == setup.service && own.contains(&service.lineage_id) }) }) .collect(), @@ -104,7 +104,7 @@ pub(crate) fn create_branch( let service = working .services .iter() - .find(|service| service.slug == setup.service.as_str()) + .find(|service| service.slug == setup.service) .filter(|service| own.contains(&service.lineage_id)) .ok_or_else(|| { error::invalid( @@ -232,7 +232,7 @@ pub(crate) fn create_branch( Ok(Branched { branch: view(tx, &branch)?, staged, - not_set: not_set(&branch.working)?, + not_set: not_set(&branch.working), }) } @@ -353,7 +353,7 @@ pub(crate) fn copy_node( Ok(Branched { branch: view(tx, &branch)?, staged, - not_set: not_set(&branch.working)?, + not_set: not_set(&branch.working), }) } @@ -399,7 +399,7 @@ pub(crate) fn keep_branch( Ok(Branched { branch: view(tx, &branch)?, staged: Vec::new(), - not_set: not_set(&branch.working)?, + not_set: not_set(&branch.working), }) } diff --git a/core/crates/ployz-store/src/branch/live.rs b/core/crates/ployz-store/src/branch/live.rs index 4bf2b19d73..d5e59c36df 100644 --- a/core/crates/ployz-store/src/branch/live.rs +++ b/core/crates/ployz-store/src/branch/live.rs @@ -265,7 +265,7 @@ pub(crate) fn view(tx: &mut dyn Tx, branch: &Environment) -> Result Result Result, RpcError> { +pub(crate) fn not_set(working: &SavedEnvironmentIntent) -> Vec { let mut not_set = Vec::new(); for service in &working.services { for variable in &service.variables { if variable.value == VariableValue::NotSet { not_set.push(review::history::NotSetVariable { - service: ServiceName::parse(&service.slug) - .map_err(|_| error::corrupt("Service name"))?, + service: service.slug.clone(), key: variable.key.clone(), }); } } } - Ok(not_set) + not_set } /// A Branch's row. @@ -618,7 +615,7 @@ pub(crate) fn lineage_named( NodeName::Service(name) => intent .services .iter() - .find(|service| service.slug == name.as_str()) + .find(|service| service.slug == *name) .map(|service| service.lineage_id.clone()), NodeName::Volume(name) => intent .volumes @@ -645,7 +642,7 @@ fn names(intent: &SavedEnvironmentIntent) -> Vec { intent .services .iter() - .map(|service| service.slug.clone()) + .map(|service| service.slug.to_string()) .chain( intent .volumes @@ -667,7 +664,7 @@ pub(crate) fn name_of(intent: &SavedEnvironmentIntent, lineage: &str) -> Option< .services .iter() .find(|service| service.lineage_id == lineage) - .map(|service| service.slug.clone()) + .map(|service| service.slug.to_string()) .or_else(|| { intent .volumes @@ -690,8 +687,7 @@ pub(crate) fn node_of(intent: &SavedEnvironmentIntent, lineage: &str) -> Option< .services .iter() .find(|service| service.lineage_id == lineage) - .and_then(|service| ServiceName::parse(service.slug.as_str()).ok()) - .map(NodeName::Service); + .map(|service| NodeName::Service(service.slug.clone())); service.or_else(|| { intent .volumes diff --git a/core/crates/ployz-store/src/branch/pair.rs b/core/crates/ployz-store/src/branch/pair.rs index afc293605b..af46e3036d 100644 --- a/core/crates/ployz-store/src/branch/pair.rs +++ b/core/crates/ployz-store/src/branch/pair.rs @@ -440,7 +440,7 @@ pub(crate) fn named(intents: &[&SavedEnvironmentIntent], row: &RowId) -> Option< let (node, at) = ployz_core::config::name_of(intent, row)?; let (node, kind) = match node { NodeRef::Service(service) => ( - NodeName::Service(ServiceName::parse(service.slug.as_str()).ok()?), + NodeName::Service(service.slug.clone()), EnvironmentNodeType::Service, ), NodeRef::Volume(volume) => ( @@ -2293,10 +2293,7 @@ pub(crate) fn land( if old == Some(service) { continue; } - staged.push(NodeName::Service( - ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, - )); + staged.push(NodeName::Service(service.slug.clone())); if old.is_some() { continue; } @@ -2355,10 +2352,7 @@ pub(crate) fn land( // back by ID. for service in &before.services { if !branch.working.services.iter().any(|s| s.id == service.id) { - staged.push(NodeName::Service( - ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, - )); + staged.push(NodeName::Service(service.slug.clone())); } } for config in &before.configs { diff --git a/core/crates/ployz-store/src/branch/setup.rs b/core/crates/ployz-store/src/branch/setup.rs index 5da425e3f1..5d7975ee33 100644 --- a/core/crates/ployz-store/src/branch/setup.rs +++ b/core/crates/ployz-store/src/branch/setup.rs @@ -129,9 +129,6 @@ impl Own { pub(crate) fn pending(&self, working: &SavedEnvironmentIntent) -> Vec { let mut pending = Vec::new(); for service in &working.services { - let Ok(name) = ServiceName::parse(service.slug.as_str()) else { - continue; - }; if self.deployed.contains(&service.id) { continue; } @@ -139,7 +136,7 @@ impl Own { self.commands(service) .into_iter() .map(|command| SetupCommand { - service: name.clone(), + service: service.slug.clone(), command: command.to_owned(), }), ); @@ -208,7 +205,7 @@ pub(crate) fn refuse_service_named_setup(environment: &Environment) -> Result<() .working .services .iter() - .any(|service| service.slug == "setup") + .any(|service| service.slug.as_str() == "setup") { return Ok(()); } diff --git a/core/crates/ployz-store/src/config_item/mod.rs b/core/crates/ployz-store/src/config_item/mod.rs index 14ecb969b8..5ea05cb480 100644 --- a/core/crates/ployz-store/src/config_item/mod.rs +++ b/core/crates/ployz-store/src/config_item/mod.rs @@ -264,8 +264,7 @@ pub(crate) fn put_file( .any(|mount| mount.config_resource_id == id) }); for node in mounting { - let service = - ServiceName::parse(node.slug.as_str()).map_err(|_| error::corrupt("Service"))?; + let service = node.slug.clone(); if let Some(error) = collision(&service, node, configs) .or_else(|| oversized(&service, node, configs, &names)) { @@ -334,8 +333,7 @@ pub(crate) fn delete_config( .config_attachments .retain(|mount| mount.config_resource_id != config.id.as_str()); if service.config_attachments.len() != before { - let name = - ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service"))?; + let name = service.slug.clone(); staged.push(SettingPath::at( &name, Target::ConfigMount(config.name.clone().into()), @@ -684,7 +682,7 @@ pub(crate) fn names_in(intents: &[&SavedEnvironmentIntent]) -> BTreeMap Result, RpcError> { +) -> Vec { let mut seen = BTreeSet::new(); let mut changes = Vec::new(); for service in working.services.iter().chain(&head.services) { @@ -217,19 +217,18 @@ fn mount_changes( let (before, after) = (dir(head), dir(working)); if before != after { changes.push(ConfigMountChange { - service: ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, + service: service.slug.clone(), before, after, }); } } changes.sort_by(|a, b| a.service.cmp(&b.service)); - Ok(changes) + changes } -fn mounts(working: &SavedEnvironmentIntent, config: &str) -> Result, RpcError> { - let mut mounts = working +fn mounts(working: &SavedEnvironmentIntent, config: &str) -> Vec { + let mut mounts: Vec = working .services .iter() .flat_map(|service| { @@ -237,15 +236,12 @@ fn mounts(working: &SavedEnvironmentIntent, config: &str) -> Result, RpcError>>()?; + .collect(); mounts.sort_by(|a, b| a.service.cmp(&b.service)); - Ok(mounts) + mounts } diff --git a/core/crates/ployz-store/src/deployment/admit.rs b/core/crates/ployz-store/src/deployment/admit.rs index 26abed63a2..af9b84c2c3 100644 --- a/core/crates/ployz-store/src/deployment/admit.rs +++ b/core/crates/ployz-store/src/deployment/admit.rs @@ -598,10 +598,8 @@ fn reapply( } let services = running .iter() - .map(|service| { - ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service name")) - }) - .collect::, _>>()?; + .map(|service| service.slug.clone()) + .collect::>(); let namespace = deployment::namespace(tx, who, &environment.summary, true)?; let cluster_domain = trusted .domains diff --git a/core/crates/ployz-store/src/deployment/lowering.rs b/core/crates/ployz-store/src/deployment/lowering.rs index 075e691f41..2a66a7ae63 100644 --- a/core/crates/ployz-store/src/deployment/lowering.rs +++ b/core/crates/ployz-store/src/deployment/lowering.rs @@ -118,10 +118,7 @@ fn ran_as( .iter() .find(|service| service.id == id.as_str()) { - name = Some( - ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, - ); + name = Some(service.slug.clone()); runtimes.extend([service.config.private_dns.clone()]); } let rows = tx.query( @@ -196,8 +193,7 @@ fn renamed( if !runtimes.is_empty() { retire.push(Retire { id, - name: ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, + name: service.slug.clone(), runtimes, }); } @@ -342,8 +338,7 @@ pub(crate) fn freeze( let mut named = services.to_vec(); if every_saved { for service in saved.services.iter().filter(|service| changed(service)) { - let name = ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?; + let name = service.slug.clone(); if !named.contains(&name) { named.push(name); } @@ -412,14 +407,14 @@ pub(crate) fn freeze( saved .services .iter() - .find(|service| service.slug == name.as_str()) + .find(|service| service.slug == *name) .map(TargetNode::service) .transpose()? .ok_or_else(|| { let staged = dropped .services .iter() - .any(|service| service.slug == name.as_str()); + .any(|service| service.slug == *name); error::not_found( if staged { format!("Service {name} is staged for removal; a full Deploy removes it") @@ -554,7 +549,7 @@ pub(crate) fn freeze( matches!(service.config.source, ServiceSource::Empty { .. }) && nodes.iter().any(|node| node.id() == service.id) }) - .map(|service| service.slug.clone()) + .map(|service| service.slug.to_string()) .collect(); Ok(Frozen { nodes, @@ -679,7 +674,7 @@ pub(super) fn lower( let targeted: Vec<&str> = saved .services .iter() - .filter(|service| services.iter().any(|name| name.as_str() == service.slug)) + .filter(|service| services.contains(&service.slug)) .map(|service| service.id.as_str()) .collect(); // Only the Services this Deployment starts must fit a Container's environment. @@ -721,7 +716,7 @@ pub(super) fn lower( saved .services .iter() - .filter(|service| services.iter().any(|name| name.as_str() == service.slug)) + .filter(|service| services.contains(&service.slug)) .map(|service| service.config.private_dns.clone()) .chain(removals.iter().cloned()) .map(|name| ServiceAttempt { name }) @@ -743,7 +738,7 @@ pub(super) fn lower( .services .iter() .find(|service| service.id == id) - .and_then(|service| ServiceName::parse(service.slug.as_str()).ok()) + .map(|service| service.slug.clone()) .unwrap_or_else(|| config.settings.private_dns.clone()); let not_set = resolved.not_set.remove(id).unwrap_or_default(); let broken = resolved.broken.remove(id).unwrap_or_default(); @@ -887,8 +882,7 @@ pub(super) fn lower( .map(|file| file.content.len()) .sum(); let names: Vec<&ConfigName> = mounted.iter().map(|config| &config.name).collect(); - let name = - ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service"))?; + let name = service.slug.clone(); if let Some(error) = crate::config_item::mounts_too_much(&name, &names, bytes) { return Err(error); } diff --git a/core/crates/ployz-store/src/deployment/mod.rs b/core/crates/ployz-store/src/deployment/mod.rs index 0f6ea09e88..11c1c595e8 100644 --- a/core/crates/ployz-store/src/deployment/mod.rs +++ b/core/crates/ployz-store/src/deployment/mod.rs @@ -651,8 +651,7 @@ impl TargetNode { fn service(service: &SavedServiceIntent) -> Result { Ok(Self::Service { id: parse_stored(&service.id)?, - name: ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, + name: service.slug.clone(), runtime: service.config.private_dns.clone(), removals: ServiceRemovals::default(), }) diff --git a/core/crates/ployz-store/src/deployment/query.rs b/core/crates/ployz-store/src/deployment/query.rs index c5e26d29c1..b98ef00919 100644 --- a/core/crates/ployz-store/src/deployment/query.rs +++ b/core/crates/ployz-store/src/deployment/query.rs @@ -382,22 +382,21 @@ pub(crate) fn namespace( let services = applied .services .iter() - .filter_map(|applied| { + .map(|applied| { let now = environment .working .services .iter() .find(|service| service.id == applied.id) .unwrap_or(applied); - let name = ServiceName::parse(now.slug.as_str()).ok()?; - Some((name, applied.config.private_dns.clone())) + (now.slug.clone(), applied.config.private_dns.clone()) }) .collect(); let authored = environment .working .services .iter() - .filter_map(|service| ServiceName::parse(service.slug.as_str()).ok()) + .map(|service| service.slug.clone()) .collect::>(); let mut routes = Vec::new(); for service in &applied.services { diff --git a/core/crates/ployz-store/src/deployment/retire_tests.rs b/core/crates/ployz-store/src/deployment/retire_tests.rs index 2aa8cff3f8..b31163a9e8 100644 --- a/core/crates/ployz-store/src/deployment/retire_tests.rs +++ b/core/crates/ployz-store/src/deployment/retire_tests.rs @@ -321,7 +321,7 @@ fn applied(store: &ConfigStore, who: &Actor) -> Vec { .unwrap() .services .into_iter() - .map(|service| service.slug) + .map(|service| service.slug.to_string()) .collect() } diff --git a/core/crates/ployz-store/src/domain/mod.rs b/core/crates/ployz-store/src/domain/mod.rs index 131a3d8c3c..427f8851d6 100644 --- a/core/crates/ployz-store/src/domain/mod.rs +++ b/core/crates/ployz-store/src/domain/mod.rs @@ -1247,9 +1247,7 @@ fn generated(prefix: String, trusted: &Trusted) -> DomainName { fn domains_of(intent: &SavedEnvironmentIntent, trusted: &Trusted) -> Vec { let mut domains = Vec::new(); for service in &intent.services { - let Ok(name) = ServiceName::parse(service.slug.as_str()) else { - continue; - }; + let name = &service.slug; for route in &service.config.routes { if let Ok(hostname) = Hostname::parse(route.hostname.as_str()) { domains.push(Domain { diff --git a/core/crates/ployz-store/src/domain/status.rs b/core/crates/ployz-store/src/domain/status.rs index 5e9eafaa3c..1acc7f2d9c 100644 --- a/core/crates/ployz-store/src/domain/status.rs +++ b/core/crates/ployz-store/src/domain/status.rs @@ -33,7 +33,7 @@ pub(super) fn row( let id = working .services .iter() - .find(|service: &&SavedServiceIntent| service.slug == domain.service.as_str()) + .find(|service: &&SavedServiceIntent| service.slug == domain.service) .map(|service| service.id.clone()) .unwrap_or_default(); let deployed = if holds(&head.applied, &id, &domain) { diff --git a/core/crates/ployz-store/src/git.rs b/core/crates/ployz-store/src/git.rs index 8edf6b6e88..757494d6ac 100644 --- a/core/crates/ployz-store/src/git.rs +++ b/core/crates/ployz-store/src/git.rs @@ -429,7 +429,7 @@ pub(crate) fn check_sources( && found.has_branch(name) }); if !vouched { - return Err(no_branch(repository, &service.slug)); + return Err(no_branch(repository, service.slug.as_str())); } } Ok(()) diff --git a/core/crates/ployz-store/src/names.rs b/core/crates/ployz-store/src/names.rs index 191d2e609e..6813126ee1 100644 --- a/core/crates/ployz-store/src/names.rs +++ b/core/crates/ployz-store/src/names.rs @@ -46,7 +46,8 @@ fn refuse_taken_among( refuse_reserved(name)?; let taken = working.services.iter().any(|service| { other(&service.id) - && (service.slug == name || service.config.private_dns.as_str() == name) + && (service.slug.as_str() == name + || service.config.private_dns.as_str() == name) }); ("Service", name, taken) } @@ -158,7 +159,7 @@ pub(crate) fn refuse_new_reserved( .iter() .any(|held| held.id == service.id && held.slug == service.slug) { - refuse_reserved(&service.slug)?; + refuse_reserved(service.slug.as_str())?; } } Ok(()) @@ -176,8 +177,9 @@ pub(crate) fn refuse_new_taken( for service in &target.services { let held = now.services.iter().find(|held| held.id == service.id); for name in [service.slug.as_str(), service.config.private_dns.as_str()] { - if held.is_none_or(|held| held.slug != name && held.config.private_dns.as_str() != name) - { + if held.is_none_or(|held| { + held.slug.as_str() != name && held.config.private_dns.as_str() != name + }) { claims.push((&service.id, Name::Service(name))); } } @@ -236,11 +238,9 @@ pub(crate) fn refuse_new_taken( |error| match name { Name::Service(name) if error.code == ployz_core::RpcErrorCode::Conflict - && !target - .working - .services - .iter() - .any(|service| service.id != owner && service.slug == name) => + && !target.working.services.iter().any(|service| { + service.id != owner && service.slug.as_str() == name + }) => { error::conflict( format!("{name} is another Service's private DNS name. Change it first."), diff --git a/core/crates/ployz-store/src/pull_request/checks.rs b/core/crates/ployz-store/src/pull_request/checks.rs index ad07f98c1e..74a68d781d 100644 --- a/core/crates/ployz-store/src/pull_request/checks.rs +++ b/core/crates/ployz-store/src/pull_request/checks.rs @@ -27,7 +27,7 @@ pub(crate) fn view( rows: landing.rows, }) .collect(), - not_set: crate::branch::not_set(&environment.working)?, + not_set: crate::branch::not_set(&environment.working), }); } let unlanded = match record.map(|record| record.phase) { diff --git a/core/crates/ployz-store/src/removal/due.rs b/core/crates/ployz-store/src/removal/due.rs index 88cc58daf4..9939649de3 100644 --- a/core/crates/ployz-store/src/removal/due.rs +++ b/core/crates/ployz-store/src/removal/due.rs @@ -164,7 +164,7 @@ pub(super) fn go_on( } }; left.push(Left { - services: services(&environment)?, + services: services(&environment), environment: environment.summary, deployment, status, @@ -183,14 +183,12 @@ pub(super) fn go_on( }) } -fn services(environment: &Environment) -> Result, RpcError> { +fn services(environment: &Environment) -> Vec { environment .working .services .iter() - .map(|service| { - ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service name")) - }) + .map(|service| service.slug.clone()) .collect() } @@ -213,7 +211,7 @@ fn finish(tx: &mut dyn Tx, mark: &mut Mark) -> Result = rollback diff --git a/core/crates/ployz-store/src/review/authored.rs b/core/crates/ployz-store/src/review/authored.rs index e182767c03..ad809795f2 100644 --- a/core/crates/ployz-store/src/review/authored.rs +++ b/core/crates/ployz-store/src/review/authored.rs @@ -84,7 +84,7 @@ impl VersionedValue { } enum ServiceField { - Name(Delta), + Name(Delta), Source(Delta), GitSource(Delta), Branch(Delta), @@ -1280,14 +1280,14 @@ impl Effect { let (before, after) = match self { Self::ServiceLifecycle(change) => lifecycle(change, |service| { vec![ - Slot::Service(&service.slug), + Slot::Service(service.slug.as_str()), Slot::Service(service.config.private_dns.as_str()), ] }), Self::Service { field: ServiceField::Name(name), .. - } => delta(name, |name| Some(Slot::Service(name))), + } => delta(name, |name| Some(Slot::Service(name.as_str()))), Self::Service { field: ServiceField::PrivateDns(dns), .. @@ -1987,7 +1987,7 @@ impl Plan { } for change in &mut changes { change.data = super::data_effect(&change.node, change.lifecycle, before, after); - change.restarts = super::restarts(&change.node, change.lifecycle, after, before)?; + change.restarts = super::restarts(&change.node, change.lifecycle, after, before); } let total_count = changes .iter() @@ -2099,7 +2099,7 @@ fn node_info( .services .iter() .find(|owner| owner.id == node.id) - .map(|owner| (owner.slug.clone(), owner.lineage_id.clone())), + .map(|owner| (owner.slug.to_string(), owner.lineage_id.clone())), EnvironmentNodeType::Volume => intent .volumes .iter() @@ -2318,8 +2318,7 @@ fn keep_sealed( None if sealed(&variable.value) || ledger.holds(&service.id, variable) => { variable.value = VariableValue::NotSet; not_set.push(NotSetVariable { - service: ServiceName::parse(&service.slug) - .map_err(|_| error::corrupt("Service name"))?, + service: service.slug.clone(), key: variable.key.clone(), }); } @@ -2590,7 +2589,7 @@ fn resolve_node( [working, baseline] .into_iter() .flat_map(|intent| &intent.services) - .find(|owner| owner.slug == name.as_str()) + .find(|owner| owner.slug == name) .map(|owner| owner.id.clone()) .ok_or_else(|| scope::no_service(&name, &environment.summary.name, working))?, ), diff --git a/core/crates/ployz-store/src/review/history.rs b/core/crates/ployz-store/src/review/history.rs index b70d2a6d0c..748a2dab2c 100644 --- a/core/crates/ployz-store/src/review/history.rs +++ b/core/crates/ployz-store/src/review/history.rs @@ -450,7 +450,7 @@ fn deleted_since( .services .iter() .find(|service| service.id == id) - .and_then(|service| named(service.slug.clone())), + .and_then(|service| named(service.slug.to_string())), "variable" => { let variables = variables(intent, id)?; variables diff --git a/core/crates/ployz-store/src/review/mod.rs b/core/crates/ployz-store/src/review/mod.rs index 6a756e55af..1c17bbc2e1 100644 --- a/core/crates/ployz-store/src/review/mod.rs +++ b/core/crates/ployz-store/src/review/mod.rs @@ -443,7 +443,7 @@ fn compare( .into_iter() .flat_map(|intent| &intent.services) .find(|service| service.id == node.id) - .map(|service| service.slug.clone()); + .map(|service| service.slug.to_string()); services .or_else(|| volume_name(&intents, &node.id)) .or_else(|| config_name(&intents, &node.id)) @@ -543,7 +543,7 @@ fn compare( let data = data_effect(&group.node, group.lifecycle, deployed, &environment.working); let restarts = - restarts(&group.node, group.lifecycle, &environment.working, deployed)?; + restarts(&group.node, group.lifecycle, &environment.working, deployed); Ok(NodeChange { node: group.node, name, @@ -684,11 +684,14 @@ fn sources(view: &mut DiffView, working: &SavedEnvironmentIntent) { else { continue; }; - let rows: Vec<_> = - introduced_source(&service.slug, &service.lineage_id, &service.config.source) - .into_iter() - .filter(|row| !change.settings.iter().any(|shown| shown.row == row.row)) - .collect(); + let rows: Vec<_> = introduced_source( + service.slug.as_str(), + &service.lineage_id, + &service.config.source, + ) + .into_iter() + .filter(|row| !change.settings.iter().any(|shown| shown.row == row.row)) + .collect(); view.total_count += rows.len(); change.settings.splice(0..0, rows); } @@ -775,7 +778,7 @@ fn renames(view: &mut DiffView, working: &SavedEnvironmentIntent, head: &SavedEn node_type: EnvironmentNodeType::Service, id: service.id.clone(), }, - name: service.slug.clone(), + name: service.slug.to_string(), row: RowId::node(&service.lineage_id), lifecycle: ReviewLifecycleKind::Update, comparison: Some(ReviewComparisonRole::Head), @@ -833,9 +836,9 @@ fn restarts( lifecycle: ReviewLifecycleKind, working: &SavedEnvironmentIntent, deployed: &SavedEnvironmentIntent, -) -> Result, RpcError> { +) -> Vec { if node.node_type != EnvironmentNodeType::Config || lifecycle != ReviewLifecycleKind::Update { - return Ok(Vec::new()); + return Vec::new(); } working .services @@ -847,10 +850,7 @@ fn restarts( .any(|mount| mount.config_resource_id == node.id) && deployed.services.iter().any(|old| old.id == service.id) }) - .map(|service| { - ployz_core::ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name")) - }) + .map(|service| service.slug.clone()) .collect() } diff --git a/core/crates/ployz-store/src/rules.rs b/core/crates/ployz-store/src/rules.rs index d9d44b6591..7866414220 100644 --- a/core/crates/ployz-store/src/rules.rs +++ b/core/crates/ployz-store/src/rules.rs @@ -107,7 +107,7 @@ pub(crate) fn broken( Some(BrokenReference { at: match at { Referrer::Variable(service, variable) => ReferenceAt::Variable { - service: named(&service.slug)?, + service: service.slug.clone(), variable: variable.to_owned(), }, Referrer::File(config, file) => ReferenceAt::File { @@ -327,7 +327,7 @@ fn problems(facts: Facts<'_>) -> BTreeSet { if !working .services .iter() - .any(|service| service.slug == command.service.as_str()) + .any(|service| service.slug == command.service) { found.insert(Problem::NoSetupService(command.service.clone())); } @@ -511,7 +511,7 @@ impl<'a> Refusing<'a> { .iter() .find_map(|facts| { service(facts.working, lineage) - .map(|service| service.slug.clone()) + .map(|service| service.slug.to_string()) .or_else(|| facts.live.get(lineage).cloned()) .or_else(|| { facts @@ -576,7 +576,7 @@ impl<'a> Refusing<'a> { format!( "{name} is already mounted by {}; allow shared writes first: {allow}", if old.is_empty() { - first.slug.clone() + first.slug.to_string() } else { slugs(&old) } @@ -676,7 +676,7 @@ impl<'a> Refusing<'a> { .working .services .iter() - .any(|old| old.slug == service.as_str()); + .any(|old| old.slug == *service); if !had { return error::choices( format!( @@ -983,7 +983,7 @@ pub(crate) mod tests { logs.name = "logs".into(); working.volumes.push(logs); for service in &mut working.services { - let at = if service.slug == "db" { + let at = if service.slug.as_str() == "db" { "/data/" } else { "/sys/logs" @@ -1076,7 +1076,7 @@ pub(crate) mod tests { let redis = working .services .iter_mut() - .find(|service| service.slug == "redis") + .find(|service| service.slug.as_str() == "redis") .unwrap(); redis.variables[0].value = VariableValue::Plain { text: PlainText::Template(vec![ValuePart::Ref { @@ -1119,7 +1119,9 @@ pub(crate) mod tests { // Saved with db gone, production keeps editing and changing its other Setup Commands. seed(&store, &who, |working| { - working.services.retain(|service| service.slug != "db"); + working + .services + .retain(|service| service.slug.as_str() != "db"); }); unrelated(&store, &who).unwrap(); setup(&store, &who, &["db", "redis"]).unwrap(); diff --git a/core/crates/ployz-store/src/scope.rs b/core/crates/ployz-store/src/scope.rs index c163268b9c..7ec00ea926 100644 --- a/core/crates/ployz-store/src/scope.rs +++ b/core/crates/ployz-store/src/scope.rs @@ -225,7 +225,7 @@ impl Environment { self.working .services .iter() - .map(|service| (service.lineage_id.clone(), service.slug.clone())), + .map(|service| (service.lineage_id.clone(), service.slug.to_string())), ); names } @@ -269,7 +269,7 @@ impl Environment { let services = &self.working.services; services .iter() - .find(|service| service.slug == name.as_str()) + .find(|service| service.slug == *name) .ok_or_else(|| no_service(name, &self.summary.name, &self.working)) } @@ -282,7 +282,7 @@ impl Environment { .working .services .iter() - .position(|service| service.slug == name.as_str()) + .position(|service| service.slug == *name) .ok_or_else(|| no_service(name, &self.summary.name, &self.working))?; Ok(self .working @@ -816,7 +816,7 @@ pub(crate) fn persist_working( if !stays(&service.lineage_id) { removed.record_beside( &service.lineage_id, - Some(&service.slug), + Some(service.slug.as_str()), &environment.inherited, ); } @@ -935,7 +935,7 @@ pub(crate) fn validate_and_refresh_working_from( if read.contains(&service.lineage_id) { environment .removed - .record(&service.lineage_id, Some(&service.slug)); + .record(&service.lineage_id, Some(service.slug.as_str())); } } environment.inherited = inherited( diff --git a/core/crates/ployz-store/src/sealing_targets.rs b/core/crates/ployz-store/src/sealing_targets.rs index ee6fa2e0df..39b76082f5 100644 --- a/core/crates/ployz-store/src/sealing_targets.rs +++ b/core/crates/ployz-store/src/sealing_targets.rs @@ -133,14 +133,11 @@ pub(crate) fn targets( .working .services .iter() - .map(|service| { - Ok(SealingService { - name: ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, - service: Uuid::parse_str(&service.id).ok(), - }) + .map(|service| SealingService { + name: service.slug.clone(), + service: Uuid::parse_str(&service.id).ok(), }) - .collect::>()?; + .collect(); Ok(SealingTargetsView::Edit { organization, environment: environment.summary, diff --git a/core/crates/ployz-store/src/service/mod.rs b/core/crates/ployz-store/src/service/mod.rs index 8e184886a9..0e4acdda1a 100644 --- a/core/crates/ployz-store/src/service/mod.rs +++ b/core/crates/ployz-store/src/service/mod.rs @@ -158,7 +158,7 @@ pub(crate) fn insert_service( id: id.to_string(), // A new Service starts its own lineage; Branch copies keep it. lineage_id: id.to_string(), - slug: name.to_string(), + slug: name.clone(), config, variables: Vec::new(), volume_attachments: Vec::new(), @@ -196,7 +196,7 @@ pub(crate) fn rename_service( crate::names::Name::Service(rename.name.as_str()), Some(&id), )?; - environment.service_mut(&rename.service)?.slug = rename.name.to_string(); + environment.service_mut(&rename.service)?.slug = rename.name.clone(); scope::save_working(tx, &mut environment, trusted)?; } let service = summary(environment.service(&rename.name)?)?; @@ -217,7 +217,7 @@ pub(crate) fn remove_service( let mut environment = scope::lock(tx, who, &remove.environment)?; let node = environment.service(&remove.service)?; let service = summary(node)?; - let names = BTreeMap::from([(node.lineage_id.clone(), node.slug.clone())]); + let names = BTreeMap::from([(node.lineage_id.clone(), node.slug.to_string())]); environment .working .services @@ -249,7 +249,7 @@ fn staged(changed: bool, name: &ServiceName) -> Vec { pub(crate) fn summary(node: &SavedServiceIntent) -> Result { Ok(ServiceSummary { id: ServiceLineageId::parse(node.id.as_str()).map_err(|_| error::corrupt("Service ID"))?, - name: ServiceName::parse(node.slug.as_str()).map_err(|_| error::corrupt("Service name"))?, + name: node.slug.clone(), private_dns: node.config.private_dns.clone(), }) } diff --git a/core/crates/ployz-store/src/settings/query.rs b/core/crates/ployz-store/src/settings/query.rs index b924b487ee..c86841de94 100644 --- a/core/crates/ployz-store/src/settings/query.rs +++ b/core/crates/ployz-store/src/settings/query.rs @@ -5,7 +5,7 @@ //! `{"secret": true}`: no read shows a secret. use ployz_core::config::{RowId, SavedEnvironmentIntent, SavedServiceIntent}; -use ployz_core::{ConfigName, RpcError, ServiceName}; +use ployz_core::{ConfigName, RpcError}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value, json}; use ts_rs::TS; @@ -101,8 +101,7 @@ pub(crate) fn environment( }; let mut settings = Vec::new(); for service in services { - let name = ServiceName::parse(service.slug.as_str()) - .map_err(|_| crate::error::corrupt("Service name"))?; + let name = service.slug.clone(); let mut row = |target: Target, value: Value, default: Value, apply: Apply| { if only.is_none_or(|only| *only == target) && !(whole && value == default) { let (row, note) = match &target { diff --git a/core/crates/ployz-store/src/typed_address.rs b/core/crates/ployz-store/src/typed_address.rs index 5b3a1855e1..6101d85686 100644 --- a/core/crates/ployz-store/src/typed_address.rs +++ b/core/crates/ployz-store/src/typed_address.rs @@ -15,7 +15,6 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use ts_rs::TS; -use crate::error; use crate::variables::VariableKey; /// What to set instead of a value with Typed Addresses. @@ -63,7 +62,7 @@ pub(crate) fn in_plain( }) .collect(); let hosts = hosts(&text, key, others); - let Some(services) = services(&hosts)? else { + let Some(services) = services(&hosts) else { return Ok(None); }; let mut refs = parts @@ -131,18 +130,14 @@ fn hosts<'env>( } /// The Services `hosts` belong to, each once; none when there are none. -fn services( - hosts: &[(Range, &SavedServiceIntent)], -) -> Result>, RpcError> { +fn services(hosts: &[(Range, &SavedServiceIntent)]) -> Option> { let mut services = Vec::new(); for (_, other) in hosts { - let name = - ServiceName::parse(other.slug.as_str()).map_err(|_| error::corrupt("Service name"))?; - if !services.contains(&name) { - services.push(name); + if !services.contains(&other.slug) { + services.push(other.slug.clone()); } } - Ok((!services.is_empty()).then_some(services)) + (!services.is_empty()).then_some(services) } /// `text` as parts, each NUL in it the next of `refs` again. diff --git a/core/crates/ployz-store/src/variables.rs b/core/crates/ployz-store/src/variables.rs index ef83189992..c82ee7b247 100644 --- a/core/crates/ployz-store/src/variables.rs +++ b/core/crates/ployz-store/src/variables.rs @@ -162,7 +162,7 @@ pub(crate) fn set( .working .services .iter() - .filter(|other| other.slug != service.as_str()) + .filter(|other| other.slug != *service) .collect(); let current = environment .service(service)? @@ -411,7 +411,7 @@ pub(crate) fn rename( )); } for referrer in &mut environment.working.services { - let own = referrer.slug == service.as_str(); + let own = referrer.slug == *service; for variable in &mut referrer.variables { let VariableValue::Plain { text: PlainText::Template(parts), @@ -464,7 +464,7 @@ fn referrers( ) -> Result, RpcError> { let mut found = Vec::new(); for referrer in &environment.working.services { - let own = referrer.slug == service.as_str(); + let own = referrer.slug == *service; for variable in sorted(referrer) { let parts = variable.value.parts(); let uses = parts.iter().any(|part| { @@ -472,9 +472,7 @@ fn referrers( if wanted == key.as_str() && references(owner, own, lineage)) }); if uses { - let name = ServiceName::parse(&referrer.slug) - .map_err(|_| error::corrupt("Service name"))?; - found.push((name, VariableKey::parse(&variable.key)?)); + found.push((referrer.slug.clone(), VariableKey::parse(&variable.key)?)); } } } diff --git a/core/crates/ployz-store/src/volume/mod.rs b/core/crates/ployz-store/src/volume/mod.rs index 45ec08047b..50213f5dd2 100644 --- a/core/crates/ployz-store/src/volume/mod.rs +++ b/core/crates/ployz-store/src/volume/mod.rs @@ -355,8 +355,7 @@ pub(crate) fn remove_volume( .volume_attachments .retain(|mount| mount.volume_resource_id != volume.id.as_str()); if service.volume_attachments.len() != before { - let name = - ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service"))?; + let name = service.slug.clone(); staged.push(SettingPath::at(&name, Target::Mount(volume.name.clone()))); } } @@ -499,7 +498,7 @@ fn broken(intent: &SavedEnvironmentIntent) -> Vec { for (key, (node, volume, mount)) in mounts(intent) { let service = node.lineage_id.clone(); let at = key.2.clone(); - if let Some(refused) = kernel_refusal(&node.slug, &volume.name, mount) { + if let Some(refused) = kernel_refusal(node.slug.as_str(), &volume.name, mount) { let rule = Broken::Kernel { service: service.clone(), at: at.clone(), diff --git a/core/crates/ployz-store/src/volume/query.rs b/core/crates/ployz-store/src/volume/query.rs index 810f5837b1..2ed936ec3c 100644 --- a/core/crates/ployz-store/src/volume/query.rs +++ b/core/crates/ployz-store/src/volume/query.rs @@ -6,7 +6,7 @@ use ployz_core::config::{ ReviewLifecycleKind, SavedEnvironmentIntent, SavedVolumeIntent, VolumeKind, }; -use ployz_core::{Namespace, ProvisionedVolumeMaximumBytes, RpcError, ServiceName}; +use ployz_core::{Namespace, ProvisionedVolumeMaximumBytes, RpcError}; use serde::{Deserialize, Serialize}; use ts_rs::TS; @@ -229,7 +229,7 @@ fn listed( Some(VolumeKind::Provisioned { maximum_bytes }) => Some(*maximum_bytes), Some(VolumeKind::Docker {}) | None => None, }, - mounts: mounts(working, &node.resource_id)?, + mounts: mounts(working, &node.resource_id), deployed: review .head .applied @@ -250,8 +250,8 @@ fn listed( Ok(listed) } -fn mounts(working: &SavedEnvironmentIntent, volume: &str) -> Result, RpcError> { - let mut mounts = working +fn mounts(working: &SavedEnvironmentIntent, volume: &str) -> Vec { + let mut mounts: Vec = working .services .iter() .flat_map(|service| { @@ -259,15 +259,12 @@ fn mounts(working: &SavedEnvironmentIntent, volume: &str) -> Result, .volume_attachments .iter() .filter(|mount| mount.volume_resource_id == volume) - .map(move |mount| { - Ok(Mount { - service: ServiceName::parse(service.slug.as_str()) - .map_err(|_| error::corrupt("Service name"))?, - path: mount.mount_path.clone(), - }) + .map(move |mount| Mount { + service: service.slug.clone(), + path: mount.mount_path.clone(), }) }) - .collect::, RpcError>>()?; + .collect(); mounts.sort_by(|a, b| a.service.cmp(&b.service)); - Ok(mounts) + mounts } diff --git a/core/crates/ployz-store/tests/review.rs b/core/crates/ployz-store/tests/review.rs index 596f03917f..693be656cd 100644 --- a/core/crates/ployz-store/tests/review.rs +++ b/core/crates/ployz-store/tests/review.rs @@ -3276,7 +3276,7 @@ impl LegacyWorking { let variable = intent .services .iter_mut() - .find(|service| service.slug == "web") + .find(|service| service.slug.as_str() == "web") .unwrap() .variables .iter_mut() @@ -3291,7 +3291,7 @@ impl LegacyWorking { self.intent() .services .into_iter() - .find(|service| service.slug == "web") + .find(|service| service.slug.as_str() == "web") .unwrap() .variables .into_iter() From 41f122cca80022e5e771e24598d1cfa24b775f15 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:55:19 +0000 Subject: [PATCH 3/4] docs(store): drop the restating sentence from drop_live --- core/crates/ployz-store/src/removal/mark.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/crates/ployz-store/src/removal/mark.rs b/core/crates/ployz-store/src/removal/mark.rs index 40bf2aa6b1..ba5145ddb8 100644 --- a/core/crates/ployz-store/src/removal/mark.rs +++ b/core/crates/ployz-store/src/removal/mark.rs @@ -311,8 +311,8 @@ pub(crate) fn clear_tombstone(tx: &mut dyn Tx, target: &Target) -> Result<(), Rp Ok(()) } -/// Delete `environment`'s live mark, leaving no tombstone: a close that came before a -/// merge is undone and the Environment stays open. A mark the sweep finished stays. +/// Delete `environment`'s live mark, leaving no tombstone, so a close that came before a +/// merge is undone. pub(crate) fn drop_live(tx: &mut dyn Tx, environment: &EnvironmentId) -> Result<(), RpcError> { tx.execute( &format!( From b3ee88dca7a88064e2d2ec19eaa17ecad8d64d18 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 13:23:08 +0000 Subject: [PATCH 4/4] Take the Service slug type change back out SavedServiceIntent.slug is a String again. Typing it as ServiceName made a stored intent whose slug is not a valid Service Name fail to decode, where main decodes and admits it. Cloud rows cannot be inspected from here, so the stricter decode waits for a data check or a lenient-decode design. Pin the property with a test: a stored slug of "Web", "web_app", "web-" or 64 characters still decodes. --- .claude/skills/verify-removal-marks/SKILL.md | 2 +- .../ployz-core/src/config/environment.rs | 4 +-- .../src/config/environment_compile.rs | 2 +- .../ployz-core/tests/environment_intent.rs | 11 +++++--- core/crates/ployz-core/tests/sync.rs | 2 +- core/crates/ployz-store/src/automation.rs | 5 ++-- core/crates/ployz-store/src/branch/create.rs | 10 +++---- core/crates/ployz-store/src/branch/live.rs | 4 +-- core/crates/ployz-store/src/branch/mod.rs | 18 +++++++----- core/crates/ployz-store/src/branch/pair.rs | 12 ++++++-- core/crates/ployz-store/src/branch/setup.rs | 7 +++-- .../crates/ployz-store/src/config_item/mod.rs | 8 ++++-- .../ployz-store/src/config_item/query.rs | 28 +++++++++++-------- .../ployz-store/src/deployment/admit.rs | 6 ++-- .../ployz-store/src/deployment/lowering.rs | 26 ++++++++++------- core/crates/ployz-store/src/deployment/mod.rs | 3 +- .../ployz-store/src/deployment/query.rs | 7 +++-- .../src/deployment/retire_tests.rs | 2 +- core/crates/ployz-store/src/domain/mod.rs | 4 ++- core/crates/ployz-store/src/domain/status.rs | 2 +- core/crates/ployz-store/src/git.rs | 2 +- core/crates/ployz-store/src/names.rs | 18 ++++++------ .../ployz-store/src/pull_request/checks.rs | 2 +- core/crates/ployz-store/src/removal/due.rs | 10 ++++--- core/crates/ployz-store/src/reseal.rs | 12 ++++---- .../crates/ployz-store/src/review/approval.rs | 12 +++----- .../crates/ployz-store/src/review/authored.rs | 15 +++++----- core/crates/ployz-store/src/review/history.rs | 2 +- core/crates/ployz-store/src/review/mod.rs | 28 +++++++++---------- core/crates/ployz-store/src/rules.rs | 18 ++++++------ core/crates/ployz-store/src/scope.rs | 10 +++---- .../crates/ployz-store/src/sealing_targets.rs | 11 +++++--- core/crates/ployz-store/src/service/mod.rs | 8 +++--- core/crates/ployz-store/src/settings/query.rs | 5 ++-- core/crates/ployz-store/src/typed_address.rs | 15 ++++++---- core/crates/ployz-store/src/variables.rs | 10 ++++--- core/crates/ployz-store/src/volume/mod.rs | 5 ++-- core/crates/ployz-store/src/volume/query.rs | 21 ++++++++------ core/crates/ployz-store/tests/review.rs | 4 +-- 39 files changed, 209 insertions(+), 162 deletions(-) diff --git a/.claude/skills/verify-removal-marks/SKILL.md b/.claude/skills/verify-removal-marks/SKILL.md index 1dc9f9a492..2ab7fd99da 100644 --- a/.claude/skills/verify-removal-marks/SKILL.md +++ b/.claude/skills/verify-removal-marks/SKILL.md @@ -74,7 +74,7 @@ Out of scope: a Project-target mark and its member Environments, two sweeps side Each model step is a copy of code at the sync commit. Reread these when a diff touches them: - `core/crates/ployz-store/src/removal/due.rs`: `sweep`, `settle`, `finish`, `close` and its `Cancelled` early return. -- `core/crates/ployz-store/src/removal/mark.rs`: `live`, `of`, `save` and its `ON CONFLICT` update, `finish` and its `WHERE state = 'removing'`, and the SQL other modules splice in: `unmarked`, `marks_holding`, `drop_live`. +- `core/crates/ployz-store/src/removal/mark.rs`: `live`, `of`, `save` and its `ON CONFLICT` update, `finish` and its `WHERE state = 'removing'`, and the helpers other modules call: `unmarked` and `marks_holding`, which return SQL they splice in, and `drop_live`, which deletes the live mark. - `core/crates/ployz-store/src/removal/mod.rs`: `cancel_removal`, `stop`, `lock_project`, `on_servers`, `ran`, `purge`. - `core/crates/ployz-store/src/scope.rs`: `environment`, `lock`, `lock_all`, `lock_id`, `owned`, `load_by_id`. - `core/crates/ployz-store/src/deployment/lifecycle.rs`: `cancel`, `record` and its `Applied`/`Failed` choice, `end` and its `(Cancelling, Failed)` to `Cancelled` mapping. diff --git a/core/crates/ployz-core/src/config/environment.rs b/core/crates/ployz-core/src/config/environment.rs index de235eb3bf..ccb93b2449 100644 --- a/core/crates/ployz-core/src/config/environment.rs +++ b/core/crates/ployz-core/src/config/environment.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use ts_rs::TS; -use crate::{ConfigFileName, ConfigName, ContainerPath, ServiceName}; +use crate::{ConfigFileName, ConfigName, ContainerPath}; use super::{ AWAITING_FINGERPRINT, AuthoredServiceConfig, ConfigError, Generator, SecretCiphertext, @@ -35,7 +35,7 @@ pub struct SavedEnvironmentIntent { pub struct SavedServiceIntent { pub id: String, pub lineage_id: String, - pub slug: ServiceName, + pub slug: String, pub config: AuthoredServiceConfig, pub variables: Vec, pub volume_attachments: Vec, diff --git a/core/crates/ployz-core/src/config/environment_compile.rs b/core/crates/ployz-core/src/config/environment_compile.rs index 6f63917b9c..63ef1e2e74 100644 --- a/core/crates/ployz-core/src/config/environment_compile.rs +++ b/core/crates/ployz-core/src/config/environment_compile.rs @@ -233,7 +233,7 @@ pub fn compile_environment_intent( let slugs = intent .services .iter() - .map(|s| (s.lineage_id.clone(), s.slug.to_string())) + .map(|s| (s.lineage_id.clone(), s.slug.clone())) .collect(); let mut node_snapshots = Vec::new(); let mut variable_producers = Vec::new(); diff --git a/core/crates/ployz-core/tests/environment_intent.rs b/core/crates/ployz-core/tests/environment_intent.rs index 92f7775c53..445ab69b53 100644 --- a/core/crates/ployz-core/tests/environment_intent.rs +++ b/core/crates/ployz-core/tests/environment_intent.rs @@ -238,8 +238,11 @@ fn configs_refuse_bare_references_and_colliding_mounts() { } #[test] -fn a_service_slug_that_is_not_a_service_name_is_refused_when_the_document_is_read() { - let mut document = intent(); - *document.pointer_mut("/services/0/slug").unwrap() = json!("Not A Name"); - assert!(parse_environment_intent(document).is_err()); +fn a_stored_service_slug_that_is_not_a_service_name_still_decodes() { + for slug in ["Web", "web_app", "web-", &"a".repeat(64)] { + let mut value = intent(); + *value.pointer_mut("/services/0/slug").unwrap() = json!(slug); + let parsed = parse_environment_intent(value).unwrap(); + assert_eq!(parsed.services.first().unwrap().slug, slug); + } } diff --git a/core/crates/ployz-core/tests/sync.rs b/core/crates/ployz-core/tests/sync.rs index 510c517c20..84ba8effd9 100644 --- a/core/crates/ployz-core/tests/sync.rs +++ b/core/crates/ployz-core/tests/sync.rs @@ -1790,7 +1790,7 @@ fn name_of_names_a_mount_by_its_volume() { let parent = intent(&parent()); let at = |row: String| row.parse::().unwrap(); let (node, place) = name_of(&parent, &at(format!("{API}:mounts.{DATA}"))).unwrap(); - assert!(matches!(node, NodeRef::Service(service) if service.slug.as_str() == "api")); + assert!(matches!(node, NodeRef::Service(service) if service.slug == "api")); assert_eq!(place, "mounts.data"); let (node, place) = name_of(&parent, &at(format!("{DATA}:storage"))).unwrap(); assert!(matches!(node, NodeRef::Volume(volume) if volume.name == "data")); diff --git a/core/crates/ployz-store/src/automation.rs b/core/crates/ployz-store/src/automation.rs index af730f6d6b..d40b2cb641 100644 --- a/core/crates/ployz-store/src/automation.rs +++ b/core/crates/ployz-store/src/automation.rs @@ -757,8 +757,9 @@ fn admit( true => Vec::new(), false => selected .iter() - .map(|(service, _)| service.slug.clone()) - .collect(), + .map(|(service, _)| ServiceName::parse(service.slug.as_str())) + .collect::, _>>() + .map_err(|_| error::corrupt("Service name"))?, }; // A push of a live rule's branch deploys only the settings the rule approves: any // other Deployment would fail on every Server, so it waits for a person instead. diff --git a/core/crates/ployz-store/src/branch/create.rs b/core/crates/ployz-store/src/branch/create.rs index 522361c524..718d84376c 100644 --- a/core/crates/ployz-store/src/branch/create.rs +++ b/core/crates/ployz-store/src/branch/create.rs @@ -90,7 +90,7 @@ pub(crate) fn create_branch( .into_iter() .filter(|setup| { working.services.iter().any(|service| { - service.slug == setup.service && own.contains(&service.lineage_id) + service.slug == setup.service.as_str() && own.contains(&service.lineage_id) }) }) .collect(), @@ -104,7 +104,7 @@ pub(crate) fn create_branch( let service = working .services .iter() - .find(|service| service.slug == setup.service) + .find(|service| service.slug == setup.service.as_str()) .filter(|service| own.contains(&service.lineage_id)) .ok_or_else(|| { error::invalid( @@ -232,7 +232,7 @@ pub(crate) fn create_branch( Ok(Branched { branch: view(tx, &branch)?, staged, - not_set: not_set(&branch.working), + not_set: not_set(&branch.working)?, }) } @@ -353,7 +353,7 @@ pub(crate) fn copy_node( Ok(Branched { branch: view(tx, &branch)?, staged, - not_set: not_set(&branch.working), + not_set: not_set(&branch.working)?, }) } @@ -399,7 +399,7 @@ pub(crate) fn keep_branch( Ok(Branched { branch: view(tx, &branch)?, staged: Vec::new(), - not_set: not_set(&branch.working), + not_set: not_set(&branch.working)?, }) } diff --git a/core/crates/ployz-store/src/branch/live.rs b/core/crates/ployz-store/src/branch/live.rs index d5e59c36df..4bf2b19d73 100644 --- a/core/crates/ployz-store/src/branch/live.rs +++ b/core/crates/ployz-store/src/branch/live.rs @@ -265,7 +265,7 @@ pub(crate) fn view(tx: &mut dyn Tx, branch: &Environment) -> Result Result Vec { +pub(crate) fn not_set( + working: &SavedEnvironmentIntent, +) -> Result, RpcError> { let mut not_set = Vec::new(); for service in &working.services { for variable in &service.variables { if variable.value == VariableValue::NotSet { not_set.push(review::history::NotSetVariable { - service: service.slug.clone(), + service: ServiceName::parse(&service.slug) + .map_err(|_| error::corrupt("Service name"))?, key: variable.key.clone(), }); } } } - not_set + Ok(not_set) } /// A Branch's row. @@ -615,7 +618,7 @@ pub(crate) fn lineage_named( NodeName::Service(name) => intent .services .iter() - .find(|service| service.slug == *name) + .find(|service| service.slug == name.as_str()) .map(|service| service.lineage_id.clone()), NodeName::Volume(name) => intent .volumes @@ -642,7 +645,7 @@ fn names(intent: &SavedEnvironmentIntent) -> Vec { intent .services .iter() - .map(|service| service.slug.to_string()) + .map(|service| service.slug.clone()) .chain( intent .volumes @@ -664,7 +667,7 @@ pub(crate) fn name_of(intent: &SavedEnvironmentIntent, lineage: &str) -> Option< .services .iter() .find(|service| service.lineage_id == lineage) - .map(|service| service.slug.to_string()) + .map(|service| service.slug.clone()) .or_else(|| { intent .volumes @@ -687,7 +690,8 @@ pub(crate) fn node_of(intent: &SavedEnvironmentIntent, lineage: &str) -> Option< .services .iter() .find(|service| service.lineage_id == lineage) - .map(|service| NodeName::Service(service.slug.clone())); + .and_then(|service| ServiceName::parse(service.slug.as_str()).ok()) + .map(NodeName::Service); service.or_else(|| { intent .volumes diff --git a/core/crates/ployz-store/src/branch/pair.rs b/core/crates/ployz-store/src/branch/pair.rs index af46e3036d..afc293605b 100644 --- a/core/crates/ployz-store/src/branch/pair.rs +++ b/core/crates/ployz-store/src/branch/pair.rs @@ -440,7 +440,7 @@ pub(crate) fn named(intents: &[&SavedEnvironmentIntent], row: &RowId) -> Option< let (node, at) = ployz_core::config::name_of(intent, row)?; let (node, kind) = match node { NodeRef::Service(service) => ( - NodeName::Service(service.slug.clone()), + NodeName::Service(ServiceName::parse(service.slug.as_str()).ok()?), EnvironmentNodeType::Service, ), NodeRef::Volume(volume) => ( @@ -2293,7 +2293,10 @@ pub(crate) fn land( if old == Some(service) { continue; } - staged.push(NodeName::Service(service.slug.clone())); + staged.push(NodeName::Service( + ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, + )); if old.is_some() { continue; } @@ -2352,7 +2355,10 @@ pub(crate) fn land( // back by ID. for service in &before.services { if !branch.working.services.iter().any(|s| s.id == service.id) { - staged.push(NodeName::Service(service.slug.clone())); + staged.push(NodeName::Service( + ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, + )); } } for config in &before.configs { diff --git a/core/crates/ployz-store/src/branch/setup.rs b/core/crates/ployz-store/src/branch/setup.rs index 5d7975ee33..5da425e3f1 100644 --- a/core/crates/ployz-store/src/branch/setup.rs +++ b/core/crates/ployz-store/src/branch/setup.rs @@ -129,6 +129,9 @@ impl Own { pub(crate) fn pending(&self, working: &SavedEnvironmentIntent) -> Vec { let mut pending = Vec::new(); for service in &working.services { + let Ok(name) = ServiceName::parse(service.slug.as_str()) else { + continue; + }; if self.deployed.contains(&service.id) { continue; } @@ -136,7 +139,7 @@ impl Own { self.commands(service) .into_iter() .map(|command| SetupCommand { - service: service.slug.clone(), + service: name.clone(), command: command.to_owned(), }), ); @@ -205,7 +208,7 @@ pub(crate) fn refuse_service_named_setup(environment: &Environment) -> Result<() .working .services .iter() - .any(|service| service.slug.as_str() == "setup") + .any(|service| service.slug == "setup") { return Ok(()); } diff --git a/core/crates/ployz-store/src/config_item/mod.rs b/core/crates/ployz-store/src/config_item/mod.rs index 5ea05cb480..14ecb969b8 100644 --- a/core/crates/ployz-store/src/config_item/mod.rs +++ b/core/crates/ployz-store/src/config_item/mod.rs @@ -264,7 +264,8 @@ pub(crate) fn put_file( .any(|mount| mount.config_resource_id == id) }); for node in mounting { - let service = node.slug.clone(); + let service = + ServiceName::parse(node.slug.as_str()).map_err(|_| error::corrupt("Service"))?; if let Some(error) = collision(&service, node, configs) .or_else(|| oversized(&service, node, configs, &names)) { @@ -333,7 +334,8 @@ pub(crate) fn delete_config( .config_attachments .retain(|mount| mount.config_resource_id != config.id.as_str()); if service.config_attachments.len() != before { - let name = service.slug.clone(); + let name = + ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service"))?; staged.push(SettingPath::at( &name, Target::ConfigMount(config.name.clone().into()), @@ -682,7 +684,7 @@ pub(crate) fn names_in(intents: &[&SavedEnvironmentIntent]) -> BTreeMap Vec { +) -> Result, RpcError> { let mut seen = BTreeSet::new(); let mut changes = Vec::new(); for service in working.services.iter().chain(&head.services) { @@ -217,18 +217,19 @@ fn mount_changes( let (before, after) = (dir(head), dir(working)); if before != after { changes.push(ConfigMountChange { - service: service.slug.clone(), + service: ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, before, after, }); } } changes.sort_by(|a, b| a.service.cmp(&b.service)); - changes + Ok(changes) } -fn mounts(working: &SavedEnvironmentIntent, config: &str) -> Vec { - let mut mounts: Vec = working +fn mounts(working: &SavedEnvironmentIntent, config: &str) -> Result, RpcError> { + let mut mounts = working .services .iter() .flat_map(|service| { @@ -236,12 +237,15 @@ fn mounts(working: &SavedEnvironmentIntent, config: &str) -> Vec .config_attachments .iter() .filter(|mount| mount.config_resource_id == config) - .map(move |mount| ConfigMountAt { - service: service.slug.clone(), - dir: mount.mount_dir.to_string(), + .map(move |mount| { + Ok(ConfigMountAt { + service: ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, + dir: mount.mount_dir.to_string(), + }) }) }) - .collect(); + .collect::, RpcError>>()?; mounts.sort_by(|a, b| a.service.cmp(&b.service)); - mounts + Ok(mounts) } diff --git a/core/crates/ployz-store/src/deployment/admit.rs b/core/crates/ployz-store/src/deployment/admit.rs index af9b84c2c3..26abed63a2 100644 --- a/core/crates/ployz-store/src/deployment/admit.rs +++ b/core/crates/ployz-store/src/deployment/admit.rs @@ -598,8 +598,10 @@ fn reapply( } let services = running .iter() - .map(|service| service.slug.clone()) - .collect::>(); + .map(|service| { + ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service name")) + }) + .collect::, _>>()?; let namespace = deployment::namespace(tx, who, &environment.summary, true)?; let cluster_domain = trusted .domains diff --git a/core/crates/ployz-store/src/deployment/lowering.rs b/core/crates/ployz-store/src/deployment/lowering.rs index 2a66a7ae63..075e691f41 100644 --- a/core/crates/ployz-store/src/deployment/lowering.rs +++ b/core/crates/ployz-store/src/deployment/lowering.rs @@ -118,7 +118,10 @@ fn ran_as( .iter() .find(|service| service.id == id.as_str()) { - name = Some(service.slug.clone()); + name = Some( + ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, + ); runtimes.extend([service.config.private_dns.clone()]); } let rows = tx.query( @@ -193,7 +196,8 @@ fn renamed( if !runtimes.is_empty() { retire.push(Retire { id, - name: service.slug.clone(), + name: ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, runtimes, }); } @@ -338,7 +342,8 @@ pub(crate) fn freeze( let mut named = services.to_vec(); if every_saved { for service in saved.services.iter().filter(|service| changed(service)) { - let name = service.slug.clone(); + let name = ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?; if !named.contains(&name) { named.push(name); } @@ -407,14 +412,14 @@ pub(crate) fn freeze( saved .services .iter() - .find(|service| service.slug == *name) + .find(|service| service.slug == name.as_str()) .map(TargetNode::service) .transpose()? .ok_or_else(|| { let staged = dropped .services .iter() - .any(|service| service.slug == *name); + .any(|service| service.slug == name.as_str()); error::not_found( if staged { format!("Service {name} is staged for removal; a full Deploy removes it") @@ -549,7 +554,7 @@ pub(crate) fn freeze( matches!(service.config.source, ServiceSource::Empty { .. }) && nodes.iter().any(|node| node.id() == service.id) }) - .map(|service| service.slug.to_string()) + .map(|service| service.slug.clone()) .collect(); Ok(Frozen { nodes, @@ -674,7 +679,7 @@ pub(super) fn lower( let targeted: Vec<&str> = saved .services .iter() - .filter(|service| services.contains(&service.slug)) + .filter(|service| services.iter().any(|name| name.as_str() == service.slug)) .map(|service| service.id.as_str()) .collect(); // Only the Services this Deployment starts must fit a Container's environment. @@ -716,7 +721,7 @@ pub(super) fn lower( saved .services .iter() - .filter(|service| services.contains(&service.slug)) + .filter(|service| services.iter().any(|name| name.as_str() == service.slug)) .map(|service| service.config.private_dns.clone()) .chain(removals.iter().cloned()) .map(|name| ServiceAttempt { name }) @@ -738,7 +743,7 @@ pub(super) fn lower( .services .iter() .find(|service| service.id == id) - .map(|service| service.slug.clone()) + .and_then(|service| ServiceName::parse(service.slug.as_str()).ok()) .unwrap_or_else(|| config.settings.private_dns.clone()); let not_set = resolved.not_set.remove(id).unwrap_or_default(); let broken = resolved.broken.remove(id).unwrap_or_default(); @@ -882,7 +887,8 @@ pub(super) fn lower( .map(|file| file.content.len()) .sum(); let names: Vec<&ConfigName> = mounted.iter().map(|config| &config.name).collect(); - let name = service.slug.clone(); + let name = + ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service"))?; if let Some(error) = crate::config_item::mounts_too_much(&name, &names, bytes) { return Err(error); } diff --git a/core/crates/ployz-store/src/deployment/mod.rs b/core/crates/ployz-store/src/deployment/mod.rs index 11c1c595e8..0f6ea09e88 100644 --- a/core/crates/ployz-store/src/deployment/mod.rs +++ b/core/crates/ployz-store/src/deployment/mod.rs @@ -651,7 +651,8 @@ impl TargetNode { fn service(service: &SavedServiceIntent) -> Result { Ok(Self::Service { id: parse_stored(&service.id)?, - name: service.slug.clone(), + name: ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, runtime: service.config.private_dns.clone(), removals: ServiceRemovals::default(), }) diff --git a/core/crates/ployz-store/src/deployment/query.rs b/core/crates/ployz-store/src/deployment/query.rs index b98ef00919..c5e26d29c1 100644 --- a/core/crates/ployz-store/src/deployment/query.rs +++ b/core/crates/ployz-store/src/deployment/query.rs @@ -382,21 +382,22 @@ pub(crate) fn namespace( let services = applied .services .iter() - .map(|applied| { + .filter_map(|applied| { let now = environment .working .services .iter() .find(|service| service.id == applied.id) .unwrap_or(applied); - (now.slug.clone(), applied.config.private_dns.clone()) + let name = ServiceName::parse(now.slug.as_str()).ok()?; + Some((name, applied.config.private_dns.clone())) }) .collect(); let authored = environment .working .services .iter() - .map(|service| service.slug.clone()) + .filter_map(|service| ServiceName::parse(service.slug.as_str()).ok()) .collect::>(); let mut routes = Vec::new(); for service in &applied.services { diff --git a/core/crates/ployz-store/src/deployment/retire_tests.rs b/core/crates/ployz-store/src/deployment/retire_tests.rs index b31163a9e8..2aa8cff3f8 100644 --- a/core/crates/ployz-store/src/deployment/retire_tests.rs +++ b/core/crates/ployz-store/src/deployment/retire_tests.rs @@ -321,7 +321,7 @@ fn applied(store: &ConfigStore, who: &Actor) -> Vec { .unwrap() .services .into_iter() - .map(|service| service.slug.to_string()) + .map(|service| service.slug) .collect() } diff --git a/core/crates/ployz-store/src/domain/mod.rs b/core/crates/ployz-store/src/domain/mod.rs index 427f8851d6..131a3d8c3c 100644 --- a/core/crates/ployz-store/src/domain/mod.rs +++ b/core/crates/ployz-store/src/domain/mod.rs @@ -1247,7 +1247,9 @@ fn generated(prefix: String, trusted: &Trusted) -> DomainName { fn domains_of(intent: &SavedEnvironmentIntent, trusted: &Trusted) -> Vec { let mut domains = Vec::new(); for service in &intent.services { - let name = &service.slug; + let Ok(name) = ServiceName::parse(service.slug.as_str()) else { + continue; + }; for route in &service.config.routes { if let Ok(hostname) = Hostname::parse(route.hostname.as_str()) { domains.push(Domain { diff --git a/core/crates/ployz-store/src/domain/status.rs b/core/crates/ployz-store/src/domain/status.rs index 1acc7f2d9c..5e9eafaa3c 100644 --- a/core/crates/ployz-store/src/domain/status.rs +++ b/core/crates/ployz-store/src/domain/status.rs @@ -33,7 +33,7 @@ pub(super) fn row( let id = working .services .iter() - .find(|service: &&SavedServiceIntent| service.slug == domain.service) + .find(|service: &&SavedServiceIntent| service.slug == domain.service.as_str()) .map(|service| service.id.clone()) .unwrap_or_default(); let deployed = if holds(&head.applied, &id, &domain) { diff --git a/core/crates/ployz-store/src/git.rs b/core/crates/ployz-store/src/git.rs index 757494d6ac..8edf6b6e88 100644 --- a/core/crates/ployz-store/src/git.rs +++ b/core/crates/ployz-store/src/git.rs @@ -429,7 +429,7 @@ pub(crate) fn check_sources( && found.has_branch(name) }); if !vouched { - return Err(no_branch(repository, service.slug.as_str())); + return Err(no_branch(repository, &service.slug)); } } Ok(()) diff --git a/core/crates/ployz-store/src/names.rs b/core/crates/ployz-store/src/names.rs index 6813126ee1..191d2e609e 100644 --- a/core/crates/ployz-store/src/names.rs +++ b/core/crates/ployz-store/src/names.rs @@ -46,8 +46,7 @@ fn refuse_taken_among( refuse_reserved(name)?; let taken = working.services.iter().any(|service| { other(&service.id) - && (service.slug.as_str() == name - || service.config.private_dns.as_str() == name) + && (service.slug == name || service.config.private_dns.as_str() == name) }); ("Service", name, taken) } @@ -159,7 +158,7 @@ pub(crate) fn refuse_new_reserved( .iter() .any(|held| held.id == service.id && held.slug == service.slug) { - refuse_reserved(service.slug.as_str())?; + refuse_reserved(&service.slug)?; } } Ok(()) @@ -177,9 +176,8 @@ pub(crate) fn refuse_new_taken( for service in &target.services { let held = now.services.iter().find(|held| held.id == service.id); for name in [service.slug.as_str(), service.config.private_dns.as_str()] { - if held.is_none_or(|held| { - held.slug.as_str() != name && held.config.private_dns.as_str() != name - }) { + if held.is_none_or(|held| held.slug != name && held.config.private_dns.as_str() != name) + { claims.push((&service.id, Name::Service(name))); } } @@ -238,9 +236,11 @@ pub(crate) fn refuse_new_taken( |error| match name { Name::Service(name) if error.code == ployz_core::RpcErrorCode::Conflict - && !target.working.services.iter().any(|service| { - service.id != owner && service.slug.as_str() == name - }) => + && !target + .working + .services + .iter() + .any(|service| service.id != owner && service.slug == name) => { error::conflict( format!("{name} is another Service's private DNS name. Change it first."), diff --git a/core/crates/ployz-store/src/pull_request/checks.rs b/core/crates/ployz-store/src/pull_request/checks.rs index 74a68d781d..ad07f98c1e 100644 --- a/core/crates/ployz-store/src/pull_request/checks.rs +++ b/core/crates/ployz-store/src/pull_request/checks.rs @@ -27,7 +27,7 @@ pub(crate) fn view( rows: landing.rows, }) .collect(), - not_set: crate::branch::not_set(&environment.working), + not_set: crate::branch::not_set(&environment.working)?, }); } let unlanded = match record.map(|record| record.phase) { diff --git a/core/crates/ployz-store/src/removal/due.rs b/core/crates/ployz-store/src/removal/due.rs index 9939649de3..88cc58daf4 100644 --- a/core/crates/ployz-store/src/removal/due.rs +++ b/core/crates/ployz-store/src/removal/due.rs @@ -164,7 +164,7 @@ pub(super) fn go_on( } }; left.push(Left { - services: services(&environment), + services: services(&environment)?, environment: environment.summary, deployment, status, @@ -183,12 +183,14 @@ pub(super) fn go_on( }) } -fn services(environment: &Environment) -> Vec { +fn services(environment: &Environment) -> Result, RpcError> { environment .working .services .iter() - .map(|service| service.slug.clone()) + .map(|service| { + ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service name")) + }) .collect() } @@ -211,7 +213,7 @@ fn finish(tx: &mut dyn Tx, mark: &mut Mark) -> Result = rollback diff --git a/core/crates/ployz-store/src/review/authored.rs b/core/crates/ployz-store/src/review/authored.rs index ad809795f2..e182767c03 100644 --- a/core/crates/ployz-store/src/review/authored.rs +++ b/core/crates/ployz-store/src/review/authored.rs @@ -84,7 +84,7 @@ impl VersionedValue { } enum ServiceField { - Name(Delta), + Name(Delta), Source(Delta), GitSource(Delta), Branch(Delta), @@ -1280,14 +1280,14 @@ impl Effect { let (before, after) = match self { Self::ServiceLifecycle(change) => lifecycle(change, |service| { vec![ - Slot::Service(service.slug.as_str()), + Slot::Service(&service.slug), Slot::Service(service.config.private_dns.as_str()), ] }), Self::Service { field: ServiceField::Name(name), .. - } => delta(name, |name| Some(Slot::Service(name.as_str()))), + } => delta(name, |name| Some(Slot::Service(name))), Self::Service { field: ServiceField::PrivateDns(dns), .. @@ -1987,7 +1987,7 @@ impl Plan { } for change in &mut changes { change.data = super::data_effect(&change.node, change.lifecycle, before, after); - change.restarts = super::restarts(&change.node, change.lifecycle, after, before); + change.restarts = super::restarts(&change.node, change.lifecycle, after, before)?; } let total_count = changes .iter() @@ -2099,7 +2099,7 @@ fn node_info( .services .iter() .find(|owner| owner.id == node.id) - .map(|owner| (owner.slug.to_string(), owner.lineage_id.clone())), + .map(|owner| (owner.slug.clone(), owner.lineage_id.clone())), EnvironmentNodeType::Volume => intent .volumes .iter() @@ -2318,7 +2318,8 @@ fn keep_sealed( None if sealed(&variable.value) || ledger.holds(&service.id, variable) => { variable.value = VariableValue::NotSet; not_set.push(NotSetVariable { - service: service.slug.clone(), + service: ServiceName::parse(&service.slug) + .map_err(|_| error::corrupt("Service name"))?, key: variable.key.clone(), }); } @@ -2589,7 +2590,7 @@ fn resolve_node( [working, baseline] .into_iter() .flat_map(|intent| &intent.services) - .find(|owner| owner.slug == name) + .find(|owner| owner.slug == name.as_str()) .map(|owner| owner.id.clone()) .ok_or_else(|| scope::no_service(&name, &environment.summary.name, working))?, ), diff --git a/core/crates/ployz-store/src/review/history.rs b/core/crates/ployz-store/src/review/history.rs index 748a2dab2c..b70d2a6d0c 100644 --- a/core/crates/ployz-store/src/review/history.rs +++ b/core/crates/ployz-store/src/review/history.rs @@ -450,7 +450,7 @@ fn deleted_since( .services .iter() .find(|service| service.id == id) - .and_then(|service| named(service.slug.to_string())), + .and_then(|service| named(service.slug.clone())), "variable" => { let variables = variables(intent, id)?; variables diff --git a/core/crates/ployz-store/src/review/mod.rs b/core/crates/ployz-store/src/review/mod.rs index 1c17bbc2e1..6a756e55af 100644 --- a/core/crates/ployz-store/src/review/mod.rs +++ b/core/crates/ployz-store/src/review/mod.rs @@ -443,7 +443,7 @@ fn compare( .into_iter() .flat_map(|intent| &intent.services) .find(|service| service.id == node.id) - .map(|service| service.slug.to_string()); + .map(|service| service.slug.clone()); services .or_else(|| volume_name(&intents, &node.id)) .or_else(|| config_name(&intents, &node.id)) @@ -543,7 +543,7 @@ fn compare( let data = data_effect(&group.node, group.lifecycle, deployed, &environment.working); let restarts = - restarts(&group.node, group.lifecycle, &environment.working, deployed); + restarts(&group.node, group.lifecycle, &environment.working, deployed)?; Ok(NodeChange { node: group.node, name, @@ -684,14 +684,11 @@ fn sources(view: &mut DiffView, working: &SavedEnvironmentIntent) { else { continue; }; - let rows: Vec<_> = introduced_source( - service.slug.as_str(), - &service.lineage_id, - &service.config.source, - ) - .into_iter() - .filter(|row| !change.settings.iter().any(|shown| shown.row == row.row)) - .collect(); + let rows: Vec<_> = + introduced_source(&service.slug, &service.lineage_id, &service.config.source) + .into_iter() + .filter(|row| !change.settings.iter().any(|shown| shown.row == row.row)) + .collect(); view.total_count += rows.len(); change.settings.splice(0..0, rows); } @@ -778,7 +775,7 @@ fn renames(view: &mut DiffView, working: &SavedEnvironmentIntent, head: &SavedEn node_type: EnvironmentNodeType::Service, id: service.id.clone(), }, - name: service.slug.to_string(), + name: service.slug.clone(), row: RowId::node(&service.lineage_id), lifecycle: ReviewLifecycleKind::Update, comparison: Some(ReviewComparisonRole::Head), @@ -836,9 +833,9 @@ fn restarts( lifecycle: ReviewLifecycleKind, working: &SavedEnvironmentIntent, deployed: &SavedEnvironmentIntent, -) -> Vec { +) -> Result, RpcError> { if node.node_type != EnvironmentNodeType::Config || lifecycle != ReviewLifecycleKind::Update { - return Vec::new(); + return Ok(Vec::new()); } working .services @@ -850,7 +847,10 @@ fn restarts( .any(|mount| mount.config_resource_id == node.id) && deployed.services.iter().any(|old| old.id == service.id) }) - .map(|service| service.slug.clone()) + .map(|service| { + ployz_core::ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name")) + }) .collect() } diff --git a/core/crates/ployz-store/src/rules.rs b/core/crates/ployz-store/src/rules.rs index 7866414220..d9d44b6591 100644 --- a/core/crates/ployz-store/src/rules.rs +++ b/core/crates/ployz-store/src/rules.rs @@ -107,7 +107,7 @@ pub(crate) fn broken( Some(BrokenReference { at: match at { Referrer::Variable(service, variable) => ReferenceAt::Variable { - service: service.slug.clone(), + service: named(&service.slug)?, variable: variable.to_owned(), }, Referrer::File(config, file) => ReferenceAt::File { @@ -327,7 +327,7 @@ fn problems(facts: Facts<'_>) -> BTreeSet { if !working .services .iter() - .any(|service| service.slug == command.service) + .any(|service| service.slug == command.service.as_str()) { found.insert(Problem::NoSetupService(command.service.clone())); } @@ -511,7 +511,7 @@ impl<'a> Refusing<'a> { .iter() .find_map(|facts| { service(facts.working, lineage) - .map(|service| service.slug.to_string()) + .map(|service| service.slug.clone()) .or_else(|| facts.live.get(lineage).cloned()) .or_else(|| { facts @@ -576,7 +576,7 @@ impl<'a> Refusing<'a> { format!( "{name} is already mounted by {}; allow shared writes first: {allow}", if old.is_empty() { - first.slug.to_string() + first.slug.clone() } else { slugs(&old) } @@ -676,7 +676,7 @@ impl<'a> Refusing<'a> { .working .services .iter() - .any(|old| old.slug == *service); + .any(|old| old.slug == service.as_str()); if !had { return error::choices( format!( @@ -983,7 +983,7 @@ pub(crate) mod tests { logs.name = "logs".into(); working.volumes.push(logs); for service in &mut working.services { - let at = if service.slug.as_str() == "db" { + let at = if service.slug == "db" { "/data/" } else { "/sys/logs" @@ -1076,7 +1076,7 @@ pub(crate) mod tests { let redis = working .services .iter_mut() - .find(|service| service.slug.as_str() == "redis") + .find(|service| service.slug == "redis") .unwrap(); redis.variables[0].value = VariableValue::Plain { text: PlainText::Template(vec![ValuePart::Ref { @@ -1119,9 +1119,7 @@ pub(crate) mod tests { // Saved with db gone, production keeps editing and changing its other Setup Commands. seed(&store, &who, |working| { - working - .services - .retain(|service| service.slug.as_str() != "db"); + working.services.retain(|service| service.slug != "db"); }); unrelated(&store, &who).unwrap(); setup(&store, &who, &["db", "redis"]).unwrap(); diff --git a/core/crates/ployz-store/src/scope.rs b/core/crates/ployz-store/src/scope.rs index 7ec00ea926..c163268b9c 100644 --- a/core/crates/ployz-store/src/scope.rs +++ b/core/crates/ployz-store/src/scope.rs @@ -225,7 +225,7 @@ impl Environment { self.working .services .iter() - .map(|service| (service.lineage_id.clone(), service.slug.to_string())), + .map(|service| (service.lineage_id.clone(), service.slug.clone())), ); names } @@ -269,7 +269,7 @@ impl Environment { let services = &self.working.services; services .iter() - .find(|service| service.slug == *name) + .find(|service| service.slug == name.as_str()) .ok_or_else(|| no_service(name, &self.summary.name, &self.working)) } @@ -282,7 +282,7 @@ impl Environment { .working .services .iter() - .position(|service| service.slug == *name) + .position(|service| service.slug == name.as_str()) .ok_or_else(|| no_service(name, &self.summary.name, &self.working))?; Ok(self .working @@ -816,7 +816,7 @@ pub(crate) fn persist_working( if !stays(&service.lineage_id) { removed.record_beside( &service.lineage_id, - Some(service.slug.as_str()), + Some(&service.slug), &environment.inherited, ); } @@ -935,7 +935,7 @@ pub(crate) fn validate_and_refresh_working_from( if read.contains(&service.lineage_id) { environment .removed - .record(&service.lineage_id, Some(service.slug.as_str())); + .record(&service.lineage_id, Some(&service.slug)); } } environment.inherited = inherited( diff --git a/core/crates/ployz-store/src/sealing_targets.rs b/core/crates/ployz-store/src/sealing_targets.rs index 39b76082f5..ee6fa2e0df 100644 --- a/core/crates/ployz-store/src/sealing_targets.rs +++ b/core/crates/ployz-store/src/sealing_targets.rs @@ -133,11 +133,14 @@ pub(crate) fn targets( .working .services .iter() - .map(|service| SealingService { - name: service.slug.clone(), - service: Uuid::parse_str(&service.id).ok(), + .map(|service| { + Ok(SealingService { + name: ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, + service: Uuid::parse_str(&service.id).ok(), + }) }) - .collect(); + .collect::>()?; Ok(SealingTargetsView::Edit { organization, environment: environment.summary, diff --git a/core/crates/ployz-store/src/service/mod.rs b/core/crates/ployz-store/src/service/mod.rs index 0e4acdda1a..8e184886a9 100644 --- a/core/crates/ployz-store/src/service/mod.rs +++ b/core/crates/ployz-store/src/service/mod.rs @@ -158,7 +158,7 @@ pub(crate) fn insert_service( id: id.to_string(), // A new Service starts its own lineage; Branch copies keep it. lineage_id: id.to_string(), - slug: name.clone(), + slug: name.to_string(), config, variables: Vec::new(), volume_attachments: Vec::new(), @@ -196,7 +196,7 @@ pub(crate) fn rename_service( crate::names::Name::Service(rename.name.as_str()), Some(&id), )?; - environment.service_mut(&rename.service)?.slug = rename.name.clone(); + environment.service_mut(&rename.service)?.slug = rename.name.to_string(); scope::save_working(tx, &mut environment, trusted)?; } let service = summary(environment.service(&rename.name)?)?; @@ -217,7 +217,7 @@ pub(crate) fn remove_service( let mut environment = scope::lock(tx, who, &remove.environment)?; let node = environment.service(&remove.service)?; let service = summary(node)?; - let names = BTreeMap::from([(node.lineage_id.clone(), node.slug.to_string())]); + let names = BTreeMap::from([(node.lineage_id.clone(), node.slug.clone())]); environment .working .services @@ -249,7 +249,7 @@ fn staged(changed: bool, name: &ServiceName) -> Vec { pub(crate) fn summary(node: &SavedServiceIntent) -> Result { Ok(ServiceSummary { id: ServiceLineageId::parse(node.id.as_str()).map_err(|_| error::corrupt("Service ID"))?, - name: node.slug.clone(), + name: ServiceName::parse(node.slug.as_str()).map_err(|_| error::corrupt("Service name"))?, private_dns: node.config.private_dns.clone(), }) } diff --git a/core/crates/ployz-store/src/settings/query.rs b/core/crates/ployz-store/src/settings/query.rs index c86841de94..b924b487ee 100644 --- a/core/crates/ployz-store/src/settings/query.rs +++ b/core/crates/ployz-store/src/settings/query.rs @@ -5,7 +5,7 @@ //! `{"secret": true}`: no read shows a secret. use ployz_core::config::{RowId, SavedEnvironmentIntent, SavedServiceIntent}; -use ployz_core::{ConfigName, RpcError}; +use ployz_core::{ConfigName, RpcError, ServiceName}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value, json}; use ts_rs::TS; @@ -101,7 +101,8 @@ pub(crate) fn environment( }; let mut settings = Vec::new(); for service in services { - let name = service.slug.clone(); + let name = ServiceName::parse(service.slug.as_str()) + .map_err(|_| crate::error::corrupt("Service name"))?; let mut row = |target: Target, value: Value, default: Value, apply: Apply| { if only.is_none_or(|only| *only == target) && !(whole && value == default) { let (row, note) = match &target { diff --git a/core/crates/ployz-store/src/typed_address.rs b/core/crates/ployz-store/src/typed_address.rs index 6101d85686..5b3a1855e1 100644 --- a/core/crates/ployz-store/src/typed_address.rs +++ b/core/crates/ployz-store/src/typed_address.rs @@ -15,6 +15,7 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use ts_rs::TS; +use crate::error; use crate::variables::VariableKey; /// What to set instead of a value with Typed Addresses. @@ -62,7 +63,7 @@ pub(crate) fn in_plain( }) .collect(); let hosts = hosts(&text, key, others); - let Some(services) = services(&hosts) else { + let Some(services) = services(&hosts)? else { return Ok(None); }; let mut refs = parts @@ -130,14 +131,18 @@ fn hosts<'env>( } /// The Services `hosts` belong to, each once; none when there are none. -fn services(hosts: &[(Range, &SavedServiceIntent)]) -> Option> { +fn services( + hosts: &[(Range, &SavedServiceIntent)], +) -> Result>, RpcError> { let mut services = Vec::new(); for (_, other) in hosts { - if !services.contains(&other.slug) { - services.push(other.slug.clone()); + let name = + ServiceName::parse(other.slug.as_str()).map_err(|_| error::corrupt("Service name"))?; + if !services.contains(&name) { + services.push(name); } } - (!services.is_empty()).then_some(services) + Ok((!services.is_empty()).then_some(services)) } /// `text` as parts, each NUL in it the next of `refs` again. diff --git a/core/crates/ployz-store/src/variables.rs b/core/crates/ployz-store/src/variables.rs index c82ee7b247..ef83189992 100644 --- a/core/crates/ployz-store/src/variables.rs +++ b/core/crates/ployz-store/src/variables.rs @@ -162,7 +162,7 @@ pub(crate) fn set( .working .services .iter() - .filter(|other| other.slug != *service) + .filter(|other| other.slug != service.as_str()) .collect(); let current = environment .service(service)? @@ -411,7 +411,7 @@ pub(crate) fn rename( )); } for referrer in &mut environment.working.services { - let own = referrer.slug == *service; + let own = referrer.slug == service.as_str(); for variable in &mut referrer.variables { let VariableValue::Plain { text: PlainText::Template(parts), @@ -464,7 +464,7 @@ fn referrers( ) -> Result, RpcError> { let mut found = Vec::new(); for referrer in &environment.working.services { - let own = referrer.slug == *service; + let own = referrer.slug == service.as_str(); for variable in sorted(referrer) { let parts = variable.value.parts(); let uses = parts.iter().any(|part| { @@ -472,7 +472,9 @@ fn referrers( if wanted == key.as_str() && references(owner, own, lineage)) }); if uses { - found.push((referrer.slug.clone(), VariableKey::parse(&variable.key)?)); + let name = ServiceName::parse(&referrer.slug) + .map_err(|_| error::corrupt("Service name"))?; + found.push((name, VariableKey::parse(&variable.key)?)); } } } diff --git a/core/crates/ployz-store/src/volume/mod.rs b/core/crates/ployz-store/src/volume/mod.rs index 50213f5dd2..45ec08047b 100644 --- a/core/crates/ployz-store/src/volume/mod.rs +++ b/core/crates/ployz-store/src/volume/mod.rs @@ -355,7 +355,8 @@ pub(crate) fn remove_volume( .volume_attachments .retain(|mount| mount.volume_resource_id != volume.id.as_str()); if service.volume_attachments.len() != before { - let name = service.slug.clone(); + let name = + ServiceName::parse(service.slug.as_str()).map_err(|_| error::corrupt("Service"))?; staged.push(SettingPath::at(&name, Target::Mount(volume.name.clone()))); } } @@ -498,7 +499,7 @@ fn broken(intent: &SavedEnvironmentIntent) -> Vec { for (key, (node, volume, mount)) in mounts(intent) { let service = node.lineage_id.clone(); let at = key.2.clone(); - if let Some(refused) = kernel_refusal(node.slug.as_str(), &volume.name, mount) { + if let Some(refused) = kernel_refusal(&node.slug, &volume.name, mount) { let rule = Broken::Kernel { service: service.clone(), at: at.clone(), diff --git a/core/crates/ployz-store/src/volume/query.rs b/core/crates/ployz-store/src/volume/query.rs index 2ed936ec3c..810f5837b1 100644 --- a/core/crates/ployz-store/src/volume/query.rs +++ b/core/crates/ployz-store/src/volume/query.rs @@ -6,7 +6,7 @@ use ployz_core::config::{ ReviewLifecycleKind, SavedEnvironmentIntent, SavedVolumeIntent, VolumeKind, }; -use ployz_core::{Namespace, ProvisionedVolumeMaximumBytes, RpcError}; +use ployz_core::{Namespace, ProvisionedVolumeMaximumBytes, RpcError, ServiceName}; use serde::{Deserialize, Serialize}; use ts_rs::TS; @@ -229,7 +229,7 @@ fn listed( Some(VolumeKind::Provisioned { maximum_bytes }) => Some(*maximum_bytes), Some(VolumeKind::Docker {}) | None => None, }, - mounts: mounts(working, &node.resource_id), + mounts: mounts(working, &node.resource_id)?, deployed: review .head .applied @@ -250,8 +250,8 @@ fn listed( Ok(listed) } -fn mounts(working: &SavedEnvironmentIntent, volume: &str) -> Vec { - let mut mounts: Vec = working +fn mounts(working: &SavedEnvironmentIntent, volume: &str) -> Result, RpcError> { + let mut mounts = working .services .iter() .flat_map(|service| { @@ -259,12 +259,15 @@ fn mounts(working: &SavedEnvironmentIntent, volume: &str) -> Vec { .volume_attachments .iter() .filter(|mount| mount.volume_resource_id == volume) - .map(move |mount| Mount { - service: service.slug.clone(), - path: mount.mount_path.clone(), + .map(move |mount| { + Ok(Mount { + service: ServiceName::parse(service.slug.as_str()) + .map_err(|_| error::corrupt("Service name"))?, + path: mount.mount_path.clone(), + }) }) }) - .collect(); + .collect::, RpcError>>()?; mounts.sort_by(|a, b| a.service.cmp(&b.service)); - mounts + Ok(mounts) } diff --git a/core/crates/ployz-store/tests/review.rs b/core/crates/ployz-store/tests/review.rs index 693be656cd..596f03917f 100644 --- a/core/crates/ployz-store/tests/review.rs +++ b/core/crates/ployz-store/tests/review.rs @@ -3276,7 +3276,7 @@ impl LegacyWorking { let variable = intent .services .iter_mut() - .find(|service| service.slug.as_str() == "web") + .find(|service| service.slug == "web") .unwrap() .variables .iter_mut() @@ -3291,7 +3291,7 @@ impl LegacyWorking { self.intent() .services .into_iter() - .find(|service| service.slug.as_str() == "web") + .find(|service| service.slug == "web") .unwrap() .variables .into_iter()