diff --git a/crates/oabctl/src/k8s_driver.rs b/crates/oabctl/src/k8s_driver.rs index e76e5c7..e7c9134 100644 --- a/crates/oabctl/src/k8s_driver.rs +++ b/crates/oabctl/src/k8s_driver.rs @@ -11,16 +11,27 @@ //! `aws-sm://`/raw-ARN values in a k8s-runtime manifest fail loudly at apply //! time — a manifest error, not a silent no-op. //! -//! `spec.bundleFrom` needs no k8s-specific handling at all (sub-slice 3c -//! turned out to be a non-issue): the actual bundle restore happens via -//! `openab`'s own `hooks.pre_seed` feature, wired into the composed -//! `config.toml`'s content at provisioning time -//! (`oabctl::studio_api::inject_pre_seed_hook`) — orchestrator-agnostic by -//! construction, since `pre_seed` is just "S3 GetObject + extract," it -//! doesn't know or care whether the booting process is an ECS task or a k8s -//! pod. `build_deployment` already points the container's command at -//! `configFrom`, same as ECS, so this Just Works without any driver code -//! here reading `bundleFrom` at all. +//! `spec.configFrom` for k8s (studio#138 — Brett: "k8s does not need to +//! fetch from s3") is `k8s-configmap://#`: `build_deployment` +//! mounts that ConfigMap at `/etc/openab` and leaves the container command +//! unset entirely, so the image's own baked-in default CMD +//! (`openab run -c /etc/openab/config.toml`, confirmed straight from every +//! `Dockerfile.*` in openabdev/openab) does the reading — no S3, no AWS +//! credentials needed in the pod at all. This reverses the original +//! sub-slice 3c decision (which reused the S3-backed `hooks.pre_seed` +//! carrier verbatim from the ECS path, "orchestrator-agnostic by +//! construction") — that path remains supported for backward compatibility +//! (a `configFrom` still carrying a legacy `s3://`/`http(s)://` URI keeps +//! getting the command-override treatment) but is no longer what fresh k8s +//! deploys produce. `spec.bundleFrom` itself is still unused either way. +//! +//! There is no create-vs-redeploy manifest lookup for k8s (unlike ECS's +//! `redeploy()`, which reuses a stored desired-state YAML) — every field +//! `studio-cp::build_default_k8s_manifest` sets is already resent by its one +//! caller (the wizard) on every call, so a fresh manifest is rebuilt from +//! scratch every time; `apply()` below still reports Created-vs-Updated +//! correctly from the live Deployment's own existence, no stored manifest +//! needed for that either. //! //! Observing k8s state into the canonical 6-state (the `apply`/`scale` //! counterpart to `status.rs`'s ECS `service_status`/`instance_status`) is @@ -36,8 +47,8 @@ use anyhow::{Context, Result}; use async_trait::async_trait; use k8s_openapi::api::apps::v1::{Deployment, DeploymentSpec}; use k8s_openapi::api::core::v1::{ - Container, EnvVar, EnvVarSource, PodSpec, PodTemplateSpec, ResourceRequirements, - SecretKeySelector, Toleration, + ConfigMapVolumeSource, Container, EnvVar, EnvVarSource, PodSpec, PodTemplateSpec, + ResourceRequirements, SecretKeySelector, Toleration, Volume, VolumeMount, }; use k8s_openapi::apimachinery::pkg::api::resource::Quantity; use k8s_openapi::apimachinery::pkg::apis::meta::v1::{LabelSelector, ObjectMeta}; @@ -186,17 +197,51 @@ fn build_deployment(m: &OABServiceManifest) -> Result { } env.extend(secret_env_vars(m)?); - // Same convention as EcsDriver (apply.rs): the image's default CMD points - // at a config.toml nothing populates, so override it to load configFrom - // directly via openab's own s3:// support — no download step needed. - let command = (!m.spec.config_from.is_empty()).then(|| { - vec![ - "openab".to_string(), - "run".to_string(), - "-c".to_string(), - m.spec.config_from.clone(), - ] - }); + // studio#138: a `k8s-configmap://#` configFrom mounts that + // ConfigMap at /etc/openab and leaves the container command unset — the + // image's own default CMD already reads /etc/openab/config.toml, so no + // override, no S3, no AWS credentials needed in the pod. Anything else + // non-empty (a legacy s3://... or http(s)://... configFrom) keeps the + // old override-the-command behavior, same convention EcsDriver + // (apply.rs) uses for its own s3:// support. + let configmap_ref = match crate::secrets::parse_k8s_configmap_uri(&m.spec.config_from) { + None => None, + Some(Ok(v)) => Some(v), + // Bake the agent name into the same message as the parse error — + // anyhow's Display only surfaces the outermost `.with_context()` + // frame, so a separate wrapper here would silently swallow the + // parser's own detail (which scheme, which malformed part). + Some(Err(e)) => { + anyhow::bail!("{e} — manifest '{}/{}'", m.metadata.namespace, m.metadata.name) + } + }; + let (command, volumes, volume_mounts) = if let Some((config_map_name, _key)) = configmap_ref { + let volume = Volume { + name: "config".to_string(), + config_map: Some(ConfigMapVolumeSource { + name: config_map_name.to_string(), + ..Default::default() + }), + ..Default::default() + }; + let mount = VolumeMount { + name: "config".to_string(), + mount_path: "/etc/openab".to_string(), + read_only: Some(true), + ..Default::default() + }; + (None, Some(vec![volume]), Some(vec![mount])) + } else { + let command = (!m.spec.config_from.is_empty()).then(|| { + vec![ + "openab".to_string(), + "run".to_string(), + "-c".to_string(), + m.spec.config_from.clone(), + ] + }); + (command, None, None) + }; let tolerations: Vec = k8s_rt .tolerations @@ -213,6 +258,7 @@ fn build_deployment(m: &OABServiceManifest) -> Result { command, env: Some(env), resources: Some(resource_requirements(&m.spec.resources)), + volume_mounts, ..Default::default() }; @@ -222,6 +268,7 @@ fn build_deployment(m: &OABServiceManifest) -> Result { node_selector: (!k8s_rt.node_selector.is_empty()) .then(|| k8s_rt.node_selector.clone().into_iter().collect()), tolerations: (!tolerations.is_empty()).then_some(tolerations), + volumes, ..Default::default() }; @@ -413,6 +460,39 @@ mod tests { assert!(msg.contains("prod/orca"), "must name the agent: {msg}"); } + #[test] + fn build_deployment_mounts_k8s_native_configmap_and_skips_command_override() { + let mut m = k8s_manifest(None, &[]); + m.spec.config_from = "k8s-configmap://orca-config#config.toml".to_string(); + let dep = build_deployment(&m).unwrap(); + let pod = dep.spec.unwrap().template.spec.unwrap(); + + // No command override — the image's own default CMD reads the + // mounted /etc/openab/config.toml, no S3 involved. + assert_eq!(pod.containers[0].command, None); + + let volumes = pod.volumes.unwrap(); + assert_eq!(volumes.len(), 1); + assert_eq!(volumes[0].name, "config"); + assert_eq!(volumes[0].config_map.as_ref().unwrap().name, "orca-config"); + + let mounts = pod.containers[0].volume_mounts.as_ref().unwrap(); + assert_eq!(mounts.len(), 1); + assert_eq!(mounts[0].name, "config"); + assert_eq!(mounts[0].mount_path, "/etc/openab"); + assert_eq!(mounts[0].read_only, Some(true)); + } + + #[test] + fn build_deployment_rejects_malformed_k8s_configmap_ref() { + let mut m = k8s_manifest(None, &[]); + m.spec.config_from = "k8s-configmap://orca-config".to_string(); // missing #key + let err = build_deployment(&m).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("k8s-configmap://"), "must name the scheme: {msg}"); + assert!(msg.contains("prod/orca"), "must name the agent: {msg}"); + } + #[test] fn build_deployment_sets_image_command_and_env() { let m = k8s_manifest(None, &[]); diff --git a/crates/oabctl/src/secrets.rs b/crates/oabctl/src/secrets.rs index ed875b5..b2ffce7 100644 --- a/crates/oabctl/src/secrets.rs +++ b/crates/oabctl/src/secrets.rs @@ -51,6 +51,23 @@ pub(crate) fn parse_k8s_secret_uri(value: &str) -> Option> }) } +/// Parse `k8s-configmap://#` into `(configmap_name, +/// key)` — `spec.configFrom`'s k8s-native counterpart to `k8s-secret://` +/// (studio#138). Same shape, same "pure parsing, no API call" contract: the +/// ConfigMap itself must already exist in the target namespace by the time +/// `k8s_driver::build_deployment` mounts it. +pub(crate) fn parse_k8s_configmap_uri(value: &str) -> Option> { + let rest = value.strip_prefix("k8s-configmap://")?; + Some(match rest.rsplit_once('#') { + Some((config_map_name, key)) if !config_map_name.is_empty() && !key.is_empty() => { + Ok((config_map_name, key)) + } + _ => Err(anyhow::anyhow!( + "invalid k8s-configmap:// ref '{value}' — expected k8s-configmap://#" + )), + }) +} + /// Resolve a `spec.secrets` value into the ECS-native `valueFrom` format ECS /// actually requires. ECS's `valueFrom` requires the *full* ARN (not just a /// secret name) whenever a JSON-key suffix is present, so an `aws-sm://` @@ -307,4 +324,30 @@ mod tests { assert!(parse_k8s_secret_uri("aws-sm://oab/telegram/pahudxbot#TOKEN").is_none()); assert!(parse_k8s_secret_uri("plain-secret-name").is_none()); } + + #[test] + fn parse_k8s_configmap_uri_extracts_name_and_key() { + let (name, key) = parse_k8s_configmap_uri("k8s-configmap://oab-orca-config#config.toml") + .unwrap() + .unwrap(); + assert_eq!(name, "oab-orca-config"); + assert_eq!(key, "config.toml"); + } + + #[test] + fn parse_k8s_configmap_uri_rejects_missing_hash() { + assert!(parse_k8s_configmap_uri("k8s-configmap://oab-orca-config").unwrap().is_err()); + } + + #[test] + fn parse_k8s_configmap_uri_rejects_empty_parts() { + assert!(parse_k8s_configmap_uri("k8s-configmap://#config.toml").unwrap().is_err()); + assert!(parse_k8s_configmap_uri("k8s-configmap://oab-orca-config#").unwrap().is_err()); + } + + #[test] + fn parse_k8s_configmap_uri_returns_none_for_other_schemes() { + assert!(parse_k8s_configmap_uri("s3://bucket/artifacts/prod/orca/config.toml").is_none()); + assert!(parse_k8s_configmap_uri("k8s-secret://oab-orca#DISCORD_BOT_TOKEN").is_none()); + } } diff --git a/crates/studio-cp/src/lib.rs b/crates/studio-cp/src/lib.rs index f6d1a69..0879d58 100644 --- a/crates/studio-cp/src/lib.rs +++ b/crates/studio-cp/src/lib.rs @@ -1571,63 +1571,66 @@ pub async fn provision_agent_k8s( directly, or deploy this agent to ECS instead." ); } + // `provision_agent_secrets` only ever touches AWS Secrets Manager when + // `input.api_key` is set (chat-platform secrets are unreachable here — + // the bail! above already refused those) — a no-op otherwise, so this + // stays harmless for the common "no vendor API key" k8s deploy. provision_agent_secrets(aws_config, namespace, name, &input).await?; let config_toml = generate_agent_config(namespace, name, &input).into_bytes(); if let Some(folder) = local_config_folder { write_local_agent_config(folder, name, &config_toml)?; } - let mut bundle = studio_compose::Bundle { + + // studio#138: k8s pods read config.toml from a mounted ConfigMap — the + // same pattern openab's own Helm chart already uses + // (`charts/openab/templates/configmap.yaml`), and the image's own + // default CMD already points at `/etc/openab/config.toml` unprompted + // (see `k8s_driver::build_deployment`) — instead of the S3 + // bundle.zip + `hooks.pre_seed` carrier the AWS path uses. A k8s deploy + // now never touches S3, and the pod itself never needs AWS credentials + // just to boot. + let config_from = provision_config_k8s_configmap(context, namespace, name, &config_toml).await?; + + // Content-address of what's actually being applied — reuses + // `studio_compose::Bundle::digest()`'s tested hashing rather than + // hand-rolling one, even though nothing here gets zipped/uploaded. + let digest = studio_compose::Bundle { image_tag: image.to_string(), files: std::collections::BTreeMap::from([("config.toml".to_string(), config_toml)]), - }; - - let bucket = oabctl::resolve_bucket(aws_config, None).await?; - let zip_uri = oabctl::studio_api::bundle_zip_uri(&bucket, namespace, name); - match bundle.files.get_mut("config.toml") { - Some(bytes) => *bytes = oabctl::studio_api::inject_pre_seed_hook(bytes, &zip_uri)?, - None => unreachable!("just inserted above"), } + .digest(); + + // Always rebuilt fresh (no stored-manifest reuse, unlike the AWS path's + // redeploy()): every field `build_default_k8s_manifest` sets is already + // resent by the wizard on every submit (image, expected_principal, + // acp_enabled, acp_token), so there's nothing a prior stored manifest + // would preserve that this call doesn't already provide — and this is + // what lets the create-vs-redeploy check itself drop S3 (no + // `manifests/{ns}/{name}.yaml` lookup needed at all; `K8sDriver::apply` + // already reports Created-vs-Updated from the live Deployment). + let manifest = build_default_k8s_manifest( + context, + namespace, + name, + image, + &config_from, + expected_principal, + input.acp_enabled, + input.acp_token.as_deref(), + ) + .await?; - let mut objects = bundle.artifact_objects(namespace, name); - let zip_key = format!( - "{}/{}", - studio_compose::artifacts_prefix(namespace, name), - oabctl::studio_api::BUNDLE_ZIP_FILENAME - ); - objects.push((zip_key, bundle.zip_bytes())); - - let digest = bundle.digest(); - - let existing_manifest = - oabctl::studio_api::load_manifest(aws_config, namespace, name, Some(&bucket)).await?; - let mut manifest = match existing_manifest { - Some(mut stored) => { - stored.spec.image = image.to_string(); - stored - } - None => { - build_default_k8s_manifest( - context, - namespace, - name, - image, - &bucket, - expected_principal, - input.acp_enabled, - input.acp_token.as_deref(), - ) - .await? - } + let driver = oabctl::K8sDriver::from_context(context).await?; + let opts = oabctl::ProvisionOptions { control_plane_bucket: None, wait: false }; + let report = { + use oabctl::ProvisionDriver; + driver.apply(std::slice::from_ref(&manifest), &opts).await? }; - manifest.spec.bundle_from = Some(oabctl::studio_api::bundle_from_uri(&bucket, namespace, name)); - - let report = - oabctl::studio_api::provision_k8s(aws_config, context, &manifest, &objects, Some(&bucket)).await?; Ok(ProvisionOutcome { image: image.to_string(), digest, - objects: objects.len(), + objects: 0, services_applied: report.services.len(), action: report .services @@ -1693,6 +1696,50 @@ async fn provision_acp_auth_k8s_secret( Ok(format!("k8s-secret://{secret_name}#OPENAB_ACP_AUTH_KEY")) } +/// Server-side-applies `config.toml` into a k8s `ConfigMap` named +/// `{name}-config` in `namespace` (studio#138 — Brett: "k8s does not need to +/// fetch from s3"). Mirrors openab's own Helm chart pattern +/// (`charts/openab/templates/configmap.yaml`, mounted at `/etc/openab`) — +/// `k8s_driver::build_deployment` mounts this ConfigMap and leaves the +/// image's own default CMD (`openab run -c /etc/openab/config.toml`, +/// confirmed straight from every `Dockerfile.*`'s `CMD`) untouched, so no +/// command override, no S3, no AWS credentials in the pod at all. Returns a +/// `k8s-configmap://#` ref — same scheme shape `spec.secrets`' +/// `k8s-secret://` refs already use — for `build_deployment` to parse back +/// into a volume + mount. `Patch::Apply`, same idempotent-retry shape as +/// [`provision_acp_auth_k8s_secret`]. +async fn provision_config_k8s_configmap( + context: Option<&str>, + namespace: &str, + name: &str, + config_toml: &[u8], +) -> anyhow::Result { + use k8s_openapi::api::core::v1::ConfigMap; + use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta; + use kube::api::{Api, Patch, PatchParams}; + + let client = k8s_client_for(context).await?; + let config_map_name = format!("{name}-config"); + 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(); + data.insert("config.toml".to_string(), text); + let config_map = ConfigMap { + metadata: ObjectMeta { + name: Some(config_map_name.clone()), + namespace: Some(namespace.to_string()), + ..Default::default() + }, + data: Some(data), + ..Default::default() + }; + let api: Api = Api::namespaced(client, namespace); + api.patch(&config_map_name, &PatchParams::apply("studio-cp"), &Patch::Apply(&config_map)) + .await + .map_err(|e| anyhow::anyhow!("failed to create/apply k8s ConfigMap '{config_map_name}': {e}"))?; + Ok(format!("k8s-configmap://{config_map_name}#config.toml")) +} + /// Build a fresh k8s `OABServiceManifest` — the `Runtime::Kubernetes` /// counterpart to [`build_default_manifest`]. No VPC/subnet/security-group /// concept (that's ECS-specific networking); k8s's per-fleet placement is @@ -1711,12 +1758,11 @@ async fn build_default_k8s_manifest( namespace: &str, name: &str, image: &str, - bucket: &str, + config_from: &str, expected_principal: Option<&str>, acp_enabled: bool, acp_token: Option<&str>, ) -> anyhow::Result { - let config_from = default_config_from_uri(bucket, namespace, name); let mut secrets = std::collections::HashMap::new(); if acp_enabled { let acp_auth_ref = provision_acp_auth_k8s_secret(context, namespace, name, acp_token).await?; @@ -1736,7 +1782,7 @@ async fn build_default_k8s_manifest( cpu: "256".to_string(), memory: "512".to_string(), }, - config_from, + config_from: config_from.to_string(), bundle_from: None, bootstrap_from: None, secrets, @@ -1820,7 +1866,7 @@ pub async fn provision_from_library_k8s( namespace, name, &image, - &bucket, + &default_config_from_uri(&bucket, namespace, name), expected_principal, true, None,