From a9afa5d7e40c59f85993fdb86b2590bab7f0f0af 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 | 69 +++++++++---------- .../rust/ockam/ockam_api/src/util.rs | 5 +- .../rust/ockam/ockam_app/src/app/state/mod.rs | 2 +- .../ockam/ockam_command/src/node/create.rs | 9 ++- .../rust/ockam/ockam_command/src/node/util.rs | 7 +- 5 files changed, 42 insertions(+), 50 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..45a4c349d88 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,7 +223,6 @@ impl NodeManager { pub struct NodeManagerGeneralOptions { cli_state: CliState, node_name: String, - skip_defaults: bool, pre_trusted_identities: Option, } @@ -232,13 +230,11 @@ impl NodeManagerGeneralOptions { pub fn new( cli_state: CliState, node_name: String, - skip_defaults: bool, pre_trusted_identities: Option, ) -> Self { Self { cli_state, node_name, - skip_defaults, pre_trusted_identities, } } @@ -339,7 +335,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,12 +350,9 @@ 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?; } info!("created a node manager for the node: {}", s.node_name); @@ -381,7 +373,7 @@ impl NodeManager { Ok(()) } - async fn initialize_defaults( + async fn initialize_default_services( &mut self, ctx: &Context, api_flow_control_id: &FlowControlId, @@ -424,6 +416,33 @@ impl NodeManager { Ok(()) } + pub 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 +849,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 f653370a538..bc660d9d4ca 100644 --- a/implementations/rust/ockam/ockam_api/src/util.rs +++ b/implementations/rust/ockam/ockam_api/src/util.rs @@ -442,9 +442,9 @@ pub mod test_utils { let node_config = NodeConfig::try_from(&cli_state).unwrap(); cli_state.nodes.create(&node_name, node_config)?; - let node_manager = NodeManager::create( + let mut node_manager = NodeManager::create( context, - NodeManagerGeneralOptions::new(cli_state.clone(), node_name, false, None), + NodeManagerGeneralOptions::new(cli_state.clone(), node_name, None), NodeManagerTransportOptions::new( FlowControls::generate_flow_control_id(), // FIXME tcp.async_try_clone().await?, @@ -461,6 +461,7 @@ pub mod test_utils { ) .await?; + node_manager.initialize_services(context, true).await?; let node_manager_worker = NodeManagerWorker::new(node_manager); let node_manager = node_manager_worker.inner().clone(); let secure_channels = node_manager.read().await.secure_channels.clone(); 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..1ccd6367b6b 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), 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 f5bc6f96bc7..6195e1dd2e0 100644 --- a/implementations/rust/ockam/ockam_command/src/node/create.rs +++ b/implementations/rust/ockam/ockam_command/src/node/create.rs @@ -302,12 +302,11 @@ async fn run_foreground_node( let pre_trusted_identities = load_pre_trusted_identities(&cmd)?; - let node_man = NodeManager::create( + let mut node_man = NodeManager::create( &ctx, NodeManagerGeneralOptions::new( opts.state.clone(), cmd.node_name.clone(), - cmd.launch_config.is_some(), pre_trusted_identities, ), NodeManagerTransportOptions::new( @@ -318,8 +317,12 @@ async fn run_foreground_node( ) .await .into_diagnostic()?; - let node_manager_worker = NodeManagerWorker::new(node_man); + node_man + .initialize_services(&ctx, cmd.launch_config.is_none()) + .await + .into_diagnostic()?; + 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..04fdf94ca27 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), NodeManagerTransportOptions::new(listener.flow_control_id().clone(), tcp), NodeManagerTrustOptions::new(trust_context_config), )