From 81a9598389be408ccddefa561a408b00501b0cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Smolarek?= <34063647+Razz4780@users.noreply.github.com> Date: Wed, 20 Nov 2024 16:23:50 +0100 Subject: [PATCH] Support passing env vars from a file (#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 <43983236+meowjesty@users.noreply.github.com> --------- Co-authored-by: meowjesty <43983236+meowjesty@users.noreply.github.com> --- Cargo.lock | 26 ++++++++++++++++++++++ Cargo.toml | 1 + changelog.d/1913.added.md | 1 + mirrord-schema.json | 10 ++++++++- mirrord/cli/Cargo.toml | 1 + mirrord/cli/src/config.rs | 33 ++++++++++++++++++++++++---- mirrord/cli/src/error.rs | 4 ++++ mirrord/cli/src/execution.rs | 29 +++++++++++++++---------- mirrord/config/configuration.md | 8 +++++++ mirrord/config/src/feature/env.rs | 36 +++++++++++++++++++++++-------- mirrord/layer/Cargo.toml | 1 + mirrord/layer/src/lib.rs | 31 +++++++++++++++----------- 12 files changed, 144 insertions(+), 37 deletions(-) create mode 100644 changelog.d/1913.added.md diff --git a/Cargo.lock b/Cargo.lock index 79aa40a53a1..6f90530d4e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2217,6 +2217,15 @@ dependencies = [ "log", ] +[[package]] +name = "envfile" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5fe7fe9d71fc403a41672475b8895d6b817cac0fe23c471e01d36f5af503008c" +dependencies = [ + "snailquote", +] + [[package]] name = "envy" version = "0.4.2" @@ -4113,6 +4122,7 @@ dependencies = [ "clap_complete", "const-random", "drain", + "envfile", "exec", "futures", "http-body 1.0.1", @@ -4384,6 +4394,7 @@ dependencies = [ "bytemuck", "chrono", "ctor", + "envfile", "errno 0.3.9", "exec", "fancy-regex", @@ -6575,6 +6586,15 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "snailquote" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77c10ec36e3a5cf387fd822263730434205464fbf2b1531054f2f92ee1a7ef4e" +dependencies = [ + "unicode_categories", +] + [[package]] name = "socket2" version = "0.5.7" @@ -7566,6 +7586,12 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "unicode_categories" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39ec24b3121d976906ece63c9daad25b85969647682eee313cb5779fdd69e14e" + [[package]] name = "unreachable" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index 77b128c89bc..e7c866fd1df 100644 --- a/Cargo.toml +++ b/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`. diff --git a/changelog.d/1913.added.md b/changelog.d/1913.added.md new file mode 100644 index 00000000000..977451af271 --- /dev/null +++ b/changelog.d/1913.added.md @@ -0,0 +1 @@ +Added a configuration option that allows for specifying an env file for mirrord execution. \ No newline at end of file diff --git a/mirrord-schema.json b/mirrord-schema.json index ca81c1b6ba2..2e9f02630f9 100644 --- a/mirrord-schema.json +++ b/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" diff --git a/mirrord/cli/Cargo.toml b/mirrord/cli/Cargo.toml index c72bfc6a6a5..50ac607c175 100644 --- a/mirrord/cli/Cargo.toml +++ b/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 diff --git a/mirrord/cli/src/config.rs b/mirrord/cli/src/config.rs index 378653cf41d..57572ace947 100644 --- a/mirrord/cli/src/config.rs +++ b/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, + + /// 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, } 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) } } diff --git a/mirrord/cli/src/error.rs b/mirrord/cli/src/error.rs index e26649d93aa..0cadf02b70c 100644 --- a/mirrord/cli/src/error.rs +++ b/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( diff --git a/mirrord/cli/src/execution.rs b/mirrord/cli/src/execution.rs index d294da8ade8..ec23590a9dc 100644 --- a/mirrord/cli/src/execution.rs +++ b/mirrord/cli/src/execution.rs @@ -448,8 +448,6 @@ impl MirrordExecution { config: &LayerConfig, connection: &mut AgentConnection, ) -> CliResult> { - 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) diff --git a/mirrord/config/configuration.md b/mirrord/config/configuration.md index 562cd800bbb..849ca25bebb 100644 --- a/mirrord/config/configuration.md +++ b/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. diff --git a/mirrord/config/src/feature/env.rs b/mirrord/config/src/feature/env.rs index 08524eba21b..cbc0a190a81 100644 --- a/mirrord/config/src/feature/env.rs +++ b/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>, /// ### 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>, /// ### 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>, // `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>, + + /// ### 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, } impl MirrordToggleableConfig for EnvFileConfig { fn disabled_config(context: &mut ConfigContext) -> Result { 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(); diff --git a/mirrord/layer/Cargo.toml b/mirrord/layer/Cargo.toml index a2ba9658609..f401d053e77 100644 --- a/mirrord/layer/Cargo.toml +++ b/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" } diff --git a/mirrord/layer/src/lib.rs b/mirrord/layer/src/lib.rs index 10d0a82a68c..599266706f3 100644 --- a/mirrord/layer/src/lib.rs +++ b/mirrord/layer/src/lib.rs @@ -433,22 +433,29 @@ fn fetch_env_vars() -> HashMap { (None, None) => (HashSet::new(), HashSet::from(EnvVars("*".to_owned()))), }; - if !env_vars_exclude.is_empty() || !env_vars_include.is_empty() { - let mut remote_env = make_proxy_request_with_response(GetEnvVarsRequest { - env_vars_filter: env_vars_exclude, - env_vars_select: env_vars_include, + let mut env_vars = (!env_vars_exclude.is_empty() || !env_vars_include.is_empty()) + .then(|| { + make_proxy_request_with_response(GetEnvVarsRequest { + env_vars_filter: env_vars_exclude, + env_vars_select: env_vars_include, + }) + .expect("failed to make request to proxy") + .expect("failed to fetch remote env") }) - .expect("failed to make request to proxy") - .expect("failed to fetch remote env"); + .unwrap_or_default(); - if let Some(overrides) = setup().env_config().r#override.as_ref() { - remote_env.extend(overrides.iter().map(|(k, v)| (k.clone(), v.clone()))); - } + if let Some(file) = &setup().env_config().env_file { + let envs_from_file = envfile::EnvFile::new(file) + .expect("failed to access env file") + .store; + env_vars.extend(envs_from_file); + } - remote_env - } else { - Default::default() + if let Some(overrides) = setup().env_config().r#override.as_ref() { + env_vars.extend(overrides.iter().map(|(k, v)| (k.clone(), v.clone()))); } + + env_vars } /// We need to hook execve syscall to allow mirrord-layer to be loaded with sip patch when loading