Skip to content

healthCheckExprs field docs merge three failure modes into one clause, and two of them behave oppositely #4154

Closed

Description

The field documentation for healthCheckExprs describes one failure where there are three, and they behave differently enough that an operator following the sentence looks in the wrong place.

api/v1alpha1/applicationdefinitions_types.go says: "a bad expression or a not-yet-installed CRD makes the HelmRelease hang until its timeout (the backstop)."

A syntactically invalid expression never reaches a wait. helm-controller v1.5.0 calls cel.NewStatusReader(obj.Spec.HealthCheckExprs) at internal/controller/helmrelease_controller.go:202-210 and, on error, marks Ready false and Stalled with reason InvalidCELExpressionReason, then returns reconcile.TerminalError(err). That runs before reconcileChartTemplate, so no Helm action happens at all: no install, no wait, no timeout, no retry. The operator gets a stalled release with a named reason almost immediately.

An expression that parses but fails at evaluation does hang, for the whole window. Flux builds these with env.Parse rather than env.Compile, deliberately, and says why in runtime/cel/status_evaluator.go: not every CRD carries the five standard top-level fields, so no variables can be declared. Nothing catches a misspelled field name at construction. statuss.phase == 'x' is accepted and then fails on every poll. The evaluation error goes to the generic status reader, which records UnknownStatus with the error attached and returns a nil Go error, so the poller carries on. Unknown is not in the {Failed, Current} terminal set that ssa/manager_wait.go fails fast on, so polling continues every five seconds until the Helm timeout expires.

A not-yet-installed CRD is the third case. It also runs the wait out, and that is the half the current sentence gets right.

Worth naming alongside, because it is the same trap one level down: an expression guarded with has(status.phase) still errors when the object carries no status key at all. Names resolve as keys of the raw map[string]any from UnstructuredContent(), so a missing root key errors before the presence test runs. I measured seven idioms against an object with no status, and all seven return no such attribute(s):

has(status.phase)
has(status.phase) && status.phase == 'Succeeded'
status.?phase.orValue('') == 'Succeeded'
has(self.status)
'status' in self
dyn(status).phase == 'Succeeded'
size(status) > 0

No expression in this model survives an absent status, so a CR whose controller has not written a status yet takes the hanging path, not the false path a has() guard suggests.

The fix is to describe the three cases separately. A syntax error stalls terminally with InvalidCELExpressionReason and never starts a wait. A name or field that does not resolve at evaluation time runs the wait to its timeout, with the CEL error visible in the message. A missing CRD does the same. Splitting only compile failure from missing CRD, which is what this issue originally proposed, still files every bad expression under the terminal case and stays wrong.

The sentence is duplicated verbatim into internal/crdinstall/manifests/cozystack.io_packagesources.yaml and packages/system/application-definition-crd/definition/cozystack.io_applicationdefinitions.yaml, so it ships to clusters and is read through kubectl explain by people who cannot check it against the controller source.

Measured on fluxcd/pkg/runtime v0.100.2 (helm-controller v1.5.0), cli-utils v0.37, cel-go v0.26.

Activity

  1. added
    kind/bugCategorizes issue or PR as related to a bug
    area/apiIssues or PRs related to the cozystack-api aggregated API server
    on Sep 7, 2026
  2. changed the title [-]healthCheckExprs field docs say a bad CEL expression hangs until timeout, but it stalls terminally without ever waiting[/-] [+]healthCheckExprs field docs merge three failure modes into one clause, and two of them behave oppositely[/+] on Sep 8, 2026
  3. added a commit that references this issue on Sep 23, 2026
    ce35da4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/apiIssues or PRs related to the cozystack-api aggregated API serverkind/bugCategorizes issue or PR as related to a bugtriage/needs-triageIndicates an issue needs triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions