Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] #4063: update configuration endpoints #4076

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 13 additions & 43 deletions cli/src/torii/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@

use eyre::{eyre, WrapErr};
use futures::TryStreamExt;
use iroha_config::{
base::proxy::Documented,
iroha::{Configuration, ConfigurationView},
torii::uri,
GetConfiguration, PostConfiguration,
};
use iroha_config::{client_api::ConfigurationDTO, iroha::Configuration, torii::uri};
use iroha_core::{
query::{pagination::Paginate, store::LiveQueryStoreHandle},
smartcontracts::query::ValidQueryRequest,
Expand Down Expand Up @@ -169,42 +164,18 @@ async fn handle_pending_transactions(
}

#[iroha_futures::telemetry_future]
async fn handle_get_configuration(
iroha_cfg: Configuration,
get_cfg: GetConfiguration,
) -> Result<Json> {
use GetConfiguration::*;

match get_cfg {
Docs(field) => <Configuration as Documented>::get_doc_recursive(
field.iter().map(AsRef::as_ref).collect::<Vec<&str>>(),
)
.wrap_err("Failed to get docs {:?field}")
.and_then(|doc| serde_json::to_value(doc).wrap_err("Failed to serialize docs")),
// Cast to configuration view to hide private keys.
Value => serde_json::to_value(ConfigurationView::from(iroha_cfg))
.wrap_err("Failed to serialize value"),
}
.map(|v| reply::json(&v))
.map_err(Error::Config)
async fn handle_get_configuration(iroha_cfg: Configuration) -> Result<Json> {
let subset = ConfigurationDTO::from(&iroha_cfg);
Ok(reply::json(&subset))
}

#[iroha_futures::telemetry_future]
async fn handle_post_configuration(
iroha_cfg: Configuration,
cfg: PostConfiguration,
) -> Result<Json> {
use iroha_config::base::runtime_upgrades::Reload;
use PostConfiguration::*;

iroha_logger::debug!(?cfg);
match cfg {
LogLevel(level) => {
iroha_cfg.logger.max_log_level.reload(level)?;
}
};

Ok(reply::json(&true))
value: ConfigurationDTO,
) -> Result<impl Reply> {
value.update_base(&iroha_cfg)?;
Ok(reply::reply())
}

#[iroha_futures::telemetry_future]
Expand Down Expand Up @@ -437,12 +408,11 @@ impl Torii {
.and(add_state!(self.queue, self.sumeragi,))
.and(paginate()),
)
.or(endpoint2(
handle_get_configuration,
warp::path(uri::CONFIGURATION)
.and(add_state!(self.iroha_cfg))
.and(warp::body::json()),
)),
.or(warp::path(uri::CONFIGURATION)
.and(add_state!(self.iroha_cfg))
.and_then(|iroha_cfg| async move {
Ok::<_, Infallible>(WarpResult(handle_get_configuration(iroha_cfg).await))
})),
);

let get_router_status = warp::path(uri::STATUS)
Expand Down
38 changes: 10 additions & 28 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use derive_more::{DebugCustom, Display};
use eyre::{eyre, Result, WrapErr};
use futures_util::StreamExt;
use http_default::{AsyncWebSocketStream, WebSocketStream};
use iroha_config::{client::Configuration, torii::uri, GetConfiguration, PostConfiguration};
use iroha_config::{client::Configuration, client_api::ConfigurationDTO, torii::uri};
use iroha_crypto::{HashOf, KeyPair};
use iroha_data_model::{
block::SignedBlock,
Expand All @@ -29,7 +29,6 @@ use iroha_telemetry::metrics::Status;
use iroha_version::prelude::*;
use parity_scale_codec::DecodeAll;
use rand::Rng;
use serde::de::DeserializeOwned;
use url::Url;

use self::{blocks_api::AsyncBlockStream, events_api::AsyncEventStream};
Expand Down Expand Up @@ -1073,13 +1072,16 @@ impl Client {
)
}

fn get_config<T: DeserializeOwned>(&self, get_config: &GetConfiguration) -> Result<T> {
/// Get value of config on peer
///
/// # Errors
/// Fails if sending request or decoding fails
pub fn get_config(&self) -> Result<ConfigurationDTO> {
let resp = DefaultRequestBuilder::new(
HttpMethod::GET,
self.torii_url.join(uri::CONFIGURATION).expect("Valid URI"),
)
.header(http::header::CONTENT_TYPE, APPLICATION_JSON)
.body(serde_json::to_vec(get_config).wrap_err("Failed to serialize")?)
.build()?
.send()?;

Expand All @@ -1097,9 +1099,8 @@ impl Client {
///
/// # Errors
/// If sending request or decoding fails
pub fn set_config(&self, post_config: PostConfiguration) -> Result<bool> {
let body = serde_json::to_vec(&post_config)
.wrap_err(format!("Failed to serialize {post_config:?}"))?;
pub fn set_config(&self, dto: ConfigurationDTO) -> Result<()> {
let body = serde_json::to_vec(&dto).wrap_err(format!("Failed to serialize {dto:?}"))?;
let url = self.torii_url.join(uri::CONFIGURATION).expect("Valid URI");
let resp = DefaultRequestBuilder::new(HttpMethod::POST, url)
.header(http::header::CONTENT_TYPE, APPLICATION_JSON)
Expand All @@ -1113,28 +1114,9 @@ impl Client {
resp.status(),
std::str::from_utf8(resp.body()).unwrap_or(""),
));
}
serde_json::from_slice(resp.body())
.wrap_err(format!("Failed to decode body {:?}", resp.body()))
}

/// Get documentation of some field on config
///
/// # Errors
/// Fails if sending request or decoding fails
pub fn get_config_docs(&self, field: &[&str]) -> Result<Option<String>> {
let field = field.iter().copied().map(ToOwned::to_owned).collect();
self.get_config(&GetConfiguration::Docs(field))
.wrap_err("Failed to get docs for field")
}
};

/// Get value of config on peer
///
/// # Errors
/// Fails if sending request or decoding fails
pub fn get_config_value(&self) -> Result<serde_json::Value> {
self.get_config(&GetConfiguration::Value)
.wrap_err("Failed to get configuration value")
Ok(())
}

/// Gets network status seen from the peer
Expand Down
36 changes: 22 additions & 14 deletions client/tests/integration/config.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,35 @@
use iroha_data_model::Level;
use test_network::*;

use super::{Builder, Configuration, ConfigurationProxy};
use super::Configuration;

#[test]
fn get_config() {
fn config_endpoints() {
// The underscored variables must not be dropped until end of closure.
let (_dont_drop, _dont_drop_either, test_client) =
<PeerBuilder>::new().with_port(10_685).start_with_runtime();
wait_for_genesis_committed(&vec![test_client.clone()], 0);
let test_cfg = Configuration::test();
const NEW_LOG_LEVEL: Level = Level::TRACE;

let field = test_client.get_config_docs(&["torii"]).unwrap().unwrap();
assert!(field.contains("IROHA_TORII"));
// Just to be sure this test suite is not useless
assert_ne!(test_cfg.logger.max_log_level.value(), NEW_LOG_LEVEL);

let test = Configuration::test();
let cfg_proxy: ConfigurationProxy =
serde_json::from_value(test_client.get_config_value().unwrap()).unwrap();
// Retrieving through API
let mut dto = test_client.get_config().unwrap();
assert_eq!(
cfg_proxy.block_sync.unwrap().build().unwrap(),
test.block_sync
);
assert_eq!(cfg_proxy.network.unwrap().build().unwrap(), test.network);
assert_eq!(
cfg_proxy.telemetry.unwrap().build().unwrap(),
*test.telemetry
dto.logger.max_log_level,
test_cfg.logger.max_log_level.value()
);

// Updating the log level
dto.logger.max_log_level = NEW_LOG_LEVEL;
test_client.set_config(dto).unwrap();

// FIXME: The updated value is not reflected
// https://github.com/hyperledger/iroha/issues/4079

// // Checking the updated value
// let dto = test_client.get_config().unwrap();
// assert_eq!(dto.logger.max_log_level, NEW_LOG_LEVEL);
}
1 change: 1 addition & 0 deletions config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ once_cell = { workspace = true }
[dev-dependencies]
proptest = "1.3.1"
stacker = "0.1.15"
expect-test = { workspace = true }

[features]
tokio-console = []
83 changes: 83 additions & 0 deletions config/src/client_api.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
//! Functionality related to working with the configuration through client API.
//!
//! Intended usage:
//!
//! - Create [`ConfigurationDTO`] from [`crate::iroha::Configuration`] and serialize it for the client
//! - Deserialize [`ConfigurationDTO`] from the client and use [`ConfigurationDTO::apply_update()`] to update the configuration
// TODO: Currently logic here is not generalised and handles only `logger.max_log_level` parameter. In future, when
// other parts of configuration are refactored and there is a solid foundation e.g. as a general
// configuration-related crate, this part should be re-written in a clean way.
// Track configuration refactoring here: https://github.com/hyperledger/iroha/issues/2585

use iroha_config_base::runtime_upgrades::{Reload, ReloadError};
use iroha_data_model::Level;
use serde::{Deserialize, Serialize};

use super::{iroha::Configuration as BaseConfiguration, logger::Configuration as BaseLogger};

/// Subset of [`super::iroha`] configuration.
#[derive(Debug, Serialize, Deserialize, Clone, Copy)]
pub struct ConfigurationDTO {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call it PartialConfiguration or DynamicConfiguration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that DTO suffix emphasizes the purpose to use this structure to interact with the client side.

Partial seems to be too general here (like Proxys we have already), while Dynamic might semantically fit.

#[allow(missing_docs)]
pub logger: Logger,
}

impl From<&'_ BaseConfiguration> for ConfigurationDTO {
fn from(value: &'_ BaseConfiguration) -> Self {
Self {
logger: value.logger.as_ref().into(),
}
}
}

impl ConfigurationDTO {
/// Update the base configuration with the values stored in [`Self`].
pub fn update_base(&self, target: &BaseConfiguration) -> Result<(), ReloadError> {
target
.logger
.max_log_level
.reload(self.logger.max_log_level)
}
}

/// Subset of [`super::logger`] configuration.
#[derive(Debug, Serialize, Deserialize, Clone, Copy)]
pub struct Logger {
#[allow(missing_docs)]
pub max_log_level: Level,
}

impl From<&'_ BaseLogger> for Logger {
fn from(value: &'_ BaseLogger) -> Self {
Self {
max_log_level: value.max_log_level.value(),
}
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn snapshot_serialized_form() {
let value = ConfigurationDTO {
logger: Logger {
max_log_level: Level::TRACE,
},
};

let actual = serde_json::to_string_pretty(&value).expect("The value is a valid JSON");

// NOTE: whenever this is updated, make sure to update the documentation accordingly:
// https://hyperledger.github.io/iroha-2-docs/reference/torii-endpoints.html
// -> Configuration endpoints
let expected = expect_test::expect![[r#"
{
"logger": {
"max_log_level": "TRACE"
}
}"#]];
expected.assert_eq(&actual);
}
}
34 changes: 1 addition & 33 deletions config/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Aggregate configuration for different Iroha modules.
pub use iroha_config_base as base;
use serde::{Deserialize, Serialize};

pub mod block_sync;
pub mod client;
pub mod client_api;
pub mod genesis;
pub mod iroha;
pub mod kura;
Expand All @@ -18,35 +18,3 @@ pub mod telemetry;
pub mod torii;
pub mod wasm;
pub mod wsv;

/// Json config for getting configuration
#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum GetConfiguration {
/// Getting docs of specific field
///
/// Top-level fields must be enclosed in an array (of strings). This array
/// provides the fully qualified path to the fields.
///
/// # Examples
///
/// To get the top-level configuration docs for `iroha_core::Torii`
/// `curl -X GET -H 'content-type: application/json' http://127.0.0.1:8080/configuration -d '{"Docs" : ["torii"]} ' -i`
///
/// To get the documentation on the [`Logger::config::Configuration.max_log_level`]
/// `curl -X GET -H 'content-type: application/json' http://127.0.0.1:8080/configuration -d '{"Docs" : ["logger", "max_log_level"]}' -i`
Docs(Vec<String>),
/// Get the original Value of the full configuration.
Value,
}

/// Message acceptable for `POST` requests to the configuration endpoint.
#[derive(Clone, Debug, Deserialize, Serialize, Copy)]
pub enum PostConfiguration {
/// Change the maximum logging level of logger.
///
/// # Examples
///
/// To silence all logging events that aren't `ERROR`s
/// `curl -X POST -H 'content-type: application/json' http://127.0.0.1:8080/configuration -d '{"LogLevel": "ERROR"}' -i`
LogLevel(iroha_data_model::Level),
}
Loading