feat: make --config-file take precedence over environment variables (#438)

When --config-file is explicitly specified, environment variables
are now ignored to provide true configuration isolation. This follows
CLI best practices where explicit arguments override implicit environment.

Changes:
- ConnectionManager tracks whether config_path was explicitly provided
- create_cloud_client() and create_enterprise_client() skip env var
  checks when config_path is Some()
- ApiCommandParams now includes config_path to preserve it through
  API command execution
- Added debug logging to show when env vars are being ignored

Tests:
- test_config_file_overrides_env_vars: verifies env vars are ignored
  when --config-file is specified
- test_env_vars_work_without_config_file: verifies env vars still work
  when --config-file is not specified

This provides true configuration isolation for testing and follows the
principle of "explicit wins" (CLI args > env vars > defaults).

Fixes #436

Author: joshrotenberg

crates/redisctl/src/commands/api.rs

@@ -12,6 +12,7 @@ use serde_json::Value;
 #[allow(dead_code)] // Used by binary target
 pub struct ApiCommandParams {
     pub config: Config,
+    pub config_path: Option<std::path::PathBuf>,
     pub profile_name: Option<String>,
     pub deployment: DeploymentType,
     pub method: HttpMethod,
@@ -24,7 +25,7 @@ pub struct ApiCommandParams {
 /// Handle raw API commands
 #[allow(dead_code)] // Used by binary target
 pub async fn handle_api_command(params: ApiCommandParams) -> CliResult<()> {
-    let connection_manager = ConnectionManager::new(params.config);
+    let connection_manager = ConnectionManager::with_config_path(params.config, params.config_path);
 
     match params.deployment {
         DeploymentType::Cloud => {

crates/redisctl/src/connection.rs

@@ -46,6 +46,10 @@ impl ConnectionManager {
     }
 
     /// Create a Cloud client from profile credentials with environment variable override support
+    ///
+    /// When --config-file is explicitly specified, environment variables are ignored to provide
+    /// true configuration isolation. This allows testing with isolated configs and follows the
+    /// principle of "explicit wins" (CLI args > env vars > defaults).
     #[allow(dead_code)] // Used by binary target
     pub async fn create_cloud_client(
         &self,
@@ -54,10 +58,35 @@ impl ConnectionManager {
         debug!("Creating Redis Cloud client");
         trace!("Profile name: {:?}", profile_name);
 
-        // Check if all required environment variables are present
-        let env_api_key = std::env::var("REDIS_CLOUD_API_KEY").ok();
-        let env_api_secret = std::env::var("REDIS_CLOUD_SECRET_KEY").ok();
-        let env_api_url = std::env::var("REDIS_CLOUD_API_URL").ok();
+        // When --config-file is explicitly specified, ignore environment variables
+        // This provides true configuration isolation for testing and follows CLI best practices
+        let use_env_vars = self.config_path.is_none();
+
+        debug!(
+            "Config path: {:?}, use_env_vars: {}",
+            self.config_path, use_env_vars
+        );
+
+        if !use_env_vars {
+            info!("--config-file specified explicitly, ignoring environment variables");
+        }
+
+        // Check if all required environment variables are present (only if we're using them)
+        let env_api_key = if use_env_vars {
+            std::env::var("REDIS_CLOUD_API_KEY").ok()
+        } else {
+            None
+        };
+        let env_api_secret = if use_env_vars {
+            std::env::var("REDIS_CLOUD_SECRET_KEY").ok()
+        } else {
+            None
+        };
+        let env_api_url = if use_env_vars {
+            std::env::var("REDIS_CLOUD_API_URL").ok()
+        } else {
+            None
+        };
 
         if env_api_key.is_some() {
             debug!("Found REDIS_CLOUD_API_KEY environment variable");
@@ -140,6 +169,10 @@ impl ConnectionManager {
     }
 
     /// Create an Enterprise client from profile credentials with environment variable override support
+    ///
+    /// When --config-file is explicitly specified, environment variables are ignored to provide
+    /// true configuration isolation. This allows testing with isolated configs and follows the
+    /// principle of "explicit wins" (CLI args > env vars > defaults).
     #[allow(dead_code)] // Used by binary target
     pub async fn create_enterprise_client(
         &self,
@@ -148,11 +181,40 @@ impl ConnectionManager {
         debug!("Creating Redis Enterprise client");
         trace!("Profile name: {:?}", profile_name);
 
-        // Check if all required environment variables are present
-        let env_url = std::env::var("REDIS_ENTERPRISE_URL").ok();
-        let env_user = std::env::var("REDIS_ENTERPRISE_USER").ok();
-        let env_password = std::env::var("REDIS_ENTERPRISE_PASSWORD").ok();
-        let env_insecure = std::env::var("REDIS_ENTERPRISE_INSECURE").ok();
+        // When --config-file is explicitly specified, ignore environment variables
+        // This provides true configuration isolation for testing and follows CLI best practices
+        let use_env_vars = self.config_path.is_none();
+
+        debug!(
+            "Config path: {:?}, use_env_vars: {}",
+            self.config_path, use_env_vars
+        );
+
+        if !use_env_vars {
+            info!("--config-file specified explicitly, ignoring environment variables");
+        }
+
+        // Check if all required environment variables are present (only if we're using them)
+        let env_url = if use_env_vars {
+            std::env::var("REDIS_ENTERPRISE_URL").ok()
+        } else {
+            None
+        };
+        let env_user = if use_env_vars {
+            std::env::var("REDIS_ENTERPRISE_USER").ok()
+        } else {
+            None
+        };
+        let env_password = if use_env_vars {
+            std::env::var("REDIS_ENTERPRISE_PASSWORD").ok()
+        } else {
+            None
+        };
+        let env_insecure = if use_env_vars {
+            std::env::var("REDIS_ENTERPRISE_INSECURE").ok()
+        } else {
+            None
+        };
 
         if env_url.is_some() {
             debug!("Found REDIS_ENTERPRISE_URL environment variable");

crates/redisctl/src/main.rs

@@ -26,11 +26,17 @@ async fn main() -> Result<()> {
     // Load configuration from specified path or default location
     let (config, config_path) = if let Some(config_file) = &cli.config_file {
         let path = std::path::PathBuf::from(config_file);
+        debug!("Loading config from explicit path: {:?}", path);
         let config = Config::load_from_path(&path)?;
         (config, Some(path))
     } else {
+        debug!("Loading config from default location");
         (Config::load()?, None)
     };
+    debug!(
+        "Creating ConnectionManager with config_path: {:?}",
+        config_path
+    );
     let conn_mgr = ConnectionManager::with_config_path(config, config_path);
 
     // Execute command
@@ -821,6 +827,7 @@ async fn execute_api_command(
 ) -> Result<(), RedisCtlError> {
     commands::api::handle_api_command(commands::api::ApiCommandParams {
         config: conn_mgr.config.clone(),
+        config_path: conn_mgr.config_path.clone(),
         profile_name: cli.profile.clone(),
         deployment: *deployment,
         method: method.clone(),

crates/redisctl/tests/cli_integration_mocked_tests.rs

@@ -997,3 +997,70 @@ async fn test_cloud_maintenance_windows() {
         .stdout(predicate::str::contains("maintenanceWindows"))
         .stdout(predicate::str::contains("weekly"));
 }
+
+#[tokio::test]
+async fn test_config_file_overrides_env_vars() {
+    let temp_dir = TempDir::new().unwrap();
+    let mock_server = MockServer::start().await;
+
+    create_cloud_profile(&temp_dir, &mock_server.uri()).unwrap();
+
+    // Mock endpoint expecting credentials from config file
+    // Note: Matches any path to debug what request is being made
+    Mock::given(method("GET"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+            "message": "config file credentials used"
+        })))
+        .mount(&mock_server)
+        .await;
+
+    let mut cmd = Command::cargo_bin("redisctl").unwrap();
+    let config_file = temp_dir.path().join("config.toml");
+
+    // Set environment variables that should be IGNORED when --config-file is specified
+    cmd.env("REDIS_CLOUD_API_KEY", "wrong-env-key");
+    cmd.env("REDIS_CLOUD_SECRET_KEY", "wrong-env-secret");
+
+    // The --config-file flag should take precedence over env vars
+    cmd.arg("--config-file")
+        .arg(config_file)
+        .arg("api")
+        .arg("cloud")
+        .arg("get")
+        .arg("/test")
+        .assert()
+        .success()
+        .stdout(predicate::str::contains("config file credentials used"));
+}
+
+#[tokio::test]
+async fn test_env_vars_work_without_config_file() {
+    let mock_server = MockServer::start().await;
+
+    // Mock endpoint expecting credentials from environment variables
+    Mock::given(method("GET"))
+        .and(path("/test"))
+        .and(header("x-api-key", "env-api-key"))
+        .and(header("x-api-secret-key", "env-api-secret"))
+        .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+            "message": "env credentials used"
+        })))
+        .expect(1)
+        .mount(&mock_server)
+        .await;
+
+    let mut cmd = Command::cargo_bin("redisctl").unwrap();
+
+    // Without --config-file, env vars should work
+    cmd.env("REDIS_CLOUD_API_KEY", "env-api-key");
+    cmd.env("REDIS_CLOUD_SECRET_KEY", "env-api-secret");
+    cmd.env("REDIS_CLOUD_API_URL", mock_server.uri());
+
+    cmd.arg("api")
+        .arg("cloud")
+        .arg("get")
+        .arg("/test")
+        .assert()
+        .success()
+        .stdout(predicate::str::contains("env credentials used"));
+}