From 658de326d6754f1e9adfd28b7725bff9a78d2796 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Mon, 6 Nov 2023 11:46:00 +0200 Subject: [PATCH 01/10] Don't drop mutex in child on fork_detour, fixes bug with elixir (#2056) --- changelog.d/2047.fixed.md | 1 + mirrord/layer/src/lib.rs | 30 ++++++++++++++---------------- 2 files changed, 15 insertions(+), 16 deletions(-) create mode 100644 changelog.d/2047.fixed.md diff --git a/changelog.d/2047.fixed.md b/changelog.d/2047.fixed.md new file mode 100644 index 00000000000..fc295eab684 --- /dev/null +++ b/changelog.d/2047.fixed.md @@ -0,0 +1 @@ +Don't drop mutex in child on fork_detour, fixes bug with elixir. \ No newline at end of file diff --git a/mirrord/layer/src/lib.rs b/mirrord/layer/src/lib.rs index ba8aae7a702..76591779625 100644 --- a/mirrord/layer/src/lib.rs +++ b/mirrord/layer/src/lib.rs @@ -466,30 +466,28 @@ pub(crate) unsafe extern "C" fn close_detour(fd: c_int) -> c_int { pub(crate) unsafe extern "C" fn fork_detour() -> pid_t { tracing::debug!("Process {} forking!.", std::process::id()); - let parent_connection = PROXY_CONNECTION.get().expect("PROXY_CONNECTION not set"); - - // After fork, this new connection lives both in the parent and in the child. - // The child will have access to cloned file descriptor of the underlying socket. - // The parent will close its descriptor at the end of this scope. - let new_connection = ProxyConnection::new( - parent_connection.proxy_addr(), - NewSessionRequest::Forked(parent_connection.layer_id()), - Duration::from_secs(5), - ) - .expect("failed to establish proxy connection for child"); - let res = FN_FORK(); match res.cmp(&0) { Ordering::Equal => { tracing::debug!("Child process initializing layer."); + let parent_connection = unsafe { PROXY_CONNECTION.take() } + .expect("parent connection doesn't exist in fork"); - PROXY_CONNECTION.take(); + let new_connection = ProxyConnection::new( + parent_connection.proxy_addr(), + NewSessionRequest::Forked(parent_connection.layer_id()), + Duration::from_secs(5), + ) + .expect("failed to establish proxy connection for child"); PROXY_CONNECTION .set(new_connection) - .expect("setting PROXY_CONNECTION"); - - mirrord_layer_entry_point() + .expect("Failed setting PROXY_CONNECTION in child fork"); + // in macOS (and tbh sounds logical) we can't just drop the old connection in the child, + // as it needs to access a mutex with invalid state, so we need to forget it. + // better implementation would be to somehow close the underlying connections + // but side effect should be trivial + std::mem::forget(parent_connection); } Ordering::Greater => tracing::debug!("Child process id is {res}."), Ordering::Less => tracing::debug!("fork failed"), From b050ef3e15d4c144f9ba7a8af7ef1e45575cb326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Smolarek?= <34063647+Razz4780@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:44:49 +0100 Subject: [PATCH 02/10] Copy target scale down (#2054) * Config * More CRD docs * warning when target is not a deployment * Config docs * Changelog entry * Scale to 0 * mirrord-schema update * Removed 'Option' from CRD * Erroring out on scale down + target is not a deployment * Config fix * doc improvement * Format * Config finally fixed * Checking target type in 'LayerConfig::verify' * Doc updated * mirrord-schema updated * Format --- changelog.d/2053.added.md | 1 + mirrord-schema.json | 35 +++++++-- mirrord/cli/src/connection.rs | 4 +- mirrord/config/src/feature.rs | 12 ++-- mirrord/config/src/feature/copy_target.rs | 87 +++++++++++++++++++++++ mirrord/config/src/lib.rs | 18 ++++- mirrord/operator/src/client.rs | 51 ++++++++++--- mirrord/operator/src/crd.rs | 5 +- 8 files changed, 186 insertions(+), 27 deletions(-) create mode 100644 changelog.d/2053.added.md create mode 100644 mirrord/config/src/feature/copy_target.rs diff --git a/changelog.d/2053.added.md b/changelog.d/2053.added.md new file mode 100644 index 00000000000..f393af407a7 --- /dev/null +++ b/changelog.d/2053.added.md @@ -0,0 +1 @@ +Added option to scale down target deployment when using `copy target` feature. \ No newline at end of file diff --git a/mirrord-schema.json b/mirrord-schema.json index 996df2194ec..911b7fa527b 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -1,7 +1,7 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "title": "LayerFileConfig", - "description": "mirrord allows for a high degree of customization when it comes to which features you want to enable, and how they should function.\n\nAll of the configuration fields have a default value, so a minimal configuration would be no configuration at all.\n\nThe configuration supports templating using the [Tera](https://keats.github.io/tera/docs/) template engine. Currently we don't provide additional values to the context, if you have anything you want us to provide please let us know.\n\nTo help you get started, here are examples of a basic configuration file, and a complete configuration file containing all fields.\n\n### Basic `config.json` {#root-basic}\n\n```json { \"target\": \"pod/bear-pod\", \"feature\": { \"env\": true, \"fs\": \"read\", \"network\": true } } ```\n\n### Basic `config.json` with templating {#root-basic-templating}\n\n```json { \"target\": \"{{ get_env(name=\"TARGET\", default=\"pod/fallback\") }}\", \"feature\": { \"env\": true, \"fs\": \"read\", \"network\": true } } ```\n\n### Complete `config.json` {#root-complete}\n\nDon't use this example as a starting point, it's just here to show you all the available options. ```json { \"accept_invalid_certificates\": false, \"skip_processes\": \"ide-debugger\", \"pause\": false, \"target\": { \"path\": \"pod/bear-pod\", \"namespace\": \"default\" }, \"connect_tcp\": null, \"agent\": { \"log_level\": \"info\", \"namespace\": \"default\", \"image\": \"ghcr.io/metalbear-co/mirrord:latest\", \"image_pull_policy\": \"IfNotPresent\", \"image_pull_secrets\": [ { \"secret-key\": \"secret\" } ], \"ttl\": 30, \"ephemeral\": false, \"communication_timeout\": 30, \"startup_timeout\": 360, \"network_interface\": \"eth0\", \"flush_connections\": true }, \"feature\": { \"env\": { \"include\": \"DATABASE_USER;PUBLIC_ENV\", \"exclude\": \"DATABASE_PASSWORD;SECRET_ENV\", \"override\": { \"DATABASE_CONNECTION\": \"db://localhost:7777/my-db\", \"LOCAL_BEAR\": \"panda\" } }, \"fs\": { \"mode\": \"write\", \"read_write\": \".+\\.json\" , \"read_only\": [ \".+\\.yaml\", \".+important-file\\.txt\" ], \"local\": [ \".+\\.js\", \".+\\.mjs\" ] }, \"network\": { \"incoming\": { \"mode\": \"steal\", \"http_header_filter\": { \"filter\": \"host: api\\..+\", \"ports\": [80, 8080] }, \"port_mapping\": [[ 7777, 8888 ]], \"ignore_localhost\": false, \"ignore_ports\": [9999, 10000] }, \"outgoing\": { \"tcp\": true, \"udp\": true, \"filter\": { \"local\": [\"tcp://1.1.1.0/24:1337\", \"1.1.5.0/24\", \"google.com\", \":53\"] }, \"ignore_localhost\": false, \"unix_streams\": \"bear.+\" }, \"dns\": false }, }, \"operator\": true, \"kubeconfig\": \"~/.kube/config\", \"sip_binaries\": \"bash\", \"telemetry\": true, \"kube_context\": \"my-cluster\" } ```\n\n# Options {#root-options}", + "description": "mirrord allows for a high degree of customization when it comes to which features you want to enable, and how they should function.\n\nAll of the configuration fields have a default value, so a minimal configuration would be no configuration at all.\n\nThe configuration supports templating using the [Tera](https://keats.github.io/tera/docs/) template engine. Currently we don't provide additional values to the context, if you have anything you want us to provide please let us know.\n\nTo help you get started, here are examples of a basic configuration file, and a complete configuration file containing all fields.\n\n### Basic `config.json` {#root-basic}\n\n```json { \"target\": \"pod/bear-pod\", \"feature\": { \"env\": true, \"fs\": \"read\", \"network\": true } } ```\n\n### Basic `config.json` with templating {#root-basic-templating}\n\n```json { \"target\": \"{{ get_env(name=\"TARGET\", default=\"pod/fallback\") }}\", \"feature\": { \"env\": true, \"fs\": \"read\", \"network\": true } } ```\n\n### Complete `config.json` {#root-complete}\n\nDon't use this example as a starting point, it's just here to show you all the available options. ```json { \"accept_invalid_certificates\": false, \"skip_processes\": \"ide-debugger\", \"pause\": false, \"target\": { \"path\": \"pod/bear-pod\", \"namespace\": \"default\" }, \"connect_tcp\": null, \"agent\": { \"log_level\": \"info\", \"namespace\": \"default\", \"image\": \"ghcr.io/metalbear-co/mirrord:latest\", \"image_pull_policy\": \"IfNotPresent\", \"image_pull_secrets\": [ { \"secret-key\": \"secret\" } ], \"ttl\": 30, \"ephemeral\": false, \"communication_timeout\": 30, \"startup_timeout\": 360, \"network_interface\": \"eth0\", \"flush_connections\": true }, \"feature\": { \"env\": { \"include\": \"DATABASE_USER;PUBLIC_ENV\", \"exclude\": \"DATABASE_PASSWORD;SECRET_ENV\", \"override\": { \"DATABASE_CONNECTION\": \"db://localhost:7777/my-db\", \"LOCAL_BEAR\": \"panda\" } }, \"fs\": { \"mode\": \"write\", \"read_write\": \".+\\.json\" , \"read_only\": [ \".+\\.yaml\", \".+important-file\\.txt\" ], \"local\": [ \".+\\.js\", \".+\\.mjs\" ] }, \"network\": { \"incoming\": { \"mode\": \"steal\", \"http_header_filter\": { \"filter\": \"host: api\\..+\", \"ports\": [80, 8080] }, \"port_mapping\": [[ 7777, 8888 ]], \"ignore_localhost\": false, \"ignore_ports\": [9999, 10000] }, \"outgoing\": { \"tcp\": true, \"udp\": true, \"filter\": { \"local\": [\"tcp://1.1.1.0/24:1337\", \"1.1.5.0/24\", \"google.com\", \":53\"] }, \"ignore_localhost\": false, \"unix_streams\": \"bear.+\" }, \"dns\": false, \"copy_target\": { \"scale_down\": false } }, }, \"operator\": true, \"kubeconfig\": \"~/.kube/config\", \"sip_binaries\": \"bash\", \"telemetry\": true, \"kube_context\": \"my-cluster\" } ```\n\n# Options {#root-options}", "type": "object", "properties": { "accept_invalid_certificates": { @@ -388,6 +388,27 @@ } ] }, + "CopyTargetFileConfig": { + "description": "Allows the user to target a pod created dynamically from the orignal [`target`](#target). The new pod inherits most of the original target's specification, e.g. labels.\n\n```json { \"feature\": { \"copy_target\": { \"scale_down\": true } } } ```\n\n```json { \"feature\": { \"copy_target\": true } } ```", + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "object", + "properties": { + "scale_down": { + "title": "feature.copy_target.scale_down {#feature-copy_target-scale_down}", + "description": "If this option is set and [`target`](#target) is a deployment, mirrord will scale it down to 0 for the time the copied pod is alive.", + "type": [ + "boolean", + "null" + ] + } + } + } + ] + }, "DeploymentTarget": { "description": " Mirror the deployment specified by [`DeploymentTarget::deployment`].", "type": "object", @@ -454,11 +475,15 @@ "type": "object", "properties": { "copy_target": { - "title": "feature.copy_target {#feature-copy-target}", + "title": "feature.copy_target {#feature-copy_target}", "description": "Creates a new copy of the target. mirrord will use this copy instead of the original target (e.g. intercept network traffic). This feature requires a [mirrord operator](https://mirrord.dev/docs/teams/introduction/).", - "type": [ - "boolean", - "null" + "anyOf": [ + { + "$ref": "#/definitions/CopyTargetFileConfig" + }, + { + "type": "null" + } ] }, "env": { diff --git a/mirrord/cli/src/connection.rs b/mirrord/cli/src/connection.rs index c88a86a9835..87e9a4a725a 100644 --- a/mirrord/cli/src/connection.rs +++ b/mirrord/cli/src/connection.rs @@ -95,8 +95,8 @@ where AgentConnection { sender: session.tx, receiver: session.rx }, )) } else { - if config.feature.copy_target { - return Err(CliError::FeatureRequiresOperatorError("copy pod".into())); + if config.feature.copy_target.enabled { + return Err(CliError::FeatureRequiresOperatorError("copy target".into())); } if matches!(config.target, mirrord_config::target::TargetConfig{ path: Some(mirrord_config::target::Target::Deployment{..}), ..}) { diff --git a/mirrord/config/src/feature.rs b/mirrord/config/src/feature.rs index 459cd84e9a1..8dccc8f5887 100644 --- a/mirrord/config/src/feature.rs +++ b/mirrord/config/src/feature.rs @@ -2,9 +2,9 @@ use mirrord_analytics::CollectAnalytics; use mirrord_config_derive::MirrordConfig; use schemars::JsonSchema; -use self::{env::EnvConfig, fs::FsConfig, network::NetworkConfig}; -use crate::MirrordConfigSource; +use self::{copy_target::CopyTargetConfig, env::EnvConfig, fs::FsConfig, network::NetworkConfig}; +pub mod copy_target; pub mod env; pub mod fs; pub mod network; @@ -79,12 +79,12 @@ pub struct FeatureConfig { #[config(nested, toggleable)] pub network: NetworkConfig, - /// ## feature.copy_target {#feature-copy-target} + /// ## feature.copy_target {#feature-copy_target} /// /// Creates a new copy of the target. mirrord will use this copy instead of the original target /// (e.g. intercept network traffic). This feature requires a [mirrord operator](https://mirrord.dev/docs/teams/introduction/). - #[config(default = false, unstable)] - pub copy_target: bool, + #[config(nested, unstable)] + pub copy_target: CopyTargetConfig, } impl CollectAnalytics for &FeatureConfig { @@ -92,6 +92,6 @@ impl CollectAnalytics for &FeatureConfig { analytics.add("env", &self.env); analytics.add("fs", &self.fs); analytics.add("network", &self.network); - analytics.add("copy_target", self.copy_target); + analytics.add("copy_target", &self.copy_target); } } diff --git a/mirrord/config/src/feature/copy_target.rs b/mirrord/config/src/feature/copy_target.rs new file mode 100644 index 00000000000..c17232b3a85 --- /dev/null +++ b/mirrord/config/src/feature/copy_target.rs @@ -0,0 +1,87 @@ +//! Config for the `copy target` feature. [`CopyTargetFileConfig`] does follow the pattern of other +//! [`feature`](crate::feature) configs by not implementing +//! [`MirrordToggleableConfig`](crate::util::MirrordToggleableConfig). The reason for this is that +//! [`ToggleableConfig`](crate::util::ToggleableConfig) is enabled by default. This config should be +//! disabled unless explicitly enabled. + +use mirrord_analytics::CollectAnalytics; +use schemars::JsonSchema; +use serde::Deserialize; + +use crate::config::{ConfigContext, FromMirrordConfig, MirrordConfig, Result}; + +/// Allows the user to target a pod created dynamically from the orignal [`target`](#target). +/// The new pod inherits most of the original target's specification, e.g. labels. +/// +/// ```json +/// { +/// "feature": { +/// "copy_target": { +/// "scale_down": true +/// } +/// } +/// } +/// ``` +/// +/// ```json +/// { +/// "feature": { +/// "copy_target": true +/// } +/// } +/// ``` +#[derive(Clone, Debug, Deserialize, JsonSchema)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(untagged)] +pub enum CopyTargetFileConfig { + Simple(bool), + Advanced { + /// ### feature.copy_target.scale_down {#feature-copy_target-scale_down} + /// + /// If this option is set and [`target`](#target) is a deployment, + /// mirrord will scale it down to 0 for the time the copied pod is alive. + scale_down: Option, + }, +} + +impl Default for CopyTargetFileConfig { + fn default() -> Self { + Self::Simple(false) + } +} + +impl MirrordConfig for CopyTargetFileConfig { + type Generated = CopyTargetConfig; + + fn generate_config(self, _context: &mut ConfigContext) -> Result { + let res = match self { + Self::Simple(enabled) => Self::Generated { + enabled, + scale_down: false, + }, + Self::Advanced { scale_down } => Self::Generated { + enabled: true, + scale_down: scale_down.unwrap_or_default(), + }, + }; + + Ok(res) + } +} + +impl FromMirrordConfig for CopyTargetConfig { + type Generator = CopyTargetFileConfig; +} + +#[derive(Clone, Debug)] +pub struct CopyTargetConfig { + pub enabled: bool, + pub scale_down: bool, +} + +impl CollectAnalytics for &CopyTargetConfig { + fn collect_analytics(&self, analytics: &mut mirrord_analytics::Analytics) { + analytics.add("enabled", self.enabled); + analytics.add("scale_down", self.scale_down); + } +} diff --git a/mirrord/config/src/lib.rs b/mirrord/config/src/lib.rs index ea0a1322898..8957a065e92 100644 --- a/mirrord/config/src/lib.rs +++ b/mirrord/config/src/lib.rs @@ -21,6 +21,7 @@ use config::{ConfigContext, ConfigError, MirrordConfig}; use mirrord_analytics::CollectAnalytics; use mirrord_config_derive::MirrordConfig; use schemars::JsonSchema; +use target::Target; use tera::Tera; use tracing::warn; @@ -143,7 +144,10 @@ const PAUSE_WITHOUT_STEAL_WARNING: &str = /// "ignore_localhost": false, /// "unix_streams": "bear.+" /// }, -/// "dns": false +/// "dns": false, +/// "copy_target": { +/// "scale_down": false +/// } /// }, /// }, /// "operator": true, @@ -419,7 +423,7 @@ impl LayerConfig { } } - if self.feature.copy_target { + if self.feature.copy_target.enabled { if !self.operator { return Err(ConfigError::Conflict( "The copy target feature requires a mirrord operator, \ @@ -445,6 +449,16 @@ impl LayerConfig { .into(), ); } + + if self.feature.copy_target.scale_down + && !matches!(self.target.path, Some(Target::Deployment(..))) + { + return Err(ConfigError::Conflict( + "The scale down feature is compatible only with deployment targets, \ + please either disable this option or specify a deployment target." + .into(), + )); + } } Ok(()) diff --git a/mirrord/operator/src/client.rs b/mirrord/operator/src/client.rs index 777368d2348..e475c6f39ae 100644 --- a/mirrord/operator/src/client.rs +++ b/mirrord/operator/src/client.rs @@ -9,7 +9,9 @@ use mirrord_auth::{ certificate::Certificate, credential_store::CredentialStoreSync, error::AuthenticationError, }; use mirrord_config::{ - feature::network::incoming::ConcurrentSteal, target::TargetConfig, LayerConfig, + feature::network::incoming::ConcurrentSteal, + target::{Target, TargetConfig}, + LayerConfig, }; use mirrord_kube::{ api::kubernetes::{create_kube_api, get_k8s_resource_api}, @@ -33,8 +35,8 @@ static CONNECTION_CHANNEL_SIZE: usize = 1000; #[derive(Debug, Error)] pub enum OperatorApiError { - #[error("unable to create target for TargetConfig")] - InvalidTarget, + #[error("invalid target: {reason}")] + InvalidTarget { reason: String }, #[error(transparent)] HttpError(#[from] http::Error), #[error(transparent)] @@ -159,7 +161,7 @@ impl OperatorApi { /// Checks used config against operator specification. fn check_config(config: &LayerConfig, operator: &MirrordOperatorCrd) -> Result<()> { - if config.feature.copy_target { + if config.feature.copy_target.enabled { let feature_enabled = operator.spec.copy_target_enabled.unwrap_or(false); if !feature_enabled { @@ -168,6 +170,14 @@ impl OperatorApi { operator_version: operator.spec.operator_version.clone(), }); } + + if config.feature.copy_target.scale_down + && !matches!(config.target.path, Some(Target::Deployment(..))) + { + return Err(OperatorApiError::InvalidTarget { + reason: "scale down feature is enabled, but target is not a deployment".into(), + }); + } } Ok(()) @@ -238,14 +248,29 @@ impl OperatorApi { } version_progress.success(None); - let raw_target = operator_api - .fetch_target() - .await? - .ok_or(OperatorApiError::InvalidTarget)?; + let raw_target = + operator_api + .fetch_target() + .await? + .ok_or(OperatorApiError::InvalidTarget { + reason: "not found in the cluster".into(), + })?; - let target_to_connect = if config.feature.copy_target { + let target_to_connect = if config.feature.copy_target.enabled { let mut copy_progress = progress.subtask("copying target"); - let copied = operator_api.copy_target(&metadata, raw_target).await?; + + if config.feature.copy_target.scale_down { + let is_deployment = matches!(config.target.path, Some(Target::Deployment(..))); + if !is_deployment { + progress.warning( + "cannot scale down while copying target - target is not a deployment", + ) + } + } + + let copied = operator_api + .copy_target(&metadata, raw_target, config.feature.copy_target.scale_down) + .await?; copy_progress.success(None); OperatorSessionTarget::Copied(copied) @@ -465,18 +490,22 @@ impl OperatorApi { &self, session_metadata: &OperatorSessionMetadata, target: TargetCrd, + scale_down: bool, ) -> Result { let raw_target = target .spec .target .clone() - .ok_or(OperatorApiError::InvalidTarget)?; + .ok_or(OperatorApiError::InvalidTarget { + reason: "copy target feature is not compatible with targetless mode".into(), + })?; let requested = CopyTargetCrd::new( &target.name(), CopyTargetSpec { target: raw_target, idle_ttl: Some(Self::COPIED_POD_IDLE_TTL), + scale_down, }, ); diff --git a/mirrord/operator/src/crd.rs b/mirrord/operator/src/crd.rs index 83c0fdb8d7c..ea8d8498096 100644 --- a/mirrord/operator/src/crd.rs +++ b/mirrord/operator/src/crd.rs @@ -135,9 +135,12 @@ pub enum OperatorFeatures { namespaced )] pub struct CopyTargetSpec { - /// Original target. + /// Original target. Only [`Target::Pod`] and [`Target::Deployment`] are accepted. pub target: Target, /// How long should the operator keep this pod alive after its creation. /// The pod is deleted when this timout has expired and there are no connected clients. pub idle_ttl: Option, + /// Should the operator scale down target deployment to 0 while this pod is alive. + /// Ignored if [`Target`] is not [`Target::Deployment`]. + pub scale_down: bool, } From 915e347e2ee33e49f602b1a8fc22f7614a8e31d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Smolarek?= <34063647+Razz4780@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:46:23 +0100 Subject: [PATCH 03/10] Improve intproxy deps (#2057) * IntProxy protocol extracted to a separate crate * mirrord-intproxy-protocol deps updated * async support feature-gated * mirrord-intproxy deps updated * mirrord-console deps updated * layer deps updated * Changelog entry --- .vscode/settings.json | 2 +- Cargo.lock | 19 ++- changelog.d/2039.internal.md | 1 + mirrord/cli/Cargo.toml | 2 +- mirrord/console/Cargo.toml | 19 +-- mirrord/console/src/async_logger.rs | 2 +- mirrord/console/src/error.rs | 2 +- mirrord/console/src/lib.rs | 2 + mirrord/console/src/logger.rs | 2 +- mirrord/console/src/main.rs | 2 +- mirrord/intproxy/Cargo.toml | 6 +- mirrord/intproxy/protocol/Cargo.toml | 27 ++++ mirrord/intproxy/{ => protocol}/src/codec.rs | 127 +----------------- .../protocol/src/codec/codec_async.rs | 125 +++++++++++++++++ .../{src/protocol.rs => protocol/src/lib.rs} | 4 +- .../{src/protocol => protocol/src}/macros.rs | 0 mirrord/intproxy/src/error.rs | 3 +- mirrord/intproxy/src/layer_conn.rs | 6 +- mirrord/intproxy/src/layer_initializer.rs | 8 +- mirrord/intproxy/src/lib.rs | 9 +- mirrord/intproxy/src/main_tasks.rs | 3 +- mirrord/intproxy/src/proxies/incoming.rs | 8 +- .../proxies/incoming/port_subscription_ext.rs | 2 +- mirrord/intproxy/src/proxies/outgoing.rs | 8 +- .../src/proxies/outgoing/net_protocol_ext.rs | 3 +- mirrord/intproxy/src/proxies/simple.rs | 2 +- mirrord/intproxy/src/remote_resources.rs | 2 +- mirrord/intproxy/src/request_queue.rs | 3 +- mirrord/layer/Cargo.toml | 2 +- mirrord/layer/src/common.rs | 2 +- mirrord/layer/src/lib.rs | 2 +- mirrord/layer/src/proxy_connection.rs | 8 +- mirrord/layer/src/setup.rs | 2 +- mirrord/layer/src/socket.rs | 2 +- mirrord/layer/src/socket/ops.rs | 2 +- 35 files changed, 228 insertions(+), 191 deletions(-) create mode 100644 changelog.d/2039.internal.md create mode 100644 mirrord/intproxy/protocol/Cargo.toml rename mirrord/intproxy/{ => protocol}/src/codec.rs (58%) create mode 100644 mirrord/intproxy/protocol/src/codec/codec_async.rs rename mirrord/intproxy/{src/protocol.rs => protocol/src/lib.rs} (99%) rename mirrord/intproxy/{src/protocol => protocol/src}/macros.rs (100%) diff --git a/.vscode/settings.json b/.vscode/settings.json index 45db4c05a3a..b5cb046847b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,5 @@ { "rust-analyzer.cargo.features": [ - "binary" + "binary", ] } \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index ce8df920a3a..5729c6ba23b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3827,7 +3827,7 @@ dependencies = [ "drain", "log", "miette", - "mirrord-intproxy", + "mirrord-intproxy-protocol", "thiserror", "tokio", "tokio-util", @@ -3839,27 +3839,33 @@ dependencies = [ name = "mirrord-intproxy" version = "3.74.1" dependencies = [ - "bincode", "bytes", - "fancy-regex", - "futures", "http-body-util", "hyper 1.0.0-rc.4", "hyper-util", "mirrord-config", + "mirrord-intproxy-protocol", "mirrord-kube", "mirrord-operator", "mirrord-protocol", "rand", "serde", - "serde_json", "thiserror", "tokio", "tokio-stream", - "tokio-util", "tracing", ] +[[package]] +name = "mirrord-intproxy-protocol" +version = "3.74.1" +dependencies = [ + "bincode", + "mirrord-protocol", + "thiserror", + "tokio", +] + [[package]] name = "mirrord-kube" version = "3.74.1" @@ -3915,6 +3921,7 @@ dependencies = [ "mirrord-config", "mirrord-console", "mirrord-intproxy", + "mirrord-intproxy-protocol", "mirrord-layer-macro", "mirrord-protocol", "mirrord-sip", diff --git a/changelog.d/2039.internal.md b/changelog.d/2039.internal.md new file mode 100644 index 00000000000..aa8ac9bbaf4 --- /dev/null +++ b/changelog.d/2039.internal.md @@ -0,0 +1 @@ +Improved crates structure around internal proxy and mirrord console. \ No newline at end of file diff --git a/mirrord/cli/Cargo.toml b/mirrord/cli/Cargo.toml index 28dc331e85c..05f68b4775d 100644 --- a/mirrord/cli/Cargo.toml +++ b/mirrord/cli/Cargo.toml @@ -22,7 +22,7 @@ mirrord-progress = { path = "../progress" } mirrord-kube = { path = "../kube" } mirrord-config = { path = "../config" } mirrord-protocol = { path = "../protocol" } -mirrord-console = { path = "../console" } +mirrord-console = { path = "../console", features = ["async-logger"] } mirrord-analytics = { path = "../analytics" } mirrord-intproxy = { path = "../intproxy" } diff --git a/mirrord/console/Cargo.toml b/mirrord/console/Cargo.toml index 8b2ef3e8cc4..5c0bc148c84 100644 --- a/mirrord/console/Cargo.toml +++ b/mirrord/console/Cargo.toml @@ -19,16 +19,19 @@ required-features = ["binary"] [features] default = [] -binary = ["dep:tracing", "dep:tracing-subscriber"] +binary = ["dep:tracing", "dep:tracing-subscriber", "dep:tokio", "mirrord-intproxy-protocol/codec-async"] +async-logger = ["mirrord-intproxy-protocol/codec-async", "dep:tokio", "dep:drain", "dep:tokio-util"] [dependencies] -mirrord-intproxy = { path = "../intproxy" } -tokio.workspace = true -tracing = { workspace = true, optional = true } -tracing-subscriber = { workspace = true, optional = true } -log = { version = "0.4", features = ["std"] } +mirrord-intproxy-protocol = { path = "../intproxy/protocol", features = ["codec"] } + bincode.workspace = true +log = { version = "0.4", features = ["std"] } miette = "5" thiserror.workspace = true -drain.workspace = true -tokio-util.workspace = true \ No newline at end of file + +tokio = { workspace = true, optional = true } +tracing = { workspace = true, optional = true } +tracing-subscriber = { workspace = true, optional = true } +drain = { workspace = true, optional = true } +tokio-util = { workspace = true, optional = true } diff --git a/mirrord/console/src/async_logger.rs b/mirrord/console/src/async_logger.rs index b3b7b63cadc..78d8d442994 100644 --- a/mirrord/console/src/async_logger.rs +++ b/mirrord/console/src/async_logger.rs @@ -1,6 +1,6 @@ use drain::Watch; use log::LevelFilter; -use mirrord_intproxy::codec::AsyncEncoder; +use mirrord_intproxy_protocol::codec::AsyncEncoder; use tokio::{ io::BufWriter, net::TcpStream, diff --git a/mirrord/console/src/error.rs b/mirrord/console/src/error.rs index ceca775eff4..e756b92f600 100644 --- a/mirrord/console/src/error.rs +++ b/mirrord/console/src/error.rs @@ -1,7 +1,7 @@ use std::io; use miette::Diagnostic; -use mirrord_intproxy::codec::CodecError; +use mirrord_intproxy_protocol::codec::CodecError; use thiserror::Error; pub type Result = std::result::Result; diff --git a/mirrord/console/src/lib.rs b/mirrord/console/src/lib.rs index 0e3f166d157..c629f808de9 100644 --- a/mirrord/console/src/lib.rs +++ b/mirrord/console/src/lib.rs @@ -1,9 +1,11 @@ #![warn(clippy::indexing_slicing)] +#[cfg(feature = "async-logger")] pub mod async_logger; pub mod error; pub mod logger; pub mod protocol; +#[cfg(feature = "async-logger")] pub use async_logger::init_async_logger; pub use logger::init_logger; diff --git a/mirrord/console/src/logger.rs b/mirrord/console/src/logger.rs index 6b9034144fc..235023934d4 100644 --- a/mirrord/console/src/logger.rs +++ b/mirrord/console/src/logger.rs @@ -1,7 +1,7 @@ use std::{io::BufWriter, net::TcpStream, sync::Mutex}; use log::LevelFilter; -use mirrord_intproxy::codec::SyncEncoder; +use mirrord_intproxy_protocol::codec::SyncEncoder; use crate::{ error::Result, diff --git a/mirrord/console/src/main.rs b/mirrord/console/src/main.rs index b8511d01567..0a927347fa3 100644 --- a/mirrord/console/src/main.rs +++ b/mirrord/console/src/main.rs @@ -1,6 +1,6 @@ use bincode::Decode; use mirrord_console::protocol::{Hello, Record}; -use mirrord_intproxy::codec::AsyncDecoder; +use mirrord_intproxy_protocol::codec::AsyncDecoder; use tokio::{ io::BufReader, net::{TcpListener, TcpStream}, diff --git a/mirrord/intproxy/Cargo.toml b/mirrord/intproxy/Cargo.toml index 0d9459d2d02..d397d128bbe 100644 --- a/mirrord/intproxy/Cargo.toml +++ b/mirrord/intproxy/Cargo.toml @@ -20,20 +20,16 @@ mirrord-config = { path = "../config" } mirrord-kube = { path = "../kube" } mirrord-operator = { path = "../operator", features = ["client"] } mirrord-protocol = { path = "../protocol" } +mirrord-intproxy-protocol = { path = "./protocol", features = ["codec-async"] } -bincode.workspace = true -futures.workspace = true serde.workspace = true -serde_json.workspace = true thiserror.workspace = true tokio.workspace = true tracing.workspace = true -tokio-util.workspace = true tokio-stream.workspace = true hyper = { workspace = true, features = ["client", "http1", "http2"] } hyper-util.workspace = true http-body-util.workspace = true -fancy-regex.workspace = true bytes.workspace = true rand = "0.8" diff --git a/mirrord/intproxy/protocol/Cargo.toml b/mirrord/intproxy/protocol/Cargo.toml new file mode 100644 index 00000000000..cab6e74c161 --- /dev/null +++ b/mirrord/intproxy/protocol/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "mirrord-intproxy-protocol" +version.workspace = true +authors.workspace = true +description.workspace = true +documentation.workspace = true +readme.workspace = true +homepage.workspace = true +repository.workspace = true +license.workspace = true +keywords.workspace = true +categories.workspace = true +publish.workspace = true +edition.workspace = true + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +mirrord-protocol = { path = "../../protocol" } + +bincode.workspace = true +thiserror = { workspace = true, optional = true } +tokio = { workspace = true, optional = true } + +[features] +codec = ["dep:thiserror"] +codec-async = ["codec", "dep:tokio"] diff --git a/mirrord/intproxy/src/codec.rs b/mirrord/intproxy/protocol/src/codec.rs similarity index 58% rename from mirrord/intproxy/src/codec.rs rename to mirrord/intproxy/protocol/src/codec.rs index 0572d4931f9..f809d775053 100644 --- a/mirrord/intproxy/src/codec.rs +++ b/mirrord/intproxy/protocol/src/codec.rs @@ -1,6 +1,6 @@ //! Custom codec used in `layer <-> proxy` communication. //! Supports both synchronous (required by the layer) and asynchronous (convenient for the proxy) -//! IO. +//! IO. Asynchronous IO is feature-gated with the `codec-async` feature. //! //! An encoded message consists of two parts: //! * prefix: 4 bytes containing payload length in bytes (big-endian [`u32`]) @@ -16,10 +16,11 @@ use bincode::{ Decode, Encode, }; use thiserror::Error; -use tokio::{ - io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}, - net::tcp::{OwnedReadHalf, OwnedWriteHalf}, -}; + +#[cfg(feature = "codec-async")] +mod codec_async; +#[cfg(feature = "codec-async")] +pub use codec_async::*; /// Errors that can occur when using this codec. #[derive(Error, Debug)] @@ -164,119 +165,3 @@ pub fn make_sync_framed( Ok((sender, receiver)) } - -/// Handles sending messages of type `T` through the underlying [AsyncWrite] of type `W`. -#[derive(Debug)] -pub struct AsyncEncoder { - buffer: Vec, - writer: W, - _phantom: std::marker::PhantomData T>, -} - -impl AsyncEncoder { - /// Wraps the underlying IO handler. - pub fn new(writer: W) -> Self { - Self { - buffer: Vec::with_capacity(BUFFER_SIZE), - writer, - _phantom: Default::default(), - } - } - - /// Unwraps the underlying IO handler. - pub fn into_inner(self) -> W { - self.writer - } -} - -impl AsyncEncoder -where - T: Encode, - W: AsyncWrite + Unpin, -{ - /// Encodes the given value into the inner IO handler. - pub async fn send(&mut self, value: &T) -> Result<()> { - self.buffer.resize(PREFIX_BYTES, 0); - let bytes: u32 = - bincode::encode_into_std_write(value, &mut self.buffer, bincode::config::standard())? - .try_into()?; - self.buffer - .get_mut(..PREFIX_BYTES) - .expect("buffer to short") - .copy_from_slice(&bytes.to_be_bytes()); - - self.writer.write_all(&self.buffer).await?; - - Ok(()) - } - - /// Flushes the inner IO handler. - pub async fn flush(&mut self) -> Result<()> { - self.writer.flush().await.map_err(Into::into) - } -} - -/// Handles receiving messages of type `T` from the underlying [AsyncRead] of type `W`. -#[derive(Debug)] -pub struct AsyncDecoder { - buffer: Vec, - reader: R, - _phantom: std::marker::PhantomData T>, -} - -impl AsyncDecoder { - /// Wraps the underlying IO handler. - pub fn new(reader: R) -> Self { - Self { - buffer: Vec::with_capacity(BUFFER_SIZE), - reader, - _phantom: Default::default(), - } - } - - /// Unwraps the underlying IO handler. - pub fn into_inner(self) -> R { - self.reader - } -} - -impl AsyncDecoder -where - T: Decode, - R: AsyncRead + Unpin, -{ - /// Decodes the next message from the underlying IO handler. - /// Does not read any excessive bytes. - pub async fn receive(&mut self) -> Result> { - let mut len_buffer = [0; 4]; - match self.reader.read_exact(&mut len_buffer).await { - Ok(..) => {} - Err(e) if e.kind() == ErrorKind::UnexpectedEof => return Ok(None), - Err(e) => Err(e)?, - } - let len = u32::from_be_bytes(len_buffer); - - self.buffer.resize(len as usize, 0); - self.reader.read_exact(&mut self.buffer).await?; - - let value = bincode::decode_from_slice(&self.buffer, bincode::config::standard())?.0; - - Ok(Some(value)) - } -} - -/// Creates a new pair of [`AsyncEncoder`] and [`AsyncDecoder`], using the given asynchronous -/// [`TcpStream`](tokio::net::TcpStream). -pub fn make_async_framed( - stream: tokio::net::TcpStream, -) -> ( - AsyncEncoder, - AsyncDecoder, -) { - let (reader, writer) = stream.into_split(); - - let sender = AsyncEncoder::new(writer); - let receiver = AsyncDecoder::new(reader); - - (sender, receiver) -} diff --git a/mirrord/intproxy/protocol/src/codec/codec_async.rs b/mirrord/intproxy/protocol/src/codec/codec_async.rs new file mode 100644 index 00000000000..894f0b511d3 --- /dev/null +++ b/mirrord/intproxy/protocol/src/codec/codec_async.rs @@ -0,0 +1,125 @@ +use std::io::ErrorKind; + +use bincode::{Decode, Encode}; +use tokio::{ + io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}, + net::tcp::{OwnedReadHalf, OwnedWriteHalf}, +}; + +use super::{Result, BUFFER_SIZE, PREFIX_BYTES}; + +/// Handles sending messages of type `T` through the underlying [AsyncWrite] of type `W`. +#[derive(Debug)] +pub struct AsyncEncoder { + buffer: Vec, + writer: W, + _phantom: std::marker::PhantomData T>, +} + +impl AsyncEncoder { + /// Wraps the underlying IO handler. + pub fn new(writer: W) -> Self { + Self { + buffer: Vec::with_capacity(BUFFER_SIZE), + writer, + _phantom: Default::default(), + } + } + + /// Unwraps the underlying IO handler. + pub fn into_inner(self) -> W { + self.writer + } +} + +impl AsyncEncoder +where + T: Encode, + W: AsyncWrite + Unpin, +{ + /// Encodes the given value into the inner IO handler. + pub async fn send(&mut self, value: &T) -> Result<()> { + self.buffer.resize(PREFIX_BYTES, 0); + let bytes: u32 = + bincode::encode_into_std_write(value, &mut self.buffer, bincode::config::standard())? + .try_into()?; + self.buffer + .get_mut(..PREFIX_BYTES) + .expect("buffer to short") + .copy_from_slice(&bytes.to_be_bytes()); + + self.writer.write_all(&self.buffer).await?; + + Ok(()) + } + + /// Flushes the inner IO handler. + pub async fn flush(&mut self) -> Result<()> { + self.writer.flush().await.map_err(Into::into) + } +} + +/// Handles receiving messages of type `T` from the underlying [AsyncRead] of type `W`. +#[derive(Debug)] +pub struct AsyncDecoder { + buffer: Vec, + reader: R, + _phantom: std::marker::PhantomData T>, +} + +impl AsyncDecoder { + /// Wraps the underlying IO handler. + pub fn new(reader: R) -> Self { + Self { + buffer: Vec::with_capacity(BUFFER_SIZE), + reader, + _phantom: Default::default(), + } + } + + /// Unwraps the underlying IO handler. + pub fn into_inner(self) -> R { + self.reader + } +} + +impl AsyncDecoder +where + T: Decode, + R: AsyncRead + Unpin, +{ + /// Decodes the next message from the underlying IO handler. + /// Does not read any excessive bytes. + pub async fn receive(&mut self) -> Result> { + let mut len_buffer = [0; 4]; + match self.reader.read_exact(&mut len_buffer).await { + Ok(..) => {} + Err(e) if e.kind() == ErrorKind::UnexpectedEof => return Ok(None), + Err(e) => Err(e)?, + } + let len = u32::from_be_bytes(len_buffer); + + self.buffer.resize(len as usize, 0); + self.reader.read_exact(&mut self.buffer).await?; + + let value = bincode::decode_from_slice(&self.buffer, bincode::config::standard())?.0; + + Ok(Some(value)) + } +} + +/// Creates a new pair of [`AsyncEncoder`] and [`AsyncDecoder`], using the given asynchronous +/// [`TcpStream`](tokio::net::TcpStream). +pub fn make_async_framed( + stream: tokio::net::TcpStream, +) -> ( + AsyncEncoder, + AsyncDecoder, +) { + let (reader, writer) = stream.into_split(); + + let sender = AsyncEncoder::new(writer); + let receiver = AsyncDecoder::new(reader); + + (sender, receiver) +} diff --git a/mirrord/intproxy/src/protocol.rs b/mirrord/intproxy/protocol/src/lib.rs similarity index 99% rename from mirrord/intproxy/src/protocol.rs rename to mirrord/intproxy/protocol/src/lib.rs index f54b245cd0b..23d25fde913 100644 --- a/mirrord/intproxy/src/protocol.rs +++ b/mirrord/intproxy/protocol/src/lib.rs @@ -23,8 +23,8 @@ use mirrord_protocol::{ FileRequest, FileResponse, Port, RemoteResult, }; -use crate::{bind_nested, impl_request}; - +#[cfg(feature = "codec")] +pub mod codec; mod macros; /// An identifier for a message sent from the layer to the internal proxy. diff --git a/mirrord/intproxy/src/protocol/macros.rs b/mirrord/intproxy/protocol/src/macros.rs similarity index 100% rename from mirrord/intproxy/src/protocol/macros.rs rename to mirrord/intproxy/protocol/src/macros.rs diff --git a/mirrord/intproxy/src/error.rs b/mirrord/intproxy/src/error.rs index 6b068d2cb4f..74419e6a035 100644 --- a/mirrord/intproxy/src/error.rs +++ b/mirrord/intproxy/src/error.rs @@ -1,14 +1,13 @@ use std::io; +use mirrord_intproxy_protocol::{codec::CodecError, LayerToProxyMessage}; use mirrord_protocol::DaemonMessage; use thiserror::Error; use crate::{ agent_conn::{AgentChannelError, AgentConnectionError}, - codec::CodecError, layer_initializer::LayerInitializerError, ping_pong::PingPongError, - protocol::LayerToProxyMessage, proxies::{incoming::IncomingProxyError, outgoing::OutgoingProxyError}, request_queue::RequestQueueEmpty, MainTaskId, diff --git a/mirrord/intproxy/src/layer_conn.rs b/mirrord/intproxy/src/layer_conn.rs index 87b0f43b2c3..19d8d124954 100644 --- a/mirrord/intproxy/src/layer_conn.rs +++ b/mirrord/intproxy/src/layer_conn.rs @@ -1,5 +1,9 @@ //! Implementation of `layer <-> proxy` connection through a [`TcpStream`]. +use mirrord_intproxy_protocol::{ + codec::{self, AsyncDecoder, AsyncEncoder, CodecError}, + LayerId, LayerToProxyMessage, LocalMessage, ProxyToLayerMessage, +}; use tokio::net::{ tcp::{OwnedReadHalf, OwnedWriteHalf}, TcpStream, @@ -7,9 +11,7 @@ use tokio::net::{ use crate::{ background_tasks::{BackgroundTask, MessageBus}, - codec::{self, AsyncDecoder, AsyncEncoder, CodecError}, main_tasks::FromLayer, - protocol::{LayerId, LayerToProxyMessage, LocalMessage, ProxyToLayerMessage}, ProxyMessage, }; diff --git a/mirrord/intproxy/src/layer_initializer.rs b/mirrord/intproxy/src/layer_initializer.rs index 967565a26d5..7dd5764dea3 100644 --- a/mirrord/intproxy/src/layer_initializer.rs +++ b/mirrord/intproxy/src/layer_initializer.rs @@ -1,15 +1,15 @@ use std::io; +use mirrord_intproxy_protocol::{ + codec::{AsyncDecoder, AsyncEncoder, CodecError}, + LayerId, LayerToProxyMessage, LocalMessage, NewSessionRequest, ProxyToLayerMessage, +}; use thiserror::Error; use tokio::net::{TcpListener, TcpStream}; use crate::{ background_tasks::{BackgroundTask, MessageBus}, - codec::{AsyncDecoder, AsyncEncoder, CodecError}, main_tasks::NewLayer, - protocol::{ - LayerId, LayerToProxyMessage, LocalMessage, NewSessionRequest, ProxyToLayerMessage, - }, ProxyMessage, }; diff --git a/mirrord/intproxy/src/lib.rs b/mirrord/intproxy/src/lib.rs index a4779b47e7d..97821f1ea7e 100644 --- a/mirrord/intproxy/src/lib.rs +++ b/mirrord/intproxy/src/lib.rs @@ -10,9 +10,9 @@ use layer_conn::LayerConnection; use layer_initializer::LayerInitializer; use main_tasks::{FromLayer, LayerForked, MainTaskId, ProxyMessage, ToLayer}; use mirrord_config::LayerConfig; +use mirrord_intproxy_protocol::{LayerId, LayerToProxyMessage, LocalMessage}; use mirrord_protocol::{ClientMessage, DaemonMessage, LogLevel, CLIENT_READY_FOR_LOGS}; use ping_pong::{AgentMessageNotification, PingPong}; -use protocol::LayerId; use proxies::{ incoming::{IncomingProxy, IncomingProxyMessage}, outgoing::{OutgoingProxy, OutgoingProxyMessage}, @@ -21,22 +21,17 @@ use proxies::{ use tokio::{net::TcpListener, time}; use crate::{ - agent_conn::AgentConnectInfo, - background_tasks::TaskError, - error::IntProxyError, + agent_conn::AgentConnectInfo, background_tasks::TaskError, error::IntProxyError, main_tasks::LayerClosed, - protocol::{LayerToProxyMessage, LocalMessage}, }; pub mod agent_conn; mod background_tasks; -pub mod codec; pub mod error; mod layer_conn; mod layer_initializer; mod main_tasks; mod ping_pong; -pub mod protocol; mod proxies; mod remote_resources; mod request_queue; diff --git a/mirrord/intproxy/src/main_tasks.rs b/mirrord/intproxy/src/main_tasks.rs index c8989c1682d..a6ad4a54b77 100644 --- a/mirrord/intproxy/src/main_tasks.rs +++ b/mirrord/intproxy/src/main_tasks.rs @@ -1,10 +1,9 @@ use std::fmt; +use mirrord_intproxy_protocol::{LayerId, LayerToProxyMessage, MessageId, ProxyToLayerMessage}; use mirrord_protocol::{ClientMessage, DaemonMessage}; use tokio::net::TcpStream; -use crate::protocol::{LayerId, LayerToProxyMessage, MessageId, ProxyToLayerMessage}; - /// Messages sent back to the [`IntProxy`](crate::IntProxy) from the main background tasks. See /// [`MainTaskId`]. #[derive(Debug)] diff --git a/mirrord/intproxy/src/proxies/incoming.rs b/mirrord/intproxy/src/proxies/incoming.rs index 1d778526d8a..761dac2af44 100644 --- a/mirrord/intproxy/src/proxies/incoming.rs +++ b/mirrord/intproxy/src/proxies/incoming.rs @@ -7,6 +7,10 @@ use std::{ }; use hyper::Version; +use mirrord_intproxy_protocol::{ + ConnMetadataRequest, ConnMetadataResponse, IncomingRequest, IncomingResponse, LayerId, + MessageId, PortSubscribe, PortSubscription, PortUnsubscribe, ProxyToLayerMessage, +}; use mirrord_protocol::{ tcp::{DaemonTcp, HttpRequestFallback, HttpResponseFallback, NewTcpConnection}, ConnectionId, Port, @@ -22,10 +26,6 @@ use self::{ use crate::{ background_tasks::{BackgroundTask, BackgroundTasks, MessageBus, TaskSender, TaskUpdate}, main_tasks::{LayerClosed, LayerForked, ToLayer}, - protocol::{ - ConnMetadataRequest, ConnMetadataResponse, IncomingRequest, IncomingResponse, LayerId, - MessageId, PortSubscribe, PortSubscription, PortUnsubscribe, ProxyToLayerMessage, - }, remote_resources::RemoteResources, request_queue::{RequestQueue, RequestQueueEmpty}, ProxyMessage, diff --git a/mirrord/intproxy/src/proxies/incoming/port_subscription_ext.rs b/mirrord/intproxy/src/proxies/incoming/port_subscription_ext.rs index 242a37522e5..fad09a1bde0 100644 --- a/mirrord/intproxy/src/proxies/incoming/port_subscription_ext.rs +++ b/mirrord/intproxy/src/proxies/incoming/port_subscription_ext.rs @@ -1,12 +1,12 @@ //! Utilities for handling toggleable `steal` feature in [`IncomingProxy`](super::IncomingProxy). +use mirrord_intproxy_protocol::PortSubscription; use mirrord_protocol::{ tcp::{HttpResponseFallback, LayerTcp, LayerTcpSteal, StealType, TcpData}, ClientMessage, ConnectionId, Port, }; use super::InterceptorMessageOut; -use crate::protocol::PortSubscription; /// Retrieves subscribed port from the given [`StealType`]. fn get_port(steal_type: &StealType) -> Port { diff --git a/mirrord/intproxy/src/proxies/outgoing.rs b/mirrord/intproxy/src/proxies/outgoing.rs index 84aeee84ef0..e9eb59b796b 100644 --- a/mirrord/intproxy/src/proxies/outgoing.rs +++ b/mirrord/intproxy/src/proxies/outgoing.rs @@ -2,6 +2,10 @@ use std::{collections::HashMap, fmt, io}; +use mirrord_intproxy_protocol::{ + LayerId, MessageId, NetProtocol, OutgoingConnectRequest, OutgoingConnectResponse, + ProxyToLayerMessage, +}; use mirrord_protocol::{ outgoing::{tcp::DaemonTcpOutgoing, udp::DaemonUdpOutgoing, DaemonConnect, DaemonRead}, ConnectionId, RemoteResult, ResponseError, @@ -12,10 +16,6 @@ use self::interceptor::Interceptor; use crate::{ background_tasks::{BackgroundTask, BackgroundTasks, MessageBus, TaskSender, TaskUpdate}, main_tasks::ToLayer, - protocol::{ - LayerId, MessageId, NetProtocol, OutgoingConnectRequest, OutgoingConnectResponse, - ProxyToLayerMessage, - }, proxies::outgoing::net_protocol_ext::NetProtocolExt, request_queue::{RequestQueue, RequestQueueEmpty}, ProxyMessage, diff --git a/mirrord/intproxy/src/proxies/outgoing/net_protocol_ext.rs b/mirrord/intproxy/src/proxies/outgoing/net_protocol_ext.rs index e35bedf5649..8c57de8bdae 100644 --- a/mirrord/intproxy/src/proxies/outgoing/net_protocol_ext.rs +++ b/mirrord/intproxy/src/proxies/outgoing/net_protocol_ext.rs @@ -7,6 +7,7 @@ use std::{ path::PathBuf, }; +use mirrord_intproxy_protocol::NetProtocol; use mirrord_protocol::{ outgoing::{ tcp::LayerTcpOutgoing, udp::LayerUdpOutgoing, LayerClose, LayerConnect, LayerWrite, @@ -21,8 +22,6 @@ use tokio::{ net::{TcpListener, TcpStream, UdpSocket, UnixListener, UnixStream}, }; -use crate::protocol::NetProtocol; - /// Trait for [`NetProtocol`] that handles differences in [`mirrord_protocol::outgoing`] between /// network protocols. Allows to unify logic. pub trait NetProtocolExt: Sized { diff --git a/mirrord/intproxy/src/proxies/simple.rs b/mirrord/intproxy/src/proxies/simple.rs index 78adb7e12cf..caf797fc632 100644 --- a/mirrord/intproxy/src/proxies/simple.rs +++ b/mirrord/intproxy/src/proxies/simple.rs @@ -1,6 +1,7 @@ //! The most basic proxying logic. Handles cases when the only job to do in the internal proxy is to //! pass requests and responses between the layer and the agent. +use mirrord_intproxy_protocol::{LayerId, MessageId, ProxyToLayerMessage}; use mirrord_protocol::{ dns::{GetAddrInfoRequest, GetAddrInfoResponse}, file::{CloseDirRequest, CloseFileRequest, OpenDirResponse, OpenFileResponse}, @@ -10,7 +11,6 @@ use mirrord_protocol::{ use crate::{ background_tasks::{BackgroundTask, MessageBus}, main_tasks::{LayerClosed, LayerForked, ToLayer}, - protocol::{LayerId, MessageId, ProxyToLayerMessage}, remote_resources::RemoteResources, request_queue::{RequestQueue, RequestQueueEmpty}, ProxyMessage, diff --git a/mirrord/intproxy/src/remote_resources.rs b/mirrord/intproxy/src/remote_resources.rs index c87ae9d04c6..14f82cc8f92 100644 --- a/mirrord/intproxy/src/remote_resources.rs +++ b/mirrord/intproxy/src/remote_resources.rs @@ -3,7 +3,7 @@ use std::{ hash::Hash, }; -use crate::protocol::LayerId; +use mirrord_intproxy_protocol::LayerId; /// For tracking remote resources allocated in the agent: open files and directories, port /// subscriptions. Remote resources can be shared by multiple layer instances because of forks. diff --git a/mirrord/intproxy/src/request_queue.rs b/mirrord/intproxy/src/request_queue.rs index 7aec77e6ac6..77513ad2874 100644 --- a/mirrord/intproxy/src/request_queue.rs +++ b/mirrord/intproxy/src/request_queue.rs @@ -13,10 +13,9 @@ use std::{collections::VecDeque, fmt}; +use mirrord_intproxy_protocol::{LayerId, MessageId}; use thiserror::Error; -use crate::protocol::{LayerId, MessageId}; - /// Erorr returned when the proxy attempts to retrieve [`MessageId`] and [`LayerId`] of a request /// corresponding to a response received from the agent, but the [`RequestQueue`] is empty. This /// error should never happen. diff --git a/mirrord/layer/Cargo.toml b/mirrord/layer/Cargo.toml index c89ea55ff30..46b838449de 100644 --- a/mirrord/layer/Cargo.toml +++ b/mirrord/layer/Cargo.toml @@ -19,7 +19,7 @@ mirrord-config = { path = "../config"} mirrord-protocol = { path = "../protocol"} mirrord-layer-macro = { path = "./macro"} mirrord-console = { path = "../console" } -mirrord-intproxy = { path = "../intproxy" } +mirrord-intproxy-protocol = { path = "../intproxy/protocol", features = ["codec"] } ctor = "0.2" libc.workspace = true diff --git a/mirrord/layer/src/common.rs b/mirrord/layer/src/common.rs index 53cb4d8c150..00aa420859a 100644 --- a/mirrord/layer/src/common.rs +++ b/mirrord/layer/src/common.rs @@ -2,7 +2,7 @@ use std::{ffi::CStr, fmt::Debug, path::PathBuf}; use libc::c_char; -use mirrord_intproxy::protocol::{IsLayerRequest, IsLayerRequestWithResponse, MessageId}; +use mirrord_intproxy_protocol::{IsLayerRequest, IsLayerRequestWithResponse, MessageId}; use mirrord_protocol::file::OpenOptionsInternal; #[cfg(target_os = "macos")] use mirrord_sip::{MIRRORD_TEMP_BIN_DIR_CANONIC_STRING, MIRRORD_TEMP_BIN_DIR_STRING}; diff --git a/mirrord/layer/src/lib.rs b/mirrord/layer/src/lib.rs index 76591779625..1b341a28565 100644 --- a/mirrord/layer/src/lib.rs +++ b/mirrord/layer/src/lib.rs @@ -81,7 +81,7 @@ use mirrord_config::{ feature::{fs::FsModeConfig, network::incoming::IncomingMode}, LayerConfig, }; -use mirrord_intproxy::protocol::NewSessionRequest; +use mirrord_intproxy_protocol::NewSessionRequest; use mirrord_layer_macro::{hook_fn, hook_guard_fn}; use proxy_connection::ProxyConnection; use setup::LayerSetup; diff --git a/mirrord/layer/src/proxy_connection.rs b/mirrord/layer/src/proxy_connection.rs index f9450010870..0bff456e33c 100644 --- a/mirrord/layer/src/proxy_connection.rs +++ b/mirrord/layer/src/proxy_connection.rs @@ -10,12 +10,10 @@ use std::{ time::Duration, }; -use mirrord_intproxy::{ +use mirrord_intproxy_protocol::{ codec::{self, CodecError, SyncDecoder, SyncEncoder}, - protocol::{ - IsLayerRequest, IsLayerRequestWithResponse, LayerId, LayerToProxyMessage, LocalMessage, - MessageId, NewSessionRequest, ProxyToLayerMessage, - }, + IsLayerRequest, IsLayerRequestWithResponse, LayerId, LayerToProxyMessage, LocalMessage, + MessageId, NewSessionRequest, ProxyToLayerMessage, }; use thiserror::Error; diff --git a/mirrord/layer/src/setup.rs b/mirrord/layer/src/setup.rs index c395391bed4..c525241b480 100644 --- a/mirrord/layer/src/setup.rs +++ b/mirrord/layer/src/setup.rs @@ -8,7 +8,7 @@ use mirrord_config::{ util::VecOrSingle, LayerConfig, }; -use mirrord_intproxy::protocol::PortSubscription; +use mirrord_intproxy_protocol::PortSubscription; use mirrord_protocol::{ tcp::{Filter, HttpFilter, StealType}, Port, diff --git a/mirrord/layer/src/socket.rs b/mirrord/layer/src/socket.rs index aa7f2613a5f..f823b28985c 100644 --- a/mirrord/layer/src/socket.rs +++ b/mirrord/layer/src/socket.rs @@ -13,7 +13,7 @@ use libc::{c_int, sockaddr, socklen_t}; use mirrord_config::feature::network::outgoing::{ AddressFilter, OutgoingConfig, OutgoingFilter, OutgoingFilterConfig, ProtocolFilter, }; -use mirrord_intproxy::protocol::{NetProtocol, PortUnsubscribe}; +use mirrord_intproxy_protocol::{NetProtocol, PortUnsubscribe}; use mirrord_protocol::outgoing::SocketAddress; use socket2::SockAddr; use tracing::warn; diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index fc7ca198b27..a8629e12125 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -14,7 +14,7 @@ use std::{ use libc::{c_int, c_void, sockaddr, socklen_t}; use mirrord_config::feature::network::incoming::IncomingMode; -use mirrord_intproxy::protocol::{ +use mirrord_intproxy_protocol::{ ConnMetadataRequest, ConnMetadataResponse, NetProtocol, OutgoingConnectRequest, OutgoingConnectResponse, PortSubscribe, }; From ce39d468ab55e172c17c4e9cae26426f65d4d413 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Wed, 8 Nov 2023 08:41:29 +0200 Subject: [PATCH 04/10] Local file filter now applies to directory listing [regex] and not just underlying files (#2060) --- changelog.d/+local-file-filter.fixed.md | 1 + .../src/file/filter/read_local_by_default.rs | 22 +++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) create mode 100644 changelog.d/+local-file-filter.fixed.md diff --git a/changelog.d/+local-file-filter.fixed.md b/changelog.d/+local-file-filter.fixed.md new file mode 100644 index 00000000000..430eca71e66 --- /dev/null +++ b/changelog.d/+local-file-filter.fixed.md @@ -0,0 +1 @@ +Local file filter now applies to directory listing [regex] and not just underlying files \ No newline at end of file diff --git a/mirrord/layer/src/file/filter/read_local_by_default.rs b/mirrord/layer/src/file/filter/read_local_by_default.rs index 6cbbd3cabcd..6ff4d73f545 100644 --- a/mirrord/layer/src/file/filter/read_local_by_default.rs +++ b/mirrord/layer/src/file/filter/read_local_by_default.rs @@ -17,23 +17,23 @@ pub fn regex_set_builder() -> RegexSetBuilder { r"^.+\.pth$", r"^.+\.plist$", r"^.*venv\.cfg$", - r"^/proc/.*$", - r"^/sys/.*$", - r"^/lib/.*$", - r"^/etc/.*$", + r"^/proc(/|$)", + r"^/sys(/|$)", + r"^/lib(/|$)", + r"^/etc(/|$)", r"^/usr(/|$).*$", r"^/home(/|$).*$", - r"^/bin/.*$", - r"^/sbin/.*$", - r"^/dev/.*$", + r"^/bin(/|$)", + r"^/sbin(/|$)", + r"^/dev(/|$)", r"^/opt(/|$)", r"^/tmp(/|$)", - r"^/snap/.*$", + r"^/snap(/|$)", // support for nixOS. - r"^/nix/.*$", + r"^/nix(/|$)", r".+\.asdf/.+", - r"^/home/iojs/.*$", - r"^/home/runner/.*$", + r"^/home/iojs(/|$)", + r"^/home/runner(/|$)", // dotnet: `/tmp/clr-debug-pipe-1` r"^.*clr-.*-pipe-.*$", // dotnet: `/home/{username}/{project}.pdb` From 62ceb1a15429e1494f39bdd909de2d576bf76a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Smolarek?= <34063647+Razz4780@users.noreply.github.com> Date: Wed, 8 Nov 2023 12:30:14 +0100 Subject: [PATCH 05/10] Fix port mapping (#2061) * Fixed 'listen' detour * Added integration test * Added test build to CI * Added changelog entry * Fix existing tests * Format --- .github/workflows/ci.yaml | 6 ++++ changelog.d/2058.fixed.md | 1 + mirrord/layer/src/socket/ops.rs | 17 +++++++--- .../layer/tests/apps/issue2058/issue2058.rs | 13 +++++++ mirrord/layer/tests/common/mod.rs | 9 +++-- mirrord/layer/tests/port_mapping.rs | 34 +++++++++++++++++++ 6 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 changelog.d/2058.fixed.md create mode 100644 mirrord/layer/tests/apps/issue2058/issue2058.rs create mode 100644 mirrord/layer/tests/port_mapping.rs diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9bde555610e..e93b0dc8b84 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -238,6 +238,9 @@ jobs: - run: | cd mirrord/layer/tests/apps/issue1458portnot53 rustc issue1458portnot53.rs --out-dir target + - run: | + cd mirrord/layer/tests/apps/issue2058 + rustc issue2058.rs --out-dir target # For the `java_temurin_sip` test. - uses: sdkman/sdkman-action@b1f9b696c79148b66d3d3a06f7ea801820318d0f id: sdkman @@ -350,6 +353,9 @@ jobs: - run: | cd mirrord/layer/tests/apps/issue1458portnot53 rustc issue1458portnot53.rs --out-dir target + - run: | + cd mirrord/layer/tests/apps/issue2058 + rustc issue2058.rs --out-dir target - uses: actions/setup-go@v4 with: go-version: "1.18" diff --git a/changelog.d/2058.fixed.md b/changelog.d/2058.fixed.md new file mode 100644 index 00000000000..d9e2d6cbb85 --- /dev/null +++ b/changelog.d/2058.fixed.md @@ -0,0 +1 @@ +Fixed `port_mapping` feature. \ No newline at end of file diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index a8629e12125..38b12e593dd 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -254,11 +254,13 @@ pub(super) fn listen(sockfd: RawFd, backlog: c_int) -> Detour { .bypass(Bypass::LocalFdNotFound(sockfd))? }; - if matches!(crate::setup().incoming_config().mode, IncomingMode::Off) { + let setup = crate::setup(); + + if matches!(setup.incoming_config().mode, IncomingMode::Off) { return Detour::Bypass(Bypass::DisabledIncoming); } - if crate::setup().targetless() { + if setup.targetless() { warn!( "Listening while running targetless. A targetless agent is not exposed by \ any service. Therefore, letting this port bind happen locally instead of on the \ @@ -281,11 +283,16 @@ pub(super) fn listen(sockfd: RawFd, backlog: c_int) -> Detour { Err(error)? } + let mapped_port = setup + .incoming_config() + .port_mapping + .get_by_left(&requested_address.port()) + .copied() + .unwrap_or_else(|| requested_address.port()); + common::make_proxy_request_with_response(PortSubscribe { listening_on: address, - subscription: crate::setup() - .incoming_mode() - .subscription(requested_address.port()), + subscription: setup.incoming_mode().subscription(mapped_port), })??; // this log message is expected by some E2E tests diff --git a/mirrord/layer/tests/apps/issue2058/issue2058.rs b/mirrord/layer/tests/apps/issue2058/issue2058.rs new file mode 100644 index 00000000000..fc7a87c003a --- /dev/null +++ b/mirrord/layer/tests/apps/issue2058/issue2058.rs @@ -0,0 +1,13 @@ +use std::{net::TcpListener, io::Read}; + +const LISTEN_ON: &'static str = "0.0.0.0:9999"; +const EXPECTED_MESSAGE: &'static [u8] = b"HELLO"; + +fn main() { + let listener = TcpListener::bind(LISTEN_ON).unwrap(); + let (mut stream, _peer) = listener.accept().unwrap(); + + let mut buff = [0_u8; 10]; + let bytes_read = stream.read(&mut buff[..]).unwrap(); + assert_eq!(&buff[..bytes_read], EXPECTED_MESSAGE); +} diff --git a/mirrord/layer/tests/common/mod.rs b/mirrord/layer/tests/common/mod.rs index e512d4e8e60..efcb732a7ed 100644 --- a/mirrord/layer/tests/common/mod.rs +++ b/mirrord/layer/tests/common/mod.rs @@ -651,6 +651,7 @@ pub enum Application { RustListenPorts, Fork, OpenFile, + RustIssue2058, // For running applications with the executable and arguments determined at runtime. DynamicApp(String, Vec), } @@ -785,7 +786,8 @@ impl Application { "{}/{}", env!("CARGO_MANIFEST_DIR"), "tests/apps/open_file/out.c_test_app", - ), // String::from("tests/apps/open_file/out.c_test_app"), + ), + Application::RustIssue2058 => String::from("tests/apps/issue2058/target/issue2058"), Application::DynamicApp(exe, _) => exe.clone(), } } @@ -874,6 +876,7 @@ impl Application { | Application::Go19SelfOpen | Application::Go19DirBypass | Application::Go20DirBypass + | Application::RustIssue2058 | Application::OpenFile => vec![], Application::RustOutgoingUdp => ["--udp", RUST_OUTGOING_LOCAL, RUST_OUTGOING_PEERS] .into_iter() @@ -896,7 +899,8 @@ impl Application { | Application::NodeHTTP | Application::RustIssue1054 | Application::PythonFlaskHTTP => 80, - Application::PythonFastApiHTTP => 9999, + // mapped from 9999 in `configs/port_mapping.json` + Application::PythonFastApiHTTP => 1234, Application::RustIssue1123 => 41222, Application::PythonListen => 21232, Application::PythonDontLoad @@ -941,6 +945,7 @@ impl Application { | Application::OpenFile | Application::DynamicApp(..) => unimplemented!("shouldn't get here"), Application::PythonSelfConnect => 1337, + Application::RustIssue2058 => 1234, } } diff --git a/mirrord/layer/tests/port_mapping.rs b/mirrord/layer/tests/port_mapping.rs new file mode 100644 index 00000000000..c7f589067b9 --- /dev/null +++ b/mirrord/layer/tests/port_mapping.rs @@ -0,0 +1,34 @@ +#![feature(assert_matches)] +#![warn(clippy::indexing_slicing)] + +use std::{path::PathBuf, time::Duration}; + +use rstest::rstest; + +mod common; +pub use common::*; + +#[rstest] +#[tokio::test] +#[timeout(Duration::from_secs(20))] +async fn port_mapping( + #[values(Application::RustIssue2058)] application: Application, + dylib_path: &PathBuf, + config_dir: &PathBuf, +) { + let (mut test_process, mut intproxy) = application + .start_process_with_layer_and_port( + dylib_path, + vec![], + Some(config_dir.join("port_mapping.json").to_str().unwrap()), + ) + .await; + + println!("Application subscribed to port, sending data."); + + intproxy + .send_connection_then_data("HELLO", application.get_app_port()) + .await; + + test_process.wait_assert_success().await; +} From 0e390f43c5e811f24454e0c397647c7dd0058955 Mon Sep 17 00:00:00 2001 From: Eyal Bukchin Date: Wed, 8 Nov 2023 15:01:38 +0200 Subject: [PATCH 06/10] Update to 3.75.0 (#2062) --- CHANGELOG.md | 27 +++++++++++++ Cargo.lock | 54 ++++++++++++------------- Cargo.toml | 2 +- changelog.d/+local-file-filter.fixed.md | 1 - changelog.d/1974.added.md | 1 - changelog.d/2039.internal.md | 1 - changelog.d/2047.fixed.md | 1 - changelog.d/2053.added.md | 1 - changelog.d/2058.fixed.md | 1 - 9 files changed, 55 insertions(+), 34 deletions(-) delete mode 100644 changelog.d/+local-file-filter.fixed.md delete mode 100644 changelog.d/1974.added.md delete mode 100644 changelog.d/2039.internal.md delete mode 100644 changelog.d/2047.fixed.md delete mode 100644 changelog.d/2053.added.md delete mode 100644 changelog.d/2058.fixed.md diff --git a/CHANGELOG.md b/CHANGELOG.md index f7ebcbce396..138151a7616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,33 @@ This project uses [*towncrier*](https://towncrier.readthedocs.io/) and the chang +## [3.75.0](https://github.com/metalbear-co/mirrord/tree/3.75.0) - 2023-11-08 + + +### Added + +- Added 'copy pod' operator feature to the CLI. + [#1974](https://github.com/metalbear-co/mirrord/issues/1974) +- Added option to scale down target deployment when using `copy target` + feature. [#2053](https://github.com/metalbear-co/mirrord/issues/2053) + + +### Fixed + +- Don't drop mutex in child on fork_detour, fixes bug with elixir. + [#2047](https://github.com/metalbear-co/mirrord/issues/2047) +- Fixed `port_mapping` feature. + [#2058](https://github.com/metalbear-co/mirrord/issues/2058) +- Local file filter now applies to directory listing [regex] and not just + underlying files + + +### Internal + +- Improved crates structure around internal proxy and mirrord console. + [#2039](https://github.com/metalbear-co/mirrord/issues/2039) + + ## [3.74.1](https://github.com/metalbear-co/mirrord/tree/3.74.1) - 2023-10-31 diff --git a/Cargo.lock b/Cargo.lock index 5729c6ba23b..a0ffe38b264 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "fileops" -version = "3.74.1" +version = "3.75.0" dependencies = [ "libc", ] @@ -3107,7 +3107,7 @@ dependencies = [ [[package]] name = "issue1317" -version = "3.74.1" +version = "3.75.0" dependencies = [ "actix-web", "env_logger", @@ -3119,7 +3119,7 @@ dependencies = [ [[package]] name = "issue1776" -version = "3.74.1" +version = "3.75.0" dependencies = [ "errno 0.3.5", "libc", @@ -3128,7 +3128,7 @@ dependencies = [ [[package]] name = "issue1776portnot53" -version = "3.74.1" +version = "3.75.0" dependencies = [ "libc", "socket2 0.5.5", @@ -3136,14 +3136,14 @@ dependencies = [ [[package]] name = "issue1899" -version = "3.74.1" +version = "3.75.0" dependencies = [ "libc", ] [[package]] name = "issue2001" -version = "3.74.1" +version = "3.75.0" dependencies = [ "libc", ] @@ -3431,7 +3431,7 @@ checksum = "da2479e8c062e40bf0066ffa0bc823de0a9368974af99c9f6df941d2c231e03f" [[package]] name = "listen_ports" -version = "3.74.1" +version = "3.75.0" [[package]] name = "local-channel" @@ -3657,7 +3657,7 @@ dependencies = [ [[package]] name = "mirrord" -version = "3.74.1" +version = "3.75.0" dependencies = [ "actix-codec", "anyhow", @@ -3702,7 +3702,7 @@ dependencies = [ [[package]] name = "mirrord-agent" -version = "3.74.1" +version = "3.75.0" dependencies = [ "actix-codec", "async-trait", @@ -3755,7 +3755,7 @@ dependencies = [ [[package]] name = "mirrord-analytics" -version = "3.74.1" +version = "3.75.0" dependencies = [ "assert-json-diff", "base64 0.21.5", @@ -3769,7 +3769,7 @@ dependencies = [ [[package]] name = "mirrord-auth" -version = "3.74.1" +version = "3.75.0" dependencies = [ "chrono", "fs4", @@ -3789,7 +3789,7 @@ dependencies = [ [[package]] name = "mirrord-config" -version = "3.74.1" +version = "3.75.0" dependencies = [ "bimap", "bitflags 2.4.1", @@ -3811,7 +3811,7 @@ dependencies = [ [[package]] name = "mirrord-config-derive" -version = "3.74.1" +version = "3.75.0" dependencies = [ "proc-macro2", "proc-macro2-diagnostics", @@ -3821,7 +3821,7 @@ dependencies = [ [[package]] name = "mirrord-console" -version = "3.74.1" +version = "3.75.0" dependencies = [ "bincode", "drain", @@ -3837,7 +3837,7 @@ dependencies = [ [[package]] name = "mirrord-intproxy" -version = "3.74.1" +version = "3.75.0" dependencies = [ "bytes", "http-body-util", @@ -3858,7 +3858,7 @@ dependencies = [ [[package]] name = "mirrord-intproxy-protocol" -version = "3.74.1" +version = "3.75.0" dependencies = [ "bincode", "mirrord-protocol", @@ -3868,7 +3868,7 @@ dependencies = [ [[package]] name = "mirrord-kube" -version = "3.74.1" +version = "3.75.0" dependencies = [ "actix-codec", "base64 0.21.5", @@ -3895,7 +3895,7 @@ dependencies = [ [[package]] name = "mirrord-layer" -version = "3.74.1" +version = "3.75.0" dependencies = [ "actix-codec", "anyhow", @@ -3950,7 +3950,7 @@ dependencies = [ [[package]] name = "mirrord-layer-macro" -version = "3.74.1" +version = "3.75.0" dependencies = [ "proc-macro2", "quote", @@ -3959,7 +3959,7 @@ dependencies = [ [[package]] name = "mirrord-macros" -version = "3.74.1" +version = "3.75.0" dependencies = [ "proc-macro2", "proc-macro2-diagnostics", @@ -3969,7 +3969,7 @@ dependencies = [ [[package]] name = "mirrord-operator" -version = "3.74.1" +version = "3.75.0" dependencies = [ "actix-codec", "async-trait", @@ -4003,7 +4003,7 @@ dependencies = [ [[package]] name = "mirrord-progress" -version = "3.74.1" +version = "3.75.0" dependencies = [ "enum_dispatch", "indicatif", @@ -4035,7 +4035,7 @@ dependencies = [ [[package]] name = "mirrord-sip" -version = "3.74.1" +version = "3.75.0" dependencies = [ "apple-codesign", "memchr", @@ -4359,7 +4359,7 @@ dependencies = [ [[package]] name = "outgoing" -version = "3.74.1" +version = "3.75.0" [[package]] name = "outref" @@ -5333,21 +5333,21 @@ dependencies = [ [[package]] name = "rust-bypassed-unix-socket" -version = "3.74.1" +version = "3.75.0" dependencies = [ "tokio", ] [[package]] name = "rust-e2e-fileops" -version = "3.74.1" +version = "3.75.0" dependencies = [ "libc", ] [[package]] name = "rust-unix-socket-client" -version = "3.74.1" +version = "3.75.0" dependencies = [ "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index c33bf15fc79..f7d8c538d53 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ resolver = "2" # latest commits on rustls suppress certificate verification [workspace.package] -version = "3.74.1" +version = "3.75.0" edition = "2021" license = "MIT" readme = "README.md" diff --git a/changelog.d/+local-file-filter.fixed.md b/changelog.d/+local-file-filter.fixed.md deleted file mode 100644 index 430eca71e66..00000000000 --- a/changelog.d/+local-file-filter.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Local file filter now applies to directory listing [regex] and not just underlying files \ No newline at end of file diff --git a/changelog.d/1974.added.md b/changelog.d/1974.added.md deleted file mode 100644 index a1527505c13..00000000000 --- a/changelog.d/1974.added.md +++ /dev/null @@ -1 +0,0 @@ -Added 'copy pod' operator feature to the CLI. \ No newline at end of file diff --git a/changelog.d/2039.internal.md b/changelog.d/2039.internal.md deleted file mode 100644 index aa8ac9bbaf4..00000000000 --- a/changelog.d/2039.internal.md +++ /dev/null @@ -1 +0,0 @@ -Improved crates structure around internal proxy and mirrord console. \ No newline at end of file diff --git a/changelog.d/2047.fixed.md b/changelog.d/2047.fixed.md deleted file mode 100644 index fc295eab684..00000000000 --- a/changelog.d/2047.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Don't drop mutex in child on fork_detour, fixes bug with elixir. \ No newline at end of file diff --git a/changelog.d/2053.added.md b/changelog.d/2053.added.md deleted file mode 100644 index f393af407a7..00000000000 --- a/changelog.d/2053.added.md +++ /dev/null @@ -1 +0,0 @@ -Added option to scale down target deployment when using `copy target` feature. \ No newline at end of file diff --git a/changelog.d/2058.fixed.md b/changelog.d/2058.fixed.md deleted file mode 100644 index d9e2d6cbb85..00000000000 --- a/changelog.d/2058.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Fixed `port_mapping` feature. \ No newline at end of file From b50477a5c52726e1d1101ddfb5bf2ae914515375 Mon Sep 17 00:00:00 2001 From: t4lz Date: Thu, 9 Nov 2023 15:47:30 +0100 Subject: [PATCH 07/10] Document the fact that the env configuration takes wildcard/glob patterns (#2064) * document env config pattern matching. * changelog --- changelog.d/+doc-env-wildcard.added.md | 1 + mirrord-schema.json | 6 +++--- mirrord/config/src/feature/env.rs | 10 +++++++--- 3 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 changelog.d/+doc-env-wildcard.added.md diff --git a/changelog.d/+doc-env-wildcard.added.md b/changelog.d/+doc-env-wildcard.added.md new file mode 100644 index 00000000000..da922d1eb3b --- /dev/null +++ b/changelog.d/+doc-env-wildcard.added.md @@ -0,0 +1 @@ +Documentation of `env` config pattern matching. diff --git a/mirrord-schema.json b/mirrord-schema.json index 911b7fa527b..69b1411d861 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -429,12 +429,12 @@ } }, "EnvFileConfig": { - "description": "Allows the user to set or override the local process' environment variables with the ones from the remote pod.\n\nWhich environment variables to load from the remote pod are controlled by setting either [`include`](#feature-env-include) or [`exclude`](#feature-env-exclude).\n\nSee the environment variables [reference](https://mirrord.dev/docs/reference/env/) for more details.\n\n```json { \"feature\": { \"env\": { \"include\": \"DATABASE_USER;PUBLIC_ENV\", \"exclude\": \"DATABASE_PASSWORD;SECRET_ENV\", \"override\": { \"DATABASE_CONNECTION\": \"db://localhost:7777/my-db\", \"LOCAL_BEAR\": \"panda\" } } } } ```", + "description": "Allows the user to set or override the local process' environment variables with the ones from the remote pod.\n\nWhich environment variables to load from the remote pod are controlled by setting either [`include`](#feature-env-include) or [`exclude`](#feature-env-exclude).\n\nSee the environment variables [reference](https://mirrord.dev/docs/reference/env/) for more details.\n\n```json { \"feature\": { \"env\": { \"include\": \"DATABASE_USER;PUBLIC_ENV;MY_APP_*\", \"exclude\": \"DATABASE_PASSWORD;SECRET_ENV\", \"override\": { \"DATABASE_CONNECTION\": \"db://localhost:7777/my-db\", \"LOCAL_BEAR\": \"panda\" } } } } ```", "type": "object", "properties": { "exclude": { "title": "feature.env.exclude {#feature-env-exclude}", - "description": "Include the remote environment variables in the local process that are **NOT** specified by this option.\n\nSome of the variables that are excluded by default: `PATH`, `HOME`, `HOMEPATH`, `CLASSPATH`, `JAVA_EXE`, `JAVA_HOME`, `PYTHONPATH`.\n\nValue is a list separated by \";\".", + "description": "Include the remote environment variables in the local process that are **NOT** specified by this option. Variable names can be matched using `*` and `?` where `?` matches exactly one occurrence of any character and `*` matches arbitrary many (including zero) occurrences of any character.\n\nSome of the variables that are excluded by default: `PATH`, `HOME`, `HOMEPATH`, `CLASSPATH`, `JAVA_EXE`, `JAVA_HOME`, `PYTHONPATH`.\n\nCan be passed as a list or as a semicolon-delimited string (e.g. `\"VAR;OTHER_VAR\"`).", "anyOf": [ { "$ref": "#/definitions/VecOrSingle_for_String" @@ -446,7 +446,7 @@ }, "include": { "title": "feature.env.include {#feature-env-include}", - "description": "Include only these remote environment variables in the local process.\n\nValue is a list separated by \";\".\n\nSome environment variables are excluded by default (`PATH` for example), including these requires specifying them with `include`", + "description": "Include only these remote environment variables in the local process. Variable names can be matched using `*` and `?` where `?` matches exactly one occurrence of any character and `*` matches arbitrary many (including zero) occurrences of any character.\n\nCan be passed as a list or as a semicolon-delimited string (e.g. `\"VAR;OTHER_VAR\"`).\n\nSome environment variables are excluded by default (`PATH` for example), including these requires specifying them with `include`", "anyOf": [ { "$ref": "#/definitions/VecOrSingle_for_String" diff --git a/mirrord/config/src/feature/env.rs b/mirrord/config/src/feature/env.rs index 9727aeb5bac..30bff8bfca1 100644 --- a/mirrord/config/src/feature/env.rs +++ b/mirrord/config/src/feature/env.rs @@ -21,7 +21,7 @@ use crate::{ /// { /// "feature": { /// "env": { -/// "include": "DATABASE_USER;PUBLIC_ENV", +/// "include": "DATABASE_USER;PUBLIC_ENV;MY_APP_*", /// "exclude": "DATABASE_PASSWORD;SECRET_ENV", /// "override": { /// "DATABASE_CONNECTION": "db://localhost:7777/my-db", @@ -38,8 +38,10 @@ pub struct EnvConfig { /// ### feature.env.include {#feature-env-include} /// /// Include only these remote environment variables in the local process. + /// Variable names can be matched using `*` and `?` where `?` matches exactly one occurrence of + /// any character and `*` matches arbitrary many (including zero) occurrences of any character. /// - /// Value is a list separated by ";". + /// Can be passed as a list or as a semicolon-delimited string (e.g. `"VAR;OTHER_VAR"`). /// /// Some environment variables are excluded by default (`PATH` for example), including these /// requires specifying them with `include` @@ -50,11 +52,13 @@ pub struct EnvConfig { /// /// Include the remote environment variables in the local process that are **NOT** specified by /// this option. + /// Variable names can be matched using `*` and `?` where `?` matches exactly one occurrence of + /// any character and `*` matches arbitrary many (including zero) occurrences of any character. /// /// Some of the variables that are excluded by default: /// `PATH`, `HOME`, `HOMEPATH`, `CLASSPATH`, `JAVA_EXE`, `JAVA_HOME`, `PYTHONPATH`. /// - /// Value is a list separated by ";". + /// Can be passed as a list or as a semicolon-delimited string (e.g. `"VAR;OTHER_VAR"`). #[config(env = "MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE")] pub exclude: Option>, From 896d85bbe5a165a9a05f26450c203a7d6b3722e7 Mon Sep 17 00:00:00 2001 From: t4lz Date: Thu, 9 Nov 2023 18:21:15 +0100 Subject: [PATCH 08/10] spammy log info->trace (#2065) --- changelog.d/+spammy-connect-log.fixed.md | 1 + mirrord/layer/src/socket/ops.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/+spammy-connect-log.fixed.md diff --git a/changelog.d/+spammy-connect-log.fixed.md b/changelog.d/+spammy-connect-log.fixed.md new file mode 100644 index 00000000000..4aeafdc4543 --- /dev/null +++ b/changelog.d/+spammy-connect-log.fixed.md @@ -0,0 +1 @@ +Change spammy connect log's level from info to trace. diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index 38b12e593dd..4468cb0e05b 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -466,7 +466,7 @@ pub(super) fn connect( let unix_streams = crate::setup().remote_unix_streams(); - info!("in connect {:#?}", SOCKETS); + trace!("in connect {:#?}", SOCKETS); let (_, user_socket_info) = { SOCKETS From e12963e9dab6a66fa9273f555ce8d71d3af1a9fd Mon Sep 17 00:00:00 2001 From: meowjesty <43983236+meowjesty@users.noreply.github.com> Date: Mon, 13 Nov 2023 17:01:56 -0300 Subject: [PATCH 09/10] Hook gethosbyname to resolve DNS for erlang/elixir. (#2071) * Hook gethosbyname for elixir outgoing requests (it uses a deprecated getaddrinfo-like function). * pointer slice trickery * convert the right address to h_addr_list * just return global reference * deal with the crazy h_aliases * always v4 * null end of list * length at 4 * redoxify * less unwrapping * more logs * woo * .. * logs * integration test * remove unused * refactor + docs * fix changelog * remove unused features * fix logs * more log changes * force pointer cast * explicit type * node -> name // cast --------- Co-authored-by: Aviram Hassan --- changelog.d/2055.fixed.md | 1 + mirrord/layer/src/error.rs | 8 +- mirrord/layer/src/socket/hooks.rs | 22 +++- mirrord/layer/src/socket/ops.rs | 110 +++++++++++++++++- .../tests/apps/gethostbyname/gethostbyname.c | 34 ++++++ mirrord/layer/tests/common/mod.rs | 10 +- mirrord/layer/tests/issue2055.rs | 62 ++++++++++ 7 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 changelog.d/2055.fixed.md create mode 100644 mirrord/layer/tests/apps/gethostbyname/gethostbyname.c create mode 100644 mirrord/layer/tests/issue2055.rs diff --git a/changelog.d/2055.fixed.md b/changelog.d/2055.fixed.md new file mode 100644 index 00000000000..6aefa8752b6 --- /dev/null +++ b/changelog.d/2055.fixed.md @@ -0,0 +1 @@ +Add a hook for [gethostbyname](https://www.man7.org/linux/man-pages/man3/gethostbyname.3.html) to allow erlang/elixir to resolve DNS. diff --git a/mirrord/layer/src/error.rs b/mirrord/layer/src/error.rs index d327fe584e8..a7dcccc4bf9 100644 --- a/mirrord/layer/src/error.rs +++ b/mirrord/layer/src/error.rs @@ -2,7 +2,7 @@ use std::{env::VarError, net::SocketAddr, ptr, str::ParseBoolError}; use errno::set_errno; use ignore_codes::*; -use libc::{c_char, DIR, FILE}; +use libc::{c_char, hostent, DIR, FILE}; use mirrord_config::{config::ConfigError, feature::network::outgoing::OutgoingFilterError}; use mirrord_protocol::{ResponseError, SerializationError}; #[cfg(target_os = "macos")] @@ -311,3 +311,9 @@ impl From for LayerError { LayerError::Frida(err) } } + +impl From for *mut hostent { + fn from(_fail: HookError) -> Self { + ptr::null_mut() + } +} diff --git a/mirrord/layer/src/socket/hooks.rs b/mirrord/layer/src/socket/hooks.rs index 576aece8544..3deb7e0d241 100644 --- a/mirrord/layer/src/socket/hooks.rs +++ b/mirrord/layer/src/socket/hooks.rs @@ -4,7 +4,7 @@ use std::{os::unix::io::RawFd, sync::LazyLock}; use dashmap::DashSet; use errno::{set_errno, Errno}; -use libc::{c_char, c_int, c_void, size_t, sockaddr, socklen_t, ssize_t, EINVAL}; +use libc::{c_char, c_int, c_void, hostent, size_t, sockaddr, socklen_t, ssize_t, EINVAL}; use mirrord_layer_macro::{hook_fn, hook_guard_fn}; use super::ops::*; @@ -108,6 +108,18 @@ pub(crate) unsafe extern "C" fn gethostname_detour( .unwrap_or_bypass_with(|_| FN_GETHOSTNAME(raw_name, name_length)) } +/// Hook for `libc::gethostbyname` (you won't find this in rust's `libc` as it's been deprecated and +/// removed). +/// +/// Resolves DNS `raw_name` and allocates a `static` [`libc::hostent`] that we change the inner +/// values whenever this function is called. The address itself of `*mut hostent` has to remain the +/// same (thus why it's a `static`). +#[hook_guard_fn] +unsafe extern "C" fn gethostbyname_detour(raw_name: *const c_char) -> *mut hostent { + let rawish_name = (!raw_name.is_null()).then(|| CStr::from_ptr(raw_name)); + gethostbyname(rawish_name).unwrap_or_bypass_with(|_| FN_GETHOSTBYNAME(raw_name)) +} + #[hook_guard_fn] pub(crate) unsafe extern "C" fn accept_detour( sockfd: c_int, @@ -503,6 +515,14 @@ pub(crate) unsafe fn enable_socket_hooks(hook_manager: &mut HookManager, enabled FN_GETHOSTNAME ); + replace!( + hook_manager, + "gethostbyname", + gethostbyname_detour, + FnGethostbyname, + FN_GETHOSTBYNAME + ); + #[cfg(target_os = "linux")] { // Here we replace a function of libuv and not libc, so we pass None as the . diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index 4468cb0e05b..ec6c4a96735 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -12,7 +12,8 @@ use std::{ sync::{Arc, OnceLock}, }; -use libc::{c_int, c_void, sockaddr, socklen_t}; +use errno::set_errno; +use libc::{c_int, c_void, hostent, sockaddr, socklen_t}; use mirrord_config::feature::network::incoming::IncomingMode; use mirrord_intproxy_protocol::{ ConnMetadataRequest, ConnMetadataResponse, NetProtocol, OutgoingConnectRequest, @@ -43,6 +44,31 @@ pub(super) static REMOTE_DNS_REVERSE_MAPPING: LazyLock> /// Hostname initialized from the agent with [`gethostname`]. pub(crate) static HOSTNAME: OnceLock = OnceLock::new(); +/// Globals used by `gethostbyname`. +static mut GETHOSTBYNAME_HOSTNAME: Option = None; +static mut GETHOSTBYNAME_ALIASES_STR: Option> = None; + +/// **Safety**: +/// There is a potential UB trigger here, as [`libc::hostent`] uses this as an `*mut _`, while we +/// have `*const _`. As this is being filled to fulfill the contract of a deprecated function, I +/// (alex) don't think we're going to hit this issue ever. +static mut GETHOSTBYNAME_ALIASES_PTR: Option> = None; +static mut GETHOSTBYNAME_ADDRESSES_VAL: Option> = None; +static mut GETHOSTBYNAME_ADDRESSES_PTR: Option> = None; + +/// Global static that the user will receive when calling [`gethostbyname`]. +/// +/// **Safety**: +/// Even though we fill it with some `*const _` while it expects `*mut _`, it shouldn't be a problem +/// as the user will most likely be doing a deep copy if they want to mess around with this struct. +static mut GETHOSTBYNAME_HOSTENT: hostent = hostent { + h_name: ptr::null_mut(), + h_aliases: ptr::null_mut(), + h_addrtype: 0, + h_length: 0, + h_addr_list: ptr::null_mut(), +}; + /// Helper struct for connect results where we want to hold the original errno /// when result is -1 (error) because sometimes it's not a real error (EINPROGRESS/EINTR) /// and the caller should have the original value. @@ -869,6 +895,88 @@ fn remote_hostname_string() -> Detour { .map(Detour::Success)? } +/// Resolves a hostname and set result to static global like the original `gethostbyname` does. +/// +/// Used by erlang/elixir to resolve DNS. +/// +/// **Safety**: +/// See the [`GETHOSTBYNAME_ALIASES_PTR`] docs. If you see this function being called and some weird +/// issue is going on, assume that you might've triggered the UB. +#[tracing::instrument(level = "trace", ret)] +pub(super) fn gethostbyname(raw_name: Option<&CStr>) -> Detour<*mut hostent> { + let name: String = raw_name + .bypass(Bypass::NullNode)? + .to_str() + .map_err(|fail| { + warn!("Failed converting `name` from `CStr` with {:#?}", fail); + + Bypass::CStrConversion + })? + .into(); + + let hosts_and_ips = remote_getaddrinfo(name.clone())?; + + // We could `unwrap` here, as this would have failed on the previous conversion. + let host_name = CString::new(name)?; + + if hosts_and_ips.is_empty() { + set_errno(errno::Errno(libc::EAI_NODATA)); + return Detour::Success(ptr::null_mut()); + } + + // We need `*mut _` at the end, so `ips` has to be `mut`. + let (aliases, mut ips) = hosts_and_ips + .into_iter() + .filter_map(|(host, ip)| match ip { + // Only care about ipv4s and hosts that exist. + IpAddr::V4(ip) => { + let c_host = CString::new(host).ok()?; + Some((c_host, ip.octets())) + } + IpAddr::V6(ip) => { + trace!("ipv6 received - ignoring - {ip:?}"); + None + } + }) + .fold( + (Vec::default(), Vec::default()), + |(mut aliases, mut ips), (host, octets)| { + aliases.push(host); + ips.push(octets); + (aliases, ips) + }, + ); + + let mut aliases_ptrs: Vec<*const i8> = aliases + .iter() + .map(|alias| alias.as_ptr().cast()) + .collect::>(); + let mut ips_ptrs = ips.iter_mut().map(|ip| ip.as_mut_ptr()).collect::>(); + + // Put a null ptr to signal end of the list. + aliases_ptrs.push(ptr::null()); + ips_ptrs.push(ptr::null_mut()); + + // Need long-lived values so we can take pointers to them. + unsafe { + GETHOSTBYNAME_HOSTNAME.replace(host_name); + GETHOSTBYNAME_ALIASES_STR.replace(aliases); + GETHOSTBYNAME_ALIASES_PTR.replace(aliases_ptrs); + GETHOSTBYNAME_ADDRESSES_VAL.replace(ips); + GETHOSTBYNAME_ADDRESSES_PTR.replace(ips_ptrs); + + // Fill the `*mut hostent` that the user will interact with. + GETHOSTBYNAME_HOSTENT.h_name = GETHOSTBYNAME_HOSTNAME.as_ref().unwrap().as_ptr() as _; + GETHOSTBYNAME_HOSTENT.h_length = 4; + GETHOSTBYNAME_HOSTENT.h_addrtype = libc::AF_INET; + GETHOSTBYNAME_HOSTENT.h_aliases = GETHOSTBYNAME_ALIASES_PTR.as_ref().unwrap().as_ptr() as _; + GETHOSTBYNAME_HOSTENT.h_addr_list = + GETHOSTBYNAME_ADDRESSES_PTR.as_ref().unwrap().as_ptr() as *mut *mut libc::c_char; + } + + Detour::Success(unsafe { std::ptr::addr_of!(GETHOSTBYNAME_HOSTENT) as _ }) +} + /// Resolve hostname from remote host with caching for the result #[tracing::instrument(level = "trace")] pub(super) fn gethostname() -> Detour<&'static CString> { diff --git a/mirrord/layer/tests/apps/gethostbyname/gethostbyname.c b/mirrord/layer/tests/apps/gethostbyname/gethostbyname.c new file mode 100644 index 00000000000..f0274b9c8ef --- /dev/null +++ b/mirrord/layer/tests/apps/gethostbyname/gethostbyname.c @@ -0,0 +1,34 @@ +#include +#include +#include +#include + +void try_gethostbyname(const char name[]) { + struct hostent *result = gethostbyname(name); + + if (result) { + printf("result %p\n\t", result); + printf("h_name %s\n\t", result->h_name); + printf("h_length %i\n\t", result->h_length); + printf("h_addrtype %i\n\t", result->h_addrtype); + + for (int i = 0; result->h_addr_list[i]; i++) { + char str[INET6_ADDRSTRLEN]; + struct in_addr address = {}; + bcopy(result->h_addr_list[i], (char *)&address, sizeof(address)); + printf("h_addresses[%i] %s\n\t", i, inet_ntoa(address)); + } + + for (int i = 0; result->h_aliases[i]; i++) { + printf("h_aliases[%i] %s\n\t", i, result->h_aliases[i]); + } + } +} + +int main(int argc, char *argv[]) { + printf("test issue 2055: START\n"); + try_gethostbyname("www.mirrord.dev"); + try_gethostbyname("www.invalid.dev"); + printf("test issue 2055: SUCCESS\n"); + printf("\n"); +} diff --git a/mirrord/layer/tests/common/mod.rs b/mirrord/layer/tests/common/mod.rs index efcb732a7ed..5ec60849da5 100644 --- a/mirrord/layer/tests/common/mod.rs +++ b/mirrord/layer/tests/common/mod.rs @@ -651,6 +651,7 @@ pub enum Application { RustListenPorts, Fork, OpenFile, + CIssue2055, RustIssue2058, // For running applications with the executable and arguments determined at runtime. DynamicApp(String, Vec), @@ -787,6 +788,11 @@ impl Application { env!("CARGO_MANIFEST_DIR"), "tests/apps/open_file/out.c_test_app", ), + Application::CIssue2055 => format!( + "{}/{}", + env!("CARGO_MANIFEST_DIR"), + "tests/apps/gethostbyname/out.c_test_app", + ), Application::RustIssue2058 => String::from("tests/apps/issue2058/target/issue2058"), Application::DynamicApp(exe, _) => exe.clone(), } @@ -877,7 +883,8 @@ impl Application { | Application::Go19DirBypass | Application::Go20DirBypass | Application::RustIssue2058 - | Application::OpenFile => vec![], + | Application::OpenFile + | Application::CIssue2055 => vec![], Application::RustOutgoingUdp => ["--udp", RUST_OUTGOING_LOCAL, RUST_OUTGOING_PEERS] .into_iter() .map(Into::into) @@ -943,6 +950,7 @@ impl Application { | Application::RustListenPorts | Application::RustRecvFrom | Application::OpenFile + | Application::CIssue2055 | Application::DynamicApp(..) => unimplemented!("shouldn't get here"), Application::PythonSelfConnect => 1337, Application::RustIssue2058 => 1234, diff --git a/mirrord/layer/tests/issue2055.rs b/mirrord/layer/tests/issue2055.rs new file mode 100644 index 00000000000..79b543c978a --- /dev/null +++ b/mirrord/layer/tests/issue2055.rs @@ -0,0 +1,62 @@ +#![feature(assert_matches)] +use std::{net::IpAddr, path::PathBuf, time::Duration}; + +use mirrord_protocol::{ + dns::{DnsLookup, GetAddrInfoRequest, GetAddrInfoResponse, LookupRecord}, + ClientMessage, DaemonMessage, DnsLookupError, + ResolveErrorKindInternal::NoRecordsFound, + ResponseError, +}; +use rstest::rstest; + +mod common; +pub use common::*; + +/// Verify that issue [#2055](https://github.com/metalbear-co/mirrord/issues/2055) is fixed. +/// "DNS Issue on Elixir macOS" +#[rstest] +#[tokio::test] +#[timeout(Duration::from_secs(60))] +async fn issue_2055(dylib_path: &PathBuf) { + let application = Application::CIssue2055; + let (mut test_process, mut intproxy) = application + .start_process_with_layer(dylib_path, Default::default(), None) + .await; + + println!("Application started, waiting for `GetAddrInfoRequest`."); + + let msg = intproxy.recv().await; + let ClientMessage::GetAddrInfoRequest(GetAddrInfoRequest { node }) = msg else { + panic!("Invalid message received from layer: {msg:?}"); + }; + + intproxy + .send(DaemonMessage::GetAddrInfoResponse(GetAddrInfoResponse(Ok( + DnsLookup(vec![LookupRecord { + name: node, + ip: "93.184.216.34".parse::().unwrap(), + }]), + )))) + .await; + + let msg = intproxy.recv().await; + let ClientMessage::GetAddrInfoRequest(GetAddrInfoRequest { node: _ }) = msg else { + panic!("Invalid message received from layer: {msg:?}"); + }; + + intproxy + .send(DaemonMessage::GetAddrInfoResponse(GetAddrInfoResponse( + Err(ResponseError::DnsLookup(DnsLookupError { + kind: NoRecordsFound(3), + })), + ))) + .await; + + test_process.wait_assert_success().await; + test_process + .assert_stdout_contains("test issue 2055: START") + .await; + test_process + .assert_stdout_contains("test issue 2055: SUCCESS") + .await; +} From fa814c2e5860d3ed28117079063a360fb48cd9b3 Mon Sep 17 00:00:00 2001 From: Eyal Bukchin Date: Tue, 14 Nov 2023 11:48:49 +0200 Subject: [PATCH 10/10] Update to 3.75.1 (#2072) --- CHANGELOG.md | 17 ++++++++ Cargo.lock | 54 ++++++++++++------------ Cargo.toml | 2 +- changelog.d/+doc-env-wildcard.added.md | 1 - changelog.d/+spammy-connect-log.fixed.md | 1 - changelog.d/2055.fixed.md | 1 - 6 files changed, 45 insertions(+), 31 deletions(-) delete mode 100644 changelog.d/+doc-env-wildcard.added.md delete mode 100644 changelog.d/+spammy-connect-log.fixed.md delete mode 100644 changelog.d/2055.fixed.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 138151a7616..817a819bd13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,23 @@ This project uses [*towncrier*](https://towncrier.readthedocs.io/) and the chang +## [3.75.1](https://github.com/metalbear-co/mirrord/tree/3.75.1) - 2023-11-14 + + +### Fixed + +- Add a hook for + [gethostbyname](https://www.man7.org/linux/man-pages/man3/gethostbyname.3.html) + to allow erlang/elixir to resolve DNS. + [#2055](https://github.com/metalbear-co/mirrord/issues/2055) +- Change spammy connect log's level from info to trace. + + +### Internal + +- Documentation of `env` config pattern matching. + + ## [3.75.0](https://github.com/metalbear-co/mirrord/tree/3.75.0) - 2023-11-08 diff --git a/Cargo.lock b/Cargo.lock index a0ffe38b264..d55f02f6e0e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "fileops" -version = "3.75.0" +version = "3.75.1" dependencies = [ "libc", ] @@ -3107,7 +3107,7 @@ dependencies = [ [[package]] name = "issue1317" -version = "3.75.0" +version = "3.75.1" dependencies = [ "actix-web", "env_logger", @@ -3119,7 +3119,7 @@ dependencies = [ [[package]] name = "issue1776" -version = "3.75.0" +version = "3.75.1" dependencies = [ "errno 0.3.5", "libc", @@ -3128,7 +3128,7 @@ dependencies = [ [[package]] name = "issue1776portnot53" -version = "3.75.0" +version = "3.75.1" dependencies = [ "libc", "socket2 0.5.5", @@ -3136,14 +3136,14 @@ dependencies = [ [[package]] name = "issue1899" -version = "3.75.0" +version = "3.75.1" dependencies = [ "libc", ] [[package]] name = "issue2001" -version = "3.75.0" +version = "3.75.1" dependencies = [ "libc", ] @@ -3431,7 +3431,7 @@ checksum = "da2479e8c062e40bf0066ffa0bc823de0a9368974af99c9f6df941d2c231e03f" [[package]] name = "listen_ports" -version = "3.75.0" +version = "3.75.1" [[package]] name = "local-channel" @@ -3657,7 +3657,7 @@ dependencies = [ [[package]] name = "mirrord" -version = "3.75.0" +version = "3.75.1" dependencies = [ "actix-codec", "anyhow", @@ -3702,7 +3702,7 @@ dependencies = [ [[package]] name = "mirrord-agent" -version = "3.75.0" +version = "3.75.1" dependencies = [ "actix-codec", "async-trait", @@ -3755,7 +3755,7 @@ dependencies = [ [[package]] name = "mirrord-analytics" -version = "3.75.0" +version = "3.75.1" dependencies = [ "assert-json-diff", "base64 0.21.5", @@ -3769,7 +3769,7 @@ dependencies = [ [[package]] name = "mirrord-auth" -version = "3.75.0" +version = "3.75.1" dependencies = [ "chrono", "fs4", @@ -3789,7 +3789,7 @@ dependencies = [ [[package]] name = "mirrord-config" -version = "3.75.0" +version = "3.75.1" dependencies = [ "bimap", "bitflags 2.4.1", @@ -3811,7 +3811,7 @@ dependencies = [ [[package]] name = "mirrord-config-derive" -version = "3.75.0" +version = "3.75.1" dependencies = [ "proc-macro2", "proc-macro2-diagnostics", @@ -3821,7 +3821,7 @@ dependencies = [ [[package]] name = "mirrord-console" -version = "3.75.0" +version = "3.75.1" dependencies = [ "bincode", "drain", @@ -3837,7 +3837,7 @@ dependencies = [ [[package]] name = "mirrord-intproxy" -version = "3.75.0" +version = "3.75.1" dependencies = [ "bytes", "http-body-util", @@ -3858,7 +3858,7 @@ dependencies = [ [[package]] name = "mirrord-intproxy-protocol" -version = "3.75.0" +version = "3.75.1" dependencies = [ "bincode", "mirrord-protocol", @@ -3868,7 +3868,7 @@ dependencies = [ [[package]] name = "mirrord-kube" -version = "3.75.0" +version = "3.75.1" dependencies = [ "actix-codec", "base64 0.21.5", @@ -3895,7 +3895,7 @@ dependencies = [ [[package]] name = "mirrord-layer" -version = "3.75.0" +version = "3.75.1" dependencies = [ "actix-codec", "anyhow", @@ -3950,7 +3950,7 @@ dependencies = [ [[package]] name = "mirrord-layer-macro" -version = "3.75.0" +version = "3.75.1" dependencies = [ "proc-macro2", "quote", @@ -3959,7 +3959,7 @@ dependencies = [ [[package]] name = "mirrord-macros" -version = "3.75.0" +version = "3.75.1" dependencies = [ "proc-macro2", "proc-macro2-diagnostics", @@ -3969,7 +3969,7 @@ dependencies = [ [[package]] name = "mirrord-operator" -version = "3.75.0" +version = "3.75.1" dependencies = [ "actix-codec", "async-trait", @@ -4003,7 +4003,7 @@ dependencies = [ [[package]] name = "mirrord-progress" -version = "3.75.0" +version = "3.75.1" dependencies = [ "enum_dispatch", "indicatif", @@ -4035,7 +4035,7 @@ dependencies = [ [[package]] name = "mirrord-sip" -version = "3.75.0" +version = "3.75.1" dependencies = [ "apple-codesign", "memchr", @@ -4359,7 +4359,7 @@ dependencies = [ [[package]] name = "outgoing" -version = "3.75.0" +version = "3.75.1" [[package]] name = "outref" @@ -5333,21 +5333,21 @@ dependencies = [ [[package]] name = "rust-bypassed-unix-socket" -version = "3.75.0" +version = "3.75.1" dependencies = [ "tokio", ] [[package]] name = "rust-e2e-fileops" -version = "3.75.0" +version = "3.75.1" dependencies = [ "libc", ] [[package]] name = "rust-unix-socket-client" -version = "3.75.0" +version = "3.75.1" dependencies = [ "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index f7d8c538d53..bbf503eea70 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ resolver = "2" # latest commits on rustls suppress certificate verification [workspace.package] -version = "3.75.0" +version = "3.75.1" edition = "2021" license = "MIT" readme = "README.md" diff --git a/changelog.d/+doc-env-wildcard.added.md b/changelog.d/+doc-env-wildcard.added.md deleted file mode 100644 index da922d1eb3b..00000000000 --- a/changelog.d/+doc-env-wildcard.added.md +++ /dev/null @@ -1 +0,0 @@ -Documentation of `env` config pattern matching. diff --git a/changelog.d/+spammy-connect-log.fixed.md b/changelog.d/+spammy-connect-log.fixed.md deleted file mode 100644 index 4aeafdc4543..00000000000 --- a/changelog.d/+spammy-connect-log.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Change spammy connect log's level from info to trace. diff --git a/changelog.d/2055.fixed.md b/changelog.d/2055.fixed.md deleted file mode 100644 index 6aefa8752b6..00000000000 --- a/changelog.d/2055.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Add a hook for [gethostbyname](https://www.man7.org/linux/man-pages/man3/gethostbyname.3.html) to allow erlang/elixir to resolve DNS.