From 51ce8ab7de2f668cc4b1c9b2cd6fc2890e530658 Mon Sep 17 00:00:00 2001 From: Kriogenia <47500377+kriogenia@users.noreply.github.com> Date: Tue, 3 Oct 2023 15:04:49 +0100 Subject: [PATCH] feat(rust): Improve JSON output of ockam node show Improves the output of the command `ockam node show --output json` adding all the information that is being displayed with the plain output. Refactors the plain response to build it using the Output trait erasing the a clippy alert in the process. Closes #6142 --- .../buildOutputCleanup.lock | Bin 0 -> 17 bytes .../buildOutputCleanup/cache.properties | 2 + .../ockam_api/src/nodes/models/address.rs | 19 + .../ockam/ockam_api/src/nodes/models/mod.rs | 2 + .../ockam_api/src/nodes/models/portal.rs | 17 +- .../src/nodes/models/secure_channel.rs | 11 +- .../ockam_api/src/nodes/models/services.rs | 3 + .../ockam/ockam_api/src/nodes/models/show.rs | 76 ++++ .../src/nodes/models/transport/response.rs | 14 +- .../rust/ockam/ockam_command/src/node/show.rs | 325 ++++++++---------- 10 files changed, 273 insertions(+), 196 deletions(-) create mode 100644 implementations/rust/.gradle/buildOutputCleanup/buildOutputCleanup.lock create mode 100644 implementations/rust/.gradle/buildOutputCleanup/cache.properties create mode 100644 implementations/rust/ockam/ockam_api/src/nodes/models/address.rs create mode 100644 implementations/rust/ockam/ockam_api/src/nodes/models/show.rs diff --git a/implementations/rust/.gradle/buildOutputCleanup/buildOutputCleanup.lock b/implementations/rust/.gradle/buildOutputCleanup/buildOutputCleanup.lock new file mode 100644 index 0000000000000000000000000000000000000000..4a5d9a6b72841ca0c29a41f02b29749f2a22bae1 GIT binary patch literal 17 UcmZQhQ#hq5dG((t: &T, s: S) -> Result +where + T: Into
+ Clone, + S: Serializer, +{ + s.serialize_some(&addr_to_multiaddr(t.clone())) +} + +pub fn serialize_route_as_multiaddr(route: &String, s: S) -> Result +where +S: Serializer, +{ + s.serialize_some(&Route::parse(route).and_then(|r| route_to_multiaddr(&r))) +} \ No newline at end of file diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/mod.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/mod.rs index 2a97cb083ec..d53f55c6e5b 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/models/mod.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/mod.rs @@ -2,6 +2,7 @@ /// /// This module is only a type facade and should not have any logic of /// its own +pub mod address; pub mod base; pub mod credentials; pub mod flow_controls; @@ -10,5 +11,6 @@ pub mod portal; pub mod relay; pub mod secure_channel; pub mod services; +pub mod show; pub mod transport; pub mod workers; diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs index 82fc7adad72..17723db638b 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/portal.rs @@ -150,11 +150,16 @@ impl CreateOutlet { #[rustfmt::skip] #[cbor(map)] pub struct InletStatus { + #[serde(rename = "listen_address")] #[n(1)] pub bind_addr: String, + #[serde(skip)] #[n(2)] pub worker_addr: String, + #[serde(skip)] #[n(3)] pub alias: String, /// An optional status payload + #[serde(skip)] #[n(4)] pub payload: Option, + #[serde(rename = "route_to_outlet", serialize_with = "super::address::serialize_route_as_multiaddr")] #[n(5)] pub outlet_route: String, } @@ -191,10 +196,14 @@ impl InletStatus { #[rustfmt::skip] #[cbor(map)] pub struct OutletStatus { + #[serde(rename = "forward_address")] #[n(1)] pub socket_addr: SocketAddr, + #[serde(rename = "address", serialize_with = "super::address::serialize_as_multiaddr")] #[n(2)] pub worker_addr: Address, + #[serde(skip)] #[n(3)] pub alias: String, /// An optional status payload + #[serde(skip)] #[n(4)] pub payload: Option, } @@ -237,10 +246,11 @@ impl OutletStatus { } /// Response body when returning a list of Inlets -#[derive(Debug, Clone, Decode, Encode)] +#[derive(Debug, Clone, Decode, Encode, Serialize)] #[rustfmt::skip] #[cbor(map)] pub struct InletList { + #[serde(rename = "inlets")] #[n(1)] pub list: Vec } @@ -251,10 +261,11 @@ impl InletList { } /// Response body when returning a list of Outlets -#[derive(Debug, Clone, Decode, Encode)] +#[derive(Debug, Clone, Decode, Encode, Serialize)] #[rustfmt::skip] #[cbor(map)] pub struct OutletList { + #[serde(rename = "outlets")] #[n(1)] pub list: Vec } @@ -262,4 +273,4 @@ impl OutletList { pub fn new(list: Vec) -> Self { Self { list } } -} +} \ No newline at end of file diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/secure_channel.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/secure_channel.rs index c1becd236d9..77e06e82f2e 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/models/secure_channel.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/secure_channel.rs @@ -1,12 +1,12 @@ use std::time::Duration; use minicbor::{Decode, Encode}; -use serde::Serialize; use ockam::identity::{Identifier, DEFAULT_TIMEOUT}; use ockam_core::flow_control::FlowControlId; use ockam_core::{route, Address, Result}; use ockam_multiaddr::MultiAddr; +use serde::Serialize; use crate::error::ApiError; use crate::nodes::registry::{SecureChannelInfo, SecureChannelListenerInfo}; @@ -144,11 +144,13 @@ impl ShowSecureChannelListenerRequest { } /// Response body to show a Secure Channel Listener -#[derive(Debug, Clone, Decode, Encode)] +#[derive(Debug, Clone, Decode, Encode, Serialize)] #[rustfmt::skip] #[cbor(map)] pub struct ShowSecureChannelListenerResponse { - #[n(1)] pub addr: Address, + #[serde(rename = "address", serialize_with = "super::address::serialize_as_multiaddr")] + #[n(1)] pub addr: Address, + #[serde(rename = "flow_control")] #[n(2)] pub flow_control_id: FlowControlId, } @@ -236,10 +238,11 @@ impl ShowSecureChannelResponse { } } -#[derive(Debug, Clone, Decode, Encode)] +#[derive(Debug, Clone, Decode, Encode, Serialize)] #[rustfmt::skip] #[cbor(map)] pub struct SecureChannelListenersList { + #[serde(rename = "secure_channel_listeners")] #[n(1)] pub list: Vec, } diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/services.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/services.rs index 8e19e09c2eb..512f63be0e0 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/models/services.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/services.rs @@ -365,7 +365,9 @@ impl StartOktaIdentityProviderRequest { #[rustfmt::skip] #[cbor(map)] pub struct ServiceStatus { + #[serde(rename = "address", serialize_with = "super::address::serialize_as_multiaddr")] #[n(2)] pub addr: String, + #[serde(rename = "type")] #[n(3)] pub service_type: String, } @@ -383,6 +385,7 @@ impl ServiceStatus { #[rustfmt::skip] #[cbor(map)] pub struct ServiceList { + #[serde(rename = "services")] #[n(1)] pub list: Vec } diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/show.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/show.rs new file mode 100644 index 00000000000..bc195290e9d --- /dev/null +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/show.rs @@ -0,0 +1,76 @@ +use ockam_multiaddr::{ + proto::{DnsAddr, Node, Tcp}, + MultiAddr, +}; +use serde::Serialize; + +use super::{ + portal::{InletList, OutletList}, + secure_channel::SecureChannelListenersList, + services::ServiceList, + transport::TransportList, +}; + +#[derive(Debug, Serialize)] +pub struct ShowNodeResponse<'a> { + pub is_default: bool, + pub name: &'a str, + pub is_up: bool, + pub route: RouteToNode, + #[serde(skip_serializing_if = "Option::is_none")] + pub identity: Option, + #[serde(skip_serializing_if = "Option::is_none", flatten)] + pub tcp_listeners: Option, + #[serde(skip_serializing_if = "Option::is_none", flatten)] + pub secure_channel_listeners: Option, + #[serde(skip_serializing_if = "Option::is_none", flatten)] + pub inlets: Option, + #[serde(skip_serializing_if = "Option::is_none", flatten)] + pub outlets: Option, + #[serde(skip_serializing_if = "Option::is_none", flatten)] + pub service_list: Option, +} +#[derive(Debug, Serialize)] +pub struct RouteToNode { + #[serde(skip_serializing_if = "Option::is_none")] + pub short: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub verbose: Option, +} + +impl<'a> ShowNodeResponse<'a> { + pub fn new( + is_default: bool, + name: &'a str, + is_up: bool, + node_port: Option, + ) -> ShowNodeResponse { + let mut m = MultiAddr::default(); + let short = m.push_back(Node::new(name)).ok().map(|_| m); + + let verbose = node_port + .and_then(|port| { + let mut m = MultiAddr::default(); + if m.push_back(DnsAddr::new("localhost")).is_ok() + && m.push_back(Tcp::new(port)).is_ok() + { + Some(m) + } else { + None + } + }); + + ShowNodeResponse { + is_default, + name, + is_up, + route: RouteToNode { short, verbose }, + identity: None, + tcp_listeners: None, + secure_channel_listeners: None, + inlets: None, + outlets: None, + service_list: None, + } + } +} diff --git a/implementations/rust/ockam/ockam_api/src/nodes/models/transport/response.rs b/implementations/rust/ockam/ockam_api/src/nodes/models/transport/response.rs index a0bf6db3eed..adb4a054038 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/models/transport/response.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/models/transport/response.rs @@ -6,24 +6,31 @@ use ockam_core::flow_control::FlowControlId; use ockam_core::{Error, Result}; use ockam_multiaddr::proto::Worker; use ockam_multiaddr::MultiAddr; +use serde::Serialize; use std::net::SocketAddrV4; /// Response body when interacting with a transport -#[derive(Debug, Clone, Decode, Encode)] +#[derive(Debug, Clone, Decode, Encode, Serialize)] #[rustfmt::skip] #[cbor(map)] pub struct TransportStatus { /// The type of transport to create + #[serde(rename = "type")] #[n(1)] pub tt: TransportType, /// The mode the transport should operate in + #[serde(rename = "mode")] #[n(2)] pub tm: TransportMode, /// Corresponding socket address + #[serde(rename = "socket")] #[n(3)] pub socket_addr: String, /// Corresponding worker address + #[serde(rename = "worker")] #[n(4)] pub worker_addr: String, /// Corresponding worker address + #[serde(skip)] #[n(5)] pub processor_address: String, /// Corresponding flow control id + #[serde(rename = "flow_control")] #[n(6)] pub flow_control_id: FlowControlId, } @@ -58,10 +65,11 @@ impl TransportStatus { } /// Response body when interacting with a transport -#[derive(Debug, Clone, Decode, Encode)] +#[derive(Debug, Clone, Decode, Encode, Serialize)] #[rustfmt::skip] #[cbor(map)] pub struct TransportList { + #[serde(rename = "transports")] #[n(1)] pub list: Vec } @@ -69,4 +77,4 @@ impl TransportList { pub fn new(list: Vec) -> Self { Self { list } } -} +} \ No newline at end of file diff --git a/implementations/rust/ockam/ockam_command/src/node/show.rs b/implementations/rust/ockam/ockam_command/src/node/show.rs index d890d3238c6..37976b47e5d 100644 --- a/implementations/rust/ockam/ockam_command/src/node/show.rs +++ b/implementations/rust/ockam/ockam_command/src/node/show.rs @@ -1,24 +1,21 @@ use clap::Args; use colorful::Colorful; +use ockam_api::nodes::BackgroundNode; +use ockam_api::nodes::models::show::ShowNodeResponse; +use ockam_node::Context; use tokio_retry::strategy::FixedInterval; use tracing::{info, trace, warn}; use ockam_api::cli_state::{CliState, StateDirTrait, StateItemTrait}; use ockam_api::nodes::models::portal::{InletList, OutletList}; -use ockam_api::nodes::models::secure_channel::SecureChannelListenersList; -use ockam_api::nodes::models::services::ServiceList; -use ockam_api::nodes::models::transport::TransportList; -use ockam_api::nodes::BackgroundNode; use ockam_api::{addr_to_multiaddr, route_to_multiaddr}; use ockam_core::Route; -use ockam_multiaddr::proto::{DnsAddr, Node, Tcp}; -use ockam_multiaddr::MultiAddr; -use ockam_node::Context; use crate::node::get_node_name; use crate::node::util::check_default; +use crate::output::Output; use crate::util::{api, node_rpc}; -use crate::{docs, CommandGlobalOpts, OutputFormat, Result}; +use crate::{docs, CommandGlobalOpts, Result}; const LONG_ABOUT: &str = include_str!("./static/show/long_about.txt"); const PREVIEW_TAG: &str = include_str!("../static/preview_tag.txt"); @@ -57,120 +54,6 @@ async fn run_impl( Ok(()) } -// TODO: This function should be replaced with a better system of -// printing the node state in the future but for now we can just tell -// clippy to stop complaining about it. -#[allow(clippy::too_many_arguments)] -fn print_node_info( - opts: &CommandGlobalOpts, - node_port: Option, - node_name: &str, - is_default: bool, - status_is_up: bool, - default_id: Option<&str>, - services: Option<&ServiceList>, - tcp_listeners: Option<&TransportList>, - secure_channel_listeners: Option<&SecureChannelListenersList>, - inlets_outlets: Option<(&InletList, &OutletList)>, -) { - if opts.global_args.output_format == OutputFormat::Json { - opts.terminal - .clone() - .stdout() - .json(serde_json::json!({ "name": &node_name })) - .write_line() - .expect("Failed to write to stdout."); - return; - } - println!(); - println!("Node:"); - if is_default { - println!(" Name: {node_name} (Default)"); - } else { - println!(" Name: {node_name}"); - } - println!( - " Status: {}", - match status_is_up { - true => "UP".light_green(), - false => "DOWN".light_red(), - } - ); - - println!(" Route To Node:"); - let mut m = MultiAddr::default(); - if m.push_back(Node::new(node_name)).is_ok() { - println!(" Short: {m}"); - } - - if let Some(port) = node_port { - let mut m = MultiAddr::default(); - if m.push_back(DnsAddr::new("localhost")).is_ok() && m.push_back(Tcp::new(port)).is_ok() { - println!(" Verbose: {m}"); - } - } - - if let Some(id) = default_id { - println!(" Identity: {id}"); - } - - if let Some(list) = tcp_listeners { - println!(" Transports:"); - for e in &list.list { - println!(" Transport:"); - println!(" Type: {}", e.tt); - println!(" Mode: {}", e.tm); - println!(" Socket: {}", e.socket_addr); - println!(" Worker: {}", e.worker_addr); - println!(" FlowControlId: {}", e.flow_control_id); - } - } - - if let Some(list) = secure_channel_listeners { - println!(" Secure Channel Listeners:"); - for e in &list.list { - println!(" Listener:"); - if let Some(ma) = addr_to_multiaddr(e.addr.clone()) { - println!(" Address: {ma}"); - println!(" FlowControlId: {}", &e.flow_control_id); - } - } - } - - if let Some((inlets, outlets)) = inlets_outlets { - println!(" Inlets:"); - for e in &inlets.list { - println!(" Inlet:"); - println!(" Listen Address: {}", e.bind_addr); - if let Some(r) = Route::parse(&e.outlet_route) { - if let Some(ma) = route_to_multiaddr(&r) { - println!(" Route To Outlet: {ma}"); - } - } - } - println!(" Outlets:"); - for e in &outlets.list { - println!(" Outlet:"); - println!(" Forward Address: {}", e.socket_addr); - - if let Some(ma) = addr_to_multiaddr(e.worker_addr.to_string()) { - println!(" Address: {ma}"); - } - } - } - - if let Some(list) = services { - println!(" Services:"); - for e in &list.list { - println!(" Service:"); - println!(" Type: {}", e.service_type); - if let Some(ma) = addr_to_multiaddr(e.addr.as_str()) { - println!(" Address: {ma}"); - } - } - } -} - pub async fn print_query_status( opts: &CommandGlobalOpts, ctx: &Context, @@ -180,77 +63,51 @@ pub async fn print_query_status( is_default: bool, ) -> miette::Result<()> { let cli_state = opts.state.clone(); - if !is_node_up(ctx, node_name, node, cli_state.clone(), wait_until_ready).await? { - let node_state = cli_state.nodes.get(node_name)?; - let node_port = node_state - .config() - .setup() - .api_transport() - .ok() - .map(|listener| listener.addr.port()); - - // it is expected to not be able to open an arbitrary TCP connection on an authority node - // so in that case we display an UP status - let is_authority_node = node_state.config().setup().authority_node.unwrap_or(false); - print_node_info( - opts, - node_port, - node_name, - is_default, - is_authority_node, - None, - None, - None, - None, - None, - ); - } else { - let node_state = cli_state.nodes.get(node_name)?; - // Get short id for the node - let default_id = match node_state.config().identity_config() { - Ok(resp) => resp.identifier().to_string(), - Err(_) => String::from("None"), - }; + let node_state = cli_state.nodes.get(node_name)?; + let node_port = node_state + .config() + .setup() + .api_transport() + .ok() + .map(|listener| listener.addr.port()); - // Get list of services for the node - let services: ServiceList = node.ask(ctx, api::list_services()).await?; + let node_info = + if !is_node_up(ctx, node_name, node, cli_state.clone(), wait_until_ready).await? { + // it is expected to not be able to open an arbitrary TCP connection on an authority node + // so in that case we display an UP status + let is_authority_node = node_state.config().setup().authority_node.unwrap_or(false); + ShowNodeResponse::new(is_default, node_name, is_authority_node, node_port) + } else { + let mut node_info = ShowNodeResponse::new(is_default, node_name, true, node_port); - // Get list of TCP listeners for node - let tcp_listeners: TransportList = node.ask(ctx, api::list_tcp_listeners()).await?; + // Get short id for the node + node_info.identity = Some(match node_state.config().identity_config() { + Ok(resp) => resp.identifier().to_string(), + Err(_) => String::from("None"), + }); - // Get list of Secure Channel Listeners - let secure_channel_listeners: SecureChannelListenersList = - node.ask(ctx, api::list_secure_channel_listener()).await?; + // Get list of services for the node + node_info.service_list = Some(node.ask(ctx, api::list_services()).await?); - // Get list of inlets - let inlets: InletList = node.ask(ctx, api::list_inlets()).await?; + // Get list of TCP listeners for node + node_info.tcp_listeners = Some(node.ask(ctx, api::list_tcp_listeners()).await?); - // Get list of outlets - let outlets: OutletList = node.ask(ctx, api::list_outlets()).await?; + // Get list of Secure Channel Listeners + node_info.secure_channel_listeners = + Some(node.ask(ctx, api::list_secure_channel_listener()).await?); - let node_state = cli_state.nodes.get(node_name)?; - let node_port = node_state - .config() - .setup() - .api_transport() - .ok() - .map(|listener| listener.addr.port()); - - print_node_info( - opts, - node_port, - node_name, - is_default, - true, - Some(&default_id), - Some(&services), - Some(&tcp_listeners), - Some(&secure_channel_listeners), - Some((&inlets, &outlets)), - ); - } + // Get list of inlets + let inlets: InletList = node.ask(ctx, api::list_inlets()).await?; + node_info.inlets = Some(inlets); - Ok(()) + // Get list of outlets + let outlets: OutletList = node.ask(ctx, api::list_outlets()).await?; + node_info.outlets = Some(outlets); + + node_info + }; + + Ok(opts.global_args.output_format.println_value(&node_info)?) } /// Send message(s) to a node to determine if it is 'up' and @@ -305,3 +162,99 @@ pub async fn is_node_up( warn!(%node_name, "node didn't respond in time"); Ok(false) } + +impl<'a> Output for ShowNodeResponse<'a> { + fn output(&self) -> Result { + use std::fmt::Write; + let mut buffer = String::new(); + + writeln!(buffer, "Node:")?; + + if self.is_default { + writeln!(buffer, " Name: {} (default)", self.name)?; + } else { + writeln!(buffer, " Name: {}", self.name)?; + } + + writeln!( + buffer, + " Status: {}", + match self.is_up { + true => "UP".light_green(), + false => "DOWN".light_red(), + } + )?; + + writeln!(buffer, " Route To Node:")?; + if let Some(short) = &self.route.short { + writeln!(buffer, " Short: {short}")?; + } + if let Some(verbose) = &self.route.verbose { + writeln!(buffer, " Verbose: {verbose}")?; + } + + if let Some(identity) = &self.identity { + writeln!(buffer, " Identity: {}", identity)?; + } + + if let Some(list) = &self.tcp_listeners { + writeln!(buffer, " Transports:")?; + for e in &list.list { + writeln!(buffer, " Transport:")?; + writeln!(buffer, " Type: {}", &e.tt)?; + writeln!(buffer, " Mode: {}", &e.tm)?; + writeln!(buffer, " Socket: {}", &e.socket_addr)?; + writeln!(buffer, " Worker: {}", &e.worker_addr)?; + writeln!(buffer, " FlowControlId: {}", &e.flow_control_id)?; + } + } + + if let Some(list) = &self.secure_channel_listeners { + writeln!(buffer, " Secure Channel Listeners:")?; + for e in &list.list { + writeln!(buffer, " Listener:")?; + if let Some(ma) = addr_to_multiaddr(e.addr.clone()) { + writeln!(buffer, " Address: {ma}")?; + writeln!(buffer, " FlowControlId: {}", &e.flow_control_id)?; + } + } + } + + if let Some(inlets) = &self.inlets { + writeln!(buffer, " Inlets:")?; + for e in &inlets.list { + writeln!(buffer, " Inlet:")?; + writeln!(buffer, " Listen Address: {}", e.bind_addr)?; + if let Some(r) = Route::parse(&e.outlet_route) { + if let Some(ma) = route_to_multiaddr(&r) { + writeln!(buffer, " Route To Outlet: {ma}")?; + } + } + } + } + + if let Some(outlets) = &self.outlets { + writeln!(buffer, " Outlets:")?; + for e in &outlets.list { + writeln!(buffer, " Outlet:")?; + writeln!(buffer, " Forward Address: {}", e.socket_addr)?; + if let Some(ma) = addr_to_multiaddr(e.worker_addr.to_string()) { + writeln!(buffer, " Address: {ma}")?; + } + } + } + + if let Some(list) = &self.service_list { + writeln!(buffer, " Services:")?; + for e in &list.list { + writeln!(buffer, " Service:")?; + writeln!(buffer, " Type: {}", e.service_type)?; + if let Some(ma) = addr_to_multiaddr(e.addr.as_str()) { + writeln!(buffer, " Address: {ma}")?; + } + } + } + + Ok(buffer) + } +}