Skip to content

Conversation

aknysh
Copy link
Member

@aknysh aknysh commented May 22, 2025

what

  • Update stacks.name_template
  • Update docs
  • Add unit tests

why

For the remote-state terraform module and terraform-provider-utils, allow using the stacks.name_template setting in atmos.yaml with a limited context:

  • .vars.namespace
  • .vars.tenant
  • .vars.environment
  • .vars.stage
name_template: "{{.vars.tenant}}-{{.vars.environment}}-{{.vars.stage}}"

This will allow using name_template (instead of name_pattern) in both Atmos CLI and the remote-state module (to get the outputs of TF components)

stacks.name_template serves the same purpose as stacks.name_pattern (defines the naming convention for the top-level Atmos stacks), but provides much more functionality. Instead of using the predefined context variables as tokens, it uses Go templates.

Atmos Template Functions, Sprig Functions, Gomplate Functions, and Gomplate Datasources are supported as well

name_template can have complex logic and use template expressions and functions. The following template defines a name_template that builds a stack_name string by validating and concatenating several input variables in a hierarchical order.

name_template: |-
  {{- $ns := .vars.namespace -}}
  {{- $tenant := .vars.tenant -}}
  {{- $env := .vars.environment -}}
  {{- $stage := .vars.stage -}}
  {{- $stack_name := "" -}}

  {{- if eq $ns "" -}}
  {{- fail "Error: 'namespace' is required." -}}
  {{- end -}}

  {{- if and (ne $tenant "") (eq $ns "") -}}
  {{- fail "Error: 'tenant' requires 'namespace'." -}}
  {{- end -}}

  {{- if and (ne $env "") (or (eq $tenant "") (eq $ns "")) -}}
  {{- fail "Error: 'environment' requires 'tenant' and 'namespace'." -}}
  {{- end -}}

  {{- if and (ne $stage "") (or (eq $env "") (eq $tenant "") (eq $ns "")) -}}
  {{- fail "Error: 'stage' requires 'environment', 'tenant', and 'namespace'." -}}
  {{- end -}}

  {{- if ne $tenant "" -}}
  {{- $stack_name = $tenant -}}
  {{- end -}}

  {{- if ne $env "" -}}
  {{- $stack_name = printf "%s-%s" $stack_name $env -}}
  {{- end -}}

  {{- if ne $stage "" -}}
  {{- $stack_name = printf "%s-%s" $stack_name $stage -}}
  {{- end -}}

  {{- $stack_name -}}

It pulls values from the Atmos section vars and assigns them to local template variables:

  • namespace
  • tenant
  • environment
  • stage

The template enforces hierarchical dependencies between variables:

  • namespace is required
  • If tenant is provided, namespace must also be set
  • If environment is provided, both tenant and namespace must be set
  • If stage is provided, then environment, tenant, and namespace must all be set

If validations pass, it constructs the stack_name progressively:

  • Starts with tenant if it exists
  • Appends environment if it exists
  • Appends stage if it exists

The template outputs the resulting stack name. For example, if the variables are:

namespace: acme
tenant: plat
environment: ue2
stage: prod

The resulting stack name will be plat-ue2-prod.

Summary by CodeRabbit

  • New Features
    • Added support for stack name templates, allowing flexible and hierarchical stack naming using Go templates and context variables.
  • Bug Fixes
    • Improved error handling when both stack name template and pattern are missing.
  • Documentation
    • Expanded documentation with detailed examples and guidance for stack name templates and supported template functions.
  • Tests
    • Introduced new tests and configuration scenarios to validate stack name templating functionality.
  • Chores
    • Upgraded several dependencies to their latest versions for improved stability and compatibility.

@aknysh aknysh self-assigned this May 22, 2025
@aknysh aknysh requested a review from a team as a code owner May 22, 2025 03:49
@aknysh aknysh added the no-release Do not create a new release (wait for additional code changes) label May 22, 2025
Copy link
Contributor

coderabbitai bot commented May 22, 2025

📝 Walkthrough

Walkthrough

This update introduces support for stack name templates in stack and component processing, adds a new accessor for stack name templates, updates related documentation, and provides comprehensive test fixtures and tests for the new functionality. Several dependencies are also bumped in the go.mod file.

Changes

File(s) Change Summary
go.mod Bumped several dependencies to newer versions; no changes to module path or Go version.
internal/exec/stack_utils.go Added GetStackNameTemplate function; clarified comment for GetStackNamePattern.
internal/exec/utils.go Moved the addition of atmos_cli_config to ComponentSection to just before the return; minor comment tweak.
pkg/component/component_processor.go Introduced error for missing stack name template/pattern; updated stack name resolution logic to support templates and patterns.
pkg/component/component_processor_test.go Added test for stack name template processing, verifying variable population and error cases.
tests/fixtures/scenarios/stack-name-template/atmos.yaml New configuration file defining stack/component settings and a complex stack name template.
tests/fixtures/scenarios/stack-name-template/stacks/catalog/c1.yaml New fixture YAML defining a Terraform component with variables.
tests/fixtures/scenarios/stack-name-template/stacks/mixins/region/us-east-2.yaml New fixture YAML defining region and environment variables.
tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml
.../prod.yaml
New fixture YAMLs defining stage variables for "dev" and "prod".
tests/fixtures/scenarios/stack-name-template/stacks/mixins/tenant/plat.yaml New fixture YAML defining tenant variable.
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/_defaults.yaml New defaults YAML defining namespace variable.
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/_defaults.yaml New defaults YAML importing org and tenant defaults.
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/_defaults.yaml New defaults YAML importing plat and stage defaults.
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/us-east-2.yaml New manifest importing dev defaults, region mixin, and catalog component.
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/_defaults.yaml New defaults YAML importing plat and prod stage defaults.
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/us-east-2.yaml New manifest importing prod defaults, region mixin, and catalog component.
website/docs/cli/configuration/configuration.mdx Expanded documentation on supported template functions and data sources.
website/docs/cli/configuration/stacks.mdx Expanded documentation for stacks.name_template, added detailed example and function library list.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ComponentProcessor
    participant StackUtils

    User->>ComponentProcessor: ProcessComponentFromContext(ctx, atmosConfig, ...)
    ComponentProcessor->>StackUtils: GetStackNameTemplate(atmosConfig)
    StackUtils-->>ComponentProcessor: stackNameTemplate
    ComponentProcessor->>StackUtils: GetStackNamePattern(atmosConfig)
    StackUtils-->>ComponentProcessor: stackNamePattern
    alt stackNameTemplate is set
        ComponentProcessor->>ComponentProcessor: Render stack name using template and context
    else stackNamePattern is set
        ComponentProcessor->>ComponentProcessor: Derive stack name using pattern and context
    else
        ComponentProcessor->>ComponentProcessor: Return error (missing template/pattern)
    end
    ComponentProcessor-->>User: Component configuration (or error)
Loading

Possibly related PRs

Suggested labels

minor

Suggested reviewers

  • osterman

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.


Note

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.
Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6708b5b and 25f90e4.

📒 Files selected for processing (1)
  • pkg/component/component_processor_test.go (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
pkg/component/component_processor_test.go (1)
pkg/component/component_processor.go (1)
  • ProcessComponentFromContext (48-103)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (3)
pkg/component/component_processor_test.go (3)

4-4: Import for OS package added correctly.

Clean addition of the os package import to support environment variable testing in the new test function.


186-204: Well-structured test for stack name template functionality.

Good test case that validates the successful processing of a component using a stack name template. The test properly verifies all expected variables are correctly set in the component map.


206-230: Comprehensive error handling test cases.

Strong test coverage for error scenarios including:

  1. Missing namespace validation
  2. Missing tenant validation with proper error messages
  3. Invalid template expression via environment variable

Good practice using deferred cleanup for the environment variable.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or @auto-summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @auto-title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented May 22, 2025

Codecov Report

Attention: Patch coverage is 81.08108% with 7 lines in your changes missing coverage. Please review.

Project coverage is 48.57%. Comparing base (b6c2077) to head (25f90e4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/component/component_processor.go 74.07% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1264      +/-   ##
==========================================
+ Coverage   48.06%   48.57%   +0.51%     
==========================================
  Files         233      233              
  Lines       25441    25461      +20     
==========================================
+ Hits        12228    12368     +140     
+ Misses      11624    11489     -135     
- Partials     1589     1604      +15     
Flag Coverage Δ
unittests 48.57% <81.08%> (+0.51%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

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: 1

🧹 Nitpick comments (2)
tests/fixtures/scenarios/stack-name-template/atmos.yaml (2)

45-47: Unreachable tenant validation
This block checks for a non-empty tenant and an empty namespace, but the previous guard already fails on an empty namespace. As a result, this condition can never be hit. Consider removing lines 45–47 to simplify the template.


57-69: Simplify name concatenation with a join operation
Instead of multiple if and printf calls, you can streamline construction by collecting non-empty parts into a list and joining with a hyphen. For example:

- {{- $stack_name := "" -}}
- {{- if ne $tenant "" -}}
-   {{- $stack_name = $tenant -}}
- {{- end -}}
- {{- if ne $env "" -}}
-   {{- $stack_name = printf "%s-%s" $stack_name $env -}}
- {{- end -}}
- {{- if ne $stage "" -}}
-   {{- $stack_name = printf "%s-%s" $stack_name $stage -}}
- {{- end -}}
- {{- $stack_name -}}
+ {{- $parts := list $tenant $env $stage | compact -}}
+ {{- join "-" $parts -}}

This reduces boilerplate and leverages Sprig/Gomplate filters for clarity.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between b6c2077 and 6708b5b.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (19)
  • go.mod (6 hunks)
  • internal/exec/stack_utils.go (1 hunks)
  • internal/exec/utils.go (2 hunks)
  • pkg/component/component_processor.go (3 hunks)
  • pkg/component/component_processor_test.go (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/atmos.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/catalog/c1.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/region/us-east-2.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/prod.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/tenant/plat.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/us-east-2.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/us-east-2.yaml (1 hunks)
  • website/docs/cli/configuration/configuration.mdx (1 hunks)
  • website/docs/cli/configuration/stacks.mdx (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
internal/exec/stack_utils.go (1)
pkg/schema/schema.go (2)
  • AtmosConfiguration (23-57)
  • Stacks (312-318)
internal/exec/utils.go (1)
pkg/schema/schema.go (3)
  • Components (307-310)
  • Stacks (312-318)
  • Workflows (320-323)
pkg/component/component_processor.go (3)
internal/exec/stack_utils.go (2)
  • GetStackNameTemplate (193-195)
  • GetStackNamePattern (188-190)
internal/exec/template_utils.go (1)
  • ProcessTmpl (26-63)
pkg/config/utils.go (1)
  • GetStackNameFromContextAndStackNamePattern (702-758)
🪛 GitHub Check: codecov/patch
pkg/component/component_processor.go

[warning] 86-88: pkg/component/component_processor.go#L86-L88
Added lines #L86 - L88 were not covered by tests


[warning] 93-95: pkg/component/component_processor.go#L93-L95
Added lines #L93 - L95 were not covered by tests


[warning] 97-99: pkg/component/component_processor.go#L97-L99
Added lines #L97 - L99 were not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Acceptance Tests (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (36)
tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/prod.yaml (1)

1-4: Looks good — stage mixin
This fixture correctly declares stage: prod with the proper schema reference.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/_defaults.yaml (1)

1-4: Looks good — org defaults
Defines namespace: acme under the correct schema and path.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/tenant/plat.yaml (1)

1-4: Looks good — tenant mixin
The tenant: plat variable is set correctly and matches the test scenario.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/region/us-east-2.yaml (1)

1-5: Looks good — region mixin
Defines region: us-east-2 and environment: ue2 with the proper schema header.

tests/fixtures/scenarios/stack-name-template/stacks/catalog/c1.yaml (1)

1-11: Looks good — component fixture
The Terraform component c1 is declared with its vars (a, b, c, d) and includes the correct schema.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml (1)

1-4: Looks good!

This fixture file properly defines a stage variable with the value dev and includes the correct schema reference for validation. It will help test the new stack name templating feature.

internal/exec/stack_utils.go (2)

192-195: Good implementation of the name template accessor

The new function correctly handles the stack name template retrieval with a pointer parameter, which is more efficient for the large atmosConfig struct (5976 bytes) than passing by value.


187-190: Minor comment formatting update looks good

The comment format update for consistency is appropriate.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/us-east-2.yaml (1)

1-6: Proper fixture configuration for the stack template feature

This manifest correctly imports the necessary components for testing the stack name template functionality, including defaults, regional configuration, and catalog entries.

website/docs/cli/configuration/configuration.mdx (1)

616-623: Improved documentation format and content

The change from paragraph format to a bulleted list improves readability, and the addition of explicit references to "Go text/template functions" and "Atmos Template Functions" with documentation links provides valuable context for users implementing the new stack name templating feature.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/_defaults.yaml (1)

1-5: Defaults file structure looks good

The defaults file correctly establishes schema validation and imports the appropriate parent defaults and stage mixin. This follows the hierarchical configuration pattern needed for stack name templating.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/_defaults.yaml (1)

1-5: Tenant defaults configuration is properly structured

The tenant-level defaults file correctly imports the organization defaults and tenant mixin, establishing the necessary configuration hierarchy for the stack name template testing.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/_defaults.yaml (1)

1-5: Development environment defaults are correctly configured

The development environment defaults file properly imports the tenant defaults and development stage mixin, continuing the configuration hierarchy needed for stack name templating.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/us-east-2.yaml (1)

1-6: Stack manifest imports are well structured

The stack manifest correctly composes the configuration by importing the appropriate defaults, region mixin, and catalog component, completing the configuration needed for testing the stack name template feature.

internal/exec/utils.go (1)

647-653: CLI configuration moved to end of processing flow

The Atmos CLI configuration is now added to the component section after all CLI variables and stack processing are complete, which is a logical improvement. This ensures all processing is finished before the configuration is integrated.

This change supports the new stack name template feature by ensuring the configuration is fully processed and available when needed.

pkg/component/component_processor_test.go (1)

185-203: Good test coverage for the new stack name template feature.

The new test function clearly validates the core functionality of the stack name template feature by ensuring that variables from the context are correctly processed and accessible in the component map.

website/docs/cli/configuration/stacks.mdx (3)

104-107: Clear explanation of supported template functions.

The documentation now explicitly lists all supported function libraries, providing users with a comprehensive understanding of available templating capabilities.


140-212: Excellent documentation with detailed example.

The added example thoroughly demonstrates the hierarchical validation pattern and provides clear explanations of the template logic. The sample input and expected output make it easy for users to understand how the feature works in practice.


213-219: Clear guidance on template vs pattern precedence.

The note about using either name_pattern or name_template is well-structured and makes the precedence relationship clear.

go.mod (8)

35-35: Dependency update: go-containerregistry.

Updated from v0.20.3 to v0.20.4.


76-76: Dependency update: google.golang.org/api.

Updated from v0.233.0 to v0.234.0.


89-89: Dependency update: cloud.google.com/go/compute/metadata.

Updated from v0.6.0 to v0.7.0.


162-162: Dependency update: docker/cli.

Updated from v27.5.0+incompatible to v28.1.1+incompatible.


164-164: Dependency update: docker/docker-credential-helpers.

Updated from v0.8.2 to v0.9.3.


291-291: Dependency update: vbatts/tar-split.

Updated from v0.11.6 to v0.12.1.


326-326: Dependency update: golang.org/x/tools.

Updated from v0.31.0 to v0.33.0.


330-330: Dependency update: google.golang.org/genproto/googleapis/rpc.

Updated to a newer commit.

pkg/component/component_processor.go (2)

12-12: Good addition of a descriptive error variable.

The new error variable provides a clear message about the configuration requirement.


68-70: Proper implementation of template preference.

The code correctly retrieves both template and pattern values, with template taking precedence in the following logic.

tests/fixtures/scenarios/stack-name-template/atmos.yaml (7)

1-2: Validate the base path usage
Using base_path: "./" is functionally identical to ".". If consistency with other fixtures is desired, consider switching to base_path: ".". Otherwise, this is correct.


3-10: Terraform component flags look good
The components.terraform section correctly defines the paths and boolean flags for init, reconfigure, and backend file generation. Ensure these keys match the schema in internal/exec/stack_utils.go.


11-24: Stacks inclusion/exclusion and comments are clear
The included_paths / excluded_paths arrays and accompanying environment‐variable comments accurately document override options. The note about name_pattern precedence is also clear.


25-34: Documentation pointers for name_template are comprehensive
Links to Atmos, Sprig, and Gomplate functions—and the YAML multiline guide—provide all the context a user needs. Nicely done.


35-43: Namespace requirement enforcement is correct
The first guard (if eq $ns "") ensures namespace is always set before proceeding. This aligns with the design that namespace is mandatory, even if not used in the final name.


49-55: Environment and stage dependency checks are solid
The checks for .vars.environment and .vars.stage correctly enforce hierarchical dependencies on prior variables. Error messages are clear and user-friendly.


71-74: Verify log level casing
The level: Info entry may need to match the Atmos parser’s expected casing (e.g. info vs. Info). Please confirm that uppercase Info is accepted or adjust to the canonical lowercase form.

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)
tests/fixtures/scenarios/stack-name-template/atmos.yaml (1)

34-70: Streamline template conditional logic
The initial if eq $ns "" check (lines 41–43) preempts the more specific tenant/env/stage validations, making them unreachable. Consider reordering or consolidating these checks to preserve granular errors. For example:

@@ name_template: |-
-   {{- if eq $ns "" -}}
-     {{- fail "Error: 'namespace' is required." -}}
-   {{- end -}}
+   {{- if ne $tenant "" -}}
+     {{- if eq $ns "" -}}
+       {{- fail "Error: 'tenant' requires 'namespace'." -}}
+     {{- end -}}
+   {{- end -}}
+   {{- if ne $env "" -}}
+     {{- if or (eq $tenant "") (eq $ns "") -}}
+       {{- fail "Error: 'environment' requires 'tenant' and 'namespace'." -}}
+     {{- end -}}
+   {{- end -}}
+   {{- if ne $stage "" -}}
+     {{- if or (eq $env "") (eq $tenant "") (eq $ns "") -}}
+       {{- fail "Error: 'stage' requires 'environment', 'tenant', and 'namespace'." -}}
+     {{- end -}}
+   {{- end -}}
+   {{- if eq $ns "" -}}
+     {{- fail "Error: 'namespace' is required." -}}
+   {{- end -}}

This arrangement surfaces the specific dependency errors before the catch-all namespace requirement.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between b6c2077 and 6708b5b.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (19)
  • go.mod (6 hunks)
  • internal/exec/stack_utils.go (1 hunks)
  • internal/exec/utils.go (2 hunks)
  • pkg/component/component_processor.go (3 hunks)
  • pkg/component/component_processor_test.go (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/atmos.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/catalog/c1.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/region/us-east-2.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/prod.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/mixins/tenant/plat.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/us-east-2.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/_defaults.yaml (1 hunks)
  • tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/us-east-2.yaml (1 hunks)
  • website/docs/cli/configuration/configuration.mdx (1 hunks)
  • website/docs/cli/configuration/stacks.mdx (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
internal/exec/stack_utils.go (1)
pkg/schema/schema.go (2)
  • AtmosConfiguration (23-57)
  • Stacks (312-318)
internal/exec/utils.go (1)
pkg/schema/schema.go (3)
  • Components (307-310)
  • Stacks (312-318)
  • Workflows (320-323)
pkg/component/component_processor.go (3)
internal/exec/stack_utils.go (2)
  • GetStackNameTemplate (193-195)
  • GetStackNamePattern (188-190)
internal/exec/template_utils.go (1)
  • ProcessTmpl (26-63)
pkg/config/utils.go (1)
  • GetStackNameFromContextAndStackNamePattern (702-758)
🪛 GitHub Check: codecov/patch
pkg/component/component_processor.go

[warning] 86-88: pkg/component/component_processor.go#L86-L88
Added lines #L86 - L88 were not covered by tests


[warning] 93-95: pkg/component/component_processor.go#L93-L95
Added lines #L93 - L95 were not covered by tests


[warning] 97-99: pkg/component/component_processor.go#L97-L99
Added lines #L97 - L99 were not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Acceptance Tests (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (28)
go.mod (1)

35-35:

❓ Verification inconclusive

Dependency version bumps look solid.
The upgraded modules (e.g., go-containerregistry, docker/cli, x/tools, etc.) align with the new templating logic and test fixtures. Please verify that no breaking changes were introduced in these patch/minor releases.

Also applies to: 76-76, 89-89, 162-162, 164-164, 291-291, 326-326, 330-330


🏁 Script executed:

#!/bin/bash
# Verify module integrity and run full test suite
go mod tidy
go test ./...

Length of output: 64734


Integration tests require Terraform & atmos binary
Dependency version bumps look solid, but running go test ./... in a clean environment fails due to missing executables needed by integration tests. Please verify that in your CI/local setup:

  • Terraform is installed and on your $PATH.
  • The atmos CLI binary is built and discoverable (e.g., go build ./cmd/atmos).
    Then re-run:
go mod tidy  
go test ./...  

Applies also to bump lines 76, 89, 162, 164, 291, 326, 330.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/prod.yaml (1)

1-4: Production stage mixin is configured correctly.
The schema reference and vars.stage: prod declaration follow the expected pattern for staging mixins.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/_defaults.yaml (1)

1-4: Organization defaults look good.
Defining namespace: acme with the correct schema header sets up the right context for tenant and environment variables.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/_defaults.yaml (1)

3-6:

✅ Verification successful

Verify the existence of the referenced stage mixin.
This defaults file imports mixins/stage/dev, but the dev mixin wasn’t included in the provided fixtures. Please confirm that tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml exists and is correctly named.


🏁 Script executed:

#!/bin/bash
# Check for the dev stage mixin file
fd --base-directory tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage dev.yaml

Length of output: 104


Stage dev mixin found
The tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml file exists as expected, so the import is correct. No changes needed.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/tenant/plat.yaml (1)

1-4: Tenant mixin is well-defined.
The vars.tenant: plat entry with the proper schema annotation fits the scenario’s requirements.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/_defaults.yaml (1)

1-5: Structure looks good.

Clean YAML structure with proper schema reference and imports. This defaults file correctly establishes the configuration hierarchy by importing both the parent organization defaults and the production stage mixin.

tests/fixtures/scenarios/stack-name-template/stacks/catalog/c1.yaml (1)

1-10: Component definition is properly structured.

This component definition follows good practices with schema validation reference and a clean variable structure. The simple variables will work well for testing the new stack name template functionality.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/stage/dev.yaml (1)

1-4: Mixin looks good.

Properly structured mixin file that defines the stage variable for development environments. This will be useful in the stack name template context.

internal/exec/stack_utils.go (1)

192-195: Good function implementation.

The new function follows the existing pattern and properly returns the stack name template. Using a pointer parameter for the configuration is more efficient than passing by value, which aligns with best practices for large structs.

tests/fixtures/scenarios/stack-name-template/stacks/mixins/region/us-east-2.yaml (1)

1-5: Well-structured regional mixin with schema validation.

This mixin correctly defines regional variables (region: us-east-2 and environment: ue2) and includes schema validation reference. The file structure follows best practices for region-specific configuration that will be imported by stack manifests.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/prod/us-east-2.yaml (1)

1-6: Well-structured stack manifest with proper imports.

This stack manifest correctly imports defaults, region-specific mixins, and components in a clean hierarchical structure. The imports follow a logical pattern from organizational defaults to regional specifics to catalog components.

tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/_defaults.yaml (1)

1-5: Proper tenant defaults configuration with schema validation.

This defaults file correctly imports organizational defaults and tenant-specific mixins, establishing a proper inheritance chain for the platform tenant. The schema validation reference is correctly included.

website/docs/cli/configuration/configuration.mdx (1)

616-623: Improved documentation clarity for template functions.

The documentation has been enhanced by transforming a paragraph into a bulleted list and adding "Atmos Template Functions" with its link. This change improves readability and properly documents the new templating capabilities.

internal/exec/utils.go (2)

310-311: Grammatical improvement in comment.

The addition of the definite article "the" improves the grammar of the comment.


647-654: Properly moved CLI config map assignment to final stage.

The atmos_cli_config map is now correctly added to the component section at the end of the function, just before it returns. This ensures that all the necessary configuration context is included, which is essential for the new stack name template processing.

The map includes all relevant configuration sections:

  • base_path
  • components
  • stacks (which now contains the new name_template field)
  • workflows
tests/fixtures/scenarios/stack-name-template/stacks/orgs/acme/plat/dev/us-east-2.yaml (1)

1-6: Well-structured test fixture for stack name template scenario.

The manifest correctly follows Atmos schema validation pattern and imports the expected components for testing the stack name template functionality.

pkg/component/component_processor_test.go (1)

185-203: Good test coverage for the new stack name template functionality.

This test effectively validates that the component processor correctly handles stack name templates by:

  1. Setting up test variables (component, namespace, tenant, environment, stage)
  2. Verifying that these variables are correctly maintained in the processed component

The test ensures that stack names created from templates properly propagate context variables to components.

pkg/component/component_processor.go (3)

12-12: Appropriate error definition for missing configuration.

Clear error message explaining that either stacks.name_pattern or stacks.name_template must be specified in the configuration.


68-70: Good implementation for retrieving stack name configuration.

Correctly retrieves both template and pattern configurations to determine which approach to use for stack name resolution.


72-100:

❓ Verification inconclusive

Well-structured logic for stack name resolution.

The switch statement properly implements the priority logic between template and pattern:

  1. Tries to use the template first if available
  2. Falls back to pattern if template is not provided
  3. Returns an error if neither is configured

The template context creation is clean and includes all necessary variables.


🏁 Script executed:

#!/bin/bash
# Check for test coverage of error paths in the switch statement

# Find uncovered error paths 
rg -A 2 "if err != nil" pkg/component/component_processor.go | grep -B 1 "return nil, err" | grep -v "^--$"

Length of output: 290


Let’s verify whether there are tests covering those error paths in pkg/component:


🏁 Script executed:

#!/bin/bash
# List test files in pkg/component
find pkg/component -type f -name "*_test.go"

# Search tests for the key functions and error
rg -n "ProcessTmpl" pkg/component --glob "*_test.go"
rg -n "GetStackNameFromContextAndStackNamePattern" pkg/component --glob "*_test.go"
rg -n "ErrMissingStackNameTemplateAndPattern" pkg/component --glob "*_test.go"

Length of output: 304


Verify error-path test coverage

It looks like pkg/component/component_processor_test.go doesn’t reference ProcessTmpl, GetStackNameFromContextAndStackNamePattern, or ErrMissingStackNameTemplateAndPattern. Please confirm that tests cover all three error branches in the switch:

  • Template resolution error (ProcessTmpl path)
  • Pattern resolution error (GetStackNameFromContextAndStackNamePattern path)
  • Missing both template and pattern error (ErrMissingStackNameTemplateAndPattern)

If these aren’t covered, consider adding tests to exercise each error case.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 86-88: pkg/component/component_processor.go#L86-L88
Added lines #L86 - L88 were not covered by tests


[warning] 93-95: pkg/component/component_processor.go#L93-L95
Added lines #L93 - L95 were not covered by tests


[warning] 97-99: pkg/component/component_processor.go#L97-L99
Added lines #L97 - L99 were not covered by tests

website/docs/cli/configuration/stacks.mdx (3)

104-107: Comprehensive documentation of supported template functions.

Clearly lists all supported template function libraries, making it easier for users to understand what's available.


140-212: Excellent detailed example with hierarchical validation.

The example template demonstrates:

  1. Validation of dependencies between variables
  2. Progressive stack name construction
  3. Proper error messages for invalid configurations

This provides users with a practical implementation pattern they can adapt for their needs.


213-219: Clear documentation of template priority.

The note effectively communicates that users should use either name_pattern or name_template, with name_template taking precedence when both are specified.

tests/fixtures/scenarios/stack-name-template/atmos.yaml (5)

1-2: Confirm base_path resolution
The base_path: "./" setting is clear, but ensure the test harness interprets it relative to the scenario directory.


3-10: Approve Terraform component configuration
Flags like deploy_run_init, apply_auto_approve, and auto_generate_backend_file are set deliberately to control behavior in tests. Looks consistent with other scenarios.


11-18: Verify stack path globs
The included_paths and excluded_paths patterns seem correct. Ensure that your fixture stacks under tests/fixtures/scenarios/stack-name-template/stacks/ match these globs.


19-33: Document precedence of name_template over name_pattern
The comments clearly explain that name_template takes priority. This matches the PR objectives for backward compatibility and user guidance.


71-74: Approve logging configuration
Writing logs to stderr at the Info level aligns with other test fixtures. Please confirm the logging parser accepts the capitalized level.

@aknysh aknysh requested a review from osterman May 22, 2025 05:15
@aknysh aknysh requested review from milldr and Benbentwo May 22, 2025 05:15
@aknysh aknysh merged commit 0f8a3c3 into main May 22, 2025
51 checks passed
@aknysh aknysh deleted the name-template-for-utils-provider branch May 22, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-release Do not create a new release (wait for additional code changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants