Uh oh!
There was an error while loading. Please reload this page.
feat(overlay-files): support {component} placeholder for pattern-discovered components - #269
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a {component} placeholder for project-level default-component-config.overlay-files to support pattern-based discovery of components and per-component overlay-file resolution, and updates schema/snapshots/docs accordingly.
Changes:
- Add projectconfig placeholder validation/splitting/substitution helpers and scope-based validation (
{component}required at project scope, forbidden elsewhere). - Add pattern-based component discovery from project-level
overlay-filesentries with resolver-level caching and deterministic unioning intoFindAllComponents. - Regenerate JSON schema / scenario snapshot and document the new placeholder behavior in the overlays config reference.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| schemas/azldev.schema.json | Updates schema description for overlay-files to document {component} rules. |
| scenario/snapshots/TestSnapshots_config_generate-schema_stdout_1.snap | Updates snapshot to match regenerated schema output. |
| internal/projectconfig/overlay_files_placeholder.go | Implements {component} placeholder validation, splitting, substitution, and absolutization helpers. |
| internal/projectconfig/overlay_files_placeholder_test.go | Unit tests for placeholder validation/splitting/substitution helpers. |
| internal/projectconfig/overlay_file.go | Substitutes {component} with component name before overlay glob expansion. |
| internal/projectconfig/configfile.go | Adds config-file validation enforcing placeholder rules by scope. |
| internal/projectconfig/configfile_test.go | Adds validation tests for allowed/forbidden placeholder usage across scopes. |
| internal/projectconfig/component.go | Updates OverlayFiles docs/schema description and absolutizes placeholder entries during WithAbsolutePaths. |
| internal/app/azldev/core/components/resolver.go | Unions pattern-discovered components into FindAllComponents with caching and shadowing behavior. |
| internal/app/azldev/core/components/pattern_discovery.go | Implements filesystem globbing + capture of {component} segment into synthesized components. |
| internal/app/azldev/core/components/pattern_discovery_test.go | Tests discovery, inheritance/override semantics, and same-scope collision errors. |
| docs/user/reference/config/overlays.md | Documents {component} placeholder discovery/inheritance/shadowing behavior for users. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
4a56921 to
f2c8de7Compare| } | ||
| count := strings.Count(entry, OverlayFilesComponentPlaceholder) | ||
| if count == 0 { |
There was a problem hiding this comment.
I think this stops us having non-component placeholder entries, ie have overlay-files = ["static/overlays/*.toml"]. If this is expected, docs need to be updated (overlays.md:166) to match.
There was a problem hiding this comment.
This is intentional. Project-level entries without {component} are rejected because they would silently apply the same files to every component. Updated the doc.
| {name: "star before placeholder", entry: "comps*/{component}/*.overlay.toml", wantErr: true}, | ||
| {name: "whole segment at start", entry: "{component}/overlays/*.overlay.toml", wantErr: false}, | ||
| {name: "whole segment in middle", entry: "comps/{component}/overlays/*.overlay.toml", wantErr: false}, | ||
| {name: "whole segment at end", entry: "comps/overlays/{component}", wantErr: false}, |
There was a problem hiding this comment.
I think this might be an error, won't it glob files with the name of the component, not directories? I think it probably always needs to be afolder right?
There was a problem hiding this comment.
Great catch, updated the check and test.
| // declaration overrides pattern-discovered overlay files for the same name. | ||
| func logShadowedByDeclaration(shadowed *patternDiscoveredComponent) { | ||
| slog.Warn( | ||
| "project 'overlay-files' pattern match shadowed by explicit component declaration; overlay files will not be applied", |
There was a problem hiding this comment.
I think this might miss shadowing from project defaults by groups. Not sure if thats something worth an error though.
There was a problem hiding this comment.
This would be resolved by the scope restriction, now rejecting overlay-files in distro-version and component-group default-component-config entirely.
| // which suppresses inheritance (see [projectconfig.ComponentConfig.MergeUpdatesFrom]). | ||
| // A declaration with no overlay-files inherits the pattern and the | ||
| // discovered files still apply, so there's nothing to warn about. | ||
| if declared, ok := r.env.Config().Components[name]; ok && declared.OverlayFiles != nil { |
There was a problem hiding this comment.
Might want to use len(declared.OverlayFiles) > 0 rather than nil, if I recall from the previous PR the way to disable inheritance is to set overlay-files=[]. 50/50 on this, we could still warn because thats weird, but it is intentionally explicitly saying "no overlays" for a component.
There was a problem hiding this comment.
Fixed. Changed to len(declared.OverlayFiles) > 0.
| } | ||
| prefix := entry[:idx] | ||
| if globMetaIdx := strings.IndexAny(prefix, "*?["); globMetaIdx >= 0 { |
There was a problem hiding this comment.
Might be able to add https://pkg.go.dev/github.com/bmatcuk/doublestar/v4#ValidatePattern here as well, get early signal back on a bad glob pattern.
There was a problem hiding this comment.
Now using doublestar.ValidatePattern on the substituted glob validation.
| err error | ||
| } | ||
| func (c *discoveredComponentsCache) load(env *azldev.Env) (map[string]*patternDiscoveredComponent, error) { |
There was a problem hiding this comment.
Found this a while back, might do the trick here: https://pkg.go.dev/sync#OnceValues
There was a problem hiding this comment.
switched to using sync.OnceValues
| } | ||
| byName := make(map[string]*patternDiscoveredComponent) | ||
| // Deduplicate files per component before appending to preserve stable ordering. |
There was a problem hiding this comment.
Curious if this is needed, my understanding was that glob would deduplicate internally.
There was a problem hiding this comment.
doublestar.Glob returns unique matches per call so the seen map was dead code and has been removed.
f2c8de7 to
1f37e09CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
1f37e09 to
27e7ffbCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
27e7ffb to
594c51bCompareNan Liu (liunan-ms)
commented
Jul 8, 2026
Thanks Daniel McIlvaney (@dmcilvaney) for the review! After thinking about the scope and usage of
I update the implementation and now the scope rule is enforced.
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
594c51b to
fc2fd2bCompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…overed components
Add '{component}' placeholder in project-level overlay-files entries. The resolver globs matching paths, extracts each captured segment as a component name, and synthesizes components not otherwise declared. Explicit component declarations shadow pattern-discovered matches (warning emitted).
- projectconfig: validate placeholder syntax per scope; reject backslash separators and glob metachars before the placeholder
- components: pattern_discovery walks overlay-files entries; Resolver caches discovery results per instance
- schema/docs: user-facing overlays.md updatedfc2fd2b to
0573473Compare| func absolutizeOverlayFilesPlaceholderEntries(referenceDir string, entries []string) []string { | ||
| if entries == nil { | ||
| return nil | ||
| } | ||
| result := make([]string, len(entries)) |
| // - Distro-version and component-group 'default-component-config': | ||
| // 'overlay-files' is not allowed at all. Broad-scope defaults for this | ||
| // field silently displace project-level discovery and per-component | ||
| // overrides, so we require callers to place the list at project or | ||
| // component scope where the intent is unambiguous. |
Summary
Adds a
{component}placeholder for project-leveloverlay-filesentries. The resolver globs matching paths, extracts each captured segment as a component name, and synthesizes components not otherwise declared. Explicit component declarations shadow pattern-discovered matches.Details
\as a path separator and glob metachars (*?[) before the placeholder (permissive in the suffix).pattern_discovery.gowalks project-leveloverlay-filesentries containing{component}, resolves matching filesystem paths via doublestar, and captures the component name from the placeholder position.Resolvercaches discovery results per-instance so repeated calls don't re-glob.FindAllComponentsunions the three sources — group members, loose declared components, pattern-discovered components — and emits sorted output for determinism.docs/user/reference/config/overlays.mddocuments the placeholder.