Merge branch 'mraszyk/deprecate-controller-in-read-state' into 'master'

disallow path containing deprecated controller data in HTTP read_state interface

This MR makes requesting the path `/canister/<canister_id>/controller` disallowed by the HTTP handler serving the public `read_state` endpoint. The motivation is that the path `/canister/<canister_id>/controller` is long deprecated according to the Interface Specification and the metrics reveal that it is only being used marginally. 

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

rs/http_endpoints/public/src/metrics.rs

@@ -30,7 +30,6 @@ pub(crate) struct HttpHandlerMetrics {
     pub(crate) response_body_size_bytes: HistogramVec,
     pub(crate) connections_total: IntCounter,
     pub(crate) health_status_transitions_total: IntCounterVec,
-    pub(crate) read_state_canister_controller_total: IntCounter,
     connection_setup_duration: HistogramVec,
     connection_duration: HistogramVec,
 }
@@ -81,10 +80,6 @@ impl HttpHandlerMetrics {
                 "Number of health status state transitions",
                 &[LABEL_HEALTH_STATUS_BEFORE,LABEL_HEALTH_STATUS_AFTER]
             ),
-            read_state_canister_controller_total: metrics_registry.int_counter(
-                "replica_http_read_state_canister_controller_total",
-                "Number of /canister/<canister_id>/controller paths in read state requests"
-            ),
             connection_setup_duration: metrics_registry.histogram_vec(
                 "replica_http_connection_setup_duration_seconds",
                 "HTTP connection setup durations, by status and detail (protocol on status=\"success\", error type on status=\"error\").",

rs/http_endpoints/public/src/read_state.rs

@@ -186,7 +186,6 @@ impl Service<Request<Vec<u8>>> for ReadStateService {
                 &read_state.paths,
                 &targets,
                 effective_canister_id,
-                &metrics,
             ) {
                 return Ok(make_plaintext_response(status, message));
             }
@@ -239,7 +238,6 @@ fn verify_paths(
     paths: &[Path],
     targets: &CanisterIdSet,
     effective_canister_id: CanisterId,
-    metrics: &HttpHandlerMetrics,
 ) -> Result<(), HttpError> {
     let mut request_status_id: Option<MessageId> = None;
 
@@ -252,11 +250,6 @@ fn verify_paths(
     for path in paths {
         match path.as_slice() {
             [b"time"] => {}
-            [b"canister", canister_id, b"controller"] => {
-                let canister_id = parse_canister_id(canister_id)?;
-                verify_canister_ids(&canister_id, &effective_canister_id)?;
-                metrics.read_state_canister_controller_total.inc();
-            }
             [b"canister", canister_id, b"controllers" | b"module_hash"] => {
                 let canister_id = parse_canister_id(canister_id)?;
                 verify_canister_ids(&canister_id, &effective_canister_id)?;
@@ -399,13 +392,11 @@ fn can_read_canister_metadata(
 mod test {
     use crate::{
         common::test::{array, assert_cbor_ser_equal, bytes, int},
-        metrics::HttpHandlerMetrics,
         read_state::{can_read_canister_metadata, verify_paths},
         HttpError,
     };
     use hyper::StatusCode;
     use ic_crypto_tree_hash::{Digest, Label, MixedHashTree, Path};
-    use ic_metrics::MetricsRegistry;
     use ic_registry_subnet_type::SubnetType;
     use ic_replicated_state::{CanisterQueues, ReplicatedState, SystemMetadata};
     use ic_test_utilities::{
@@ -541,7 +532,6 @@ mod test {
                 &[Path::from(Label::from("time"))],
                 &CanisterIdSet::all(),
                 canister_test_id(1),
-                &HttpHandlerMetrics::new(&MetricsRegistry::default())
             ),
             Ok(())
         );
@@ -563,7 +553,6 @@ mod test {
                 ],
                 &CanisterIdSet::all(),
                 canister_test_id(1),
-                &HttpHandlerMetrics::new(&MetricsRegistry::default())
             ),
             Ok(())
         );
@@ -576,7 +565,6 @@ mod test {
             ],
             &CanisterIdSet::all(),
             canister_test_id(1),
-            &HttpHandlerMetrics::new(&MetricsRegistry::default())
         )
         .is_err());
     }