From fc6408cac4514e724a4d1cab6c4271e5321ba19e Mon Sep 17 00:00:00 2001 From: etorreborre Date: Fri, 29 Sep 2023 19:06:45 +0200 Subject: [PATCH] refactor(rust): remove the need to keep a flag to skip defaults the only case where it is needed is when starting a new node and this can be determined by the presence / absence of a launch config file --- .../rust/ockam/ockam_api/src/nodes/service.rs | 75 +++++++++---------- .../rust/ockam/ockam_api/src/util.rs | 2 +- .../rust/ockam/ockam_app/src/app/state/mod.rs | 2 +- .../ockam/ockam_command/src/node/create.rs | 4 +- .../rust/ockam/ockam_command/src/node/util.rs | 7 +- 5 files changed, 42 insertions(+), 48 deletions(-) diff --git a/implementations/rust/ockam/ockam_api/src/nodes/service.rs b/implementations/rust/ockam/ockam_api/src/nodes/service.rs index b462df1f8de..34743f6c711 100644 --- a/implementations/rust/ockam/ockam_api/src/nodes/service.rs +++ b/implementations/rust/ockam/ockam_api/src/nodes/service.rs @@ -90,7 +90,6 @@ pub struct NodeManager { api_transport_flow_control_id: FlowControlId, pub(crate) tcp_transport: TcpTransport, pub(crate) controller_identity_id: Identifier, - skip_defaults: bool, enable_credential_checks: bool, identifier: Identifier, pub(crate) secure_channels: Arc, @@ -224,22 +223,22 @@ impl NodeManager { pub struct NodeManagerGeneralOptions { cli_state: CliState, node_name: String, - skip_defaults: bool, pre_trusted_identities: Option, + start_default_services: bool, } impl NodeManagerGeneralOptions { pub fn new( cli_state: CliState, node_name: String, - skip_defaults: bool, pre_trusted_identities: Option, + start_default_services: bool, ) -> Self { Self { cli_state, node_name, - skip_defaults, pre_trusted_identities, + start_default_services, } } } @@ -339,7 +338,6 @@ impl NodeManager { api_transport_flow_control_id: transport_options.api_transport_flow_control_id, tcp_transport: transport_options.tcp_transport, controller_identity_id: Self::load_controller_identifier()?, - skip_defaults: general_options.skip_defaults, enable_credential_checks: trust_options.trust_context_config.is_some() && trust_options .trust_context_config @@ -355,13 +353,13 @@ impl NodeManager { policies, }; - if !general_options.skip_defaults { - debug!("starting default services"); - if let Some(tc) = trust_options.trust_context_config { - debug!("configuring trust context"); - s.configure_trust_context(&tc).await?; - } + if let Some(tc) = trust_options.trust_context_config { + debug!("configuring trust context"); + s.configure_trust_context(&tc).await?; } + + s.initialize_services(ctx, general_options.start_default_services) + .await?; info!("created a node manager for the node: {}", s.node_name); Ok(s) @@ -381,7 +379,7 @@ impl NodeManager { Ok(()) } - async fn initialize_defaults( + async fn initialize_default_services( &mut self, ctx: &Context, api_flow_control_id: &FlowControlId, @@ -424,6 +422,33 @@ impl NodeManager { Ok(()) } + async fn initialize_services( + &mut self, + ctx: &Context, + start_default_services: bool, + ) -> Result<()> { + let api_flow_control_id = self.api_transport_flow_control_id.clone(); + + if start_default_services { + self.initialize_default_services(ctx, &api_flow_control_id) + .await?; + } + + // Always start the echoer service as ockam_api::Medic assumes it will be + // started unconditionally on every node. It's used for liveliness checks. + ctx.flow_controls() + .add_consumer(DefaultAddress::ECHO_SERVICE, &api_flow_control_id); + self.start_echoer_service_impl(ctx, DefaultAddress::ECHO_SERVICE.into()) + .await?; + + ctx.flow_controls() + .add_consumer(DefaultAddress::RPC_PROXY, &api_flow_control_id); + ctx.start_worker(DefaultAddress::RPC_PROXY, RpcProxyService::new()) + .await?; + + Ok(()) + } + /// Resolve project ID (if any), create secure channel (if needed) and create a tcp connection /// Returns [`ConnectionInstance`] pub(crate) async fn connect( @@ -830,32 +855,6 @@ impl Worker for NodeManagerWorker { type Message = Vec; type Context = Context; - async fn initialize(&mut self, ctx: &mut Context) -> Result<()> { - let mut node_manager = self.node_manager.write().await; - let api_flow_control_id = node_manager.api_transport_flow_control_id.clone(); - - if !node_manager.skip_defaults { - node_manager - .initialize_defaults(ctx, &api_flow_control_id) - .await?; - } - - // Always start the echoer service as ockam_api::Medic assumes it will be - // started unconditionally on every node. It's used for liveliness checks. - ctx.flow_controls() - .add_consumer(DefaultAddress::ECHO_SERVICE, &api_flow_control_id); - node_manager - .start_echoer_service_impl(ctx, DefaultAddress::ECHO_SERVICE.into()) - .await?; - - ctx.flow_controls() - .add_consumer(DefaultAddress::RPC_PROXY, &api_flow_control_id); - ctx.start_worker(DefaultAddress::RPC_PROXY, RpcProxyService::new()) - .await?; - - Ok(()) - } - async fn shutdown(&mut self, ctx: &mut Self::Context) -> Result<()> { let node_manager = self.node_manager.read().await; node_manager.medic_handle.stop_medic(ctx).await diff --git a/implementations/rust/ockam/ockam_api/src/util.rs b/implementations/rust/ockam/ockam_api/src/util.rs index 0504ee0c238..e0e5f8e5d44 100644 --- a/implementations/rust/ockam/ockam_api/src/util.rs +++ b/implementations/rust/ockam/ockam_api/src/util.rs @@ -446,7 +446,7 @@ pub mod test_utils { let node_manager = NodeManager::create( context, - NodeManagerGeneralOptions::new(cli_state.clone(), node_name, false, None), + NodeManagerGeneralOptions::new(cli_state.clone(), node_name, None, true), NodeManagerTransportOptions::new( FlowControls::generate_flow_control_id(), // FIXME tcp.async_try_clone().await?, diff --git a/implementations/rust/ockam/ockam_app/src/app/state/mod.rs b/implementations/rust/ockam/ockam_app/src/app/state/mod.rs index 85fc68fe488..3f8ef55d36b 100644 --- a/implementations/rust/ockam/ockam_app/src/app/state/mod.rs +++ b/implementations/rust/ockam/ockam_app/src/app/state/mod.rs @@ -314,7 +314,7 @@ pub(crate) async fn make_node_manager_worker( let node_manager = NodeManager::create( &ctx, - NodeManagerGeneralOptions::new(cli_state.clone(), NODE_NAME.to_string(), false, None), + NodeManagerGeneralOptions::new(cli_state.clone(), NODE_NAME.to_string(), None, true), NodeManagerTransportOptions::new(listener.flow_control_id().clone(), tcp), NodeManagerTrustOptions::new(trust_context_config), ) diff --git a/implementations/rust/ockam/ockam_command/src/node/create.rs b/implementations/rust/ockam/ockam_command/src/node/create.rs index fbc3826606a..25f50cf28fd 100644 --- a/implementations/rust/ockam/ockam_command/src/node/create.rs +++ b/implementations/rust/ockam/ockam_command/src/node/create.rs @@ -307,8 +307,8 @@ async fn run_foreground_node( NodeManagerGeneralOptions::new( opts.state.clone(), cmd.node_name.clone(), - cmd.launch_config.is_some(), pre_trusted_identities, + cmd.launch_config.is_none(), ), NodeManagerTransportOptions::new( listener.flow_control_id().clone(), @@ -318,8 +318,8 @@ async fn run_foreground_node( ) .await .into_diagnostic()?; - let node_manager_worker = NodeManagerWorker::new(node_man); + let node_manager_worker = NodeManagerWorker::new(node_man); ctx.flow_controls() .add_consumer(NODEMANAGER_ADDR, listener.flow_control_id()); ctx.start_worker(NODEMANAGER_ADDR, node_manager_worker) diff --git a/implementations/rust/ockam/ockam_command/src/node/util.rs b/implementations/rust/ockam/ockam_command/src/node/util.rs index 72272124039..2a9386775fd 100644 --- a/implementations/rust/ockam/ockam_command/src/node/util.rs +++ b/implementations/rust/ockam/ockam_command/src/node/util.rs @@ -73,12 +73,7 @@ pub async fn start_embedded_node_with_vault_and_identity( let node_man = NodeManager::create( ctx, - NodeManagerGeneralOptions::new( - cli_state.clone(), - cmd.node_name.clone(), - cmd.launch_config.is_some(), - None, - ), + NodeManagerGeneralOptions::new(cli_state.clone(), cmd.node_name.clone(), None, true), NodeManagerTransportOptions::new(listener.flow_control_id().clone(), tcp), NodeManagerTrustOptions::new(trust_context_config), )