fix: Reduce log spam for imports outside base directory by osterman · Pull Request #1780 · cloudposse/atmos

osterman · 2025-11-10T21:16:04Z

what

Changed import path validation logging from WARN to TRACE level
Added test coverage for imports outside base directory

why

When using import paths that resolve outside the base directory, the warning message was being logged repeatedly during CI/CD workflows, creating excessive log spam. This is particularly noticeable in GitHub Actions where atmos is invoked multiple times.

The message "Import path is outside of base directory" is informational trace data about the import resolution process, not a user-actionable warning. Imports outside the base directory are a valid use case for shared configurations.

references

Consistent with other import-related logging in the same file (lines 108, 111, 114 of pkg/config/imports.go) which use TRACE level
Follows the logging level pattern:
- TRACE: Detailed import flow (what's happening during resolution)
- DEBUG: Error conditions (what went wrong)
- WARN: User-actionable problems (what needs fixing)

Summary by CodeRabbit

Tests
- Added test coverage for local imports outside the base directory to ensure proper resolution behavior.
Bug Fixes
- Reduced log verbosity by changing import path warnings to trace-level logs, decreasing warning-level noise in logs.

Changed import path validation logging from WARN to TRACE level to prevent excessive log spam when using legitimate parent directory imports. **Problem:** When atmos.yaml is placed in `.github/atmos.yaml` and imports `../atmos.yaml`, each import resolution triggers a warning. In GitHub Actions, this creates hundreds of duplicate warnings since atmos is called multiple times. **Example scenario:** ``` .github/atmos.yaml: import: - ../atmos.yaml Base directory: /home/runner/_work/infra/infra Import resolves to: /home/runner/_work/infra/atmos.yaml (outside base) Result: WARN spam on every atmos invocation ``` **Solution:** Import paths outside the base directory are a valid use case for shared configurations. Changed from `log.Warn()` to `log.Trace()` so the message is only visible at trace logging level, consistent with other import-related logging in the same file (lines 108, 111, 114). **Logging level rationale:** - TRACE: Detailed import flow (attempting merge, success/failure per file) - DEBUG: Error conditions (failed to resolve path, invalid imports) - WARN: User-actionable problems - INFO: High-level operations This message is informational trace data about import resolution, not a debug-level error condition or user-actionable warning. **Testing:** Added TestProcessLocalImport_OutsideBaseDirectory to verify imports outside base directory work correctly and only log at trace level. Resolves parent directory import warning spam in GitHub Actions and CI/CD. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2025-11-10T21:16:55Z

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

coderabbitai · 2025-11-10T21:18:42Z

📝 Walkthrough

Walkthrough

Modified logging for imports outside the base directory from warning to trace level and added a test case validating that processLocalImport correctly resolves imports referencing paths outside the base directory.

Changes

Cohort / File(s)	Summary
Import logging adjustment `pkg/config/imports.go`	Changed log level from warning to trace when a local import path is detected outside the base directory
Test coverage `pkg/config/import_test.go`	Added `TestProcessLocalImport_OutsideBaseDirectory` test function to validate cross-directory import resolution behavior

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

The logging change is straightforward—only the severity level is affected, not the condition logic
The new test is self-contained and directly exercises existing functionality

Suggested labels

patch

Suggested reviewers

aknysh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: reducing log spam by adjusting the logging level for imports outside the base directory.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch osterman/fix-import-path-warning

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ff26f86 and 9c1926b.

📒 Files selected for processing (2)

pkg/config/import_test.go (1 hunks)
pkg/config/imports.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

pkg/**/*.go

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

Place business logic in pkg rather than in cmd

Files:

pkg/config/import_test.go
pkg/config/imports.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

Files:

pkg/config/import_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

Files:

pkg/config/import_test.go
pkg/config/imports.go

**/!(*_test).go

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

Document all exported functions, types, and methods with Go doc comments

Files:

pkg/config/imports.go

🧠 Learnings (11)

📓 Common learnings

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 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.

Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.

Learnt from: aknysh
Repo: cloudposse/atmos PR: 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.

Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.

Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.

Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/load.go:219-221
Timestamp: 2025-02-24T22:46:39.744Z
Learning: In the Atmos configuration system, imports from atmos.d are optional. When import errors occur, they should be logged at debug level and the process should continue, rather than failing completely.

Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.

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

Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors in `mergeDefaultImports` and `mergeImports` functions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions like `mergeConfigFile` are returned to halt execution.

Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.

📚 Learning: 2025-05-23T19:51:47.091Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.

Applied to files:

pkg/config/import_test.go

📚 Learning: 2025-09-23T02:30:42.362Z

Learnt from: CR
Repo: cloudposse/atmos PR: 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/config/import_test.go

📚 Learning: 2024-11-19T23:00:45.899Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.

Applied to files:

pkg/config/import_test.go

📚 Learning: 2025-05-23T19:51:47.091Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.

Applied to files:

pkg/config/import_test.go

📚 Learning: 2025-05-23T19:51:47.091Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests.

Applied to files:

pkg/config/import_test.go

📚 Learning: 2025-09-23T02:30:42.362Z

Learnt from: CR
Repo: cloudposse/atmos PR: 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/config/import_test.go

📚 Learning: 2025-05-23T19:51:47.091Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method that can be called on *testing.T objects. This functionality is implemented through custom testing framework extensions and is used consistently throughout the test suite for changing working directories during tests.

Applied to files:

pkg/config/import_test.go

📚 Learning: 2024-10-28T01:51:30.811Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.

Applied to files:

pkg/config/imports.go

📚 Learning: 2025-02-21T20:56:05.539Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.

Applied to files:

pkg/config/imports.go

📚 Learning: 2025-02-21T20:56:20.761Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1077
File: pkg/downloader/custom_github_detector_test.go:0-0
Timestamp: 2025-02-21T20:56:20.761Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log`, not `clog`.

Applied to files:

pkg/config/imports.go

🧬 Code graph analysis (1)

pkg/config/imports.go (1)

pkg/logger/log.go (1)

Trace (14-16)

⏰ 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). (10)

GitHub Check: Lint (golangci)
GitHub Check: Analyze (go)
GitHub Check: Build (macos)
GitHub Check: Build (windows)
GitHub Check: Review Dependency Licenses
GitHub Check: Run pre-commit hooks
GitHub Check: autofix
GitHub Check: Lint (golangci)
GitHub Check: Analyze (go)
GitHub Check: Summary

🔇 Additional comments (2)

pkg/config/imports.go (1)

222-227: LGTM! Logging level change is appropriate.

Changing from WARN to TRACE for imports outside the base directory is consistent with the other import flow logging in this file (lines 108, 111, 114) and correctly reflects that this is informational trace data rather than a user-actionable warning.

pkg/config/import_test.go (1)

258-301: Good test coverage for imports outside base directory.

The test properly validates that processLocalImport resolves imports from outside the base directory without errors. The setup with parent/subdirectory structure accurately simulates the real-world scenario described in the PR objectives.

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

❤️ Share

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

codecov · 2025-11-10T21:27:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.76%. Comparing base (256a9b7) to head (a8380f2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1780      +/-   ##
==========================================
+ Coverage   70.71%   70.76%   +0.05%     
==========================================
  Files         432      432              
  Lines       39916    39916              
==========================================
+ Hits        28227    28248      +21     
+ Misses       9286     9267      -19     
+ Partials     2403     2401       -2

Flag	Coverage Δ
unittests	`70.76% <100.00%> (+0.05%)`	⬆️

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

Files with missing lines	Coverage Δ
pkg/config/imports.go	`91.05% <100.00%> (ø)`

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Resolved snapshot conflicts by taking main's version. User will regenerate snapshots as needed for ai-1 branch changes. Merged changes from main: - feat: add error handling infrastructure with context-aware capabilities (#1763) - fix: Resolve changelog check failures on large PRs (#1782) - Refine homepage hero and typing animation (#1781) - fix: Reduce log spam for imports outside base directory (#1780) - feat: Add custom sanitization map to test CLI for test-specific output rules (#1779) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2025-11-12T01:16:26Z

These changes were released in v1.199.0-rc.0.

osterman requested a review from a team as a code owner November 10, 2025 21:16

github-actions bot added the size/s Small size PR label Nov 10, 2025

osterman added the patch A minor, backward compatible change label Nov 10, 2025

coderabbitai bot approved these changes Nov 10, 2025

View reviewed changes

Merge branch 'main' into osterman/fix-import-path-warning

a8380f2

aknysh approved these changes Nov 11, 2025

View reviewed changes

aknysh merged commit dd3883f into main Nov 11, 2025
53 checks passed

aknysh deleted the osterman/fix-import-path-warning branch November 11, 2025 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Reduce log spam for imports outside base directory#1780

fix: Reduce log spam for imports outside base directory#1780
aknysh merged 2 commits intomainfrom
osterman/fix-import-path-warning

osterman commented Nov 10, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

github-actions bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 10, 2025

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Uh oh!

codecov bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

osterman commented Nov 10, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what

why

references

Summary by CodeRabbit

Uh oh!

github-actions bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

coderabbitai bot commented Nov 10, 2025

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

github-actions bot commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

osterman commented Nov 10, 2025 •

edited by coderabbitai bot

Loading

github-actions bot commented Nov 10, 2025 •

edited

Loading

codecov bot commented Nov 10, 2025 •

edited

Loading