fix(vdev): volume path resolution (#23637)

* fix(vdev): volume path resolution

* push env var resolution to the forefront

* ran cargo fmt

* rename to environment.rs

* ran cargo fmt

* clippy fixes

* refactor

* unix only - fix windows clippy

Author: pront

Cargo.lock

@@ -37,3 +37,4 @@ toml.workspace = true
 semver.workspace = true
 indoc.workspace = true
 git2 = { version = "0.20.2" }
+cfg-if.workspace = true

vdev/Cargo.toml

@@ -1,7 +1,7 @@
+use crate::environment::Environment;
+use crate::testing::{config::ComposeTestConfig, state};
 use anyhow::Result;
 
-use crate::testing::{config::ComposeTestConfig, config::Environment, state};
-
 pub fn exec(integration: Option<&String>, path: &str) -> Result<()> {
     match integration {
         None => show_all(path),

vdev/src/commands/compose_tests/show.rs

@@ -0,0 +1,74 @@
+use cfg_if::cfg_if;
+use std::collections::BTreeMap;
+use std::process::Command;
+
+cfg_if! {
+    if #[cfg(unix)] {
+        use std::sync::OnceLock;
+        use regex::{Captures, Regex};
+    }
+}
+
+pub type Environment = BTreeMap<String, Option<String>>;
+
+pub(crate) fn rename_environment_keys(environment: &Environment) -> Environment {
+    environment
+        .iter()
+        .map(|(var, value)| {
+            (
+                format!("CONFIG_{}", var.replace('-', "_").to_uppercase()),
+                value.clone(),
+            )
+        })
+        .collect()
+}
+
+pub(crate) fn extract_present(environment: &Environment) -> BTreeMap<String, String> {
+    environment
+        .iter()
+        .filter_map(|(k, v)| v.as_ref().map(|s| (k.clone(), s.clone())))
+        .collect()
+}
+
+pub(crate) fn append_environment_variables(command: &mut Command, environment: &Environment) {
+    for (key, value) in environment {
+        command.arg("--env");
+        match value {
+            Some(value) => command.arg(format!("{key}={value}")),
+            None => command.arg(key),
+        };
+    }
+}
+
+cfg_if! {
+if #[cfg(unix)] {
+    /// Resolve all environment variable placeholders. If the variable is not found or is `None`, it is left unchanged.
+    pub fn resolve_placeholders(input: &str, environment: &Environment) -> String {
+        static BRACED: OnceLock<Regex> = OnceLock::new();
+        static BARE: OnceLock<Regex> = OnceLock::new();
+
+        let braced =
+            BRACED.get_or_init(|| Regex::new(r"\$\{([A-Za-z0-9_]+)\}").expect("cannot build regex"));
+        let bare = BARE.get_or_init(|| Regex::new(r"\$([A-Za-z0-9_]+)").expect("cannot build regex"));
+
+        // First replace ${VAR}
+        let step1 = braced.replace_all(input, |captures: &Captures| {
+            resolve_or_keep(&captures[0], &captures[1], environment)
+        });
+
+        // Then replace $VAR
+        bare.replace_all(&step1, |captures: &Captures| {
+            resolve_or_keep(&captures[0], &captures[1], environment)
+        })
+        .into_owned()
+    }
+
+    #[cfg(unix)]
+    fn resolve_or_keep(full: &str, name: &str, env: &Environment) -> String {
+        env.get(name)
+            .and_then(Clone::clone)
+            .or_else(|| std::env::var(name).ok())
+            .unwrap_or_else(|| full.to_string())
+    }
+}
+}

vdev/src/environment.rs

@@ -11,6 +11,7 @@ mod macros;
 mod app;
 mod commands;
 mod config;
+mod environment;
 mod features;
 mod git;
 mod platform;
@@ -19,9 +20,8 @@ mod util;
 
 use anyhow::Result;
 use clap::Parser;
-use std::env;
-
 use commands::Cli;
+use std::env;
 
 fn main() -> Result<()> {
     let cli = Cli::parse();

vdev/src/main.rs

@@ -1,5 +1,6 @@
 use crate::app;
 use crate::app::CommandExt;
+use crate::environment::{Environment, extract_present};
 use crate::testing::config::RustToolchainConfig;
 use crate::testing::docker::docker_command;
 use crate::util::IS_A_TTY;
@@ -17,20 +18,21 @@ pub fn prepare_build_command(
     image: &str,
     dockerfile: &Path,
     features: Option<&[String]>,
+    config_environment_variables: &Environment,
 ) -> Command {
     // Start with `docker build`
-    let mut cmd = docker_command(["build"]);
+    let mut command = docker_command(["build"]);
 
     // Ensure we run from the repo root (so `.` context is correct)
-    cmd.current_dir(app::path());
+    command.current_dir(app::path());
 
     // If we're attached to a TTY, show fancy progress
     if *IS_A_TTY {
-        cmd.args(["--progress", "tty"]);
+        command.args(["--progress", "tty"]);
     }
 
     // Add all of the flags in one go
-    cmd.args([
+    command.args([
         "--pull",
         "--tag",
         image,
@@ -42,10 +44,12 @@ pub fn prepare_build_command(
         &format!("RUST_VERSION={}", RustToolchainConfig::rust_version()),
         "--build-arg",
         &format!("FEATURES={}", features.unwrap_or(&[]).join(",")),
-        ".",
     ]);
 
-    cmd
+    command.envs(extract_present(config_environment_variables));
+
+    command.args(["."]);
+    command
 }
 
 #[allow(dead_code)]
@@ -59,6 +63,7 @@ pub fn build_integration_image() -> Result<()> {
         &image,
         &dockerfile,
         Some(&[ALL_INTEGRATIONS_FEATURE_FLAG.to_string()]),
+        &Environment::default(),
     );
     waiting!("Building {image}");
     cmd.check_run()

vdev/src/testing/build.rs

@@ -8,15 +8,14 @@ use itertools::{self, Itertools};
 use serde::{Deserialize, Serialize};
 use serde_yaml::Value;
 
+use crate::environment::Environment;
 use crate::{app, util};
 
 const FILE_NAME: &str = "test.yaml";
 
 pub const INTEGRATION_TESTS_DIR: &str = "integration";
 pub const E2E_TESTS_DIR: &str = "e2e";
 
-pub type Environment = BTreeMap<String, Option<String>>;
-
 #[derive(Deserialize, Debug)]
 pub struct RustToolchainRootConfig {
     pub toolchain: RustToolchainConfig,

vdev/src/testing/config.rs

@@ -4,12 +4,12 @@ use anyhow::{Context, Result, bail};
 use tempfile::{Builder, NamedTempFile};
 
 use super::config::{
-    ComposeConfig, ComposeTestConfig, E2E_TESTS_DIR, Environment, INTEGRATION_TESTS_DIR,
-    RustToolchainConfig,
+    ComposeConfig, ComposeTestConfig, E2E_TESTS_DIR, INTEGRATION_TESTS_DIR, RustToolchainConfig,
 };
 use super::runner::{ContainerTestRunner as _, IntegrationTestRunner, TestRunner as _};
 use super::state::EnvsDir;
 use crate::app::CommandExt as _;
+use crate::environment::{Environment, extract_present, rename_environment_keys};
 use crate::testing::build::ALL_INTEGRATIONS_FEATURE_FLAG;
 use crate::testing::docker::{CONTAINER_TOOL, DOCKER_SOCKET};
 
@@ -103,7 +103,7 @@ impl ComposeTest {
             envs_dir,
             runner,
             compose,
-            env_config,
+            env_config: rename_environment_keys(&env_config),
             build_all,
             retries,
         };
@@ -121,8 +121,8 @@ impl ComposeTest {
 
         let mut env_vars = self.config.env.clone();
         // Make sure the test runner has the same config environment vars as the services do.
-        for (key, value) in config_env(&self.env_config) {
-            env_vars.insert(key, Some(value));
+        for (key, value) in self.env_config.clone() {
+            env_vars.insert(key, value);
         }
 
         env_vars.insert("TEST_LOG".to_string(), Some("info".into()));
@@ -181,8 +181,11 @@ impl ComposeTest {
         // image for the runner. So we must build that image before starting the
         // compose so that it is available.
         if self.local_config.kind == ComposeTestKind::E2E {
-            self.runner
-                .build(Some(&self.config.features), self.local_config.directory)?;
+            self.runner.build(
+                Some(&self.config.features),
+                self.local_config.directory,
+                &self.env_config,
+            )?;
         }
 
         self.config.check_required()?;
@@ -275,9 +278,11 @@ impl Compose {
         }
     }
 
-    fn start(&self, config: &Environment) -> Result<()> {
-        self.prepare()?;
-        self.run("Starting", &["up", "--detach"], Some(config))
+    fn start(&self, environment: &Environment) -> Result<()> {
+        #[cfg(unix)]
+        unix::prepare_compose_volumes(&self.config, &self.test_dir, environment)?;
+
+        self.run("Starting", &["up", "--detach"], Some(environment))
     }
 
     fn stop(&self) -> Result<()> {
@@ -289,7 +294,12 @@ impl Compose {
         )
     }
 
-    fn run(&self, action: &str, args: &[&'static str], config: Option<&Environment>) -> Result<()> {
+    fn run(
+        &self,
+        action: &str,
+        args: &[&'static str],
+        environment: Option<&Environment>,
+    ) -> Result<()> {
         let mut command = Command::new(CONTAINER_TOOL.clone());
         command.arg("compose");
         // When the integration test environment is already active, the tempfile path does not
@@ -320,30 +330,13 @@ impl Compose {
                 command.env(key, value);
             }
         }
-        if let Some(config) = config {
-            command.envs(config_env(config));
+        if let Some(environment) = environment {
+            command.envs(extract_present(environment));
         }
 
         waiting!("{action} service environment");
         command.check_run()
     }
-
-    fn prepare(&self) -> Result<()> {
-        #[cfg(unix)]
-        unix::prepare_compose_volumes(&self.config, &self.test_dir)?;
-        Ok(())
-    }
-}
-
-fn config_env(config: &Environment) -> impl Iterator<Item = (String, String)> + '_ {
-    config.iter().filter_map(|(var, value)| {
-        value.as_ref().map(|value| {
-            (
-                format!("CONFIG_{}", var.replace('-', "_").to_uppercase()),
-                value.to_string(),
-            )
-        })
-    })
 }
 
 #[cfg(unix)]
@@ -353,6 +346,7 @@ mod unix {
     use std::path::{Path, PathBuf};
 
     use super::super::config::ComposeConfig;
+    use crate::environment::{Environment, resolve_placeholders};
     use crate::testing::config::VolumeMount;
     use anyhow::{Context, Result};
 
@@ -362,7 +356,11 @@ mod unix {
     const ALL_READ_DIR: u32 = 0o555;
 
     /// Fix up potential issues before starting a compose container
-    pub fn prepare_compose_volumes(config: &ComposeConfig, test_dir: &Path) -> Result<()> {
+    pub fn prepare_compose_volumes(
+        config: &ComposeConfig,
+        test_dir: &Path,
+        environment: &Environment,
+    ) -> Result<()> {
         for service in config.services.values() {
             if let Some(volumes) = &service.volumes {
                 for volume in volumes {
@@ -374,12 +372,12 @@ mod unix {
                         }
                         VolumeMount::Long { source, .. } => source,
                     };
-
-                    if !config.volumes.contains_key(source)
+                    let source = resolve_placeholders(source, environment);
+                    if !config.volumes.contains_key(&source)
                         && !source.starts_with('/')
                         && !source.starts_with('$')
                     {
-                        let path: PathBuf = [test_dir, Path::new(source)].iter().collect();
+                        let path: PathBuf = [test_dir, Path::new(&source)].iter().collect();
                         add_read_permission(&path)?;
                     }
                 }