Skip to content

Conversation

@joshrotenberg
Copy link
Collaborator

Summary

Fixes #436 - When --config-file is explicitly specified, environment variables are now ignored to provide true configuration isolation.

Problem

Previously, environment variables always took precedence over config files, even when --config-file was explicitly specified. This made it impossible to achieve true configuration isolation for testing and violated the principle of "explicit wins" in CLI design.

Old behavior (problematic):

Priority: Env vars > --config-file > Default config

Solution

New behavior:

Priority: --config-file (explicit) > Env vars > Default config

When --config-file is explicitly provided, environment variables are completely ignored for credential resolution.

Changes

  1. ConnectionManager: Tracks whether config_path was explicitly provided
  2. Credential Resolution:
    • create_cloud_client() and create_enterprise_client() skip env var checks when config_path is Some()
    • Added debug logging showing when env vars are ignored
  3. API Command: Fixed bug where ApiCommandParams wasn't preserving config_path, causing it to be lost
  4. Tests: Added 2 new integration tests to verify the behavior

Testing

New 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 without --config-file

Test Results

redisctl tests: 132 passed (31 unit + 24 basic CLI + 29 mocked + 15 profile + 2 output + 31 other)

Benefits

  1. Testing Isolation: Tests can use --config-file without worrying about env vars
  2. Principle of Least Surprise: Explicit flags override implicit environment
  3. CLI Best Practice: Follows patterns from kubectl --kubeconfig, git --config, etc.
  4. Backward Compatible: Only affects behavior when --config-file is explicitly used

Example

Before (env vars override config file):

export REDIS_CLOUD_API_KEY=production-key
redisctl --config-file test.toml cloud subscription list
# Uses production-key (WRONG!)

After (config file wins when explicit):

export REDIS_CLOUD_API_KEY=production-key  
redisctl --config-file test.toml cloud subscription list
# Uses test.toml credentials (CORRECT!)

Without --config-file (env vars still work):

export REDIS_CLOUD_API_KEY=my-key
redisctl cloud subscription list
# Uses my-key from env (works as before)

Closes #436

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
@joshrotenberg joshrotenberg merged commit abe6620 into main Oct 30, 2025
16 checks passed
@joshrotenberg joshrotenberg deleted the fix/config-file-precedence branch October 30, 2025 04:41
@joshrotenberg joshrotenberg mentioned this pull request Oct 30, 2025
@joshrotenberg joshrotenberg mentioned this pull request Nov 25, 2025
This was referenced Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: --config-file should take precedence over environment variables

2 participants