Merge branch 'dimitris/subnet-read-state-service' into 'master'

feat: EXC-1469: Expose subnet metrics through the http api

This MR implements the new http endpoint to retrieve subnet specific information from the system state tree as specified in dfinity/interface-spec#191. As noted in the spec PR, the canister endpoint supports the existing subnet related paths for backward compatibility reasons. Eventually, these paths will only be accessible through the new subnet endpoint.

The key changes included:
1. Introducing a new `SubnetReadStateService` service modelled similarly to the existing `ReadStateService` which is now named `CanisterReadStateService`.
2. Moving the two services in a sub-module of read_state while keeping some common functions under `read_state.rs`.
3. The new service differs mainly in `verify_paths` and has some other small differences in `call` compared to the existing service due to certain checks that are not relevant.

There are some unit tests included to make sure `SubnetReadStateService` correctly responds only to subnet paths requested and end to end tests will be in a follow up MR. 

See merge request dfinity-lab/public/ic!15000

rs/http_endpoints/public/BUILD.bazel

@@ -22,6 +22,7 @@ DEPENDENCIES = [
     "//rs/registry/provisional_whitelist",
     "//rs/registry/subnet_type",
     "//rs/replicated_state",
+    "//rs/phantom_newtype",
     "//rs/types/error_types",
     "//rs/types/types",
     "//rs/validator",
@@ -75,6 +76,7 @@ DEV_DEPENDENCIES = [
     "@crate_index//:maplit",
     "@crate_index//:pretty_assertions",
     "@crate_index//:proptest",
+    "@crate_index//:serde_bytes",
     "@crate_index//:tower-test",
 ]
 

rs/http_endpoints/public/Cargo.toml

@@ -10,7 +10,7 @@ byte-unit = "4.0.14"
 crossbeam = "0.8.2"
 hex = "0.4.2"
 http = "0.2.5"
-futures ={ workspace = true }
+futures = { workspace = true }
 futures-util = "0.3.13"
 hyper = { version = "0.14.18", features = ["full"] }
 hyper-tls = "0.5.0"
@@ -35,6 +35,7 @@ ic-replicated-state = { path = "../../replicated_state" }
 ic-types = { path = "../../types/types" }
 ic-validator = { path = "../../validator" }
 native-tls = { version = "0.2.7", features = ["alpn"] }
+phantom_newtype = { path = "../../phantom_newtype" }
 prometheus = { version = "0.12.0", features = ["process"] }
 prost = { workspace = true }
 rand = "0.8.3"
@@ -70,6 +71,7 @@ mockall = "0.11.4"
 maplit = "1.0.2"
 pretty_assertions = "0.7.1"
 proptest = "1.0.0"
+serde_bytes = "0.11"
 tower-test = "0.4.0"
 
 [features]

rs/http_endpoints/public/src/call.rs

@@ -3,7 +3,7 @@
 use crate::{
     body::BodyReceiverLayer,
     common::{
-        get_cors_headers, make_plaintext_response, make_response, remove_effective_canister_id,
+        get_cors_headers, make_plaintext_response, make_response, remove_effective_principal_id,
     },
     metrics::LABEL_UNKNOWN,
     types::ApiReqType,
@@ -174,8 +174,8 @@ impl Service<Request<Vec<u8>>> for CallService {
             }
         };
 
-        let effective_canister_id = match remove_effective_canister_id(&mut parts) {
-            Ok(canister_id) => canister_id,
+        let effective_principal_id = match remove_effective_principal_id(&mut parts) {
+            Ok(principal_id) => principal_id,
             Err(res) => {
                 error!(
                     self.log,
@@ -185,6 +185,17 @@ impl Service<Request<Vec<u8>>> for CallService {
             }
         };
 
+        let effective_canister_id = match CanisterId::new(effective_principal_id) {
+            Ok(canister_id) => canister_id,
+            Err(_) => {
+                let res = make_plaintext_response(
+                    StatusCode::BAD_REQUEST,
+                    format!("Invalid canister id: {}", effective_principal_id),
+                );
+                return Box::pin(async move { Ok(res) });
+            }
+        };
+
         // Reject requests where `canister_id` != `effective_canister_id` for non mgmt canister calls.
         // This needs to be enforced because boundary nodes block access based on the `effective_canister_id`
         // in the url and the replica processes the request based on the `canister_id`.

rs/http_endpoints/public/src/common.rs

@@ -8,9 +8,9 @@ use ic_interfaces_registry::RegistryClient;
 use ic_logger::{info, warn, ReplicaLogger};
 use ic_registry_client_helpers::crypto::CryptoRegistry;
 use ic_replicated_state::ReplicatedState;
-use ic_types::CanisterId;
 use ic_types::{
-    crypto::threshold_sig::ThresholdSigPublicKey, messages::MessageId, RegistryVersion, SubnetId,
+    crypto::threshold_sig::ThresholdSigPublicKey, messages::MessageId, PrincipalId,
+    RegistryVersion, SubnetId,
 };
 use ic_validator::RequestValidationError;
 use serde::Serialize;
@@ -213,14 +213,14 @@ pub(crate) async fn get_latest_certified_state(
 /// Remove the effective canister id from the request parts.
 /// The effective canister id is added to the request during routing by looking at the url.
 /// Returns an BAD_REQUEST response if the effective canister id is not found in the request parts.
-pub(crate) fn remove_effective_canister_id(
+pub(crate) fn remove_effective_principal_id(
     parts: &mut Parts,
-) -> Result<CanisterId, Response<Body>> {
-    match parts.extensions.remove::<CanisterId>() {
-        Some(canister_id) => Ok(canister_id),
+) -> Result<PrincipalId, Response<Body>> {
+    match parts.extensions.remove::<PrincipalId>() {
+        Some(principal_id) => Ok(principal_id),
         _ => Err(make_plaintext_response(
             StatusCode::INTERNAL_SERVER_ERROR,
-            "Failed to get effective canister id from request. This is a bug.".to_string(),
+            "Failed to get effective principal id from request. This is a bug.".to_string(),
         )),
     }
 }

rs/http_endpoints/public/src/lib.rs

@@ -36,7 +36,7 @@ use crate::{
     },
     pprof::{PprofFlamegraphService, PprofHomeService, PprofProfileService},
     query::QueryService,
-    read_state::ReadStateService,
+    read_state::{canister::CanisterReadStateService, subnet::SubnetReadStateService},
     state_reader_executor::StateReaderExecutor,
     status::StatusService,
     types::*,
@@ -80,7 +80,7 @@ use ic_types::{
         HttpReadStateResponse, HttpRequestEnvelope, QueryResponseHash, ReplicaHealthStatus,
     },
     time::expiry_time_from_now,
-    CanisterId, NodeId, SubnetId,
+    NodeId, PrincipalId, SubnetId,
 };
 use metrics::{HttpHandlerMetrics, LABEL_UNKNOWN};
 use rand::Rng;
@@ -123,7 +123,8 @@ struct HttpHandler {
     catchup_service: EndpointService,
     dashboard_service: EndpointService,
     status_service: EndpointService,
-    read_state_service: EndpointService,
+    canister_read_state_service: EndpointService,
+    subnet_read_state_service: EndpointService,
     pprof_home_service: EndpointService,
     pprof_profile_service: EndpointService,
     pprof_flamegraph_service: EndpointService,
@@ -312,7 +313,7 @@ pub fn start_server(
         Arc::clone(&registry_client),
         query_execution_service,
     );
-    let read_state_service = ReadStateService::new_service(
+    let canister_read_state_service = CanisterReadStateService::new_service(
         config.clone(),
         log.clone(),
         metrics.clone(),
@@ -327,6 +328,14 @@ pub fn start_server(
         ),
         Arc::clone(&registry_client),
     );
+    let subnet_read_state_service = SubnetReadStateService::new_service(
+        config.clone(),
+        log.clone(),
+        metrics.clone(),
+        Arc::clone(&health_status),
+        Arc::clone(&delegation_from_nns),
+        state_reader_executor.clone(),
+    );
     let status_service = StatusService::new_service(
         config.clone(),
         log.clone(),
@@ -380,7 +389,8 @@ pub fn start_server(
         status_service,
         catchup_service,
         dashboard_service,
-        read_state_service,
+        canister_read_state_service,
+        subnet_read_state_service,
         pprof_home_service,
         pprof_profile_service,
         pprof_flamegraph_service,
@@ -647,7 +657,8 @@ async fn make_router(
     let status_service = http_handler.status_service.clone();
     let catch_up_package_service = http_handler.catchup_service.clone();
     let dashboard_service = http_handler.dashboard_service.clone();
-    let read_state_service = http_handler.read_state_service.clone();
+    let canister_read_state_service = http_handler.canister_read_state_service.clone();
+    let subnet_read_state_service = http_handler.subnet_read_state_service.clone();
     let pprof_home_service = http_handler.pprof_home_service.clone();
     let pprof_profile_service = http_handler.pprof_profile_service.clone();
     let pprof_flamegraph_service = http_handler.pprof_flamegraph_service.clone();
@@ -678,19 +689,47 @@ async fn make_router(
 
             // Check the path
             let path = req.uri().path();
-            let (svc, effective_canister_id) =
+            let (svc, effective_principal_id) =
                 match *path.split('/').collect::<Vec<&str>>().as_slice() {
                     ["", "api", "v2", "canister", effective_canister_id, "call"] => {
                         timer.set_label(LABEL_REQUEST_TYPE, ApiReqType::Call.into());
-                        (call_service, Some(effective_canister_id))
+                        (
+                            call_service,
+                            Some(
+                                PrincipalId::from_str(effective_canister_id)
+                                    .map_err(|err| (effective_canister_id, err.to_string())),
+                            ),
+                        )
                     }
                     ["", "api", "v2", "canister", effective_canister_id, "query"] => {
                         timer.set_label(LABEL_REQUEST_TYPE, ApiReqType::Query.into());
-                        (query_service, Some(effective_canister_id))
+                        (
+                            query_service,
+                            Some(
+                                PrincipalId::from_str(effective_canister_id)
+                                    .map_err(|err| (effective_canister_id, err.to_string())),
+                            ),
+                        )
                     }
                     ["", "api", "v2", "canister", effective_canister_id, "read_state"] => {
                         timer.set_label(LABEL_REQUEST_TYPE, ApiReqType::ReadState.into());
-                        (read_state_service, Some(effective_canister_id))
+                        (
+                            canister_read_state_service,
+                            Some(
+                                PrincipalId::from_str(effective_canister_id)
+                                    .map_err(|err| (effective_canister_id, err.to_string())),
+                            ),
+                        )
+                    }
+                    ["", "api", "v2", "subnet", subnet_id, "read_state"] => {
+                        timer.set_label(LABEL_REQUEST_TYPE, ApiReqType::ReadState.into());
+                        (
+                            subnet_read_state_service,
+                            Some(
+                                PrincipalId::from_str(subnet_id)
+                                    .map_err(|err| (subnet_id, err.to_string())),
+                            ),
+                        )
                     }
                     ["", "_", "catch_up_package"] => {
                         timer.set_label(LABEL_REQUEST_TYPE, ApiReqType::CatchUpPackage.into());
@@ -708,19 +747,19 @@ async fn make_router(
                     }
                 };
 
-            // If url contains effective canister id we attach it to the request.
-            if let Some(effective_canister_id) = effective_canister_id {
-                match CanisterId::from_str(effective_canister_id) {
-                    Ok(effective_canister_id) => {
-                        req.extensions_mut().insert(effective_canister_id);
+            // If url contains effective principal id we attach it to the request.
+            if let Some(effective_principal_id) = effective_principal_id {
+                match effective_principal_id {
+                    Ok(id) => {
+                        req.extensions_mut().insert(id);
                     }
-                    Err(e) => {
+                    Err((id, e)) => {
                         return (
                             make_plaintext_response(
                                 StatusCode::BAD_REQUEST,
                                 format!(
-                                    "Malformed request: Invalid efffective canister id {}: {}",
-                                    effective_canister_id, e
+                                    "Malformed request: Invalid efffective principal id {}: {}",
+                                    id, e
                                 ),
                             ),
                             timer,

rs/http_endpoints/public/src/query.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     body::BodyReceiverLayer,
-    common::{cbor_response, make_plaintext_response, remove_effective_canister_id},
+    common::{cbor_response, make_plaintext_response, remove_effective_principal_id},
     metrics::LABEL_UNKNOWN,
     types::ApiReqType,
     validator_executor::ValidatorExecutor,
@@ -25,7 +25,7 @@ use ic_types::{
         HttpRequestEnvelope, HttpSignedQueryResponse, NodeSignature, QueryResponseHash,
         SignedRequestBytes, UserQuery,
     },
-    NodeId,
+    CanisterId, NodeId,
 };
 use std::convert::{Infallible, TryFrom};
 use std::future::Future;
@@ -140,12 +140,23 @@ impl Service<Request<Vec<u8>>> for QueryService {
             }
         };
 
-        let effective_canister_id = match remove_effective_canister_id(&mut parts) {
-            Ok(canister_id) => canister_id,
+        let effective_principal_id = match remove_effective_principal_id(&mut parts) {
+            Ok(principal_id) => principal_id,
             Err(res) => {
                 error!(
                     self.log,
-                    "Effective canister ID is not attached to query request. This is a bug."
+                    "Effective canister ID is not attached to call request. This is a bug."
+                );
+                return Box::pin(async move { Ok(res) });
+            }
+        };
+
+        let effective_canister_id = match CanisterId::new(effective_principal_id) {
+            Ok(canister_id) => canister_id,
+            Err(_) => {
+                let res = make_plaintext_response(
+                    StatusCode::BAD_REQUEST,
+                    format!("Invalid canister id: {}", effective_principal_id),
                 );
                 return Box::pin(async move { Ok(res) });
             }