fix: TLS Fixes in helm chart - #3194
Draft
SantanM wants to merge 7 commits into
Draft
Conversation
logto-check failed with: wget: unrecognized option '--ca-certificate=/etc/d2e/tls/ca.pem' That flag is GNU wget; the containers run busybox:latest, whose wget offers only --no-check-certificate. So the check could not verify at all, and the only busybox-compatible spelling would have been a verification bypass -- the exact thing this branch removes everywhere else. Both affected init containers now use curl --cacert, which verifies chain and hostname by default, on an image already pulled for their own pod: the trex image for logto-check, the worker image for dependencies-check (its Dockerfile installs curl). No new image pull, no second tag to track. The loops become `until <check>; do sleep; done` rather than polling and testing for a marker file, which also drops the success-trex/success-dataflow temp files. Verified curl exists and supports --cacert by running both images, rather than assuming it -- assuming a flag existed is what caused this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The trex container copied the internal CA from /etc/d2e/tls/ca.pem and pointed WEBAPI_TRUST_CERTS at the same path, but never mounted the d2e-tls volume -- the only container referencing that path without it. The cp failed, update-ca-certificates had nothing to add, and the process started with public roots only, so every internal TLS call died: UnknownIssuer from rustls (Deno edge functions, DENO_TLS_CA_STORE=system) and PKIX from the GraalVM WebAPI. Nothing caught it. The args have no set -e, so the failed cp did not stop startup; the liveness probe is `curl -fk`, which disables the very check that was broken; and trex has no readiness probe, so the pod reported Ready. CI runs a real `helm install --wait` plus `kubectl wait --for=condition=Ready` and went green throughout. Mount d2e-tls read-only at /etc/d2e/tls, matching the five sibling containers that already do, which fixes the OS-store and WEBAPI_TRUST_CERTS paths together since both read that one file. Then assert instead of hoping: the file is non-empty, is PEM, the copy succeeded, and -- the part that actually matters -- the anchor is present in the rebuilt bundle. update-ca-certificates exits 0 over an empty directory, so "it ran" proves nothing; verifying the self-signed CA against ca-certificates.crt proves membership. Starting untrusted is worse than not starting. Verified in containers: volume absent -> exit 1; valid CA -> "1 added", entrypoint reached; non-PEM -> exit 1; and the old script with the volume absent -> exit 0, started anyway. End to end, curl with neither -k nor --cacert reached idp.d2e.svc.cluster.local and trex.d2e.svc.cluster.local at HTTP 200, ssl_verify_result 0 -- chain and hostname verified from the OS store alone. TLS__EXTRA__CA_CRTS parity with compose is still missing and is left for a separate change.
…into SantanM/fix-tls-bug
Flow runs crashed with ssl.SSLCertVerificationError (CERTIFICATE_VERIFY_FAILED, unable to get local issuer certificate) from prefect/events/clients.py -> websockets, plus a preceding "run-flow: deployment lookup failed" from run-flow.sh's urllib call. docker-compose sets SSL_CERT_FILE and REQUESTS_CA_BUNDLE on alp-dataflow-gen-worker to /etc/ssl/certs/ca-certificates.crt; the chart never got those two lines. Both setups use the same https PREFECT_API_URL, so that was the entire difference -- which is why compose and the DQD run on CI were fine while k8s failed. Without them only clients that explicitly pass certifi.where() are covered (prefect's httpx client, requests). stdlib urlopen and websockets use OpenSSL's defaults, which under `pixi run` resolve inside the env prefix (<env>/ssl/cert.pem, capath empty) and never see the internal CA -- so neither update-ca-certificates nor the certifi append reaches them. Worse, flows exec in their own per-plugin pixi env provisioned at run time, so no startup-time append can ever reach them; an inherited env var can. Also fail loudly, as with trex: assert the CA is present, is PEM, copies, and actually lands in /etc/ssl/certs/ca-certificates.crt. That last check matters more here than anywhere else, because the two new env vars point every Python HTTP stack at that bundle -- if the anchor is missing from it, verification fails everywhere. Verified in ghcr.io/ohdsi/d2e-dataflow-gen-worker:develop against a server presenting the real leaf. With the chart's rendered args and env: stdlib urlopen, requests and websockets all pass. Guard cases: volume absent -> exit 1, non-PEM -> exit 1, valid CA -> proceeds. Most directly, in /var/lib/d2e-flows/d2e-flows/baked/.pixi/envs/default -- the exact env from the traceback -- the crash reproduces without the env vars and is gone with them.
… name
Caddy had no certificate for gateway.<ns>.svc.cluster.local and aborted the
handshake ('no certificate available' -> 'Received fatal alert:
internal_error'). WebAPI dials that name for the jwks_uri advertised by OIDC
discovery, so its OIDC bean failed to build and every analytics-svc call
proxying to WebAPI returned 500.
Caddy is the server on this hop, so no client-side trust setting helps; it
already trusts the internal CA as a client for its upstreams. Add the internal
cert/key to its d2e-tls mount and a site block serving that name, with routes
mirroring the existing :8080 block. SERVER__CRT is the environment's public
certificate and has no cluster-internal SANs; CI hid this by using one
self-signed cert for both.
Verified: caddy validate passes on caddy:2.11-alpine with two independent SNI
policies; deployed and confirmed from trex that the jwks_uri returns HTTP 200
with tls_verify=0. The rendered Caddyfile is byte-identical to the running
ConfigMap.
SantanM
force-pushed
the
SantanM/fix-tls-bug
branch
from
August 27, 2026 02:38
3dde768 to
8045838
Compare
SantanM
force-pushed
the
SantanM/fix-tls-bug
branch
from
August 27, 2026 10:14
c145e2a to
8045838
Compare
SantanM
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
logto-check failed with:
wget: unrecognized option '--ca-certificate=/etc/d2e/tls/ca.pem'
That flag is GNU wget; the containers run busybox:latest, whose wget offers only --no-check-certificate. So the check could not verify at all, and the only busybox-compatible spelling would have been a verification bypass -- the exact thing this branch removes everywhere else.
Both affected init containers now use curl --cacert, which verifies chain and hostname by default, on an image already pulled for their own pod: the trex image for logto-check, the worker image for dependencies-check (its Dockerfile installs curl). No new image pull, no second tag to track.
The loops become
until <check>; do sleep; donerather than polling and testing for a marker file, which also drops the success-trex/success-dataflow temp files.Verified curl exists and supports --cacert by running both images, rather than assuming it -- assuming a flag existed is what caused this.
Merge Checklist
Please cross check this list if additions / modifications needs to be done on top of your core changes and tick them off. Reviewer can as well glance through and help the developer if something is missed out.
developbranch)