Repository navigation
Helm chart 1.0.0: native 3.x mode by default, PHP selectable - #963
Conversation
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>
|
Fixes for the first CI run, in 9f95148:
Verified locally:
|
|
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
left a comment
There was a problem hiding this comment.
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_TOKENSrefused;DW_AUTH_TOKENrequired (config_audit.rs,main.rs). schema-upgradeas the single argument with the sameDB_*env, refused while any native pod can write. Thelookupquiescence guard matches the README's "stop every node, back up, then upgrade". Startup never upgrades, and an older schema refuses withnative_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'tDW_*, 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=sqliteisn't refused today, please refuse it until that mode exists (one replica, RWO volume at/app/database). - Compose: #956 moves
docker-compose.published.ymlto 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>
|
At 8be71fe: the Security hold on the schema-upgrade hook is addressed, and so is the Server review. Schema-upgrade hook (Security)
Server review
Counterfactuals against 9f95148
Verified locally
|
|
Security carry review at exact head The chart emits user-controlled annotation maps after annotations it later treats as trusted authority:
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: nativeand a common hook override renders the migration Job with both Please reserve the chart's authority namespace and hook keys instead of allowing value maps to shadow them. At minimum, fail rendering when 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>
|
At 70acff6: the annotation authority hold is addressed.
|
|
Security re-review of the annotation/label correction at exact |
|
|
Security final publication clearance: exact head Carry identity:
Security boundaries remain the source-cleared design:
The exact correction and counterfactual evidence are recorded in the source review. Security clears merge/publication of Helm chart 1.0.0 at |
|
Security final merge-group clearance: generated commit 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. |
|
Security post-publication acceptance for Helm chart 1.0.0:
The disposable verification container used 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. |



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
runtimevalue:native,phpor empty.appVersionand the default tag 3.0.1.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 explicitruntime.latesttag 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.native.schemaUpgrade.enabled(default false) adds apre-upgradehook Job that runs the image with the single argumentschema-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, becauseschema-upgradeneeds 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 templatecannot see the live release, so an emptyruntimerenders native for them. That is called out in the README and UPGRADING.md (setruntimeexplicitly in GitOps values). The chart major bump also keeps^0.3constraints 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
/api/healthliveness,/api/readyreadiness,terminationGracePeriodSeconds45,/app/storagevolume (emptyDir ornative.storage.existingClaim), DB TLS mountDB_*,DB_SSLMODE/DB_SSLROOTCERT,DW_AUTH_DRIVER,config.extraEnv. No Redis, queue, cache,APP_*,DW_AUTH_BACKWARD_COMPATIBLEor metrics limitAPP_VERSIONnow comes from a 2.ximage.tag(orconfig.appVersion), never the native chart appVersionI 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.authTokenwithoutauth.existingSecret,auth.principalTokens, an auth driver other thantoken, a grace period at or below aDW_WORKER_POLL_TIMEOUTset inconfig.extraEnv, and the reservedserver-storagevolume name and/app/storagepath.Native runtime contract used (from source on
main0d88054)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).DW_BIND_ADDRESS=0.0.0.0:8080andDW_RUST_MAINTENANCE_LOOP=1(Dockerfile.native:42-44). The chart does not override them. The binary's own bind default is127.0.0.1:8080(rust/src/main.rs:581-583). Maintenance repair needs that flag (rust/src/runtime/execution.rs:59-68).DW_RUST_EXPERIMENTAL(rust/src/main.rs:248-259)./api/healthand/api/readyroutes (rust/src/runtime/http.rs:28-29) skip auth (http.rs:435-437). Health reports database liveness (http.rs:1005-1009). Ready reportsschema_ready(http.rs:1011-1034), which is false when the maintenance scheduler stopped or is unhealthy (execution.rs:342-347). Readiness is therefore/api/readyonly and liveness is/api/health, per Native: one schedule's persistent database error stalls every schedule, the repair pass and readiness #955.schema-upgrade(main.rs:260-276).schema-upgradeuses the sameDB_*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_CONNECTION,DB_HOST,DB_PORT,DB_DATABASE,DB_USERNAME,DB_PASSWORDorDB_URL(main.rs:106-153,rust/src/database_tls.rs:279). TLS usesDB_SSLMODE,DB_SSLROOTCERTand MySQLDB_SOCKET(database_tls.rs:239-256). Native waits for an unreachable database instead of exiting (main.rs:19-62).max_connections(4)(rust/src/runtime/postgres.rs:359,mysql.rs:727). Documented as four connections per pod.DW_AUTH_TOKENis required and must not be empty (main.rs:576-580). A release build accepts only thetokendriver (main.rs:219-229).DW_AUTH_BACKWARD_COMPATIBLEmust be unset or true (rust/src/config_audit.rs:571).DW_PRINCIPAL_TOKENSmust be empty (config_audit.rs:639).DW_METRICS_WORKFLOW_TASK_FAILURE_TYPE_LIMITmust be 20 (config_audit.rs:623), so native mode leaves the PHP auth and metrics keys out.DW_SERVER_KEYthenAPP_KEYis read (rust/src/runtime/schedule_list.rs:752-756).DW_STORAGE_ADMISSION_FILEandDW_STORAGE_ADMISSION_SOURCErefuse startup (config_audit.rs:664,666). The chart never sets them. UnknownDW_*names only warn unlessDW_ENV_AUDIT_STRICT=1(config_audit.rs:478-491).main.rs:82-96,:630-636). The defaultDW_WORKER_POLL_TIMEOUTis 30 seconds (rust/src/runtime/query_authority.rs:79-83), hence the 45-second default grace./api/ready(docker-compose.published.yml:55-68).Release tooling
resources/release/source-release.jsongainshelm_chart.app_version(3.0.1).server.versionstays the frozen PHP 2.11.8.scripts/ci/sync-source-release.mjsnow renders Chart.yamlappVersion, the release image annotation, thevalues.yamldefault tag and the README tag fromapp_version. The PHP-mode examples and CI fixtures still followserver.version. An older record without the field falls back toserver.version, sorelease.ymlcan still check old release sources.helm_chart_release.py validate-sourcechecks againstapp_version, and its template and install fixtures passauth.authToken. That makes the publish workflow's clean kind install a native install against the MySQL fixture.native-release.ymldoes 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 sixci/*-values.yaml: pass.ct lint(chart-testing 3.14.0, repoct.yaml): pass for every fixture.kubeconform -strictv0.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(replacestest_helm_native_image_guard.py): 13 tests OK. It covers default native render, shared server options, the schema-upgrade hook, native refusals, PHP shape andAPP_VERSION, tag refusals in both directions, and the--is-upgraderuntime 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 onmainin 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) andvalidate-source: pass.check-public-boundary.shandcheck-docker-workflow-isolation.py: pass.php -lon 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, publisheddurableworkflow/server:3.0.1, indexsha256:485295819c7739d3e937a2c89b8593532de0d9c2fbc8f2e7aaddb0b45fe4b319, MySQL 8.4 fixture) passed end to end. The steps:runtimegives one Deployment and no PHP roles or Redis Secret. The pod went Ready andhelm testpassed./api/readyreturned"status":"ready","qualification":"release"and/api/cluster/inforeturned"version":"3.0.1","implementation":"rust".runtimestayed native.runtimeand rendered withruntime=native.The cluster and its network were removed afterwards.
CI wiring:
helm-chart-checks.ymlruns 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 pinruntime: phpthrough their values.Follow-ups and open points
maintriggershelm-chart-release.yml, which publishes chart 1.0.0. Only merge when that is intended.Recreateand one replica. The schema enum stays mysql, mariadb and pgsql.native.storage.existingClaimmust be ReadWriteMany. This is documented, but the chart cannot see which namespaces enable payload storage, so it does not enforce it./api/healthchecks 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.ymlstill defaults to 3.0.0 while the chart packages 3.0.1. Nothing generates native versions there yet.🤖 Generated with Claude Code