Skip to content

Commit 01a524c

Browse files
authored
fix(ecs): scope DescribeTasks to cluster, region from ARN, drain CI pending on run failure (#2257)
2 parents 5e28cc7 + ee9cfa3 commit 01a524c

1 file changed

Lines changed: 139 additions & 6 deletions

File tree

crates/fakecloud-ecs/src/service_tasks.rs

Lines changed: 139 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ use fakecloud_core::service::{AwsRequest, AwsResponse, AwsServiceError};
99

1010
use super::*;
1111

12+
/// Extract the region field (index 3) from a standard ARN
13+
/// (`arn:partition:service:region:account:resource`). Returns `None` when
14+
/// the string isn't an ARN or the region segment is empty.
15+
fn region_from_arn(arn: &str) -> Option<&str> {
16+
let parts: Vec<&str> = arn.splitn(6, ':').collect();
17+
if parts.len() >= 4 && parts[0] == "arn" && !parts[3].is_empty() {
18+
Some(parts[3])
19+
} else {
20+
None
21+
}
22+
}
23+
1224
impl EcsService {
1325
/// Spawn a task from a cross-service caller (EventBridge Scheduler /
1426
/// EventBridge Rules) without going through the AwsRequest dispatch
@@ -35,10 +47,17 @@ impl EcsService {
3547
});
3648
let body_bytes =
3749
Bytes::from(serde_json::to_vec(&body).map_err(|e| format!("encode body: {e}"))?);
50+
// Derive the region from the task-definition or cluster ARN (EventBridge
51+
// targets pass full ARNs) so the spawned task's ARNs carry the caller's
52+
// region instead of a hardcoded us-east-1.
53+
let region = region_from_arn(task_definition)
54+
.or_else(|| region_from_arn(cluster))
55+
.unwrap_or("us-east-1")
56+
.to_string();
3857
let req = AwsRequest {
3958
service: "ecs".into(),
4059
action: "RunTask".into(),
41-
region: "us-east-1".into(),
60+
region,
4261
account_id: account_id.to_string(),
4362
request_id: uuid::Uuid::new_v4().to_string(),
4463
headers: HeaderMap::new(),
@@ -356,7 +375,11 @@ impl EcsService {
356375
// block later DeleteCluster calls.
357376
let mut accounts = self.state.write();
358377
if let Some(state) = accounts.get_mut(&account) {
359-
let mut cluster_drains: Vec<String> = Vec::new();
378+
// (cluster_name, container_instance_arn) for each failed task so
379+
// BOTH pending counters that RunTask incremented get drained —
380+
// previously only the cluster counter was decremented, leaking
381+
// the container-instance pendingTasksCount for EC2/EXTERNAL tasks.
382+
let mut drains: Vec<(String, Option<String>)> = Vec::new();
360383
for id in &spawned_tasks {
361384
if let Some(t) = state.tasks.get_mut(id) {
362385
t.last_status = "STOPPED".into();
@@ -374,15 +397,26 @@ impl EcsService {
374397
for c in t.containers.iter_mut() {
375398
c.last_status = "STOPPED".into();
376399
}
377-
cluster_drains.push(t.cluster_name.clone());
400+
drains.push((t.cluster_name.clone(), t.container_instance_arn.clone()));
378401
}
379402
}
380-
for name in cluster_drains {
403+
for (name, ci_arn) in drains {
381404
if let Some(cluster) = state.clusters.get_mut(&name) {
382405
if cluster.pending_tasks_count > 0 {
383406
cluster.pending_tasks_count -= 1;
384407
}
385408
}
409+
if let Some(arn) = ci_arn {
410+
if let Some(ci) = state
411+
.container_instances
412+
.values_mut()
413+
.find(|ci| ci.container_instance_arn == arn)
414+
{
415+
if ci.pending_tasks_count > 0 {
416+
ci.pending_tasks_count -= 1;
417+
}
418+
}
419+
}
386420
}
387421
}
388422
}
@@ -453,6 +487,9 @@ impl EcsService {
453487
.and_then(|v| v.as_array())
454488
.map(|arr| arr.iter().any(|v| v.as_str() == Some("TAGS")))
455489
.unwrap_or(false);
490+
// AWS scopes DescribeTasks to the given cluster (default "default"):
491+
// a task that exists in another cluster is reported MISSING, not found.
492+
let cluster_name = EcsState::resolve_cluster_name(opt_str(&body, "cluster"));
456493

457494
let account = request.account_id.clone();
458495
let accounts = self.state.read();
@@ -467,7 +504,7 @@ impl EcsService {
467504
for input in &refs {
468505
let task_id = task_id_from_ref(input);
469506
match state.tasks.get(&task_id) {
470-
Some(t) => {
507+
Some(t) if t.cluster_name == cluster_name => {
471508
let mut v = task_to_json(t);
472509
if include_tags {
473510
v.as_object_mut()
@@ -476,7 +513,7 @@ impl EcsService {
476513
}
477514
found.push(v);
478515
}
479-
None => {
516+
_ => {
480517
failures.push(json!({
481518
"arn": input,
482519
"reason": "MISSING",
@@ -628,6 +665,102 @@ mod multi_container_tests {
628665
assert_eq!(err.code(), "ClusterNotFoundException");
629666
}
630667

668+
#[test]
669+
fn region_from_arn_extracts_region_or_none() {
670+
assert_eq!(
671+
region_from_arn("arn:aws:ecs:eu-west-1:000000000000:task-definition/web:3"),
672+
Some("eu-west-1")
673+
);
674+
assert_eq!(
675+
region_from_arn("arn:aws:ecs:ap-southeast-2:000000000000:cluster/prod"),
676+
Some("ap-southeast-2")
677+
);
678+
// Bare names (not ARNs) and empty-region ARNs yield None.
679+
assert_eq!(region_from_arn("web:3"), None);
680+
assert_eq!(
681+
region_from_arn("arn:aws:ecs::000000000000:cluster/prod"),
682+
None
683+
);
684+
}
685+
686+
fn insert_task(svc: &EcsService, id: &str, cluster: &str) {
687+
let state = svc.state.clone();
688+
let mut accounts = state.write();
689+
let acct = accounts.get_or_create("000000000000");
690+
let task = Task {
691+
task_arn: format!("arn:aws:ecs:us-east-1:000000000000:task/{cluster}/{id}"),
692+
task_id: id.into(),
693+
cluster_arn: format!("arn:aws:ecs:us-east-1:000000000000:cluster/{cluster}"),
694+
cluster_name: cluster.into(),
695+
task_definition_arn: "arn:aws:ecs:us-east-1:000000000000:task-definition/web:1".into(),
696+
family: "web".into(),
697+
revision: 1,
698+
container_instance_arn: None,
699+
capacity_provider_name: None,
700+
last_status: "RUNNING".into(),
701+
desired_status: "RUNNING".into(),
702+
launch_type: "FARGATE".into(),
703+
platform_version: None,
704+
cpu: None,
705+
memory: None,
706+
containers: Vec::new(),
707+
overrides: serde_json::json!({}),
708+
started_by: None,
709+
group: None,
710+
connectivity: "CONNECTED".into(),
711+
stop_code: None,
712+
stopped_reason: None,
713+
created_at: Utc::now(),
714+
started_at: None,
715+
stopping_at: None,
716+
stopped_at: None,
717+
pull_started_at: None,
718+
pull_stopped_at: None,
719+
connectivity_at: None,
720+
started_by_ref_id: None,
721+
execution_role_arn: None,
722+
task_role_arn: None,
723+
tags: Vec::new(),
724+
awslogs: None,
725+
captured_logs: String::new(),
726+
protection: None,
727+
enable_execute_command: false,
728+
attachments: Vec::new(),
729+
volume_configurations: Vec::new(),
730+
task_set_arn: None,
731+
};
732+
acct.tasks.insert(id.into(), task);
733+
}
734+
735+
#[test]
736+
fn describe_tasks_scopes_to_requested_cluster() {
737+
let svc = fresh_service();
738+
insert_task(&svc, "abc", "other");
739+
let task_arn = "arn:aws:ecs:us-east-1:000000000000:task/other/abc";
740+
741+
// Wrong cluster (default) -> MISSING, not found.
742+
let resp = svc
743+
.describe_tasks(&make_request(
744+
"DescribeTasks",
745+
json!({"tasks": [task_arn], "cluster": "default"}),
746+
))
747+
.unwrap();
748+
let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap();
749+
assert_eq!(body["tasks"].as_array().unwrap().len(), 0);
750+
assert_eq!(body["failures"][0]["reason"], "MISSING");
751+
752+
// Correct cluster -> found.
753+
let resp = svc
754+
.describe_tasks(&make_request(
755+
"DescribeTasks",
756+
json!({"tasks": [task_arn], "cluster": "other"}),
757+
))
758+
.unwrap();
759+
let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap();
760+
assert_eq!(body["tasks"].as_array().unwrap().len(), 1);
761+
assert_eq!(body["failures"].as_array().unwrap().len(), 0);
762+
}
763+
631764
#[test]
632765
fn register_task_def_with_two_containers_then_run_task_starts_both() {
633766
let svc = fresh_service();

0 commit comments

Comments
 (0)