feat(stacks): support global-scope metadata defaults - #2808
Conversation
Add a restricted allowlist of `metadata` fields (labels, tags, custom, enabled, locked, terraform_workspace_pattern) that can be set once at the stack-manifest root and deep-merged into every component's own metadata, with the lowest precedence (global -> metadata.inherits base chain -> component-local, which always wins). Previously a top-level `metadata:` block was silently ignored; setting a component-identity field (component/inherits/type/name/terraform_workspace) at global scope is now a hard validation error instead of a silent no-op, both at runtime and via the manifest JSON Schema. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
|
Note Release Documentation Complete ✅
Thank you! |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Add the stack-wide metadata defaults milestone to the discoverability initiative, linked to PR #2808 and the global-metadata-defaults changelog post. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds root-level stack metadata defaults with schema validation, allowlisted fields, propagation to built-in and custom components, precedence-aware merging, tests, and documentation. ChangesGlobal metadata defaults
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant StackConfig
participant ProcessStackConfig
participant ComponentProcessor
StackConfig->>ProcessStackConfig: provide root metadata
ProcessStackConfig->>ComponentProcessor: pass GlobalMetadata
ComponentProcessor->>ComponentProcessor: merge global, inherited, and local metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
website/docs/stacks/components/metadata.mdx (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify: no
terraform:/helmfile:/packer:metadata defaults feature exists.The schema's
terraform/helmfile/packerdefinitions don't expose ametadataproperty at all (additionalProperties: false, nometadatakey), so this parenthetical implies a capability that isn't schema-supported rather than one that's merely restricted to identity fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/docs/stacks/components/metadata.mdx` at line 39, Update the component-identity fields documentation to remove the unsupported claim about setting metadata under terraform, helmfile, or packer defaults; describe only the global-level restriction unless schema-supported nested metadata defaults are actually available.internal/exec/stack_processor_process_stacks.go (1)
1296-1308: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueShallow merge for custom-component metadata (no nested deep-merge).
Nested fields like
metadata.labelswill be fully overwritten by component-local values here rather than deep-merged, unlike the mergo-based merge for built-in component types. This matches the existing shallow-merge pattern already used forvars/settings/envin this same function, so it's consistent with established conventions here, just worth being aware of for custom component types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/exec/stack_processor_process_stacks.go` around lines 1296 - 1308, Preserve the existing shallow merge behavior in the custom-component metadata handling around componentMetadata: copy global metadata first, then let component-local metadata replace matching top-level keys, including entire nested values such as labels. Do not introduce deep-merge behavior or alter the established vars/settings/env merge conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@website/docs/stacks/components/metadata.mdx`:
- Line 285: Update the metadata.custom inheritance documentation in the manifest
schema referenced by the stack-config schema so it matches the implementation
and docs: remove the statement that custom is not inherited, or explicitly
include custom in the documented inherited metadata behavior. Preserve the
existing deep-merge and derived-component override semantics.
---
Nitpick comments:
In `@internal/exec/stack_processor_process_stacks.go`:
- Around line 1296-1308: Preserve the existing shallow merge behavior in the
custom-component metadata handling around componentMetadata: copy global
metadata first, then let component-local metadata replace matching top-level
keys, including entire nested values such as labels. Do not introduce deep-merge
behavior or alter the established vars/settings/env merge conventions.
In `@website/docs/stacks/components/metadata.mdx`:
- Line 39: Update the component-identity fields documentation to remove the
unsupported claim about setting metadata under terraform, helmfile, or packer
defaults; describe only the global-level restriction unless schema-supported
nested metadata defaults are actually available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2561841b-b2a6-4b15-8db1-b54897cf2c3a
📒 Files selected for processing (11)
errors/errors.gointernal/exec/stack_processor_merge.gointernal/exec/stack_processor_merge_test.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_processor_process_stacks_helpers.gointernal/exec/stack_processor_process_stacks_test.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema_global_metadata_test.gopkg/datafetcher/schema_section_coverage_test.gowebsite/blog/2026-07-25-global-metadata-defaults.mdxwebsite/docs/stacks/components/metadata.mdx
Reframe the intro and problem statement around the agreed-upon common case (shared labels/tags) instead of asserting that locking every component in a stack is a widely shared need; keep enabled/locked as a secondary capability with a narrower, concrete example (decommissioning a legacy stack). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The manifest schema's metadata.custom description claimed it was "not inherited by derived components," but internal/exec/stack_processor_utils.go only excludes 'inherits' and 'type' from the metadata.inherits base-component merge — custom IS inherited, matching the docs and existing tests. Also correct the same copy-pasted phrasing in the new global_metadata.custom description to reflect actual precedence instead of the per-component wording it was copied from. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2808 +/- ##
=======================================
Coverage 81.84% 81.84%
=======================================
Files 1793 1793
Lines 173173 173207 +34
=======================================
+ Hits 141725 141769 +44
+ Misses 23649 23640 -9
+ Partials 7799 7798 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The custom (non-built-in) component type passthrough loop merges stack-root global metadata the same way built-in types do, but the loop body that copies global metadata keys into a custom component's metadata was never exercised by any existing test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/exec/stack_processor_process_stacks_test.go`:
- Around line 1331-1344: The test fixture and expectations around
custom-component metadata in the relevant stack-processing test do not verify
deep merging of nested labels. Add a non-conflicting global label and a
local-only label, then assert the merged result retains both labels while the
nested local org value overrides the global value; update the corresponding
expectation near the alternate referenced section as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 15148ec2-c83a-4592-aa79-80b2341d8a8e
📒 Files selected for processing (1)
internal/exec/stack_processor_process_stacks_test.go
The custom (non-built-in) component type passthrough loop shallow-merged stack-root global metadata into a component's own metadata, so nested maps like metadata.labels were wholesale-replaced by a component's local value instead of deep-merged. This was inconsistent with built-in component types (terraform/helmfile/packer/ansible/kubernetes/helm), which use m.Merge and retain non-conflicting keys from both sides. Switch the custom-type metadata merge to m.Merge, matching the pattern already used elsewhere in this file, and extend the existing test to assert that non-conflicting global and local label keys both survive while the conflicting key resolves to the local value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
|
These changes were released in v1.225.0-rc.1. |
what
metadatafields (labels,tags,custom,enabled,locked,terraform_workspace_pattern) at the stack-manifest root, deep-merged into every component's ownmetadataas a stack-wide default.metadata.inheritsbase-component chain → the component's own localmetadata:block, which always wins.component,inherits,type,name,terraform_workspace) remain component-only; setting one of these at global scope is now a hard validation error, both at runtime and via the manifest JSON Schema, instead of a silent no-op.metadata.mdxdocs and adds a changelog post explaining the new global scope.why
metadata:block (e.g. in_defaults.yaml) was previously accepted by the schema but never applied —metadata.labels/metadata.tags/etc. set there silently did nothing, which is worse than an error, since users had no signal their config wasn't taking effect.metadatablock into each component definition instead of declaring it once.references
Summary by CodeRabbit
New Features
metadata:defaults that are deep-merged into each component’smetadata.metadata.inheritschain (when enabled) → component-local metadata.Validation & Tests
metadata(onlylabels,tags,custom,enabled,locked,terraform_workspace_pattern).Documentation