Support passing env vars from a file (metalbear-co#2924)

* Arg and field added

* Reading env file

* Fix env precedence

* Schema and configuration.md

* Added docs

* changelog

* Update mirrord/cli/src/config.rs

Co-authored-by: meowjesty <[email protected]>

---------

Co-authored-by: meowjesty <[email protected]>

Cargo.toml

@@ -97,6 +97,7 @@ num-traits = "0.2"
 regex = { version = "1", features = ["unicode-case"] }
 fancy-regex = { version = "0.13" }
 enum_dispatch = "0.3"
+envfile = "0.2"
 
 
 # If you update `hyper`, check that `h2` version is compatible in `intproxy/Cargo.toml`.

changelog.d/1913.added.md

@@ -0,0 +1 @@
+Added a configuration option that allows for specifying an env file for mirrord execution.

mirrord-schema.json

@@ -702,6 +702,14 @@
       "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": {
+        "env_file": {
+          "title": "feature.env_file {#feature-env-file}",
+          "description": "Allows for passing environment variables from an env file.\n\nThese variables will override environment fetched from the remote target.",
+          "type": [
+            "string",
+            "null"
+          ]
+        },
         "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. 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\"`).",
@@ -736,7 +744,7 @@
         },
         "override": {
           "title": "feature.env.override {#feature-env-override}",
-          "description": "Allows setting or overriding environment variables (locally) with a custom value.\n\nFor example, if the remote pod has an environment variable `REGION=1`, but this is an undesirable value, it's possible to use `override` to set `REGION=2` (locally) instead.",
+          "description": "Allows setting or overriding environment variables (locally) with a custom value.\n\nFor example, if the remote pod has an environment variable `REGION=1`, but this is an undesirable value, it's possible to use `override` to set `REGION=2` (locally) instead.\n\nEnvironment specified here will also override variables passed via the env file.",
           "type": [
             "object",
             "null"

mirrord/cli/Cargo.toml

@@ -36,6 +36,7 @@ mirrord-vpn = { path = "../vpn" }
 
 actix-codec.workspace = true
 clap.workspace = true
+envfile.workspace = true
 tun2 = { version = "3", features = ["async"] }
 tracing.workspace = true
 serde_json.workspace = true

mirrord/cli/src/config.rs

@@ -10,7 +10,13 @@ use std::{
 
 use clap::{ArgGroup, Args, Parser, Subcommand, ValueEnum, ValueHint};
 use clap_complete::Shell;
-use mirrord_config::MIRRORD_CONFIG_FILE_ENV;
+use mirrord_config::{
+    feature::env::{
+        MIRRORD_OVERRIDE_ENV_FILE_ENV, MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV,
+        MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV,
+    },
+    MIRRORD_CONFIG_FILE_ENV,
+};
 use mirrord_operator::setup::OperatorNamespace;
 use thiserror::Error;
 
@@ -200,6 +206,14 @@ pub(super) struct ExecParams {
     /// Kube context to use from Kubeconfig
     #[arg(long)]
     pub context: Option<String>,
+
+    /// Path to env file that should be used for the execution.
+    ///
+    /// Allows for passing environment variables from an env file.
+    ///
+    /// These variables will override environment fetched from the remote target.
+    #[arg(long, value_hint = ValueHint::FilePath)]
+    pub env_file: Option<PathBuf>,
 }
 
 impl ExecParams {
@@ -250,14 +264,14 @@ impl ExecParams {
 
         if let Some(override_env_vars_exclude) = &self.override_env_vars_exclude {
             envs.insert(
-                "MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE".into(),
+                MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV.into(),
                 override_env_vars_exclude.into(),
             );
         }
 
         if let Some(override_env_vars_include) = &self.override_env_vars_include {
             envs.insert(
-                "MIRRORD_OVERRIDE_ENV_VARS_INCLUDE".into(),
+                MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV.into(),
                 override_env_vars_include.into(),
             );
         }
@@ -298,7 +312,7 @@ impl ExecParams {
         }
 
         if let Some(config_file) = &self.config_file {
-            // Set canoncialized path to config file, in case forks/children are in different
+            // Set canonicalized path to config file, in case forks/children are in different
             // working directories.
             let full_path = std::fs::canonicalize(config_file)
                 .map_err(|e| CliError::CanonicalizeConfigPathFailed(config_file.clone(), e))?;
@@ -308,6 +322,17 @@ impl ExecParams {
             );
         }
 
+        if let Some(env_file) = &self.env_file {
+            // Set canonicalized path to env file, in case forks/children are in different
+            // working directories.
+            let full_path = std::fs::canonicalize(env_file)
+                .map_err(|e| CliError::EnvFileAccessError(env_file.clone(), e))?;
+            envs.insert(
+                MIRRORD_OVERRIDE_ENV_FILE_ENV.into(),
+                full_path.as_os_str().to_owned(),
+            );
+        }
+
         Ok(envs)
     }
 }

mirrord/cli/src/error.rs

@@ -244,6 +244,10 @@ pub(crate) enum CliError {
     #[diagnostic(help("Please check that the path is correct and that you have permissions to read it.{GENERAL_HELP}"))]
     CanonicalizeConfigPathFailed(PathBuf, std::io::Error),
 
+    #[error("Failed to access env file at `{}`: {1}", .0.display())]
+    #[diagnostic(help("Please check that the path is correct and that you have permissions to read it.{GENERAL_HELP}"))]
+    EnvFileAccessError(PathBuf, std::io::Error),
+
     #[cfg(target_os = "macos")]
     #[error("SIP Error: `{0:#?}`")]
     #[diagnostic(help(

mirrord/cli/src/execution.rs

@@ -448,8 +448,6 @@ impl MirrordExecution {
         config: &LayerConfig,
         connection: &mut AgentConnection,
     ) -> CliResult<HashMap<String, String>> {
-        let mut env_vars = HashMap::new();
-
         let (env_vars_exclude, env_vars_include) = match (
             config
                 .feature
@@ -475,20 +473,29 @@ impl MirrordExecution {
             (None, None) => (HashSet::new(), HashSet::from(EnvVars("*".to_owned()))),
         };
 
-        let communication_timeout =
-            Duration::from_secs(config.agent.communication_timeout.unwrap_or(30).into());
+        let mut env_vars = if !env_vars_exclude.is_empty() || !env_vars_include.is_empty() {
+            let communication_timeout =
+                Duration::from_secs(config.agent.communication_timeout.unwrap_or(30).into());
 
-        if !env_vars_exclude.is_empty() || !env_vars_include.is_empty() {
-            let remote_env = tokio::time::timeout(
+            tokio::time::timeout(
                 communication_timeout,
                 Self::get_remote_env(connection, env_vars_exclude, env_vars_include),
             )
             .await
-            .map_err(|_| CliError::InitialAgentCommFailed("timeout".to_string()))??;
-            env_vars.extend(remote_env);
-            if let Some(overrides) = &config.feature.env.r#override {
-                env_vars.extend(overrides.iter().map(|(k, v)| (k.clone(), v.clone())));
-            }
+            .map_err(|_| CliError::InitialAgentCommFailed("timeout".to_string()))??
+        } else {
+            Default::default()
+        };
+
+        if let Some(file) = &config.feature.env.env_file {
+            let envs_from_file = envfile::EnvFile::new(file)
+                .map_err(|error| CliError::EnvFileAccessError(file.clone(), error))?
+                .store;
+            env_vars.extend(envs_from_file);
+        }
+
+        if let Some(overrides) = &config.feature.env.r#override {
+            env_vars.extend(overrides.iter().map(|(k, v)| (k.clone(), v.clone())));
         }
 
         Ok(env_vars)

mirrord/config/configuration.md

@@ -649,6 +649,12 @@ See the environment variables [reference](https://mirrord.dev/docs/reference/env
 }
 ```
 
+### feature.env_file {#feature-env-file}
+
+Allows for passing environment variables from an env file.
+
+These variables will override environment fetched from the remote target.
+
 ### feature.env.exclude {#feature-env-exclude}
 
 Include the remote environment variables in the local process that are **NOT** specified by
@@ -687,6 +693,8 @@ Allows setting or overriding environment variables (locally) with a custom value
 For example, if the remote pod has an environment variable `REGION=1`, but this is an
 undesirable value, it's possible to use `override` to set `REGION=2` (locally) instead.
 
+Environment specified here will also override variables passed via the env file.
+
 ### feature.env.unset {#feature-env-unset}
 
 Allows unsetting environment variables in the executed process.

mirrord/config/src/feature/env.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::{collections::HashMap, path::PathBuf};
 
 use mirrord_analytics::CollectAnalytics;
 use mirrord_config_derive::MirrordConfig;
@@ -10,6 +10,10 @@ use crate::{
     util::{MirrordToggleableConfig, VecOrSingle},
 };
 
+pub const MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV: &str = "MIRRORD_OVERRIDE_ENV_VARS_INCLUDE";
+pub const MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV: &str = "MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE";
+pub const MIRRORD_OVERRIDE_ENV_FILE_ENV: &str = "MIRRORD_OVERRIDE_ENV_VARS_FILE";
+
 /// Allows the user to set or override the local process' environment variables with the ones
 /// from the remote pod.
 ///
@@ -46,7 +50,7 @@ pub struct EnvConfig {
     ///
     /// Some environment variables are excluded by default (`PATH` for example), including these
     /// requires specifying them with `include`
-    #[config(env = "MIRRORD_OVERRIDE_ENV_VARS_INCLUDE")]
+    #[config(env = MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV)]
     pub include: Option<VecOrSingle<String>>,
 
     /// ### feature.env.exclude {#feature-env-exclude}
@@ -60,7 +64,7 @@ pub struct EnvConfig {
     /// `PATH`, `HOME`, `HOMEPATH`, `CLASSPATH`, `JAVA_EXE`, `JAVA_HOME`, `PYTHONPATH`.
     ///
     /// Can be passed as a list or as a semicolon-delimited string (e.g. `"VAR;OTHER_VAR"`).
-    #[config(env = "MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE")]
+    #[config(env = MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV)]
     pub exclude: Option<VecOrSingle<String>>,
 
     /// ### feature.env.override {#feature-env-override}
@@ -69,6 +73,8 @@ pub struct EnvConfig {
     ///
     /// For example, if the remote pod has an environment variable `REGION=1`, but this is an
     /// undesirable value, it's possible to use `override` to set `REGION=2` (locally) instead.
+    ///
+    /// Environment specified here will also override variables passed via the env file.
     pub r#override: Option<HashMap<String, String>>, // `r#`: `override` is a Rust keyword.
 
     /// ### feature.env.load_from_process {#feature-env-load_from_process}
@@ -91,21 +97,32 @@ pub struct EnvConfig {
     /// This is case insensitive, meaning if you'd put `AWS_PROFILE` it'd unset both `AWS_PROFILE`
     /// and `Aws_Profile` and other variations.
     pub unset: Option<VecOrSingle<String>>,
+
+    /// ### feature.env_file {#feature-env-file}
+    ///
+    /// Allows for passing environment variables from an env file.
+    ///
+    /// These variables will override environment fetched from the remote target.
+    #[config(env = MIRRORD_OVERRIDE_ENV_FILE_ENV)]
+    pub env_file: Option<PathBuf>,
 }
 
 impl MirrordToggleableConfig for EnvFileConfig {
     fn disabled_config(context: &mut ConfigContext) -> Result<Self::Generated> {
         Ok(EnvConfig {
-            include: FromEnv::new("MIRRORD_OVERRIDE_ENV_VARS_INCLUDE")
+            include: FromEnv::new(MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV)
                 .source_value(context)
                 .transpose()?,
-            exclude: FromEnv::new("MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE")
+            exclude: FromEnv::new(MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV)
                 .source_value(context)
                 .transpose()?
                 .or_else(|| Some(VecOrSingle::Single("*".to_owned()))),
             load_from_process: None,
             r#override: None,
             unset: None,
+            env_file: FromEnv::new(MIRRORD_OVERRIDE_ENV_FILE_ENV)
+                .source_value(context)
+                .transpose()?,
         })
     }
 }
@@ -140,6 +157,7 @@ impl CollectAnalytics for &EnvConfig {
                 .map(|v| v.len() as u32)
                 .unwrap_or_default(),
         );
+        analytics.add("env_file_used", self.env_file.is_some());
     }
 }
 
@@ -164,8 +182,8 @@ mod tests {
     ) {
         with_env_vars(
             vec![
-                ("MIRRORD_OVERRIDE_ENV_VARS_INCLUDE", include.0),
-                ("MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE", exclude.0),
+                (MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV, include.0),
+                (MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV, exclude.0),
             ],
             || {
                 let mut cfg_context = ConfigContext::default();
@@ -192,8 +210,8 @@ mod tests {
     ) {
         with_env_vars(
             vec![
-                ("MIRRORD_OVERRIDE_ENV_VARS_INCLUDE", include.0),
-                ("MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE", exclude.0),
+                (MIRRORD_OVERRIDE_ENV_VARS_INCLUDE_ENV, include.0),
+                (MIRRORD_OVERRIDE_ENV_VARS_EXCLUDE_ENV, exclude.0),
             ],
             || {
                 let mut cfg_context = ConfigContext::default();

mirrord/layer/Cargo.toml

@@ -49,6 +49,7 @@ exec.workspace = true
 syscalls = { version = "0.6", features = ["full"] }
 null-terminated = "0.3"
 base64.workspace = true
+envfile.workspace = true
 
 [target.'cfg(target_os = "macos")'.dependencies]
 mirrord-sip = { path = "../sip" }