docs: Clarify imports, inheritance, and variable merge order#2146
Open
docs: Clarify imports, inheritance, and variable merge order#2146
Conversation
Improved documentation around stack configuration patterns and variable resolution: - Added 'Blueprint Configuration' pattern for reusable deployment templates - Added 'Variable Merge Order' guide explaining scope levels and import precedence - Clarified overrides are file-scoped and when they're needed - Updated multi-cloud examples with clearer directory hierarchy - Updated variable scope list to include inheritance and overrides - Added cross-references between related documentation Fixes variable resolution confusion by documenting that scope level beats import order - component-level vars override global-level vars regardless of which file defines them. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Address PR review feedback: - Remove standalone merge-order.mdx — its concepts belong in existing docs - Simplify the scope-level tip in imports-basics.mdx - Add "Scope Levels Beat Import Order" section with concrete example - Add debugging tip for atmos describe component --provenance - Remove all cross-references to the deleted page Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
osterman
added
the
no-release
Explain that Azure resource groups are Terraform-managed resources (not directory-level organizational boundaries) and that locations can be files or directories depending on deployment complexity. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Blueprint Configuration design-pattern doc, restructures multi-cloud and imports/overrides/vars guidance, introduces a native-terraform example with Atmos config and mock Terraform component, updates many documentation anchors and embeds, and adds tests/snapshots for the example. Changes
Sequence Diagram(s)(Skipped — changes are documentation, examples, and tests without new multi-component runtime control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Add a "What Is a Blueprint?" section that formally defines the term and distinguishes it from catalogs (single-component defaults) and archetypes (component variants). Note that the example is one opinionated implementation of the pattern. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@website/docs/design-patterns/stack-organization/multi-cloud-configuration.mdx`:
- Around line 165-166: The opening sentence claiming the directory layout
"mirrors Management Groups → Subscriptions → Resource Groups → Locations" is
misleading; revise the opener to say the directory layout is inspired by Azure's
Management Groups and Subscriptions hierarchy but does not literally model
Resource Groups as directory levels or Locations as separate folders. Update the
paragraph around the phrase "Only the first folder (`management-groups/`) is
named after the resource type..." and add a clarifying sentence that resource
groups are encoded as names inside subscription folders and that location is
used only as a logical file boundary (refer to the existing wording about
resource groups not being directory levels and location being the chosen file
boundary) so the intro and lines 188–190 consistently state the intended
mapping.
In `@website/docs/learn/imports-basics.mdx`:
- Around line 338-339: The example is inconsistent: the text shows the stack
file `stacks/deploy/prod-us.yaml` but the debug command uses `-s prod-us-east-1`
(`atmos describe component vpc -s prod-us-east-1`); either make the command
match the shown stack name (e.g., change `-s` value to `deploy/prod-us` or
`prod-us` to mirror `stacks/deploy/prod-us.yaml`) or add a short clarifying note
that `-s` takes a stack name (naming template like `prod-{region}` such as
`prod-us-east-1`) so readers know why the command differs from the file path.
In `@website/docs/stacks/overrides.mdx`:
- Around line 13-14: The intro for the "overrides" section incorrectly limits
overrides to "variable overrides" and contrasts them only with top-level `vars`;
update the paragraph that references `overrides` and `vars` to describe
file-scoped overrides more broadly, mentioning that `overrides` apply to
components defined in the current manifest file and its imports and can affect
`command`, `env`, `hooks`, `providers`, `settings`, and `vars` (not just
variables), while top-level `vars` remain stack-wide after import resolution.
In `@website/docs/stacks/vars.mdx`:
- Around line 88-90: Replace the non-runnable example text with a placeholder,
runnable CLI example so the tip is accurate; update the sentence that references
"atmos describe component --provenance" to show a concrete placeholder form such
as using the `atmos describe` command with explicit component and stack
placeholders and the `--provenance` flag (e.g., atmos describe <component>
<stack> --provenance or atmos describe <component> --stack <stack>
--provenance), so readers can copy/paste and run it and the documentation stays
in sync with the CLI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b77e2fa5-fe7f-4048-8660-813cb2d160f9
📒 Files selected for processing (6)
website/docs/design-patterns/stack-organization/blueprint-configuration.mdxwebsite/docs/design-patterns/stack-organization/multi-cloud-configuration.mdxwebsite/docs/howto/inheritance.mdxwebsite/docs/learn/imports-basics.mdxwebsite/docs/stacks/overrides.mdxwebsite/docs/stacks/vars.mdx
website/docs/design-patterns/stack-organization/multi-cloud-configuration.mdx
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2146 +/- ##
=======================================
Coverage 77.29% 77.29%
=======================================
Files 960 960
Lines 91088 91088
=======================================
+ Hits 70410 70411 +1
+ Misses 16593 16591 -2
- Partials 4085 4086 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…bedFile integration Create a runnable example under examples/native-terraform/ demonstrating migration from vanilla Terraform to Atmos. The example shows two strategies side by side: dev stack uses !include to keep existing .tfvars files, prod stack is fully converted to native YAML with an explicit stack name. Update the migration doc to embed real example files via EmbedFile instead of inline snippets, and replace the Hello World section with a VPC migration walkthrough using the actual example. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
website/docs/stacks/overrides.mdx (1)
13-13:⚠️ Potential issue | 🟡 MinorBroaden the intro beyond
vars.This still describes
overridesas variable-only, but the page now documentscommand,env,hooks,providers, andsettingstoo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/stacks/overrides.mdx` at line 13, The intro currently limits "overrides" to file-scoped variable overrides; update the opening sentence to state that "overrides" are file-scoped overrides for multiple component fields (not just vars) and explicitly list the supported fields (command, env, hooks, providers, settings) so readers understand it applies to components in the current manifest file and its imports; locate the paragraph describing "overrides" and replace the single-var wording with a concise line such as "The `overrides` section provides file-scoped overrides (e.g., command, env, hooks, providers, settings) that apply only to components defined in the current manifest file and its imports," keeping the rest of the content intact.
🧹 Nitpick comments (1)
website/docs/migration/native-terraform.mdx (1)
319-319: Pick one stack-naming pattern in the generic snippets.
name: dev/name: prodare redundant for those filenames, so they read like required overrides and blur the laterprod.yaml -> productionexample. I'd either drop these lines or make the override example useproductionconsistently.Also applies to: 335-335, 388-388
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/migration/native-terraform.mdx` at line 319, The snippets use inconsistent stack-naming patterns — lines showing `name: dev` and `name: prod` confuse readers and conflict with the later `prod.yaml -> production` example; pick one pattern and apply it consistently across the three occurrences (the `name: dev`/`name: prod` snippets and the `prod.yaml -> production` example) by either removing the redundant `name:` lines in the generic snippets or changing them to `production` so the override example and filenames match consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/native-terraform/components/terraform/vpc/main.tf`:
- Around line 1-11: Add a required_providers block to pin the null provider
version and commit the generated .terraform.lock.hcl so Terraform init is
reproducible; update the module containing resource "null_resource" "vpc" (and
any related vars like var.cidr_block, var.environment, var.enable_dns_hostnames,
var.tags) to include a required_providers entry for hashicorp/null with a
specific version constraint (e.g., = x.y.z or >=x.y,<x+1) and run terraform init
to generate and then add .terraform.lock.hcl to source control.
In `@examples/native-terraform/README.md`:
- Around line 16-26: The README's unlabeled fenced code block containing the
directory tree (the block showing lines like "├── atmos.yaml", "├──
components/terraform/vpc/", "└── stacks/") should be labeled to satisfy MD040;
change the opening fence from ``` to ```text or ```plaintext so the
directory-tree fence is explicitly typed and the linter stops flagging it.
In `@website/docs/migration/native-terraform.mdx`:
- Line 496: The EmbedFile invocation for the Terraform vars example is missing
an explicit language, causing dev.tfvars to render as plain text; update the
EmbedFile component that embeds dev.tfvars by adding language="hcl" (e.g.,
EmbedFile ... language="hcl") so the example renders with HCL highlighting and
remains readable.
In `@website/docs/stacks/overrides.mdx`:
- Around line 309-312: Update the wording in the important-note block that
currently says "Inline wins over imported overrides when they conflict" to
qualify scope precedence: change it to state that "Inline overrides win over
imported overrides only when they are at the same scope level; merge-order rules
still apply so an imported overrides file from a narrower/closer scope can
override an inline global value (see merge-order section)". Edit the paragraph
containing "Imported overrides apply..." / "Inline overrides apply..." / "Inline
wins..." to replace the last sentence with this clarified phrasing so it
accurately reflects scope vs merge-order behavior.
- Around line 24-26: The tip incorrectly implies that stacks importing a single
blueprint never need `overrides`; update the tip text around `overrides` to
scope it to variable-only cases by stating that plain `vars` at the same scope
level can substitute for `overrides` only when the only customizations are
`vars`. Mention that when customizing other manifest elements (e.g., `command`,
`env`, `hooks`, `providers`, `settings`) you still need `overrides`, and adjust
the sentence that references the Blueprint Configuration pattern so it clearly
applies only to `vars`-only scenarios.
---
Duplicate comments:
In `@website/docs/stacks/overrides.mdx`:
- Line 13: The intro currently limits "overrides" to file-scoped variable
overrides; update the opening sentence to state that "overrides" are file-scoped
overrides for multiple component fields (not just vars) and explicitly list the
supported fields (command, env, hooks, providers, settings) so readers
understand it applies to components in the current manifest file and its
imports; locate the paragraph describing "overrides" and replace the single-var
wording with a concise line such as "The `overrides` section provides
file-scoped overrides (e.g., command, env, hooks, providers, settings) that
apply only to components defined in the current manifest file and its imports,"
keeping the rest of the content intact.
---
Nitpick comments:
In `@website/docs/migration/native-terraform.mdx`:
- Line 319: The snippets use inconsistent stack-naming patterns — lines showing
`name: dev` and `name: prod` confuse readers and conflict with the later
`prod.yaml -> production` example; pick one pattern and apply it consistently
across the three occurrences (the `name: dev`/`name: prod` snippets and the
`prod.yaml -> production` example) by either removing the redundant `name:`
lines in the generic snippets or changing them to `production` so the override
example and filenames match consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 282d892e-7f2d-4a99-9c6c-6ce482d96bf1
📒 Files selected for processing (25)
examples/native-terraform/.gitignoreexamples/native-terraform/README.mdexamples/native-terraform/atmos.yamlexamples/native-terraform/components/terraform/vpc/main.tfexamples/native-terraform/components/terraform/vpc/outputs.tfexamples/native-terraform/components/terraform/vpc/variables.tfexamples/native-terraform/legacy/dev.tfvarsexamples/native-terraform/legacy/prod.tfvarsexamples/native-terraform/stacks/_defaults.yamlexamples/native-terraform/stacks/dev.yamlexamples/native-terraform/stacks/prod.yamltests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_dev_(native-terraform_example).stderr.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_dev_(native-terraform_example).stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_prod_(invalid_-_native-terraform_example).stderr.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_prod_(invalid_-_native-terraform_example).stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_production_(native-terraform_example).stderr.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_production_(native-terraform_example).stdout.goldentests/snapshots/TestCLICommands_atmos_list_stacks_(native-terraform_example).stderr.goldentests/snapshots/TestCLICommands_atmos_list_stacks_(native-terraform_example).stdout.goldentests/test-cases/native-terraform-example.yamlwebsite/docs/design-patterns/component-catalog/template.mdxwebsite/docs/migration/native-terraform.mdxwebsite/docs/stacks/imports.mdxwebsite/docs/stacks/overrides.mdxwebsite/docs/templates/templates.mdx
✅ Files skipped from review due to trivial changes (7)
- tests/snapshots/TestCLICommands_atmos_list_stacks_(native-terraform_example).stderr.golden
- examples/native-terraform/legacy/dev.tfvars
- tests/snapshots/TestCLICommands_atmos_list_stacks_(native-terraform_example).stdout.golden
- examples/native-terraform/legacy/prod.tfvars
- tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_dev_(native-terraform_example).stderr.golden
- tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_production_(native-terraform_example).stderr.golden
- website/docs/templates/templates.mdx
…mponent dir - Replace manual EmbedFile tabs with EmbedExample component in native-terraform migration guide - Remove name_template from examples, use imperative `name` key in stack manifests - Move tfvars from legacy/ to components/terraform/vpc/envs/ to match real-world layout - Update multi-cloud doc with provider-specific variable names and tips Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/docs/design-patterns/stack-organization/multi-cloud-configuration.mdx (1)
94-96: Consider adding a tip aboutname_templatevars.Azure (lines 258-260) and GCP (lines 372-374) include a tip that vars referenced in
name_templatemust be defined as Terraform variables. The AWS section uses the same pattern but lacks this tip. Adding it here would keep documentation consistent across all three clouds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/design-patterns/stack-organization/multi-cloud-configuration.mdx` around lines 94 - 96, Add a short tip to the AWS section stating that any vars referenced in the name_template (e.g., name_template: "{{.vars.stage}}-{{.vars.environment}}") must be declared as Terraform variables, mirroring the existing notes in the Azure and GCP sections; place the tip near the AWS name_template example and reference the specific symbols "name_template" and ".vars" so readers know to define "stage" and "environment" as Terraform variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@website/docs/design-patterns/stack-organization/multi-cloud-configuration.mdx`:
- Around line 94-96: Add a short tip to the AWS section stating that any vars
referenced in the name_template (e.g., name_template:
"{{.vars.stage}}-{{.vars.environment}}") must be declared as Terraform
variables, mirroring the existing notes in the Azure and GCP sections; place the
tip near the AWS name_template example and reference the specific symbols
"name_template" and ".vars" so readers know to define "stage" and "environment"
as Terraform variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 097abb75-2725-4302-9541-f5f7cacd094a
📒 Files selected for processing (6)
examples/native-terraform/README.mdexamples/native-terraform/components/terraform/vpc/envs/dev.tfvarsexamples/native-terraform/components/terraform/vpc/envs/prod.tfvarsexamples/native-terraform/stacks/dev.yamlwebsite/docs/design-patterns/stack-organization/multi-cloud-configuration.mdxwebsite/docs/migration/native-terraform.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- website/docs/migration/native-terraform.mdx
- examples/native-terraform/stacks/dev.yaml
- Fix Azure hierarchy description to not overclaim directory mirrors resource groups/locations - Use generic placeholder in imports-basics debug command - Broaden overrides intro to cover all overridable sections, not just vars - Add missing component/stack args to provenance command example - Pin hashicorp/null provider version in native-terraform example - Label directory-tree fence block in README - Add language="hcl" to .tfvars EmbedFile for syntax highlighting - Scope "don't need overrides" tip to vars-only cases - Qualify "inline wins" to same scope level with scope precedence note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n.tf Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_production_`(native-terraform_example).stdout.golden:
- Around line 161-163: The CLI is incorrectly escaping the '>' in provider
version constraints (producing "/u003e" instead of "~> 3.2"); locate the code
path that builds the snapshot JSON (e.g., the function responsible for
provider/version serialization such as serializeConstraints or the CLI
snapshot/toJson formatter) and stop performing incorrect manual replaces or
HTML-escaping of '>' characters. Fix by using a proper JSON serializer
(JSON.stringify) for the value or, if a manual escape is needed, produce a
correct Unicode escape ("\\u003e") or better yet emit the literal ">" in "~>
3.2" so the array element becomes "~> 3.2"; update the serializer (e.g.,
formatVersionConstraint / toJsonSnapshot) to return the unescaped constraint
string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66e36198-7bbc-484b-91ad-1a30a1bafe80
📒 Files selected for processing (2)
tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_dev_(native-terraform_example).stdout.goldentests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_production_(native-terraform_example).stdout.golden
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/snapshots/TestCLICommands_atmos_describe_component_vpc_-s_dev_(native-terraform_example).stdout.golden
...Commands_atmos_describe_component_vpc_-s_production_(native-terraform_example).stdout.golden
Show resolved
Hide resolved
The sanitizeOutput function's backslash-to-forward-slash regex for Windows path normalization was also corrupting JSON unicode escape sequences like \u003e, turning them into /u003e. Fix by protecting \uXXXX sequences with a placeholder before the replacement. Also fix pre-existing errorlint issues in pkg/git/git.go (%s -> %w). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The unicode escape protection regex ran after filepath.ToSlash, which on Windows converts ALL backslashes to forward slashes — turning \u003e into /u003e before the protection could save it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove unnecessary filepath.ToSlash() calls before filepath.Join() in vendor target path construction. On Windows, ToSlash converts backslashes to forward slashes before Join, creating mixed-separator paths that fail file operations (caused myapp2 vendor failure). Exclude docs.github.com from link checker — it intermittently returns 503s in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ConvertToJSON used EscapeHTML:true which escaped `>` as `\u003e` in JSON output, turning version constraints like `~> 3.2` into `~\u003e 3.2`. CLI JSON output is never embedded in HTML, so disable HTML escaping to emit literal characters. Update golden snapshots. Also remove unused copyToTarget function in vendor_utils.go (dead code from prior refactor that caused lint failure). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
…bal setting reference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
@coderabbitai what is not resolved? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 82526
In short, resolving the merge conflicts is the only hard blocker. Would you like me to trigger a fresh review once conflicts are resolved? 🧠 Learnings used |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
overridesare file-scoped (preventing cross-team leakage) vs regularvarswhich apply globallywhy
references
Related to improving Atmos documentation clarity around stack configuration patterns and variable resolution.
Summary by CodeRabbit
Documentation
New Features
Other