Skip to content

Helm chart 1.0.0: native 3.x mode by default, PHP selectable - #963

Merged
rmcdaniel merged 5 commits into
mainfrom
feat/946-helm-native-mode
Nov 3, 2026
Merged

rmcdaniel merged 5 commits into
mainfrom
feat/946-helm-native-mode

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

Step 2 of #946. Chart 0.3.9 (#947) refuses native 3.x tags because every template was PHP-shaped. This adds a native chart mode, makes it the default for new installs in a new chart major, and keeps PHP selectable for existing installs.

Design and default

New runtime value: native, php or empty.

  • Empty on install runs native. PHP Server 2.x is frozen at 2.11.8 and gets no further releases, so a new user should land on the line that keeps shipping. Chart goes to 1.0.0 with appVersion and the default tag 3.0.1.
  • Empty on upgrade never switches a PHP release. It stays native only when the live server Deployment carries workflows.durable-workflow.dev/runtime: native, which only chart 1.x writes. Every release chart 0.x installed runs PHP and lacks it, so its upgrade refuses with a message that names both choices and links UPGRADING.md. This covers --reuse-values, which keeps 0.x defaults. A client-side dry run cannot see the Deployment, so it refuses too and asks for an explicit runtime.
  • Tag guard in both directions. A 3.x or latest tag in PHP mode is refused, as in 0.3.9. A 2.x tag in native mode is refused too. Custom tags and digests pass in either mode.
  • Schema upgrades are explicit. native.schemaUpgrade.enabled (default false) adds a pre-upgrade hook Job that runs the image with the single argument schema-upgrade. It never runs on install. Before rendering it, the chart reads the live server Deployment and pods and refuses while replicas are above zero or a server pod is not terminal, because schema-upgrade needs every native node stopped and a backup first. The documented flow is: scale to zero, back up, upgrade with the hook, then turn it off again.

Why not keep PHP as the default and make native opt-in? Then every new install would get a frozen runtime, and the chart default would trail the published Compose path (#920). The cost of switching the default is that GitOps controllers that render with helm template cannot see the live release, so an empty runtime renders native for them. That is called out in the README and UPGRADING.md (set runtime explicitly in GitOps values). The chart major bump also keeps ^0.3 constraints from picking 1.0.0 up. Native pods also refuse a PHP database at startup and stay unready, so a mistaken switch cannot write to it.

What renders

native php
server Deployment native image, no command or args, port 8080, /api/health liveness, /api/ready readiness, terminationGracePeriodSeconds 45, /app/storage volume (emptyDir or native.storage.existingClaim), DB TLS mount unchanged from 0.3.9
Service, Ingress, PDB, HPA, NetworkPolicy, ServiceAccount, helm test pod reused unchanged
ConfigMap DB_*, DB_SSLMODE/DB_SSLROOTCERT, DW_AUTH_DRIVER, config.extraEnv. No Redis, queue, cache, APP_*, DW_AUTH_BACKWARD_COMPATIBLE or metrics limit unchanged, except that APP_VERSION now comes from a 2.x image.tag (or config.appVersion), never the native chart appVersion
app and database Secrets reused unchanged
worker Deployment/HPA/PDB, scheduler CronJob, migration Job, Redis Secret not rendered unchanged
schema-upgrade Job opt-in pre-upgrade hook not rendered

I diffed PHP-mode output for every existing CI fixture against main. Apart from the chart label, the version label and the config checksums, the output is identical.

Native render-time refusals mirror native startup refusals: a missing auth.authToken without auth.existingSecret, auth.principalTokens, an auth driver other than token, a grace period at or below a DW_WORKER_POLL_TIMEOUT set in config.extraEnv, and the reserved server-storage volume name and /app/storage path.

Native runtime contract used (from source on main 0d88054)

  • Entrypoint, user and port: Dockerfile.native:51 (ENTRYPOINT ["/usr/local/bin/durable-workflow-server"]), :45 (USER 1000:1000, so the pod uses runAsUser and fsGroup 1000), :48 (EXPOSE 8080), :50 (STOPSIGNAL SIGTERM), :47 (VOLUME /app/database /app/storage).
  • The image sets DW_BIND_ADDRESS=0.0.0.0:8080 and DW_RUST_MAINTENANCE_LOOP=1 (Dockerfile.native:42-44). The chart does not override them. The binary's own bind default is 127.0.0.1:8080 (rust/src/main.rs:581-583). Maintenance repair needs that flag (rust/src/runtime/execution.rs:59-68).
  • A release build needs no DW_RUST_EXPERIMENTAL (rust/src/main.rs:248-259).
  • Probes: /api/health and /api/ready routes (rust/src/runtime/http.rs:28-29) skip auth (http.rs:435-437). Health reports database liveness (http.rs:1005-1009). Ready reports schema_ready (http.rs:1011-1034), which is false when the maintenance scheduler stopped or is unhealthy (execution.rs:342-347). Readiness is therefore /api/ready only and liveness is /api/health, per Native: one schedule's persistent database error stalls every schedule, the repair pass and readiness #955.
  • Subcommands: a single argument from the fixed list, including schema-upgrade (main.rs:260-276). schema-upgrade uses the same DB_* settings for sqlite, pgsql, mysql and mariadb (main.rs:278-307). It never runs at startup, and the procedure is stop every node, back up, run once (rust/README.md:251-291, docs/rust-server-upgrade-takeover.md:153-162).
  • DB settings: DB_CONNECTION, DB_HOST, DB_PORT, DB_DATABASE, DB_USERNAME, DB_PASSWORD or DB_URL (main.rs:106-153, rust/src/database_tls.rs:279). TLS uses DB_SSLMODE, DB_SSLROOTCERT and MySQL DB_SOCKET (database_tls.rs:239-256). Native waits for an unreachable database instead of exiting (main.rs:19-62).
  • Pool: max_connections(4) (rust/src/runtime/postgres.rs:359, mysql.rs:727). Documented as four connections per pod.
  • Auth: DW_AUTH_TOKEN is required and must not be empty (main.rs:576-580). A release build accepts only the token driver (main.rs:219-229). DW_AUTH_BACKWARD_COMPATIBLE must be unset or true (rust/src/config_audit.rs:571). DW_PRINCIPAL_TOKENS must be empty (config_audit.rs:639). DW_METRICS_WORKFLOW_TASK_FAILURE_TYPE_LIMIT must be 20 (config_audit.rs:623), so native mode leaves the PHP auth and metrics keys out. DW_SERVER_KEY then APP_KEY is read (rust/src/runtime/schedule_list.rs:752-756).
  • DW_STORAGE_ADMISSION_FILE and DW_STORAGE_ADMISSION_SOURCE refuse startup (config_audit.rs:664,666). The chart never sets them. Unknown DW_* names only warn unless DW_ENV_AUDIT_STRICT=1 (config_audit.rs:478-491).
  • Shutdown: graceful drain on SIGTERM (main.rs:82-96, :630-636). The default DW_WORKER_POLL_TIMEOUT is 30 seconds (rust/src/runtime/query_authority.rs:79-83), hence the 45-second default grace.
  • Compose reference: the native readiness check reads /api/ready (docker-compose.published.yml:55-68).

Release tooling

resources/release/source-release.json gains helm_chart.app_version (3.0.1). server.version stays the frozen PHP 2.11.8. scripts/ci/sync-source-release.mjs now renders Chart.yaml appVersion, the release image annotation, the values.yaml default tag and the README tag from app_version. The PHP-mode examples and CI fixtures still follow server.version. An older record without the field falls back to server.version, so release.yml can still check old release sources. helm_chart_release.py validate-source checks against app_version, and its template and install fixtures pass auth.authToken. That makes the publish workflow's clean kind install a native install against the MySQL fixture. native-release.yml does not touch chart defaults, before or after this change.

Verification run locally

All in disposable containers on Colima (--rm --init, CPU, memory and PID limits, read-only mounts):

  • helm lint (alpine/helm 3.16.2): chart plus all six ci/*-values.yaml: pass.

  • ct lint (chart-testing 3.14.0, repo ct.yaml): pass for every fixture.

  • kubeconform -strict v0.6.7 on all six rendered fixtures against Kubernetes 1.27, 1.28, 1.29 and 1.30: 58/58 valid each.

  • scripts/ci/test_helm_runtime_modes.py (replaces test_helm_native_image_guard.py): 13 tests OK. It covers default native render, shared server options, the schema-upgrade hook, native refusals, PHP shape and APP_VERSION, tag refusals in both directions, and the --is-upgrade runtime requirement.

  • scripts/ci/test_database_tls_chart.py: 4 OK (CA reaches the native server and schema-upgrade Job, plus every PHP role).

  • scripts/ci/test_helm_chart_release.py: 28 of 30 pass. The same 2 kind-teardown fixture tests fail identically on main in the alpine container, so the failure is environmental.

  • node --test scripts/ci/test-source-release.mjs (with php-cli): 3/3. sync-source-release.mjs --check: synchronized.

  • helm_chart_release.py package, check-version (0.3.9 to 1.0.0) and validate-source: pass. check-public-boundary.sh and check-docker-workflow-isolation.py: pass. php -l on the edited PHPUnit test. PHPUnit itself was not run locally.

  • Native kind smoke (scripts/helm-chart-native-kind-smoke.sh, kind 0.23.0, node v1.29.4, published durableworkflow/server:3.0.1, index sha256:485295819c7739d3e937a2c89b8593532de0d9c2fbc8f2e7aaddb0b45fe4b319, MySQL 8.4 fixture) passed end to end. The steps:

    1. Install with no runtime gives one Deployment and no PHP roles or Redis Secret. The pod went Ready and helm test passed. /api/ready returned "status":"ready","qualification":"release" and /api/cluster/info returned "version":"3.0.1","implementation":"rust".
    2. An upgrade with an empty runtime stayed native.
    3. The schema-upgrade hook was refused while a pod ran. After scaling to 0, the hook ran and the pod came back Ready.
    4. A stub release without the marker refused an empty runtime and rendered with runtime=native.
    5. Uninstall left nothing behind.

    The cluster and its network were removed afterwards.

CI wiring: helm-chart-checks.yml runs the new test file and the native kind smoke after the PHP smoke in the same cluster. The PHP smoke and the memo-transition guard pin runtime: php through their values.

Follow-ups and open points

  • Merging to main triggers helm-chart-release.yml, which publishes chart 1.0.0. Only merge when that is intended.
  • The chart does not run native on SQLite. That would need a ReadWriteOnce volume, Recreate and one replica. The schema enum stays mysql, mariadb and pgsql.
  • External payloads: native stores them on local disk only. With more than one replica, native.storage.existingClaim must be ReadWriteMany. This is documented, but the chart cannot see which namespaces enable payload storage, so it does not enforce it.
  • Liveness on /api/health checks database liveness, so a long database outage restarts pods (PHP had the same behaviour). Server owner: say if you prefer a TCP liveness probe.
  • docker-compose.published.yml still defaults to 3.0.0 while the chart packages 3.0.1. Nothing generates native versions there yet.
  • Takeover is planned for deprecation in 3.1.x (Rust #325: note takeover's planned deprecation in 3.1.x #930). UPGRADING.md tells PHP users to move while on 3.0.x.

🤖 Generated with Claude Code

The chart was PHP-shaped and refused native 3.x tags (#947). It now has
a runtime value. Native, the default for new installs, renders one
server Deployment for the native image with /api/health liveness,
/api/ready readiness, a drain-length grace period and a payload storage
volume, and no worker, scheduler, migration Job or Redis. PHP mode
renders exactly what chart 0.3.9 rendered and stays available for
existing 2.x installs.

An empty runtime stays native on upgrade only when the live server
Deployment carries the native runtime marker, so helm upgrade never
moves a PHP release from chart 0.x to native on its own. A 3.x tag in
PHP mode and a 2.x tag in native mode are refused at render time, as
are native settings the binary would refuse at startup.

Native schema upgrades run only through an opt-in pre-upgrade Job that
invokes the image's schema-upgrade command, and the chart refuses to
render it while a server pod can still run.

The source release record gains helm_chart.app_version so the chart can
package native 3.0.1 while server.version stays the frozen PHP 2.11.8.
CI covers both runtimes, including a kind smoke of the published native
image that exercises the runtime and schema-upgrade guards.

Refs #946

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The memo-transition CI scenario took its PHP successor tag from the chart
appVersion, which is now the native 3.0.1, so PHP mode correctly refused
it before the guard ran. It now reads the 2.x tag the PHP fixture pins.

Native pods mount no service account token (the native Server never
calls the Kubernetes API) unless native.automountServiceAccountToken is
set, and their containers get an ephemeral-storage request and limit
from native.ephemeralStorage unless the resources already name one. The
payload emptyDir counts toward that limit.

Also tidy the new test and smoke code Sonar flagged: String.raw and a
quantifier in the source-release tests, assertLessEqual, and [[ ]] in
the native kind smoke.

Refs #946

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Fixes for the first CI run, in 9f95148:

  • Memo-transition guard (kind job). scripts/ci/helm-memo-transition-upgrade.sh took its PHP successor tag from Chart.yaml appVersion, which is now native 3.0.1. PHP mode correctly refused that tag before the guard ran. The script now reads the 2.x tag that the PHP fixture ci/inline-secrets-values.yaml pins, and it fails if that tag is not 2.x. The chart's refusal is unchanged. No other PHP scenario relied on the default tag: the PHP fixtures, the TLS test and the PHP kind smoke all set a 2.x or custom tag. Re-run locally in kind 0.23.0 / node v1.29.4, all 20 memo scenarios pass. The native smoke also passes in the same cluster.
  • Sonar security rating. The two findings were real:
    • S6865, service account token: native pods now set automountServiceAccountToken: false, because the native Server never calls the Kubernetes API. native.automountServiceAccountToken: true re-enables it for a sidecar.
    • S6870 / S6897, storage limit: native containers get an ephemeral-storage request and limit from native.ephemeralStorage (128Mi / 2Gi) unless server.resources already names one. This also bounds the payload emptyDir.
    • The schema-upgrade Job gets the same treatment.
  • Sonar code smells in new code. Fixed: String.raw and the {2} quantifier in the source-release generator and tests, assertLessEqual, and [[ ]] in the native smoke.

Verified locally:

  • test_helm_runtime_modes.py: 14 OK, with new assertions for the token and storage settings.
  • test_database_tls_chart.py: 4 OK.
  • helm lint on the chart and all 6 fixtures.
  • kubeconform -strict on the 6 rendered fixtures for Kubernetes 1.27 to 1.30: 58/58 valid.
  • node --test scripts/ci/test-source-release.mjs with php-cli: 3/3. sync-source-release.mjs --check passes.
  • Memo-transition and native kind smokes: both pass.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security bounded review at exact head 9f95148 found one publication-blocking schema authority gap.

native-schema-upgrade-job.yaml renders the database-mutating Job whenever native.schemaUpgrade.enabled is true, even when Release.IsUpgrade is false. validateNativeSchemaUpgradeQuiescence performs all lookups only inside Release.IsUpgrade and otherwise succeeds. The current unit test deliberately proves this with plain helm template. A direct Kubernetes/GitOps apply of that render does not enforce Helms pre-upgrade annotation, so it can start schema-upgrade alongside a newly created or already running Server, contrary to the comments and backup/stop-first contract.

Even in upgrade mode, an invisible server Deployment passes, and a zero-replica PHP Deployment passes because the helper does not require the native runtime marker. That lets client-side/GitOps rendering and the wrong predecessor runtime bypass the positive authority check.

Please render the Job only when Release.IsUpgrade is true; when enabled, require a visible live server Deployment carrying workflows.durable-workflow.dev/runtime=native, replicas exactly zero, no live release server pod, and no live release HPA that can repopulate it before the hook. Refuse when lookup cannot prove those conditions. GitOps/client-rendered schema changes should use an explicitly documented manual stop/backup/one-shot path instead of emitting an unchecked Job. Add counterfactual coverage for install/template, invisible/PHP predecessor, live HPA, replicas and pods. Runtime selection itself and the new service-account/ephemeral-storage defaults are source-clear so far; this hook gate is the current Security hold.

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server review of the native runtime facts at 9f951481, checked against native source at 923d45b3 (native-3.0.1). One blocker, two recommendations; everything else checks out.

Blocker: liveness on /api/health restarts every pod during a database outage.
/api/health (rust/src/runtime/http.rs:1005-1008) calls database_live() and answers 503 degraded when the database is unreachable. With liveness on it, a database blip restarts every native pod at once. That's worse than staying up and reconnecting, because the pool reconnects by itself. I suggested /api/health earlier; that was wrong, sorry. For native, liveness should be tcpSocket: {port: http}, which answers your open question: yes, TCP. Keep readiness on /api/ready, which correctly fails on schema, database and a stalled maintenance loop (#955).

Recommendation 1: refuse more than one replica with pod-local payload storage.
Native stores external payload objects only with the local driver, under /app/storage (payload_cleanup.rs module docs). With server.replicaCount > 1 or server.autoscaling.enabled and server-storage on emptyDir (or an RWO claim), an upload accepted by one pod is unreadable from the others. Its run then fails on fetch, and cleanup on another pod can't delete it. Please have validateNative fail in that case unless native.storage.existingClaim is set, and document that the claim must be ReadWriteMany for more than one replica. A single replica on emptyDir is fine apart from losing objects on restart, which the template comment already says.

Recommendation 2: grace period.
A poll may request timeout_seconds up to 60 (http.rs:2118, 2131); DW_WORKER_POLL_TIMEOUT (default 30, query_authority.rs:53) is only the default for polls that don't ask. At 45 s, SIGKILL can cut a 60 s held poll. It's harmless (the uncommitted claim rolls back and the worker re-polls), but a default of 65 covers every allowed poll. The validateNative check against DW_WORKER_POLL_TIMEOUT is still right to keep.

Checked and correct:

  • One process per pod with the in-process maintenance loop (DW_RUST_MAINTENANCE_LOOP=1, Dockerfile.native).
  • Port 8080, SIGTERM drain via axum graceful shutdown (main.rs:634).
  • automountServiceAccountToken: false, since native never calls the Kubernetes API.
  • Token driver only; DW_PRINCIPAL_TOKENS refused; DW_AUTH_TOKEN required (config_audit.rs, main.rs).
  • schema-upgrade as the single argument with the same DB_* env, refused while any native pod can write. The lookup quiescence guard matches the README's "stop every node, back up, then upgrade". Startup never upgrades, and an older schema refuses with native_schema_upgrade_required.
  • No storage-admission env is passed (native refuses DW_STORAGE_ADMISSION_FILE/SOURCE, #951).
  • The ConfigMap's PHP-only keys (QUEUE_CONNECTION, REDIS_*, CACHE_STORE, APP_*) aren't DW_*, so native's audit ignores them. If they're only rendered in PHP mode, even better.

Other open questions:

  • SQLite as a follow-up: fine. If native mode with DB_CONNECTION=sqlite isn't refused today, please refuse it until that mode exists (one replica, RWO volume at /app/database).
  • Compose: #956 moves docker-compose.published.yml to 3.0.1. It's queued.

…erver review

Security hold: the schema-upgrade Job rendered whenever the option was
on, including installs, helm template and GitOps renders, where a direct
apply ignores the hook annotation, and the upgrade check passed an
invisible or non-native predecessor. The hook now renders only during
helm upgrade, and only when lookup proves every Server is stopped: the
release's server Deployment is visible, carries the native runtime
marker and has exactly zero replicas, autoscaling is off in the values,
no HorizontalPodAutoscaler targets it, and no server pod of the release
is running. Everything else is refused with a pointer to a documented
manual path (scale to zero, back up, run schema-upgrade as your own Job,
start).

Server review:
- Native liveness is a TCP check on the http port. /api/health fails
  while the database is unreachable, so probing it would restart every
  pod during a database outage. Readiness stays on /api/ready.
- More than one native replica, or autoscaling, needs
  native.storage.existingClaim (ReadWriteMany): payload objects are
  pod-local. Native now defaults to one replica (server.replicaCount
  empty takes the runtime default, still 2 for PHP), and its PDB renders
  only when more than one pod can run.
- The default grace period is 65 seconds, above the 60-second poll
  timeout a worker may request.
- SQLite is refused in both runtimes until the chart supports it.

Tests: runtime-mode tests for each refusal, and kind smoke scenarios for
the live hook gate (replicas, client-side render, autoscaling values, a
live autoscaler, a live pod, unmarked and PHP-marked predecessors, and
the authorized render with the database CA).

Refs #946

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

At 8be71fe: the Security hold on the schema-upgrade hook is addressed, and so is the Server review.

Schema-upgrade hook (Security)

  • The Job now renders only when .Release.IsUpgrade is true. With native.schemaUpgrade.enabled set on an install, helm template or a GitOps render, the chart refuses. The message points to the new README section "Manual path (GitOps and helm template)": scale to zero, back up, apply a one-shot schema-upgrade Job (manifest given), then start the Server.
  • During an upgrade, the chart requires positive proof from lookup. It refuses whenever any of these does not hold:
    • the server Deployment is visible;
    • it carries workflows.durable-workflow.dev/runtime=native;
    • its spec.replicas is exactly 0;
    • server.autoscaling.enabled is off;
    • no live HPA targets the Deployment;
    • no non-terminal server pod of the release exists.
      An empty result under a client-side dry run is therefore refused.

Server review

  • Native liveness is now tcpSocket on http, set in native.livenessProbe. Readiness stays on /api/ready.
  • More than one replica, or autoscaling, is refused unless native.storage.existingClaim is set, and the docs say it must be ReadWriteMany. Native now defaults to 1 replica: an empty server.replicaCount takes the runtime default, and PHP still gets 2. The PodDisruptionBudget renders in native mode only when more than one pod can run.
  • The default grace period is 65 seconds. The check against DW_WORKER_POLL_TIMEOUT is kept.
  • SQLite is refused in both modes with an explicit message. The schema enum now lists sqlite only so the chart can explain the refusal.

Counterfactuals against 9f95148

  • The new test_helm_runtime_modes.py (17 tests) fails 8 of them on the old chart:

    • the install/template hook render;
    • the native liveness, replicas and PDB defaults;
    • the shared-options test, whose fixture no longer enables the hook;
    • more than one replica, and autoscaling, without a claim;
    • SQLite in native mode, both through connection and through DB_URL;
    • SQLite in PHP mode.
  • The old chart renders the Job under both a plain template and --is-upgrade with no visible predecessor.

  • In kind (0.23.0, node v1.29.4, published 3.0.1, MySQL 8.4), I ran the same live dry-runs against both charts:

    Scenario 9f95148 8be71fe
    Client-side render, no cluster view allowed refused
    Autoscaling enabled in values allowed refused
    Live HPA targeting the Deployment allowed refused
    Unmarked predecessor allowed refused
    PHP-marked predecessor allowed refused
    Replicas above zero refused refused
    Live server pod refused refused

    Replicas above zero and a live pod were already refused before this change and remain covered. The authorized render still emits the pre-upgrade Job with schema-upgrade, automountServiceAccountToken: false and the database CA, and the real hook ran to completion.

Verified locally

  • test_helm_runtime_modes.py: 17 OK.
  • test_database_tls_chart.py: 4 OK.
  • helm lint on the chart and all 6 fixtures.
  • kubeconform -strict on Kubernetes 1.27 and 1.30: 55/55 valid. There are fewer resources than before because a single-replica native install has no PDB and the native fixture no longer renders the hook.
  • sync-source-release --check passes.
  • In kind:
    • the memo-transition script passes all 20 scenarios;
    • the full native smoke, including every new hook scenario, exits 0.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security carry review at exact head 8be71fe517a042c303c49b55283fa3c077811d94: the prior schema-quiescence and runtime-facts corrections are directionally correct, but one publication-blocking authority override remains.

The chart emits user-controlled annotation maps after annotations it later treats as trusted authority:

  • A PHP server Deployment has no chart-owned runtime annotation, then appends commonAnnotations. Rendering PHP with commonAnnotations.workflows.durable-workflow.dev/runtime=native produces a PHP Deployment carrying the exact native marker. On the next upgrade, empty runtime accepts that marker and silently selects native; the new schema-upgrade authority check accepts the same forged marker on a stopped PHP predecessor.
  • native-schema-upgrade-job.yaml emits the three helm.sh/hook* authority annotations and then appends commonAnnotations. A value such as commonAnnotations.helm.sh/hook=post-upgrade creates a duplicate later key. Helm/Kubernetes YAML decoding uses the later value, so the mutating Job can cease to be a pre-upgrade hook and run after the new Server starts—the original concurrency hazard through a different values path.
  • The same ordering lets commonAnnotations and role podAnnotations override workflows.durable-workflow.dev/memo-payload-storage, which the live transition guard later trusts.

Concrete exact-head render evidence, using a disposable Helm 3.19 container: PHP mode plus the common runtime annotation renders

metadata:
  name: forged-durable-workflow-server
  annotations:
    workflows.durable-workflow.dev/memo-payload-storage: "dual-v1"
    workflows.durable-workflow.dev/runtime: native

and a common hook override renders the migration Job with both "helm.sh/hook": pre-install,pre-upgrade and the later helm.sh/hook: post-upgrade. The schema Job uses the same later-append pattern at lines 24–29.

Please reserve the chart's authority namespace and hook keys instead of allowing value maps to shadow them. At minimum, fail rendering when commonAnnotations, server.podAnnotations, worker.podAnnotations, or scheduler.podAnnotations contains a workflows.durable-workflow.dev/ key, and when commonAnnotations contains helm.sh/hook, helm.sh/hook-weight, or helm.sh/hook-delete-policy. Preserve normal user annotations. Add counterfactuals proving a PHP render cannot acquire the native marker, a schema-upgrade render cannot replace its hook type/weight/delete policy, and memo authority markers cannot be replaced on the workload or pod.

The corrected source otherwise closes the prior hold: non-upgrade/client/GitOps schema renders refuse; a live native-marked zero-replica Deployment, no values/live HPA, and no release server pod are positively required; liveness is TCP with readiness unchanged; pod-local storage refuses multiple replicas/autoscaling; native defaults to one replica; grace is 65 seconds; SQLite refuses; service-account and resource bounds remain. Security hold is now limited to annotation authority shadowing above.

Security found that user label and annotation maps were emitted after
the chart's own markers, so a later duplicate key could replace them:
commonAnnotations could give a PHP Deployment the native runtime marker
that the next empty-runtime upgrade and the schema-upgrade gate trust,
override the schema-upgrade Job's helm.sh/hook so it ran after the new
Server started, or replace the memo storage marker the PHP transition
guard trusts.

The chart now refuses any user map (commonAnnotations, commonLabels,
role podAnnotations and podLabels, Service, Ingress and ServiceAccount
annotations) that sets a key under workflows.durable-workflow.dev/ or
helm.sh/hook, helm.sh/hook-weight or helm.sh/hook-delete-policy. As
defense in depth, user maps are emitted first, without reserved keys and
without any key the chart sets on that object, and the chart's keys
follow, so no rendered map carries a key twice and a chart value always
wins. Renders with ordinary user metadata are otherwise unchanged.

Tests refuse every reserved key in every user map in both runtimes,
reject Security's two examples, and parse renders with a loader that
fails on duplicate keys. The native kind smoke refuses a hook override
against the live gate and checks the authorized Job keeps user
annotations and exactly one of each hook key.

Refs #946

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

At 70acff6: the annotation authority hold is addressed.

  • Reserved keys: the chart now refuses any user map that sets a key under workflows.durable-workflow.dev/ (and so also the runtime and memo markers), or helm.sh/hook, helm.sh/hook-weight or helm.sh/hook-delete-policy. The maps covered are:

    • commonAnnotations and commonLabels;
    • podAnnotations and podLabels for server, worker, scheduler and bootstrap;
    • the Service, Ingress and ServiceAccount annotations.

    The check lives in the always-rendered ConfigMap. The error names the map and the key.

  • Defense in depth: every metadata block now emits user keys first and the chart's own keys after. User keys go through one helper, durable-workflow.userMetadata, which drops reserved keys and any key the chart sets on that object. That covers the Argo/Flux annotations, the checksums, app.kubernetes.io/*, the selector labels and the hook keys. No rendered map carries a key twice, and a chart value wins even if the check were skipped. With ordinary user maps, every CI fixture renders semantically identically to 8be71fe (checksums aside).

  • Tests (test_helm_runtime_modes.py, 20 OK):

    • each of the 13 maps against 6 reserved keys, in both runtimes, is refused (156 subtests);
    • Security's two examples are refused: PHP with commonAnnotations setting runtime=native, and helm.sh/hook=post-upgrade;
    • a positive case renders with ordinary user metadata and colliding non-reserved keys. It is parsed with a loader that fails on duplicate keys, and the chart values win.

    All 6 rendered fixtures also pass the duplicate-key check.

  • Counterfactual on 8be71fe:

    • the same tests fail 159 subtests;
    • PHP with the forged runtime marker renders exit 0 with 7 runtime: native lines;
    • the hook override renders exit 0 with 7 helm.sh/hook: post-upgrade lines;
    • in kind, a live --dry-run=server schema upgrade with commonAnnotations={"helm.sh/hook":"post-upgrade"} is allowed on 8be71fe and refused now.
  • Kind (0.23.0 / v1.29.4 / published 3.0.1 / MySQL 8.4):

    • the memo-transition script passes all 20 scenarios;
    • the native smoke exits 0. It now also refuses the hook override, and checks that the authorized Job keeps a user annotation and carries each of its three hook keys exactly once.
  • Lint and schemas: helm lint passes on all fixtures, kubeconform 1.27 and 1.30 shows 55/55 valid, and sync-source-release --check passes.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-review of the annotation/label correction at exact 70acff667f242d5a8fe0650d1ee0091a46fe5556: the original authority bypass is closed at source. All 13 user metadata maps are globally refused on the workflows.durable-workflow.dev/ prefix and the three Helm hook keys; object helpers strip reserved/chart-owned keys, emit user metadata first, and chart authority last. The duplicate-key loader exercises ordinary metadata in native and PHP renders. Independent bounded Helm 3.19 renders on this exact tree refuse both the PHP forged native-runtime marker and the post-upgrade hook override with nonzero status and the exact reserved-key reason; review containers/worktree are removed. Source hold is lifted for this correction. Final chart-1.0.0 merge/publication clearance still waits terminal hosted checks and a current-main carry, because the PR is presently behind main.

@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security final publication clearance: exact head c40a57f60a0f4b6bc09faf477a822e8e27fdeee6 is terminal and remains the reviewed chart source.

Carry identity:

  • c40a57f6… is the plain merge of current reviewed main 9524e80b into corrected chart head 70acff66….
  • k8s/helm and the two chart test files are byte-unchanged from 70acff66…; the full PR's stable patch identity was unchanged in the carry review.
  • Fresh readback shows all 60 reported hosted contexts terminal: 59 success, 1 expected skip, 0 failed/cancelled/timed out/pending.

Security boundaries remain the source-cleared design:

  • upgrades never silently switch PHP/native: live predecessor lookup and runtime markers decide; ambiguous/empty lookup refuses;
  • the schema-upgrade hook is upgrade-only and requires a visible native-marked, zero-replica predecessor, no configured autoscaling, no targeting HPA and no live server pod; the manual GitOps path is documented;
  • native pods and the upgrade job default automountServiceAccountToken: false, with explicit opt-in, and receive bounded ephemeral-storage resources;
  • SQLite, unsafe replica/storage combinations and inappropriate PDB shapes refuse; readiness/liveness/grace behavior matches the reviewed runtime contract;
  • every user metadata map reserves workflows.durable-workflow.dev/ plus the Helm hook keys, strips chart-owned keys and emits chart authority last, preventing forged runtime markers, hook overrides and duplicate YAML keys.

The exact correction and counterfactual evidence are recorded in the source review. Security clears merge/publication of Helm chart 1.0.0 at c40a57f6…. Any content, head or publication-workflow change requires fresh qualification.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 3, 2026 00:23
@rmcdaniel
rmcdaniel added this pull request to the merge queue Nov 3, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security final merge-group clearance: generated commit 17c52f5f9e57d07b2a270c39c9ded07a4c3e50c4 has parents accepted protected main 02654110766abc37ac944ddbb930394cb61b199c (#968) and reviewed head c40a57f60a0f4b6bc09faf477a822e8e27fdeee6. Relative to main it changes exactly the PR’s 45 paths, and all 45 resulting blobs are byte-identical to the reviewed head. The merge-group suite is terminal with 51 successful contexts plus one intentional skip and no failed, cancelled, timed-out, action-required, or pending context.

The exact merge group is Security-cleared. Merging publishes chart 1.0.0, so public OCI availability, anonymous pull/install evidence, package digest, embedded source revision, and default Server image identity remain a separate post-merge acceptance boundary.

Merged via the queue into main with commit 17c52f5 Nov 3, 2026
60 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security post-publication acceptance for Helm chart 1.0.0:

  • protected-main run 41559952646 completed successfully at exact merge 17c52f5f9e57d07b2a270c39c9ded07a4c3e50c4; both protected-source validation jobs and the publish/verify job are green;
  • the publish job built the package once, anonymously resolved the immutable Server image, passed existing-version preflight, removed its registry login, and then passed a clean anonymous OCI pull/install in a new kind cluster;
  • uploaded durable-workflow-helm-release/v1 evidence binds chart durable-workflow 1.0.0 to reviewed chart source 70acff667f242d5a8fe0650d1ee0091a46fe5556, package sha256:8f422c5a4a221e59d5e4b8391708736d8a27543a5dbe530ca6f9e283a5c964dc, and docker.io/durableworkflow/server:3.0.1 index sha256:485295819c7739d3e937a2c89b8593532de0d9c2fbc8f2e7aaddb0b45fe4b319;
  • independent anonymous readback of ghcr.io/durable-workflow/charts/durable-workflow:1.0.0 resolves OCI manifest sha256:6d980357be8361e7abd785c0c5ba4bc2a3a4f128c9fca7ed047d175346c554af;
  • an independent clean Helm 3.16.2 pull returned the same OCI digest and byte-identical package SHA-256, whose packaged Chart.yaml names version 1.0.0, appVersion 3.0.1, image reference docker.io/durableworkflow/server:3.0.1, and source revision 70acff667f242d5a8fe0650d1ee0091a46fe5556;
  • an independent anonymous image readback returns the evidence's exact 3.0.1 index digest and the expected amd64/arm64 children.

The disposable verification container used --rm and bounded read-only/no-new-privileges/cap-drop limits; no container remains. The downloaded public evidence was moved to the local Trash after comparison.

Chart 1.0.0 publication is accepted. This acceptance covers the published 1.0.0 artifact only; the planned appVersion update after Server 3.0.2 is a new release change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant