Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add listener unit tests #672

Merged
merged 1 commit into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions conmon-rs/server/src/attach.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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")?;

Expand Down
193 changes: 169 additions & 24 deletions conmon-rs/server/src/listener.rs
Original file line number Diff line number Diff line change
@@ -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(|_| 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());
}
}
4 changes: 3 additions & 1 deletion conmon-rs/server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions conmon-rs/server/src/terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down