Skip to content

Conversation

petabook
Copy link

@petabook petabook commented Sep 17, 2025

Changes

Previously

Imported stack manifests had their Go templates processed only when len(context) > 0:

import:
  - path: catalog/aws-account.yaml.tmpl
    context:
      foo: bar

Now

If process_without_context is true, templates are processed even when context is empty. This makes it cleaner to trigger template processing:

import:
  - path: catalog/aws-account.yaml.tmpl

Default behavior remains unchanged unless explicitly opted-in.

New config option

  • Added templates.settings.import.process_without_context to the Atmos schema.
  • Defined under a new struct TemplateSettingsImport with a single boolean field process_without_context.
    Example usage in atmos.yaml:
templates:
  settings:
    import:
      process_without_context: true

Summary by CodeRabbit

  • New Features
    • Added an option to process templates in imported stack manifests even when no context is provided.
    • Introduced a new configuration path to control this behavior: templates.settings.import.process_without_context.
    • Enables more flexible and consistent template processing across imports when the setting is enabled.

@petabook petabook requested a review from a team as a code owner September 17, 2025 00:23
@mergify mergify bot added the triage Needs triage label Sep 17, 2025
Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

📝 Walkthrough

Walkthrough

Adds a new templates.settings.import.process_without_context flag to the schema and updates stack processing to allow Go template evaluation for imported manifests when this flag is true, even if the context is empty. Existing skip logic remains in place.

Changes

Cohort / File(s) Summary of Changes
Template settings schema
pkg/schema/schema.go
Added TemplateSettingsImport with process_without_context and introduced TemplatesSettings.Import field to expose it under templates.settings.import.
Stack manifest processing
internal/exec/stack_processor_utils.go
Broadened condition in ProcessYAMLConfigFile to process templates in imports when Templates.Settings.Import.ProcessWithoutContext is true or when context is non-empty, still respecting skipTemplatesProcessingInImports.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant StackProcessor
  participant Config

  Caller->>StackProcessor: ProcessYAMLConfigFile(file, context)
  StackProcessor->>Config: Read atmosConfig.Templates.Settings
  alt Import template processing allowed
    note over StackProcessor: Condition:<br/>!skipTemplatesProcessingInImports &&<br/>(Import.ProcessWithoutContext || len(context)>0)
    StackProcessor->>StackProcessor: Process Go templates in imported manifests
  else Skipped
    StackProcessor->>StackProcessor: Do not process templates in imports
  end
  StackProcessor-->>Caller: Result / error
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

minor

Suggested reviewers

  • kevcube
  • nitrocode
  • aknysh

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Allow processing of templates without context" succinctly and accurately describes the primary change in this PR: enabling template processing for imported manifests when the new templates.settings.import.process_without_context option is set, alongside the code change in stack_processor_utils.go. It is a single clear sentence that focuses on the main behavior change without extraneous details, making the intent obvious to reviewers. The title aligns with the raw changes (schema addition and logic update) and therefore appropriately summarizes the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

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 (2)
pkg/schema/schema.go (1)

307-310: Add Godoc for the new exported type.

Exported types require brief comments (golangci-lint/godot). Add a doc comment clarifying scope.

Apply this diff:

+// TemplateSettingsImport configures import template processing behavior.
+// When ProcessWithoutContext is true, Atmos will process Go templates in imported
+// stack manifests even when the provided context is empty.
 type TemplateSettingsImport struct {
  	ProcessWithoutContext bool `yaml:"process_without_context" json:"process_without_context" mapstructure:"process_without_context"`
 }
internal/exec/stack_processor_utils.go (1)

218-229: Confirm scope: this also enables processing for top‑level manifests with empty context.

With process_without_context=true, the root stack (initial call with nil context) will now have templates processed, which may surprise users expecting the flag to apply only to imports. If you intend to limit it to imports, gate on non‑nil context (imports pass a merged, non‑nil map) and update the comment.

Apply this minimal guard:

-	// Process `Go` templates in the imported stack manifest using the provided `context`
-	// https://atmos.tools/core-concepts/stacks/imports#go-templates-in-imports
-	if !skipTemplatesProcessingInImports && (atmosConfig.Templates.Settings.Import.ProcessWithoutContext || len(context) > 0) {
+	// Process Go templates in the imported stack manifest using the provided context.
+	// When templates.settings.import.process_without_context is true, also process imports even when context is empty.
+	// https://atmos.tools/core-concepts/stacks/imports#go-templates-in-imports
+	if !skipTemplatesProcessingInImports && (len(context) > 0 || (atmosConfig.Templates.Settings.Import.ProcessWithoutContext && context != nil)) {
 		stackManifestTemplatesProcessed, err = ProcessTmpl(relativeFilePath, stackYamlConfig, context, ignoreMissingTemplateValues)

If context can be nil for imports in some paths, consider adding an explicit isImport bool parameter to ProcessYAMLConfigFile and using that instead.

Want me to add tests that cover:

  • root manifest: empty context, flag on → no processing;
  • import manifest: empty context, flag on → processed?
📜 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 3e2ed8a and 139e64c.

📒 Files selected for processing (2)
  • internal/exec/stack_processor_utils.go (1 hunks)
  • pkg/schema/schema.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

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

**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults

**/*.go: All comments in Go code must end with periods (enforced by golangci-lint godot).
Wrap all returned errors using static errors from the errors package; never return dynamic errors directly.
Always bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for each external var.
Separate structured logging from UI output: use stderr for prompts/errors to user; stdout only for data; never use logging for UI.
Most text UI must go to stderr; only data/results to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry with telemetry.CaptureCmd or telemetry.CaptureCmdString and never capture user data.
Ensure cross-platform compatibility: prefer SDKs to external binaries, use filepath/os/runtime for portability, and add build constraints when needed.

Files:

  • internal/exec/stack_processor_utils.go
  • pkg/schema/schema.go
internal/exec/stack_processor_utils.go

📄 CodeRabbit inference engine (CLAUDE.md)

internal/exec/stack_processor_utils.go: Place core stack processing utilities here and reuse them.
Stack processing utilities should reside here; test multiple inheritance scenarios.

Files:

  • internal/exec/stack_processor_utils.go
🧠 Learnings (10)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#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: 2024-11-19T23:00:45.899Z
Learnt from: osterman
PR: cloudposse/atmos#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:

  • internal/exec/stack_processor_utils.go
📚 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:

  • internal/exec/stack_processor_utils.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to internal/exec/stack_processor_utils.go : Place core stack processing utilities here and reuse them.

Applied to files:

  • internal/exec/stack_processor_utils.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
PR: cloudposse/atmos#764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.

Applied to files:

  • internal/exec/stack_processor_utils.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to internal/exec/stack_processor_utils.go : Stack processing utilities should reside here; test multiple inheritance scenarios.

Applied to files:

  • internal/exec/stack_processor_utils.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to pkg/utils/yaml_utils.go : Prefer existing YAML processing utilities in pkg/utils/yaml_utils.go before adding new ones.

Applied to files:

  • internal/exec/stack_processor_utils.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to pkg/stack/** : Make stack processing changes in pkg/stack/ and validate inheritance/template rendering.

Applied to files:

  • internal/exec/stack_processor_utils.go
📚 Learning: 2025-09-07T15:13:17.831Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T15:13:17.831Z
Learning: Applies to **/*.{yaml,yml} : YAML configs should support Go templating using provided template functions.

Applied to files:

  • internal/exec/stack_processor_utils.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:

  • internal/exec/stack_processor_utils.go
🧬 Code graph analysis (1)
internal/exec/stack_processor_utils.go (1)
pkg/schema/schema.go (2)
  • Templates (278-280)
  • Settings (839-843)
⏰ 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 (1)
pkg/schema/schema.go (1)

289-290: LGTM: new import settings hook is well‑placed.

Tags and zero‑value semantics look good; backward compatibility preserved.

@mergify mergify bot removed the triage Needs triage label Sep 17, 2025
@aknysh aknysh added the no-release Do not create a new release (wait for additional code changes) label Sep 18, 2025
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