Skip to content

Update overrides section behavior in imports and inline#1122

Merged
aknysh merged 48 commits intomainfrom
update-overrides-in-imports
Mar 11, 2025
Merged

Update overrides section behavior in imports and inline#1122
aknysh merged 48 commits intomainfrom
update-overrides-in-imports

Conversation

@aknysh
Copy link
Member

@aknysh aknysh commented Mar 8, 2025

what

  • Update overrides section behavior in imports and inline
  • Add acceptance tests for the overrides section for many scenarios (overrides at the stack level, team level, inline and in imports)
  • Update docs
  • Update Go to 1.24.0 and fix some issues in the code (1.24.0 is more strict about the Go format functions always requiring the format as the first argument)
  • Add unit tests

why

  • Importing the overrides did not cover all possible scenarios (e.g. when importing the overrides for the imported components, the inline overrides for the same stack were not taken into account). This PR fixes this and adds acceptance tests for all scenarios
  • Update the Share Data Between Components doc to describe how the !terraform.output YAML function can (and should be) used to to read the outputs (remote state) of components directly in Atmos stack manifests

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Enhanced configuration processing distinguishes inline and imported overrides for greater deployment flexibility.
    • Added sample configuration files for various environments (development, production, staging, sandbox, and test) with team-specific settings.
    • Introduced new YAML function !terraform.output for improved data sharing in stack manifests.
    • New workflows added for testing various Atmos commands across multiple environments.
  • Documentation

    • Updated documentation to showcase improved data sharing and new output functionalities.
  • Chores

    • Upgraded Atmos CLI and related dependencies for enhanced performance.
    • Improved command output formatting for clearer error messages.
    • Added .gitignore entry to exclude specific directories from version control.
  • Tests

    • Introduced test cases to validate configuration override behavior across environments.
    • Added tests for the aboutCmd and supportCmd commands to ensure correct output.
    • New test functions added for ProcessYAMLConfigFile, ValidateStacks, and workflow command to enhance coverage.
    • Comprehensive tests for markdown utility functions to ensure correct behavior.
    • New tests for handling invalid configurations and edge cases in YAML processing.
    • Added tests for vendor configuration handling and workflow execution.

…verrides-in-imports

# Conflicts:
#	examples/quick-start-advanced/Dockerfile
#	website/docs/integrations/atlantis.mdx
coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 11, 2025
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 11, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
pkg/utils/markdown_utils.go (1)

70-70: Fix typo in comment.

There's a typo in the comment: "exist" should be "exit".

-// PrintErrorMarkdownAndExit prints an error message in Markdown format and exist with the exit code 1.
+// PrintErrorMarkdownAndExit prints an error message in Markdown format and exits with the exit code 1.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfa2360 and 46f3f1f.

📒 Files selected for processing (8)
  • .golangci.yml (0 hunks)
  • pkg/utils/markdown_utils.go (1 hunks)
  • pkg/utils/markdown_utils.go (5 hunks)
  • .golangci.yml (4 hunks)
  • .golangci.yml (4 hunks)
  • pkg/utils/markdown_utils.go (1 hunks)
  • pkg/utils/markdown_utils.go (1 hunks)
  • .golangci.yml (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • pkg/utils/markdown_utils.go
  • pkg/utils/markdown_utils.go
  • pkg/utils/markdown_utils.go
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Build (ubuntu-latest, linux)
  • GitHub Check: Analyze (go)
  • GitHub Check: website-deploy-preview
  • GitHub Check: Summary
🔇 Additional comments (12)
pkg/utils/markdown_utils.go (8)

12-12: LGTM! Import alias change improves readability.

Changing the import alias from l to log makes the code more self-descriptive and aligns with Go naming conventions.


20-20: LGTM! Improved comment formatting.

Adding the period maintains consistent comment formatting throughout the codebase.


23-23: LGTM! Proper noun capitalization.

Correctly capitalizing "Markdown" as it's a proper noun.


47-47: LGTM! Consistent with import alias change.

This line correctly uses the updated import alias.


52-52: LGTM! Consistent comment improvements.

Both capitalization and punctuation improvements match the other comment updates.


83-86: Fix typo in new function comment and consider refactoring exit pattern.

This new helper function is useful, but there's a typo in the comment and it inherits the same testability issue from PrintErrorMarkdownAndExit.

  1. Fix the typo:
-// PrintInvalidUsageErrorAndExit prints a message about the incorrect command usage and exist with the exit code 1.
+// PrintInvalidUsageErrorAndExit prints a message about the incorrect command usage and exits with the exit code 1.
  1. Consider addressing the testability issue mentioned in past reviews:
 // PrintInvalidUsageErrorAndExit prints a message about the incorrect command usage and exits with the exit code 1.
+// TODO: Refactor so that we only call `os.Exit` in `main()` or `init()` functions.
+// Exiting here makes it difficult to test.
+// revive:disable-next-line:deep-exit
 func PrintInvalidUsageErrorAndExit(err error, suggestion string) {
     PrintErrorMarkdownAndExit("Incorrect Usage", err, suggestion)
 }

88-88: LGTM! Consistent comment style.

Updated comment matches the style of other comments in the file.


106-106: LGTM! Clear and concise comment.

The updated comment clearly describes the function's purpose.

.golangci.yml (4)

12-12: Update for err113 Linter Description
The updated description now emphasizes enforcing best practices by focusing on error comparisons, which aligns well with our overall error-handling improvements.


28-28: Addition of nolintlint Linter
Adding nolintlint to check for nolint comments is a smart move to enforce better linting hygiene. It should help prevent misuse of suppression comments.


100-101: Introduction of function-result-limit Rule
The new Revive rule limits functions to returning at most 2 values, which encourages improved function design and better readability. The inline comment clearly explains the intent to use objects for cases needing more data.


165-179: New Severity Configuration Block
The added severity section sets a strong default by treating violations as errors while downgrading specific linter issues (e.g., function-length, function-result-limit, etc.) to warnings. This balanced approach should provide clearer guidance and prevent overwhelming developers with non-critical issues.

@aknysh aknysh merged commit 5dab42b into main Mar 11, 2025
49 of 50 checks passed
@aknysh aknysh deleted the update-overrides-in-imports branch March 11, 2025 16:04
@mergify mergify bot removed the needs-cloudposse Needs Cloud Posse assistance label Mar 11, 2025
@github-actions
Copy link

These changes were released in v1.165.3.

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

Labels

patch A minor, backward compatible change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants