Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/redisctl/src/commands/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 => {
Expand Down
80 changes: 71 additions & 9 deletions crates/redisctl/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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");
Expand Down Expand Up @@ -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,
Expand All @@ -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");
Expand Down
7 changes: 7 additions & 0 deletions crates/redisctl/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
67 changes: 67 additions & 0 deletions crates/redisctl/tests/cli_integration_mocked_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}