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

prepare to use system call mocks in unit tests #304

Merged
merged 1 commit into from
Sep 18, 2021
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
4 changes: 2 additions & 2 deletions src/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ impl CapabilityExt for SpecCapability {
/// reset capabilities of process calling this to effective capabilities
/// effective capability set is set of capabilities used by kernel to perform checks
/// see https://man7.org/linux/man-pages/man7/capabilities.7.html for more information
pub fn reset_effective(syscall: &impl Syscall) -> Result<()> {
pub fn reset_effective<S: Syscall + ?Sized>(syscall: &S) -> Result<()> {
log::debug!("reset all caps");
syscall.set_capability(CapSet::Effective, &caps::all())?;
Ok(())
}

/// Drop any extra granted capabilities, and reset to defaults which are in oci specification
pub fn drop_privileges(cs: &LinuxCapabilities, syscall: &impl Syscall) -> Result<()> {
pub fn drop_privileges<S: Syscall + ?Sized>(cs: &LinuxCapabilities, syscall: &S) -> Result<()> {
log::debug!("dropping bounding capabilities to {:?}", cs.bounding);
if let Some(bounding) = cs.bounding.as_ref() {
syscall.set_capability(CapSet::Bounding, &to_set(bounding))?;
Expand Down
5 changes: 3 additions & 2 deletions src/commands/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use anyhow::Result;
use clap::Clap;
use std::path::PathBuf;

use crate::container::builder::ContainerBuilder;
use crate::{container::builder::ContainerBuilder, syscall::syscall::create_syscall};

/// This is the main structure which stores various commandline options given by
/// high-level container runtime
Expand Down Expand Up @@ -50,7 +50,8 @@ impl Create {
}
/// Starts a new container process
pub fn exec(&self, root_path: PathBuf, systemd_cgroup: bool) -> Result<()> {
ContainerBuilder::new(self.container_id.clone())
let syscall = create_syscall();
ContainerBuilder::new(self.container_id.clone(), syscall.as_ref())
.with_pid_file(self.pid_file.as_ref())
.with_console_socket(self.console_socket.as_ref())
.with_root_path(root_path)
Expand Down
5 changes: 3 additions & 2 deletions src/commands/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use clap::Clap;
use std::{error::Error, path::PathBuf};

use crate::container::builder::ContainerBuilder;
use crate::{container::builder::ContainerBuilder, syscall::syscall::create_syscall};

#[derive(Clap, Debug)]
pub struct Exec {
Expand Down Expand Up @@ -38,7 +38,8 @@ pub struct Exec {

impl Exec {
pub fn exec(&self, root_path: PathBuf) -> Result<()> {
ContainerBuilder::new(self.container_id.clone())
let syscall = create_syscall();
ContainerBuilder::new(self.container_id.clone(), syscall.as_ref())
.with_root_path(root_path)
.with_console_socket(self.console_socket.as_ref())
.with_pid_file(self.pid_file.as_ref())
Expand Down
40 changes: 24 additions & 16 deletions src/container/builder.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use crate::syscall::linux::LinuxSyscall;
use crate::syscall::Syscall;
use std::path::PathBuf;

use super::{init_builder::InitContainerBuilder, tenant_builder::TenantContainerBuilder};
pub struct ContainerBuilder {
pub struct ContainerBuilder<'a> {
/// Id of the container
pub(super) container_id: String,
/// Root directory for container state
pub(super) root_path: PathBuf,
/// Interface to operating system primitives
pub(super) syscall: LinuxSyscall,
pub(super) syscall: &'a dyn Syscall,
/// File which will be used to communicate the pid of the
/// container process to the higher level runtime
pub(super) pid_file: Option<PathBuf>,
Expand All @@ -25,32 +25,34 @@ pub struct ContainerBuilder {
///
/// ```no_run
/// use youki::container::builder::ContainerBuilder;
/// use youki::syscall::syscall::create_syscall;;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .with_root_path("/run/containers/youki")
/// .with_pid_file(Some("/var/run/docker.pid"))
/// .with_console_socket(Some("/var/run/docker/sock.tty"))
/// .as_init("/var/run/docker/bundle")
/// .build();
/// ```
impl ContainerBuilder {
impl<'a> ContainerBuilder<'a> {
/// Generates the base configuration for a container which can be
/// transformed into either a init container or a tenant container
///
/// # Example
///
/// ```no_run
/// use youki::container::builder::ContainerBuilder;
/// use youki::syscall::syscall::create_syscall;;
///
/// let builder = ContainerBuilder::new("74f1a4cb3801".to_owned());
/// let builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref());
/// ```
pub fn new(container_id: String) -> Self {
pub fn new(container_id: String, syscall: &'a dyn Syscall) -> Self {
let root_path = PathBuf::from("/run/youki");

Self {
container_id,
root_path,
syscall: LinuxSyscall,
syscall,
pid_file: None,
console_socket: None,
preserve_fds: 0,
Expand All @@ -62,14 +64,15 @@ impl ContainerBuilder {
///
/// ```no_run
/// # use youki::container::builder::ContainerBuilder;
/// # use youki::syscall::syscall::create_syscall;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .as_tenant()
/// .with_container_args(vec!["sleep".to_owned(), "9001".to_owned()])
/// .build();
/// ```
#[allow(clippy::wrong_self_convention)]
pub fn as_tenant(self) -> TenantContainerBuilder {
pub fn as_tenant(self) -> TenantContainerBuilder<'a> {
TenantContainerBuilder::new(self)
}

Expand All @@ -78,14 +81,15 @@ impl ContainerBuilder {
///
/// ```no_run
/// # use youki::container::builder::ContainerBuilder;
/// # use youki::syscall::syscall::create_syscall;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .as_init("/var/run/docker/bundle")
/// .with_systemd(false)
/// .build();
/// ```
#[allow(clippy::wrong_self_convention)]
pub fn as_init<P: Into<PathBuf>>(self, bundle: P) -> InitContainerBuilder {
pub fn as_init<P: Into<PathBuf>>(self, bundle: P) -> InitContainerBuilder<'a> {
InitContainerBuilder::new(self, bundle.into())
}

Expand All @@ -94,8 +98,9 @@ impl ContainerBuilder {
///
/// ```no_run
/// # use youki::container::builder::ContainerBuilder;
/// # use youki::syscall::syscall::create_syscall;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .with_root_path("/run/containers/youki");
/// ```
pub fn with_root_path<P: Into<PathBuf>>(mut self, path: P) -> Self {
Expand All @@ -109,8 +114,9 @@ impl ContainerBuilder {
///
/// ```no_run
/// # use youki::container::builder::ContainerBuilder;
/// # use youki::syscall::syscall::create_syscall;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .with_pid_file(Some("/var/run/docker.pid"));
/// ```
pub fn with_pid_file<P: Into<PathBuf>>(mut self, path: Option<P>) -> Self {
Expand All @@ -124,8 +130,9 @@ impl ContainerBuilder {
///
/// ```no_run
/// # use youki::container::builder::ContainerBuilder;
/// # use youki::syscall::syscall::create_syscall;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .with_console_socket(Some("/var/run/docker/sock.tty"));
/// ```
pub fn with_console_socket<P: Into<PathBuf>>(mut self, path: Option<P>) -> Self {
Expand All @@ -139,8 +146,9 @@ impl ContainerBuilder {
///
/// ```no_run
/// # use youki::container::builder::ContainerBuilder;
/// # use youki::syscall::syscall::create_syscall;
///
/// ContainerBuilder::new("74f1a4cb3801".to_owned())
/// ContainerBuilder::new("74f1a4cb3801".to_owned(), create_syscall().as_ref())
/// .with_preserved_fds(5);
/// ```
pub fn with_preserved_fds(mut self, preserved_fds: i32) -> Self {
Expand Down
6 changes: 3 additions & 3 deletions src/container/builder_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
notify_socket::NotifyListener,
process::{args::ContainerArgs, channel, fork, intermediate},
rootless::{self, Rootless},
syscall::linux::LinuxSyscall,
syscall::Syscall,
utils,
};
use anyhow::{Context, Result};
Expand All @@ -17,7 +17,7 @@ pub(super) struct ContainerBuilderImpl<'a> {
/// Flag indicating if an init or a tenant container should be created
pub init: bool,
/// Interface to operating system primitives
pub syscall: LinuxSyscall,
pub syscall: &'a dyn Syscall,
/// Flag indicating if systemd should be used for cgroup management
pub use_systemd: bool,
/// Id of the container
Expand Down Expand Up @@ -102,7 +102,7 @@ impl<'a> ContainerBuilderImpl<'a> {
// is a shared reference, we have to clone these variables here.
let intermediate_args = ContainerArgs {
init: self.init,
syscall: self.syscall.clone(),
syscall: self.syscall,
spec: self.spec.clone(),
rootfs: self.rootfs.clone(),
console_socket: self.console_socket,
Expand Down
8 changes: 4 additions & 4 deletions src/container/init_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ use super::{
};

// Builder that can be used to configure the properties of a new container
pub struct InitContainerBuilder {
base: ContainerBuilder,
pub struct InitContainerBuilder<'a> {
base: ContainerBuilder<'a>,
bundle: PathBuf,
use_systemd: bool,
}

impl InitContainerBuilder {
impl<'a> InitContainerBuilder<'a> {
/// Generates the base configuration for a new container from which
/// configuration methods can be chained
pub(super) fn new(builder: ContainerBuilder, bundle: PathBuf) -> Self {
pub(super) fn new(builder: ContainerBuilder<'a>, bundle: PathBuf) -> Self {
Self {
base: builder,
bundle,
Expand Down
12 changes: 6 additions & 6 deletions src/container/tenant_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ use std::{
str::FromStr,
};

use crate::capabilities::CapabilityExt;
use crate::{capabilities::CapabilityExt, container::builder_impl::ContainerBuilderImpl};
use crate::{notify_socket::NotifySocket, rootless::Rootless, tty, utils};

use super::{builder::ContainerBuilder, builder_impl::ContainerBuilderImpl, Container};
use super::{builder::ContainerBuilder, Container};

const NAMESPACE_TYPES: &[&str] = &["ipc", "uts", "net", "pid", "mnt", "cgroup"];
const TENANT_NOTIFY: &str = "tenant-notify-";
const TENANT_TTY: &str = "tenant-tty-";

/// Builder that can be used to configure the properties of a process
/// that will join an existing container sandbox
pub struct TenantContainerBuilder {
base: ContainerBuilder,
pub struct TenantContainerBuilder<'a> {
base: ContainerBuilder<'a>,
env: HashMap<String, String>,
cwd: Option<PathBuf>,
args: Vec<String>,
Expand All @@ -37,11 +37,11 @@ pub struct TenantContainerBuilder {
process: Option<PathBuf>,
}

impl TenantContainerBuilder {
impl<'a> TenantContainerBuilder<'a> {
/// Generates the base configuration for a process that will join
/// an existing container sandbox from which configuration methods
/// can be chained
pub(super) fn new(builder: ContainerBuilder) -> Self {
pub(super) fn new(builder: ContainerBuilder<'a>) -> Self {
Self {
base: builder,
env: HashMap::new(),
Expand Down
4 changes: 2 additions & 2 deletions src/process/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ use std::os::unix::prelude::RawFd;
use std::path::PathBuf;

use crate::rootless::Rootless;
use crate::{container::Container, notify_socket::NotifyListener, syscall::linux::LinuxSyscall};
use crate::{container::Container, notify_socket::NotifyListener, syscall::Syscall};

pub struct ContainerArgs<'a> {
/// Flag indicating if an init or a tenant container should be created
pub init: bool,
/// Interface to operating system primitives
pub syscall: LinuxSyscall,
pub syscall: &'a dyn Syscall,
/// OCI complient runtime spec
pub spec: Spec,
/// Root filesystem of the container
Expand Down
4 changes: 2 additions & 2 deletions src/process/init.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::args::ContainerArgs;
use crate::{
capabilities, hooks, namespaces::Namespaces, process::channel, rootfs, rootless::Rootless,
seccomp, syscall::Syscall, tty, utils,
seccomp, tty, utils,
};
use anyhow::{bail, Context, Result};
use nix::mount::mount as nix_mount;
Expand Down Expand Up @@ -176,7 +176,7 @@ pub fn container_init(
args: ContainerArgs,
sender_to_intermediate: &mut channel::SenderInitToIntermediate,
) -> Result<()> {
let command = &args.syscall;
let command = args.syscall;
let spec = &args.spec;
let linux = spec.linux.as_ref().context("no linux in spec")?;
let proc = spec.process.as_ref().context("no process in spec")?;
Expand Down
2 changes: 1 addition & 1 deletion src/process/intermediate.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{namespaces::Namespaces, process::channel, process::fork, syscall::Syscall};
use crate::{namespaces::Namespaces, process::channel, process::fork};
use anyhow::{Context, Result};
use nix::unistd::{Gid, Uid};
use oci_spec::runtime::LinuxNamespaceType;
Expand Down