Skip to content

Commit 02b5b0d

Browse files
authored
fix(ecs): real DescribeServiceRevisions + opaque List pagination cursors (#2259)
2 parents f440587 + 86279c5 commit 02b5b0d

7 files changed

Lines changed: 229 additions & 35 deletions

File tree

crates/fakecloud-ecs/src/helpers.rs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,55 @@ pub(crate) fn opt_str<'a>(body: &'a Value, field: &str) -> Option<&'a str> {
7676
body.get(field).and_then(|v| v.as_str())
7777
}
7878

79+
/// Paginate a list of ARN-like strings with a key-based cursor instead of a
80+
/// numeric offset. The list is sorted for a stable order (HashMap/BTreeMap
81+
/// value order is not a contract callers can rely on), and the returned
82+
/// `nextToken` is an opaque base64 of the last-emitted key. Resuming finds the
83+
/// first key strictly greater than the cursor, so pages stay correct even when
84+
/// items are added or removed between calls — a numeric offset would skip or
85+
/// duplicate rows there. A non-empty token that doesn't base64-decode (a stale
86+
/// or corrupted cursor we never emitted) is treated as past the end and yields
87+
/// an empty page — restarting at the beginning would silently re-list page 1
88+
/// and risk an infinite pagination loop for the client.
89+
pub(crate) fn paginate_arns(
90+
mut arns: Vec<String>,
91+
next_token: &str,
92+
max_results: usize,
93+
) -> (Vec<String>, Option<String>) {
94+
use base64::Engine;
95+
arns.sort();
96+
arns.dedup();
97+
let cursor = if next_token.is_empty() {
98+
None
99+
} else {
100+
base64::engine::general_purpose::STANDARD
101+
.decode(next_token)
102+
.ok()
103+
.and_then(|b| String::from_utf8(b).ok())
104+
};
105+
let start = if next_token.is_empty() {
106+
0
107+
} else {
108+
match &cursor {
109+
Some(c) => arns
110+
.iter()
111+
.position(|a| a.as_str() > c.as_str())
112+
.unwrap_or(arns.len()),
113+
// Non-empty but undecodable token: a cursor we never emitted.
114+
// Treat as past the end (empty page) rather than restarting at 0.
115+
None => arns.len(),
116+
}
117+
};
118+
let end = (start + max_results).min(arns.len());
119+
let page = arns[start..end].to_vec();
120+
let next = if end < arns.len() {
121+
Some(base64::engine::general_purpose::STANDARD.encode(arns[end - 1].as_bytes()))
122+
} else {
123+
None
124+
};
125+
(page, next)
126+
}
127+
79128
/// Validate that an optional string field, if present, is one of the allowed
80129
/// enum values. Returns InvalidParameterException if the field is set to a
81130
/// value outside `allowed`. Absent or null fields are accepted.
@@ -1508,3 +1557,43 @@ pub(crate) fn task_protection_json(task: &Task) -> Value {
15081557
"expirationDate": p.and_then(|p| p.expiration).map(|e| e.timestamp()),
15091558
})
15101559
}
1560+
1561+
#[cfg(test)]
1562+
mod pagination_tests {
1563+
use super::paginate_arns;
1564+
1565+
#[test]
1566+
fn paginate_arns_key_cursor_is_stable_across_delete() {
1567+
let all: Vec<String> = (0..5).map(|i| format!("arn:{i}")).collect();
1568+
let (p1, next) = paginate_arns(all.clone(), "", 2);
1569+
assert_eq!(p1, vec!["arn:0", "arn:1"]);
1570+
let token = next.expect("nextToken after page 1");
1571+
1572+
// Delete the first item before fetching page 2. A numeric offset (2)
1573+
// would skip "arn:2"; the key cursor resumes correctly after "arn:1".
1574+
let after: Vec<String> = all.iter().filter(|a| *a != "arn:0").cloned().collect();
1575+
let (p2, next2) = paginate_arns(after, &token, 2);
1576+
assert_eq!(p2, vec!["arn:2", "arn:3"]);
1577+
assert!(next2.is_some());
1578+
}
1579+
1580+
#[test]
1581+
fn paginate_arns_sorts_and_dedups_and_tolerates_garbage_token() {
1582+
let dupes = vec![
1583+
"b".to_string(),
1584+
"a".to_string(),
1585+
"a".to_string(),
1586+
"c".to_string(),
1587+
];
1588+
let (page, _) = paginate_arns(dupes, "", 10);
1589+
assert_eq!(page, vec!["a", "b", "c"]);
1590+
// A non-empty, undecodable token is treated as past the end: empty
1591+
// page + no nextToken, never a silent restart at page 1.
1592+
let (page, next) = paginate_arns(vec!["a".into(), "b".into()], "!!!not-base64", 1);
1593+
assert!(page.is_empty());
1594+
assert!(next.is_none());
1595+
// An empty token still starts at the beginning.
1596+
let (page, _) = paginate_arns(vec!["a".into(), "b".into()], "", 1);
1597+
assert_eq!(page, vec!["a"]);
1598+
}
1599+
}

crates/fakecloud-ecs/src/service_clusters.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,7 @@ impl EcsService {
174174
.collect(),
175175
None => Vec::new(),
176176
};
177-
let start = next_token.parse::<usize>().unwrap_or(0).min(arns.len());
178-
let end = (start + max_results).min(arns.len());
179-
let page = arns[start..end].to_vec();
180-
let next = if end < arns.len() {
181-
Some(end.to_string())
182-
} else {
183-
None
184-
};
177+
let (page, next) = paginate_arns(arns, next_token, max_results);
185178
let mut out = json!({ "clusterArns": page });
186179
if let Some(n) = next {
187180
out.as_object_mut()

crates/fakecloud-ecs/src/service_container_instances_etc.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,12 @@ impl EcsService {
201201
None => Vec::new(),
202202
};
203203
arns.sort();
204-
let start = next_token.parse::<usize>().unwrap_or(0).min(arns.len());
205-
let end = (start + max_results).min(arns.len());
206-
let page = arns[start..end].to_vec();
204+
let (page, next) = paginate_arns(arns, next_token, max_results);
207205
let mut out = json!({"containerInstanceArns": page});
208-
if end < arns.len() {
206+
if let Some(n) = next {
209207
out.as_object_mut()
210208
.unwrap()
211-
.insert("nextToken".into(), json!(end.to_string()));
209+
.insert("nextToken".into(), json!(n));
212210
}
213211
Ok(AwsResponse::ok_json(out))
214212
}
@@ -1173,9 +1171,27 @@ impl EcsService {
11731171
.iter()
11741172
.filter_map(|v| v.as_str().map(String::from))
11751173
.collect();
1174+
let account = request.account_id.clone();
1175+
let accounts = self.state.read();
1176+
let state = accounts.get(&account);
1177+
let mut found = Vec::new();
1178+
let mut failures = Vec::new();
1179+
for r in &refs {
1180+
match state.and_then(|s| s.service_revisions.get(r)) {
1181+
Some(rev) => found.push(json!({
1182+
"serviceRevisionArn": rev.service_revision_arn,
1183+
"serviceArn": rev.service_arn,
1184+
"clusterArn": rev.cluster_arn,
1185+
"taskDefinition": rev.task_definition_arn,
1186+
"launchType": rev.launch_type,
1187+
"createdAt": rev.created_at.timestamp(),
1188+
})),
1189+
None => failures.push(json!({"arn": r, "reason": "MISSING"})),
1190+
}
1191+
}
11761192
Ok(AwsResponse::ok_json(json!({
1177-
"serviceRevisions": [],
1178-
"failures": refs.iter().map(|r| json!({"arn": r, "reason": "MISSING"})).collect::<Vec<_>>(),
1193+
"serviceRevisions": found,
1194+
"failures": failures,
11791195
})))
11801196
}
11811197
}

crates/fakecloud-ecs/src/service_services_resource.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ impl EcsService {
238238
volume_configurations: volume_configurations.clone(),
239239
};
240240
state.services.insert(key.clone(), service.clone());
241+
state.record_service_revision(&service);
241242
if let Some(cluster) = state.clusters.get_mut(&cluster_name) {
242243
cluster.active_services_count += 1;
243244
}
@@ -791,6 +792,14 @@ impl EcsService {
791792
}),
792793
});
793794

795+
// A task-definition change mints a new service revision so
796+
// DescribeServiceRevisions reflects the deployment history.
797+
if new_deployment_triggered {
798+
if let Some(svc) = state.services.get(&key).cloned() {
799+
state.record_service_revision(&svc);
800+
}
801+
}
802+
794803
let svc = state.services.get(&key).unwrap();
795804
let mut service_json = service_to_json(svc);
796805
if controller == "CODE_DEPLOY" {
@@ -1080,14 +1089,12 @@ impl EcsService {
10801089
None => Vec::new(),
10811090
};
10821091
arns.sort();
1083-
let start = next_token.parse::<usize>().unwrap_or(0).min(arns.len());
1084-
let end = (start + max_results).min(arns.len());
1085-
let page = arns[start..end].to_vec();
1092+
let (page, next) = paginate_arns(arns, next_token, max_results);
10861093
let mut out = json!({"serviceArns": page});
1087-
if end < arns.len() {
1094+
if let Some(n) = next {
10881095
out.as_object_mut()
10891096
.unwrap()
1090-
.insert("nextToken".into(), json!(end.to_string()));
1097+
.insert("nextToken".into(), json!(n));
10911098
}
10921099
Ok(AwsResponse::ok_json(out))
10931100
}

crates/fakecloud-ecs/src/service_task_definitions.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -343,14 +343,12 @@ impl EcsService {
343343
} else {
344344
arns.sort();
345345
}
346-
let start = next_token.parse::<usize>().unwrap_or(0).min(arns.len());
347-
let end = (start + max_results).min(arns.len());
348-
let page = arns[start..end].to_vec();
346+
let (page, next) = paginate_arns(arns, next_token, max_results);
349347
let mut out = json!({"taskDefinitionArns": page});
350-
if end < arns.len() {
348+
if let Some(n) = next {
351349
out.as_object_mut()
352350
.unwrap()
353-
.insert("nextToken".into(), json!(end.to_string()));
351+
.insert("nextToken".into(), json!(n));
354352
}
355353
Ok(AwsResponse::ok_json(out))
356354
}
@@ -395,14 +393,12 @@ impl EcsService {
395393
}
396394
}
397395
families.sort();
398-
let start = next_token.parse::<usize>().unwrap_or(0).min(families.len());
399-
let end = (start + max_results).min(families.len());
400-
let page = families[start..end].to_vec();
396+
let (page, next) = paginate_arns(families, next_token, max_results);
401397
let mut out = json!({"families": page});
402-
if end < families.len() {
398+
if let Some(n) = next {
403399
out.as_object_mut()
404400
.unwrap()
405-
.insert("nextToken".into(), json!(end.to_string()));
401+
.insert("nextToken".into(), json!(n));
406402
}
407403
Ok(AwsResponse::ok_json(out))
408404
}

crates/fakecloud-ecs/src/service_tasks.rs

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -563,14 +563,12 @@ impl EcsService {
563563
None => Vec::new(),
564564
};
565565
arns.sort();
566-
let start = next_token.parse::<usize>().unwrap_or(0).min(arns.len());
567-
let end = (start + max_results).min(arns.len());
568-
let page = arns[start..end].to_vec();
566+
let (page, next) = paginate_arns(arns, next_token, max_results);
569567
let mut out = json!({"taskArns": page});
570-
if end < arns.len() {
568+
if let Some(n) = next {
571569
out.as_object_mut()
572570
.unwrap()
573-
.insert("nextToken".into(), json!(end.to_string()));
571+
.insert("nextToken".into(), json!(n));
574572
}
575573
Ok(AwsResponse::ok_json(out))
576574
}
@@ -761,6 +759,55 @@ mod multi_container_tests {
761759
assert_eq!(body["failures"].as_array().unwrap().len(), 0);
762760
}
763761

762+
#[test]
763+
fn describe_service_revisions_returns_stored_revision_or_missing() {
764+
let svc = fresh_service();
765+
svc.register_task_definition(&make_request(
766+
"RegisterTaskDefinition",
767+
json!({"family": "web", "containerDefinitions": [{"name": "app", "image": "alpine"}]}),
768+
))
769+
.unwrap();
770+
let created = svc
771+
.create_service(&make_request(
772+
"CreateService",
773+
json!({"serviceName": "s", "taskDefinition": "web", "desiredCount": 0}),
774+
))
775+
.unwrap();
776+
let cbody: Value = serde_json::from_slice(created.body.expect_bytes()).unwrap();
777+
let service_arn = cbody["service"]["serviceArn"].as_str().unwrap().to_string();
778+
779+
// CreateService minted revision 1.
780+
let rev1 = format!("{service_arn}:1");
781+
let d = svc
782+
.describe_service_revisions(&make_request(
783+
"DescribeServiceRevisions",
784+
json!({"serviceRevisionArns": [rev1.clone()]}),
785+
))
786+
.unwrap();
787+
let dbody: Value = serde_json::from_slice(d.body.expect_bytes()).unwrap();
788+
assert_eq!(dbody["serviceRevisions"].as_array().unwrap().len(), 1);
789+
assert_eq!(
790+
dbody["serviceRevisions"][0]["serviceRevisionArn"],
791+
json!(rev1)
792+
);
793+
assert!(dbody["serviceRevisions"][0]["taskDefinition"]
794+
.as_str()
795+
.unwrap()
796+
.ends_with("web:1"));
797+
assert_eq!(dbody["failures"].as_array().unwrap().len(), 0);
798+
799+
// Unknown revision -> MISSING.
800+
let d2 = svc
801+
.describe_service_revisions(&make_request(
802+
"DescribeServiceRevisions",
803+
json!({"serviceRevisionArns": [format!("{service_arn}:99")]}),
804+
))
805+
.unwrap();
806+
let d2body: Value = serde_json::from_slice(d2.body.expect_bytes()).unwrap();
807+
assert_eq!(d2body["serviceRevisions"].as_array().unwrap().len(), 0);
808+
assert_eq!(d2body["failures"][0]["reason"], "MISSING");
809+
}
810+
764811
#[test]
765812
fn register_task_def_with_two_containers_then_run_task_starts_both() {
766813
let svc = fresh_service();

crates/fakecloud-ecs/src/state.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,25 @@ pub struct EcsState {
9494
/// with built-in load balancing and autoscaling.
9595
#[serde(default)]
9696
pub express_gateway_services: BTreeMap<String, ExpressGatewayService>,
97+
/// Service revisions keyed by `serviceRevisionArn` (`{service_arn}:{n}`).
98+
/// A revision is minted on CreateService and on each UpdateService that
99+
/// changes the task definition, so DescribeServiceRevisions can return the
100+
/// real configuration snapshot instead of an empty stub.
101+
#[serde(default)]
102+
pub service_revisions: BTreeMap<String, ServiceRevision>,
103+
}
104+
105+
/// A point-in-time snapshot of a service's configuration, addressable by
106+
/// `serviceRevisionArn`. Mirrors the subset of AWS's ServiceRevision that
107+
/// callers key on when auditing a deployment.
108+
#[derive(Clone, Debug, Serialize, Deserialize)]
109+
pub struct ServiceRevision {
110+
pub service_revision_arn: String,
111+
pub service_arn: String,
112+
pub cluster_arn: String,
113+
pub task_definition_arn: String,
114+
pub launch_type: String,
115+
pub created_at: DateTime<Utc>,
97116
}
98117

99118
impl EcsState {
@@ -118,6 +137,7 @@ impl EcsState {
118137
daemons: BTreeMap::new(),
119138
daemon_deployments: BTreeMap::new(),
120139
express_gateway_services: BTreeMap::new(),
140+
service_revisions: BTreeMap::new(),
121141
}
122142
}
123143

@@ -139,6 +159,32 @@ impl EcsState {
139159
self.daemons.clear();
140160
self.daemon_deployments.clear();
141161
self.express_gateway_services.clear();
162+
self.service_revisions.clear();
163+
}
164+
165+
/// Mint and store a new service revision for `service`, numbered by how
166+
/// many revisions already exist for that service (`{service_arn}:{n}`).
167+
/// Returns the new serviceRevisionArn.
168+
pub fn record_service_revision(&mut self, service: &Service) -> String {
169+
let n = self
170+
.service_revisions
171+
.values()
172+
.filter(|r| r.service_arn == service.service_arn)
173+
.count() as i32
174+
+ 1;
175+
let arn = format!("{}:{}", service.service_arn, n);
176+
self.service_revisions.insert(
177+
arn.clone(),
178+
ServiceRevision {
179+
service_revision_arn: arn.clone(),
180+
service_arn: service.service_arn.clone(),
181+
cluster_arn: service.cluster_arn.clone(),
182+
task_definition_arn: service.task_definition_arn.clone(),
183+
launch_type: service.launch_type.clone(),
184+
created_at: Utc::now(),
185+
},
186+
);
187+
arn
142188
}
143189

144190
/// Services are uniquely identified by `(cluster, name)` within an

0 commit comments

Comments
 (0)