Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/oabctl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ serde_json = "1"
serde_yaml = "0.9"
tokio = { version = "1.40", features = ["full"] }
toml = "0.8"
toml_edit = "0.22"
anyhow = "1.0"
dirs = "6"
rpassword = "7"
Expand Down
4 changes: 3 additions & 1 deletion crates/oabctl/src/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ impl ApplyError {
}
}

fn reconciliation(
// pub(crate): driver.rs's tests construct one to verify the
// ApplyError -> anyhow::Error conversion preserves it in the source chain.
pub(crate) fn reconciliation(
failed_service: ServiceTarget,
completed: ApplyReport,
source: impl Into<anyhow::Error>,
Expand Down
9 changes: 8 additions & 1 deletion crates/oabctl/src/control_plane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ pub(crate) fn select_bucket(configured: Option<&str>, env: Option<&str>) -> Opti
.map(str::to_owned)
}

pub(crate) async fn resolve_bucket(
/// Resolve the control-plane bucket: `configured`, else `$OAB_CONTROL_PLANE_BUCKET`,
/// else `oab-control-plane-{account}` derived from the caller's AWS identity
/// (an STS call — only made when neither of the first two is set). `provision`/
/// `redeploy`/`push_bundle`/`delete` all resolve it internally via this same
/// function; exposed `pub` so a caller that needs the resolved value *before*
/// calling one of those (e.g. to compute a bundle's zip URI ahead of upload)
/// doesn't have to reimplement or guess at the resolution order.
pub async fn resolve_bucket(
aws_config: &aws_config::SdkConfig,
configured: Option<&str>,
) -> Result<String> {
Expand Down
53 changes: 52 additions & 1 deletion crates/oabctl/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl<'a> ProvisionDriver for EcsDriver<'a> {
}
crate::apply::apply_manifests(self.aws_config, manifests, &ecs_opts)
.await
.map_err(|e| anyhow::anyhow!("apply failed [{:?}]: {e}", e.kind))
.map_err(ecs_apply_error_to_anyhow)
}

async fn scale(&self, namespace: &str, name: &str, size: i32) -> Result<()> {
Expand All @@ -93,3 +93,54 @@ impl<'a> ProvisionDriver for EcsDriver<'a> {
.await
}
}

/// Convert `apply_manifests`'s structured error into an `anyhow::Error`
/// while keeping the `ApplyError` itself in the source chain — a caller can
/// still `err.chain().find_map(anyhow::Error::downcast_ref::<apply::ApplyError>)`
/// to recover `.completed`/`.failed_service` for a partial fleet-apply
/// failure, same as callers could before `ProvisionDriver` existed (when
/// `studio_api::provision` used `.context(..)` directly on `apply_manifests`'s
/// result). A prior version of this flattened `e` into `anyhow::anyhow!(...)`,
/// which silently dropped that recovery path — extracted as its own function
/// so the conversion is unit-testable without a live ECS call.
fn ecs_apply_error_to_anyhow(e: crate::apply::ApplyError) -> anyhow::Error {
let kind = e.kind;
anyhow::Error::new(e).context(format!("apply failed [{kind:?}]"))
}

#[cfg(test)]
mod tests {
use super::*;
use crate::apply::{ApplyAction, AppliedService, ApplyError, ApplyErrorKind, ApplyReport, ServiceTarget};

#[test]
fn ecs_apply_error_to_anyhow_preserves_downcast_and_partial_progress() {
let completed = ApplyReport {
services: vec![AppliedService {
namespace: "prod".to_string(),
name: "orca".to_string(),
resource_name: "oab-prod-orca".to_string(),
action: ApplyAction::Updated,
webhook_urls: vec![],
warnings: vec![],
}],
};
let failed = ServiceTarget {
namespace: "prod".to_string(),
name: "mira".to_string(),
ecs_service_name: "oab-prod-mira".to_string(),
};
let source = ApplyError::reconciliation(failed.clone(), completed.clone(), anyhow::anyhow!("boom"));

let err = ecs_apply_error_to_anyhow(source);

assert!(err.to_string().contains("Reconciliation"));
let recovered = err
.chain()
.find_map(|e| e.downcast_ref::<ApplyError>())
.expect("ApplyError must survive in the source chain");
assert_eq!(recovered.kind, ApplyErrorKind::Reconciliation);
assert_eq!(recovered.failed_service, Some(failed));
assert_eq!(recovered.completed, completed);
}
}
83 changes: 42 additions & 41 deletions crates/oabctl/src/k8s_driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@
//! `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` (the composed persona/skills bundle) is explicitly
//! **not yet supported** and fails loudly rather than silently
//! mis-deploying — ECS gets this for free via its S3 file carrier, k8s needs
//! a ConfigMap/volume carrier, tracked as sub-slice 3c.
//! `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.
//!
//! Observing k8s state into the canonical 6-state (the `apply`/`scale`
//! counterpart to `status.rs`'s ECS `service_status`/`instance_status`) is
Expand Down Expand Up @@ -88,19 +94,6 @@ fn require_kubernetes_runtime(m: &OABServiceManifest) -> Result<&crate::manifest
}
}

/// Reject the still-not-yet-supported manifest feature explicitly (see
/// module docs) instead of silently dropping it.
fn reject_unsupported(m: &OABServiceManifest) -> Result<()> {
if m.spec.bundle_from.is_some() {
anyhow::bail!(
"k8s bundle carrier not implemented yet (studio#97 sub-slice 3c) — '{}/{}' has spec.bundleFrom set",
m.metadata.namespace,
m.metadata.name
);
}
Ok(())
}

/// Build the `env[]` entries for `spec.secrets`: each value must be a
/// `k8s-secret://<secret-name>#<key>` ref, which becomes a `secretKeyRef` —
/// kubelet resolves it at pod-start time, no API call needed here (unlike
Expand All @@ -112,14 +105,22 @@ fn secret_env_vars(m: &OABServiceManifest) -> Result<Vec<EnvVar>> {
.secrets
.iter()
.map(|(env_name, value)| {
// parse_k8s_secret_uri returns Option<Result<..>>: None for the
// wrong scheme, Some(Err(..)) for the right scheme but a
// malformed body (e.g. missing '#'). anyhow's Context impl for
// Option<T> only fires on None, so a chained `.with_context()??`
// here would silently drop this context on the Some(Err(..))
// path — attach it explicitly to both instead.
let context = || {
format!(
"spec.secrets['{env_name}'] for k8s runtime must use \
k8s-secret://<secret-name>#<key> (got '{value}') — '{}/{}'",
m.metadata.namespace, m.metadata.name
)
};
let (secret_name, key) = crate::secrets::parse_k8s_secret_uri(value)
.with_context(|| {
format!(
"spec.secrets['{env_name}'] for k8s runtime must use \
k8s-secret://<secret-name>#<key> (got '{value}') — '{}/{}'",
m.metadata.namespace, m.metadata.name
)
})??;
.ok_or_else(|| anyhow::anyhow!(context()))?
.with_context(context)?;
Ok(EnvVar {
name: env_name.clone(),
value_from: Some(EnvVarSource {
Expand Down Expand Up @@ -249,7 +250,6 @@ impl ProvisionDriver for K8sDriver {
async fn apply(&self, manifests: &[OABServiceManifest], _opts: &ProvisionOptions) -> Result<ApplyReport> {
let mut services = Vec::with_capacity(manifests.len());
for m in manifests {
reject_unsupported(m)?;
let deployment = build_deployment(m)?;
let name = k8s_deployment_name(&m.metadata.name);
let api: Api<Deployment> = Api::namespaced(self.client.clone(), &m.metadata.namespace);
Expand Down Expand Up @@ -359,22 +359,12 @@ mod tests {
}

#[test]
fn reject_unsupported_passes_a_plain_manifest() {
let m = k8s_manifest(None, &[]);
reject_unsupported(&m).unwrap();
}

#[test]
fn reject_unsupported_bails_on_bundle_from() {
let m = k8s_manifest(Some("s3://bucket/artifacts/prod/orca/"), &[]);
let err = reject_unsupported(&m).unwrap_err();
assert!(err.to_string().contains("3c"));
}

#[test]
fn reject_unsupported_passes_manifests_with_k8s_secrets() {
let m = k8s_manifest(None, &[("DISCORD_BOT_TOKEN", "k8s-secret://oab-orca#DISCORD_BOT_TOKEN")]);
reject_unsupported(&m).unwrap();
fn build_deployment_ignores_bundle_from_no_special_handling_needed() {
// bundleFrom isn't consumed by the driver at all (see module docs) —
// a manifest carrying it builds identically to one without.
let with = k8s_manifest(Some("s3://bucket/artifacts/prod/orca/"), &[]);
let without = k8s_manifest(None, &[]);
assert_eq!(build_deployment(&with).unwrap(), build_deployment(&without).unwrap());
}

#[test]
Expand Down Expand Up @@ -402,6 +392,17 @@ mod tests {
assert!(err.to_string().contains("k8s-secret://"));
}

#[test]
fn build_deployment_attributes_malformed_k8s_secret_ref_to_its_env_var() {
// Right scheme, malformed body (missing #key) — must still name the
// offending spec.secrets entry, not just the bare parser error.
let m = k8s_manifest(None, &[("DISCORD_BOT_TOKEN", "k8s-secret://oab-orca")]);
let err = build_deployment(&m).unwrap_err();
let msg = err.to_string();
assert!(msg.contains("DISCORD_BOT_TOKEN"), "must name the env var: {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, &[]);
Expand Down
1 change: 1 addition & 0 deletions crates/oabctl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ pub use manifest::{
Runtime, Spec,
};

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};
Expand Down
Loading
Loading