From d2910fc038b317fe34a2f06df6d08d4200fc0083 Mon Sep 17 00:00:00 2001 From: brettchien Date: Fri, 14 Aug 2026 10:56:16 +0800 Subject: [PATCH] =?UTF-8?q?feat(oab-mcp):=20fleet-aware=20tool=20surface?= =?UTF-8?q?=20=E2=80=94=20target=20by=20fleet=20name,=20scope=20to=20membe?= =?UTF-8?q?rs=20(ADR=20Part=20B=20slice=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tools took only `cluster`, so they could not name a fleet when two fleets share a cluster (orca + mira on `oab`) — deploy_list returned both. This makes the MCP surface fleet-aware, the precondition for the Part B single-`oab`-server design. - All 8 cluster-bearing tools gain a `fleet` arg. A new `target()` resolver maps `fleet` (name) → the fleet's cluster (→ managing credential) + its member scope. An unknown fleet is an error, never a silent fall-through to the default cluster (that silent fallback is the AccessDenied class of bug the fleet work kills). Absent `fleet`, behaviour is unchanged (cluster arg / default) — back-compat. - Listing tools (deploy_list, get_agent_states) filter to the fleet's members. - Writes (deploy_scale, deploy_delete) refuse a service outside the named fleet's members — a fleet handle only operates its own members. - runtime_context reports the *named* fleet's binding, so two fleets on one cluster resolve distinctly (not first-on-cluster). - Membership match centralized in studio-cp `FleetBinding::includes` (full ECS name or short name; empty members ⇒ whole cluster), with unit tests. Credential stays cluster/account-granular (Part A): a `fleet` changes which members are seen/acted on and which binding is reported, not the auth identity; cross-account per-fleet credentials are fleet ADR §5 Q4 (future). Verification: `cargo check --all-targets` clean (oab-mcp + studio-cp, full aws-sdk). Test execution left to CI's `cargo test --workspace` — the test-profile codegen of aws-sdk-ec2 OOMs this box (the constraint ci.yml documents). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- crates/oab-mcp/src/main.rs | 123 +++++++++++++++++++++++++++++------- crates/studio-cp/src/lib.rs | 44 +++++++++++++ 2 files changed, 144 insertions(+), 23 deletions(-) diff --git a/crates/oab-mcp/src/main.rs b/crates/oab-mcp/src/main.rs index 2efac4f..ee57eb4 100644 --- a/crates/oab-mcp/src/main.rs +++ b/crates/oab-mcp/src/main.rs @@ -62,6 +62,7 @@ fn tools() -> Vec { as_map(json!({ "type": "object", "properties": { + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential and, for listing tools, restricts results to its members. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." } } })), @@ -73,6 +74,7 @@ fn tools() -> Vec { "type": "object", "properties": { "service": { "type": "string", "description": "ECS service name (or bare agent name)." }, + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential and, for listing tools, restricts results to its members. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." } }, "required": ["service"] @@ -85,6 +87,7 @@ fn tools() -> Vec { "type": "object", "properties": { "service": { "type": "string", "description": "Optional: limit to one service." }, + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential and, for listing tools, restricts results to its members. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." } } })), @@ -99,6 +102,7 @@ fn tools() -> Vec { "since_minutes": { "type": "integer", "description": "Look-back window in minutes (default 1440 = 24h)." }, "limit": { "type": "integer", "description": "Max events, newest first (default 50, max 1000)." }, "log_group": { "type": "string", "description": "CloudWatch Logs group the events are archived to (default $OAB_EVENTS_LOG_GROUP, else /oab/ecs-events)." }, + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential and, for listing tools, restricts results to its members. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." } } })), @@ -110,6 +114,7 @@ fn tools() -> Vec { "type": "object", "properties": { "manifest_yaml": { "type": "string", "description": "Full manifest YAML document." }, + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential; a write to a service outside the fleet's members is refused. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." }, "wait": { "type": "boolean", "description": "Wait for services to stabilize (default false)." } }, @@ -124,6 +129,7 @@ fn tools() -> Vec { "properties": { "name": { "type": "string", "description": "Agent / service name (service = oab-{namespace}-{name})." }, "size": { "type": "integer", "enum": [0, 1], "description": "0 = off, 1 = on." }, + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential; a write to a service outside the fleet's members is refused. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." }, "namespace": { "type": "string", "description": "Namespace (default \"default\")." } }, @@ -138,6 +144,7 @@ fn tools() -> Vec { "properties": { "resource": { "type": "string", "description": "Resource kind, e.g. \"service\"." }, "name": { "type": "string" }, + "fleet": { "type": "string", "description": "Fleet name (see fleet_config): targets the fleet's cluster and managing credential; a write to a service outside the fleet's members is refused. Overrides the cluster arg." }, "cluster": { "type": "string", "description": "ECS cluster (defaults to the server's configured cluster)." }, "namespace": { "type": "string", "description": "Namespace (default \"default\")." } }, @@ -150,6 +157,7 @@ fn tools() -> Vec { as_map(json!({ "type": "object", "properties": { + "fleet": { "type": "string", "description": "Resolve the identity the named fleet's binding selects (distinguishes two fleets that share a cluster). Overrides the cluster arg." }, "cluster": { "type": "string", "description": "Resolve the identity the fleet binding for this cluster selects (defaults to the server's configured cluster)." } } })), @@ -203,6 +211,28 @@ fn event_json(e: &scp::EcsEvent) -> Value { }) } +/// The resolved target of a tool call: which cluster to act against (→ which +/// credential) and, when a fleet was named, which fleet governs the call (its +/// members scope listing tools; its binding drives `runtime_context`). +struct Target { + cluster: String, + /// The governing fleet when the call named one, else `None` (a bare + /// `cluster`/default call, unscoped). + binding: Option, +} + +impl Target { + /// Whether a service is in scope for this target — always true for an + /// unscoped call; else the fleet's membership test (empty members ⇒ whole + /// cluster). + fn includes(&self, service_name: &str, short_name: &str) -> bool { + match &self.binding { + None => true, + Some(b) => b.includes(service_name, short_name), + } + } +} + impl OabMcp { fn cluster(&self, args: &Map) -> String { args.get("cluster") @@ -211,6 +241,30 @@ impl OabMcp { .unwrap_or_else(|| self.default_cluster.clone()) } + /// Resolve a call's target from its args. A **`fleet`** (name) selects the + /// fleet's cluster (and thus its managing credential) and its member scope; + /// an unknown name is an **error**, never a silent fall-through to the + /// default cluster (that silent fallback is the AccessDenied class of bug the + /// fleet work exists to kill). Absent a `fleet`, the target is the `cluster` + /// arg (or the server default) with no member scope — back-compat. + fn target(&self, args: &Map) -> Result { + if let Some(name) = args.get("fleet").and_then(Value::as_str) { + let guard = self.bindings.read().unwrap(); + let binding = guard.get(name).cloned().ok_or_else(|| { + let known: Vec<&str> = guard.fleets.iter().map(|f| f.name.as_str()).collect(); + anyhow::anyhow!("unknown fleet {name:?}; configured fleets: [{}]", known.join(", ")) + })?; + return Ok(Target { + cluster: binding.cluster.clone(), + binding: Some(binding), + }); + } + Ok(Target { + cluster: self.cluster(args), + binding: None, + }) + } + /// The AWS config to act as for `cluster`: the fleet binding's credential /// when one governs it (resolved once, then memoized), else the default /// chain. This is where the per-fleet **switch** takes effect — a bound @@ -238,10 +292,12 @@ impl OabMcp { } async fn t_list(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let t = self.target(args)?; + let cluster = t.cluster.clone(); let svcs = scp::observe_services(&self.aws_for(&cluster).await, &cluster).await?; let deployments: Vec = svcs .iter() + .filter(|s| t.includes(&s.service_name, &s.name)) .map(|s| { json!({ "name": s.name, @@ -259,7 +315,7 @@ impl OabMcp { } async fn t_get(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let cluster = self.target(args)?.cluster; let service = args .get("service") .and_then(Value::as_str) @@ -271,12 +327,14 @@ impl OabMcp { } async fn t_states(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let t = self.target(args)?; + let cluster = t.cluster.clone(); let services: Vec = match args.get("service").and_then(Value::as_str) { Some(s) => vec![s.to_string()], None => scp::observe_services(&self.aws_for(&cluster).await, &cluster) .await? .into_iter() + .filter(|s| t.includes(&s.service_name, &s.name)) .map(|s| s.name) .collect(), }; @@ -299,7 +357,7 @@ impl OabMcp { } async fn t_events(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let cluster = self.target(args)?.cluster; let service = args.get("service").and_then(Value::as_str); let since_minutes = args .get("since_minutes") @@ -345,7 +403,7 @@ impl OabMcp { } async fn t_apply(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let cluster = self.target(args)?.cluster; let manifest = args .get("manifest_yaml") .and_then(Value::as_str) @@ -357,7 +415,8 @@ impl OabMcp { } async fn t_scale(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let t = self.target(args)?; + let cluster = t.cluster.clone(); let namespace = args .get("namespace") .and_then(Value::as_str) @@ -370,6 +429,13 @@ impl OabMcp { args.get("size") .and_then(Value::as_i64) .ok_or_else(|| anyhow::anyhow!("missing or invalid arg: size"))? as i32; + // Guard: a fleet handle only operates its own members — refuse to scale a + // service outside the named fleet (a no-op for unscoped or whole-cluster + // calls). Stops a fleet-scoped call from reaching a co-located non-member. + let service_name = format!("oab-{namespace}-{name}"); + if !t.includes(&service_name, name) { + anyhow::bail!("service {service_name:?} is not a member of the named fleet"); + } scp::scale_deployment( &self.aws_for(&cluster).await, &cluster, @@ -384,24 +450,28 @@ impl OabMcp { } async fn t_runtime_context(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let t = self.target(args)?; + let cluster = t.cluster.clone(); let aws = self.aws_for(&cluster).await; let ctx = scp::observe_identity(&aws).await?; - // Snapshot the governing binding under a short read-lock (no await held). - let (binding, expected) = { - let guard = self.bindings.read().unwrap(); - match guard.for_cluster(&cluster) { - Some(b) => ( - Some(json!({ - "name": b.name, - "profile": b.profile, - "region": b.region, - "expected_principal": b.expected_principal, - })), - b.expected_principal.clone(), - ), - None => (None, None), - } + // The governing binding: the **named fleet** when the call named one (so + // two fleets sharing a cluster resolve distinctly), else the first fleet + // bound to the cluster. Cloned so no lock is held across the await above. + let governing = match t.binding { + Some(b) => Some(b), + None => self.bindings.read().unwrap().for_cluster(&cluster).cloned(), + }; + let (binding, expected) = match governing { + Some(b) => ( + Some(json!({ + "name": b.name, + "profile": b.profile, + "region": b.region, + "expected_principal": b.expected_principal, + })), + b.expected_principal.clone(), + ), + None => (None, None), }; // Reconcile: expected (declared) vs actual (resolved). null when no // expectation is declared. Non-blocking — a warning signal, not a gate. @@ -485,7 +555,8 @@ impl OabMcp { } async fn t_delete(&self, args: &Map) -> Result { - let cluster = self.cluster(args); + let t = self.target(args)?; + let cluster = t.cluster.clone(); let resource = args .get("resource") .and_then(Value::as_str) @@ -498,6 +569,12 @@ impl OabMcp { .get("namespace") .and_then(Value::as_str) .unwrap_or("default"); + // Guard: a fleet handle only operates its own members (no-op unscoped / + // whole-cluster). + let service_name = format!("oab-{namespace}-{name}"); + if !t.includes(&service_name, name) { + anyhow::bail!("service {service_name:?} is not a member of the named fleet"); + } scp::delete_deployment( &self.aws_for(&cluster).await, resource, diff --git a/crates/studio-cp/src/lib.rs b/crates/studio-cp/src/lib.rs index 135512f..28952da 100644 --- a/crates/studio-cp/src/lib.rs +++ b/crates/studio-cp/src/lib.rs @@ -292,6 +292,21 @@ pub struct FleetBinding { pub expected_principal: Option, } +impl FleetBinding { + /// Whether a service belongs to this fleet, matched by **either** its full + /// ECS name (`oab-{ns}-{name}`) or its short agent name — mirroring + /// [`resolve_service`], which accepts both forms. An **empty** member list ⇒ + /// the fleet covers the whole cluster (legacy semantics), so everything + /// matches. + pub fn includes(&self, service_name: &str, short_name: &str) -> bool { + self.members.is_empty() + || self + .members + .iter() + .any(|m| m == service_name || m == short_name) + } +} + /// The body of a `[fleet.]` table — the fields of a [`FleetBinding`] minus /// `name`, which is the table key. #[derive(Debug, Clone, serde::Deserialize)] @@ -753,6 +768,35 @@ members = ["oab-prod-mira"] assert!(b.for_cluster("oab").is_some()); } + #[test] + fn binding_includes_matches_full_or_short_name_and_whole_cluster() { + let orca = FleetBinding { + name: "orca".into(), + cluster: "oab".into(), + members: vec!["oab-prod-orca".into()], + region: None, + profile: None, + expected_principal: None, + }; + // full ECS name and short agent name both match (mirrors resolve_service) + assert!(orca.includes("oab-prod-orca", "orca")); + // a co-located non-member is excluded — this is what keeps two fleets on + // one cluster distinct + assert!(!orca.includes("oab-prod-mira", "mira")); + + // empty members ⇒ whole cluster: everything matches (legacy semantics) + let whole = FleetBinding { + name: "prod".into(), + cluster: "oab".into(), + members: vec![], + region: None, + profile: None, + expected_principal: None, + }; + assert!(whole.includes("oab-prod-orca", "orca")); + assert!(whole.includes("anything", "at-all")); + } + #[test] fn empty_config_and_no_fleet_key_parse_to_empty() { assert!(toml::from_str::("").unwrap().fleets.is_empty());