Skip to content

[helm] Add existingSecret form for the readiness probe credential - #4331

Open
morazow wants to merge 2 commits into
apache:mainfrom
morazow:oss-4330-probe-existing-secret
Open

morazow wants to merge 2 commits into
apache:mainfrom
morazow:oss-4330-probe-existing-secret

Conversation

@morazow

@morazow morazow commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #4330

The cluster-health readiness probe authenticates as a Fluss client, so a SASL-enforced cluster needs a credential for it. tablet.readinessProbe.healthCheckAuth takes that credential as a literal, which the chart inlines into the StatefulSet's exec command, leaving it in plain text in the rendered manifest.

Keeping it out of the manifest is possible today only through the generic top-level secrets.env list: the credential sits far from the probe settings it belongs to, and the link between the two is a naming convention on READINESS_HEALTH_CHECK_AUTH rather than something the values schema expresses.

The chart already has an existingSecret {name, key} idiom for exactly this, used three times under security.*. This PR accepts the same shape for the probe credential.

Brief change log

New value, rendered as a secretKeyRef instead of an inline export in the probe command:

security:
  readinessProbe:
    existingSecret:
      name: fluss-readiness-probe-auth
      key: auth          # optional, defaults to `auth`
  • fluss.security.readinessProbe.env renders READINESS_HEALTH_CHECK_AUTH as a secretKeyRef on every server container. The exec probe inherits the container environment, so readiness-check.sh and ClusterHealthReadinessCheck are unchanged — no Java or shell change in this PR.
  • fluss.security.readinessProbe.validateAuth rejects setting the credential more than once. It now has three possible sources and any two conflict: the probe command would export the literal over the Secret-sourced env var, and two env entries of the same name make the StatefulSet invalid.

Two decisions the issue left open:

  1. Placement — under security, not tablet. Every component's probe authenticates with the same credential; the coordinator gains a health-based readiness probe in [helm] First-class coordinator HA: standby-aware readiness gate #4115 and should not need a second copy of it. A key under tablet.* that also configures the coordinator container would misdescribe itself.
  2. Backward compatibility — a new key, not a string-or-map overload. tablet.readinessProbe.healthCheckAuth keeps its string type, so existing values files render exactly as before and the inline form stays supported. Overloading one key to accept both a string and a map is not expressible in a values schema and is awkward under --set.

This supersedes #4329, which documents the secrets.env route for the same credential. Happy to keep that one and rebase instead if reviewers prefer the incremental split.

Tests

helm/tests/readiness_probe_auth_test.yaml (new suite, 6 cases):

  • renders no probe credential by default
  • sources it from a Secret on both the tablet and coordinator containers, and does not export it in the probe command
  • defaults the Secret key to auth
  • rejects inline + existingSecret
  • rejects existingSecret + a secrets.env entry named READINESS_HEALTH_CHECK_AUTH
  • rejects inline + a secrets.env entry named READINESS_HEALTH_CHECK_AUTH

helm lint ./helm and helm unittest ./helm pass (56 suites, 154 tests).

API and Format

No API or storage format change. Chart values gain security.readinessProbe.existingSecret, defaulting to {}; no existing value changes meaning or type.

Documentation

website/docs/install-deploy/deploying-with-helm.md: a new "Probe credentials on a SASL cluster" subsection covering both the inline and the Secret-backed form, plus a row in the Security Configuration parameter table.

The cluster-health readiness probe authenticates as a Fluss client, so a
SASL-enforced cluster needs a credential for it.
tablet.readinessProbe.healthCheckAuth takes it as a literal, which the
chart inlines into the StatefulSet's exec command, leaving it in plain
text in the rendered manifest. Keeping it out is possible today only
through the generic secrets.env list, where the credential sits far from
the probe settings it belongs to and the link between the two is the
naming convention on READINESS_HEALTH_CHECK_AUTH rather than something
the values schema expresses.

Accept the chart's existing existingSecret {name, key} idiom for it:

  security:
    readinessProbe:
      existingSecret:
        name: fluss-readiness-probe-auth
        key: auth

The chart renders READINESS_HEALTH_CHECK_AUTH as a secretKeyRef on every
server container; the exec probe inherits the container environment, so
readiness-check.sh and ClusterHealthReadinessCheck are unchanged.

Two placement decisions. The key lives under security rather than under
tablet, because every component's probe authenticates with the same
credential - the coordinator gains a health-based probe in apache#4115 and
needs no second copy. And it is a new key rather than a map accepted on
tablet.readinessProbe.healthCheckAuth, so that key keeps its string type
and existing values files keep rendering the inline form unchanged.

The credential now has three possible sources, and any two of them
conflict: the probe command exports the literal over the Secret-sourced
env var, and two env entries of the same name make the StatefulSet
invalid. Reject every combination at render time.

Closes apache#4330. Supersedes apache#4329, which documents the secrets.env route
for the same credential.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[helm] First-class existingSecret form for the readiness probe credential

1 participant