You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix: Profile and identity flags loading and propagation (#1805)
* fix profiles
* fix profiles
* fix profiles
* [autofix.ci] apply automated fixes
* test: Add tests for profile and identity flag handling
Added comprehensive unit tests to increase code coverage for the profile
and identity flag fixes:
- pkg/config/load_flags_test.go: Tests getProfilesFromFlagsOrEnv() function
covering environment variables, CLI flags (both syntaxes), and edge cases
- internal/exec/terraform_nested_auth_helper_test.go: Added TestIdentityInheritanceLogic
to verify identity extraction from AuthManager chain for nested component operations
These tests cover the new code paths added for fixing:
1. Profile loading from --profile CLI flag
2. Identity propagation to nested components via parent AuthManager chain
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* test: Add comprehensive tests for auth helper edge cases
Added tests to increase coverage for profile and identity flag handling:
- TestGetProfilesFromFlagsOrEnv: Added test for environment variable precedence
- TestResolveAuthManagerForNestedComponent_WithoutAuthSection: Tests early return paths
- TestHasDefaultIdentity_EdgeCases: Tests additional edge cases including:
- Multiple defaults
- Missing default field
- Mixed valid/invalid configs
These tests target the uncovered code paths in:
- pkg/config/load.go: getProfilesFromFlagsOrEnv() and edge cases
- internal/exec/terraform_nested_auth_helper.go: hasDefaultIdentity() and early returns
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix: Correct precedence - CLI flags override environment variables
Fixed the precedence order in getProfilesFromFlagsOrEnv() to ensure
CLI flags take precedence over environment variables, which is the
expected behavior for CLI applications.
Before: Environment variables took precedence over CLI flags
After: CLI flags override environment variables (correct behavior)
Precedence order (highest to lowest):
1. CLI flags (--profile)
2. Environment variables (ATMOS_PROFILE)
3. Config file
4. Defaults
Updated test case to verify correct precedence behavior.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* docs: Update documentation to reflect correct CLI flag precedence
Updated documentation to clarify that CLI flags take precedence over
environment variables (standard CLI behavior):
- Fixed "Fix Overview" section to show CLI flags checked first
- Updated code example to show correct precedence order
- Added explicit precedence order list
- Clarified logic flow and why this works
Precedence (highest to lowest):
1. CLI flags (--profile)
2. Environment variables (ATMOS_PROFILE)
3. Config file
4. Defaults
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix: Use os.Getenv for ATMOS_PROFILE to avoid Viper caching in tests
Fixed CI test failures by reading ATMOS_PROFILE directly from environment
instead of using Viper, which caches environment variables and causes test
isolation issues.
Root cause:
- Viper caches environment variable values on first read
- In CI, if ATMOS_PROFILE was set by environment or previous tests,
Viper retained the cached value even after tests cleaned up
- This caused "profile not found" errors when trying to load non-existent
cached profile names
Solution:
- Read ATMOS_PROFILE using os.Getenv() for fresh reads on every call
- Added nolint:forbidigo with explanation for linter exception
- Updated test to use switch statement (staticcheck QF1003)
- Tests use t.Setenv() instead of viper.Set() to match implementation
Benefits:
- Proper test isolation (t.Setenv cleanup works correctly)
- No cached stale values
- CLI flags still take precedence (checked first)
- Comma-separated values handled consistently
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix: Handle comma-separated profiles in --profile value syntax
Fixed regression where --profile dev,staging (space syntax with commas)
was treated as a single profile instead of being split into multiple profiles.
Both syntaxes now handle comma-separated values consistently:
- --profile dev,staging → ["dev", "staging"]
- --profile=dev,staging → ["dev", "staging"]
Added test case to verify comma-separated values work with space syntax.
Fixes CodeRabbit feedback about comma-separated spaced syntax regression.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* test: Reset Viper state per subtest to prevent pollution
Added viper.Reset() before and after each subtest to ensure test isolation.
Viper is a global singleton, so mutations in one test could affect subsequent
tests without proper cleanup.
Changes:
- Call viper.Reset() at start of each subtest
- Use t.Cleanup(viper.Reset) to ensure cleanup after test
- Prevents test pollution where one test's Viper state affects another
Fixes CodeRabbit feedback about Viper state isolation.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* refactor: Use AuthProfileFlag constant instead of hardcoded strings
Replace hardcoded "--profile" strings with AuthProfileFlag constant
from pkg/config/const.go for consistency and maintainability.
Changes:
- pkg/config/load.go: Use AuthProfileFlag in parseProfilesFromArgs()
- pkg/config/load_flags_test.go: Use AuthProfileFlag in test cases
- pkg/config/load_profile_test.go: Use AuthProfileFlag in test cases
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* refactor: Rename AuthProfileFlag to AtmosProfileFlag
Rename the constant from AuthProfileFlag to AtmosProfileFlag because
profiles are not only for authentication. They configure multiple
aspects of Atmos behavior including base paths, component locations,
and various settings.
Changes:
- pkg/config/const.go: Rename constant and update comment
- pkg/config/load.go: Update all references
- pkg/config/load_flags_test.go: Update all references
- pkg/config/load_profile_test.go: Update all references
- internal/exec/cli_utils.go: Update global flags list
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* refactor: Rename AuthProfileFlag to AtmosProfileFlag
Rename the constant from AuthProfileFlag to AtmosProfileFlag because
profiles are not only for authentication. They configure multiple
aspects of Atmos behavior including base paths, component locations,
and various settings.
Changes:
- pkg/config/const.go: Rename constant and update comment
- pkg/config/load.go: Update all references
- pkg/config/load_flags_test.go: Update all references
- pkg/config/load_profile_test.go: Update all references
- internal/exec/cli_utils.go: Update global flags list
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* test: Add comprehensive edge case tests for profile parsing
Add extensive test coverage for profile parsing edge cases including:
- Empty entries in comma-separated lists
- Whitespace handling (leading, trailing, and standalone)
- Leading and trailing commas
- Multiple consecutive commas
- Mixed whitespace and empty values
- Empty profile values
Test improvements:
- pkg/config/load_flags_test.go: Added 4 new test cases for env var edge cases
- pkg/config/load_profile_test.go: Added 7 new test cases for flag parsing edge cases
- internal/exec/cli_utils_test.go: Added 2 new test functions with 8 test cases for ProcessCommandLineArgs
Coverage improvements:
- parseProfilesFromArgs: 100.0% (maintained)
- getProfilesFromFlagsOrEnv: 100.0% (maintained)
- ProcessCommandLineArgs: 86.1% → 91.1% (+5.0%)
- internal/exec overall: 65.7% → 65.8%
All tests verify the robustness improvements from filtering empty entries
when parsing comma-separated profile values.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix: Profile and identity flag handling (sync flags to Viper) (#1806)
* refactor: Sync global flags to Viper in root.go instead of passing cmd parameter
Remove the cmd parameter from InitCliConfig and LoadConfig signatures by syncing
global flags (profile, identity) from Cobra's FlagSet to Viper in root.go
PersistentPreRun before config loading. This is cleaner than passing cmd through
100+ function calls where 99% were nil.
Also simplify getProfilesFromFlagsOrEnv() to just read from Viper, which now has
synced flag values. Keep parseProfilesFromOsArgs() as fallback for
DisableFlagParsing commands (terraform/helmfile/packer) that bypass Cobra's
flag parsing.
Update docs/fixes/profile-and-identity-flags-loading.md to document Phase 3
(root cause fix) completion and future migration path.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* docs: Update implementation section to reflect Phase 3 solution
Update the 'Solution Implemented' section to accurately describe the Phase 3
implementation (syncing Viper in root.go) instead of the outdated Phase 2
approach (passing cmd parameter). Clarify the two-part solution: syncing in
root.go for normal commands and pflag fallback for DisableFlagParsing commands.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* docs: Rewrite fix documentation to focus on the solution
- Remove commit-by-commit evolutionary phases
- Focus on the final implementation and how it works
- Clearer explanation of root cause (Viper timing issue)
- Simplified structure: Problem → Root Cause → Solution → Verification
- Removed confusing 'rejected' alternative that we actually implemented
* test: Fix TestResolveIdentityName by clearing Viper state
The test was failing because resolveIdentityName() now reads from Viper
when the flag is not changed (due to syncGlobalFlagsToViper changes).
Tests need to ensure Viper has clean state for the identity flag.
* docs: Document source detection limitation in profile loading tests
Source detection (env vs flag) is best-effort for logging purposes only.
When both ATMOS_PROFILE env var and --profile flag are set, source may
be incorrectly reported as 'env' even though flag value was actually used.
This doesn't affect functionality - Viper and syncGlobalFlagsToViper()
ensure flag precedence is correct. The source string is only used for
debug logging.
* fix: Handle Viper's quirky comma-separated env var parsing for profiles
Viper does NOT automatically parse comma-separated environment variables
for StringSlice types. This commit adds proper handling for all Viper quirks:
- "dev,staging,prod" → []string{"dev,staging,prod"} (single element)
- "dev staging prod" → []string{"dev", "staging", "prod"} (splits on whitespace)
- " dev , staging " → []string{"dev", ",", "staging"} (splits on whitespace, keeps commas!)
Changes:
- pkg/config/load.go: Add parseViperProfilesFromEnv() helper to handle
Viper environment variable quirks. Refactor getProfilesFromFlagsOrEnv()
to reduce complexity (passes gocognit and nestif linters).
- pkg/config/load_flags_test.go: Add TestGetProfilesFromFlagsOrEnvWithRealViper()
that uses actual Viper BindEnv() to test real-world behavior
- pkg/config/load_flags_test.go: Restore environment variable edge case tests
(empty values, leading/trailing commas, whitespace, precedence)
- cmd/*_test.go: Add Viper.Reset() and clear identity env vars to prevent
test pollution
This fixes the critical bug where comma-separated profiles in ATMOS_PROFILE
environment variable were not parsed correctly.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
---------
Co-authored-by: Claude <[email protected]>
* docs: Replace hard tabs with spaces in code blocks
Fix markdown formatting by replacing all hard tabs with 4 spaces
in code blocks throughout the documentation file. This ensures
consistent rendering across different markdown parsers and editors.
Changes:
- Replaced 50+ instances of hard tabs with 4 spaces
- Maintains proper code block indentation
- Improves markdown consistency
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix profiles
* [autofix.ci] apply automated fixes
* fix: Add fallback to parseProfilesFromOsArgs for DisableFlagParsing commands
PR #1806 broke profile loading for terraform/helmfile/packer commands because
these commands have DisableFlagParsing=true, which means Cobra never parses
flags, so syncGlobalFlagsToViper() never syncs profile values to Viper.
This commit restores profile loading by:
1. Adding fallback to parseProfilesFromOsArgs(os.Args) when Viper doesn't have profiles
2. Adding fallback to os.Getenv("ATMOS_PROFILE") as final resort
3. Extracting helper functions to reduce cyclomatic complexity:
- parseProfilesFromEnvString() for comma-separated parsing
- getProfilesFromFallbacks() for fallback logic
4. Using profileDelimiter constant for "," literal
This ensures --profile flag works for all commands (normal and DisableFlagParsing).
Fixes: #1806 merge regression
Complexity: Reduced from 11 to 5 (nestif from 6 to 2)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix: Merge global auth config from profiles into component sections
PR #1806 regression: Profile auth config was loaded into atmosConfig.Auth
but not propagated to ComponentAuthSection, causing terraform pre-hook to
skip authentication with "No auth configuration found".
Root cause: postProcessTemplatesAndYamlFunctions() overwrote ComponentAuthSection
with component-level auth from ComponentSection["auth"], which didn't include
the global auth config from profiles.
Solution:
1. Pass atmosConfig to ProcessComponentConfig()
2. When component has no auth section, call mergeGlobalAuthConfig()
3. Helper function merges atmosConfig.Auth into both componentAuthSection
AND componentSection["auth"] (prevents overwrite by postProcess)
4. Extracted to helper function to reduce cyclomatic complexity
Flow:
- ProcessComponentConfig calls mergeGlobalAuthConfig
- Writes merged config to ComponentSection["auth"] AND ComponentAuthSection
- postProcessTemplatesAndYamlFunctions copies ComponentSection["auth"] → ComponentAuthSection
- TerraformPreHook receives merged auth config ✅
Impact:
- Profiles with auth config work without explicit auth.yaml import
- --profile flag properly propagates auth config to terraform commands
- Backward compatible: explicit auth.yaml imports still work
Fixes: terraform commands failing with --profile when auth.yaml not imported
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* add tests
* add tests
* [autofix.ci] apply automated fixes
* [autofix.ci] apply automated fixes (attempt 2/3)
* [autofix.ci] apply automated fixes (attempt 3/3)
* add tests
* [autofix.ci] apply automated fixes
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude <[email protected]>
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <[email protected]>
![autofix-ci[bot]](https://avatars.githubusercontent.com/in/243519?v=4&size=40){kind=avatar}
0 commit comments