Add listener unit tests

Increase the test coverage of the listener module by adding unit tests.

Signed-off-by: Sascha Grunert <[email protected]>

conmon-rs/server/src/attach.rs

@@ -1,4 +1,7 @@
-use crate::{container_io::Pipe, listener};
+use crate::{
+    container_io::Pipe,
+    listener::{DefaultListener, Listener},
+};
 use anyhow::{bail, Context, Result};
 use nix::{
     errno::Errno,
@@ -128,7 +131,8 @@ impl Attach {
         .context("bind socket")?;
 
         // keep parent_fd in scope until the bind, or else the socket will not work
-        let (shortened_path, _parent_dir) = listener::shorten_socket_path(path)?;
+        let (shortened_path, _parent_dir) =
+            Listener::<DefaultListener>::default().shorten_socket_path(path)?;
         let addr = UnixAddr::new(&shortened_path).context("create socket addr")?;
         bind(fd, &addr).context("bind socket fd")?;
 

conmon-rs/server/src/listener.rs

@@ -1,34 +1,179 @@
 use anyhow::{Context, Result};
 use std::{
-    fs,
+    fs::{self, File},
+    io,
     os::unix::io::AsRawFd,
     path::{Path, PathBuf},
 };
 use tokio::net::UnixListener;
 
-pub fn bind_long_path(path: &Path) -> Result<UnixListener> {
-    // keep parent_fd in scope until the bind, or else the socket will not work
-    let (path, _parent_dir) = shorten_socket_path(path)?;
-    UnixListener::bind(&path).context("bind server socket")
+#[cfg(test)]
+use mockall::{automock, predicate::*};
+
+#[derive(Debug, Default)]
+/// The main structure for this module.
+pub struct Listener<T> {
+    imp: T,
+}
+
+impl<T> Listener<T>
+where
+    T: ListenerImpl,
+{
+    pub fn bind_long_path<P>(&self, path: P) -> Result<UnixListener>
+    where
+        P: AsRef<Path>,
+    {
+        // keep parent_fd in scope until the bind, or else the socket will not work
+        let (path, _parent_dir) = self.shorten_socket_path(path)?;
+        self.imp.bind(path.as_ref()).context("bind server socket")
+    }
+
+    pub fn shorten_socket_path<P>(&self, path: P) -> Result<(PathBuf, File)>
+    where
+        P: AsRef<Path>,
+    {
+        let path = path.as_ref();
+
+        let parent = path.parent().context(format!(
+            "tried to specify / as socket to bind to: {}",
+            path.display()
+        ))?;
+        let name = path.file_name().context(format!(
+            "tried to specify '..' as socket to bind to: {}",
+            path.display(),
+        ))?;
+
+        self.imp
+            .create_dir_all(parent)
+            .context("create parent directory")?;
+
+        let parent = self.imp.open(parent).context("open parent directory")?;
+        let fd = parent.as_raw_fd();
+
+        Ok((
+            PathBuf::from("/proc/self/fd")
+                .join(fd.to_string())
+                .join(name),
+            parent,
+        ))
+    }
+}
+
+#[cfg_attr(test, automock)]
+pub trait ListenerImpl {
+    fn bind(&self, path: &Path) -> io::Result<UnixListener>;
+    fn create_dir_all(&self, path: &Path) -> io::Result<()>;
+    fn open(&self, path: &Path) -> io::Result<File>;
 }
 
-pub fn shorten_socket_path(path: &Path) -> Result<(PathBuf, fs::File)> {
-    let parent = path.parent().context(format!(
-        "tried to specify / as socket to bind to: {}",
-        path.display()
-    ))?;
-    let name = path.file_name().context(format!(
-        "tried to specify '..' as socket to bind to: {}",
-        path.display(),
-    ))?;
-
-    fs::create_dir_all(parent).context("create parent directory")?;
-    let parent = fs::File::open(parent).context("open parent directory")?;
-    let fd = parent.as_raw_fd();
-    Ok((
-        PathBuf::from("/proc/self/fd")
-            .join(fd.to_string())
-            .join(name),
-        parent,
-    ))
+#[derive(Debug, Default)]
+/// The default implementation for the Listener.
+pub struct DefaultListener;
+
+impl ListenerImpl for DefaultListener {
+    fn bind(&self, path: &Path) -> io::Result<UnixListener> {
+        UnixListener::bind(&path)
+    }
+
+    fn create_dir_all(&self, path: &Path) -> io::Result<()> {
+        fs::create_dir_all(path)
+    }
+
+    fn open(&self, path: &Path) -> io::Result<File> {
+        File::open(path)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::io::ErrorKind;
+    use tempfile::{tempdir, tempfile};
+
+    fn new_sut(mock: MockListenerImpl) -> Listener<MockListenerImpl> {
+        Listener::<MockListenerImpl> { imp: mock }
+    }
+
+    fn permission_denied<T>() -> Result<T, io::Error> {
+        Err(io::Error::new(ErrorKind::PermissionDenied, ""))
+    }
+
+    #[tokio::test]
+    async fn bind_long_path_success() -> Result<()> {
+        let mut mock = MockListenerImpl::new();
+
+        mock.expect_create_dir_all().returning(|_| Ok(()));
+        mock.expect_open().returning(|_| tempfile());
+        mock.expect_bind()
+            .returning(|_| Ok(UnixListener::bind(tempdir()?.path().join("foo"))?));
+
+        let sut = new_sut(mock);
+        let first = "foo";
+        let listener = sut.bind_long_path(PathBuf::from(first).join("bar"))?;
+
+        let addr = listener.local_addr()?;
+        assert!(addr.as_pathname().context("no path name")?.ends_with(first));
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn bind_long_path_failure_on_bind() {
+        let mut mock = MockListenerImpl::new();
+
+        mock.expect_create_dir_all().returning(|_| Ok(()));
+        mock.expect_open().returning(|_| tempfile());
+        mock.expect_bind().returning(|_| permission_denied());
+
+        let sut = new_sut(mock);
+        assert!(sut
+            .bind_long_path(PathBuf::from("foo").join("bar"))
+            .is_err());
+    }
+
+    #[test]
+    fn shorten_socket_path_success() -> Result<()> {
+        let mut mock = MockListenerImpl::new();
+
+        mock.expect_create_dir_all().returning(|_| Ok(()));
+        mock.expect_open().returning(|_| tempfile());
+
+        let sut = new_sut(mock);
+        let last = "bar";
+        let (res_file_path, res_parent) =
+            sut.shorten_socket_path(PathBuf::from("/foo").join(last))?;
+
+        assert!(res_file_path.ends_with(last));
+        assert!(res_file_path
+            .display()
+            .to_string()
+            .contains(&res_parent.as_raw_fd().to_string()));
+
+        Ok(())
+    }
+
+    #[test]
+    fn shorten_socket_path_failure_on_open() {
+        let mut mock = MockListenerImpl::new();
+
+        mock.expect_create_dir_all().returning(|_| Ok(()));
+        mock.expect_open().returning(|_| permission_denied());
+
+        let sut = new_sut(mock);
+
+        assert!(sut.shorten_socket_path("/foo/bar").is_err());
+    }
+
+    #[test]
+    fn shorten_socket_path_failure_on_create_dir_all() {
+        let mut mock = MockListenerImpl::new();
+
+        mock.expect_create_dir_all()
+            .returning(|_| permission_denied());
+
+        let sut = new_sut(mock);
+
+        assert!(sut.shorten_socket_path("/foo/bar").is_err());
+    }
 }

conmon-rs/server/src/server.rs

@@ -5,6 +5,7 @@ use crate::{
     config::{CgroupManager, Config, LogDriver},
     container_io::{ContainerIO, ContainerIOType},
     init::{DefaultInit, Init},
+    listener::{DefaultListener, Listener},
     version::Version,
 };
 use anyhow::{format_err, Context, Result};
@@ -194,7 +195,8 @@ impl Server {
     }
 
     async fn start_backend(self, mut shutdown_rx: oneshot::Receiver<()>) -> Result<()> {
-        let listener = crate::listener::bind_long_path(&self.config().socket())?;
+        let listener =
+            Listener::<DefaultListener>::default().bind_long_path(&self.config().socket())?;
         let client: conmon::Client = capnp_rpc::new_client(self);
 
         loop {

conmon-rs/server/src/terminal.rs

@@ -4,7 +4,7 @@ use crate::{
     attach::SharedContainerAttach,
     container_io::{ContainerIO, Message, Pipe},
     container_log::SharedContainerLog,
-    listener,
+    listener::{DefaultListener, Listener},
 };
 use anyhow::{bail, format_err, Context, Result};
 use getset::{Getters, MutGetters, Setters};
@@ -137,7 +137,7 @@ impl Terminal {
     ) -> Result<()> {
         let path = config.path();
         debug!("Listening terminal socket on {}", path.display());
-        let listener = listener::bind_long_path(path)?;
+        let listener = Listener::<DefaultListener>::default().bind_long_path(path)?;
 
         // Update the permissions
         let mut perms = fs::metadata(path).await?.permissions();