Skip to content

Commit e449bec

Browse files
committed
fix(db): count IP-space subscriptions with COUNT(*), not SELECT ips.*
list_ip_range_subscriptions_by_space_paginated built its count query as 'SELECT ips.* ...' and ran it via query_scalar().fetch_one(). For any IP space with zero subscriptions that fetch_one hit an empty result set and returned RowNotFound, surfacing as a spurious 404 from the admin GET /ip_space/{id}/subscriptions endpoint (and a wrong count otherwise). Use 'SELECT COUNT(*)' so fetch_one always returns exactly one row. Also harden the e2e test: it previously read spaces.first() off the shared list, which was None on a fresh DB (a vacuous pass that hid the bug) and a TOCTOU race against other tests under parallel load. It now creates its own 198.51.100.0/24 space and queries that id. Full e2e suite: 115/115.
1 parent 6724818 commit e449bec

2 files changed

Lines changed: 52 additions & 24 deletions

File tree

lnvps_db/src/mysql.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,13 +2788,20 @@ impl LNVpsDbBase for LNVpsDbMysql {
27882788
extra.push_str(" AND ips.is_active = ?");
27892789
}
27902790

2791-
let base = "SELECT ips.* FROM ip_range_subscription ips \
2791+
// Shared FROM/JOIN/WHERE; the count selects COUNT(*) (NOT `ips.*`) so
2792+
// `fetch_one` always returns exactly one row — selecting `ips.*` and
2793+
// fetching one scalar yields `RowNotFound` (a spurious 404) whenever a
2794+
// space has zero subscriptions, and a wrong count otherwise.
2795+
let from = "FROM ip_range_subscription ips \
27922796
INNER JOIN subscription_line_item sli ON ips.subscription_line_item_id = sli.id \
27932797
INNER JOIN subscription s ON sli.subscription_id = s.id \
27942798
WHERE 1=1";
27952799

2796-
let count_sql = format!("{} {}", base, extra);
2797-
let data_sql = format!("{} {} ORDER BY ips.id DESC LIMIT ? OFFSET ?", base, extra);
2800+
let count_sql = format!("SELECT COUNT(*) {} {}", from, extra);
2801+
let data_sql = format!(
2802+
"SELECT ips.* {} {} ORDER BY ips.id DESC LIMIT ? OFFSET ?",
2803+
from, extra
2804+
);
27982805

27992806
let mut count_q = sqlx::query_scalar(&count_sql).bind(available_ip_space_id);
28002807
if let Some(u) = user_id {

lnvps_e2e/src/admin_api.rs

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,31 +1015,52 @@ mod tests {
10151015
#[tokio::test]
10161016
async fn test_admin_get_ip_space_with_pricing_and_subscriptions() {
10171017
let client = setup().await;
1018-
let resp = client.get_auth("/api/admin/v1/ip_space").await.unwrap();
1018+
1019+
// Create a dedicated IP space and query THAT id, rather than picking
1020+
// `spaces.first()` off the shared list: other tests create/delete IP
1021+
// spaces concurrently, so the first-listed id could be deleted between
1022+
// the sub-calls (a TOCTOU that made this test flaky under parallel
1023+
// load). A self-owned space with a unique CIDR is deterministic.
1024+
let resp = client.get_auth("/api/admin/v1/companies").await.unwrap();
10191025
let body: Value = serde_json::from_str(&resp.text().await.unwrap()).unwrap();
1020-
if let Some(spaces) = body["data"].as_array() {
1021-
if let Some(s) = spaces.first() {
1022-
let s_id = s["id"].as_u64().unwrap();
1026+
let company_id = body["data"]
1027+
.as_array()
1028+
.and_then(|a| a.first())
1029+
.and_then(|c| c["id"].as_u64())
1030+
.unwrap_or(1);
10231031

1024-
let resp = client
1025-
.get_auth(&format!("/api/admin/v1/ip_space/{s_id}"))
1026-
.await
1027-
.unwrap();
1028-
assert_eq!(resp.status(), StatusCode::OK);
1032+
let create_body = serde_json::json!({
1033+
"company_id": company_id,
1034+
"cidr": "198.51.100.0/24",
1035+
"min_prefix_size": 32,
1036+
"max_prefix_size": 24,
1037+
"registry": 1
1038+
});
1039+
let resp = client
1040+
.post_auth("/api/admin/v1/ip_space", &create_body)
1041+
.await
1042+
.unwrap();
1043+
assert_eq!(resp.status(), StatusCode::OK, "create IP space");
1044+
let body: Value = serde_json::from_str(&resp.text().await.unwrap()).unwrap();
1045+
let s_id = body["data"]["id"].as_u64().expect("created IP space id");
10291046

1030-
let resp = client
1031-
.get_auth(&format!("/api/admin/v1/ip_space/{s_id}/pricing"))
1032-
.await
1033-
.unwrap();
1034-
assert_eq!(resp.status(), StatusCode::OK);
1047+
let resp = client
1048+
.get_auth(&format!("/api/admin/v1/ip_space/{s_id}"))
1049+
.await
1050+
.unwrap();
1051+
assert_eq!(resp.status(), StatusCode::OK);
10351052

1036-
let resp = client
1037-
.get_auth(&format!("/api/admin/v1/ip_space/{s_id}/subscriptions"))
1038-
.await
1039-
.unwrap();
1040-
assert_eq!(resp.status(), StatusCode::OK);
1041-
}
1042-
}
1053+
let resp = client
1054+
.get_auth(&format!("/api/admin/v1/ip_space/{s_id}/pricing"))
1055+
.await
1056+
.unwrap();
1057+
assert_eq!(resp.status(), StatusCode::OK);
1058+
1059+
let resp = client
1060+
.get_auth(&format!("/api/admin/v1/ip_space/{s_id}/subscriptions"))
1061+
.await
1062+
.unwrap();
1063+
assert_eq!(resp.status(), StatusCode::OK);
10431064
}
10441065

10451066
// ========================================================================

0 commit comments

Comments
 (0)