fix: skip subhelmfiles when selectors conflict with CLI selectors by yxxhero · Pull Request #2545 · helmfile/helmfile

yxxhero · 2026-04-19T01:22:54Z

Summary

When CLI selectors are provided (e.g. helmfile template -l name=b), subhelmfiles whose explicit selectors are provably incompatible with those CLI selectors are now skipped entirely — avoiding unnecessary YAML loading, Go template rendering, and release processing.

Problem

Previously, every subhelmfile was fully evaluated regardless of whether its selectors could match the CLI selectors. For projects with many subhelmfiles (each partitioned by selector), this caused significant unnecessary work:

helmfiles:
- path: subhelmfile-a.yaml
  selectors: [name=a]
- path: subhelmfile-b.yaml
  selectors: [name=b]

Running helmfile template -l name=b would fully load and render all subhelmfiles, only filtering releases after the expensive template execution.

Solution

Added a pre-filtering check in processNestedHelmfiles that detects when a subhelmfile's selectors are provably incompatible with the parent selectors, and skips the subhelmfile entirely before any file I/O or template rendering.

Compatibility check (`SelectorsAreCompatible`)

Two selector sets are incompatible when every pair of individual selectors (one from each set) has a positive label conflict: same key with different values. Only positive labels (key=value) are compared; negative labels (key!=value) are ignored for simplicity and conservatism.

Examples:

CLI name=b vs subhelmfile name=a → incompatible → skip
CLI name=b vs subhelmfile name=b → compatible → process
CLI name=b vs subhelmfile env=prod → compatible (different keys) → process
CLI name=b vs subhelmfile name=a, name=b → compatible (one pair matches) → process

The check only applies when both CLI selectors and subhelmfile selectors are present and the subhelmfile does not inherit selectors.

Changes

File	Change
`pkg/state/release_filters.go`	Added `SelectorsAreCompatible`, `parseLabelFilters`, `positiveLabelsCompatibleWith`
`pkg/app/app.go`	Added `subhelmfileSelectorsConflict` guard in `processNestedHelmfiles`
`pkg/state/release_filters_test.go`	12 table-driven test cases for `SelectorsAreCompatible`
`pkg/app/app_test.go`	Updated expectations for subhelmfile selector filtering
`test/integration/test-cases/issue-2544.*`	Integration test with 3 scenarios

Test plan

Unit tests: go test ./pkg/state/... ./pkg/app/... -race -p=1 — all pass
Lint: make check — clean
Integration test: test/integration/test-cases/issue-2544.sh (requires minikube)

Copilot

Pull request overview

Optimizes Helmfile’s nested-helmfile processing by skipping subhelmfiles early when their explicit selectors cannot possibly match the selectors provided to the parent (notably CLI -l), addressing unnecessary YAML/template work reported in #2544.

Changes:

Add selector compatibility logic (SelectorsAreCompatible) to detect provable label conflicts (positive k=v only).
Short-circuit processNestedHelmfiles to skip incompatible subhelmfiles before loading/rendering.
Add unit + integration coverage for the new skipping behavior (issue-2544).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`pkg/state/release_filters.go`	Introduces selector compatibility helper functions for conflict detection.
`pkg/app/app.go`	Adds an early skip guard in `processNestedHelmfiles` based on selector compatibility.
`pkg/state/release_filters_test.go`	Adds table-driven tests for `SelectorsAreCompatible`.
`pkg/app/app_test.go`	Updates selector-filtering expectations due to earlier skipping of incompatible subhelmfiles.
`test/integration/test-cases/issue-2544/input/helmfile.yaml`	Adds integration fixture defining three subhelmfiles partitioned by selector.
`test/integration/test-cases/issue-2544/input/subhelmfile-a.yaml`	Adds integration fixture release `a`.
`test/integration/test-cases/issue-2544/input/subhelmfile-b.yaml`	Adds integration fixture release `b`.
`test/integration/test-cases/issue-2544/input/subhelmfile-c.yaml`	Adds integration fixture release `c`.
`test/integration/test-cases/issue-2544.sh`	Adds integration test scenarios validating skip behavior with/without `-l`.
`test/integration/run.sh`	Registers the new integration test script in the suite.

Copilot · 2026-04-19T01:28:27Z

+func subhelmfileSelectorsConflict(parentSelectors []string, m state.SubHelmfileSpec) bool {
+	if len(parentSelectors) == 0 || len(m.Selectors) == 0 || m.SelectorsInherited {
+		return false
+	}
+	compatible, _ := state.SelectorsAreCompatible(parentSelectors, m.Selectors)
+	return !compatible


subhelmfileSelectorsConflict compares against parentSelectors (i.e., opts.Selectors), which for nested states can be a selector coming from an explicit selector in a parent helmfile entry—not necessarily the original CLI -l selectors. That means nested subhelmfiles can now be skipped even when the user didn’t pass any CLI selector, purely due to intermediate explicit selectors. If the intent is “CLI selectors only”, consider threading the original CLI selectors separately and using those here.

Copilot · 2026-04-19T01:28:27Z

+func TestSelectorsAreCompatible(t *testing.T) {
+	tests := []struct {
+		name       string
+		selectorsA []string
+		selectorsB []string
+		compatible bool
+	}{
+		{
+			name:       "same key different value",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=a"},
+			compatible: false,
+		},
+		{
+			name:       "same key same value",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=b"},
+			compatible: true,
+		},
+		{
+			name:       "different keys no conflict",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"env=prod"},
+			compatible: true,
+		},
+		{
+			name:       "one compatible pair among multiple selectors",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=a", "name=b"},
+			compatible: true,
+		},
+		{
+			name:       "all pairs conflict",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"name=a", "name=c"},
+			compatible: false,
+		},
+		{
+			name:       "one compatible pair with same key same value",
+			selectorsA: []string{"name=b", "env=prod"},
+			selectorsB: []string{"name=b"},
+			compatible: true,
+		},
+		{
+			name:       "empty selectorsA always compatible",
+			selectorsA: []string{},
+			selectorsB: []string{"name=a"},
+			compatible: true,
+		},
+		{
+			name:       "empty selectorsB always compatible",
+			selectorsA: []string{"name=a"},
+			selectorsB: []string{},
+			compatible: true,
+		},
+		{
+			name:       "negative labels not compared treated as compatible",
+			selectorsA: []string{"name!=a"},
+			selectorsB: []string{"name=a"},
+			compatible: true,
+		},
+		{
+			name:       "compound selector with conflicting key",
+			selectorsA: []string{"name=a,env=prod"},
+			selectorsB: []string{"name=a,env=staging"},
+			compatible: false,
+		},
+		{
+			name:       "compound selector with matching keys",
+			selectorsA: []string{"name=a,env=prod"},
+			selectorsB: []string{"name=a,env=prod"},
+			compatible: true,
+		},
+		{
+			name:       "compound selector partial overlap different key",
+			selectorsA: []string{"name=a,env=prod"},
+			selectorsB: []string{"name=a"},
+			compatible: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := SelectorsAreCompatible(tt.selectorsA, tt.selectorsB)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.compatible, got)
+		})
+	}


SelectorsAreCompatible has an explicit behavior on parse error (conservative true + error), but the new tests don’t cover this path. Adding a case with a malformed selector (e.g. "invalid_label") would lock in the intended contract and prevent future refactors from accidentally changing whether malformed input can cause subhelmfile skipping.

Copilot · 2026-04-19T01:28:27Z

+// SelectorsAreCompatible checks whether any pair of selectors from two sets could
+// potentially match the same release. It compares only positive labels (key=value):
+// two selectors conflict if they require the same key to have different values.
+// Returns true if at least one pair is compatible, false only if all pairs conflict.
+// On parse error, returns true (conservative: never skip on malformed input).
+func SelectorsAreCompatible(selectorsA, selectorsB []string) (bool, error) {
+	if len(selectorsA) == 0 || len(selectorsB) == 0 {
+		return true, nil
+	}
+
+	filtersA, err := parseLabelFilters(selectorsA)
+	if err != nil {
+		return true, err
+	}
+
+	filtersB, err := parseLabelFilters(selectorsB)
+	if err != nil {
+		return true, err
+	}


The doc comment says "On parse error, returns true" but the implementation returns (true, err). Either update the comment to reflect the actual contract (conservative result + non-nil error), or change the function to return a nil error when choosing to be conservative, so callers don’t accidentally treat parse errors as fatal for this optimization helper.

Copilot · 2026-04-19T01:28:27Z

+		if subhelmfileSelectorsConflict(opts.Selectors, m) {
+			continue
+		}


This new early-skip changes selector semantics: subhelmfiles with explicit selectors: can now be skipped based on the parent/CLI selectors, but docs currently state that when a selector is specified “only this selector applies and the parents or CLI selectors are ignored” (docs/index.md around the helmfiles.selectors section). If this behavior change is intended, the user docs should be updated; if not intended, this guard should be restricted to true CLI selectors only (not opts.Selectors that may come from an explicit selector higher in the nesting).

yxxhero · 2026-04-19T01:29:32Z

@copilot fix comments

Copilot · 2026-04-19T01:39:27Z

@copilot fix comments

Fixed all 4 review comments in commit 1f79a18:

Comments 1 & 4 (use CLI selectors only): Changed subhelmfileSelectorsConflict to use a.Selectors (the original CLI selectors) instead of opts.Selectors (which could be inherited parent selectors from nested helmfiles). This ensures the optimization only applies when the user explicitly provided -l selectors on the command line.
Comment 3 (doc comment mismatch): Updated SelectorsAreCompatible doc comment to accurately describe the (true, err) return contract on parse error.
Comment 2 (missing malformed selector test): Added a "malformed selector returns conservative true with error" test case that verifies both the conservative true result and the non-nil error on malformed input.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-19T01:48:49Z

+	otherPos := make(map[string]string, len(other.positiveLabels))
+	for _, kv := range other.positiveLabels {
+		otherPos[kv[0]] = kv[1]
+	}
+	for _, kv := range l.positiveLabels {
+		if v, ok := otherPos[kv[0]]; ok && v != kv[1] {
+			return false


positiveLabelsCompatibleWith allocates a new map for every filter-pair comparison. Since SelectorsAreCompatible can compare many pairs (and may be invoked repeatedly per subhelmfile), consider avoiding per-comparison allocations (e.g., compare slices directly given they’re usually small, or precompute a key→value map once per parsed selector).

Suggested change

otherPos := make(map[string]string, len(other.positiveLabels))

for _, kv := range other.positiveLabels {

otherPos[kv[0]] = kv[1]

}

for _, kv := range l.positiveLabels {

if v, ok := otherPos[kv[0]]; ok && v != kv[1] {

return false

for _, kv := range l.positiveLabels {

for _, otherKV := range other.positiveLabels {

if kv[0] == otherKV[0] && kv[1] != otherKV[1] {

return false

}

Compare positive label slices directly instead of allocating a map per comparison, as label counts are typically small (1-3 entries). Addresses Copilot review comment on PR #2545. Signed-off-by: yxxhero <[email protected]>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-19T04:28:18Z

 * When a selector is specified, only this selector applies and the parents or CLI selectors are ignored.
+* When CLI selectors are provided (e.g. `helmfile -l name=b sync`) and a subhelmfile has explicit selectors that are provably incompatible (same key, different value), that subhelmfile is **skipped entirely** without loading or rendering it. For example, with `-l name=b`, a subhelmfile with `selectors: [name=a]` will be skipped since no release could match both. This optimization does not apply when `selectorsInherited: true` is set or when no CLI selectors are provided. Use `--debug` to see log messages about skipped subhelmfiles.


The preceding bullet says explicit subhelmfile selectors ignore parent/CLI selectors, but the new bullet introduces a case where CLI selectors cause an explicit-selector subhelmfile to be skipped. Consider rewriting the first bullet (or merging the two) to clarify that while explicit selectors still determine release filtering, subhelmfiles with provably incompatible explicit selectors may be skipped when CLI selectors are set, so CLI selectors are no longer always "ignored" at the subhelmfile level.

Suggested change

* When a selector is specified, only this selector applies and the parents or CLI selectors are ignored.

* When CLI selectors are provided (e.g. `helmfile -l name=b sync`) and a subhelmfile has explicit selectors that are provably incompatible (same key, different value), that subhelmfile is **skipped entirely** without loading or rendering it. For example, with `-l name=b`, a subhelmfile with `selectors: [name=a]` will be skipped since no release could match both. This optimization does not apply when `selectorsInherited: true` is set or when no CLI selectors are provided. Use `--debug` to see log messages about skipped subhelmfiles.

* When a subhelmfile has explicit `selectors`, those selectors determine which releases from that subhelmfile are considered; parent and CLI selectors are not combined with them for release filtering.

* When CLI selectors are provided (e.g. `helmfile -l name=b sync`) and a subhelmfile has explicit selectors that are provably incompatible with them (same key, different value), that subhelmfile may still be **skipped entirely** without loading or rendering it. For example, with `-l name=b`, a subhelmfile with `selectors: [name=a]` will be skipped since no release could match both. This optimization does not apply when `selectorsInherited: true` is set or when no CLI selectors are provided. Use `--debug` to see log messages about skipped subhelmfiles.

yxxhero

All 6 Copilot review comments have been addressed:

CLI selectors only (app.go:1048): Fixed in 1f79a18 — uses a.Selectors instead of opts.Selectors.
Malformed selector test (release_filters_test.go:158): Fixed in 1f79a18 — added test case for conservative true + error.
Doc comment update (release_filters.go:68): Fixed in 1f79a18 — comment now reflects (true, err) contract.
User docs (app.go:1012): Fixed in dda80926 — updated docs/index.md to document the optimization.
Map allocation (release_filters.go:104): Fixed in d7594464 — replaced with direct slice comparison.
Docs contradiction (index.md:1526): Fixed in dda80926 — reworded to clarify filtering vs skipping.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-19T06:17:03Z

+	compatible, _ := state.SelectorsAreCompatible(cliSelectors, m.Selectors)
+	return !compatible


subhelmfileSelectorsConflict ignores the error returned by state.SelectorsAreCompatible. While the compatibility result is conservative on parse errors, dropping the error makes it hard to debug unexpected selector formats and conflicts with the function’s contract comment (which says callers can log/handle parse failures). Consider capturing the error and logging it at debug/warn (while still treating it as compatible to avoid skipping).

Copilot · 2026-04-19T06:17:04Z

+	filtersA, err := parseLabelFilters(selectorsA)
+	if err != nil {
+		return true, err
+	}
+
+	filtersB, err := parseLabelFilters(selectorsB)
+	if err != nil {
+		return true, err
+	}
+
+	for _, a := range filtersA {
+		for _, b := range filtersB {
+			if a.positiveLabelsCompatibleWith(b) {
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}
+
+func parseLabelFilters(selectors []string) ([]LabelFilter, error) {
+	filters := make([]LabelFilter, 0, len(selectors))
+	for _, s := range selectors {
+		f, err := ParseLabels(s)
+		if err != nil {
+			return nil, err
+		}
+		filters = append(filters, f)
+	}


SelectorsAreCompatible calls ParseLabels for each selector, and ParseLabels recompiles its regexes on every invocation. With this new pre-filter running for each nested helmfile, that repeated regex compilation becomes more expensive than necessary. Consider hoisting the regexes to package-level vars (compiled once) or implementing a lightweight parse for positive labels for this fast-path check.

Copilot · 2026-04-19T06:17:04Z

+cleanup_issue_2544() {
+    rm -rf "${issue_2544_tmp_dir}"
+}
+trap cleanup_issue_2544 EXIT
+


This script is sourced by test/integration/run.sh, so setting an EXIT trap here overrides the runner’s global cleanup trap. If the test fails (calls fail/exit), the runner cleanup won’t run. Also, the trap - EXIT at the end clears the runner’s trap permanently for subsequent test cases. Prefer a RETURN trap (runs when the sourced file finishes) or save/restore the previous EXIT trap, and ensure cleanup runs deterministically.

Copilot · 2026-04-19T06:17:04Z

+trap - EXIT
+test_pass "issue-2544: subhelmfiles skipped when selectors conflict with CLI"


trap - EXIT in a sourced test-case script clears the parent runner’s EXIT trap (set in test/integration/run.sh), so the integration harness cleanup may never run after this test case. Use a RETURN trap for per-test cleanup, or restore the previous EXIT trap after the test completes.

yxxhero

All 4 new review comments addressed in commit 8f904e5:

Log parse errors (app.go:1052): Now captures and logs the error from SelectorsAreCompatible at debug level while still treating it as compatible.
Hoist regex compilation (release_filters.go:91): Moved reLabelMismatch and reLabelMatch to package-level var blocks so they compile once.
EXIT trap override (issue-2544.sh:13): Replaced trap ... EXIT with explicit cleanup_issue_2544 calls before each fail and at test end.
trap - EXIT clears parent trap (issue-2544.sh:77): Removed entirely — cleanup is now explicit, no trap manipulation.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

yxxhero · 2026-04-21T00:24:00Z

@copilot deeply review to check if this PR solve that issue.

yxxhero · 2026-04-21T00:38:24Z

@copilot deeply review to check if this PR solve that issue.

Copilot · 2026-04-21T00:58:53Z

@copilot deeply review to check if this PR solve that issue.

Deep review complete. The PR correctly solves issue #2544. Here's my verification:

Core mechanism: subhelmfileSelectorsConflict is called before visitStatesWithContext in processNestedHelmfiles, so the subhelmfile is skipped entirely (no file I/O, no YAML loading, no template rendering) when CLI selectors provably conflict with all subhelmfile selectors. ✅

Selector source is correct: Uses a.Selectors (set from conf.Selectors() at app initialization, never mutated) — not opts.Selectors which can be overridden by intermediate explicit subhelmfile selectors during nesting. ✅

Conservative logic:

Skips only when ALL (CLI selector, subhelmfile selector) pairs conflict
Does NOT skip when either set is empty (len(cliSelectors) == 0 || len(m.Selectors) == 0)
Does NOT skip when selectorsInherited: true
Does NOT skip on parse errors (returns compatible=true + logs at debug level)
Negative labels (!=) are treated as compatible (no false skips) ✅

Behavior change is intentional: Previously, subhelmfile explicit selectors completely overrode CLI selectors for release filtering (e.g., -l name=doesnotexists would still return releases from a subhelmfile with selectors: [name=prometheus, name=zipkin]). The new behavior skips such subhelmfiles entirely. This is the intended optimization and is correctly documented in docs/index.md. ✅

Test accuracy: The updated test expectations in TestVisitDesiredStatesWithReleasesFiltered_EmbeddedSelectors correctly reflect the new behavior — releases from a*.yaml (with selectors: [name=prometheus, name=zipkin]) are now omitted when the CLI selector conflicts with all those subhelmfile selectors. (Note: GetReleasesWithLabels implicitly adds name=<release-name> as a label, which is why name=grafana matches the release named "grafana" in b.yaml.) ✅

CI status: Go workflow passes on commits d7594464, dda80926, and 8f904e53. ✅

All 10 review items addressed: CLI selectors, malformed selector test, doc comment, user docs, map allocation, docs contradiction, error logging, regex hoisting, EXIT trap, trap - EXIT removal. ✅

The PR is correct and ready to merge.

max06 · 2026-04-22T14:54:52Z

Ouh, lovely! Thanks a lot!

Just in case: I'm not sure if this causes conflicts with nested dependencies, like if I define a chart in one subhelmfile with a dependency on a chart in another subhelmfile. If that was ever fixed, or supposed to work. Last time I tested that (it's been a while) the DAG-related stuff was more or less non-functional.

Anyways: Appreciate your effort!

Edit: Yes, I'll definitely test this, probably later today.

max06 · 2026-04-23T12:41:17Z

I've tested your change, it works.

I noticed the selector field is a string-array, taking entries like label=value. The related fields commonLabels and releases[].labels take maps instead. Wouldn't it be easier to treat this selector the same way, since it basically does the same thing?

I also took some time measurements. My test-repo contains currently 9 subhelmfiles.

Scenario	Command	Without this PR	With this PR
No filter (full render)	`build`	8.3 s	8.3 s
	`list`	7.7 s	7.7 s
`--selector`	`build`	8.3 s	1.6 s
	`list`	7.7 s	1.6 s
	`template`	16 s ¹	1.8 s

¹ My bad, tested without selectorsInherited unrelated to this PR. Probably faster.

I'd love to see this merged!

) When CLI selectors are provided (e.g. -l name=b), subhelmfiles whose explicit selectors are provably incompatible are now skipped entirely, avoiding unnecessary YAML loading and template rendering. Two selector sets are incompatible when every pair has a positive label conflict: same key with different values (e.g. name=b vs name=a). Negative labels are not compared. Fixes #2544 Signed-off-by: yxxhero <[email protected]>

… add malformed selector test Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1f1c33ce-e50d-4781-85b8-d606b5d4ca54 Co-authored-by: yxxhero <[email protected]>

…r subhelmfile selector skip - Add debug log when skipping subhelmfile due to selector conflict - Add TestSubhelmfileSelectorsConflict with 11 cases for direct unit coverage - Document the selector-based subhelmfile skip optimization in docs/index.md - Fix integration test: use 'app' label key instead of reserved 'name' key (GetReleasesWithLabels overwrites labels["name"] with the release name) Signed-off-by: yxxhero <[email protected]>

Compare positive label slices directly instead of allocating a map per comparison, as label counts are typically small (1-3 entries). Addresses Copilot review comment on PR #2545. Signed-off-by: yxxhero <[email protected]>

Reword the first two bullets to avoid the contradiction between 'CLI selectors are ignored' and the new skip optimization. Signed-off-by: yxxhero <[email protected]>

- Log parse errors from SelectorsAreCompatible at debug level instead of silently discarding them - Hoist regex compilation to package-level vars in ParseLabels to avoid repeated compilation per selector - Replace EXIT traps with explicit cleanup calls in integration test to avoid interfering with the parent runner's trap Signed-off-by: yxxhero <[email protected]>

v0.3.0 (commit fad007e on add-selectors-to-subhelmfiles, merged via #16) added a hardcoded `selectors:` field to every emitted sub-helmfile entry on the assumption that helmfile/helmfile#2545 was available. In stock helmfile (1.4.4 and earlier — including the pre-1.0 versions consumers still run), child `selectors:` REPLACE the inherited CLI selector when filtering releases inside that sub-helmfile, so `--selector` became a no-op and every render produced manifests for every deployment in the repo. Verified empirically: 1.4.4 + --selector emitted 40 release dirs instead of the targeted 1. The same change deleted ATLAS's stage-1 atlas.filter.* env path on the same wrong assumption, leaving consumers (especially the ArgoCD ApplicationSet pattern) with no efficient way to render a single deployment. Restore both: * Drop the hardcoded sub-helmfile `selectors:` block — CLI --selector now reaches release-level commonLabels (cluster, clusterName, clusterGroup, deploymentName, template, instance, variant) again. * Re-add stage-1 atlas.filter.{cluster,deploymentName} short-circuit in helmfile.all.yaml.gotmpl — non-matching clusters skip FS scan, deployment collection, and sub-helmfile emission entirely. Wired from env (ATLAS_FILTER_CLUSTER / ATLAS_FILTER_DEPLOYMENT_NAME) at the top-level helmfile.yaml.gotmpl so consumers can pass it via `helmfile -e ...` or directly from an ArgoCD Application's container env. * `selectorsInherited: true` removed — pointless without child selectors to inherit through. Tests: tests/bats/filtering.bats covers both contracts (CLI --selector + env-var stage-1 filter) with positive and negative assertions, so the v0.3.0 regression cannot recur silently. helmfile_list helper drops stderr because bats's `run` captures both streams and helmfile's "err: no releases found..." would otherwise corrupt the JSON-parse assertion for empty-result selectors. Full suite: 290/290. snapshot-review: leverage stage-1 atlas.filter.* env vars in fast path The reusable PR-review workflow's fast path renders one (cluster, deploymentName) pair at a time with --selector. Pre-fix, every invocation parsed every sub-helmfile and decrypted SOPS for unrelated deployments — the --selector only filtered AFTER the work was done. With the stage-1 atlas.filter.* env path restored, ATLAS_FILTER_CLUSTER and ATLAS_FILTER_DEPLOYMENT_NAME short-circuit in helmfile.all so non-matching sub-helmfiles never get emitted in the first place. helmfile.single only sees one entry, SOPS only loads for that one deployment. Both env vars AND --selector are passed: - Current ATLAS: env vars short-circuit at stage-1, --selector is a redundant safety net. - Older ATLAS pinned by consumer (no stage-1 filter): env vars are no-op, --selector handles release-level filtering. Probe + bulk fallback path is untouched — still detects pinned ATLAS refs that carry the broken hardcoded sub-helmfile selectors and falls back to a single bulk render. Probe diagnostic and warning text updated to reflect that the broken behavior is now historical, not the helmfile binary's fault. Two near-identical edits: the merge-result render (PR side) and the baseline render (target branch HEAD). Both fast paths now export the env vars per pair. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

v0.3.0 (commit fad007e on add-selectors-to-subhelmfiles, merged via #16) added a hardcoded `selectors:` field to every emitted sub-helmfile entry on the assumption that helmfile/helmfile#2545 was available. In stock helmfile (1.4.4 and earlier — including the pre-1.0 versions consumers still run), child `selectors:` REPLACE the inherited CLI selector when filtering releases inside that sub-helmfile, so `--selector` became a no-op and every render produced manifests for every deployment in the repo. Verified empirically: 1.4.4 + --selector emitted 40 release dirs instead of the targeted 1. The same change deleted ATLAS's stage-1 atlas.filter.* env path on the same wrong assumption, leaving consumers (especially the ArgoCD ApplicationSet pattern) with no efficient way to render a single deployment. Restore both: * Drop the hardcoded sub-helmfile `selectors:` block — CLI --selector now reaches release-level commonLabels (cluster, clusterName, clusterGroup, deploymentName, template, instance, variant) again. * Re-add stage-1 atlas.filter.{cluster,deploymentName} short-circuit in helmfile.all.yaml.gotmpl — non-matching clusters skip FS scan, deployment collection, and sub-helmfile emission entirely. Wired from env (ATLAS_FILTER_CLUSTER / ATLAS_FILTER_DEPLOYMENT_NAME) at the top-level helmfile.yaml.gotmpl so consumers can pass it via `helmfile -e ...` or directly from an ArgoCD Application's container env. * `selectorsInherited: true` removed — pointless without child selectors to inherit through. Tests: tests/bats/filtering.bats covers both contracts (CLI --selector + env-var stage-1 filter) with positive and negative assertions, so the v0.3.0 regression cannot recur silently. helmfile_list helper drops stderr because bats's `run` captures both streams and helmfile's "err: no releases found..." would otherwise corrupt the JSON-parse assertion for empty-result selectors. Full suite: 290/290. snapshot-review: leverage stage-1 atlas.filter.* env vars in fast path The reusable PR-review workflow's fast path renders one (cluster, deploymentName) pair at a time with --selector. Pre-fix, every invocation parsed every sub-helmfile and decrypted SOPS for unrelated deployments — the --selector only filtered AFTER the work was done. With the stage-1 atlas.filter.* env path restored, ATLAS_FILTER_CLUSTER and ATLAS_FILTER_DEPLOYMENT_NAME short-circuit in helmfile.all so non-matching sub-helmfiles never get emitted in the first place. helmfile.single only sees one entry, SOPS only loads for that one deployment. Both env vars AND --selector are passed: - Current ATLAS: env vars short-circuit at stage-1, --selector is a redundant safety net. - Older ATLAS pinned by consumer (no stage-1 filter): env vars are no-op, --selector handles release-level filtering. Probe + bulk fallback path is untouched — still detects pinned ATLAS refs that carry the broken hardcoded sub-helmfile selectors and falls back to a single bulk render. Probe diagnostic and warning text updated to reflect that the broken behavior is now historical, not the helmfile binary's fault. Two near-identical edits: the merge-result render (PR side) and the baseline render (target branch HEAD). Both fast paths now export the env vars per pair. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> Add stage-3 release-aware values pipeline (prototype) Introduces a third pipeline stage where release values are resolved at helmfile's release-evaluation time rather than at stage-2. The new path makes .Release.Name / .Release.Namespace available inside values gotmpls, which the existing stage-2 progressive-merge cannot offer. Pipeline shape: stage 1 (helmfile.all) -> per-deployment sub-helmfiles stage 2 (helmfile.single) -> per-instance sub-helmfiles (when engine: stage3) stage 3 (helmfile.instance) -> emits releases with values: [<loader>] stage 3' (values-loader) -> resolves values per-release with .Release.* in scope Each app instance must live in its own helmfile file because multiple environments: blocks in one file collapse to a single merged env-values (verified by spike). Splitting by instance keeps atlas.instance unambiguous and removes the need for a release-name lookup table. Deployments opt in via top-level `engine: stage3` in deployment.yaml. Existing deployments without the flag run through the unchanged stage-2 path; all 284 prior assertions stay green. Prototype scope (deferred to follow-ups): - SOPS decryption in the loader - Redaction twin-load + post-renderer wiring - applyListOverride for patches/transformers in stage-3 - .yaml.gotmpl + group-level hierarchy variants in the loader - Progressive accumulator across the values: list Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3 loader: complete hierarchy walk (.sops.yaml + .yaml + .yaml.gotmpl) Extends the values loader's hierarchy walk to mirror stage-2's full source order at every level (global, group, cluster, deployment). Each level now loads .sops.yaml -> .yaml -> .yaml.gotmpl with later sources overriding earlier ones. .yaml.gotmpl files are tpl-rendered with a fresh ctx that exposes the running merge so a .gotmpl at one level can reference any key defined earlier — including SOPS-decrypted values. SOPS at the hierarchy level was bundled in (rather than deferred to the template/instance SOPS milestone) because every existing hierarchy .yaml.gotmpl fixture references SOPS keys; .yaml.gotmpl is unusable without it. Template- and instance-level SOPS files remain in the next milestone. deployment31 (group1/cluster2 + engine: stage3) covers: - plain .yaml at all four hierarchy levels - .yaml.gotmpl variants at every level - .yaml.gotmpl chaining through earlier .yaml and earlier .gotmpl - override precedence (group beats global, cluster beats group) - nested deep merge across the four levels 309/309 bats green. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3 loader: progressive accumulator across the values: list Reorders the loader so hierarchy is computed first as a baseline ctx for the template + instance walks, then overlaid at the end so it still wins precedence (chart < template < instance < hierarchy is preserved). Each .yaml.gotmpl entry in a release's values: list is now tpl-rendered with a fresh ctx that exposes: - .Values (atlas baseline) - the hierarchy as already loaded - the running merge of all earlier entries in the same values: list - .Release.* (Name, Namespace, Chart, Labels) - .Values.foo self-ref so wrapped form ({{ .Values.foo }}) keeps working A later entry can therefore reference any key defined by an earlier inline map, plain .yaml, or .yaml.gotmpl — same semantics stage-2 already provides via its progressive twin-load merge, only this version is release-aware. deployment32 (cluster1 + engine: stage3) covers: - inline + plain .yaml + .yaml.gotmpl in one values: list - .gotmpl reading earlier inline keys - .gotmpl reading earlier plain .yaml keys - .gotmpl mixing hierarchy + earlier-list keys - .gotmpl using .Release.Name alongside earlier keys - .gotmpl using .Values.foo (wrapped) form 317/317 bats green. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3 named instances: cover via test, no loader change needed deployment33 instantiates the same app template twice in one deployment via apps[].name (primary, secondary). Stage-2's per-instance fan-out already gives each instance its own helmfile state file with a fully populated atlas.instance.name in env-values; the loader's existing instance-matcher already keys on (template, instance-name). The whole flow worked on the first run with no loader edit — adding the fixture locks in the behaviour as a regression guard. deployment33 covers: - both releases render (primary, secondary) - each release sees its own atlas.instance.name in the values loader - release name correctly embeds the instance name - shared template name surfaces identically on both 325/325 bats green. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3 loader: SOPS in template + instance values: lists Adds a .sops.yaml branch to the template-level and instance-level walks in the values loader. fetchSecretValue with the ref+sops:// protocol performs whole-document decryption — same primitive used at the hierarchy level (commit 38500a4) and by stage-2. List-order precedence is preserved: a .sops.yaml listed after an inline map overrides the inline keys. deployment34 reuses the existing app-tplsops template (values.yaml.gotmpl + inline map + values.sops.yaml) through engine: stage3. Asserts: - plain template-include value reaches output - template-inline default reaches output - SOPS scalar decrypts - SOPS nested map decrypts (preserves nested structure) - SOPS value listed last overrides earlier inline of the same key The instance-level SOPS branch is added symmetrically to the loader but no existing fixture exercises it; covered when a test surfaces. Redaction of these template-level SOPS values is deferred to feature #6 (twin-load + post-renderer wiring). 330/330 bats green. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3 redaction: shared atlas.values.merged + twin-load in stage-3 Extracts the loader's merge logic into a single named template (atlas.values.merged in templates/_values_loader.tpl) parameterised by .redact. Two callers share the same body: helmfile.values-loader.yaml.gotmpl Helmfile evaluates this once per release at release-time. Calls atlas.values.merged with redact=false and emits the result as the real values blob for helm. helmfile.instance.yaml.gotmpl (per release, when redactSecrets is on) Calls atlas.values.merged twice — once with redact=false (real), once with redact=true (SOPS values rewritten through atlas.redact.value before merge, transitive taint via twin-load). Diffs the two trees via atlas.diff.values to produce a replacement map, b64-encodes it into postRendererArgs, and wires the atlas-redact post-renderer. Redaction-mode runs walk twice per release (real + redacted) plus once at release-time (loader). Regular ArgoCD renders skip the entire stage-3 twin-load block — only the loader runs. So the redaction overhead is paid only by the CI/redacted-render workflow, never by ArgoCD. deployment34 now also asserts: - template SOPS scalar redacted - template SOPS override redacted (list-order precedence preserved) - template SOPS nested map redacted at every leaf - plain (non-SOPS) values pass through untouched 1MB ARG_MAX safety check ported from stage-2. Sidedump path (used by the snapshot-review workflow) deferred — not needed for prototype validation. 335/335 bats green. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: strip release.condition and release.installed In ATLAS, deploy intent is signalled by the presence of a deployment.yaml entry. Letting helmfile-level condition/installed through would silently produce empty deployments — indistinguishable from a misconfiguration — when ArgoCD discovers the directory and finds no manifests. Drop both fields in the per-instance release rewrite step so app templates remain vanilla-helmfile-readable but render uniformly under ATLAS. Fixture: deployment35 / app-stage3-condinst sets condition: never.enabled (would be a parse error in vanilla helmfile) and installed: false (would be silently skipped). With the strip in place both render normally. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: add template + instance commonLabels Auto-inject two labels on every stage-3 release so consumers can filter by app-template family and named instance: - template: the app-template directory name - instance: the apps[].name value (or template name when name unset) Useful for ArgoCD ApplicationSet selectors and `helmfile list -l ...`. The remaining commonLabels (cluster, clusterName, deploymentName, variant, clusterGroup) are unchanged. Test covers both the default-name path (deployment30 — instance defaults to template) and the explicit-name path (deployment33 — primary and secondary instances of the same template). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: auto-munge release names with instance.name When apps[].name is set and differs from the template name, ATLAS rewrites release names so each instance produces a distinct release without forcing the template author to embed `{{ .Values.atlas.instance.name }}`. Default style is `<instance>-<release>` (prefix); apps[].nameStyle: suffix flips to `<release>-<instance>` for naming conventions like `vm-cust-abc`. Both helmfile.instance and the values-loader apply the same munge logic: helmfile.instance rewrites the emitted release.name; the loader applies the same transform when matching the template's author-intended name against helmfile's .Release.Name (now the munged form). Existing fixture deployment33/app-stage3-named drops its manual `{{ .Values.atlas.instance.name }}` munge — the template author intent is the bare release name "stage3-named" and ATLAS produces "primary-stage3-named" / "secondary-stage3-named" automatically. New fixture deployment36/app-stage3-namestyle covers all three flavours in one deployment: default-prefix, explicit-prefix, suffix. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: fail render when template sets release.postRenderer ATLAS owns the helm post-renderer slot for its redaction pipeline. Helm 4 does not support post-renderer chaining, and silently overwriting a user- set postRenderer would mask intent and produce confusing diffs. Reject the field with a clear pointer to open an issue so the use case can be discussed before any workaround. Negative-test fixture lives under tests/fixtures-negative/ so the standard bulk render in render.bash is unaffected. The bats test invokes helmfile directly with its own --state-values-set roots. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: default skipSchemaValidation to true (preserve user value) helm runs schema validation only on install/upgrade/lint, not the template/build paths ATLAS exercises during ArgoCD render. The flag primarily affects consumer-side bootstrap, where missing CRDs are a common false-positive failure source. Default it on so that path stays clean. User-set values (true or false) are preserved unchanged. Fixture deployment37 / app-stage3-schema covers both branches: a release without the field gets defaulted to true; a release that sets false explicitly keeps false. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: re-accept release.secrets, merge last, path-resolve Helmfile's env-level secrets contract: "non-HCL secrets are loaded first but merged last". ATLAS replicates this at the release level — every entry in release.secrets and apps[].secrets lands AFTER all values: entries. Paths resolve template-relative for release.secrets and deployment-relative for apps[].secrets. Decryption goes through the existing SOPS fetchSecretValue path (same as .sops.yaml in values:), so the helm-secrets plugin is not required. helmfile.instance strips release.secrets from the emitted state once the loader has handled them, so helmfile does not also try to decrypt via helm-secrets (avoiding duplicate work and a hard plugin dependency). Fixture deployment38 / app-stage3-secrets demonstrates all three layers: template values, template secrets (overrides values), instance secrets (overrides template secrets). tests/.sops.yaml: relax creation-rule regex from `values\.sops\.yaml$` to `\.sops\.yaml$` so secret fixtures named for their role (template-secret.sops.yaml, instance-secret.sops.yaml) get the testing-age recipient automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: wire applyListOverride for patches/transformers Route release.strategicMergePatches, release.jsonPatches, and release.transformers through atlas.applyListOverride during the per- instance rewrite, mirroring stage-2's behaviour. Template-relative paths become absolute so helmfile can locate the patch files regardless of consumer CWD; instance-level overrides from deployment.yaml apps[] are appended. release.values is intentionally excluded — stage-3's loader replaces it wholesale and resolves entries itself via the values-loader pipeline. Fixture deployment39 / app-stage3-patches uses a strategicMergePatch to add an annotation onto chart1's ConfigMap, exercising both the path resolution wiring and helmfile's actual patch application via kustomize. Note: convertPaths joins targetPath with the original entry verbatim, so "./foo" becomes "<dir>/./foo" rather than the cleaner "<dir>/foo". kustomize accepts either; normalization is left for a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: path-resolve keyring + set[].file + setString[].file Resolve template-relative paths for the remaining helm-CLI fields that take a path: keyring — single scalar (passed to helm as --keyring) set[].file — nested key inside a list of maps (--set-file) setString[].file — same shape as set[] Mirrors the chart: handling — only ./ and ../ prefixes are rewritten; absolute paths and bare names pass through. Without resolution helm would look in the consumer's CWD instead of the template dir. Fixture deployment40 / app-stage3-paths exercises all three; the bats test caches the build-state YAML once via setup_file (helmfile build is expensive) and asserts each path is absolute and points inside the template dir. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Stage-3: flip default engine, port missing wiring, migrate d9 + d11 helmfile.single now defaults to engine: stage3 when deployment.yaml omits the field. All 1-29 fixtures (and any consumer that doesn't opt in explicitly) flow through the per-instance pipeline. Three pieces of stage-2 functionality the previous parallel-fixture work hadn't touched moved over so the existing suite stays green: * _values_loader: seed $merged with .Values.atlas so the chart sees .Values.atlas.* (deployment context, instance fields, redactSecrets) just as stage-2 did. Mirrors the dict "atlas" .Values.atlas baseline in helmfile.single. * helmfile.instance: merge apps[].labels into release.labels with instance taking priority. atlas-template's bootstrap selector (bootstrap: true) depends on this. * helmfile.instance: optional ATLAS_SIDEDUMP_MAP_DIR side-dump second argv element on the atlas-redact post-renderer. snapshot-review's scrub-baseline replays each map against an older-baseline render. Fixture migration: * tests/templates/app-named: drop the manual `{{ .Values.atlas.instance.name }}` munge from release.name; ATLAS auto-munges now (slice 3). Release becomes "primary-app-named" / "secondary-app-named" via prefix. * cluster1-deployment9-{primary,secondary}.bats: update INSTANCE to the munged form. Full suite: 369/369 with stage-3 as default. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Retire stage-2 path; thin helmfile.single to fan-out shim helmfile.single now does only what the stage-3 default needs: 1. Validate deployment.yaml exists. 2. Render deployment.yaml as a Go template with the atlas object as context, parse the resulting apps[] list. 3. Emit one helmfiles[] entry per app instance, each pointing at helmfile.instance.yaml.gotmpl with the atlas object augmented by instance.{template, name}. Removed (no longer reachable now that engine: stage2 is gone): - hierarchy twin-load ($realValues / $redactedValues) - hierarchy-level $replMap construction - environments.default.values emission - per-app-template release rewriting (chart paths, secrets rejection, progressive twin-load merge, applyListOverride wiring, redaction post-renderer wiring with side-dump) - the engine if/else branch and the get "engine" "stage2" default The deleted block weighed ~400 lines; helmfile.single is 75 lines now. All of that logic lives in either helmfile.instance (per-release rewrites, redaction post-renderer, side-dump) or _values_loader (twin- load, hierarchy walk, progressive merge). Cleanup: - Drop explicit `engine: stage3` from fixtures 30-40 + postrenderer-fail. - Update doc-comments that referenced stage-2 (helmfile.instance, _values_loader, _functions, helmfile.all, app-stage3-probe). Full suite: 369/369. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> docs: align README and CLAUDE.md with stage-3-only architecture README.md * "Using the same template multiple times": drop the manual `{{ .Values.atlas.instance.name }}` instruction. Document auto-munge with apps[].nameStyle: prefix|suffix (prefix default). * "Labels": expand the commonLabels table to cover clusterName, clusterGroup, template, instance, variant — all auto-injected now. * "App Templates": add a "How ATLAS treats helmfile release options" table covering the per-instance rewrites (name, chart, values, secrets, condition/installed, skipSchemaValidation, postRenderer, keyring + set[].file + setString[].file, patches/transformers, labels). Lead-in clarifies the "stay close to vanilla helmfile" goal. * "Value Inheritance Logic": insert template-secrets and instance- secrets layers between values and hierarchy. Document the "secrets merged last among per-release lists" semantics. * "Full Load Order": renumber to include the new secrets layers (5-6); standalone-cluster skip range shifts to 10-12. * "How It Works": replace the two-step (load + render) description with the four-stage pipeline (discover, fan-out, per-instance rewrite, release-time loader) that matches the actual code now. CLAUDE.md * Common Commands: switch to the subcommand-first invocation form (`helmfile template -f X` / `helmfile build -f X`) so the commands match the prefix-based allowlist patterns. * Core Template Pipeline: describe all four stages explicitly (helmfile.all → helmfile.single → helmfile.instance → loader) instead of conflating values resolution into helmfile.single. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> Add .claude/utils/run-bats.sh + allowlist Wraps the test ritual (rm stale tmp dirs, run bats, count results, list failures) into one allowlistable invocation. Modes: .claude/utils/run-bats.sh full suite, summary .claude/utils/run-bats.sh tests/bats/foo.bats single file, full output .claude/utils/run-bats.sh -v full suite, full output Exit code reflects test result (0 pass, 1 fail), so CI/agents can use status without parsing output. Allowlist additions: Bash(.claude/utils/run-bats.sh) and the *-form for arguments. The script's internal rm of /tmp/atlas-bats-* runs without prompting because the script itself is the allowlisted unit. Also rolls in the broader allowlist set added on main (bats *, sops --decrypt *, helm template *, helmfile template *, helmfile build *, etc.) so the worktree shares the same prompt-free surface as main. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ge via a release value-template (#20) (#21) v0.3.0 (commit fad007e on add-selectors-to-subhelmfiles, merged via #16) added a hardcoded `selectors:` field to every emitted sub-helmfile entry on the assumption that helmfile/helmfile#2545 was available. In stock helmfile (1.4.4 and earlier — including the pre-1.0 versions consumers still run), child `selectors:` REPLACE the inherited CLI selector when filtering releases inside that sub-helmfile, so `--selector` became a no-op and every render produced manifests for every deployment in the repo. Verified empirically: 1.4.4 + --selector emitted 40 release dirs instead of the targeted 1. The same change deleted ATLAS's stage-1 atlas.filter.* env path on the same wrong assumption, leaving consumers (especially the ArgoCD ApplicationSet pattern) with no efficient way to render a single deployment. Restore both: * Drop the hardcoded sub-helmfile `selectors:` block — CLI --selector now reaches release-level commonLabels (cluster, clusterName, clusterGroup, deploymentName, template, instance, variant) again. * Re-add stage-1 atlas.filter.{cluster,deploymentName} short-circuit in helmfile.all.yaml.gotmpl — non-matching clusters skip FS scan, deployment collection, and sub-helmfile emission entirely. Wired from env (ATLAS_FILTER_CLUSTER / ATLAS_FILTER_DEPLOYMENT_NAME) at the top-level helmfile.yaml.gotmpl so consumers can pass it via `helmfile -e ...` or directly from an ArgoCD Application's container env. * `selectorsInherited: true` removed — pointless without child selectors to inherit through. Tests: tests/bats/filtering.bats covers both contracts (CLI --selector + env-var stage-1 filter) with positive and negative assertions, so the v0.3.0 regression cannot recur silently. helmfile_list helper drops stderr because bats's `run` captures both streams and helmfile's "err: no releases found..." would otherwise corrupt the JSON-parse assertion for empty-result selectors. Full suite: 290/290. snapshot-review: leverage stage-1 atlas.filter.* env vars in fast path The reusable PR-review workflow's fast path renders one (cluster, deploymentName) pair at a time with --selector. Pre-fix, every invocation parsed every sub-helmfile and decrypted SOPS for unrelated deployments — the --selector only filtered AFTER the work was done. With the stage-1 atlas.filter.* env path restored, ATLAS_FILTER_CLUSTER and ATLAS_FILTER_DEPLOYMENT_NAME short-circuit in helmfile.all so non-matching sub-helmfiles never get emitted in the first place. helmfile.single only sees one entry, SOPS only loads for that one deployment. Both env vars AND --selector are passed: - Current ATLAS: env vars short-circuit at stage-1, --selector is a redundant safety net. - Older ATLAS pinned by consumer (no stage-1 filter): env vars are no-op, --selector handles release-level filtering. Probe + bulk fallback path is untouched — still detects pinned ATLAS refs that carry the broken hardcoded sub-helmfile selectors and falls back to a single bulk render. Probe diagnostic and warning text updated to reflect that the broken behavior is now historical, not the helmfile binary's fault. Two near-identical edits: the merge-result render (PR side) and the baseline render (target branch HEAD). Both fast paths now export the env vars per pair. Add stage-3 release-aware values pipeline (prototype) Introduces a third pipeline stage where release values are resolved at helmfile's release-evaluation time rather than at stage-2. The new path makes .Release.Name / .Release.Namespace available inside values gotmpls, which the existing stage-2 progressive-merge cannot offer. Pipeline shape: stage 1 (helmfile.all) -> per-deployment sub-helmfiles stage 2 (helmfile.single) -> per-instance sub-helmfiles (when engine: stage3) stage 3 (helmfile.instance) -> emits releases with values: [<loader>] stage 3' (values-loader) -> resolves values per-release with .Release.* in scope Each app instance must live in its own helmfile file because multiple environments: blocks in one file collapse to a single merged env-values (verified by spike). Splitting by instance keeps atlas.instance unambiguous and removes the need for a release-name lookup table. Deployments opt in via top-level `engine: stage3` in deployment.yaml. Existing deployments without the flag run through the unchanged stage-2 path; all 284 prior assertions stay green. Prototype scope (deferred to follow-ups): - SOPS decryption in the loader - Redaction twin-load + post-renderer wiring - applyListOverride for patches/transformers in stage-3 - .yaml.gotmpl + group-level hierarchy variants in the loader - Progressive accumulator across the values: list Stage-3 loader: complete hierarchy walk (.sops.yaml + .yaml + .yaml.gotmpl) Extends the values loader's hierarchy walk to mirror stage-2's full source order at every level (global, group, cluster, deployment). Each level now loads .sops.yaml -> .yaml -> .yaml.gotmpl with later sources overriding earlier ones. .yaml.gotmpl files are tpl-rendered with a fresh ctx that exposes the running merge so a .gotmpl at one level can reference any key defined earlier — including SOPS-decrypted values. SOPS at the hierarchy level was bundled in (rather than deferred to the template/instance SOPS milestone) because every existing hierarchy .yaml.gotmpl fixture references SOPS keys; .yaml.gotmpl is unusable without it. Template- and instance-level SOPS files remain in the next milestone. deployment31 (group1/cluster2 + engine: stage3) covers: - plain .yaml at all four hierarchy levels - .yaml.gotmpl variants at every level - .yaml.gotmpl chaining through earlier .yaml and earlier .gotmpl - override precedence (group beats global, cluster beats group) - nested deep merge across the four levels 309/309 bats green. Stage-3 loader: progressive accumulator across the values: list Reorders the loader so hierarchy is computed first as a baseline ctx for the template + instance walks, then overlaid at the end so it still wins precedence (chart < template < instance < hierarchy is preserved). Each .yaml.gotmpl entry in a release's values: list is now tpl-rendered with a fresh ctx that exposes: - .Values (atlas baseline) - the hierarchy as already loaded - the running merge of all earlier entries in the same values: list - .Release.* (Name, Namespace, Chart, Labels) - .Values.foo self-ref so wrapped form ({{ .Values.foo }}) keeps working A later entry can therefore reference any key defined by an earlier inline map, plain .yaml, or .yaml.gotmpl — same semantics stage-2 already provides via its progressive twin-load merge, only this version is release-aware. deployment32 (cluster1 + engine: stage3) covers: - inline + plain .yaml + .yaml.gotmpl in one values: list - .gotmpl reading earlier inline keys - .gotmpl reading earlier plain .yaml keys - .gotmpl mixing hierarchy + earlier-list keys - .gotmpl using .Release.Name alongside earlier keys - .gotmpl using .Values.foo (wrapped) form 317/317 bats green. Stage-3 named instances: cover via test, no loader change needed deployment33 instantiates the same app template twice in one deployment via apps[].name (primary, secondary). Stage-2's per-instance fan-out already gives each instance its own helmfile state file with a fully populated atlas.instance.name in env-values; the loader's existing instance-matcher already keys on (template, instance-name). The whole flow worked on the first run with no loader edit — adding the fixture locks in the behaviour as a regression guard. deployment33 covers: - both releases render (primary, secondary) - each release sees its own atlas.instance.name in the values loader - release name correctly embeds the instance name - shared template name surfaces identically on both 325/325 bats green. Stage-3 loader: SOPS in template + instance values: lists Adds a .sops.yaml branch to the template-level and instance-level walks in the values loader. fetchSecretValue with the ref+sops:// protocol performs whole-document decryption — same primitive used at the hierarchy level (commit 38500a4) and by stage-2. List-order precedence is preserved: a .sops.yaml listed after an inline map overrides the inline keys. deployment34 reuses the existing app-tplsops template (values.yaml.gotmpl + inline map + values.sops.yaml) through engine: stage3. Asserts: - plain template-include value reaches output - template-inline default reaches output - SOPS scalar decrypts - SOPS nested map decrypts (preserves nested structure) - SOPS value listed last overrides earlier inline of the same key The instance-level SOPS branch is added symmetrically to the loader but no existing fixture exercises it; covered when a test surfaces. Redaction of these template-level SOPS values is deferred to feature #6 (twin-load + post-renderer wiring). 330/330 bats green. Stage-3 redaction: shared atlas.values.merged + twin-load in stage-3 Extracts the loader's merge logic into a single named template (atlas.values.merged in templates/_values_loader.tpl) parameterised by .redact. Two callers share the same body: helmfile.values-loader.yaml.gotmpl Helmfile evaluates this once per release at release-time. Calls atlas.values.merged with redact=false and emits the result as the real values blob for helm. helmfile.instance.yaml.gotmpl (per release, when redactSecrets is on) Calls atlas.values.merged twice — once with redact=false (real), once with redact=true (SOPS values rewritten through atlas.redact.value before merge, transitive taint via twin-load). Diffs the two trees via atlas.diff.values to produce a replacement map, b64-encodes it into postRendererArgs, and wires the atlas-redact post-renderer. Redaction-mode runs walk twice per release (real + redacted) plus once at release-time (loader). Regular ArgoCD renders skip the entire stage-3 twin-load block — only the loader runs. So the redaction overhead is paid only by the CI/redacted-render workflow, never by ArgoCD. deployment34 now also asserts: - template SOPS scalar redacted - template SOPS override redacted (list-order precedence preserved) - template SOPS nested map redacted at every leaf - plain (non-SOPS) values pass through untouched 1MB ARG_MAX safety check ported from stage-2. Sidedump path (used by the snapshot-review workflow) deferred — not needed for prototype validation. 335/335 bats green. Stage-3: strip release.condition and release.installed In ATLAS, deploy intent is signalled by the presence of a deployment.yaml entry. Letting helmfile-level condition/installed through would silently produce empty deployments — indistinguishable from a misconfiguration — when ArgoCD discovers the directory and finds no manifests. Drop both fields in the per-instance release rewrite step so app templates remain vanilla-helmfile-readable but render uniformly under ATLAS. Fixture: deployment35 / app-stage3-condinst sets condition: never.enabled (would be a parse error in vanilla helmfile) and installed: false (would be silently skipped). With the strip in place both render normally. Stage-3: add template + instance commonLabels Auto-inject two labels on every stage-3 release so consumers can filter by app-template family and named instance: - template: the app-template directory name - instance: the apps[].name value (or template name when name unset) Useful for ArgoCD ApplicationSet selectors and `helmfile list -l ...`. The remaining commonLabels (cluster, clusterName, deploymentName, variant, clusterGroup) are unchanged. Test covers both the default-name path (deployment30 — instance defaults to template) and the explicit-name path (deployment33 — primary and secondary instances of the same template). Stage-3: auto-munge release names with instance.name When apps[].name is set and differs from the template name, ATLAS rewrites release names so each instance produces a distinct release without forcing the template author to embed `{{ .Values.atlas.instance.name }}`. Default style is `<instance>-<release>` (prefix); apps[].nameStyle: suffix flips to `<release>-<instance>` for naming conventions like `vm-cust-abc`. Both helmfile.instance and the values-loader apply the same munge logic: helmfile.instance rewrites the emitted release.name; the loader applies the same transform when matching the template's author-intended name against helmfile's .Release.Name (now the munged form). Existing fixture deployment33/app-stage3-named drops its manual `{{ .Values.atlas.instance.name }}` munge — the template author intent is the bare release name "stage3-named" and ATLAS produces "primary-stage3-named" / "secondary-stage3-named" automatically. New fixture deployment36/app-stage3-namestyle covers all three flavours in one deployment: default-prefix, explicit-prefix, suffix. Stage-3: fail render when template sets release.postRenderer ATLAS owns the helm post-renderer slot for its redaction pipeline. Helm 4 does not support post-renderer chaining, and silently overwriting a user- set postRenderer would mask intent and produce confusing diffs. Reject the field with a clear pointer to open an issue so the use case can be discussed before any workaround. Negative-test fixture lives under tests/fixtures-negative/ so the standard bulk render in render.bash is unaffected. The bats test invokes helmfile directly with its own --state-values-set roots. Stage-3: default skipSchemaValidation to true (preserve user value) helm runs schema validation only on install/upgrade/lint, not the template/build paths ATLAS exercises during ArgoCD render. The flag primarily affects consumer-side bootstrap, where missing CRDs are a common false-positive failure source. Default it on so that path stays clean. User-set values (true or false) are preserved unchanged. Fixture deployment37 / app-stage3-schema covers both branches: a release without the field gets defaulted to true; a release that sets false explicitly keeps false. Stage-3: re-accept release.secrets, merge last, path-resolve Helmfile's env-level secrets contract: "non-HCL secrets are loaded first but merged last". ATLAS replicates this at the release level — every entry in release.secrets and apps[].secrets lands AFTER all values: entries. Paths resolve template-relative for release.secrets and deployment-relative for apps[].secrets. Decryption goes through the existing SOPS fetchSecretValue path (same as .sops.yaml in values:), so the helm-secrets plugin is not required. helmfile.instance strips release.secrets from the emitted state once the loader has handled them, so helmfile does not also try to decrypt via helm-secrets (avoiding duplicate work and a hard plugin dependency). Fixture deployment38 / app-stage3-secrets demonstrates all three layers: template values, template secrets (overrides values), instance secrets (overrides template secrets). tests/.sops.yaml: relax creation-rule regex from `values\.sops\.yaml$` to `\.sops\.yaml$` so secret fixtures named for their role (template-secret.sops.yaml, instance-secret.sops.yaml) get the testing-age recipient automatically. Stage-3: wire applyListOverride for patches/transformers Route release.strategicMergePatches, release.jsonPatches, and release.transformers through atlas.applyListOverride during the per- instance rewrite, mirroring stage-2's behaviour. Template-relative paths become absolute so helmfile can locate the patch files regardless of consumer CWD; instance-level overrides from deployment.yaml apps[] are appended. release.values is intentionally excluded — stage-3's loader replaces it wholesale and resolves entries itself via the values-loader pipeline. Fixture deployment39 / app-stage3-patches uses a strategicMergePatch to add an annotation onto chart1's ConfigMap, exercising both the path resolution wiring and helmfile's actual patch application via kustomize. Note: convertPaths joins targetPath with the original entry verbatim, so "./foo" becomes "<dir>/./foo" rather than the cleaner "<dir>/foo". kustomize accepts either; normalization is left for a follow-up. Stage-3: path-resolve keyring + set[].file + setString[].file Resolve template-relative paths for the remaining helm-CLI fields that take a path: keyring — single scalar (passed to helm as --keyring) set[].file — nested key inside a list of maps (--set-file) setString[].file — same shape as set[] Mirrors the chart: handling — only ./ and ../ prefixes are rewritten; absolute paths and bare names pass through. Without resolution helm would look in the consumer's CWD instead of the template dir. Fixture deployment40 / app-stage3-paths exercises all three; the bats test caches the build-state YAML once via setup_file (helmfile build is expensive) and asserts each path is absolute and points inside the template dir. Stage-3: flip default engine, port missing wiring, migrate d9 + d11 helmfile.single now defaults to engine: stage3 when deployment.yaml omits the field. All 1-29 fixtures (and any consumer that doesn't opt in explicitly) flow through the per-instance pipeline. Three pieces of stage-2 functionality the previous parallel-fixture work hadn't touched moved over so the existing suite stays green: * _values_loader: seed $merged with .Values.atlas so the chart sees .Values.atlas.* (deployment context, instance fields, redactSecrets) just as stage-2 did. Mirrors the dict "atlas" .Values.atlas baseline in helmfile.single. * helmfile.instance: merge apps[].labels into release.labels with instance taking priority. atlas-template's bootstrap selector (bootstrap: true) depends on this. * helmfile.instance: optional ATLAS_SIDEDUMP_MAP_DIR side-dump second argv element on the atlas-redact post-renderer. snapshot-review's scrub-baseline replays each map against an older-baseline render. Fixture migration: * tests/templates/app-named: drop the manual `{{ .Values.atlas.instance.name }}` munge from release.name; ATLAS auto-munges now (slice 3). Release becomes "primary-app-named" / "secondary-app-named" via prefix. * cluster1-deployment9-{primary,secondary}.bats: update INSTANCE to the munged form. Full suite: 369/369 with stage-3 as default. Retire stage-2 path; thin helmfile.single to fan-out shim helmfile.single now does only what the stage-3 default needs: 1. Validate deployment.yaml exists. 2. Render deployment.yaml as a Go template with the atlas object as context, parse the resulting apps[] list. 3. Emit one helmfiles[] entry per app instance, each pointing at helmfile.instance.yaml.gotmpl with the atlas object augmented by instance.{template, name}. Removed (no longer reachable now that engine: stage2 is gone): - hierarchy twin-load ($realValues / $redactedValues) - hierarchy-level $replMap construction - environments.default.values emission - per-app-template release rewriting (chart paths, secrets rejection, progressive twin-load merge, applyListOverride wiring, redaction post-renderer wiring with side-dump) - the engine if/else branch and the get "engine" "stage2" default The deleted block weighed ~400 lines; helmfile.single is 75 lines now. All of that logic lives in either helmfile.instance (per-release rewrites, redaction post-renderer, side-dump) or _values_loader (twin- load, hierarchy walk, progressive merge). Cleanup: - Drop explicit `engine: stage3` from fixtures 30-40 + postrenderer-fail. - Update doc-comments that referenced stage-2 (helmfile.instance, _values_loader, _functions, helmfile.all, app-stage3-probe). Full suite: 369/369. docs: align README and CLAUDE.md with stage-3-only architecture README.md * "Using the same template multiple times": drop the manual `{{ .Values.atlas.instance.name }}` instruction. Document auto-munge with apps[].nameStyle: prefix|suffix (prefix default). * "Labels": expand the commonLabels table to cover clusterName, clusterGroup, template, instance, variant — all auto-injected now. * "App Templates": add a "How ATLAS treats helmfile release options" table covering the per-instance rewrites (name, chart, values, secrets, condition/installed, skipSchemaValidation, postRenderer, keyring + set[].file + setString[].file, patches/transformers, labels). Lead-in clarifies the "stay close to vanilla helmfile" goal. * "Value Inheritance Logic": insert template-secrets and instance- secrets layers between values and hierarchy. Document the "secrets merged last among per-release lists" semantics. * "Full Load Order": renumber to include the new secrets layers (5-6); standalone-cluster skip range shifts to 10-12. * "How It Works": replace the two-step (load + render) description with the four-stage pipeline (discover, fan-out, per-instance rewrite, release-time loader) that matches the actual code now. CLAUDE.md * Common Commands: switch to the subcommand-first invocation form (`helmfile template -f X` / `helmfile build -f X`) so the commands match the prefix-based allowlist patterns. * Core Template Pipeline: describe all four stages explicitly (helmfile.all → helmfile.single → helmfile.instance → loader) instead of conflating values resolution into helmfile.single. Add .claude/utils/run-bats.sh + allowlist Wraps the test ritual (rm stale tmp dirs, run bats, count results, list failures) into one allowlistable invocation. Modes: .claude/utils/run-bats.sh full suite, summary .claude/utils/run-bats.sh tests/bats/foo.bats single file, full output .claude/utils/run-bats.sh -v full suite, full output Exit code reflects test result (0 pass, 1 fail), so CI/agents can use status without parsing output. Allowlist additions: Bash(.claude/utils/run-bats.sh) and the *-form for arguments. The script's internal rm of /tmp/atlas-bats-* runs without prompting because the script itself is the allowlisted unit. Also rolls in the broader allowlist set added on main (bats *, sops --decrypt *, helm template *, helmfile template *, helmfile build *, etc.) so the worktree shares the same prompt-free surface as main. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

yxxhero requested review from aditmeno, itscaro, mumoshu, xiaomudk and zhaque44 as code owners April 19, 2026 01:22

yxxhero requested a review from Copilot April 19, 2026 01:23

yxxhero force-pushed the fix-2544-subhelmfile-selector-skip branch from 227ba6d to 06e3466 Compare April 19, 2026 01:23

Copilot started reviewing on behalf of yxxhero April 19, 2026 01:23 View session