Skip to content

fix(auth): normalize ATMOS_IDENTITY=false (issue #1931)#1935

Merged
aknysh merged 3 commits intomainfrom
osterman/fix-vendor-path-1931
Jan 10, 2026
Merged

fix(auth): normalize ATMOS_IDENTITY=false (issue #1931)#1935
aknysh merged 3 commits intomainfrom
osterman/fix-vendor-path-1931

Conversation

@osterman
Copy link
Copy Markdown
Member

@osterman osterman commented Jan 5, 2026

what

  • Fix ATMOS_IDENTITY=false not being recognized in the legacy ProcessCommandLineArgs() code path
  • Create shared NormalizeIdentityValue() function in pkg/config/identity.go to centralize normalization logic
  • Add comprehensive tests for ATMOS_IDENTITY=false, 0, no, off in cli_utils_test.go
  • Refactor three duplicate normalizeIdentityValue() functions to use shared implementation

why

PR #1900 fixed ATMOS_IDENTITY=false normalization for the flag parsing path (pkg/flags/global_registry.go), but missed a separate code path in internal/exec/cli_utils.go that reads ATMOS_IDENTITY directly via os.Getenv().

Two independent code paths exist:

  1. Flag path (pkg/flags/global_registry.go): Used by Cobra commands with proper global flag inheritance - ✅ Fixed in PR Fix ATMOS_IDENTITY=false not working (issue #1898) #1900
  2. Legacy path (internal/exec/cli_utils.go:ProcessCommandLineArgs()): Used by terraform commands - ❌ Not fixed until this PR

The legacy path was added in PR #1720 (Oct 2025) without normalization. When PR #1900 fixed the flag path (Dec 2025), this code path was missed, causing the regression reported in issue #1931.

Verified with user's reproduction case:

# From issue #1931's auth_bug.tar.gz
ATMOS_IDENTITY=false atmos terraform plan myapp -s dev
# Before: "Error: identity not found" (treats "false" as identity name)
# After: Terraform plan runs successfully with auth disabled

Note: This class of problems (duplicate code paths for auth handling) should be eliminated by #1919, which refactors Atmos auth to use the command registry pattern. Once that lands, there will be a single, unified code path for identity/auth handling across all commands.

references

Closes #1931
Related: #1900 (partial fix), #1720 (introduced the unfixed code path), #1919 (architectural fix to prevent this class of bugs)

Summary by CodeRabbit

  • Refactor

    • Consolidated identity value normalization logic across the system. Identity values are now consistently processed to treat false-like representations (false, 0, no, off in various cases) uniformly, regardless of whether they originate from environment variables, configuration files, or command-line arguments.
  • Tests

    • Added test coverage for identity value normalization with comprehensive edge case handling.

✏️ Tip: You can customize this high-level summary in your review settings.

…issue #1931)

## Summary

Fix `ATMOS_IDENTITY=false` environment variable not working for disabling authentication. Previously, the literal string "false" was treated as an identity name, causing "identity not found" errors. Now boolean false representations ("false", "0", "no", "off") correctly disable authentication.

## Changes

- Create shared `NormalizeIdentityValue()` function in `pkg/config/identity.go`
- Fix bug in `internal/exec/cli_utils.go` to normalize env var before use
- Add comprehensive tests for `ATMOS_IDENTITY=false` in `cli_utils_test.go`
- Refactor duplicate normalization functions to use shared implementation

## Testing

- All unit tests pass
- Verified `ATMOS_IDENTITY=false`, `0`, `no`, `off` all disable authentication correctly
- No regressions in existing tests

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@osterman osterman requested a review from a team as a code owner January 5, 2026 22:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 5, 2026

📝 Walkthrough

Walkthrough

This PR centralizes identity value normalization logic. A new NormalizeIdentityValue function in the config package normalizes false-like representations ("false", "0", "no", "off" in any case) to a disabled sentinel. Multiple files replace inline normalization with delegation to this function, and environment variable processing now applies normalization.

Changes

Cohort / File(s) Summary
Config Package Normalization
pkg/config/identity.go, pkg/config/identity_test.go
Introduces NormalizeIdentityValue(value string) string to centralize identity normalization, mapping false-like values to disabled sentinel; adds comprehensive unit tests covering empty input, case-insensitive false variants, pass-through values, and edge cases.
Migration to Centralized Normalization
cmd/identity_flag.go, cmd/list/utils.go, pkg/flags/global_registry.go
Replaces inline normalization logic with delegation to cfg.NormalizeIdentityValue() in three files; removes redundant strings import; adds deprecation notices to local wrapper functions.
Environment Variable Processing
internal/exec/cli_utils.go
Updates ProcessCommandLineArgs to normalize ATMOS_IDENTITY environment variable via cfg.NormalizeIdentityValue() before assignment, ensuring false-like values are properly handled.
Test Coverage
internal/exec/cli_utils_test.go
Extends TestProcessCommandLineArgs_IdentityFromEnvironmentVariable with new test cases for false-negative environment values (false, FALSE, 0, no, off) expecting the disabled identity placeholder.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • aknysh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: fixing normalization of ATMOS_IDENTITY=false, which is the core issue addressed across multiple files.
Linked Issues check ✅ Passed All objectives from issue #1931 are met: ATMOS_IDENTITY false values are normalized, the legacy env-reading code path is fixed, tests cover false representations, and behavior is consistent across both paths.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing identity normalization. Refactoring to centralize logic and updates to both code paths stay within the bounds of resolving issue #1931.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch osterman/fix-vendor-path-1931

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 80c7937 and a4fbe0f.

📒 Files selected for processing (7)
  • cmd/identity_flag.go
  • cmd/list/utils.go
  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
  • pkg/config/identity.go
  • pkg/config/identity_test.go
  • pkg/flags/global_registry.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under cmd/ directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands

Files:

  • cmd/identity_flag.go
  • cmd/list/utils.go
**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using fmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: use gofmt and goimports to format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging

**/*.go: All comments must end with periods (enforced by godot linter) in Go code
Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (cfg, log, u, errUtils)
All errors MUST be wrapped using static errors defined in errors/errors.go - use errors.Join for combining errors, fmt.Errorf with %w for context, and errors.Is() for error checking
Never manually create mocks - use go.uber.org/mock/mockgen with //go:generate directives in Go code
Keep files small and focused - under 600 lines with one cmd/impl per file, co-locate tests, never use //revive:disable:file-length-limit
Use colors from pkg/ui/theme/colors.go for all UI theming in Go code
Code must be compatible with Linux, macOS, and Windows - use SDKs over binaries, use filepath.Join() instead of h...

Files:

  • cmd/identity_flag.go
  • internal/exec/cli_utils.go
  • pkg/config/identity_test.go
  • pkg/config/identity.go
  • cmd/list/utils.go
  • internal/exec/cli_utils_test.go
  • pkg/flags/global_registry.go
**/{pkg,internal,cmd}/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Add defer perf.Track(atmosConfig, "pkg.FuncName")() plus blank line to all public functions, using nil if no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions

Files:

  • cmd/identity_flag.go
  • internal/exec/cli_utils.go
  • pkg/config/identity_test.go
  • pkg/config/identity.go
  • cmd/list/utils.go
  • internal/exec/cli_utils_test.go
  • pkg/flags/global_registry.go
**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures

Prefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with go.uber.org/mock/mockgen, use table-driven tests, target >80% coverage

Files:

  • pkg/config/identity_test.go
  • internal/exec/cli_utils_test.go
🧠 Learnings (39)
📓 Common learnings
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.

Applied to files:

  • cmd/identity_flag.go
  • internal/exec/cli_utils.go
  • pkg/config/identity.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.

Applied to files:

  • cmd/identity_flag.go
  • cmd/list/utils.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration. 

Applied to files:

  • cmd/identity_flag.go
  • internal/exec/cli_utils.go
  • pkg/config/identity_test.go
  • pkg/config/identity.go
  • cmd/list/utils.go
  • internal/exec/cli_utils_test.go
  • pkg/flags/global_registry.go
📚 Learning: 2025-12-13T06:10:25.156Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: internal/exec/workflow_utils.go:0-0
Timestamp: 2025-12-13T06:10:25.156Z
Learning: Atmos workflows: In internal/exec/workflow_utils.go ExecuteWorkflow, non-identity steps intentionally use baseWorkflowEnv, which is constructed from the parent environment with PATH modifications for the toolchain. Avoid appending os.Environ() again; prefer documenting this behavior and testing that standard environment variables are preserved.

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
Learning: In Atmos cmd/root.go Execute(), after cfg.InitCliConfig, we must call both toolchainCmd.SetAtmosConfig(&atmosConfig) and toolchain.SetAtmosConfig(&atmosConfig) so the CLI wrapper and the toolchain package receive configuration; missing either can cause nil-pointer panics in toolchain path resolution.

Applied to files:

  • internal/exec/cli_utils.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Applies to **/*.go : Use `viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")` for environment variables - ATMOS_ prefix required in Go code

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.

Applied to files:

  • internal/exec/cli_utils.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.

Applied to files:

  • internal/exec/cli_utils.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.

Applied to files:

  • internal/exec/cli_utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures

Applied to files:

  • pkg/config/identity_test.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages

Applied to files:

  • pkg/config/identity_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go

Applied to files:

  • pkg/config/identity_test.go
  • internal/exec/cli_utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use Viper for managing configuration, environment variables, and flags in CLI commands

Applied to files:

  • cmd/list/utils.go
  • pkg/flags/global_registry.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in `errors/errors.go` - use `errors.Join` for combining errors, `fmt.Errorf` with `%w` for context, and `errors.Is()` for error checking

Applied to files:

  • cmd/list/utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory

Applied to files:

  • cmd/list/utils.go
  • pkg/flags/global_registry.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Commands MUST use `flags.NewStandardParser()` for command-specific flags and NEVER call `viper.BindEnv()` or `viper.BindPFlag()` directly

Applied to files:

  • cmd/list/utils.go
  • pkg/flags/global_registry.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions

Applied to files:

  • cmd/list/utils.go
  • pkg/flags/global_registry.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors

Applied to files:

  • cmd/list/utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging

Applied to files:

  • cmd/list/utils.go
📚 Learning: 2026-01-04T00:55:21.698Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.698Z
Learning: Applies to **/*.go : Code must be compatible with Linux, macOS, and Windows - use SDKs over binaries, use `filepath.Join()` instead of hardcoded path separators

Applied to files:

  • cmd/list/utils.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.

Applied to files:

  • internal/exec/cli_utils_test.go
📚 Learning: 2025-12-13T04:37:45.831Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:45.831Z
Learning: In cmd/toolchain/get.go, the get subcommand intentionally uses cobra.MaximumNArgs(1) so it works with zero args (list all tools) or one arg (a specific tool). Flags are defined via flags.NewStandardParser() with --all (bool) and --limit (int); no direct viper.BindEnv/BindPFlag calls are used.

Applied to files:

  • pkg/flags/global_registry.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.

Applied to files:

  • pkg/flags/global_registry.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.

Applied to files:

  • pkg/flags/global_registry.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults

Applied to files:

  • pkg/flags/global_registry.go
📚 Learning: 2025-09-29T15:47:10.908Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/terraform_cli_args_utils.go:64-73
Timestamp: 2025-09-29T15:47:10.908Z
Learning: In the Atmos codebase, viper.BindEnv is required for CLI commands in the cmd/ package, but internal utilities can use os.Getenv directly when parsing environment variables for business logic purposes. The requirement to use viper is specific to the CLI interface layer, not all environment variable access throughout the codebase.

Applied to files:

  • pkg/flags/global_registry.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.

Applied to files:

  • pkg/flags/global_registry.go
🧬 Code graph analysis (7)
cmd/identity_flag.go (1)
pkg/config/identity.go (1)
  • NormalizeIdentityValue (16-28)
internal/exec/cli_utils.go (1)
pkg/config/identity.go (1)
  • NormalizeIdentityValue (16-28)
pkg/config/identity_test.go (2)
pkg/config/const.go (1)
  • IdentityFlagDisabledValue (143-143)
pkg/config/identity.go (1)
  • NormalizeIdentityValue (16-28)
pkg/config/identity.go (1)
pkg/config/const.go (1)
  • IdentityFlagDisabledValue (143-143)
cmd/list/utils.go (1)
pkg/config/identity.go (1)
  • NormalizeIdentityValue (16-28)
internal/exec/cli_utils_test.go (1)
pkg/config/const.go (1)
  • IdentityFlagDisabledValue (143-143)
pkg/flags/global_registry.go (1)
pkg/config/identity.go (1)
  • NormalizeIdentityValue (16-28)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Acceptance Tests (windows)
  • GitHub Check: Acceptance Tests (macos)
  • GitHub Check: Acceptance Tests (linux)
  • GitHub Check: Summary
🔇 Additional comments (7)
pkg/config/identity.go (1)

1-28: LGTM! Clean centralized normalization logic.

The implementation is straightforward and solves the stated problem. The function correctly handles case-insensitive boolean false representations and provides clear documentation about its usage across CLI flags, environment variables, and config files.

pkg/config/identity_test.go (1)

1-125: Excellent test coverage with edge cases.

The table-driven tests comprehensively cover all normalization scenarios including case variations, sentinel values, real identity names, and important edge cases. This follows the coding guidelines and should provide strong protection against regressions.

internal/exec/cli_utils.go (1)

180-187: Key fix correctly applied.

This change addresses the core issue where ATMOS_IDENTITY=false wasn't being normalized in the legacy code path. The normalization is now applied consistently with the flag-based path, and the comment clearly explains the intent.

cmd/identity_flag.go (1)

61-68: Good refactoring with deprecation notice.

The delegation to cfg.NormalizeIdentityValue eliminates code duplication while maintaining backward compatibility. The deprecation notice guides future refactoring.

cmd/list/utils.go (1)

170-177: Consistent refactoring pattern applied.

The delegation to cfg.NormalizeIdentityValue matches the pattern used in cmd/identity_flag.go, eliminating the third duplicate implementation. The deprecation notice is appropriate.

internal/exec/cli_utils_test.go (1)

623-663: Excellent test coverage for false-like identity values.

The new test cases comprehensively verify that ATMOS_IDENTITY environment variable correctly handles false-like representations (false, FALSE, 0, no, off) and normalizes them to the disabled sentinel. The tests properly use t.Setenv for test isolation and follow the established table-driven pattern. This directly addresses the regression reported in issue #1931.

pkg/flags/global_registry.go (1)

130-134: Clean refactoring to centralized normalization.

The deprecation wrapper properly delegates to cfg.NormalizeIdentityValue() while maintaining backward compatibility. This eliminates code duplication and ensures consistent identity value normalization across all code paths. Well-executed refactor.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the size/m Medium size PR label Jan 5, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 5, 2026

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.04%. Comparing base (6ddae50) to head (3720bbb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1935   +/-   ##
=======================================
  Coverage   74.03%   74.04%           
=======================================
  Files         769      770    +1     
  Lines       69336    69326   -10     
=======================================
- Hits        51334    51332    -2     
+ Misses      14592    14585    -7     
+ Partials     3410     3409    -1     
Flag Coverage Δ
unittests 74.04% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/identity_flag.go 100.00% <100.00%> (ø)
cmd/list/utils.go 64.91% <100.00%> (-1.76%) ⬇️
internal/exec/cli_utils.go 72.50% <100.00%> (ø)
pkg/config/identity.go 100.00% <100.00%> (ø)
pkg/flags/global_registry.go 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@osterman osterman added the patch A minor, backward compatible change label Jan 6, 2026
@aknysh aknysh merged commit 4f7f6d8 into main Jan 10, 2026
56 checks passed
@aknysh aknysh deleted the osterman/fix-vendor-path-1931 branch January 10, 2026 15:51
@github-actions
Copy link
Copy Markdown

These changes were released in v1.204.0-rc.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch A minor, backward compatible change size/m Medium size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ATMOS_IDENTITY=false still not working with 1.203.0

2 participants