From 035fc3493006862d30cb6ef59634b5044f3f296c Mon Sep 17 00:00:00 2001 From: Brett Chien Date: Sun, 30 Aug 2026 09:41:49 +0800 Subject: [PATCH] fix(k8s): slugify agent/namespace names into valid k8s object names (studio#138 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brett hit "ConfigMap \"Persephone-config\" is invalid ... must be a lowercase RFC 1123 subdomain" — the Agent name field's "suggest a Greek god name" default (studio#128) capitalizes the first letter, and nothing downstream lowercased it before using it to build k8s object names. ## Change - New `oabctl::k8s_safe_name` (in k8s_driver.rs, re-exported at crate root): lowercases and maps any character outside `[a-z0-9-]` to `-`, then trims leading/trailing `-` so the result still starts/ends alphanumeric per the RFC 1123 regex. - `k8s_deployment_name` now slugifies internally — every Deployment name derived from an agent name is safe by construction. - `provision_acp_auth_k8s_secret`/`provision_config_k8s_configmap` (studio-cp) now slugify the agent name before building their Secret/ConfigMap names. - `provision_agent_k8s`/`provision_from_library_k8s`: the free-typed "+ Create new namespace…" field has the exact same risk, so `namespace` is now normalized once up front and shadowed for the rest of the function — `ensure_namespace_k8s` (#140) and every object placed into that namespace now agree on the same slugified value, instead of `ensure_namespace_k8s` creating one casing while everything else tries to apply into another. Verification: could not compile locally (sandbox OOMs on aws-sdk-ec2). Added unit tests for `k8s_safe_name` and a regression test reproducing the exact "Persephone" case Brett hit. CI is the real gate. Co-Authored-By: Claude Sonnet 5 --- crates/oabctl/src/k8s_driver.rs | 38 ++++++++++++++++++++++++++++++++- crates/oabctl/src/lib.rs | 2 +- crates/studio-cp/src/lib.rs | 16 ++++++++++++-- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/oabctl/src/k8s_driver.rs b/crates/oabctl/src/k8s_driver.rs index e7c9134..f3a34db 100644 --- a/crates/oabctl/src/k8s_driver.rs +++ b/crates/oabctl/src/k8s_driver.rs @@ -56,6 +56,24 @@ use kube::api::{Api, DeleteParams, Patch, PatchParams}; use kube::{Client, Config}; use std::collections::BTreeMap; +/// Slugify an OAB agent name into a valid k8s object name (RFC 1123 lowercase +/// subdomain — `[a-z0-9]([-a-z0-9]*[a-z0-9])?`). The Agent name field is free +/// text (and its "suggest a Greek god name" default capitalizes — Brett hit +/// this directly: "Persephone-config" 422'd), so nothing upstream guarantees +/// it's already k8s-safe the way an ECS service name doesn't need to be. +/// Lowercases and maps any character outside `[a-z0-9-]` to `-`, then trims +/// leading/trailing `-` so the result still starts/ends alphanumeric (a +/// dash run in the middle is valid per the RFC, so no need to collapse +/// those). +pub fn k8s_safe_name(name: &str) -> String { + name.to_lowercase() + .chars() + .map(|c| if c.is_ascii_alphanumeric() || c == '-' { c } else { '-' }) + .collect::() + .trim_matches('-') + .to_string() +} + /// The k8s Deployment name for an OAB agent. OAB's own `metadata.namespace` /// maps directly to the k8s namespace (a k8s namespace is already an /// isolation/grouping boundary, same job OAB's `namespace` does for ECS @@ -63,7 +81,7 @@ use std::collections::BTreeMap; /// flat because ECS has no per-namespace boundary), the Deployment only needs /// `oab-{name}` within that namespace. pub fn k8s_deployment_name(name: &str) -> String { - format!("oab-{name}") + format!("oab-{}", k8s_safe_name(name)) } /// The k8s implementation. Bound to one kubeconfig context (and therefore one @@ -415,6 +433,24 @@ mod tests { assert_eq!(k8s_deployment_name("orca"), "oab-orca"); } + #[test] + fn deployment_name_lowercases_a_capitalized_agent_name() { + // Brett hit this live: the Agent name field's "suggest a Greek god + // name" default capitalizes ("Persephone"), which 422'd every k8s + // object derived from it. + assert_eq!(k8s_deployment_name("Persephone"), "oab-persephone"); + } + + #[test] + fn k8s_safe_name_maps_invalid_characters_to_hyphens() { + assert_eq!(k8s_safe_name("My Agent!"), "my-agent"); + } + + #[test] + fn k8s_safe_name_trims_leading_and_trailing_hyphens() { + assert_eq!(k8s_safe_name("-orca-"), "orca"); + } + #[test] fn build_deployment_ignores_bundle_from_no_special_handling_needed() { // bundleFrom isn't consumed by the driver at all (see module docs) — diff --git a/crates/oabctl/src/lib.rs b/crates/oabctl/src/lib.rs index 0bfbd43..2fab885 100644 --- a/crates/oabctl/src/lib.rs +++ b/crates/oabctl/src/lib.rs @@ -78,7 +78,7 @@ pub use manifest::{ pub use control_plane::resolve_bucket; pub use driver::{EcsDriver, ProvisionDriver, ProvisionOptions}; pub use events::{fetch_ecs_events, EcsEvent, DEFAULT_EVENTS_LOG_GROUP}; -pub use k8s_driver::{k8s_deployment_name, K8sDriver}; +pub use k8s_driver::{k8s_deployment_name, k8s_safe_name, K8sDriver}; pub use status::{instance_status, service_status, InstanceStatus, ServiceStatus}; #[doc(hidden)] diff --git a/crates/studio-cp/src/lib.rs b/crates/studio-cp/src/lib.rs index f34ef64..2b97e50 100644 --- a/crates/studio-cp/src/lib.rs +++ b/crates/studio-cp/src/lib.rs @@ -1571,6 +1571,14 @@ pub async fn provision_agent_k8s( directly, or deploy this agent to ECS instead." ); } + // Normalized once, up front, and shadowed for the rest of this function — + // every k8s object placed below (the namespace itself, the ConfigMap, the + // ACP Secret, the Deployment) must agree on the same slugified value, or + // `ensure_namespace_k8s` would create one namespace while everything else + // tries to apply into a differently-cased one that was never created. + // The wizard's "+ Create new namespace…" field is free text, same as the + // Agent name field ("Persephone-config" 422'd for the same reason). + let namespace = &oabctl::k8s_safe_name(namespace); ensure_namespace_k8s(context, namespace).await?; // `provision_agent_secrets` only ever touches AWS Secrets Manager when // `input.api_key` is set (chat-platform secrets are unreachable here — @@ -1677,7 +1685,7 @@ async fn provision_acp_auth_k8s_secret( use kube::api::{Api, Patch, PatchParams}; let client = k8s_client_for(context).await?; - let secret_name = format!("{name}-acp"); + let secret_name = format!("{}-acp", oabctl::k8s_safe_name(name)); let key = token.map(str::to_string).unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); let mut string_data = std::collections::BTreeMap::new(); string_data.insert("OPENAB_ACP_AUTH_KEY".to_string(), key); @@ -1748,7 +1756,7 @@ async fn provision_config_k8s_configmap( use kube::api::{Api, Patch, PatchParams}; let client = k8s_client_for(context).await?; - let config_map_name = format!("{name}-config"); + let config_map_name = format!("{}-config", oabctl::k8s_safe_name(name)); let text = String::from_utf8(config_toml.to_vec()) .map_err(|e| anyhow::anyhow!("config.toml must be valid UTF-8: {e}"))?; let mut data = std::collections::BTreeMap::new(); @@ -1852,6 +1860,10 @@ pub async fn provision_from_library_k8s( image_override: Option<&str>, expected_principal: Option<&str>, ) -> anyhow::Result { + // Same normalize-once-up-front reasoning as `provision_agent_k8s` — keep + // `ensure_namespace_k8s`'s created namespace consistent with every other + // k8s object placed into it below. + let namespace = &oabctl::k8s_safe_name(namespace); ensure_namespace_k8s(context, namespace).await?; let mut bundle = studio_compose::compose_named(library, template, overlay) .map_err(|e| anyhow::anyhow!("compose failed: {e}"))?;