Add Atmos CLI performance profiling with multiple profile types #1534
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # cmd/root.go
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
aknysh
added
the
no-release
aknysh
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughAdds a new Go pprof-based profiler package and integrates it into the CLI lifecycle, adding config schema, env/flag bindings, CLI flags, help/output updates, docs, tests, example version bumps, dependency updates, and a noop logger method. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as Atmos CLI
participant Cfg as Config Loader
participant Prof as Profiler (pkg/profiler)
participant Cmd as Command Execution
User->>CLI: atmos <cmd> [global flags/env]
CLI->>Cfg: Load config + env
Cfg-->>CLI: Config (includes profiler.*)
CLI->>CLI: buildProfilerConfig(env → flags → defaults)
alt Help command
CLI->>Cmd: Show help (skip profiler)
else Executable command
CLI->>Prof: New(config)
CLI->>Prof: Start()
alt File-based profiling
Note over Prof: Collect specified profile into file\n(profile-type)
else Server-based profiling
Note over Prof: Start pprof HTTP server (host:port)
end
CLI->>Cmd: Run command
Cmd-->>CLI: Exit
CLI->>Prof: Stop()
end
CLI-->>User: Output / exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
website/docs/cli/configuration/configuration.mdx (1)
832-903: Document the new ATMOS_PROFILER_ env vars.*Loving the profiler write-up. One thing: the environment variable mapping table still omits the freshly wired
ATMOS_PROFILER_ENABLED,ATMOS_PROFILER_HOST,ATMOS_PROFILER_PORT,ATMOS_PROFILE_FILE, andATMOS_PROFILE_TYPE. Without them, the precedence story you outlined (flags → env → config) isn’t discoverable from here. Let’s extend the table to keep the docs accurate.| ATMOS_LOGS_LEVEL | logs.level | Logs level. Supported log levels are `Trace`, `Debug`, `Info`, `Warning`, `Off`. If the log level is set to `Off`, Atmos will not log any messages (note that this does not prevent other tools like Terraform from logging) | +| ATMOS_PROFILER_ENABLED | profiler.enabled | Enable/disable the pprof profiling server | +| ATMOS_PROFILER_HOST | profiler.host | Host/interface to bind the profiling server to (default `localhost`) | +| ATMOS_PROFILER_PORT | profiler.port | Port to expose the profiling server on (default `6060`) | +| ATMOS_PROFILE_FILE | profiler.file | File path for on-demand profile dumps created via `--profile-file` | +| ATMOS_PROFILE_TYPE | profiler.type | Profile type to capture when using `--profile-file` (cpu, heap, allocs, goroutine, block, mutex, threadcreate, trace) | | ATMOS_SETTINGS_LIST_MERGE_STRATEGY | settings.list_merge_strategy | Specifies how lists are merged in Atmos stack manifests. The following strategies are supported: `replace`, `append`, `merge` |cmd/root.go (1)
51-79: Start profiler before executing the commandAppreciate the new profiler hooks. Right now
setupProfileris only triggered afterRootCmd.ExecuteC()completes, so the target command has already finished by the time profiling starts (file captures stay empty and the HTTP server scarcely runs). Move setup/teardown into the command lifecycle—for example, callsetupProfilerfromPersistentPreRunand stop it in a matchingPersistentPostRun—so the profiler is active while the command actually runs.@@ - PersistentPreRun: func(cmd *cobra.Command, args []string) { + PersistentPreRun: func(cmd *cobra.Command, args []string) { // Determine if the command is a help command or if the help flag is set isHelpCommand := cmd.Name() == "help" helpFlag := cmd.Flags().Changed("help") @@ } configAndStacksInfo := schema.ConfigAndStacksInfo{} // Only validate the config, don't store it yet since commands may need to add more info _, err := cfg.InitCliConfig(configAndStacksInfo, false) @@ errUtils.CheckErrorPrintAndExit(err, "", "") } } + + if setupErr := setupProfiler(cmd, &atmosConfig); setupErr != nil { + log.Error("Failed to setup profiler", "error", setupErr) + } }, + PersistentPostRun: func(cmd *cobra.Command, args []string) { + if profilerServer != nil { + if stopErr := profilerServer.Stop(); stopErr != nil { + log.Error("Failed to stop profiler", "error", stopErr) + } + } + }, @@ - cmd, err := RootCmd.ExecuteC() - - // Setup profiler after command parsing but before telemetry - if cmd != nil { - if setupErr := setupProfiler(cmd, &atmosConfig); setupErr != nil { - log.Error("Failed to setup profiler", "error", setupErr) - } else if profilerServer != nil { - // Ensure profiler stops when the command finishes - defer func() { - if stopErr := profilerServer.Stop(); stopErr != nil { - log.Error("Failed to stop profiler", "error", stopErr) - } - }() - } - } + cmd, err := RootCmd.ExecuteC()
📜 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.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
cmd/root.go(5 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(11 hunks)pkg/config/load.go(2 hunks)pkg/profiler/profiler.go(1 hunks)pkg/profiler/profiler_test.go(1 hunks)pkg/schema/schema.go(2 hunks)pkg/store/artifactory_store_noop_logger.go(1 hunks)tests/fixtures/scenarios/stack-templates/atmos.yaml(1 hunks)website/docs/cli/configuration/configuration.mdx(2 hunks)website/docs/cli/global-flags.mdx(5 hunks)website/docs/integrations/atlantis.mdx(1 hunks)website/docs/troubleshoot/debugging.md(1 hunks)website/docs/troubleshoot/profiling.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Target >80% coverage for packages, focusing on pkg/ and internal/exec/.
Files:
pkg/schema/schema.gopkg/config/load.gopkg/profiler/profiler_test.gopkg/profiler/profiler.gopkg/store/artifactory_store_noop_logger.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods.
Wrap all returned errors with static errors from errors/errors.go; never return dynamic errors directly.
Use fmt.Errorf with %w to wrap the static error first, then add context details.
Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
All new configurations must support Go templating using the shared FuncMap(); test template rendering with various contexts.
Prefer SDKs over shelling out to binaries; use standard library for cross-platform behavior (filepath.Join, os.PathSeparator, runtime.GOOS).
For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Files:
pkg/schema/schema.gocmd/root.gopkg/config/load.gopkg/profiler/profiler_test.gopkg/profiler/profiler.gopkg/store/artifactory_store_noop_logger.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/schema/schema.gocmd/root.gopkg/config/load.gopkg/profiler/profiler.gopkg/store/artifactory_store_noop_logger.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
cmd/**/*.go: One Cobra command per file in cmd/; follow embedded markdown example pattern with //go:embed and utils.PrintfMarkdown.
Use markdown formatting in Cobra Short/Long descriptions for commands.
Distinguish UI output (stderr) from structured logging; never use logging for UI elements.
Most text UI must go to stderr; only data/results go to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
Files:
cmd/root.go
cmd/root.go
📄 CodeRabbit inference engine (CLAUDE.md)
New commands automatically receive telemetry via RootCmd.ExecuteC() in cmd/root.go: ensure they’re added to RootCmd.
Files:
cmd/root.go
tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place integration tests and shared test utilities under tests/ with fixtures in tests/test-cases/.
Files:
tests/fixtures/scenarios/stack-templates/atmos.yaml
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/global-flags.mdxwebsite/docs/integrations/atlantis.mdxwebsite/docs/troubleshoot/profiling.mdxwebsite/docs/troubleshoot/debugging.mdwebsite/docs/cli/configuration/configuration.mdx
pkg/config/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use Viper for configuration management (SetConfigName, AddConfigPath, AutomaticEnv, SetEnvPrefix).
Files:
pkg/config/load.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests where applicable.
Always use t.Skipf() with a reason; do not use t.Skip() or t.Skipf() without explanation.
TestMain must call os.Exit(m.Run()) to propagate test exit code for CLI tests.
Files:
pkg/profiler/profiler_test.go
{pkg,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Co-locate unit/command tests with implementation files using _test.go naming.
Files:
pkg/profiler/profiler_test.go
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
pkg/store/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/store/**/*.go: Follow multi-provider interface pattern with a provider registry; implement providers per interface and register them.
Generate mocks for interfaces with //go:generate mockgen; keep mocks alongside interfaces.
Files:
pkg/store/artifactory_store_noop_logger.go
🧠 Learnings (34)
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
pkg/schema/schema.gocmd/root.gopkg/config/load.gogo.mod
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
cmd/root.gowebsite/docs/cli/global-flags.mdx
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/root.go : New commands automatically receive telemetry via RootCmd.ExecuteC() in cmd/root.go: ensure they’re added to RootCmd.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands
Applied to files:
cmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/markdown/atmos_*_*_usage.md : Example files must be named atmos_<command>_<subcommand>_usage.md and live under cmd/markdown/.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/global-flags.mdxwebsite/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdxpkg/config/load.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
Applied to files:
website/docs/cli/global-flags.mdxpkg/config/load.go
📚 Learning: 2025-09-23T04:43:31.857Z
Learnt from: osterman
PR: cloudposse/atmos#1430
File: pkg/config/config.go:98-107
Timestamp: 2025-09-23T04:43:31.857Z
Learning: In the Atmos codebase, NO_PAGER environment variable handling is intentionally kept as direct os.Getenv() access in pkg/config/config.go rather than using Viper binding, because adding no_pager to the config file would be confusing for users. This is an acknowledged exception to the normal Viper binding pattern for environment variables.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-11-12T03:15:15.627Z
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T03:15:15.627Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Applied to files:
website/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 Learning: 2024-11-23T00:13:22.004Z
Learnt from: osterman
PR: cloudposse/atmos#801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
Applied to files:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#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:
pkg/config/load.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to pkg/config/**/*.go : Use Viper for configuration management (SetConfigName, AddConfigPath, AutomaticEnv, SetEnvPrefix).
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for managing configuration, environment variables, and flags in the CLI
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*_test.go : Use table-driven tests for unit tests where applicable.
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Use table-driven tests for multiple scenarios
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Keep dependencies up to date
Applied to files:
go.mod
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Manage dependencies with Go modules
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.mod
🧬 Code graph analysis (3)
pkg/schema/schema.go (1)
pkg/profiler/profiler.go (1)
Config(42-48)
cmd/root.go (2)
pkg/profiler/profiler.go (5)
Server(61-70)DefaultConfig(51-58)ProfileType(20-20)ParseProfileType(326-332)New(73-80)pkg/schema/schema.go (1)
AtmosConfiguration(27-63)
pkg/profiler/profiler_test.go (1)
pkg/profiler/profiler.go (15)
DefaultConfig(51-58)ProfileType(20-20)ProfileTypeCPU(24-24)Config(42-48)New(73-80)ProfileTypeHeap(26-26)ProfileTypeAllocs(28-28)ProfileTypeGoroutine(30-30)ProfileTypeBlock(32-32)ProfileTypeMutex(34-34)ProfileTypeThreadCreate(36-36)ProfileTypeTrace(38-38)IsValidProfileType(301-309)ParseProfileType(326-332)GetSupportedProfileTypes(312-323)
🪛 GitHub Actions: Dependency Review
go.mod
[error] 1-1: Dependency review detected incompatible licenses. go-redis/[email protected] has BSD-2-Clause license which is incompatible with project licenses.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
aknysh
changed the title
aknysh
changed the title
Member
Author
|
@coderabbitai full review and approval |
Contributor
|
@aknysh I'll conduct a full review of this comprehensive profiling feature implementation. This PR introduces significant new capabilities with file-based and server-based profiling, environment variable configuration, and CLI integration across 8 different profile types. Let me review all the changes thoroughly to ensure the implementation is solid and ready for approval. ✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
cmd/root.go (3)
88-93: Nil out profiler after stopping.Prevents accidental reuse and eases GC.
if profilerServer != nil { if stopErr := profilerServer.Stop(); stopErr != nil { log.Error("Failed to stop profiler", "error", stopErr) } + profilerServer = nil }
358-370: Unify quoting in error message for profile-type.Use %q for consistency with other messages and Go conventions.
- return fmt.Errorf("%w: invalid profile type '%s': %v", errUtils.ErrParseFlag, profileType, parseErr) + return fmt.Errorf("%w: invalid profile type %q: %v", errUtils.ErrParseFlag, profileType, parseErr)
459-466: Flag defaults/readability LGTM; minor validation idea.Defaults align with help snapshots. Optionally validate profiler-port range (1–65535) during buildProfilerConfig to fail fast on bad input.
📜 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.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (35)
cmd/root.go(14 hunks)errors/errors.go(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(11 hunks)pkg/config/load.go(2 hunks)pkg/profiler/profiler.go(1 hunks)pkg/profiler/profiler_test.go(1 hunks)pkg/schema/schema.go(2 hunks)pkg/store/artifactory_store_noop_logger.go(1 hunks)tests/fixtures/scenarios/stack-templates/atmos.yaml(1 hunks)tests/snapshots/TestCLICommands_atmos_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_about_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden(1 hunks)website/docs/cli/configuration/configuration.mdx(3 hunks)website/docs/cli/global-flags.mdx(5 hunks)website/docs/integrations/atlantis.mdx(1 hunks)website/docs/troubleshoot/debugging.md(1 hunks)website/docs/troubleshoot/profiling.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (61)
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.goldentests/snapshots/TestCLICommands_atmos_about_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.goldenwebsite/docs/cli/global-flags.mdxtests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.goldentests/snapshots/TestCLICommands_atmos_about_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.goldenwebsite/docs/cli/global-flags.mdxtests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.goldentests/snapshots/TestCLICommands_atmos_about_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.goldenwebsite/docs/cli/global-flags.mdxtests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
PR: cloudposse/atmos#1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
tests/snapshots/TestCLICommands_atmos_about_--help.stdout.goldenwebsite/docs/cli/global-flags.mdx
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/configuration/configuration.mdxwebsite/docs/cli/global-flags.mdxwebsite/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
website/docs/cli/configuration/configuration.mdx
📚 Learning: 2025-01-19T22:30:27.600Z
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!env` is used to retrieve environment variables and assign them to sections in stack manifests. It supports both simple types (string, number, boolean) and complex types (JSON-encoded lists, maps, objects).
Applied to files:
website/docs/cli/configuration/configuration.mdx
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
Applied to files:
pkg/config/load.gowebsite/docs/cli/global-flags.mdx
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
PR: cloudposse/atmos#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:
pkg/config/load.gowebsite/docs/cli/global-flags.mdxcmd/root.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
pkg/config/load.gogo.modpkg/schema/schema.gocmd/root.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
PR: cloudposse/atmos#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:
pkg/config/load.gocmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to pkg/config/**/*.go : Use Viper for configuration management (SetConfigName, AddConfigPath, AutomaticEnv, SetEnvPrefix).
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Applied to files:
pkg/config/load.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for managing configuration, environment variables, and flags in the CLI
Applied to files:
pkg/config/load.gocmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : Use fmt.Errorf with %w to wrap the static error first, then add context details.
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-06-07T19:28:21.289Z
Learnt from: samtholiya
PR: cloudposse/atmos#1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#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:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : All comments must end with periods.
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Provide clear error messages to users and troubleshooting hints where appropriate
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
PR: cloudposse/atmos#1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
pkg/profiler/profiler.gocmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Keep dependencies up to date
Applied to files:
go.mod
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to go.{mod,sum} : Manage dependencies with Go modules
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.mod
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.goldenwebsite/docs/cli/global-flags.mdx
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags
Applied to files:
tests/snapshots/TestCLICommands_atmos_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/markdown/atmos_*_*_usage.md : Example files must be named atmos_<command>_<subcommand>_usage.md and live under cmd/markdown/.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdxcmd/root.go
📚 Learning: 2025-09-23T04:43:31.857Z
Learnt from: osterman
PR: cloudposse/atmos#1430
File: pkg/config/config.go:98-107
Timestamp: 2025-09-23T04:43:31.857Z
Learning: In the Atmos codebase, NO_PAGER environment variable handling is intentionally kept as direct os.Getenv() access in pkg/config/config.go rather than using Viper binding, because adding no_pager to the config file would be confusing for users. This is an acknowledged exception to the normal Viper binding pattern for environment variables.
Applied to files:
website/docs/cli/global-flags.mdxcmd/root.go
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2024-11-12T03:15:15.627Z
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T03:15:15.627Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Applied to files:
website/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 Learning: 2024-11-23T00:13:22.004Z
Learnt from: osterman
PR: cloudposse/atmos#801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
Applied to files:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
cmd/root.go
📚 Learning: 2024-10-19T17:42:29.152Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#729
File: internal/exec/terraform.go:258-264
Timestamp: 2024-10-19T17:42:29.152Z
Learning: Prefer not to declare variables within 'if' statements in Go code.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Keep separation of concerns between CLI interface (cmd) and business logic
Applied to files:
cmd/root.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/root.go
📚 Learning: 2024-11-07T20:16:15.381Z
Learnt from: pkbhowmick
PR: cloudposse/atmos#766
File: cmd/cmd_utils.go:177-178
Timestamp: 2024-11-07T20:16:15.381Z
Learning: In 'cmd/cmd_utils.go', using `os.Exit(1)` directly is acceptable in this context, and wrapping it with deferred cleanup is considered unnecessary.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*.go : Wrap errors with context using fmt.Errorf("context: %w", err)
Applied to files:
cmd/root.go
📚 Learning: 2025-02-03T05:57:18.407Z
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Applied to files:
cmd/root.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
cmd/root.go
📚 Learning: 2024-12-05T22:33:54.807Z
Learnt from: aknysh
PR: cloudposse/atmos#820
File: cmd/list_stacks.go:55-56
Timestamp: 2024-12-05T22:33:54.807Z
Learning: In the atmos project, the `u.LogErrorAndExit` function logs the error and exits the command execution appropriately within flag completion functions.
Applied to files:
cmd/root.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*.go : Document complex logic with inline comments
Applied to files:
cmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands
Applied to files:
cmd/root.go
📚 Learning: 2024-12-11T18:46:02.483Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/terraform.go:39-39
Timestamp: 2024-12-11T18:46:02.483Z
Learning: `cliConfig` is initialized in `cmd/root.go` and can be used across the `cmd` package.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/root.go : New commands automatically receive telemetry via RootCmd.ExecuteC() in cmd/root.go: ensure they’re added to RootCmd.
Applied to files:
cmd/root.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
PR: cloudposse/atmos#984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Applied to files:
cmd/root.go
📚 Learning: 2025-01-18T15:16:54.196Z
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/root.go:91-92
Timestamp: 2025-01-18T15:16:54.196Z
Learning: The `showUsageAndExit` function in `cmd/root.go` is designed to display usage information and terminate the program, making error handling unnecessary as the function never returns.
Applied to files:
cmd/root.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/new_command.go : When adding a new CLI command, create cmd/new_command.go with Cobra command, embed examples, register markdown help, and place business logic in pkg/ or internal/exec/.
Applied to files:
cmd/root.go
📚 Learning: 2024-12-12T17:13:53.409Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#848
File: pkg/utils/doc_utils.go:19-22
Timestamp: 2024-12-12T17:13:53.409Z
Learning: In `pkg/utils/doc_utils.go`, the `DisplayDocs` function uses the `PAGER` environment variable, which is intentionally user-configurable to allow users to specify custom pager commands that fit their workflow; adding validation to restrict it is not desired.
Applied to files:
cmd/root.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
PR: cloudposse/atmos#808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (13)
pkg/profiler/profiler.go (2)
189-216: Still need deterministic startup failure detection.Nice improvement with the startup check, but once the 100 ms timer fires the
selectcan choose that branch even iferrChanalready holds a bind failure, soStart()may still return nil while the server never came up. Let’s bind synchronously withnet.Listenand pass that listener toServeso we always surface bind errors before returning.- p.server = &http.Server{ - Addr: addr, - Handler: http.DefaultServeMux, - ReadHeaderTimeout: DefaultReadHeaderTimeout, // #nosec G112 - Prevent Slowloris attacks - } - - // Channel to receive startup errors - errChan := make(chan error, 1) - - go func() { - if err := p.server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { - log.Error("Profiler server error", "error", err) - errChan <- err - } - close(errChan) - }() - - // Give the server a moment to start and check for immediate errors - select { - case err := <-errChan: - if err != nil { - p.server = nil - return fmt.Errorf("failed to start profiler server: %w", err) - } - case <-time.After(100 * time.Millisecond): - // Server started successfully (no immediate error) - } + ln, err := net.Listen("tcp", addr) + if err != nil { + return fmt.Errorf("failed to bind profiler listener %q: %w", addr, err) + } + + p.server = &http.Server{ + Addr: addr, + Handler: http.DefaultServeMux, + ReadHeaderTimeout: DefaultReadHeaderTimeout, // #nosec G112 - Prevent Slowloris attacks + } + + go func() { + if serveErr := p.server.Serve(ln); serveErr != nil && !errors.Is(serveErr, http.ErrServerClosed) { + log.Error("Profiler server error", "error", serveErr) + } + }()
135-138: Fix the panic from double %w usage.Thanks for wiring the file-based paths.
fmt.Errorf("%w: %s: %w"still hits the fmt double-wrap panic, so anyos.Createfailure will crash Atmos instead of returningErrCreateProfileFile. Please keep the sentinel%wand render the inner error with%v.- return fmt.Errorf("%w: %s: %w", ErrCreateProfileFile, p.config.File, err) + return fmt.Errorf("%w: %s: %v", ErrCreateProfileFile, p.config.File, err)Based on learnings
pkg/store/artifactory_store_noop_logger.go (1)
72-76: Interface coverage restoredMatches the expanded
al.Logsurface; noop behavior stays intact.website/docs/cli/configuration/configuration.mdx (1)
579-865: Docs aligned with profiler configContent mirrors the new defaults, env bindings, and flag behavior precisely.
tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden (1)
69-84: Snapshot matches new globalsHelp output now surfaces the profiling knobs exactly as wired in root.
tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden (1)
57-72: Snapshot updated correctlyAtlantis help mirrors the shared global profiling flags; no drift.
tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden (1)
186-201: Terraform help in syncThe new profiling flags appear with the right descriptions and defaults.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden (1)
97-113: Profiling flags surfaced globally look good.Flags and help text align with the PR intent; defaults read clearly in help.
Please confirm snapshots for other commands also reflect these five flags with the same defaults, and that profiler.DefaultProfilerPort matches the “(default 6060)” shown in help.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden (1)
85-101: Consistent profiler flags across commands.Good consistency with the atlantis snapshot; keeps UX predictable.
Double‑check docs list exactly these options for --profile-type (cpu, heap, allocs, goroutine, block, mutex, threadcreate, trace) and match the default cpu shown here.
cmd/root.go (4)
200-220: Skip‑when‑disabled logic and wrapped error are solid.Conditional start, sentinel wrap with ErrProfilerStart, and %v for the inner error look correct.
222-238: Correct precedence: config/defaults → env → CLI.buildProfilerConfig applies env before CLI; matches documented precedence.
240-268: Partial defaulting is well handled.Independent field defaults avoid “host:0” issues while preserving user input.
283-289: ProfileType casting — verify underlying type.string(atmosConfig.Profiler.ProfileType) assumes the field is a string or string alias. If it’s already string, the cast is redundant; if not, confirm the alias has string underlying type.
Would you like me to scan schema.AtmosConfiguration.Profiler.ProfileType usage to confirm?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
pkg/profiler/profiler_test.go (5)
36-43: Avoid magic number for default port; use the constant.Compare against DefaultProfilerPort to keep the test resilient if the default changes.
- if config.Port != 6060 { - t.Errorf("Expected default port 6060, got %d", config.Port) - } + if config.Port != DefaultProfilerPort { + t.Errorf("Expected default port %d, got %d", DefaultProfilerPort, config.Port) + }As per coding guidelines.
70-103: Harden server readiness: replace fixed sleep with a timed poll and HTTP timeout.Reduces flakes and avoids hanging GETs.
@@ func TestServerStartStop(t *testing.T) { @@ profiler := New(config) // Test start err := profiler.Start() if err != nil { t.Fatalf("Failed to start profiler: %v", err) } + // Ensure we always stop even if the test fails later. + t.Cleanup(func() { _ = profiler.Stop() }) if !profiler.IsRunning() { t.Error("Profiler should be running after Start()") } - // Give the server a moment to fully start - time.Sleep(200 * time.Millisecond) - - // Test that the server is actually listening - resp, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/", port)) - if err != nil { - t.Fatalf("Failed to connect to profiler server: %v", err) - } - resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - t.Errorf("Expected status 200, got %d", resp.StatusCode) - } + // Wait for the profiler endpoint to become ready with a timeout. + url := fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/", port) + waitForHTTP200(t, url, 2*time.Second) @@ // Test stop err = profiler.Stop() if err != nil { t.Fatalf("Failed to stop profiler: %v", err) } @@ } + +// waitForHTTP200 polls a URL until it returns 200 OK or the timeout elapses. +func waitForHTTP200(t *testing.T, url string, timeout time.Duration) { + t.Helper() + deadline := time.Now().Add(timeout) + client := &http.Client{Timeout: 200 * time.Millisecond} + var lastErr error + for time.Now().Before(deadline) { + resp, err := client.Get(url) + if err == nil { + resp.Body.Close() + if resp.StatusCode == http.StatusOK { + return + } + } else { + lastErr = err + } + time.Sleep(50 * time.Millisecond) + } + t.Fatalf("profiler endpoint %s not ready within %s (last error: %v)", url, timeout, lastErr) +}As per coding guidelines.
294-310: Assert error wrapping with ErrUnsupportedProfileType.Verifies that ParseProfileType wraps the static sentinel error, matching our error-handling contract.
@@ -import ( +import ( + "errors" "encoding/json" "fmt" "net" "net/http" "os" @@ - if test.expectError && err == nil { - t.Errorf("ParseProfileType(%s) expected error, but got none", test.input) - continue - } + if test.expectError && err == nil { + t.Errorf("ParseProfileType(%s) expected error, but got none", test.input) + continue + } + if test.expectError && !errors.Is(err, ErrUnsupportedProfileType) { + t.Errorf("Expected ErrUnsupportedProfileType, got: %v", err) + continue + }As per coding guidelines.
773-787: Use a read-only-but-traversable directory mode on Unix.0o444 makes the dir non-traversable; 0o555 better models “read-only dir” while still allowing traversal that triggers a predictable EACCES on file creation.
- if err := os.Mkdir(readOnlyDir, 0o444); err != nil { + if err := os.Mkdir(readOnlyDir, 0o555); err != nil { t.Fatalf("Failed to create read-only dir: %v", err) }As per coding guidelines.
1-16: Minor: comments should end with periods.Several inline comments across the file lack trailing periods. Touch-up keeps us aligned with the repo’s comment style.
As per coding guidelines.
📜 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.
📒 Files selected for processing (3)
cmd/root.go(14 hunks)pkg/profiler/profiler_test.go(1 hunks)website/docs/cli/global-flags.mdx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/root.go
🧰 Additional context used
📓 Path-based instructions (5)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/global-flags.mdx
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Target >80% coverage for packages, focusing on pkg/ and internal/exec/.
Files:
pkg/profiler/profiler_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests where applicable.
Always use t.Skipf() with a reason; do not use t.Skip() or t.Skipf() without explanation.
TestMain must call os.Exit(m.Run()) to propagate test exit code for CLI tests.
Files:
pkg/profiler/profiler_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods.
Wrap all returned errors with static errors from errors/errors.go; never return dynamic errors directly.
Use fmt.Errorf with %w to wrap the static error first, then add context details.
Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
All new configurations must support Go templating using the shared FuncMap(); test template rendering with various contexts.
Prefer SDKs over shelling out to binaries; use standard library for cross-platform behavior (filepath.Join, os.PathSeparator, runtime.GOOS).
For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Files:
pkg/profiler/profiler_test.go
{pkg,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Co-locate unit/command tests with implementation files using _test.go naming.
Files:
pkg/profiler/profiler_test.go
🧠 Learnings (18)
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to website/** : Ensure consistency between CLI help text and website documentation
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to cmd/markdown/atmos_*_*_usage.md : Example files must be named atmos_<command>_<subcommand>_usage.md and live under cmd/markdown/.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
PR: cloudposse/atmos#1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#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:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-23T04:43:31.857Z
Learnt from: osterman
PR: cloudposse/atmos#1430
File: pkg/config/config.go:98-107
Timestamp: 2025-09-23T04:43:31.857Z
Learning: In the Atmos codebase, NO_PAGER environment variable handling is intentionally kept as direct os.Getenv() access in pkg/config/config.go rather than using Viper binding, because adding no_pager to the config file would be confusing for users. This is an acknowledged exception to the normal Viper binding pattern for environment variables.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
website/docs/cli/global-flags.mdx
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Use table-driven tests for multiple scenarios
Applied to files:
pkg/profiler/profiler_test.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*_test.go : Use table-driven tests for unit tests where applicable.
Applied to files:
pkg/profiler/profiler_test.go
🧬 Code graph analysis (1)
pkg/profiler/profiler_test.go (1)
pkg/profiler/profiler.go (15)
DefaultConfig(69-76)ProfileType(33-33)ProfileTypeCPU(42-42)Config(60-66)New(91-98)ProfileTypeHeap(44-44)ProfileTypeAllocs(46-46)ProfileTypeGoroutine(48-48)ProfileTypeBlock(50-50)ProfileTypeMutex(52-52)ProfileTypeThreadCreate(54-54)ProfileTypeTrace(56-56)IsValidProfileType(352-360)ParseProfileType(377-384)GetSupportedProfileTypes(363-374)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: website-deploy-preview
- GitHub Check: Run pre-commit hooks
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (2)
website/docs/cli/global-flags.mdx (1)
45-50: Default level now alignedDoc default matches the CLI behavior (
Info) again—thanks for closing that gap.pkg/profiler/profiler_test.go (1)
18-28: Good fix: ephemeral port helper removes flakiness.Allocating an ephemeral TCP port via 127.0.0.1:0 is solid and resolves the earlier flaky tests from fixed ports.
osterman
approved these changes
Sep 27, 2025
|
These changes were released in v1.193.0-test.3. |
osterman
pushed a commit
that referenced
this pull request
Sep 29, 2025
* updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * [autofix.ci] apply automated fixes * updates * updates * updates * updates * updates * updates --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This was referenced Sep 29, 2025
aknysh
added a commit
that referenced
this pull request
Oct 2, 2025
…ility (#1504) * fix: handle triple-slash vendor URIs after go-getter v1.7.9 update The go-getter update from v1.7.8 to v1.7.9 (introduced in Atmos v1.189.0) broke the documented triple-slash pattern (///) for vendoring from Git repository roots. This pattern was widely used and documented in Atmos examples. Changes: - Add normalizeVendorURI() to convert /// patterns to go-getter v1.7.9 format - Convert "repo.git///?ref=v1.0" to "repo.git?ref=v1.0" (empty subdir) - Convert "repo.git///path?ref=v1.0" to "repo.git//path?ref=v1.0" (with subdir) - Add comprehensive test cases for the vendor pull issue Root Cause: go-getter v1.7.9 included security fixes for CVE-2025-8959 that changed subdirectory path handling, making the triple-slash pattern no longer functional for downloading Git repository contents. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * refactor: rename vendor triple-slash test files to be more descriptive - Renamed test file from vendor_issue_dev3639_test.go to vendor_triple_slash_test.go - Renamed test scenario directory from vendor-pull-issue-dev-3639 to vendor-triple-slash - Updated test function name and comments to describe what is being tested (triple-slash pattern) - Test files should be named based on what they test, not issue numbers * docs: improve triple-slash pattern documentation - Clarified that // is a delimiter and / after it represents the subdirectory path - Explained that /// means empty subdirectory (root of repository) - Added reference to CVE-2025-8959 which caused the breaking change in go-getter v1.7.9 - Updated inline comments to be more precise about the pattern's meaning * test: add unit tests for normalizeVendorURI and fix missing test function - Added comprehensive unit tests for normalizeVendorURI function - Fixed normalizeVendorURI to be more precise about .git/// patterns - Preserved file:/// URIs unchanged (valid file scheme) - Added missing verifyFileExists function in terraform_clean_test.go - Addressed linter issues (nestif complexity, magic numbers) * docs: clarify triple-slash pattern as root of repository Updated documentation to describe the triple-slash pattern (///) as indicating the root of the repository rather than an "empty subdirectory", which is more accurate and intuitive for understanding the go-getter URL syntax. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix: normalize vendor URIs to handle triple-slash pattern correctly - Created unified normalizeVendorURI function to handle all URI normalization in one place - Converts triple-slash (///) to double-slash-dot (//.) for repository root - Adds //. to Git URLs without subdirectory delimiter - Extracted URI pattern detection logic into separate helper functions for better maintainability - Deprecated adjustSubdir in CustomGitDetector as normalization now happens earlier in pipeline - Fixes vendor pull failures with triple-slash patterns after go-getter v1.7.9 update This ensures backward compatibility with existing vendor configurations while properly handling the go-getter v1.7.9+ requirements for subdirectory paths. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix: address linting comment formatting * fix: improve vendor URI normalization for triple-slash patterns - Handles all URI normalization in one place - Properly converts /// to //. for repository root - Adds //. to Git URLs without subdirectory - Works with go-getter v1.7.9+ * test: address CodeRabbit feedback and improve test coverage - Remove duplicate test helper function in terraform_clean_test.go - Replace helper calls with inline file existence checks - Add comprehensive test cases for HTTPS and SCP-style Git URLs - Add test coverage for git protocol with triple-slash patterns - Improve test coverage for URI normalization edge cases These changes address the review feedback while maintaining existing functionality and test coverage. * style: add missing period to comment per linting requirements Per godot linter requirements, all comments must end with periods. This is enforced by golangci-lint in the project. * test: update golden snapshots for vendor pull tests with new debug logs Updated golden snapshots to include new debug log messages that show Git URL normalization with //. pattern. These logs help trace the URL transformation process during vendor operations. Tests updated: - TestCLICommands_atmos_vendor_pull_using_SSH - TestCLICommands_atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]> * Add Atmos CLI performance profiling with multiple profile types (#1534) * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * updates * [autofix.ci] apply automated fixes * updates * updates * updates * updates * updates * updates --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: merge commands from all sources preserving precedence hierarchy (#1533) * fix: merge commands from .atmos.d instead of replacing them Fixed a regression where commands defined in .atmos.d/ directories were being replaced instead of merged with main configuration commands. This prevented custom commands like 'atmos dev' from working properly. The issue was introduced in PRs #1447 and #1489 which formalized the override behavior for configuration imports. While this behavior is correct for most configuration fields, command arrays should be merged to allow extending the CLI with custom commands. Changes: - Added mergeCommandArrays function to properly merge command arrays with deduplication based on command names - Updated mergeConfigFile to preserve and merge command arrays when processing imports - Modified processConfigImportsAndReapply to ensure commands from .atmos.d and other imports are preserved when re-applying main config - Added comprehensive tests for command merging from .atmos.d - Fixed existing test that was checking for incorrect error message This fix ensures that: - Commands from .atmos.d directories are properly merged with main configuration commands - Multiple command sources can coexist without replacing each other - The 'atmos dev' command and its subcommands work as expected Fixes the regression where 'atmos dev --help' was not working. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * test: add comprehensive test coverage for command import merging - Add command_merging_behavior_test.go with detailed merge scenarios - Add import_commands_test.go for various import configurations - Add final_test.go with real-world validation tests - Update imports.go with debug logging for import processing - Fix load.go command merging to properly handle import precedence These tests validate: - Commands from imports merge with local commands - Local commands override imported commands with duplicate names - Deep nested imports (4+ levels) work correctly - CloudPosse use case: 10 upstream + 1 local = 11 total commands * refactor: rename final_test.go to command_merge_core_test.go - Rename test file to better describe its purpose - Update function name from TestFinalCommandMergingBehavior to TestCommandMergeCore - This test validates core command merging functionality * docs: add comprehensive PRD for command merging functionality - Added Product Requirements Document detailing command merging implementation - Included PR description with technical details and test coverage information - Documents fix for regression where imported commands were replaced instead of merged - Captures CloudPosse's use case and multi-level import requirements * chore: remove PR description file from repository * test: fix failing test expectations after command merge order changes - Update test expectations to match actual command ordering behavior - Commands now appear in order: main, defaults (.atmos.d), imports - This allows main commands to override imported commands by name - Fix test YAML to use string steps instead of complex step objects - Update error message expectation in processConfigImportsAndReapply test * test: update golden snapshots for command merging functionality The test 'atmos_describe_config_imports' was failing because it now correctly merges commands from imported configurations (including remote imports from GitHub) instead of replacing them. Updated the golden snapshots to reflect the expected output with merged commands (tf, terraform, show, and test). This fixes test failures on Linux, macOS, and Windows CI pipelines. * test: fix failing test expectations after command merge order changes - Fixed all inline comments in command_merging_behavior_test.go to end with periods as required by golangci-lint's godot linter - Replaced fragment-only markdown links with full GitHub URLs in command-merging.md to satisfy markdownlint MD051 (fixes #1447 and #1489 links) - Added 'text' language hint to fenced code block in command-merging.md to satisfy markdownlint MD040 These changes address all linting warnings and ensure consistent code style. --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Andriy Knysh <[email protected]> * chore(deps): bump github.com/aws/aws-sdk-go-v2/service/ssm (#1539) Bumps [github.com/aws/aws-sdk-go-v2/service/ssm](https://github.com/aws/aws-sdk-go-v2) from 1.62.0 to 1.65.1. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Changelog](https://github.com/aws/aws-sdk-go-v2/blob/main/changelog-template.json) - [Commits](aws/aws-sdk-go-v2@service/s3/v1.62.0...service/s3/v1.65.1) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2/service/ssm dependency-version: 1.65.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump github.com/aws/aws-sdk-go-v2/feature/s3/manager (#1536) Bumps [github.com/aws/aws-sdk-go-v2/feature/s3/manager](https://github.com/aws/aws-sdk-go-v2) from 1.18.3 to 1.19.9. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Changelog](https://github.com/aws/aws-sdk-go-v2/blob/main/changelog-template.json) - [Commits](aws/aws-sdk-go-v2@config/v1.18.3...service/m2/v1.19.9) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2/feature/s3/manager dependency-version: 1.19.9 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: Improve test infrastructure and fix critical environment variable bug (#1543) * feat: implement vendor diff command with update and version checking capabilities (#1519) * feat: implement vendor diff command with update and outdated flags * feat: enhance vendor diff command to support detailed update checks and progress display * fix: store latest version information in vendor model for update checks * Allow processing of templates without context (#1485) * update * update * refactor: simplify template processing to use file extension detection Replace process_without_context configuration flag with automatic template detection based on file extension. Files with .yaml.tmpl or .yml.tmpl extensions are now always processed as Go templates, regardless of whether context is provided. This simplifies the mental model and reduces configuration surface area. Changes: - Remove ProcessWithoutContext flag and TemplateSettingsImport struct from schema - Add IsTemplateFile() utility to detect template files by extension - Update template processing logic to use file extension instead of config flag - Add comprehensive unit tests for template processing without context - Update documentation to clarify new behavior This makes the behavior more intuitive: .tmpl files are templates, non-.tmpl files are not. Users can still use skip_templates_processing flag for explicit control when needed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix: resolve golangci-lint commentedOutCode warnings Remove assignment operator from inline comments to fix linter warnings. Changed comments from 'skipTemplatesProcessingInImports = true/false' to just 'skipTemplatesProcessingInImports' to avoid triggering the commentedOutCode linter rule. * chore: remove vendor diff functionality Remove vendor diff command and related functionality that was pulled in from the devel branch. This PR is focused solely on import context functionality. - Remove cmd/vendor_diff.go command file - Revert vendor.go, vendor_model.go, vendor_utils.go to main branch versions - Update vendor_test.go to remove vendorDiffCmd test - Refactor stack_processor_utils.go to fix linting complexity issue * fix: handle all .tmpl files in template processing Fix test failures by properly detecting and processing all template files. The previous change only processed .yaml.tmpl and .yml.tmpl files, but missed plain .tmpl files which are used in test fixtures. Now processes templates when: 1. File has .yaml.tmpl or .yml.tmpl extension (always) 2. OR file has .tmpl extension AND context is provided This maintains backward compatibility while fixing test failures across all platforms (Windows, Linux, macOS). * fix: restore vendor diff stub from main branch The vendor diff command exists in main as an unimplemented stub. We had incorrectly deleted it entirely when removing the devel implementation. This restores the stub version that returns 'not implemented yet' error, maintaining compatibility with main. * fix: restore original template processing behavior with testable logic This commit fixes template processing to match the documented behavior where ANY file with context is processed as a template. The previous change incorrectly restricted template processing to specific file extensions only. Key changes: - Added ShouldProcessFileAsTemplate function for testable decision logic - Restored behavior: process any file as template when context provided - Always process .yaml.tmpl, .yml.tmpl, and .tmpl files - Added comprehensive unit tests for all scenarios - Created PRD documenting template processing requirements This fixes test failures where template syntax was leaking into output (e.g., tenant1-{{ .environment }}-test-1) because plain YAML files with context were not being processed as templates. * fix: restore correct golden snapshots for git repo test The golden snapshots were incorrectly regenerated to show error output when they should show the successful list of stacks. This test verifies that Atmos doesn't warn about missing git repo when it has a valid config, and should successfully list the stacks. Restored golden files from main branch which contain the correct expected output. * fix: restore circuit breaker functionality with proper PATH inheritance - Fix PATH environment variable inheritance for nested atmos subprocess calls - Add testable utility functions for PATH management in pkg/utils/env_utils.go - Prevent test case environment from overriding AtmosRunner's carefully constructed PATH - Remove problematic "which atmos" test that relied on system PATH lookup - Ensure circuit breaker properly detects infinite recursion in workflow commands - Clean up debugging output to restore clean test execution 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: resolve AtmosRunner test failures for GOCOVERDIR and cleanup - Filter out GOCOVERDIR environment variable when coverage is disabled to prevent test runner's GOCOVERDIR from interfering - Fix cleanup logic to handle both subdirectory and direct temp file cleanup scenarios - Ensure cleanup properly removes test binaries created directly in temp directory - All AtmosRunner tests now pass while maintaining circuit breaker functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: resolve git repository warning test failures in git worktrees - Fix AtmosRunner to build binary before changing test working directory - Create dedicated git-repository-warnings.yaml test file for proper test organization - Remove misplaced git repository tests from empty-dir.yaml - Add working directory inheritance to AtmosRunner Command methods - Enable testing of git repository warnings from outside git repo contexts This resolves snapshot test failures where git repository warning tests were incorrectly placed and couldn't execute properly due to AtmosRunner trying to build from outside the git repository. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: make PATH environment tests cross-platform compatible - Update TestEnsureBinaryInPath to use cross-platform path construction - Update TestUpdateEnvironmentPath to use filepath.Join and os.PathListSeparator - Fix TestEnvironmentPathIntegration to use dynamic path building - Replace hardcoded Unix paths with Windows-compatible path handling This resolves Windows test failures where hardcoded Unix-style paths (/usr/bin:/bin) were incompatible with Windows path formats and separators. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: address multiple code quality and cross-platform issues - Fix template test assertion to expect int(3) instead of string "3" in stack_processor_template_test.go - Add missing period to comment in cmd_utils.go for consistency with project style - Make UpdateEnvironmentPath case-insensitive for Windows PATH handling with findPathIndex helper - Fix GOCOVERDIR handling in AtmosRunner to filter existing entries before setting new ones - Update GetPathFromEnvironment to use case-insensitive PATH detection These fixes improve Windows compatibility, prevent environment variable duplication, and ensure consistent code style and test assertions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: resolve critical environment variable bug in executeCustomCommand - Implement UpdateEnvVar function in pkg/utils/env_utils.go for proper environment variable management - Fix executeCustomCommand to use custom environment variables instead of ignoring them - Remove references to AtmosRunner from production code comments - Add comprehensive tests for UpdateEnvVar function covering all scenarios - Ensure cross-platform compatibility with case-sensitive environment variable handling - Pass prepared environment with custom variables to ExecuteShell subprocess - Add envVarFormat constant to satisfy golangci-lint revive rule 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * chore: remove template processing test from infrastructure branch This test file belongs in the template-context-processing branch, not the infrastructure and environment fixes branch. * chore: remove template processing functionality from infrastructure branch These features belong in the template-context-processing branch: - docs/prd/template-processing-requirements.md - internal/exec/template_processing_test.go - ShouldProcessFileAsTemplate function in stack_processor_utils.go - formatTemplateProcessingError function in stack_processor_utils.go * chore: remove .go-version file The Go version is already specified in go.mod and doesn't need a separate version manager file. * fix: resolve Windows test failure with environment variable isolation Fixed the failing test "!terraform.output from component with !env function test" on Windows by improving test environment isolation and adding missing environment variable. Changes: - Add t.Setenv() loop in cli_test.go to set all test environment variables with proper isolation - Add missing ATMOS_TEST_1 environment variable to the terraform.output test case - Convert os.Setenv() calls to t.Setenv() in test files for automatic cleanup - Fix XDG_CACHE_HOME setting to use t.Setenv() instead of os.Setenv() - Simplify sandbox test environment setup using t.Setenv() The issue was caused by improved test isolation in PR #1524 which prevented tests from inheriting environment variables from the parent process. The test was previously working because it accidentally inherited ATMOS_TEST_1 from the CI environment, but now requires explicit configuration. This fix ensures: - component-5 can resolve !env ATMOS_TEST_1 to "test-env-and-terraform-output-functions" - component-6 can get that value via !terraform.output component-5 foo - Test passes consistently across all platforms with proper isolation 🤖 Generated with [Claude Code](https://claude.ai/claude-code) Co-Authored-By: Claude <[email protected]> * chore: remove file_extensions template processing functionality Remove IsTemplateFile function and associated test file that were added for template processing functionality. This doesn't belong in the infrastructure-and-env-fixes branch which should focus only on infrastructure improvements and environment variable fixes. Changes: - Remove IsTemplateFile() function from pkg/utils/file_extensions.go - Remove strings import that was only needed for IsTemplateFile - Delete pkg/utils/file_extensions_test.go test file 🤖 Generated with [Claude Code](https://claude.ai/claude-code) Co-Authored-By: Claude <[email protected]> * fix: resolve Windows coverage collection failure - Replace problematic grep one-liner with explicit conditional - Check for mock files existence before filtering - Handle Windows grep behavior differences - Suppress stderr to avoid warnings on cross-platform runs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Miguel Zablah <[email protected]> Co-authored-by: Sharon Dagan <[email protected]> Co-authored-by: Claude <[email protected]> * fix: final linting adjustments after merge resolution - Use helper function pattern for comment consistency - Address remaining godot linter issues Note: Bypassing pre-commit hooks as the remaining godot issue appears to be a linter bug where it incorrectly flags properly capitalized sentences. * fix: improve Windows path handling with UNC volume preservation and safe testing - Add UNC path detection and preservation in component path utilities - Fix filepath.Join() issues that corrupted UNC paths (\\server\share format) - Add UNC-aware path joining logic to maintain consistent separators - Create safe temp-based Windows path testing to avoid destructive hard-coded paths - Add comprehensive test coverage for UNC path scenarios and helper functions - Replace potential C:\Users\test usage with t.TempDir() for safety - Refactor complex nested blocks to improve code maintainability - Define constants for UNC prefix and Windows path separator to reduce duplication 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: add nolint directive for godot linter issue The godot linter incorrectly flags the 'Helper function' comment pattern as not starting with a capital letter. This appears to be a linter bug as 'Helper' clearly starts with capital 'H'. Adding nolint directive to work around this issue while maintaining standard Go documentation style. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: remove obsolete UNC path tests after merge The UNC path preservation tests were written for the feature branch's standalone UNC helper functions. After merging main's comprehensive path deduplication system (which includes integrated UNC handling via filepath.VolumeName), these tests are no longer applicable. Main's approach handles UNC paths through: - preserveVolume() with isUNCPath(volume) checks - handleUNCPath() for UNC-specific logic - Integrated into cleanDuplicatedPath() system 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: add YAML-based precondition system for vendor pull tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: remove no-op adjustSubdir and restore integration tests - Remove deprecated adjustSubdir function from CustomGitDetector - Function was a no-op after URI normalization moved to vendor pipeline - Remove corresponding test TestAdjustSubdir - Restore comprehensive vendor pull integration tests - Add vendor_pull_integration_test.go with 4 test cases - Restore TestVendorPullBasicExecution (vendor2 fixture) - Restore TestVendorPullConfigFileProcessing (config parsing) - Restore TestVendorPullFullWorkflow (complete workflow with verification) - Add new TestVendorPullTripleSlashNormalization (end-to-end URI normalization) - Tests use t.Cleanup() and t.Setenv() for better resource management - Fix CodeRabbit security alert - Capitalize function comment in vendor_utils.go:436 - Satisfies Go documentation standards Restores 200+ lines of integration test coverage that was accidentally deleted in commit 27e12d8. All tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: resolve merge conflicts and apply pending changes - Add CLI flag handling for base-path, config, and config-path overrides in root.go - Add profiler-related error definitions in errors/errors.go - Update vendor triple-slash test comments for consistency - Update config merge and import tests - Add environment variable utilities and tests - Update global flags documentation These changes resolve conflicts from the main branch merge and ensure all pending work-in-progress changes are committed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: enhance sanitizeImport and restore test best practices ## what - Enhanced sanitizeImport to handle all URL schemes (git::, s3::, gcs::, oci::, hg::, etc.) - Added comprehensive test coverage for sanitizeImport with 16 test cases - Restored verifyFileExists and verifyFileDeleted test helper functions - Converted os.Setenv to t.Setenv in vendor_triple_slash_test.go ## why - sanitizeImport only sanitized HTTP/HTTPS URLs, missing credentials in other schemes like git::https://, s3::, oci:: - go-getter supports many schemes that can contain credentials and need sanitization - verifyFileExists and verifyFileDeleted helpers were incorrectly removed in commit 32065af, reducing testability and reusability - os.Setenv requires manual cleanup; t.Setenv handles cleanup automatically 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: add case-insensitive path matching for Windows CI snapshots ## what - Fixed sanitizeOutput to use case-insensitive regex matching for repo paths - Added explicit backslash-to-forward-slash normalization for Windows paths - Created comprehensive test suite (cli_sanitize_test.go) with 50+ test cases - Added nolint directive for acceptable complexity in template error handling ## why - Windows CI was failing on TestCLICommands/atmos_describe_config_imports - Issue: repo root D:\a\atmos\atmos didn't match output d:/a/atmos/atmos - Root causes: 1. Case sensitivity in path matching (D: vs d:) 2. Backslashes in Windows paths not being normalized - Golden snapshots expect normalized paths like /absolute/path/to/repo ## tests - TestSanitizeOutput_WindowsCIFailureScenario: Reproduces exact CI failure - Tests D:\a\atmos\atmos (backslashes, uppercase) - Tests d:\a\atmos\atmos (backslashes, lowercase) - Tests d:/a/atmos/atmos (forward slashes, lowercase) - Tests mixed case in path segments - TestSanitizeOutput: Core functionality (URLs, debug logs, tokens) - TestCollapseExtraSlashes: Protocol and slash normalization - All 50+ test cases pass ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: only replace backslashes on Windows in sanitizeOutput ## what - Restrict backslash-to-forward-slash conversion to Windows only in sanitizeOutput - Preserve backslashes on Unix/macOS where they are legitimate characters (escape sequences) ## why - Previous fix for Windows broke macOS and Linux CI tests - The unconditional `strings.ReplaceAll(output, "\\", "/")` was corrupting legitimate backslashes - On Unix systems, backslashes are NOT path separators - they're used in escape sequences, regexes, etc. - filepath.ToSlash() already handles path separators correctly per platform ## tests - TestCLICommands/atmos_describe_config passes locally on macOS - Windows tests already passing with previous logic - Should fix macOS and Linux CI failures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: use regex to replace backslashes in path contexts only ## what - Changed sanitizeOutput to use regex for targeted backslash replacement - Only replaces backslashes followed by path-like characters (alphanumeric, ., -, _, *, /) - Preserves backslashes in escape sequence contexts (\n, \t, \r, etc.) - Works cross-platform - handles Windows paths even on Unix/Linux ## why - Previous OS-conditional approach broke Windows tests on Unix/Linux - Tests need to verify Windows path handling on all platforms - Escape sequences are already processed at runtime (appear as actual newlines/tabs) - Backslashes in CLI output are either path separators or literal characters - Regex approach safely handles both cases ## tests - All TestSanitizeOutput tests pass on macOS - Windows CI failure scenario tests work cross-platform - Handles backslash paths: `D:\a\atmos\atmos` → `/absolute/path/to/repo` - Preserves non-path backslashes if they appear 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: add github_token precondition to slow vendor tests ## what - Added github_token precondition to atmos_vendor_pull_with_globs - Added github_token precondition to atmos vendor pull (tty and no-tty variants) ## why - Without GitHub token, these tests hit API rate limits (60 req/hr) - go-getter respects rate limits and backs off, causing 8+ minute waits - With token, rate limit is 5000 req/hr and tests complete quickly - Tests now skip immediately when token unavailable instead of hanging ## tests - atmos_vendor_pull_with_globs: pulls from github.com with globs (took 25s without token) - atmos vendor pull: pulls from github.com 3x (took 494s without token) - atmos vendor pull no tty: same as above - All tests properly skip when GITHUB_TOKEN not set 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: generalize github_token precondition message ## what - Updated RequireOCIAuthentication message to cover all GitHub API use cases - Changed from OCI-specific to general GitHub API access message - Updated function comment to mention OCI, cloning, and rate limits ## why - Precondition is now used for general GitHub API access, not just OCI - Previous message "required for pulling OCI images from ghcr.io" was misleading - Tests use this for: OCI pulls, git clones from github.com, and avoiding rate limits - Message should accurately reflect all use cases ## changes - Skip message: "required for GitHub API access (OCI images, cloning repos, avoiding rate limits)" - Log message: "GitHub authentication available via token" (more general) - Function comment: clarifies all three use cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: replace heuristic vendor URI parsing with go-getter API ## what - Replace hardcoded domain pattern matching with go-getter's SourceDirSubdir() API - Add comprehensive test coverage for Azure DevOps, Gitea, and self-hosted Git platforms - Create documentation-first approach with PRD and user-facing URL syntax guide - Extract helper functions with inline examples to reduce cyclomatic complexity ## why - Previous implementation relied on observational heuristics (hardcoded .git///, .com///, .org/// patterns) - go-getter provides official API for parsing source/subdirectory, eliminating guesswork - Missing test cases for vendor URI variations (Azure DevOps _git/, self-hosted domains) - Code complexity violated golangci-lint limits (cyclomatic complexity 15 > max 10) - User feedback: "impossible to follow without examples" - needed inline documentation ## references - Closes #1504 (DEV-3639: vendor pull failures after go-getter v1.7.9 update) ## changes ### Documentation (new files) - docs/prd/vendor-uri-normalization.md - Technical design document explaining vendor URI system - Cataloged all 15 URI patterns with examples - Documented Atmos custom extensions (token injection, OCI, SCP rewriting) - Test coverage matrix - website/docs/core-concepts/vendor/url-syntax.mdx - User-facing URL syntax reference - All URL schemes with platform-specific examples - Interactive tabs for subdirectory syntax - Authentication and troubleshooting guides ### Core implementation changes - internal/exec/vendor_uri_helpers.go - containsTripleSlash(): Simplified to literal check (removed .git///, .com///, .org/// patterns) - parseSubdirFromTripleSlash(): Uses go-getter's SourceDirSubdir() API - isLocalPath(): Refactored with helper functions to reduce complexity - isGitLikeURI(): Extracted helper - detects Git repository patterns (github.com, .git, _git/) - isDomainLikeURI(): Extracted helper - detects domain structure (hostname.domain/path) - Added comprehensive inline examples to all functions per user feedback - internal/exec/vendor_utils.go - normalizeTripleSlash(): Uses parseSubdirFromTripleSlash() for robust parsing ### Test coverage - internal/exec/vendor_utils_test.go - Added 7 new test cases for Azure DevOps, Gitea, self-hosted Git - All 27 tests passing ### Documentation updates - website/docs/core-concepts/vendor/vendor-manifest.mdx: Added URL syntax reference - website/docs/cli/commands/vendor/vendor-pull.mdx: Added URL syntax reference - website/docs/core-concepts/vendor/components-manifest.mdx: Fixed example (/// → //modules/name) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: extract named helper functions in vendor URI detection ## what - Extract each URI check in `isLocalPath()` into dedicated helper functions - Add comprehensive test coverage with 104 test cases across 6 functions - Reduce cyclomatic complexity and improve code clarity ## why - Each check in `isLocalPath()` now has a clear, named function describing its purpose - Follows same pattern as existing `isDomainLikeURI()` and `isGitLikeURI()` - Comprehensive tests ensure all edge cases are covered - Makes the code easier to understand, test, and maintain ## changes ### New helper functions (vendor_uri_helpers.go) - `hasLocalPathPrefix()` - checks for `/`, `./`, `../` prefixes - `hasSchemeSeparator()` - checks for `://` or `::` scheme separators - `hasSubdirectoryDelimiter()` - checks for `//` go-getter delimiter ### Refactored function - `isLocalPath()` - now uses helper functions for clarity ### Comprehensive test coverage (vendor_uri_helpers_test.go) - TestHasLocalPathPrefix: 11 test cases - TestHasSchemeSeparator: 13 test cases - TestHasSubdirectoryDelimiter: 12 test cases - TestIsGitLikeURI: 13 test cases - TestIsDomainLikeURI: 13 test cases - TestIsLocalPath: 22 test cases (integration) All 104 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: fix PRD to compare against main branch baseline ## what - Correct PRD to accurately describe main branch baseline - Remove incorrect reference to "hardcoded patterns" that never existed in main ## why - The PRD incorrectly stated main branch had hardcoded `.git///`, `.com///`, `.org///` patterns - Those patterns only existed in an intermediate commit (e881711) during this PR - Main branch actually has NO triple-slash handling at all - PRD should compare before/after against main, not intermediate PR states ## changes - Updated "The Triple-Slash Problem" section to accurately reflect: - **Before (main)**: No triple-slash handling → silent failures - **After (this PR)**: Proper normalization using go-getter's SourceDirSubdir() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: replace broad .Contains() checks with precise URL parsing ## what - Replace overly broad `.Contains()` checks in Git/S3 URI detection - Parse URLs properly to separate host from path before matching - Add S3 `.amazonaws.com` auto-detection - Remove duplicate `isGitLikeURI` function, keep only `isGitURI` - Extract helper functions to reduce cyclomatic complexity ## why - Previous `.Contains()` checks had false positives: - `www.gitman.com` incorrectly matched as Git URL - `evil.com/github.com/fake` incorrectly matched (github.com in path) - `/local/path/.git/config` incorrectly matched (.git in path) - `s3::` prefix exists but S3 also auto-detects `.amazonaws.com` URLs - No need for duplicate Git detection functions - Cyclomatic complexity violation (12 > max 10) ## changes - `isS3URI`: Now checks both `s3::` prefix and `.amazonaws.com/` pattern - `isGitURI`: Properly parses URLs with helper functions: - `parseGitHostAndPath()`: Separates host from path - `isKnownGitHost()`: Checks known Git platforms - `hasGitExtension()`: Validates .git extension position - Removed `isGitLikeURI` dead code path - Added comprehensive test cases for edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: add SCP-style Git URL detection and security edge case tests - Extract scpURLPattern as single source of truth for SCP URL matching - Add SCP-style URL detection to isGitURI using regex pattern - Replace custom parseGitHostAndPath with net/url.Parse for proper host/path separation - Add 16 security edge case tests (unicode homograph, path traversal, XSS, etc.) - Fix regression where [email protected]:owner/repo.git URLs were not detected 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: clarify needsDoubleSlashDot function logic - Clarify that only Git URIs need double-slash-dot pattern - Add example showing transformation (github.com/owner/repo.git → github.com/owner/repo.git//.) - Improve inline comments to explain what each condition checks - Replace "Skip" wording with clearer "Only" and "Already has" phrasing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: reorganize vendor URL examples by platform and update repos - Reorganize URL patterns section: group by platform (GitHub, GitLab, Bitbucket, OCI, etc.) instead of by example type - Update all examples from old terraform-aws-components monorepo to new cloudposse-terraform-components/aws-[component] repos - Fix template examples to use aws- prefix (e.g., aws-{{.Component}}) - Improve subdirectory best practices to reflect new per-component repo structure - Simplify navigation by reducing nested tabs (each platform now has one tab) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: add language identifiers to fenced code blocks in PRD - Add `text` language identifier to all URL example code blocks - Fixes markdownlint warnings (MD040) for fenced-code-language - Improves syntax highlighting and linting support 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: add comprehensive coverage for triple-slash normalization helpers - Add TestContainsTripleSlash with 12 test cases - Add TestParseSubdirFromTripleSlash with 9 test cases - Add TestNeedsDoubleSlashDot with 17 test cases - Add TestAppendDoubleSlashDot with 11 test cases - All tests use table-driven pattern with clear test names - Add nolint directives for legitimate test duplication - Fix gocritic warning for commented code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: correct hasSubdirectoryDelimiter to skip scheme separators - Fix hasSubdirectoryDelimiter to not match :// in schemes (https://, file://, oci://) - Only match go-getter subdirectory delimiter // when not preceded by : - Fix test expectation for subdirectory_delimiter_only (should be false, not true) - All tests now pass on all platforms 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: rename profiler-prefixed config errors to appropriate names - Rename ErrProfilerParseMainConfig → ErrParseMainConfig - Rename ErrProfilerMergeMainConfig → ErrMergeMainConfig - Rename ErrProfilerReapplyMainConfig → ErrReapplyMainConfig - Move config loading errors to separate section from profiler errors - These errors are used in general config loading, not profiler-specific code Addresses review feedback from @aknysh 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: handle URIs ending with // in appendDoubleSlashDot - Remove trailing // before appending //. to avoid creating //// - Add test case for URI already ending with // (e.g., github.com/org/repo.git//?ref=v1.0) - Ensures correct normalization to github.com/org/repo.git//.?ref=v1.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: use ErrMerge sentinel for config loading error wrapping - Replace invalid double %w format with proper sentinel error wrapping - Use ErrMerge as sentinel with %w and original error as context with %v - Remove unused ErrParseMainConfig, ErrMergeMainConfig, ErrReapplyMainConfig - Add Azure DevOps triple-slash regression test for DEV-3639 - Prevents runtime panic from invalid fmt.Errorf format string Addresses code review feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: move import errors to centralized errors package - Move ErrBasePath, ErrTempDir, ErrResolveLocal, etc. to errors/errors.go - Update pkg/config/imports.go to use errUtils package - Remove local error declarations per project error policy - Add ErrNoValidAbsolutePaths for line 323 usage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: sort resolved import paths for deterministic logging - Add sort.Slice to sort resolvedPaths by filePath before iteration - Ensures import logs always appear in consistent alphabetical order - Makes test snapshots deterministic across platforms and runs - Path normalization already handled by tests/cli_test.go sanitizeOutput() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Revert "refactor: sort resolved import paths for deterministic logging" This reverts commit 774c6f9. * refactor: use t.Cleanup and t.Setenv for test teardown - Replace defer with t.Cleanup for better test cleanup management - Use t.Setenv for all environment variables (auto-restores on cleanup) - Set ATMOS_CLI_CONFIG_PATH and ATMOS_BASE_PATH properly - Cleanup runs even if subtests are added later - Addresses CodeRabbit review feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: add PR review thread response instructions to CLAUDE.md - Document how to reply to specific review threads using GraphQL API - Include query to find unresolved threads - Mandate replying to threads, not creating new PR comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Claude <[email protected]> Co-authored-by: Andriy Knysh <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Miguel Zablah <[email protected]> Co-authored-by: Sharon Dagan <[email protected]>
This was referenced Oct 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
--profile-typeCLI flag for selecting profile type in file-based profilingwhy
pproftool. Profiling helps identify performance bottlenecks, memory usage patterns, and CPU hotspots to optimize Atmos operations.references
pkg/profiler/profiler.gowith multi-profile-type supportpkg/config/load.gocmd/root.gopkg/profiler/profiler_test.gowebsite/docs/troubleshoot/profiling.mdxandwebsite/docs/cli/global-flags.mdxKey Features Added
Multiple Profile Types Support:
Environment Variables Added:
ATMOS_PROFILER_ENABLED- Enable/disable pprof HTTP profiling serverATMOS_PROFILER_HOST- Set host address for profiling serverATMOS_PROFILER_PORT- Set port for profiling serverATMOS_PROFILE_FILE- Set file path for file-based profilingATMOS_PROFILE_TYPE- Set profile type for file-based profilingCLI Enhancements:
--profile-typeflag with validation and error handling--profile-fileto automatically enable profilingConfiguration Precedence:
Usage Examples
File-based profiling with different types:
Environment variable usage:
Error handling for invalid profile types:
$ atmos terraform plan vpc -s prod --profile-file=test.prof --profile-type=invalid 👽 Atmos 1.192.0 on darwin/arm64 ERRO Failed to setup profiler error="invalid profile type: unsupported profile type: invalid. Supported types: [cpu heap allocs goroutine block mutex threadcreate trace]"Server-based profiling:
Profile Analysis Examples
Analyzing CPU profiles:
Analyzing trace profiles:
Analyzing memory profiles:
Testing
Files Modified
pkg/profiler/profiler.go- Enhanced with multi-profile supportpkg/profiler/profiler_test.go- Added comprehensive test coveragecmd/root.go- Enhanced CLI flag and environment variable handlingpkg/config/load.go- Added environment variable bindingswebsite/docs/troubleshoot/profiling.mdx- Updated documentationwebsite/docs/cli/global-flags.mdx- Added profiling flag docs🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]
Summary by CodeRabbit
New Features
Documentation
Tests
Chores