Skip to content

Commit

Permalink
Added config switch for disabling exec hooks on Linux (#2626)
Browse files Browse the repository at this point in the history
* Switch added

* Replaced raw env name with const

* disabled by default

* Fixed integration test
  • Loading branch information
Razz4780 authored Aug 1, 2024
1 parent d5e1a19 commit f282c55
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog.d/+linux-exec-hooks.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Disabled exec hooks on Linux by default. Added `experimental.enable_exec_hooks_linux` switch to the mirrord config.
8 changes: 8 additions & 0 deletions mirrord-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,14 @@
"description": "mirrord Experimental features. This shouldn't be used unless someone from MetalBear/mirrord tells you to.",
"type": "object",
"properties": {
"enable_exec_hooks_linux": {
"title": "_experimental_ enable_exec_hooks_linux {#experimental-enable_exec_hooks_linux}",
"description": "Enables exec hooks on Linux. Enable Linux hooks can fix issues when the application shares sockets with child commands (e.g Python web servers with reload), but the feature is not stable and may cause other issues.",
"type": [
"boolean",
"null"
]
},
"readlink": {
"title": "_experimental_ readlink {#experimental-readlink}",
"description": "Enables the `readlink` hook.",
Expand Down
8 changes: 7 additions & 1 deletion mirrord/config/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,12 @@ IP:PORT to connect to instead of using k8s api, for testing purposes.
mirrord Experimental features.
This shouldn't be used unless someone from MetalBear/mirrord tells you to.

## _experimental_ enable_exec_hooks_linux {#experimental-enable_exec_hooks_linux}

Enables exec hooks on Linux. Enable Linux hooks can fix issues when the application
shares sockets with child commands (e.g Python web servers with reload),
but the feature is not stable and may cause other issues.

## _experimental_ readlink {#experimental-readlink}

Enables the `readlink` hook.
Expand All @@ -395,7 +401,7 @@ Enables the `readlink` hook.

<https://github.com/metalbear-co/mirrord/issues/2421#issuecomment-2093200904>

# _experimental_ trust_any_certificate {#experimental-trust_any_certificate}
## _experimental_ trust_any_certificate {#experimental-trust_any_certificate}

Enables trusting any certificate on macOS, useful for <https://github.com/golang/go/issues/51991#issuecomment-2059588252>

Expand Down
11 changes: 10 additions & 1 deletion mirrord/config/src/experimental.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,26 @@ pub struct ExperimentalConfig {
#[config(default = false)]
pub readlink: bool,

/// # _experimental_ trust_any_certificate {#experimental-trust_any_certificate}
/// ## _experimental_ trust_any_certificate {#experimental-trust_any_certificate}
///
/// Enables trusting any certificate on macOS, useful for <https://github.com/golang/go/issues/51991#issuecomment-2059588252>
#[config(default = false)]
pub trust_any_certificate: bool,

/// ## _experimental_ enable_exec_hooks_linux {#experimental-enable_exec_hooks_linux}
///
/// Enables exec hooks on Linux. Enable Linux hooks can fix issues when the application
/// shares sockets with child commands (e.g Python web servers with reload),
/// but the feature is not stable and may cause other issues.
#[config(default = false)]
pub enable_exec_hooks_linux: bool,
}

impl CollectAnalytics for &ExperimentalConfig {
fn collect_analytics(&self, analytics: &mut mirrord_analytics::Analytics) {
analytics.add("tcp_ping4_mock", self.tcp_ping4_mock);
analytics.add("readlink", self.readlink);
analytics.add("trust_any_certificate", self.trust_any_certificate);
analytics.add("enable_exec_hooks_linux", self.enable_exec_hooks_linux);
}
}
2 changes: 1 addition & 1 deletion mirrord/layer/src/exec_hooks/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ unsafe extern "C" fn execv_detour(path: *const c_char, argv: *const *const c_cha

// `encoded` is emtpy if the encoding failed, so we don't set the env var.
if !encoded.is_empty() {
std::env::set_var("MIRRORD_SHARED_SOCKETS", encoded);
std::env::set_var(SHARED_SOCKETS_ENV_VAR, encoded);
}

FN_EXECVE(path, argv, environ())
Expand Down
4 changes: 3 additions & 1 deletion mirrord/layer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,9 @@ fn enable_hooks(state: &LayerSetup) {

unsafe { socket::hooks::enable_socket_hooks(&mut hook_manager, enabled_remote_dns) };

unsafe { exec_hooks::hooks::enable_exec_hooks(&mut hook_manager) };
if cfg!(target_os = "macos") || state.experimental().enable_exec_hooks_linux {
unsafe { exec_hooks::hooks::enable_exec_hooks(&mut hook_manager) };
}

#[cfg(target_os = "macos")]
{
Expand Down
13 changes: 13 additions & 0 deletions mirrord/layer/tests/configs/port_mapping_shared_sockets.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"feature": {
"network": {
"incoming": {
"mode": "mirror",
"port_mapping": [[9999, 1234]]
}
}
},
"experimental": {
"enable_exec_hooks_linux": true
}
}
7 changes: 6 additions & 1 deletion mirrord/layer/tests/issue864.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ async fn test_issue864(
("MIRRORD_UDP_OUTGOING", "false"),
("OBJC_DISABLE_INITIALIZE_FORK_SAFETY", "YES"),
],
Some(config_dir.join("port_mapping.json").to_str().unwrap()),
Some(
config_dir
.join("port_mapping_shared_sockets.json")
.to_str()
.unwrap(),
),
)
.await;

Expand Down

0 comments on commit f282c55

Please sign in to comment.