feat(helm): add engine authentication and TLS support (FB-1943) - #23
Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Cert-manager signing rotation breaks
- Cert-manager signing Certificate and Secret names now derive stably from each signing key id instead of the list index, preserving outgoing key material when keys are prepended.
- ✅ Fixed: Auth test ignores nodeHostSuffix
- The auth test engine host now appends engineSpec.nodeHostSuffix instead of hardcoding .svc.cluster.local.
- ✅ Fixed: customEngineConfig overrides chart auth
- Chart-owned auth fields are stripped from customEngineConfig before merging when auth is enabled, while non-owned auth settings still merge normally.
Or push these changes by commenting:
@cursor push 584ed9b656
Preview (584ed9b656)
diff --git a/helm/AGENTS.md b/helm/AGENTS.md
--- a/helm/AGENTS.md
+++ b/helm/AGENTS.md
@@ -29,10 +29,10 @@
- **Pod label set is intentionally narrower than resource label set.** `fbinstance.podLabels` excludes `app.kubernetes.io/version` so that an `appVersion` bump on an unrelated component does not mutate the pod template and trigger an unwanted rollout.
- **`engineSpec` defaults, per-engine override.** Shared engine pod settings live under `engineSpec`. Each entry under `engines:` may override any of those keys for its own StatefulSets. Fall back to `engineSpec.defaultStorage` when `engines[i].storage` is omitted.
- **Engine FQDN format.** `engine-{name}-node-{i}-0.{baseName}-hl.{namespace}.svc{nodeHostSuffix}` (default `nodeHostSuffix` is `.cluster.local`). Generated by the `fbinstance.engineConfig` helper. Changing this format breaks engine peer discovery in `config.yaml`.
-- **`customEngineConfig` is merged into the rendered engine `config.yaml`.** `fbinstance.engineConfig` builds a canonical document on the new Firebolt Core schema (`schema_version: "1.0"`, `engine.{id,nodes,termination_grace_period}`, `instance.{type,multi_engine.metadata_endpoint}`, plus `instance.auth`/`endpoints` when `auth.enabled`/`tls.engine.enabled`) and deep-merges `.Values.customEngineConfig` on top at the root. Use it to set `instance.id` (which the engine fans out to `account_id`/`account_name`/`organization_id`/`organization_name`), add a `logging:` block, or add auth settings the chart doesn't expose as first-class values (`instance.auth.oidc`, `password_login`, `jwt`, `preferred_authorization_server`, `signing_algorithm`) — these deep-merge alongside the admin account and signing keys the chart renders. Chart-authoritative paths are silently stripped before the merge: `schema_version`, `engine.id`, `engine.nodes`, `engine.termination_grace_period`, `instance.type`, `instance.multi_engine`, and — only while `tls.engine.enabled` is actually rendering it — `endpoints.http.listeners`. `engine.termination_grace_period` is derived from `engineSpec.terminationGracePeriodSeconds` (minus a 5s margin, floored at 1s). Add new fields here rather than carving out new top-level value keys. The rendered file is YAML; the engine reads it from `/var/lib/firebolt/config.yaml` (the data dir, `--data-dir /var/lib/firebolt`).
+- **`customEngineConfig` is merged into the rendered engine `config.yaml`.** `fbinstance.engineConfig` builds a canonical document on the new Firebolt Core schema (`schema_version: "1.0"`, `engine.{id,nodes,termination_grace_period}`, `instance.{type,multi_engine.metadata_endpoint}`, plus `instance.auth`/`endpoints` when `auth.enabled`/`tls.engine.enabled`) and deep-merges `.Values.customEngineConfig` on top at the root. Use it to set `instance.id` (which the engine fans out to `account_id`/`account_name`/`organization_id`/`organization_name`), add a `logging:` block, or add auth settings the chart doesn't expose as first-class values (`instance.auth.oidc`, `password_login`, `jwt`, `preferred_authorization_server`, `signing_algorithm`) — these deep-merge alongside the admin account and signing keys the chart renders. Chart-authoritative paths are silently stripped before the merge: `schema_version`, `engine.id`, `engine.nodes`, `engine.termination_grace_period`, `instance.type`, `instance.multi_engine`, and — only while `auth.enabled` / `tls.engine.enabled` are actually rendering them — `instance.auth.enabled`, `instance.auth.admin`, `instance.auth.local.signing_keys`, and `endpoints.http.listeners`. `engine.termination_grace_period` is derived from `engineSpec.terminationGracePeriodSeconds` (minus a 5s margin, floored at 1s). Add new fields here rather than carving out new top-level value keys. The rendered file is YAML; the engine reads it from `/var/lib/firebolt/config.yaml` (the data dir, `--data-dir /var/lib/firebolt`).
- **Pensieve `default_account_id` reads from `customEngineConfig.instance.id`.** `templates/metadata-service-configmap.yaml` populates `<pensieve_lite><default_account_id>` from the same value the engine sees as `instance.id`, so Pensieve and the engine agree on the account ULID without two separate value keys.
- **Auth and TLS secrets are provisioned by `existingSecret` or `certManager`, never both.** `fbinstance.secretSourceName` (`_helpers.tpl`) resolves each of `auth.signingKeys[]`, `tls.gateway`, and `tls.engine` to the Secret name to mount, and `fail`s the render if a source sets both or neither (detected via a non-empty `existingSecret.secretRef` vs. a non-empty `certManager.issuerRef.name` — the `certManager` map is always present with structural defaults, so `issuerRef.name` is the actual signal). `fbinstance.usesCertManager` mirrors the same check as a non-failing template guard, used by `certificates.yaml` (which also gates each artifact's `Certificate` on its owning feature — `tls.gateway.enabled` / `tls.engine.enabled` / `auth.enabled` — so a stale `certManager` block on a disabled feature never renders one) to decide whether to render a `Certificate`. `auth.admin.password` mirrors the same `existingSecret.secretRef` shape for consistency but only ever supports `existingSecret` — a password isn't a certificate, so there's no `certManager` alternative — and is validated by its own `fail` check rather than the shared helper. Nothing under `instance.auth` is rendered at all unless `auth.enabled` — the engine refuses to start if `admin`/`oidc`/`preferred_authorization_server` are present while auth is disabled.
-- **`auth.signingKeys` is ordered; the first entry signs.** The engine always uses `signing_keys[0]` to sign new tokens; every other entry only verifies. Rotation is prepend-a-new-entry, not replace-the-list — removing an entry immediately invalidates tokens signed under it. `auth.enabled` with an empty `signingKeys` fails the render (a per-pod dev key would differ across nodes and break cross-node token validation), and so does any entry with an empty `id` (schema-required, plus a template `fail` for defense in depth) — it's published as the JWT `kid` header and used as the cert-manager Certificate's `commonName`.
+- **`auth.signingKeys` is ordered; the first entry signs.** The engine always uses `signing_keys[0]` to sign new tokens; every other entry only verifies. Rotation is prepend-a-new-entry, not replace-the-list — removing an entry immediately invalidates tokens signed under it. `auth.enabled` with an empty `signingKeys` fails the render (a per-pod dev key would differ across nodes and break cross-node token validation), and so does any entry with an empty `id` (schema-required, plus a template `fail` for defense in depth) — it's published as the JWT `kid` header, used as the cert-manager Certificate's `commonName`, and hashed into the chart-managed cert-manager Secret/Certificate name so list reordering preserves old private key material.
- **The engine's own TLS cert must be a full chain, not leaf-only.** When `tls.engine.enabled`, the engine's internal self-health-check dials its own query listener over HTTPS and validates using `certificate_file` as its curl trust bundle — a leaf-only cert (cert-manager's default `tls.crt`) fails self-verification. The `tls-chain-setup` init container in `engine-statefulset.yaml` concatenates `tls.crt` + `ca.crt` from the resolved Secret into `certificate_file`'s path; `ca.crt` is therefore a required key in `tls.engine`'s Secret (BYO or cert-manager), not optional. The Envoy gateway's `trusted_ca` reads `ca.crt` directly (standard chain validation needs only the CA, no concatenation).
- **`endpoints.http.listeners` only ever governs the query port (3473).** Confirmed against the engine source: health (8122), metrics (9090), and metadata (6500) listeners are configured by independent keys untouched by `endpoints`, so the chart's minimal TLS overlay never needs to re-declare them. kubelet probes stay on plain HTTP against 8122 regardless of `tls.engine.enabled`.
- **No secrets in `values.yaml`.** `postgresql.password` is required when `local_enabled: true` and is left empty in `values.yaml`. External-DB users should set `postgresql.credentials.existingSecret` instead.
diff --git a/helm/README.md b/helm/README.md
--- a/helm/README.md
+++ b/helm/README.md
@@ -14,16 +14,16 @@
| Key | Type | Default | Description |
|-----|------|---------|-------------|
-| auth | object | {} | Authentication configuration for the Firebolt engine, rendered into the engine's `config.yaml` under `instance.auth`. When `enabled` is false, nothing is rendered under `instance.auth` — the engine refuses to start if `admin` or `oidc` are present while auth is disabled. Everything beyond the admin account and signing keys (OIDC providers, `password_login`, `jwt.*`, JIT provisioning, `preferred_authorization_server`) is not a first-class value here — set it under `customEngineConfig.instance.auth`, which is deep-merged on top of the block this chart renders. |
+| auth | object | {} | Authentication configuration for the Firebolt engine, rendered into the engine's `config.yaml` under `instance.auth`. When `enabled` is false, nothing is rendered under `instance.auth` — the engine refuses to start if `admin` or `oidc` are present while auth is disabled. Everything beyond the admin account and signing keys (OIDC providers, `password_login`, `jwt.*`, JIT provisioning, `preferred_authorization_server`) is not a first-class value here — set it under `customEngineConfig.instance.auth`, which is deep-merged on top of the block this chart renders. The chart-owned `enabled`, `admin`, and `local.signing_keys` fields always come from this `auth` block when auth is enabled. |
| auth.admin | object | {} | Bootstrap administrator account (`instance.auth.admin`). Required when `auth.enabled` is true. |
| auth.admin.name | string | `"firebolt"` | Admin username (`instance.auth.admin.name`). |
| auth.admin.password | object | {} | Admin password source. The chart only accepts an existing Secret — there is no literal-password value or certManager option (a password isn't a certificate), so enabling auth always requires a Secret to already exist. |
| auth.admin.password.existingSecret | object | {} | Existing Secret containing the admin password under key `password`. Mounted read-only and passed to the engine as `password_file`. |
| auth.admin.password.existingSecret.secretRef | string | `""` | Secret name. |
| auth.enabled | bool | `false` | Enable authentication on the engine (`instance.auth.enabled`). |
-| auth.signingKeys | list | [] | JWT signing keys for the engine's embedded Authorization Server (`instance.auth.local.signing_keys`). An ordered list: the **first** entry is the active signer used for new tokens; every entry remains valid for verifying tokens already issued under it. **To rotate the signing key, prepend a new entry** rather than replacing the list, so tokens signed by the outgoing key keep validating until it's removed. At least one entry is required when `auth.enabled` is true — an engine with no explicit signing key falls back to a per-pod dev key that differs across nodes and breaks token validation in a multi-node engine. Each entry sets exactly one of `existingSecret` / `certManager`: - `id` — key identifier, published as the JWT `kid` header (`instance.auth.local.signing_keys[].id`). - `existingSecret.secretRef` — Secret containing the PEM private key under key `tls.key`. - `certManager` — a chart-rendered cert-manager `Certificate` request (`algorithm`, `size`, `issuerRef.{name,kind}`). Requires cert-manager and its CRDs installed in-cluster. The chart pins `privateKey.rotationPolicy: Never` so cert-manager never silently rotates a key out from under already-issued tokens — rotate by prepending a new list entry instead. |
+| auth.signingKeys | list | [] | JWT signing keys for the engine's embedded Authorization Server (`instance.auth.local.signing_keys`). An ordered list: the **first** entry is the active signer used for new tokens; every entry remains valid for verifying tokens already issued under it. **To rotate the signing key, prepend a new entry** rather than replacing the list, so tokens signed by the outgoing key keep validating until it's removed. At least one entry is required when `auth.enabled` is true — an engine with no explicit signing key falls back to a per-pod dev key that differs across nodes and breaks token validation in a multi-node engine. Each entry sets exactly one of `existingSecret` / `certManager`: - `id` — key identifier, published as the JWT `kid` header (`instance.auth.local.signing_keys[].id`). For cert-manager keys, chart-managed Certificate and Secret names are derived from this id, so prepending a new key preserves outgoing key material. - `existingSecret.secretRef` — Secret containing the PEM private key under key `tls.key`. - `certManager` — a chart-rendered cert-manager `Certificate` request (`algorithm`, `size`, `issuerRef.{name,kind}`). Requires cert-manager and its CRDs installed in-cluster. The chart pins `privateKey.rotationPolicy: Never` so cert-manager never silently rotates a key out from under already-issued tokens — rotate by prepending a new list entry instead. |
| createNamespace | bool | `false` | When true, a Namespace resource is included in the chart output. Pair with `helm install --create-namespace --set createNamespace=false`. |
-| customEngineConfig | object | {} | Custom engine configuration deep-merged into the rendered engine config.yaml at the root. The rendered document follows the Firebolt Core configuration schema (`schema_version: "1.0"`); user-supplied keys at the top of `customEngineConfig` become siblings of the chart-managed `engine:` and `instance:` blocks (e.g. `logging:`), and keys nested under `instance:` merge into the instance block (e.g. `instance.id`, or `instance.auth.*` alongside the admin account and signing keys the `auth` value block below renders when `auth.enabled` is true — see `auth` for what belongs there instead of here). Chart-authoritative paths are silently stripped from this input and cannot be overridden: `schema_version`, `engine.id`, `engine.nodes`, `engine.termination_grace_period`, `instance.type`, `instance.multi_engine`, and — only while `tls.engine.enabled` is actually rendering it — `endpoints.http.listeners`. |
+| customEngineConfig | object | {} | Custom engine configuration deep-merged into the rendered engine config.yaml at the root. The rendered document follows the Firebolt Core configuration schema (`schema_version: "1.0"`); user-supplied keys at the top of `customEngineConfig` become siblings of the chart-managed `engine:` and `instance:` blocks (e.g. `logging:`), and keys nested under `instance:` merge into the instance block (e.g. `instance.id`, or `instance.auth.*` alongside the admin account and signing keys the `auth` value block below renders when `auth.enabled` is true — see `auth` for what belongs there instead of here). Chart-authoritative paths are silently stripped from this input and cannot be overridden: `schema_version`, `engine.id`, `engine.nodes`, `engine.termination_grace_period`, `instance.type`, `instance.multi_engine`, and — only while `auth.enabled` / `tls.engine.enabled` are actually rendering them — `instance.auth.enabled`, `instance.auth.admin`, `instance.auth.local.signing_keys`, and `endpoints.http.listeners`. |
| customEngineConfig.instance | object | {} | Instance identity. `id` propagates internally to `account_id`, `account_name`, `organization_id`, and `organization_name`, so the chart only needs to set the ULID once. |
| customEngineConfig.instance.id | string | `"01KP98J0000000000000000000"` | ULID for the Firebolt instance. Must match the account reconciled by the metadata service at startup. |
| engineSpec | object | {} | Shared engine pod defaults applied to all engines unless overridden per-engine. |
diff --git a/helm/templates/_helpers.tpl b/helm/templates/_helpers.tpl
--- a/helm/templates/_helpers.tpl
+++ b/helm/templates/_helpers.tpl
@@ -131,10 +131,9 @@
Chart-authoritative paths are silently stripped from the user input
before the merge: `schema_version`, `engine.id`, `engine.nodes`,
`engine.termination_grace_period`, `instance.type`,
-`instance.multi_engine`, and — only when `tls.engine.enabled` — the
-`endpoints.http.listeners` this helper renders for the query
-listener's TLS. The same customEngineConfig therefore stays portable
-across chart versions.
+`instance.multi_engine`, and — only when `auth.enabled` /
+`tls.engine.enabled` — the auth/TLS subtrees this helper renders. The
+same customEngineConfig therefore stays portable across chart versions.
When `auth.enabled` is true, `instance.auth.{enabled,admin,
local.signing_keys}` are built from `auth.admin` / `auth.signingKeys`
@@ -198,6 +197,21 @@
{{- if kindIs "map" $user.instance -}}
{{- $_ := unset $user.instance "type" -}}
{{- $_ := unset $user.instance "multi_engine" -}}
+{{- if and $root.Values.auth.enabled (hasKey $user.instance "auth") -}}
+{{- if kindIs "map" $user.instance.auth -}}
+{{- $_ := unset $user.instance.auth "enabled" -}}
+{{- $_ := unset $user.instance.auth "admin" -}}
+{{- if hasKey $user.instance.auth "local" -}}
+{{- if kindIs "map" $user.instance.auth.local -}}
+{{- $_ := unset $user.instance.auth.local "signing_keys" -}}
+{{- else -}}
+{{- $_ := unset $user.instance.auth "local" -}}
+{{- end -}}
+{{- end -}}
+{{- else -}}
+{{- $_ := unset $user.instance "auth" -}}
+{{- end -}}
+{{- end -}}
{{- else -}}
{{- $_ := unset $user "instance" -}}
{{- end -}}
@@ -306,6 +320,17 @@
{{- end -}}
{{/*
+Stable chart-managed Secret/Certificate name for a cert-manager auth signing key.
+Use the key id instead of the list index so prepending a new signer does not move
+the outgoing key to a freshly-generated Secret.
+
+Usage: {{ include "fbinstance.authSigningKeySecretName" (dict "root" $ "key" $key) }}
+*/}}
+{{- define "fbinstance.authSigningKeySecretName" -}}
+{{- printf "%s-auth-signing-%s" (include "fbinstance.fullname" .root) (.key.id | sha256sum | trunc 16) -}}
+{{- end -}}
+
+{{/*
DNS names for the shared engine TLS certificate (one Secret/Certificate mounted by every
engine pod across every engine in .Values.engines). Includes every node's per-node FQDN,
each engine's headless-service FQDN, and "localhost".
diff --git a/helm/templates/certificates.yaml b/helm/templates/certificates.yaml
--- a/helm/templates/certificates.yaml
+++ b/helm/templates/certificates.yaml
@@ -69,13 +69,13 @@
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
- name: {{ include "fbinstance.fullname" $ }}-auth-signing-{{ $ki }}
+ name: {{ include "fbinstance.authSigningKeySecretName" (dict "root" $ "key" $key) }}
namespace: {{ $.Release.Namespace }}
labels:
{{- include "fbinstance.labels" $ | nindent 4 }}
firebolt/component: engine
spec:
- secretName: {{ include "fbinstance.fullname" $ }}-auth-signing-{{ $ki }}
+ secretName: {{ include "fbinstance.authSigningKeySecretName" (dict "root" $ "key" $key) }}
# No DNS/service identity — this is a JWT signing key, not a TLS endpoint cert. commonName
# is the only identity cert-manager requires when dnsNames/ipAddresses/uris are all empty.
commonName: {{ $key.id | quote }}
diff --git a/helm/templates/engine-statefulset.yaml b/helm/templates/engine-statefulset.yaml
--- a/helm/templates/engine-statefulset.yaml
+++ b/helm/templates/engine-statefulset.yaml
@@ -21,7 +21,7 @@
{{- if $.Values.auth.enabled -}}
{{- $authAdminSecret = $.Values.auth.admin.password.existingSecret.secretRef -}}
{{- range $ki, $key := $.Values.auth.signingKeys -}}
-{{- $authSigningSecrets = append $authSigningSecrets (include "fbinstance.secretSourceName" (dict "source" $key "certManagerSecretName" (printf "%s-auth-signing-%d" (include "fbinstance.fullname" $) $ki) "context" (printf "auth.signingKeys[%d]" $ki))) -}}
+{{- $authSigningSecrets = append $authSigningSecrets (include "fbinstance.secretSourceName" (dict "source" $key "certManagerSecretName" (include "fbinstance.authSigningKeySecretName" (dict "root" $ "key" $key)) "context" (printf "auth.signingKeys[%d]" $ki))) -}}
{{- end -}}
{{- end -}}
{{- $engineTlsSecret := "" -}}
diff --git a/helm/templates/tests/test-auth.yaml b/helm/templates/tests/test-auth.yaml
--- a/helm/templates/tests/test-auth.yaml
+++ b/helm/templates/tests/test-auth.yaml
@@ -36,7 +36,7 @@
- |
set -e
{{- include "fbinstance.testShellHelpers" . | nindent 10 }}
- HOST="{{ include "fbinstance.fullname" . }}-engine-{{ $engine.name }}-node-0-0.{{ include "fbinstance.fullname" . }}-engine-{{ $engine.name }}-hl.{{ .Release.Namespace }}.svc.cluster.local"
+ HOST="{{ include "fbinstance.fullname" . }}-engine-{{ $engine.name }}-node-0-0.{{ include "fbinstance.fullname" . }}-engine-{{ $engine.name }}-hl.{{ .Release.Namespace }}.svc{{ .Values.engineSpec.nodeHostSuffix }}"
{{- if .Values.tls.engine.enabled }}
# tls.engine.enabled: the query listener speaks HTTPS. This test only exercises
# auth enforcement, not certificate trust (the live TLS path covers that
diff --git a/helm/values.yaml b/helm/values.yaml
--- a/helm/values.yaml
+++ b/helm/values.yaml
@@ -424,7 +424,9 @@
# the admin account and signing keys (OIDC providers, `password_login`,
# `jwt.*`, JIT provisioning, `preferred_authorization_server`) is not a
# first-class value here — set it under `customEngineConfig.instance.auth`,
-# which is deep-merged on top of the block this chart renders.
+# which is deep-merged on top of the block this chart renders. The
+# chart-owned `enabled`, `admin`, and `local.signing_keys` fields always
+# come from this `auth` block when auth is enabled.
# @default -- {}
auth:
# -- Enable authentication on the engine (`instance.auth.enabled`).
@@ -463,7 +465,9 @@
#
# Each entry sets exactly one of `existingSecret` / `certManager`:
# - `id` — key identifier, published as the JWT `kid` header
- # (`instance.auth.local.signing_keys[].id`).
+ # (`instance.auth.local.signing_keys[].id`). For cert-manager keys,
+ # chart-managed Certificate and Secret names are derived from this id,
+ # so prepending a new key preserves outgoing key material.
# - `existingSecret.secretRef` — Secret containing the PEM private key
# under key `tls.key`.
# - `certManager` — a chart-rendered cert-manager `Certificate` request
@@ -629,8 +633,10 @@
# Chart-authoritative paths are silently stripped from this input and
# cannot be overridden: `schema_version`, `engine.id`, `engine.nodes`,
# `engine.termination_grace_period`, `instance.type`,
-# `instance.multi_engine`, and — only while `tls.engine.enabled` is
-# actually rendering it — `endpoints.http.listeners`.
+# `instance.multi_engine`, and — only while `auth.enabled` /
+# `tls.engine.enabled` are actually rendering them —
+# `instance.auth.enabled`, `instance.auth.admin`,
+# `instance.auth.local.signing_keys`, and `endpoints.http.listeners`.
# @default -- {}
customEngineConfig:
# -- Instance identity. `id` propagates internally to `account_id`,You can send follow-ups to the cloud agent here.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 429b8ec. Configure here.
Mintlify docs previewDocs were aggregated into the Firebolt documentation site.
Mintlify may take a few minutes to finish building. This comment updates on each sync while the source PR is open. |
Adds first-class chart support for engine authentication (bootstrap
admin account + JWT signing keys with rotation), and TLS on the
gateway's client listener and the engine's query listener. Every
secret artifact is provisioned by existingSecret or a chart-rendered
cert-manager Certificate — no Helm self-signed generation, no
plaintext secret values.
- auth.enabled renders instance.auth from auth.admin / auth.signingKeys;
signingKeys is ordered (first entry signs) to support rotation.
- tls.gateway / tls.engine enable TLS independently, including the
engine's own self-health-check (full-chain cert via an init
container) and the gateway's upstream TLS to engines (auto_sni /
auto_san_validation on the dynamic_forward_proxy cluster).
- Removes the old auth.{mode,local,oidc} auth.json scaffold: it was
never enforced and the engine never read it.
- New certificates.yaml (cert-manager Certificate rendering, gated on
each feature's enabled flag) and test-auth.yaml helm test;
test-gateway-ready / test-sql-query are now auth/TLS-aware.
- core-ui sidecar's FIREBOLT_CORE_URL follows tls.engine.enabled.
- Docs: new usage/authentication page, prerequisites and security
updates, including a callout that the chart's pinned appVersion
predates the instance.auth config schema.
Live-verified in a kind cluster: the auth-enabled helm test and the
TLS engine/gateway request path both pass end-to-end.
- Narrow the gateway's engine-ca mount to project only ca.crt instead of
the whole engine TLS Secret, keeping the engine's private key off the
gateway pod's filesystem.
- Document the SAN requirements for BYO tls.engine certs and the ca.crt
requirement for tls.engine's cert-manager issuer.
- Derive cert-manager signing-key Secret/Certificate names from a hash of
each key's id instead of its list position, so prepending a new key for
rotation no longer reissues fresh material for shifted entries.
- Fix templates/tests/test-auth.yaml to respect engineSpec.nodeHostSuffix
instead of a hardcoded .svc.cluster.local.
- Strip chart-owned instance.auth.{enabled,admin,local.signing_keys} from
customEngineConfig before merging, so user config can no longer flip
auth off or repoint admin/signing-key paths the chart doesn't mount.
Split the "Authentication and TLS" documentation into two separate pages to improve clarity and navigation. Removed stale notes about unsupported engine versions as the current `appVersion` now supports these features. - Move TLS configuration details to `docs/usage/tls.mdx` - Keep authentication and signing key rotation in `docs/usage/authentication.mdx` - Update `docs/docs.json` and `docs/known_pages.json` navigation - Add `.legitignore` to suppress false positive secret name leaks
kop
force-pushed
the
kop/fb-1943-auth-and-access-sso-support
branch
from
July 7, 2026 12:41
0c09617 to
1fbc301
Compare
kop
marked this pull request as ready for review
July 7, 2026 12:44
Contributor
|
LGTM |
dig-whois
approved these changes
Jul 7, 2026
Packdb docs aggregation ready for reviewDocs were merged to Squash-merge the packdb PR into |
gm42
added a commit
that referenced
this pull request
Jul 28, 2026
Automated release PR. --- ## [0.3.0](0.2.0...0.3.0) (2026-07-27) ### ⚠ BREAKING CHANGES * **storage:** migrate engine storage config to schema (FB-1684) ([#25](#25)) ### Features * **helm:** add engine authentication and TLS support (FB-1943) ([#23](#23)) ([a9a8aa2](a9a8aa2)) * **storage:** migrate engine storage config to schema (FB-1684) ([#25](#25)) ([91f1f5d](91f1f5d)) ### Bug Fixes * **agent:** remove floci AWS env credentials (FB-2197) ([#36](#36)) ([58b56d3](58b56d3)) * keep the Core UI sidecar image fresh and probe its readiness (FB-2179, FB-2180) ([#32](#32)) ([fb88c96](fb88c96)) * **security:** disable service account token automount on postgres and gateway ([#47](#47)) ([181ac91](181ac91)) ### Dependencies * **deps:** bump packdb appVersion to release-5.0.1-0.20260709071413.53735f172429 ([#5](#5)) ([76ecd18](76ecd18)) * **deps:** bump packdb appVersion to release-5.0.1-0.20260713060957.513515666721 ([#29](#29)) ([1dd11b3](1dd11b3)) * **deps:** bump packdb appVersion to release-5.0.1-0.20260727005216.d09b51086f14 ([#38](#38)) ([69909db](69909db)) --- This PR was generated with Release Please. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Version and changelog metadata plus lint/format guardrails; no chart template or runtime behavior changes in this diff. > > **Overview** > **Automated release-please PR** that publishes **`firebolt-instance` chart `0.3.0`** (from `0.2.0`) and syncs `.release-please-manifest.json`, `helm/Chart.yaml` `version`, and `helm/CHANGELOG.md` with the accumulated conventional-commit history since `0.2.0`. > > The changelog entry highlights **breaking storage config migration**, **engine auth/TLS**, security and sidecar fixes, and **`appVersion`** bumps to `release-5.0.1-0.20260727005216.d09b51086f14`. This diff does not re-land those features—it only records the release. > > **Release CI hygiene:** `helm/Chart.yaml` `sources` moves from flow to **block** style (with comments), `.yamllint.yaml` relaxes **`brackets`** spacing so release-please’s `[ "x" ]` formatting does not fail `validate-chart`, and `helm/AGENTS.md` documents keeping block-style collections. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e1e85e3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: fireboltdb-ci-writer[bot] <207707662+fireboltdb-ci-writer[bot]@users.noreply.github.com> Co-authored-by: Giuseppe Mazzotta <giuseppe@firebolt.io>
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.
Summary
Adds first-class chart support for:
auth.signingKeysis the active signer; prepend to rotate).tls.gateway) and the engine's query listener (tls.engine), including the engine's own self-health-check and the gateway's upstream TLS to engines.Every secret artifact (admin password, signing keys, TLS certs) is provisioned by exactly one of
existingSecret(bring-your-own) or a chart-rendered cert-managerCertificate— no Helm-generated self-signed certs, no plaintext secret values anywhere.The old
auth.{mode,local,oidc}→auth.jsonscaffold is removed: it was never enforced (values.yamlsaid so explicitly) and the engine never read that file.Notable design points
auth.admin.password,tls.gateway,tls.engine, and eachauth.signingKeys[]entry all share the sameexistingSecret/certManagershape, resolved by a sharedfbinstance.secretSourceNamehelper that fails the render if a source sets both or neither.tls.engine.enabled, and validates using the same file it serves ascertificate_file— this requires a full certificate chain, not a leaf-only cert, so an init container concatenatestls.crt+ca.crtfrom the resolved Secret.dynamic_forward_proxycluster getsauto_sni/auto_san_validationon its upstream TLS context, since the Lua filter rewrites:authorityper request.certificates.yamlgates every rendered cert-managerCertificateon its owning feature'senabledflag, so a stalecertManagerblock on a disabled feature never creates an unused cert-manager dependency.core-uidev sidecar'sFIREBOLT_CORE_URLnow followstls.engine.enabled(its nginx reverse proxy defaultsproxy_ssl_verifytooff, so this needs no extra trust configuration for the loopback connection).Caveat
The chart's pinned
appVersionpredates theinstance.authconfig schema landing in the engine — an engine on that build crash-loops withConfiguration entry does not existifauth.enabledortls.engine.enabledis set. This is documented as a prerequisite indocs/usage/authentication.mdxanddocs/prerequisites.mdx; the chart code itself is verified correct against engine source and against a:devengine build. Merging as-is (docs-only) per discussion — the feature activates automatically once a release image with the schema ships.One open judgment call left for reviewers: this PR does not add a
BREAKING CHANGE:footer even though it removes the old (non-functional)auth.*values shape, since nothing could have relied on it for real auth. Flag in review if a major version bump is wanted instead of the default minor bump afeat:commit triggers.Test plan
make lint/helm lint --strictpass./helm/scripts/validate-chart.shpasses (one pre-existing, unrelated yamllint blank-line issue confirmed present onmaintoo)auth.enabled—test-authhelm test passes all 4 assertions (unauthenticated rejection, health-check exemption, token issuance, authenticated query) against an engine build withinstance.authsupporttls.engine.enabled— engine self-probe validates its own full-chain cert (pod reaches Ready); gateway's upstream TLS withauto_sni/auto_san_validationcorrectly proxies to the engine (details=via_upstreamin Envoy access logs, not a local TLS failure)helm template+ schema): cert-manager paths for signing keys, gateway TLS, and engine TLS; stale-certManager-on-disabled-feature no longer renders aCertificate; missingdnsNameson gatewaycertManagerfails the render; missingsigningKeys[].idfails both schema validation and (if skipped) the template renderexistingSecretwas live-tested; cert-manager paths are static-only so far)Note
High Risk
Touches authentication, JWT signing, and TLS on the query path and Envoy upstream routing; misconfiguration can block engine readiness or break gateway-to-engine traffic. Values shape for
auth.*changes from the old non-functional scaffold.Overview
Adds first-class Helm support for engine authentication and TLS, replacing the unused
auth.json/auth.modescaffold with values that render into engineconfig.yamland wire through gateway and engine pods.Authentication (
auth.enabled) rendersinstance.auth(bootstrap admin from a required Secret, ordered JWTsigningKeyswith prepend-to-rotate semantics). TLS adds independenttls.gateway(client termination) andtls.engine(query listener on 3473 plus gateway upstream and active health checks). Secrets are BYOexistingSecretor cert-managerCertificateonly—no inline passwords or chart-generated certs—with sharedfbinstance.secretSourceNamevalidation.Implementation highlights: optional
certificates.yaml; enginetls-chain-setupinit buildsfullchain.pemfromtls.crt+ca.crt; Envoy gets downstream TLS and upstreamauto_sni/auto_san_validation; pod rollout checksums uselookupon live Secrets; helm tests cover auth, gateway HTTPS, and token-backed SQL via the gateway.Docs: new
usage/authentication.mdx, nav/prerequisites/security updates, and note that pinnedappVersionneeds a newer engine image when auth or engine TLS is enabled.Reviewed by Cursor Bugbot for commit 429b8ec. Configure here.