Make control interface optional for a workload (#353)

* Fix failing stests

* First attempt at defining the necessary stests

* Revert "Fix failing stests"

This reverts commit 51a0287.

* Initial implementation of optional control interface

* Fix control interface path and creation logic

* Add tests for start-up and update without control interface

* Modify control interface optional stests

* Update: all three control interface system tests are running

* Update: Refactor the check of mount point creation

* Update: checking for all workloads of an agent

* Small update on workload indexing in mount point checking

* Remove unused variables

* Fix unit tests and other small changes

* Add more unit tests for control interface creation logic

* Add and link requirements

* Fix image registry from config

* Fix attempt: authorization - no rules system test

* Update control interface tester tag

* Update control interface stests for mount point existence verification

* Fix findings

* Check for no access to Control Interface when no rules are being defined

* Fix requirement findings

---------

Co-authored-by: Tomuta Gabriel <gaby_unalaq@yahoo.com>
Co-authored-by: Tomuta Gabriel <gabriel.tomuta@elektrobit.com>

agent/doc/swdesign/README.md

@@ -543,6 +543,27 @@ Needs:
 - impl
 - utest
 
+#### Control Interface created for eligible workloads
+`swdd~agent-control-interface-created-for-eligible-workloads~1`
+
+Status: approved
+
+When the workload has control interface access rules configured, the Control Interface shall be created for that workload.
+
+Comment:
+Due to the logic that by default, the pipes are restricted, it makes sense to check only the allowed rules.
+
+Rationale:
+Creating a control interface affects the start-up time of a workload and thus it should be created only if it's used.
+
+Tags:
+- ControlInterface
+
+Needs:
+- impl
+- utest
+- stest
+
 #### Agent skips unknown runtime
 `swdd~agent-skips-unknown-runtime~1`
 
@@ -658,7 +679,7 @@ Status: approved
 
 When the RuntimeFacade gets a requests to create a workload, the RuntimeFacade shall:
 * start the WorkloadControlLoop waiting for WorkloadCommands
-* create a new ControlInterface instance for the new workload
+* create a new ControlInterface instance for the new workload if the workload has access rules configured
 * request the create of the workload by sending a create command to the WorkloadControlLoop
 * return a new workload object containing a WorkloadCommandSender to communicate with the WorkloadControlLoop
 
@@ -866,8 +887,8 @@ Status: approved
 When the WorkloadObject receives a trigger to update the workload, it:
 * triggers a comparison of the existing and new control interface metadata
 * stops the old control interface if the comparison returns that the metadata has changed
-* creates a new ControlInterface instance if the comparison returns that the metadata has changed
-* stores the new ControlInterface instance after the creation
+* creates a new ControlInterface instance if the comparison returns that the metadata has changed and if access rules are configured
+* stores the new ControlInterface instance instead of the old one
 * sends a command via the WorkloadCommandSender to the WorkloadControlLoop to update the workload
 
 Tags:

agent/src/control_interface/control_interface_info.rs

@@ -26,10 +26,10 @@ use crate::control_interface::ControlInterface;
 
 pub struct ControlInterfaceInfo {
     run_folder: PathBuf,
-    pub workload_instance_name: WorkloadInstanceName,
+    workload_instance_name: WorkloadInstanceName,
     #[cfg_attr(test, allow(dead_code))]
-    pub control_interface_to_server_sender: ToServerSender,
-    pub authorizer: Authorizer,
+    control_interface_to_server_sender: ToServerSender,
+    authorizer: Authorizer,
 }
 
 #[cfg_attr(test, automock)]

agent/src/runtime_connectors/runtime_facade.rs

@@ -44,7 +44,7 @@ pub trait RuntimeFacade: Send + Sync + 'static {
     fn resume_workload(
         &self,
         runtime_workload: WorkloadSpec,
-        control_interface: Option<ControlInterface>,
+        control_interface: Option<ControlInterfaceInfo>,
         update_state_tx: &WorkloadStateSender,
     ) -> Workload;
 
@@ -112,13 +112,13 @@ impl<
     fn resume_workload(
         &self,
         workload_spec: WorkloadSpec,
-        control_interface: Option<ControlInterface>,
+        control_interface_info: Option<ControlInterfaceInfo>,
         update_state_tx: &WorkloadStateSender,
     ) -> Workload {
         let (_task_handle, workload) = Self::resume_workload_non_blocking(
             self,
             workload_spec,
-            control_interface,
+            control_interface_info,
             update_state_tx,
         );
         workload
@@ -155,39 +155,43 @@ impl<
     ) -> (JoinHandle<()>, Workload) {
         let runtime = self.runtime.to_owned();
         let update_state_tx = update_state_tx.clone();
+        let workload_name = workload_spec.instance_name.workload_name().to_owned();
 
         let (control_interface_path, control_interface) = if let Some(info) = control_interface_info
         {
             let run_folder = info.get_run_folder().clone();
             let output_pipe_sender = info.get_to_server_sender();
             let instance_name = info.get_instance_name().clone();
             let authorizer = info.move_authorizer();
-            let control_interface = match ControlInterface::new(
-                &run_folder,
-                &instance_name,
-                output_pipe_sender,
-                authorizer,
-            ) {
-                Ok(control_interface) => Some(control_interface),
+            match ControlInterface::new(&run_folder, &instance_name, output_pipe_sender, authorizer)
+            {
+                Ok(result) => {
+                    log::info!(
+                        "Successfully created control interface for workload '{}'.",
+                        workload_name
+                    );
+                    (
+                        Some(workload_spec.instance_name.pipes_folder_name(&run_folder)),
+                        Some(result),
+                    )
+                }
                 Err(err) => {
                     log::warn!(
                         "Could not create control interface when creating workload '{}': '{}'",
-                        instance_name,
+                        workload_name,
                         err
                     );
-                    None
+                    (None, None)
                 }
-            };
-
-            (
-                Some(workload_spec.instance_name.pipes_folder_name(&run_folder)),
-                control_interface,
-            )
+            }
         } else {
+            log::debug!(
+                "Skipping creation of control interface for workload '{}'.",
+                workload_name
+            );
             (None, None)
         };
 
-        let workload_name = workload_spec.instance_name.workload_name().to_owned();
         log::debug!(
             "Creating '{}' workload '{}'.",
             runtime.name(),
@@ -226,7 +230,7 @@ impl<
     fn resume_workload_non_blocking(
         &self,
         workload_spec: WorkloadSpec,
-        control_interface: Option<ControlInterface>,
+        control_interface_info: Option<ControlInterfaceInfo>,
         update_state_tx: &WorkloadStateSender,
     ) -> (JoinHandle<()>, Workload) {
         let workload_name = workload_spec.instance_name.workload_name().to_owned();
@@ -239,6 +243,32 @@ impl<
             workload_name,
         );
 
+        // [impl->swdd~agent-control-interface-created-for-eligible-workloads~1]
+        let control_interface = control_interface_info.and_then(|info| { if workload_spec.needs_control_interface() {
+            let run_folder = info.get_run_folder().clone();
+            let output_pipe_sender = info.get_to_server_sender();
+            let instance_name = info.get_instance_name().clone();
+            let authorizer = info.move_authorizer();
+            match ControlInterface::new(&run_folder, &instance_name, output_pipe_sender, authorizer)
+            {
+                Ok(result) => Some(result),
+                Err(err) => {
+                    log::warn!(
+                                "Could not reuse or create control interface when resuming workload '{}': '{}'",
+                                workload_spec.instance_name,
+                                err
+                            );
+                    None
+                }
+            }
+        } else {
+            log::info!(
+                    "No control interface access rights specified for workload '{}'. Skipping creation of control interface.",
+                    workload_spec.instance_name.clone().workload_name()
+                );
+            None
+        }});
+
         let (workload_command_tx, workload_command_receiver) = WorkloadCommandSender::new();
         let workload_command_sender = workload_command_tx.clone();
         let task_handle = tokio::spawn(async move {
@@ -335,7 +365,9 @@ impl<
 #[cfg(test)]
 mod tests {
     use common::objects::{
-        generate_test_workload_spec_with_param, ExecutionState, WorkloadInstanceName, WorkloadState,
+        generate_test_workload_spec_with_control_interface_access,
+        generate_test_workload_spec_with_param, ExecutionState, WorkloadInstanceName,
+        WorkloadState,
     };
 
     use crate::{
@@ -408,7 +440,7 @@ mod tests {
             .get_lock_async()
             .await;
 
-        let workload_spec = generate_test_workload_spec_with_param(
+        let workload_spec = generate_test_workload_spec_with_control_interface_access(
             AGENT_NAME.to_string(),
             WORKLOAD_1_NAME.to_string(),
             RUNTIME_NAME.to_string(),
@@ -480,13 +512,93 @@ mod tests {
     }
 
     // [utest->swdd~agent-resume-workload~2]
+    // [utest->swdd~agent-control-interface-created-for-eligible-workloads~1]
     #[tokio::test]
-    async fn utest_runtime_facade_resume_workload() {
+    async fn utest_runtime_facade_resume_workload_with_control_interface_access() {
         let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC
             .get_lock_async()
             .await;
 
-        let control_interface_mock = MockControlInterface::default();
+        let mut control_interface_info_mock = MockControlInterfaceInfo::default();
+        control_interface_info_mock
+            .expect_get_run_folder()
+            .once()
+            .return_const(PIPES_LOCATION.into());
+        control_interface_info_mock
+            .expect_get_to_server_sender()
+            .once()
+            .return_const(tokio::sync::mpsc::channel::<common::to_server_interface::ToServer>(1).0);
+        control_interface_info_mock
+            .expect_get_instance_name()
+            .once()
+            .return_const(
+                WorkloadInstanceName::builder()
+                    .workload_name(WORKLOAD_1_NAME)
+                    .build(),
+            );
+        control_interface_info_mock
+            .expect_move_authorizer()
+            .once()
+            .return_once(MockAuthorizer::default);
+
+        let control_interface_new_context = MockControlInterface::new_context();
+        control_interface_new_context
+            .expect()
+            .once()
+            .return_once(|_, _, _, _| Ok(MockControlInterface::default()));
+
+        let workload_spec = generate_test_workload_spec_with_control_interface_access(
+            AGENT_NAME.to_string(),
+            WORKLOAD_1_NAME.to_string(),
+            RUNTIME_NAME.to_string(),
+        );
+
+        let (wl_state_sender, _wl_state_receiver) =
+            tokio::sync::mpsc::channel(TEST_CHANNEL_BUFFER_SIZE);
+
+        let mock_control_loop = MockWorkloadControlLoop::run_context();
+        mock_control_loop
+            .expect()
+            .once()
+            .return_once(|_: ControlLoopState<String, StubStateChecker>| ());
+
+        let mock_workload = MockWorkload::default();
+        let new_workload_context = MockWorkload::new_context();
+        new_workload_context
+            .expect()
+            .once()
+            .return_once(|_, _, _| mock_workload);
+
+        let runtime_mock = MockRuntimeConnector::new();
+
+        let ownable_runtime_mock: Box<dyn OwnableRuntime<String, StubStateChecker>> =
+            Box::new(runtime_mock.clone());
+        let test_runtime_facade = Box::new(GenericRuntimeFacade::<String, StubStateChecker>::new(
+            ownable_runtime_mock,
+        ));
+
+        let (task_handle, _workload) = test_runtime_facade.resume_workload_non_blocking(
+            workload_spec.clone(),
+            Some(control_interface_info_mock),
+            &wl_state_sender,
+        );
+
+        tokio::task::yield_now().await;
+        assert!(task_handle.await.is_ok());
+        runtime_mock.assert_all_expectations().await;
+    }
+
+    // [utest->swdd~agent-control-interface-created-for-eligible-workloads~1]
+    #[tokio::test]
+    async fn utest_runtime_facade_resume_workload_without_control_interface_access() {
+        let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC
+            .get_lock_async()
+            .await;
+
+        let control_interface_info_mock = MockControlInterfaceInfo::default();
+
+        let control_interface_new_context = MockControlInterface::new_context();
+        control_interface_new_context.expect().never();
 
         let workload_spec = generate_test_workload_spec_with_param(
             AGENT_NAME.to_string(),
@@ -520,7 +632,7 @@ mod tests {
 
         let (task_handle, _workload) = test_runtime_facade.resume_workload_non_blocking(
             workload_spec.clone(),
-            Some(control_interface_mock),
+            Some(control_interface_info_mock),
             &wl_state_sender,
         );