Skip to content

Commit

Permalink
Make readlink stable (enabled by default) now, with the protocol part…
Browse files Browse the repository at this point in the history
… gated on version support.
  • Loading branch information
meowjesty committed Sep 18, 2024
1 parent e713731 commit bd1e725
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 35 deletions.
2 changes: 1 addition & 1 deletion mirrord/agent/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ impl FileManager {
}

/// Handles our `readlink_detour` with [`std::fs::read_link`].
#[tracing::instrument(level = "trace", skip_all)]
#[tracing::instrument(level = Level::TRACE, skip_all, err(level = Level::ERROR))]
pub(crate) fn read_link(&mut self, path: PathBuf) -> RemoteResult<ReadLinkFileResponse> {
let path = path
.strip_prefix("/")
Expand Down
8 changes: 8 additions & 0 deletions mirrord/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,14 @@ impl LayerConfig {
self.feature.network.dns.verify(context)?;
self.feature.network.outgoing.verify(context)?;

if self.experimental.readlink {
context.add_warning(
"experimenta.readlink config has been deprecated, and `readlink` is now\
enabled by default! You may remove it from your config."
.into(),
);
}

Ok(())
}
}
Expand Down
30 changes: 28 additions & 2 deletions mirrord/intproxy/src/proxies/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,34 @@ impl BackgroundTask for SimpleProxy {
.await;
}
}
SimpleProxyMessage::FileReq(message_id, session_id, req) => {
self.file_reqs.insert(message_id, session_id);
// TODO(alex): We can remove this case when users are up-to-date for `readlink`.
SimpleProxyMessage::FileReq(
message_id,
layer_id,
req @ FileRequest::ReadLink(_),
) => {
if protocol_version
.as_ref()
.is_some_and(|version| READDIR_BATCH_VERSION.matches(version))
{
self.file_reqs.insert(message_id, layer_id);
message_bus
.send(ProxyMessage::ToAgent(ClientMessage::FileRequest(req)))
.await;
} else {
message_bus
.send(ToLayer {
message_id,
message: ProxyToLayerMessage::File(FileResponse::ReadLink(Err(
ResponseError::NotImplemented,
))),
layer_id,
})
.await;
}
}
SimpleProxyMessage::FileReq(message_id, layer_id, req) => {
self.file_reqs.insert(message_id, layer_id);
message_bus
.send(ProxyMessage::ToAgent(ClientMessage::FileRequest(req)))
.await;
Expand Down
22 changes: 8 additions & 14 deletions mirrord/layer/src/file/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use mirrord_protocol::file::{
ReadLinkFileResponse, SeekFileResponse, WriteFileResponse, XstatFsResponse, XstatResponse,
};
use rand::distributions::{Alphanumeric, DistString};
use tracing::{error, trace};
use tracing::{error, trace, Level};

use super::{hooks::FN_OPEN, open_dirs::OPEN_DIRS, *};
#[cfg(target_os = "linux")]
Expand Down Expand Up @@ -295,24 +295,18 @@ pub(crate) fn pread(local_fd: RawFd, buffer_size: u64, offset: u64) -> Detour<Re
}

/// Resolves the symbolic link `path`.
///
/// Bypassed if the `experimental.readlink` config is not set to `true`.
#[mirrord_layer_macro::instrument(level = "trace", ret)]
#[mirrord_layer_macro::instrument(level = Level::TRACE, ret)]
pub(crate) fn read_link(path: Detour<PathBuf>) -> Detour<ReadLinkFileResponse> {
if crate::setup().experimental().readlink {
let path = remap_path!(path?);
let path = remap_path!(path?);

check_relative_paths!(path);
check_relative_paths!(path);

ensure_not_ignored!(path, false);
ensure_not_ignored!(path, false);

let requesting_path = ReadLinkFileRequest { path };
let response = common::make_proxy_request_with_response(requesting_path)??;
let requesting_path = ReadLinkFileRequest { path };
let response = common::make_proxy_request_with_response(requesting_path)??;

Detour::Success(response)
} else {
None?
}
Detour::Success(response)
}

pub(crate) fn pwrite(local_fd: RawFd, buffer: &[u8], offset: u64) -> Detour<WriteFileResponse> {
Expand Down
1 change: 1 addition & 0 deletions mirrord/layer/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ pub enum Application {
RustRecvFrom,
RustListenPorts,
Fork,
/// `mirrord/layer/tests/apps/readlink/readlink.c`
ReadLink,
OpenFile,
CIssue2055,
Expand Down
5 changes: 0 additions & 5 deletions mirrord/layer/tests/configs/readlink.json

This file was deleted.

15 changes: 2 additions & 13 deletions mirrord/layer/tests/readlink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,11 @@ pub use common::*;
#[rstest]
#[tokio::test]
#[timeout(Duration::from_secs(60))]
async fn readlink(
#[values(Some("readlink.json"))] with_config: Option<&str>,
dylib_path: &Path,
config_dir: &Path,
) {
async fn readlink(dylib_path: &Path) {
let application = Application::ReadLink;

let config = with_config.map(|config| {
let mut config_path = config_dir.to_path_buf();
config_path.push(config);
config_path
});
let config = config.as_ref().map(|path_buf| path_buf.to_str().unwrap());

let (mut test_process, mut intproxy) = application
.start_process_with_layer(dylib_path, Default::default(), config)
.start_process_with_layer(dylib_path, Default::default(), None)
.await;

println!("waiting for file request.");
Expand Down

0 comments on commit bd1e725

Please sign in to comment.