diff --git a/CHANGELOG.md b/CHANGELOG.md index bb4db9339ca..6bfbf28a028 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,16 +13,14 @@ It is now possible to use the remote's `addrinfo` by setting the `MIRRORD_REMOTE `true`, or using the `-d` option in mirrord-cli. - New feature, [Ephemeral Containers](https://github.com/metalbear-co/mirrord/issues/172). Use Kubernetes beta feature `Ephemeral Containers` to mirror traffic with the `--ephemeral-container` flag. -- E2E tests for Golang using the Gin framework. +- E2E tests on macos for Golang using the Gin framework. ### Changed - Refactored `mirrord-layer/socket` into a module structure similar to `mirrord-layer/file`. -<<<<<<< HEAD - Refactored the error part of the many `Result`. -======= - Refactored `file` related functions, created `FileHandler` and improved structure. +- Refactored error handling in mirrord-layer. - E2E: Collect minikube logs and fix collecting container logs ->>>>>>> 0d7f1b10697426981e0908a915667944314c0dbe - E2E: macOS use colima instead of minikube. ### Fixed diff --git a/mirrord-layer/src/error.rs b/mirrord-layer/src/error.rs index 13a46667e55..5b55921583f 100644 --- a/mirrord-layer/src/error.rs +++ b/mirrord-layer/src/error.rs @@ -1,9 +1,12 @@ -use std::{env::VarError, os::unix::io::RawFd, str::ParseBoolError}; +use std::{env::VarError, os::unix::io::RawFd, path::PathBuf, ptr, str::ParseBoolError}; +use errno::set_errno; use kube::config::InferConfigError; +use libc::FILE; use mirrord_protocol::{tcp::LayerTcp, ResponseError}; use thiserror::Error; use tokio::sync::{mpsc::error::SendError, oneshot::error::RecvError}; +use tracing::error; use super::HookMessage; @@ -40,7 +43,7 @@ pub enum LayerError { TryFromInt(#[from] std::num::TryFromIntError), #[error("mirrord-layer: Failed to find local fd `{0}`!")] - LocalFDNotFound(RawFd), + LocalFDNotFound(RawFd, PathBuf), #[error("mirrord-layer: HOOK_SENDER is `None`!")] EmptyHookSender, @@ -86,6 +89,9 @@ pub enum LayerError { #[error("mirrord-layer: DNS does not resolve!")] DNSNoName, + + #[error("mirrord-layer: Failed converting `to_str` with `{0}`!")] + Utf8(#[from] std::str::Utf8Error), } // Cannot have a generic From implementation for this error, so explicitly implemented here. @@ -94,3 +100,79 @@ impl<'a, T> From>> for Layer LayerError::LockError } } + +// mapping based on - https://man7.org/linux/man-pages/man3/errno.3.html + +impl From for i64 { + fn from(fail: LayerError) -> Self { + error!("Error occured in Layer >> {:?}", fail); + + let libc_error = match fail { + LayerError::VarError(_) => libc::EINVAL, + LayerError::ParseBoolError(_) => libc::EINVAL, + LayerError::SendErrorHookMessage(_) => libc::EBADMSG, + LayerError::SendErrorConnection(_) => libc::EBADMSG, + LayerError::SendErrorLayerTcp(_) => libc::EBADMSG, + LayerError::RecvError(_) => libc::EBADMSG, + LayerError::Null(_) => libc::EINVAL, + LayerError::TryFromInt(_) => libc::EINVAL, + LayerError::LocalFDNotFound(..) => libc::EBADF, + LayerError::EmptyHookSender => libc::EINVAL, + LayerError::NoConnectionId(_) => libc::ECONNREFUSED, + LayerError::IO(io_fail) => io_fail.raw_os_error().unwrap_or(libc::EIO), + LayerError::PortNotFound(_) => libc::EADDRNOTAVAIL, + LayerError::ConnectionIdNotFound(_) => libc::EADDRNOTAVAIL, + LayerError::ListenAlreadyExists => libc::EEXIST, + LayerError::SendErrorFileResponse => libc::EINVAL, + LayerError::SendErrorGetAddrInfoResponse => libc::EINVAL, + LayerError::LockError => libc::EINVAL, + LayerError::ResponseError(response_fail) => match response_fail { + ResponseError::AllocationFailure(_) => libc::ENOMEM, + ResponseError::NotFound(_) => libc::ENOENT, + ResponseError::NotDirectory(_) => libc::ENOTDIR, + ResponseError::NotFile(_) => libc::EISDIR, + ResponseError::RemoteIO(io_fail) => io_fail.raw_os_error.unwrap_or(libc::EIO), + }, + LayerError::UnmatchedPong => libc::ETIMEDOUT, + LayerError::KubeConfigError(_) => libc::EINVAL, + LayerError::PodSpecNotFound(_) => libc::EINVAL, + LayerError::KubeError(_) => libc::EINVAL, + LayerError::JSONConvertError(_) => libc::EINVAL, + LayerError::TimeOutError => libc::ETIMEDOUT, + LayerError::DNSNoName => libc::EFAULT, + LayerError::Utf8(_) => libc::EINVAL, + }; + + set_errno(errno::Errno(libc_error)); + + -1 + } +} + +impl From for isize { + fn from(fail: LayerError) -> Self { + i64::from(fail) as _ + } +} + +impl From for usize { + fn from(fail: LayerError) -> Self { + let _ = i64::from(fail); + + 0 + } +} + +impl From for i32 { + fn from(fail: LayerError) -> Self { + i64::from(fail) as _ + } +} + +impl From for *mut FILE { + fn from(fail: LayerError) -> Self { + let _ = i64::from(fail); + + ptr::null_mut() + } +} diff --git a/mirrord-layer/src/file/hooks.rs b/mirrord-layer/src/file/hooks.rs index fc015031b9b..cd190068671 100644 --- a/mirrord-layer/src/file/hooks.rs +++ b/mirrord-layer/src/file/hooks.rs @@ -10,6 +10,7 @@ use super::{ OpenOptionsInternalExt, IGNORE_FILES, OPEN_FILES, }; use crate::{ + error::LayerError, file::ops::{lseek, open, read, write}, macros::hook, }; @@ -18,15 +19,13 @@ use crate::{ /// /// **Bypassed** by `raw_path`s that match `IGNORE_FILES` regex. pub(super) unsafe extern "C" fn open_detour(raw_path: *const c_char, open_flags: c_int) -> RawFd { - let path: PathBuf = match CStr::from_ptr(raw_path).to_str().map_err(|fail| { - error!( - "Failed converting raw_path {:#?} from `c_char` with {:#?}", - raw_path, fail - ); - -1 - }) { - Ok(path_str) => path_str.into(), - Err(fail) => return fail, + let path = match CStr::from_ptr(raw_path) + .to_str() + .map_err(LayerError::from) + .map(PathBuf::from) + { + Ok(path) => path, + Err(fail) => return fail.into(), }; // Calls with non absolute paths are sent to libc::open. @@ -36,12 +35,8 @@ pub(super) unsafe extern "C" fn open_detour(raw_path: *const c_char, open_flags: let open_options = OpenOptionsInternalExt::from_flags(open_flags); let open_result = open(path, open_options); - open_result - .map_err(|fail| { - error!("Failed opening file with {:#?}", fail); - -1 - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = open_result.map_err(From::from); + result } } @@ -52,26 +47,22 @@ pub(super) unsafe extern "C" fn fopen_detour( raw_path: *const c_char, raw_mode: *const c_char, ) -> *mut FILE { - let path: PathBuf = match CStr::from_ptr(raw_path).to_str().map_err(|fail| { - error!( - "Failed converting raw_path {:#?} from `c_char` with {:#?}", - raw_path, fail - ); - std::ptr::null_mut() - }) { - Ok(path_str) => path_str.into(), - Err(fail) => return fail, + let path = match CStr::from_ptr(raw_path) + .to_str() + .map_err(LayerError::from) + .map(PathBuf::from) + { + Ok(path) => path, + Err(fail) => return fail.into(), }; - let mode: String = match CStr::from_ptr(raw_mode).to_str().map_err(|fail| { - error!( - "Failed converting raw_mode {:#?} from `c_char` with {:#?}", - raw_mode, fail - ); - std::ptr::null_mut() - }) { - Ok(mode_str) => mode_str.into(), - Err(fail) => return fail, + let mode = match CStr::from_ptr(raw_mode) + .to_str() + .map(String::from) + .map_err(LayerError::from) + { + Ok(mode) => mode, + Err(fail) => return fail.into(), }; if IGNORE_FILES.is_match(path.to_str().unwrap()) || !path.is_absolute() { @@ -80,12 +71,8 @@ pub(super) unsafe extern "C" fn fopen_detour( let open_options = OpenOptionsInternalExt::from_mode(mode); let fopen_result = fopen(path, open_options); - fopen_result - .map_err(|fail| { - error!("Failed opening file with {fail:#?}"); - ptr::null_mut() - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = fopen_result.map_err(From::from); + result } } @@ -94,15 +81,13 @@ pub(super) unsafe extern "C" fn fopen_detour( /// Converts a `RawFd` into `*mut FILE` only for files that are already being managed by /// mirrord-layer. pub(super) unsafe extern "C" fn fdopen_detour(fd: RawFd, raw_mode: *const c_char) -> *mut FILE { - let mode: String = match CStr::from_ptr(raw_mode).to_str().map_err(|fail| { - error!( - "Failed converting raw_mode {:#?} from `c_char` with {:#?}", - raw_mode, fail - ); - std::ptr::null_mut() - }) { - Ok(mode_str) => mode_str.into(), - Err(fail) => return fail, + let mode = match CStr::from_ptr(raw_mode) + .to_str() + .map(String::from) + .map_err(LayerError::from) + { + Ok(mode) => mode, + Err(fail) => return fail.into(), }; let open_files = OPEN_FILES.lock().unwrap(); @@ -112,12 +97,8 @@ pub(super) unsafe extern "C" fn fdopen_detour(fd: RawFd, raw_mode: *const c_char let open_options = OpenOptionsInternalExt::from_mode(mode); let fdopen_result = fdopen(local_fd, *remote_fd, open_options); - fdopen_result - .map_err(|fail| { - error!("Failed fdopen file with {fail:#?}"); - ptr::null_mut() - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = fdopen_result.map_err(From::from); + result } else { libc::fdopen(fd, raw_mode) } @@ -133,15 +114,13 @@ pub(super) unsafe extern "C" fn openat_detour( raw_path: *const c_char, open_flags: c_int, ) -> RawFd { - let path: PathBuf = match CStr::from_ptr(raw_path).to_str().map_err(|fail| { - error!( - "Failed converting raw_path {:#?} from `c_char` with {:#?}", - raw_path, fail - ); - -1 - }) { - Ok(path_str) => path_str.into(), - Err(fail) => return fail, + let path = match CStr::from_ptr(raw_path) + .to_str() + .map_err(LayerError::from) + .map(PathBuf::from) + { + Ok(path) => path, + Err(fail) => return fail.into(), }; // `openat` behaves the same as `open` when the path is absolute. @@ -158,12 +137,8 @@ pub(super) unsafe extern "C" fn openat_detour( if let Some(remote_fd) = remote_fd { let openat_result = openat(path, open_flags, remote_fd); - openat_result - .map_err(|fail| { - error!("Failed opening file with {fail:#?}"); - -1 - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = openat_result.map_err(From::from); + result } else { // Nope, it's relative outside of our hands. @@ -200,12 +175,8 @@ pub(crate) unsafe extern "C" fn read_detour( read_amount.try_into().unwrap() }); - read_result - .map_err(|fail| { - error!("Failed reading file with {fail:#?}"); - -1 - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = read_result.map_err(From::from); + result } else { libc::read(fd, out_buffer, count) } @@ -243,12 +214,8 @@ pub(crate) unsafe extern "C" fn fread_detour( read_amount }); - read_result - .map_err(|fail| { - error!("Failed reading file with {fail:#?}"); - 0 - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = read_result.map_err(From::from); + result } else { libc::fread(out_buffer, element_size, number_of_elements, file_stream) } @@ -289,12 +256,8 @@ pub(crate) unsafe extern "C" fn lseek_detour(fd: RawFd, offset: off_t, whence: c let lseek_result = lseek(remote_fd, seek_from).map(|offset| offset.try_into().unwrap()); - lseek_result - .map_err(|fail| { - error!("Failed lseek operation with {fail:#?}"); - -1 - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = lseek_result.map_err(From::from); + result } else { libc::lseek(fd, offset, whence) } @@ -322,12 +285,8 @@ pub(crate) unsafe extern "C" fn write_detour( let write_result = write(remote_fd, write_bytes); - write_result - .map_err(|fail| { - error!("Failed writing file with {fail:#?}"); - -1 - }) - .unwrap_or_else(|fail| fail) + let (Ok(result) | Err(result)) = write_result.map_err(From::from); + result } else { libc::write(fd, buffer, count) } diff --git a/mirrord-layer/src/file/ops.rs b/mirrord-layer/src/file/ops.rs index 33bdb3f3f4e..8855c53c712 100644 --- a/mirrord-layer/src/file/ops.rs +++ b/mirrord-layer/src/file/ops.rs @@ -157,12 +157,12 @@ pub(crate) fn fopen( path, open_options ); - let local_file_fd = open(path, open_options)?; + let local_file_fd = open(path.clone(), open_options)?; let open_files = OPEN_FILES.lock().unwrap(); open_files .get_key_value(&local_file_fd) - .ok_or(LayerError::LocalFDNotFound(local_file_fd)) + .ok_or(LayerError::LocalFDNotFound(local_file_fd, path)) // Convert the fd into a `*FILE`, this is be ok as long as `OPEN_FILES` holds the fd. .map(|(local_fd, _)| local_fd as *const _ as *mut _) }