-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Problem
When --config-file is explicitly specified, environment variables still take precedence over the config file values. This makes it difficult to achieve true configuration isolation for testing and creates surprising behavior.
Current Behavior (Problematic)
Priority: Env vars > --config-file > Default config
If you have:
REDIS_CLOUD_API_KEY=production-keyin environment--config-file /tmp/test.tomlwithapi_key = "test-key"
Result: Uses production-key (from env) instead of test-key (from explicit config)
Expected Behavior
Priority: --config-file (explicit) > Env vars > Default config
When --config-file is explicitly provided, it should take precedence over environment variables.
Rationale
- Principle of Least Surprise: Explicit flags should override implicit environment
- Testing Isolation: Makes it possible to test with isolated configs without unsetting env vars
- Explicit Wins: CLI best practice - explicit arguments > environment > defaults
- Consistency: Similar to how tools like
kubectl --kubeconfigorgit --configwork
Proposed Solution
Option 1: Auto-disable env vars when --config-file is used (Recommended)
When --config-file is specified, ignore environment variable overrides for credential lookups.
Pros:
- Simple and intuitive
- Complete isolation by default
- No additional flags needed
Cons:
- Changes existing behavior (breaking change)
- Can't selectively override single credentials from env
Option 2: Add --ignore-env or --strict-config flag
Add a flag to explicitly disable environment variable lookups.
redisctl --config-file /tmp/test.toml --ignore-env cloud subscription listPros:
- Opt-in, not breaking
- Explicit control
Cons:
- More flags to remember
- Verbose
Use Cases
Testing (Primary driver)
// Currently fails because env vars override test config
let test_config = create_test_config_with_mock_server();
Command::cargo_bin("redisctl")
.arg("--config-file").arg(test_config)
.arg("cloud").arg("subscription").arg("list")
.assert()
.success(); // ❌ Fails - uses real credentials from envTemporary config override
# Want to test against staging without modifying main config
redisctl --config-file staging.toml cloud subscription list
# ❌ Currently uses production creds from env vars anywayRecommendation
Implement Option 1 (auto-disable env vars when --config-file specified) as the new behavior in next major version. This provides the most intuitive behavior and is consistent with other CLI tools.
Related
- Discovered while adding mocked integration tests in test: add comprehensive CLI tests with assert_cmd #435
- Current workaround: Tests must explicitly unset all env vars using
env_remove()