Prevent config rendering on every process (#3011)

* Add detecting encoded config from env var

* [WIP] check for empty env var value

* Skip test_error config field when deserializing

* Add unit test to check if encoding and decoding matches the original config

* Changelog

* Fix clippy

* Remove direct interactions with env vars for fields in config, use update_env() instead

* Various

* Add method to forcefully recalculate config instead of taking from existing env

* Revert to using custom ser/deser on BiMap in config

* Doc comments

changelog.d/2936.fixed.md

@@ -0,0 +1 @@
+Stopped mirrord entering a crash loop when trying to load into some processes like VSCode's `watchdog.js` when the user config contained a call to `get_env()`, which occurred due to missing env - the config is now only rendered once and set into an env var.

mirrord/cli/src/container.rs

@@ -333,6 +333,7 @@ pub(crate) async fn container_command(
         (CONTAINER_EXECUTION_KIND as u32).to_string(),
     );
 
+    // LayerConfig must be created after setting relevant env vars
     let (mut config, mut analytics) = create_config_and_analytics(&mut progress, watch)?;
 
     let (_internal_proxy_tls_guards, _external_proxy_tls_guards) =
@@ -427,6 +428,7 @@ pub(crate) async fn container_ext_command(
         env.insert("MIRRORD_IMPERSONATED_TARGET".into(), target.to_string());
     }
 
+    // LayerConfig must be created after setting relevant env vars
     let (mut config, mut analytics) = create_config_and_analytics(&mut progress, watch)?;
 
     let (_internal_proxy_tls_guards, _external_proxy_tls_guards) =

mirrord/cli/src/execution.rs

@@ -177,7 +177,7 @@ impl MirrordExecution {
     /// [`tokio::time::sleep`] or [`tokio::task::yield_now`] after calling this function.
     #[tracing::instrument(level = Level::TRACE, skip_all)]
     pub(crate) async fn start<P>(
-        config: &LayerConfig,
+        config: &mut LayerConfig,
         // We only need the executable on macos, for SIP handling.
         #[cfg(target_os = "macos")] executable: Option<&str>,
         progress: &mut P,
@@ -289,10 +289,8 @@ impl MirrordExecution {
             })?;
 
         // Provide details for layer to connect to agent via internal proxy
-        env_vars.insert(
-            MIRRORD_CONNECT_TCP_ENV.to_string(),
-            format!("127.0.0.1:{}", address.port()),
-        );
+        config.connect_tcp = Some(format!("127.0.0.1:{}", address.port()));
+        config.update_env_var()?;
 
         // Fixes <https://github.com/metalbear-co/mirrord/issues/1745>
         // by disabling the fork safety check in the Objective-C runtime.

mirrord/cli/src/extension.rs

@@ -10,7 +10,7 @@ use crate::{config::ExtensionExecArgs, error::CliError, execution::MirrordExecut
 async fn mirrord_exec<P>(
     #[cfg(target_os = "macos")] executable: Option<&str>,
     env: HashMap<String, String>,
-    config: LayerConfig,
+    mut config: LayerConfig,
     mut progress: P,
     analytics: &mut AnalyticsReporter,
 ) -> CliResult<()>
@@ -21,9 +21,9 @@ where
     // or run tasks before actually launching.
     #[cfg(target_os = "macos")]
     let mut execution_info =
-        MirrordExecution::start(&config, executable, &mut progress, analytics).await?;
+        MirrordExecution::start(&mut config, executable, &mut progress, analytics).await?;
     #[cfg(not(target_os = "macos"))]
-    let mut execution_info = MirrordExecution::start(&config, &mut progress, analytics).await?;
+    let mut execution_info = MirrordExecution::start(&mut config, &mut progress, analytics).await?;
 
     // We don't execute so set envs aren't passed, so we need to add config file and target to
     // env.

mirrord/cli/src/external_proxy.rs

@@ -60,7 +60,7 @@ fn print_addr(listener: &TcpListener) -> io::Result<()> {
 }
 
 pub async fn proxy(listen_port: u16, watch: drain::Watch) -> CliResult<()> {
-    let config = LayerConfig::from_env()?;
+    let config = LayerConfig::recalculate_from_env()?;
 
     init_extproxy_tracing_registry(&config)?;
     tracing::info!(?config, "external_proxy starting");

mirrord/cli/src/internal_proxy.rs

@@ -50,7 +50,7 @@ pub(crate) async fn proxy(
     listen_port: u16,
     watch: drain::Watch,
 ) -> CliResult<(), InternalProxyError> {
-    let config = LayerConfig::from_env()?;
+    let config = LayerConfig::recalculate_from_env()?;
 
     init_intproxy_tracing_registry(&config)?;
     tracing::info!(?config, "internal_proxy starting");

mirrord/cli/src/main.rs

@@ -64,7 +64,7 @@ pub(crate) use error::{CliError, CliResult};
 use verify_config::verify_config;
 
 async fn exec_process<P>(
-    config: LayerConfig,
+    mut config: LayerConfig,
     args: &ExecArgs,
     progress: &P,
     analytics: &mut AnalyticsReporter,
@@ -75,10 +75,15 @@ where
     let mut sub_progress = progress.subtask("preparing to launch process");
 
     #[cfg(target_os = "macos")]
-    let execution_info =
-        MirrordExecution::start(&config, Some(&args.binary), &mut sub_progress, analytics).await?;
+    let execution_info = MirrordExecution::start(
+        &mut config,
+        Some(&args.binary),
+        &mut sub_progress,
+        analytics,
+    )
+    .await?;
     #[cfg(not(target_os = "macos"))]
-    let execution_info = MirrordExecution::start(&config, &mut sub_progress, analytics).await?;
+    let execution_info = MirrordExecution::start(&mut config, &mut sub_progress, analytics).await?;
 
     // This is not being yielded, as this is not proper async, something along those lines.
     // We need an `await` somewhere in this function to drive our socket IO that happens
@@ -352,6 +357,7 @@ async fn exec(args: &ExecArgs, watch: drain::Watch) -> CliResult<()> {
         std::env::set_var(name, value);
     }
 
+    // LayerConfig must be created after setting relevant env vars
     let (config, mut context) = LayerConfig::from_env_with_warnings()?;
 
     let mut analytics = AnalyticsReporter::only_error(config.telemetry, Default::default(), watch);
@@ -472,6 +478,7 @@ async fn port_forward(args: &PortForwardArgs, watch: drain::Watch) -> CliResult<
         std::env::set_var("MIRRORD_CONFIG_FILE", config_file);
     }
 
+    // LayerConfig must be created after setting relevant env vars
     let (config, mut context) = LayerConfig::from_env_with_warnings()?;
 
     let mut analytics = AnalyticsReporter::new(config.telemetry, ExecutionKind::PortForward, watch);

mirrord/config/Cargo.toml

@@ -17,8 +17,8 @@ edition.workspace = true
 workspace = true
 
 [dependencies]
-mirrord-config-derive = { path = "./derive"}
-mirrord-analytics = { path = "../analytics"}
+mirrord-config-derive = { path = "./derive" }
+mirrord-analytics = { path = "../analytics" }
 
 serde.workspace = true
 serde_json.workspace = true
@@ -27,13 +27,14 @@ tracing.workspace = true
 serde_yaml.workspace = true
 toml = "0.8"
 schemars.workspace = true
-bimap = "0.6"
+bimap = { version = "0.6" }
 nom = "7.1"
 ipnet.workspace = true
 bitflags = "2"
 k8s-openapi = { workspace = true, features = ["schemars", "earliest"] }
 tera = "1"
 fancy-regex.workspace = true
+base64.workspace = true
 
 [dev-dependencies]
 rstest.workspace = true

mirrord/config/src/agent.rs

@@ -67,7 +67,7 @@ impl fmt::Display for LinuxCapability {
 ///   }
 /// }
 /// ```
-#[derive(MirrordConfig, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[config(map_to = "AgentFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq"))]
 pub struct AgentConfig {
@@ -354,7 +354,7 @@ pub struct AgentConfig {
     /// Create an agent that returns an error after accepting the first client. For testing
     /// purposes. Only supported with job agents (not with ephemeral agents).
     #[cfg(all(debug_assertions, not(test)))] // not(test) so that it's not included in the schema json.
-    #[serde(skip_serializing)]
+    #[serde(skip)]
     #[config(env = "MIRRORD_AGENT_TEST_ERROR", default = false, unstable)]
     pub test_error: bool,
 }
@@ -477,7 +477,7 @@ impl AgentFileConfig {
     }
 }
 
-#[derive(MirrordConfig, Default, PartialEq, Eq, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Default, PartialEq, Eq, Clone, Debug, Serialize, Deserialize)]
 #[config(derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq, Eq"))]
 pub struct AgentDnsConfig {

mirrord/config/src/config.rs

@@ -86,6 +86,12 @@ pub enum ConfigError {
         value: String,
         fail: Box<fancy_regex::Error>,
     },
+
+    #[error("mirrord-config: decoding resolved config from env var failed with `{0}`")]
+    EnvVarDecodeError(String),
+
+    #[error("mirrord-config: encoding resolved config failed with `{0}`")]
+    EnvVarEncodeError(String),
 }
 
 impl From<tera::Error> for ConfigError {

mirrord/config/src/container.rs

@@ -2,7 +2,7 @@ use std::path::PathBuf;
 
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use crate::config::source::MirrordConfigSource;
 
@@ -12,7 +12,7 @@ static DEFAULT_CLI_IMAGE: &str = concat!(
 );
 
 /// Unstable: `mirrord container` command specific config.
-#[derive(MirrordConfig, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[config(map_to = "ContainerFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq"))]
 pub struct ContainerConfig {

mirrord/config/src/experimental.rs

@@ -1,13 +1,13 @@
 use mirrord_analytics::CollectAnalytics;
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use crate::config::source::MirrordConfigSource;
 
 /// mirrord Experimental features.
 /// This shouldn't be used unless someone from MetalBear/mirrord tells you to.
-#[derive(MirrordConfig, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[config(map_to = "ExperimentalFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq, Eq"))]
 pub struct ExperimentalConfig {

mirrord/config/src/external_proxy.rs

@@ -2,7 +2,7 @@ use std::path::PathBuf;
 
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use crate::config::source::MirrordConfigSource;
 
@@ -23,7 +23,7 @@ pub static MIRRORD_EXTERNAL_TLS_KEY_ENV: &str = "MIRRORD_EXTERNAL_TLS_KEY";
 ///   }
 /// }
 /// ```
-#[derive(MirrordConfig, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[config(map_to = "ExternalProxyFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq"))]
 pub struct ExternalProxyConfig {

mirrord/config/src/feature.rs

@@ -1,7 +1,7 @@
 use mirrord_analytics::CollectAnalytics;
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use self::{copy_target::CopyTargetConfig, env::EnvConfig, fs::FsConfig, network::NetworkConfig};
 use crate::{config::source::MirrordConfigSource, feature::split_queues::SplitQueuesConfig};
@@ -64,7 +64,7 @@ pub mod split_queues;
 ///   }
 /// }
 /// ```
-#[derive(MirrordConfig, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[config(map_to = "FeatureFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq, Eq"))]
 pub struct FeatureConfig {

mirrord/config/src/feature/copy_target.rs

@@ -75,7 +75,7 @@ impl FromMirrordConfig for CopyTargetConfig {
 ///   }
 /// }
 /// ```
-#[derive(Clone, Debug, Serialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct CopyTargetConfig {
     pub enabled: bool,
 

mirrord/config/src/feature/env.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, path::PathBuf};
 use mirrord_analytics::CollectAnalytics;
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use crate::{
     config::{from_env::FromEnv, source::MirrordConfigSource, ConfigContext, Result},
@@ -47,7 +47,7 @@ pub const MIRRORD_OVERRIDE_ENV_FILE_ENV: &str = "MIRRORD_OVERRIDE_ENV_VARS_FILE"
 ///   }
 /// }
 /// ```
-#[derive(MirrordConfig, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[config(map_to = "EnvFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq, Eq"))]
 pub struct EnvConfig {

mirrord/config/src/feature/fs/advanced.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use mirrord_analytics::{AnalyticValue, CollectAnalytics};
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use super::{FsModeConfig, FsUserConfig};
 use crate::{
@@ -80,7 +80,7 @@ use crate::{
 ///   }
 /// }
 /// ```
-#[derive(MirrordConfig, Default, Clone, PartialEq, Eq, Debug, Serialize)]
+#[derive(MirrordConfig, Default, Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
 #[config(
     map_to = "AdvancedFsUserConfig",
     derive = "PartialEq,Eq,JsonSchema",

mirrord/config/src/feature/network.rs

@@ -2,7 +2,7 @@ use dns::{DnsConfig, DnsFileConfig};
 use mirrord_analytics::CollectAnalytics;
 use mirrord_config_derive::MirrordConfig;
 use schemars::JsonSchema;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use self::{incoming::*, outgoing::*};
 use crate::{
@@ -54,7 +54,7 @@ pub mod outgoing;
 ///   }
 /// }
 /// ```
-#[derive(MirrordConfig, Default, PartialEq, Eq, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Default, PartialEq, Eq, Clone, Debug, Serialize, Deserialize)]
 #[config(map_to = "NetworkFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq, Eq"))]
 pub struct NetworkConfig {

mirrord/config/src/feature/network/dns.rs

@@ -87,7 +87,7 @@ pub enum DnsFilterConfig {
 ///   `read_only: ["/etc/resolv.conf"]`.
 /// - DNS filter currently works only with frameworks that use `getaddrinfo`/`gethostbyname`
 ///   functions.
-#[derive(MirrordConfig, Default, PartialEq, Eq, Clone, Debug, Serialize)]
+#[derive(MirrordConfig, Default, PartialEq, Eq, Clone, Debug, Serialize, Deserialize)]
 #[config(map_to = "DnsFileConfig", derive = "JsonSchema")]
 #[cfg_attr(test, config(derive = "PartialEq, Eq"))]
 pub struct DnsConfig {