feat: Add ConditionTemplate support in releaseSpec

[agarov]

This pull request adds support for a new conditionTemplate field for Helmfile releases, allowing users to specify dynamic, templated conditions for enabling or disabling releases. It also improves the handling of the condition field to accept boolean literals (true/false) directly, and updates documentation and tests to reflect these enhancements.

New Features:

• Added support for a conditionTemplate field in ReleaseSpec, which allows release conditions to be specified as Go templates that must render to a boolean. If set, conditionTemplate takes precedence over condition [1] [2] [3] [4].
• Implemented template execution for conditionTemplate in the release templating logic, converting the rendered result to a boolean and updating the release's condition accordingly [1] [2].

Improvements to Condition Handling:

• Enhanced the condition field to directly accept boolean literals (true/false) in addition to value lookups, simplifying configuration for static conditions [1] [2] [3].

Documentation Updates:

• Updated user documentation and skill reference files to describe the new conditionTemplate field, its precedence, and the expanded capabilities of the condition field [1] [2] [3] [4].

Testing:

• Added and updated unit tests to cover conditionTemplate and the new boolean literal handling for condition [1] [2] [3] [4].

[agarov]

I created this PR in order to resolve #2665.

I'm not proficient in Go so any advice is welcome.

The main problem was that we currently accept a path and not directly a boolean, so I made this little hack to cast the template result into a string and cast it back as a boolean. This keeps backward compatibility but for sure is not the cleanest.

[yxxhero]

Code Review: ConditionTemplate support

Overall this is a clean, well-scoped PR that follows the existing installedTemplate/waitTemplate/verifyTemplate pattern faithfully. The feature makes sense and the core implementation is correct. Below are issues by severity.

🔴 Bugs / should fix

1. Wrong field name in error message (pkg/state/release.go:67)

The new ConditionTemplate block reuses the .version = text from the neighboring blocks:

return nil, fmt.Errorf("failed executing template expressions in release \"%s\".version = \"%s\": %v", r.Name, ts, err)

This is a copy-paste from the pre-existing buggy WaitTemplate/InstalledTemplate/VerifyTemplate blocks (which all mistakenly say .version). Since this is fresh code, use the correct field:

"...in release \"%s\".conditionTemplate = \"%s\": %v"

(FWIW, the same typo in the three older blocks is worth a follow-up cleanup.)

🟡 Correctness / design concerns

2. Silent discard of condition when conditionTemplate is set

In updateBoolTemplatedValues, when both are set, conditionTemplate overwrites condition with no warning. This matches the installed/installedTemplate precedent, so it's acceptable — but the docs claim "takes precedence" without stating the original condition is ignored. Consider logging at debug level when condition is non-empty and gets overridden, and/or document the discard explicitly.

3. Behavior change in ConditionEnabled is a (minor) breaking change

Previously condition: true errored with "must be in the form 'foo.enabled'". Now it silently enables the release. Low risk since the old behavior was an error, but it should be called out in the PR description / changelog as a behavior change, not just a "feat". Also, a YAML literal condition: true (unquoted boolean) relies on the YAML lib coercing bool→string to "true"; worth confirming that works with the project's yaml package for both quoted and unquoted forms.

4. getBoolRefFromStringTemplate accepts YAML-1.1 booleans beyond true/false

go-yaml treats yes/no/on/off/1/0 as booleans. So a conditionTemplate rendering to "yes" silently becomes true. The docs say "Must render to a boolean" — strictly accurate, but users may be surprised. This is consistent with existing *Template fields, so not blocking; just be aware when documenting.

🟢 Test gaps

5. Dead template field in TestConditionEnabled (pkg/state/state_test.go:3528)

A template *string field was added to the test struct and passed as ConditionTemplate: tt.template to ConditionEnabled, but:

• No test case ever sets template (always nil), and
• ConditionEnabled never reads ConditionTemplate anyway — the template→condition conversion happens earlier in updateBoolTemplatedValues.

So the field is vestigial and suggests a misunderstanding of the flow. Either remove it, or — better — add tests in state_exec_tmpl_test.go covering: template rendering to "false", and the precedence case (both condition + conditionTemplate set).

6. Missing coverage

• No test that conditionTemplate rendering to a non-boolean (e.g. "maybe") produces the expected error.
• No test for precedence (condition set + conditionTemplate set → conditionTemplate wins).

🔵 Nit

7. Doc inconsistency

skills/helmfile/SKILL.md describes conditionTemplate only as "Templated condition flag", omitting the "must render to boolean / takes precedence" detail that docs/configuration.md includes. Keep them in sync.

---

Summary: The core feature is correctly implemented and follows house patterns. Before merge I'd ask for at least #1 (error message) and #5/#6 (test cleanup + precedence/error coverage). #3 deserves a changelog note. The rest are doc/polish.

[yxxhero]

Deep review: data-flow, design, and edge cases

Did a deeper pass tracing the actual execution paths. Findings below, most-impactful first.

1. helmfile list silently ignores conditionTemplate (most impactful)

Traced the two ConditionEnabled call sites:

• Run path (apply/sync/destroy/…): app.go:1061 runs ExecuteTemplates() → converge(templated) → PlanReleases → SelectReleases → markExcludedReleases (state.go:3346). By here updateBoolTemplatedValues has already converted conditionTemplate → condition. ✅ works.
• List path: ListReleases (app.go:754) → ForEachState (app.go:1367) → list() (app.go:821) calls ConditionEnabled(r, resolvedState.Values()) directly on ResolveDeps() output — ExecuteTemplates is never called. So r.ConditionTemplate is still the un-rendered template and r.Condition is empty → ConditionEnabled returns true.

Net effect: a release gated by conditionTemplate: '{{ false }}' is correctly skipped on apply, but helmfile list reports it as Enabled: true. This is a pre-existing limitation inherited from installedTemplate etc. (none of the *Template fields are reflected in list), but since this PR markets conditionTemplate as the enable/disable driver, the inconsistency is now user-visible and surprising.

Options: (a) document explicitly that *Template fields (incl. conditionTemplate) do not affect helmfile list; or (b) have list() template the state first. I'd lean toward (a) for this PR + a tracking issue for (b).

2. Semantic overload of Condition string + fragile cross-site contract

After rendering, the Condition field holds one of two unrelated things:

• a values-lookup path (vault.enabled), or
• a boolean literal ("true" / "false").

The two are coupled by an implicit string contract across two files:

• state_exec_tmpl.go:75 writes r.Condition = fmt.Sprintf("%t", *condition)
• state.go:3369 reads it back via switch strings.ToLower(r.Condition) { case "true"/"false" }

If anyone changes the %t formatting (or a future refactor routes through a different setter), ConditionEnabled silently breaks with no compile-time linkage. Suggest either:

• a code comment at both sites cross-referencing the contract, or
• better: a small helper like setConditionFromBool(r, b) used only by the template path, so the relationship is explicit.

3. Condition-as-boolean-literal is an undocumented semantic tier

ConditionEnabled now has three behaviors with no single place documenting them:

1. empty → enabled
2. true/false (any case) → literal boolean
3. foo.enabled → values lookup

The docs touch each individually (inline comment at configuration.md, a table row) but there's no consolidated explanation. Worth a short "### Condition" subsection, especially to state that the literal form bypasses values lookup entirely (the new test "condition true literal takes precedence over values lookup" codifies this, but a user reading docs wouldn't know).

4. Behavior-change / breaking-change classification

condition: true previously errored ("must be in the form 'foo.enabled'"); now it enables. Since old behavior was an error this won't break working configs, but it's strictly speaking a behavior change. The PR title/type is feat: — consider a BREAKING CHANGE: or at least a CHANGELOG note so users grepping for condition semantics see it. Also worth confirming the project's YAML lib coerces an unquoted condition: true (bool node) into the string "true" for a string field; quoted "true" is safe regardless.

5. Test coverage gaps (deepening the earlier note)

The state_exec_tmpl_test.go:68 "Has template expressions in boolean values" case is the natural place to exercise conditionTemplate, and it only covers the → "true" path. Please add:

• conditionTemplate rendering to false (mirrors the existing WaitTemplate: {{ false }} case),
• precedence: condition: "foo.enabled" + conditionTemplate: '{{ true }}' → assert Condition == "true",
• error: conditionTemplate: '{{ "maybe" }}' → assert updateBoolTemplatedValues returns the conditionTemplate: error.

The template *string field added to the TestConditionEnabled table (state_test.go:3528) is never populated and ConditionEnabled never reads ConditionTemplate anyway — it's dead. Remove it, or move the real coverage into state_exec_tmpl_test.go where the conversion actually happens.

6. Minor / nits

• Error field name (release.go:67): still says .version = — copy of the pre-existing typo in the Wait/Installed/Verify blocks. Use .conditionTemplate. (Nice opportunity to fix the three older ones too.)
• Convergence: adding a field to the reflect.DeepEqual convergence check (state_exec_tmpl.go:124) is harmless — static templates still converge in 2 iterations like the other *Template fields. No regression, just noting it's part of the loop now.
• mergo/inherit pointer sharing: ConditionTemplate is *string; mergo.Merge shallow-copies the pointer during inherit. This is safe only because ExecuteTemplateExpressions deep-clones via a yaml round-trip before mutating. Non-obvious; a one-line comment near the field or the merge would help future maintainers.
• getBoolRefFromStringTemplate (state_exec_tmpl.go:43) accepts YAML-1.1 booleans (yes/no/on/1/…), so conditionTemplate: '{{ "yes" }}' becomes true. Consistent with sibling fields, but "Must render to a boolean" in docs is slightly looser than users may expect.
• Doc sync: skills/helmfile/SKILL.md row omits the "must render to boolean / precedence" detail present in docs/configuration.md.

Verdict

Core mechanism is correct on the main run path. Blocking for me would be #1 (document the list gap or fix it) and the test coverage in #5; the rest is design hygiene and polish.

[agarov]

Hello @yxxhero,
Thanks a lot for your detailed review !

I just updated the PR with the following changes :

• Fixed the copy-paste error labels in ReleaseSpec.ExecuteTemplateExpressions: conditionTemplate now reports .conditionTemplate instead of .version, and I also corrected the neighboring waitTemplate, installedTemplate, and verifyTemplate labels.
• Added state-level coverage for:
  • conditionTemplate rendering to false
  • conditionTemplate taking precedence over condition
  • conditionTemplate rendering to a non-boolean value returning an error
  • unquoted YAML condition: true being unmarshaled into the literal "true" condition path
• Removed the unused ConditionTemplate field from TestConditionEnabled, since template conversion happens earlier in ExecuteTemplates via updateBoolTemplatedValues.
• Made the conditionTemplate -> condition conversion contract explicit with a small helper.
• Updated docs and the Helmfile skill reference to clarify that:
  • condition can be empty, direct true/false, or a values lookup path ending in .enabled
  • direct boolean conditions bypass values lookup
  • conditionTemplate must render to a boolean
  • when both condition and conditionTemplate are set, the rendered template value replaces condition
• Added a changelog note for conditionTemplate and direct condition: true/false support.

On the helmfile list concern: for me it's currently working as expected, the release is rightfully shown as ENABLED: false when setting conditionTemplate: '{{ false }}' for example.

[yxxhero]

Review: feat: Add ConditionTemplate support in releaseSpec

Overall the feature is well-designed and mirrors the existing installedTemplate/waitTemplate/verifyTemplate lifecycle cleanly. The new unit tests pass and golangci-lint is clean. Requesting changes because the PR breaks the pre-existing TestGenerateID suite (see below).

---

🔴 Blocker — breaks TestGenerateID (CI-failing)

Adding ConditionTemplate *string to ReleaseSpec changes the struct's spew serialization, which feeds the FNV hash in generateValuesID (pkg/state/temp.go:66). As a result the golden hashes hardcoded in pkg/state/temp_test.go are now stale:

go test ./pkg/state/... -run TestGenerateID   # FAILS

I reproduced this deterministically on this PR branch; main passes. The 6 want values in temp_test.go need updating:

Case	Old	New
baseline (L41)	foo-values-6fcd6fc479	foo-values-6765d87f7c
different bytes content (L48)	foo-values-6556658b7b	foo-values-6d7db5f8b
different map content (L55)	foo-values-fd86f44b	foo-values-5c55f66dd
different chart (L61)	foo-values-58bdc49774	foo-values-7665888bf4
different name (L67)	bar-values-856b998888	bar-values-bc6f974bd
specific ns (L73)	myns-foo-values-7bdfd95fbd	myns-foo-values-76f8c7c596

(Yes, this golden-hash approach is fragile to any ReleaseSpec field addition — but the update belongs in this PR so make test stays green.)

🟡 Minor — CHANGELOG placement (CHANGELOG.md:3)

The new ### Added block is inserted under ## [1.4.1] - 2026-03-03, which is already shipped (latest release is v1.6.0). It belongs in a new ## [Unreleased] section at the top.

🟡 Minor — stale doc comment (pkg/state/state.go:3357)

This line is now inaccurate:

// If the condition is specified but not in the form 'foo.enabled', it returns an error.

After this PR, true/false literals are not in foo.enabled form yet they no longer error. Worth reconciling with the newly inserted "cast boolean" line above it.

🟢 Nits

• setConditionFromBool (pkg/state/state_exec_tmpl.go:51) is a single-use one-liner (r.Condition = fmt.Sprintf("%t", condition)); could be inlined, though harmless.
• Grammar: "casted boolean" → "cast boolean" (cast is already past tense).
• Like the other *Template fields, when both condition and conditionTemplate are set, the original condition is silently overwritten with no log. Consistent with existing behavior, but a logger.Debugf would help users debugging precedence.

✅ What's good

• Design parity with the other templated bool fields: render in ExecuteTemplateExpressions → resolve to bool in updateBoolTemplatedValues → nil out the template field. Precedence (conditionTemplate overrides condition) is correctly implemented and tested (state_exec_tmpl_test.go:113).
• The release.go error-message fixes (".version =" → ".waitTemplate =" / ".installedTemplate =" / ".verifyTemplate =") are a welcome drive-by fix for copy-paste bugs.
• condition: true/false literal handling is backward-compatible — it converts a previous hard error ("must be in the form 'foo.enabled'") into valid behavior. strings.ToLower correctly handles True/FALSE.
• Test coverage is solid: true/false literals, precedence, unresolvable template ({{ "maybe" }}), and YAML bool unmarshal.
• golangci-lint: 0 issues; go vet: clean.