Skip to content

Commit a51d96b

Browse files
fix: isolate downstream subscription registrations
Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
1 parent 9584ec3 commit a51d96b

3 files changed

Lines changed: 114 additions & 20 deletions

File tree

crates/contextforge-data-plane-lib/src/gateway/downstream_subscriptions.rs

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use std::{
22
collections::HashMap,
3-
sync::{Arc, Mutex},
3+
sync::{
4+
Arc, Mutex,
5+
atomic::{AtomicU64, Ordering},
6+
},
47
};
58

69
use rmcp::{
@@ -13,6 +16,7 @@ use crate::layers::request_context::GatewayRequestContext;
1316
#[derive(Clone, Default)]
1417
pub(crate) struct DownstreamSubscriptionRegistry {
1518
inner: Arc<Mutex<HashMap<DownstreamSubscriptionKey, SubscriptionSink>>>,
19+
next_registration_id: Arc<AtomicU64>,
1620
}
1721

1822
impl DownstreamSubscriptionRegistry {
@@ -22,7 +26,8 @@ impl DownstreamSubscriptionRegistry {
2226
filter: &SubscriptionFilter,
2327
sink: &SubscriptionSink,
2428
) -> DownstreamSubscriptionGuard {
25-
let keys = subscription_keys(context, filter, sink.id());
29+
let registration_id = self.next_registration_id.fetch_add(1, Ordering::Relaxed);
30+
let keys = subscription_keys(context, filter, sink.id(), registration_id);
2631
let mut subscriptions = self.inner.lock().expect("downstream subscription registry lock poisoned");
2732
for key in &keys {
2833
subscriptions.insert(key.clone(), sink.clone());
@@ -54,6 +59,7 @@ pub(crate) struct DownstreamSubscriptionKey {
5459
principal: String,
5560
virtual_host_id: String,
5661
subscription_id: RequestId,
62+
registration_id: u64,
5763
notification: DownstreamSubscriptionNotification,
5864
}
5965

@@ -69,22 +75,39 @@ pub(super) fn subscription_keys(
6975
context: &GatewayRequestContext,
7076
filter: &SubscriptionFilter,
7177
subscription_id: &RequestId,
78+
registration_id: u64,
7279
) -> Vec<DownstreamSubscriptionKey> {
7380
let mut keys = Vec::new();
7481
if filter.tools_list_changed == Some(true) {
75-
keys.push(subscription_key(context, subscription_id, DownstreamSubscriptionNotification::ToolsListChanged));
82+
keys.push(subscription_key(
83+
context,
84+
subscription_id,
85+
registration_id,
86+
DownstreamSubscriptionNotification::ToolsListChanged,
87+
));
7688
}
7789
if filter.prompts_list_changed == Some(true) {
78-
keys.push(subscription_key(context, subscription_id, DownstreamSubscriptionNotification::PromptsListChanged));
90+
keys.push(subscription_key(
91+
context,
92+
subscription_id,
93+
registration_id,
94+
DownstreamSubscriptionNotification::PromptsListChanged,
95+
));
7996
}
8097
if filter.resources_list_changed == Some(true) {
81-
keys.push(subscription_key(context, subscription_id, DownstreamSubscriptionNotification::ResourcesListChanged));
98+
keys.push(subscription_key(
99+
context,
100+
subscription_id,
101+
registration_id,
102+
DownstreamSubscriptionNotification::ResourcesListChanged,
103+
));
82104
}
83105
if let Some(uris) = &filter.resource_subscriptions {
84106
keys.extend(uris.iter().map(|uri| {
85107
subscription_key(
86108
context,
87109
subscription_id,
110+
registration_id,
88111
DownstreamSubscriptionNotification::ResourceUpdated { uri: uri.clone() },
89112
)
90113
}));
@@ -95,12 +118,91 @@ pub(super) fn subscription_keys(
95118
fn subscription_key(
96119
context: &GatewayRequestContext,
97120
subscription_id: &RequestId,
121+
registration_id: u64,
98122
notification: DownstreamSubscriptionNotification,
99123
) -> DownstreamSubscriptionKey {
100124
DownstreamSubscriptionKey {
101125
principal: context.principal().to_owned(),
102126
virtual_host_id: context.virtual_host_id().to_owned(),
103127
subscription_id: subscription_id.clone(),
128+
registration_id,
104129
notification,
105130
}
106131
}
132+
133+
#[cfg(test)]
134+
mod tests {
135+
use std::collections::HashMap;
136+
137+
use contextforge_data_plane_apis::user_store::{BackendMCPGateway, Transport, VirtualHost};
138+
use rmcp::model::RequestId;
139+
140+
use super::*;
141+
142+
#[test]
143+
fn keys_include_subscription_id_and_notification_kind() {
144+
let gateway_context = GatewayRequestContext::new(&test_claims(), &test_virtual_host_id(), &test_virtual_host());
145+
let filter = SubscriptionFilter::builder().tools_list_changed().resource_subscription("memo://known").build();
146+
147+
let keys = subscription_keys(&gateway_context, &filter, &RequestId::Number(7), 9);
148+
149+
assert_eq!(2, keys.len());
150+
assert!(keys.iter().all(|key| key.subscription_id == RequestId::Number(7)));
151+
assert!(keys.iter().all(|key| key.registration_id == 9));
152+
}
153+
154+
#[test]
155+
fn registration_id_is_part_of_key_identity() {
156+
let gateway_context = GatewayRequestContext::new(&test_claims(), &test_virtual_host_id(), &test_virtual_host());
157+
let filter = SubscriptionFilter::builder().tools_list_changed().build();
158+
159+
let first = subscription_keys(&gateway_context, &filter, &RequestId::Number(7), 0);
160+
let second = subscription_keys(&gateway_context, &filter, &RequestId::Number(7), 1);
161+
162+
assert_ne!(first, second);
163+
}
164+
165+
fn test_virtual_host() -> VirtualHost {
166+
VirtualHost {
167+
backends: HashMap::from([(
168+
"backend-one".to_owned(),
169+
BackendMCPGateway {
170+
name: "backend-one".to_owned(),
171+
url: "http://127.0.0.1:9999/mcp".parse().expect("valid URL"),
172+
transport: Transport::default(),
173+
passthrough_headers: Vec::new(),
174+
add_headers: HashMap::new(),
175+
remove_headers: Vec::new(),
176+
allowed_tool_names: Vec::new(),
177+
tool_name_aliases: HashMap::new(),
178+
allowed_resource_names: Vec::new(),
179+
allowed_prompt_names: Vec::new(),
180+
},
181+
)]),
182+
}
183+
}
184+
185+
fn test_claims() -> crate::common::ContextForgeClaims {
186+
crate::common::ContextForgeClaims {
187+
sub: "test-principal".to_owned(),
188+
jti: "test-jti".to_owned(),
189+
token_use: None,
190+
iat: None,
191+
iss: "test-issuer".to_owned(),
192+
aud: "test-audience".to_owned(),
193+
exp: 1,
194+
teams: None,
195+
user: crate::common::User::builder()
196+
.email("test@example.com".to_owned())
197+
.full_name(None)
198+
.is_admin(false)
199+
.auth_provider("test".to_owned())
200+
.build(),
201+
scopes: None,
202+
}
203+
}
204+
205+
fn test_virtual_host_id() -> crate::layers::virtual_host_id::VirtualHostId {
206+
crate::layers::virtual_host_id::VirtualHostId::new("test-vhost".to_owned())
207+
}
208+
}

crates/contextforge-data-plane-lib/src/gateway/mcp_service.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,12 @@ mod tests {
376376
let virtual_host = virtual_host_with_backends(&["backend-one"]);
377377
let gateway_context = GatewayRequestContext::new(&test_claims(), &test_virtual_host_id(), &virtual_host);
378378
let filter = SubscriptionFilter::builder().tools_list_changed().resource_subscription("memo://known").build();
379-
let keys =
380-
super::super::downstream_subscriptions::subscription_keys(&gateway_context, &filter, &RequestId::Number(7));
379+
let keys = super::super::downstream_subscriptions::subscription_keys(
380+
&gateway_context,
381+
&filter,
382+
&RequestId::Number(7),
383+
0,
384+
);
381385

382386
assert_eq!(2, keys.len());
383387
}

crates/contextforge-data-plane-lib/src/layers/virtual_host_config.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,26 +31,14 @@ pub async fn virtual_host_config_layer(request: http::Request<axum::body::Body>,
3131
let gateway_context = GatewayRequestContext::new(claims, virtual_host_id, virtual_host);
3232
return scope_gateway_request_context(gateway_context, next.run(request)).await;
3333
}
34-
}
3534

36-
if let (Some(virtual_host_id), Some(user_config)) = (virtual_host_id, user_config)
37-
&& !has_virtual_host(user_config, virtual_host_id)
38-
{
3935
let virtual_host_id = virtual_host_id.value();
40-
let virtual_hosts = user_config.virtual_hosts.len();
41-
debug!(
42-
"virtual_host_config_layer - virtual host config missing virtual_host_id = {virtual_host_id} virtual_hosts = {virtual_hosts}"
43-
);
44-
return server_not_found_response();
36+
debug!("virtual_host_config_layer - claims missing virtual_host_id = {virtual_host_id}");
4537
}
4638

4739
next.run(request).await
4840
}
4941

50-
fn has_virtual_host(user_config: &UserConfig, virtual_host_id: &VirtualHostId) -> bool {
51-
user_config.virtual_hosts.contains_key(virtual_host_id.value())
52-
}
53-
5442
fn server_not_found_response() -> Response {
5543
Response::builder()
5644
.status(StatusCode::NOT_FOUND)

0 commit comments

Comments
 (0)