refactor: type-safe sub-agent ID (#1618)

* refactor: type-safe sub-agent ID

There are (or will be) functions around that are intended to operate with a SubAgentID and should not **ever** receive the AC AgentID or any other reserved AgentID that we add in the future. With this we can be a bit more sure that we are using the correct IDs.

* style: remove `new_unchecked`

* style: remove `AgentID::is_valid_format`

Author: DavSanchez

agent-control/src/agent_control/agent_id.rs

@@ -15,52 +15,23 @@ const AGENT_ID_MAX_LENGTH: usize = 32;
 /// following [RFC 1035 Label names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names).
 pub enum AgentID {
     AgentControl,
-    SubAgent(String),
-}
-
-#[derive(Error, Debug)]
-pub enum AgentIDError {
-    #[error(
-        "AgentID must contain 32 characters at most, contain lowercase alphanumeric characters or dashes only, start with alphabetic, and end with alphanumeric"
-    )]
-    InvalidFormat,
-    #[error("AgentID '{0}' is reserved")]
-    Reserved(String),
+    SubAgent(SubAgentID),
 }
 
 impl AgentID {
     pub fn as_str(&self) -> &str {
         match self {
             Self::AgentControl => AGENT_CONTROL_ID,
-            Self::SubAgent(id) => id,
+            Self::SubAgent(id) => id.as_str(),
         }
     }
-
-    /// Checks if a string reference has valid format to build an [AgentID].
-    /// It follows [RFC 1035 Label names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names),
-    /// and sets a shorter maximum length to avoid issues when the agent-id is used to compose names.
-    fn is_valid_format(s: &str) -> bool {
-        s.len() <= AGENT_ID_MAX_LENGTH
-            && s.starts_with(|c: char| c.is_ascii_alphabetic())
-            && s.ends_with(|c: char| c.is_ascii_alphanumeric())
-            && s.chars()
-                .all(|c| c.eq(&'-') || c.is_ascii_digit() || c.is_ascii_lowercase())
-    }
 }
 
 impl TryFrom<String> for AgentID {
     type Error = AgentIDError;
     fn try_from(input: String) -> Result<Self, Self::Error> {
-        if RESERVED_AGENT_IDS
-            .iter()
-            .any(|id| input.eq_ignore_ascii_case(id))
-        {
-            Err(AgentIDError::Reserved(input))
-        } else if AgentID::is_valid_format(&input) {
-            Ok(AgentID::SubAgent(input))
-        } else {
-            Err(AgentIDError::InvalidFormat)
-        }
+        agent_id_not_reserved_and_valid(&input)?;
+        Ok(Self::SubAgent(SubAgentID(input)))
     }
 }
 
@@ -75,7 +46,7 @@ impl From<AgentID> for String {
     fn from(val: AgentID) -> Self {
         match val {
             AgentID::AgentControl => AGENT_CONTROL_ID.to_string(),
-            AgentID::SubAgent(id) => id,
+            AgentID::SubAgent(id) => String::from(id),
         }
     }
 }
@@ -93,6 +64,86 @@ impl AsRef<Path> for AgentID {
     }
 }
 
+/// Type with the same API as [`AgentID`], but used to represent only sub-agent IDs.
+#[derive(Debug, Deserialize, Serialize, PartialEq, Clone, Hash, Eq)]
+pub struct SubAgentID(String);
+
+impl SubAgentID {
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+}
+
+impl TryFrom<String> for SubAgentID {
+    type Error = AgentIDError;
+    fn try_from(input: String) -> Result<Self, Self::Error> {
+        agent_id_not_reserved_and_valid(&input)?;
+        Ok(Self(input))
+    }
+}
+
+impl TryFrom<&str> for SubAgentID {
+    type Error = AgentIDError;
+    fn try_from(input: &str) -> Result<Self, Self::Error> {
+        Self::try_from(input.to_string())
+    }
+}
+
+impl From<SubAgentID> for String {
+    fn from(val: SubAgentID) -> Self {
+        val.0
+    }
+}
+
+impl Display for SubAgentID {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_str())
+    }
+}
+
+impl AsRef<Path> for SubAgentID {
+    fn as_ref(&self) -> &Path {
+        // TODO: define how SubAgentID should be converted to a Path here.
+        Path::new(self.as_str())
+    }
+}
+
+#[derive(Error, Debug)]
+pub enum AgentIDError {
+    #[error(
+        "AgentID must contain 32 characters at most, contain lowercase alphanumeric characters or dashes only, start with alphabetic, and end with alphanumeric"
+    )]
+    InvalidFormat,
+    #[error("AgentID '{0}' is reserved")]
+    Reserved(String),
+}
+
+/// Checks if a string reference has valid format to build an [`AgentID`] or [`SubAgentID`].
+/// It follows [RFC 1035 Label names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names),
+/// and sets a shorter maximum length to avoid issues when the agent-id is used to compose names.
+fn agent_id_not_reserved_and_valid(s: impl AsRef<str>) -> Result<(), AgentIDError> {
+    let s = s.as_ref();
+    if RESERVED_AGENT_IDS
+        .iter()
+        .any(|id| s.eq_ignore_ascii_case(id))
+    {
+        Err(AgentIDError::Reserved(s.to_string()))
+    } else if agent_id_str_validation(s) {
+        Ok(())
+    } else {
+        Err(AgentIDError::InvalidFormat)
+    }
+}
+
+fn agent_id_str_validation(s: impl AsRef<str>) -> bool {
+    let s = s.as_ref();
+    s.len() <= AGENT_ID_MAX_LENGTH
+        && s.starts_with(|c: char| c.is_ascii_alphabetic())
+        && s.ends_with(|c: char| c.is_ascii_alphanumeric())
+        && s.chars()
+            .all(|c| c.eq(&'-') || c.is_ascii_digit() || c.is_ascii_lowercase())
+}
+
 #[cfg(test)]
 pub(crate) mod tests {
     use crate::agent_control::agent_id::AgentID;