Enforce naming conventions

ank/src/cli_commands/apply_manifests.rs

@@ -96,7 +96,16 @@ pub fn update_request_obj(
 ) -> Result<(), String> {
     for workload_path in paths.iter() {
         let workload_name = &workload_path.parts()[1];
-        let cur_workload_spec = cur_obj.get(workload_path).unwrap().clone();
+        cur_obj
+            .clone()
+            .check_if_provided_path_exists(workload_path)
+            .map_err(|err| {
+                format!(
+                    "Got error `{}`. This may be caused by improper naming. Ankaios supports names defined by '^[a-zA-Z0-9_-]+[a-zA-Z0-9_-]*$'.",
+                    err
+                )
+            })?;
+        let cur_workload_spec = cur_obj.get(workload_path).unwrap();
         if req_obj.get(workload_path).is_none() {
             let _ = req_obj.set(workload_path, cur_workload_spec.clone());
         } else {
@@ -779,4 +788,109 @@ mod tests {
             .await;
         assert!(apply_result.is_ok());
     }
+
+    #[tokio::test]
+    async fn utest_apply_manifest_invalid_names() {
+        let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC
+            .get_lock_async()
+            .await;
+
+        let manifest_content = io::Cursor::new(
+            b"apiVersion: \"v0.1\"\nworkloads:
+            simple.manifest1:
+              runtime: podman
+              agent: agent_A
+              runtimeConfig: \"\"
+                ",
+        );
+
+        let mut manifest_data = String::new();
+        let _ = manifest_content.clone().read_to_string(&mut manifest_data);
+
+        let updated_state = CompleteState {
+            desired_state: serde_yaml::from_str(&manifest_data).unwrap(),
+            ..Default::default()
+        };
+
+        let mut mock_server_connection = MockServerConnection::default();
+        mock_server_connection
+            .expect_update_state()
+            .with(
+                eq(updated_state.clone()),
+                eq(vec!["desiredState.workloads.simple.manifest1".to_string()]),
+            )
+            .return_once(|_, _| {
+                Ok(UpdateStateSuccess {
+                    added_workloads: vec!["simple_manifest1.abc.agent_B".to_string()],
+                    deleted_workloads: vec![],
+                })
+            });
+        mock_server_connection
+            .expect_get_complete_state()
+            .with(eq(vec![]))
+            .return_once(|_| {
+                Ok((ank_base::CompleteState::from(CompleteState {
+                    desired_state: updated_state.desired_state,
+                    ..Default::default()
+                }))
+                .into())
+            });
+        mock_server_connection
+            .expect_take_missed_from_server_messages()
+            .return_once(|| {
+                vec![
+                    FromServer::Response(ank_base::Response {
+                        request_id: OTHER_REQUEST_ID.into(),
+                        response_content: Some(ank_base::response::ResponseContent::Error(
+                            Default::default(),
+                        )),
+                    }),
+                    FromServer::UpdateWorkloadState(UpdateWorkloadState {
+                        workload_states: vec![WorkloadState {
+                            instance_name: "simple_manifest1.abc.agent_B".try_into().unwrap(),
+                            execution_state: ExecutionState {
+                                state: objects::ExecutionStateEnum::Running(RunningSubstate::Ok),
+                                ..Default::default()
+                            },
+                        }],
+                    }),
+                ]
+            });
+        mock_server_connection
+            .expect_read_next_update_workload_state()
+            .return_once(|| {
+                Ok(UpdateWorkloadState {
+                    workload_states: vec![WorkloadState {
+                        instance_name: "simple_manifest1.abc.agent_B".try_into().unwrap(),
+                        execution_state: ExecutionState {
+                            state: objects::ExecutionStateEnum::Running(RunningSubstate::Ok),
+                            ..Default::default()
+                        },
+                    }],
+                })
+            });
+
+        let mut cmd = CliCommands {
+            _response_timeout_ms: RESPONSE_TIMEOUT_MS,
+            no_wait: false,
+            server_connection: mock_server_connection,
+        };
+
+        FAKE_GET_INPUT_SOURCE_MOCK_RESULT_LIST
+            .lock()
+            .unwrap()
+            .push_back(Ok(vec![(
+                "manifest.yml".to_string(),
+                Box::new(manifest_content),
+            )]));
+
+        let apply_result = cmd
+            .apply_manifests(ApplyArgs {
+                agent_name: None,
+                delete_mode: false,
+                manifest_files: vec!["manifest_yaml".to_string()],
+            })
+            .await;
+        assert!(apply_result.is_err());
+    }
 }

common/Cargo.toml

@@ -23,6 +23,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_yaml = "0.9"
 log = "0.4"
 sha256 = "1.5"
+regex = "1.10"
 
 [features]
 default = []

common/src/objects/state.rs

@@ -12,6 +12,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
+use regex::Regex;
 use serde::{Deserialize, Serialize};
 
 use std::collections::HashMap;
@@ -22,6 +23,7 @@ use crate::objects::StoredWorkloadSpec;
 use api::ank_base;
 
 const CURRENT_API_VERSION: &str = "v0.1";
+const MAX_CHARACTERS_WORKLOAD_NAME: usize = 63;
 
 // [impl->swdd~common-object-representation~1]
 // [impl->swdd~common-object-serialization~1]
@@ -75,8 +77,41 @@ impl TryFrom<ank_base::State> for State {
 }
 
 impl State {
-    pub fn is_compatible_format(api_version: &String) -> bool {
-        api_version == CURRENT_API_VERSION
+    pub fn verify_format(provided_state: &State) -> Result<(), String> {
+        if provided_state.api_version != CURRENT_API_VERSION {
+            return Err(format!(
+                "Unsupported API version. Received '{}', expected '{}'",
+                provided_state.api_version,
+                State::default().api_version
+            ));
+        }
+
+        let re_workloads = Regex::new(r"^[a-zA-Z0-9_-]+[a-zA-Z0-9_-]*$").unwrap();
+        let re_agent = Regex::new(r"^[a-zA-Z0-9_-]*$").unwrap();
+
+        for (workload_name, workload_spec) in &provided_state.workloads {
+            if !re_workloads.is_match(workload_name.as_str()) {
+                return Err(format!(
+                    "Unsupported workload name. Received '{}', expected to have characters in ^[a-zA-Z0-9_-]+[a-zA-Z0-9_-]*$",
+                    workload_name
+                ));
+            }
+            if workload_name.len() > MAX_CHARACTERS_WORKLOAD_NAME {
+                return Err(format!(
+                    "Workload name length {} exceeds the maximum limit of {} characters",
+                    workload_name.len(),
+                    MAX_CHARACTERS_WORKLOAD_NAME
+                ));
+            }
+            if !re_agent.is_match(workload_spec.agent.as_str()) {
+                return Err(format!(
+                    "Unsupported agent name. Received '{}', expected to have characters in ^[a-zA-Z0-9_-]*$",
+                    workload_spec.agent
+                ));
+            }
+        }
+
+        Ok(())
     }
 }
 
@@ -94,12 +129,12 @@ impl State {
 #[cfg(test)]
 mod tests {
 
-    use std::collections::HashMap;
-
+    use super::{CURRENT_API_VERSION, MAX_CHARACTERS_WORKLOAD_NAME};
     use api::ank_base;
+    use std::collections::HashMap;
 
     use crate::{
-        objects::State,
+        objects::{State, StoredWorkloadSpec},
         test_utils::{generate_test_proto_state, generate_test_state},
     };
 
@@ -140,20 +175,72 @@ mod tests {
         let state_compatible_version = State {
             ..Default::default()
         };
-        assert!(State::is_compatible_format(
-            &state_compatible_version.api_version
-        ));
+        assert_eq!(State::verify_format(&state_compatible_version), Ok(()));
     }
 
     #[test]
-    fn utest_state_rejects_incompatible_state() {
+    fn utest_state_rejects_incompatible_state_on_api_version() {
+        let api_version = "incompatible_version".to_string();
         let state_incompatible_version = State {
-            api_version: "incompatible_version".to_string(),
+            api_version: api_version.clone(),
             ..Default::default()
         };
-        assert!(!State::is_compatible_format(
-            &state_incompatible_version.api_version
-        ));
+        assert_eq!(
+            State::verify_format(&state_incompatible_version),
+            Err(format!(
+                "Unsupported API version. Received '{}', expected '{}'",
+                api_version, CURRENT_API_VERSION
+            ))
+        );
+    }
+
+    #[test]
+    fn utest_state_rejects_incompatible_state_on_workload_name() {
+        let workload_name = "nginx.test".to_string();
+        let state_incompatible_version = State {
+            api_version: "v0.1".to_string(),
+            workloads: HashMap::from([(workload_name.clone(), StoredWorkloadSpec::default())]),
+        };
+        assert_eq!(State::verify_format(&state_incompatible_version), Err(format!("Unsupported workload name. Received '{}', expected to have characters in ^[a-zA-Z0-9_-]+[a-zA-Z0-9_-]*$", "nginx.test")));
+    }
+
+    #[test]
+    fn utest_state_rejects_incompatible_state_on_inordinately_long_workload_name() {
+        let workload_name = "workload_name_is_too_long_for_ankaios_to_accept_it_and_I_don_t_know_what_else_to_write".to_string();
+        let state_incompatible_version = State {
+            api_version: "v0.1".to_string(),
+            workloads: HashMap::from([(workload_name.clone(), StoredWorkloadSpec::default())]),
+        };
+        assert_eq!(
+            State::verify_format(&state_incompatible_version),
+            Err(format!(
+                "Workload name length {} exceeds the maximum limit of {} characters",
+                workload_name.len(),
+                MAX_CHARACTERS_WORKLOAD_NAME
+            ))
+        );
+    }
+
+    #[test]
+    fn utest_state_rejects_incompatible_state_on_agent_name() {
+        let agent_name = "agent_A.test".to_string();
+        let state_incompatible_version = State {
+            api_version: "v0.1".to_string(),
+            workloads: HashMap::from([(
+                "sample".to_string(),
+                StoredWorkloadSpec {
+                    agent: agent_name.clone(),
+                    ..Default::default()
+                },
+            )]),
+        };
+        assert_eq!(
+            State::verify_format(&state_incompatible_version),
+            Err(format!(
+                "Unsupported agent name. Received '{}', expected to have characters in ^[a-zA-Z0-9_-]*$",
+                agent_name
+            ))
+        );
     }
 
     #[test]

common/src/state_manipulation/object.rs

@@ -271,6 +271,15 @@ impl Object {
         }
         Some(current_obj)
     }
+
+    pub fn check_if_provided_path_exists(&mut self, path: &Path) -> Result<(), String> {
+        if self.get_as_mapping(path).is_none() {
+            return Err(
+                "The provided path could not be found as a mapping inside the object".to_string(),
+            );
+        }
+        Ok(())
+    }
 }
 
 //////////////////////////////////////////////////////////////////////////////

server/src/ankaios_server.rs

@@ -67,14 +67,7 @@ impl AnkaiosServer {
 
     pub async fn start(&mut self, startup_state: Option<CompleteState>) -> Result<(), String> {
         if let Some(state) = startup_state {
-            if !State::is_compatible_format(&state.desired_state.api_version) {
-                let message = format!(
-                    "Unsupported API version. Received '{}', expected '{}'",
-                    state.desired_state.api_version,
-                    State::default().api_version
-                );
-                return Err(message);
-            }
+            State::verify_format(&state.desired_state)?;
 
             match self.server_state.update(state, vec![]) {
                 Ok(Some((added_workloads, deleted_workloads))) => {
@@ -256,21 +249,13 @@ impl AnkaiosServer {
 
                         // [impl->swdd~update-desired-state-with-invalid-version~1]
                         // [impl->swdd~update-desired-state-with-missing-version~1]
-                        if !State::is_compatible_format(
-                            &update_state_request.state.desired_state.api_version,
-                        ) {
-                            log::warn!("The CompleteState in the request has wrong format. Received '{}', expected '{}' -> ignoring the request.",
-                                update_state_request.state.desired_state.api_version, State::default().api_version);
+                        if let Err(error_message) =
+                            State::verify_format(&update_state_request.state.desired_state)
+                        {
+                            log::warn!("The CompleteState in the request has wrong format. {} -> ignoring the request", error_message);
 
                             self.to_agents
-                                .error(
-                                    request_id,
-                                    format!(
-                                        "Unsupported API version. Received '{}', expected '{}'",
-                                        update_state_request.state.desired_state.api_version,
-                                        State::default().api_version
-                                    ),
-                                )
+                                .error(request_id, error_message)
                                 .await
                                 .unwrap_or_illegal_state();
                             continue;
@@ -432,7 +417,7 @@ mod tests {
 
         let startup_state = CompleteState {
             desired_state: State {
-                workloads: HashMap::from([("workload A".to_string(), workload)]),
+                workloads: HashMap::from([("workload_A".to_string(), workload)]),
                 ..Default::default()
             },
             ..Default::default()
@@ -496,7 +481,7 @@ mod tests {
         it contains a self cycle in the inter workload dependencies config */
         let mut updated_workload = generate_test_workload_spec_with_param(
             AGENT_A.to_string(),
-            "workload A".to_string(),
+            "workload_A".to_string(),
             RUNTIME_NAME.to_string(),
         );