From b758224c6d815277cc3b3d3af8831f35bc175734 Mon Sep 17 00:00:00 2001 From: yoavGrs <97383386+yoavGrs@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:56:44 +0300 Subject: [PATCH] feat(monitoring): two config representation types (#1168) --- .github/workflows/ci.yml | 2 +- Cargo.lock | 1 + config/default_config.json | 5 ++ crates/papyrus_monitoring_gateway/Cargo.toml | 1 + .../src/gateway_test.rs | 42 +++++++++-- crates/papyrus_monitoring_gateway/src/lib.rs | 71 +++++++++++++++---- .../bin/central_source_integration_test.rs | 1 + crates/papyrus_node/src/config/config_test.rs | 33 ++++++++- ...fig__config_test__dump_default_config.snap | 5 ++ crates/papyrus_node/src/main.rs | 1 + 10 files changed, 139 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55d6a1e8b6..777adff600 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,7 +51,7 @@ jobs: - name: Run executable run: > - target/release/papyrus_node --base_layer.node_url ${{ secrets.CI_BASE_LAYER_NODE_URL }} + target/release/papyrus_node --base_layer.node_url ${{ secrets.CI_BASE_LAYER_NODE_URL }} --monitoring_gateway.config_representation_secret qwerty & sleep 30 ; kill $! test: diff --git a/Cargo.lock b/Cargo.lock index 02f782f1a3..3e153095c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4411,6 +4411,7 @@ dependencies = [ "tokio", "tower", "tracing", + "validator", ] [[package]] diff --git a/config/default_config.json b/config/default_config.json index 527dca8eb2..fb489f698d 100644 --- a/config/default_config.json +++ b/config/default_config.json @@ -69,6 +69,11 @@ "pointer_target": "collect_metrics", "privacy": "Public" }, + "monitoring_gateway.config_representation_secret": { + "description": "A secret for representing the full general config.", + "privacy": "Private", + "required_type": "String" + }, "monitoring_gateway.server_address": { "description": "node's monitoring server.", "privacy": "Public", diff --git a/crates/papyrus_monitoring_gateway/Cargo.toml b/crates/papyrus_monitoring_gateway/Cargo.toml index 019bbe7635..6686e4a814 100644 --- a/crates/papyrus_monitoring_gateway/Cargo.toml +++ b/crates/papyrus_monitoring_gateway/Cargo.toml @@ -18,6 +18,7 @@ serde_json = { workspace = true, features = ["arbitrary_precision"]} thiserror.workspace = true tokio = { workspace = true, features = ["full", "sync"] } tracing.workspace = true +validator = { workspace = true, features = ["derive"] } [dev-dependencies] http-body = { version = "0.4.5" } diff --git a/crates/papyrus_monitoring_gateway/src/gateway_test.rs b/crates/papyrus_monitoring_gateway/src/gateway_test.rs index 2a58a6ca9a..1d5550eddd 100644 --- a/crates/papyrus_monitoring_gateway/src/gateway_test.rs +++ b/crates/papyrus_monitoring_gateway/src/gateway_test.rs @@ -14,7 +14,9 @@ use tower::ServiceExt; use crate::{app, MONITORING_PREFIX}; -const TEST_CONFIG_REPRESENTATION: &str = "general_config_representation"; +const TEST_CONFIG_REPRESENTATION: &str = "full_general_config_representation"; +const PUBLIC_TEST_CONFIG_REPRESENTATION: &str = "public_general_config_representation"; +const SECRET: &str = "abcd"; const TEST_VERSION: &str = "1.2.3-dev"; // TODO(dan): consider using a proper fixture. @@ -24,6 +26,8 @@ fn setup_app() -> Router { storage_reader, TEST_VERSION, serde_json::to_value(TEST_CONFIG_REPRESENTATION).unwrap(), + serde_json::to_value(PUBLIC_TEST_CONFIG_REPRESENTATION).unwrap(), + SECRET.to_string(), None, ) } @@ -67,16 +71,34 @@ async fn version() { assert_eq!(&body[..], TEST_VERSION.as_bytes()); } -#[tokio::test] -async fn node_config() { +async fn validate_response(request: &str, expected_response: &str) { let app = setup_app(); - let response = request_app(app, "nodeConfig").await; + let response = request_app(app, request).await; assert_eq!(response.status(), StatusCode::OK); let body = hyper::body::to_bytes(response.into_body()).await.unwrap(); let body: Value = serde_json::from_slice(&body).unwrap(); - assert_eq!(body, json!(TEST_CONFIG_REPRESENTATION)); + assert_eq!(body, json!(expected_response)); +} + +#[tokio::test] +async fn public_node_config() { + validate_response("nodeConfig", PUBLIC_TEST_CONFIG_REPRESENTATION).await; +} + +#[tokio::test] +async fn node_config_valid_secret() { + validate_response(format!("nodeConfigFull/{SECRET}").as_str(), TEST_CONFIG_REPRESENTATION) + .await; +} + +#[tokio::test] +async fn node_config_invalid_secret() { + let app = setup_app(); + let response = request_app(app, "nodeConfigFull/zzz".to_string().as_str()).await; + + assert_eq!(response.status(), StatusCode::FORBIDDEN); } #[tokio::test] @@ -102,8 +124,14 @@ async fn with_metrics() { // Creates an app with prometheus handle. let ((storage_reader, _), _temp_dir) = test_utils::get_test_storage(); let prometheus_handle = PrometheusBuilder::new().install_recorder().unwrap(); - let app = - app(storage_reader, TEST_VERSION, serde_json::Value::default(), Some(prometheus_handle)); + let app = app( + storage_reader, + TEST_VERSION, + serde_json::Value::default(), + serde_json::Value::default(), + String::new(), + Some(prometheus_handle), + ); // Register a metric. let metric_name = "metric_name"; diff --git a/crates/papyrus_monitoring_gateway/src/lib.rs b/crates/papyrus_monitoring_gateway/src/lib.rs index 2a48da522e..ed8c3d68a7 100644 --- a/crates/papyrus_monitoring_gateway/src/lib.rs +++ b/crates/papyrus_monitoring_gateway/src/lib.rs @@ -10,24 +10,28 @@ use std::fmt::Display; use std::net::SocketAddr; use std::str::FromStr; +use axum::extract::Path; use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use axum::routing::get; use axum::{Json, Router}; use metrics_exporter_prometheus::{BuildError, PrometheusBuilder, PrometheusHandle}; use metrics_process::Collector; -use papyrus_config::dumping::{ser_param, SerializeConfig}; -use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam}; +use papyrus_config::dumping::{ser_param, ser_required_param, SerializeConfig}; +use papyrus_config::{ParamPath, ParamPrivacyInput, SerializationType, SerializedParam}; use papyrus_storage::{DbTablesStats, StorageError, StorageReader}; use serde::{Deserialize, Serialize}; use tracing::{debug, instrument}; +use validator::Validate; const MONITORING_PREFIX: &str = "monitoring"; -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Validate)] pub struct MonitoringGatewayConfig { pub server_address: String, pub collect_metrics: bool, + #[validate(length(min = 1))] + pub config_representation_secret: String, } impl Default for MonitoringGatewayConfig { @@ -35,6 +39,7 @@ impl Default for MonitoringGatewayConfig { MonitoringGatewayConfig { server_address: String::from("0.0.0.0:8081"), collect_metrics: false, + config_representation_secret: String::from("qwerty"), } } } @@ -54,6 +59,12 @@ impl SerializeConfig for MonitoringGatewayConfig { "If true, collect and return metrics in the monitoring gateway.", ParamPrivacyInput::Public, ), + ser_required_param( + "config_representation_secret", + SerializationType::String, + "A secret for representing the full general config.", + ParamPrivacyInput::Private, + ), ]) } } @@ -67,7 +78,10 @@ impl Display for MonitoringGatewayConfig { pub struct MonitoringServer { config: MonitoringGatewayConfig, - general_config_representation: serde_json::Value, + // Nested Json representation of all the parameters in the node config. + full_general_config_representation: serde_json::Value, + // Nested Json representation of the public parameters in the node config. + public_general_config_representation: serde_json::Value, storage_reader: StorageReader, version: &'static str, prometheus_handle: Option, @@ -76,7 +90,8 @@ pub struct MonitoringServer { impl MonitoringServer { pub fn new( config: MonitoringGatewayConfig, - general_config_representation: serde_json::Value, + full_general_config_representation: serde_json::Value, + public_general_config_representation: serde_json::Value, storage_reader: StorageReader, version: &'static str, ) -> Result { @@ -88,7 +103,8 @@ impl MonitoringServer { Ok(MonitoringServer { config, storage_reader, - general_config_representation, + full_general_config_representation, + public_general_config_representation, version, prometheus_handle, }) @@ -104,7 +120,9 @@ impl MonitoringServer { fields( version = %self.version, config = %self.config, - general_config_representation = %self.general_config_representation), + full_general_config_representation = %self.full_general_config_representation, + public_general_config_representation = %self.public_general_config_representation, + config_representation_secret = %self.config.config_representation_secret), level = "debug")] async fn run_server(&self) -> std::result::Result<(), hyper::Error> { let server_address = SocketAddr::from_str(&self.config.server_address) @@ -112,7 +130,9 @@ impl MonitoringServer { let app = app( self.storage_reader.clone(), self.version, - self.general_config_representation.clone(), + self.full_general_config_representation.clone(), + self.public_general_config_representation.clone(), + self.config.config_representation_secret.clone(), self.prometheus_handle.clone(), ); debug!("Starting monitoring gateway."); @@ -123,7 +143,9 @@ impl MonitoringServer { fn app( storage_reader: StorageReader, version: &'static str, - general_config_representation: serde_json::Value, + full_general_config_representation: serde_json::Value, + public_general_config_representation: serde_json::Value, + config_representation_secret: String, prometheus_handle: Option, ) -> Router { Router::new() @@ -133,7 +155,18 @@ fn app( ) .route( format!("/{MONITORING_PREFIX}/nodeConfig").as_str(), - get(move || node_config(general_config_representation)), + get(move || node_config(public_general_config_representation)), + ) + .route( + // The "*secret" captures the end of the path and stores it in "secret". + format!("/{MONITORING_PREFIX}/nodeConfigFull/*secret").as_str(), + get(move |secret| { + node_config_by_secret( + full_general_config_representation, + secret, + config_representation_secret, + ) + }), ) .route( format!("/{MONITORING_PREFIX}/nodeVersion").as_str(), @@ -160,9 +193,23 @@ async fn db_tables_stats( /// Returns the node config. #[instrument(level = "debug", ret)] async fn node_config( - general_config_representation: serde_json::Value, + full_general_config_representation: serde_json::Value, ) -> axum::Json { - general_config_representation.into() + full_general_config_representation.into() +} + +/// Returns the node config. +#[instrument(level = "debug", ret)] +async fn node_config_by_secret( + full_general_config_representation: serde_json::Value, + given_secret: Path, + expected_secret: String, +) -> Result, StatusCode> { + if given_secret.to_string() == expected_secret { + Ok(node_config(full_general_config_representation).await) + } else { + Err(StatusCode::FORBIDDEN) + } } /// Returns prometheus metrics. diff --git a/crates/papyrus_node/src/bin/central_source_integration_test.rs b/crates/papyrus_node/src/bin/central_source_integration_test.rs index 49749bb49e..91f242ef52 100644 --- a/crates/papyrus_node/src/bin/central_source_integration_test.rs +++ b/crates/papyrus_node/src/bin/central_source_integration_test.rs @@ -21,6 +21,7 @@ async fn main() { "--central.url=https://alpha4.starknet.io/".to_owned(), "--base_layer.node_url=https://mainnet.infura.io/v3/1234".to_owned(), format!("--storage.db_config.path_prefix={}", path.display()), + "--monitoring_gateway.config_representation_secret=abcd".to_owned(), ]) .expect("Load config"); let (storage_reader, _) = open_storage(config.storage.db_config).expect("Open storage"); diff --git a/crates/papyrus_node/src/config/config_test.rs b/crates/papyrus_node/src/config/config_test.rs index 2d2180d78a..c5e9217778 100644 --- a/crates/papyrus_node/src/config/config_test.rs +++ b/crates/papyrus_node/src/config/config_test.rs @@ -2,12 +2,16 @@ use std::collections::{BTreeMap, HashMap}; use std::env::{self, args}; use std::fs::File; use std::io::{BufWriter, Write}; +use std::ops::IndexMut; use std::path::{Path, PathBuf}; use std::str::FromStr; +use itertools::Itertools; use papyrus_base_layer::ethereum_base_layer_contract::EthereumBaseLayerConfig; use papyrus_config::dumping::SerializeConfig; -use papyrus_config::SerializedParam; +use papyrus_config::representation::get_config_representation; +use papyrus_config::{SerializedContent, SerializedParam}; +use papyrus_monitoring_gateway::MonitoringGatewayConfig; use pretty_assertions::assert_eq; use serde_json::{json, Map, Value}; use starknet_api::core::ChainId; @@ -17,9 +21,32 @@ use validator::Validate; use crate::config::{node_command, NodeConfig, DEFAULT_CONFIG_PATH}; -// Fill here all the required params in default_config.json with the default value. +// Returns the required params in default_config.json with the default value from the config +// representation. fn required_args() -> Vec { - vec!["--base_layer.node_url".to_owned(), EthereumBaseLayerConfig::default().node_url] + let default_config = NodeConfig::default(); + let mut args = Vec::new(); + let mut config_representation = get_config_representation(&default_config, true).unwrap(); + + for (param_path, serialized_param) in default_config.dump() { + let SerializedContent::RequiredType(serialization_type) = serialized_param.content else { + continue; + }; + args.push(format!("--{param_path}")); + + let required_param_json_value = param_path + .split('.') + .fold(&mut config_representation, |entry, config_name| entry.index_mut(config_name)); + + let required_param_string_value = match serialization_type { + papyrus_config::SerializationType::String => { + required_param_json_value.as_str().unwrap().to_string() + } + _ => required_param_json_value.to_string(), + }; + args.push(required_param_string_value); + } + args } fn get_args(additional_args: Vec<&str>) -> Vec { diff --git a/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap b/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap index 649caed992..511be46225 100644 --- a/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap +++ b/crates/papyrus_node/src/config/snapshots/papyrus_node__config__config_test__dump_default_config.snap @@ -77,6 +77,11 @@ expression: dumped_default_config "value": false, "privacy": "Public" }, + "monitoring_gateway.config_representation_secret": { + "description": "A secret for representing the full general config.", + "required_type": "String", + "privacy": "Private" + }, "monitoring_gateway.server_address": { "description": "node's monitoring server.", "value": "0.0.0.0:8081", diff --git a/crates/papyrus_node/src/main.rs b/crates/papyrus_node/src/main.rs index a427ccf984..3e7e6040ee 100644 --- a/crates/papyrus_node/src/main.rs +++ b/crates/papyrus_node/src/main.rs @@ -32,6 +32,7 @@ async fn run_threads(config: NodeConfig) -> anyhow::Result<()> { let monitoring_server = MonitoringServer::new( config.monitoring_gateway.clone(), get_config_representation(&config, true)?, + get_config_representation(&config, false)?, storage_reader.clone(), VERSION_FULL, )?;