Skip to content

Commit a8beefd

Browse files
mraszykeichhorl
andauthored
feat: enable listing canisters via query call with subnet ID in the URL (#9977)
This PR adds a new public HTTP endpoint `/api/v3/subnet/<effective_subnet_id>/query` on the replica that supports call to the `list_canisters` endpoint of the management canister (`aaaaa-aa`). It is specified in this [PR](dfinity/portal#6224). Note. PocketIC support will be added in a separate PR. --------- Co-authored-by: Leo Eichhorn <99166915+eichhorl@users.noreply.github.com>
1 parent a95a31e commit a8beefd

10 files changed

Lines changed: 294 additions & 46 deletions

File tree

rs/http_endpoints/public/src/call.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ impl IngressValidator {
261261
Err(HttpError {
262262
status: StatusCode::BAD_REQUEST,
263263
message: format!(
264-
"Specified CanisterId {} does not match effective canister id in URL {}",
264+
"Specified canister ID {} does not match effective canister ID in URL {}",
265265
msg.canister_id(),
266266
effective_canister_id
267267
),
@@ -273,7 +273,7 @@ impl IngressValidator {
273273
Err(HttpError {
274274
status: StatusCode::BAD_REQUEST,
275275
message: format!(
276-
"Specified SubnetId {} does not match the subnet id of this node {}",
276+
"Specified subnet ID {} does not match the subnet ID of this node {}",
277277
effective_subnet_id, subnet_id
278278
),
279279
})?;

rs/http_endpoints/public/src/lib.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ struct HttpHandler {
133133
subnet_call_v4_router: Router,
134134
query_v2_router: Router,
135135
query_v3_router: Router,
136+
subnet_query_v3_router: Router,
136137
catchup_router: Router,
137138
dashboard_router: Router,
138139
status_router: Router,
@@ -339,6 +340,7 @@ pub fn start_server(
339340
ingress_verifier.clone(),
340341
nns_delegation_reader.clone(),
341342
query_execution_service.clone(),
343+
subnet_id,
342344
version,
343345
)
344346
.with_health_status(health_status.clone())
@@ -348,6 +350,7 @@ pub fn start_server(
348350

349351
let query_v2_router = query_router(query::Version::V2);
350352
let query_v3_router = query_router(query::Version::V3);
353+
let subnet_query_v3_router = query_router(query::Version::SubnetV3);
351354

352355
let read_state_router = |version, target| {
353356
ReadStateServiceBuilder::builder(
@@ -417,6 +420,7 @@ pub fn start_server(
417420
subnet_call_v4_router,
418421
query_v2_router,
419422
query_v3_router,
423+
subnet_query_v3_router,
420424
status_router,
421425
catchup_router,
422426
dashboard_router,
@@ -609,6 +613,9 @@ fn make_router(
609613
.merge(http_handler.query_v3_router.layer(service_builder(
610614
GlobalConcurrencyLimitLayer::new(config.max_query_concurrent_requests),
611615
)))
616+
.merge(http_handler.subnet_query_v3_router.layer(service_builder(
617+
GlobalConcurrencyLimitLayer::new(config.max_query_concurrent_requests),
618+
)))
612619
.merge(
613620
http_handler
614621
.subnet_read_state_v2_router
@@ -816,6 +823,10 @@ pub(crate) mod tests {
816823
QueryService::route(query::Version::V3),
817824
axum::routing::post(dummy_cbor),
818825
),
826+
subnet_query_v3_router: Router::new().route(
827+
QueryService::route(query::Version::SubnetV3),
828+
axum::routing::post(dummy_cbor),
829+
),
819830
catchup_router: Router::new().route(
820831
CatchUpPackageService::route(),
821832
axum::routing::post(dummy_cbor),

rs/http_endpoints/public/src/query.rs

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Module that deals with requests to /api/v2/canister/.../query
1+
//! Module that deals with requests to /api/v{2,3}/canister/.../query and /api/v3/subnet/.../query
22
33
use crate::{
44
ReplicaHealthStatus,
@@ -29,7 +29,7 @@ use ic_logger::{ReplicaLogger, error};
2929
use ic_nns_delegation_manager::{CanisterRangesFilter, NNSDelegationReader};
3030
use ic_registry_client_helpers::crypto::root_of_trust::RegistryRootOfTrustProvider;
3131
use ic_types::{
32-
CanisterId, NodeId,
32+
CanisterId, NodeId, PrincipalId, SubnetId,
3333
crypto::threshold_sig::IcRootOfTrust,
3434
ingress::WasmResult,
3535
malicious_flags::MaliciousFlags,
@@ -53,6 +53,8 @@ pub enum Version {
5353
V2,
5454
// Endpoint with the NNS delegation using the tree format of the canister ranges.
5555
V3,
56+
// Subnet endpoint with no canister ranges in the NNS delegation.
57+
SubnetV3,
5658
}
5759

5860
#[derive(Clone)]
@@ -67,6 +69,7 @@ pub struct QueryService {
6769
registry_client: Arc<dyn RegistryClient>,
6870
additional_root_of_trust: Option<IcRootOfTrust>,
6971
query_execution_service: Arc<Mutex<QueryExecutionService>>,
72+
subnet_id: SubnetId,
7073
version: Version,
7174
}
7275

@@ -82,6 +85,7 @@ pub struct QueryServiceBuilder {
8285
registry_client: Arc<dyn RegistryClient>,
8386
additional_root_of_trust: Option<IcRootOfTrust>,
8487
query_execution_service: QueryExecutionService,
88+
subnet_id: SubnetId,
8589
version: Version,
8690
}
8791

@@ -90,6 +94,7 @@ impl QueryService {
9094
match version {
9195
Version::V2 => "/api/v2/canister/{effective_canister_id}/query",
9296
Version::V3 => "/api/v3/canister/{effective_canister_id}/query",
97+
Version::SubnetV3 => "/api/v3/subnet/{effective_subnet_id}/query",
9398
}
9499
}
95100
}
@@ -103,6 +108,7 @@ impl QueryServiceBuilder {
103108
ingress_verifier: Arc<dyn IngressSigVerifier>,
104109
nns_delegation_reader: NNSDelegationReader,
105110
query_execution_service: QueryExecutionService,
111+
subnet_id: SubnetId,
106112
version: Version,
107113
) -> Self {
108114
Self {
@@ -117,6 +123,7 @@ impl QueryServiceBuilder {
117123
registry_client,
118124
additional_root_of_trust: None,
119125
query_execution_service,
126+
subnet_id,
120127
version,
121128
}
122129
}
@@ -162,6 +169,7 @@ impl QueryServiceBuilder {
162169
registry_client: self.registry_client,
163170
additional_root_of_trust: self.additional_root_of_trust,
164171
query_execution_service: Arc::new(Mutex::new(self.query_execution_service)),
172+
subnet_id: self.subnet_id,
165173
version: self.version,
166174
};
167175
Router::new().route_service(
@@ -179,7 +187,7 @@ impl QueryServiceBuilder {
179187
}
180188

181189
pub(crate) async fn query(
182-
axum::extract::Path(effective_canister_id): axum::extract::Path<CanisterId>,
190+
axum::extract::Path(id): axum::extract::Path<PrincipalId>,
183191
State(QueryService {
184192
log,
185193
node_id,
@@ -191,6 +199,7 @@ pub(crate) async fn query(
191199
nns_delegation_reader,
192200
additional_root_of_trust,
193201
query_execution_service,
202+
subnet_id,
194203
version,
195204
}): State<QueryService>,
196205
WithTimeout(Cbor(request)): WithTimeout<Cbor<HttpRequestEnvelope<HttpQueryContent>>>,
@@ -217,12 +226,41 @@ pub(crate) async fn query(
217226
}
218227
};
219228
let canister_id = request.content().canister_id();
220-
if canister_id != CanisterId::ic_00() && canister_id != effective_canister_id {
221-
let status = StatusCode::BAD_REQUEST;
222-
let text = format!(
223-
"Specified CanisterId {canister_id} does not match effective canister id in URL {effective_canister_id}"
224-
);
225-
return (status, text).into_response();
229+
230+
// Validate effective destination.
231+
match version {
232+
Version::V2 | Version::V3 => {
233+
let effective_canister_id = CanisterId::unchecked_from_principal(id);
234+
if canister_id != CanisterId::ic_00() && canister_id != effective_canister_id {
235+
let status = StatusCode::BAD_REQUEST;
236+
let text = format!(
237+
"Specified canister ID {canister_id} does not match effective canister ID in URL {effective_canister_id}"
238+
);
239+
return (status, text).into_response();
240+
}
241+
}
242+
Version::SubnetV3 => {
243+
let effective_subnet_id = SubnetId::from(id);
244+
if effective_subnet_id != subnet_id {
245+
let status = StatusCode::BAD_REQUEST;
246+
let text = format!(
247+
"Specified subnet ID {effective_subnet_id} does not match the subnet ID of this node {subnet_id}"
248+
);
249+
return (status, text).into_response();
250+
}
251+
if canister_id != CanisterId::ic_00()
252+
|| request.content().method_name != "list_canisters"
253+
{
254+
let status = StatusCode::BAD_REQUEST;
255+
let text = format!(
256+
"Subnet query endpoint only accepts queries to the management canister ({}) 'list_canisters' method, got canister_id={} method_name='{}'",
257+
CanisterId::ic_00(),
258+
canister_id,
259+
request.content().method_name
260+
);
261+
return (status, text).into_response();
262+
}
263+
}
226264
}
227265

228266
let root_of_trust_provider = if let Some(additional_root_of_trust) = additional_root_of_trust {
@@ -263,8 +301,12 @@ pub(crate) async fn query(
263301
Version::V2 => {
264302
nns_delegation_reader.get_delegation_with_metadata(CanisterRangesFilter::Flat)
265303
}
266-
Version::V3 => nns_delegation_reader
267-
.get_delegation_with_metadata(CanisterRangesFilter::Tree(effective_canister_id)),
304+
Version::V3 => nns_delegation_reader.get_delegation_with_metadata(
305+
CanisterRangesFilter::Tree(CanisterId::unchecked_from_principal(id)),
306+
),
307+
Version::SubnetV3 => {
308+
nns_delegation_reader.get_delegation_with_metadata(CanisterRangesFilter::None)
309+
}
268310
};
269311
let query_execution_input = QueryExecutionInput {
270312
query: user_query.clone(),

rs/http_endpoints/public/tests/common/mod.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use ic_crypto_test_utils_crypto_returning_ok::CryptoReturningOk;
1313
use ic_crypto_tls_interfaces::TlsConfig;
1414
use ic_crypto_tls_interfaces_mocks::MockTlsConfig;
1515
use ic_crypto_tree_hash::{Digest, LabeledTree, MatchPatternPath, MixedHashTree, Witness};
16-
use ic_http_endpoints_public::start_server;
16+
use ic_http_endpoints_public::{query, start_server};
1717
use ic_interfaces::{
1818
consensus_pool::ConsensusPoolCache,
1919
execution_environment::{
@@ -48,7 +48,7 @@ use ic_replicated_state::{
4848
};
4949
use ic_test_utilities_types::ids::{node_test_id, subnet_test_id};
5050
use ic_types::{
51-
CanisterId, CryptoHashOfPartialState, Height, RegistryVersion,
51+
CanisterId, CryptoHashOfPartialState, Height, PrincipalId, RegistryVersion,
5252
artifact::UnvalidatedArtifactMutation,
5353
batch::RawQueryStats,
5454
consensus::certification::{Certification, CertificationContent},
@@ -117,6 +117,25 @@ impl UpdateEndpoint {
117117
}
118118
}
119119

120+
pub async fn query_endpoint(version: query::Version, addr: SocketAddr) -> reqwest::Response {
121+
match version {
122+
query::Version::V2 | query::Version::V3 => {
123+
ic_http_endpoints_test_agent::Query::new(
124+
PrincipalId::default(),
125+
PrincipalId::default(),
126+
version,
127+
)
128+
.query(addr)
129+
.await
130+
}
131+
query::Version::SubnetV3 => {
132+
ic_http_endpoints_test_agent::Query::new_subnet(subnet_test_id(1).get())
133+
.query(addr)
134+
.await
135+
}
136+
}
137+
}
138+
120139
fn setup_query_execution_mock() -> (QueryExecutionService, QueryExecutionHandle) {
121140
let (service, handle) = tower_test::mock::pair::<QueryExecutionInput, QueryExecutionResponse>();
122141

rs/http_endpoints/public/tests/load_shed_test.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,17 @@ pub mod common;
33
use crate::common::{
44
HttpEndpointBuilder, MockIngressPoolThrottler, UpdateEndpoint, default_certified_state_reader,
55
default_get_latest_state, default_read_certified_state, get_free_localhost_socket_addr,
6+
query_endpoint,
67
};
78
use async_trait::async_trait;
89
use axum::body::Body;
910
use hyper::{Method, Request, StatusCode};
1011
use hyper_util::{client::legacy::Client, rt::TokioExecutor};
1112
use ic_config::http_handler::Config;
1213
use ic_crypto_tree_hash::{Label, Path};
13-
use ic_http_endpoints_public::query;
14-
use ic_http_endpoints_public::read_state;
14+
use ic_http_endpoints_public::{query, read_state};
1515
use ic_http_endpoints_test_agent::{
16-
self, Call, CallSubnet, CanisterReadState, IngressMessage, Query, wait_for_status_healthy,
16+
self, Call, CallSubnet, CanisterReadState, IngressMessage, wait_for_status_healthy,
1717
};
1818
use ic_test_utilities_types::ids::subnet_test_id;
1919

@@ -35,7 +35,8 @@ use tokio::{runtime::Runtime, sync::Notify};
3535
/// we return 429.
3636
#[rstest]
3737
fn test_load_shedding_query(
38-
#[values(query::Version::V2, query::Version::V3)] version: query::Version,
38+
#[values(query::Version::V2, query::Version::V3, query::Version::SubnetV3)]
39+
version: query::Version,
3940
) {
4041
let rt = Runtime::new().unwrap();
4142
let addr = get_free_localhost_socket_addr();
@@ -58,9 +59,7 @@ fn test_load_shedding_query(
5859
let load_shedded_request = rt.spawn(async move {
5960
query_exec_running_clone.notified().await;
6061

61-
let response = Query::new(PrincipalId::default(), PrincipalId::default(), version)
62-
.query(addr)
63-
.await;
62+
let response = query_endpoint(version, addr).await;
6463

6564
load_shedder_returned_clone.notify_one();
6665

@@ -82,9 +81,7 @@ fn test_load_shedding_query(
8281
rt.block_on(async {
8382
wait_for_status_healthy(&addr).await.unwrap();
8483

85-
let response = Query::new(PrincipalId::default(), PrincipalId::default(), version)
86-
.query(addr)
87-
.await;
84+
let response = query_endpoint(version, addr).await;
8885

8986
assert_eq!(
9087
StatusCode::OK,

0 commit comments

Comments
 (0)