Add pod-level TLS termination for Keystone and Barbican - 007 WP-4 - #12641
Open
bbobrov wants to merge 56 commits into
Open
Add pod-level TLS termination for Keystone and Barbican - 007 WP-4#12641bbobrov wants to merge 56 commits into
bbobrov wants to merge 56 commits into
Conversation
bbobrov
requested review from
BerndKue,
JoJoPuppe,
dorneanu,
galkindmitrii,
stanislav-zaprudskiy and
tz3
as code owners
August 24, 2026 14:20
rajivmucheli
previously approved these changes
Aug 24, 2026
This was referenced Aug 24, 2026
Contributor
|
just realised, should we also split the PR's per service ? |
Contributor
Author
I don't see why this would be needed. I would rather leave it as is, but if you want i can do it. |
tz3
reviewed
Aug 25, 2026
tz3
left a comment
Member
There was a problem hiding this comment.
A few things worth checking before this lands.
bbobrov
force-pushed
the
007-wp4-implementation
branch
2 times, most recently
from
August 28, 2026 15:20
15256dc to
97718da
Compare
bbobrov
force-pushed
the
007-wp4-implementation
branch
2 times, most recently
from
August 30, 2026 13:46
1112d13 to
84b6596
Compare
Add a blackbox Probe for the public HTTPS endpoint of Keystone and Barbican that observes the certificate actually served on the wire, and alert when it expires within 7 days. Also alert when the rotation CronJob has not completed successfully in over 8 days, covering the case where it never runs at all. The Probe is disabled by default and only renders where TLS and the external endpoint are configured, so it stays inert in regions that have not switched over. The expiry and rotation alerts carry no_alert_on_absence and match on the specific endpoint and CronJob, so they never fire where the probe or rotation is absent.
The Keystone start script mutated Apache state at boot: a2enmod ssl/headers, a2dissite 000-default, and a2dismod status. These runtime changes write into /etc/apache2 and prevent a read-only root filesystem. Replace them with static config: - Load mod_ssl, mod_socache_shmcb and mod_headers via a conf-enabled snippet (ssl-load.conf). a2enmod ssl previously pulled in socache_shmcb implicitly; static loading has no dependency resolution, so it is listed explicitly. - Because conf-enabled is parsed after ports.conf, the ssl_module-gated Listen 443 there no longer fires, so the TLS vhost declares Listen 443. - Suppress the default :80 vhost with an emptyDir over sites-enabled instead of a2dissite. - mod_status is enabled by default in the image and ships a status.conf that exposes /server-status with ExtendedStatus On. Neutralize it with a conf-enabled override (status-off.conf) rather than a2dismod, since the module cannot be unloaded statically. mod_wsgi, mod_auth_openidc and mod_shib are already enabled in the image and need no action. The root filesystem stays writable for now; this change removes the config mutations that would block making it read-only later. Verified against the deployed API image: Apache starts, ssl/socache_shmcb/ headers load, port 443 binds, TLS 1.3 negotiates, the default :80 vhost is gone, and /server-status returns 403.
…ican Mirror the Keystone exposure model for pod-level TLS termination: a LoadBalancer Service on port 443 that announces the public /32 via Calico loadBalancerIPs (or spec.externalIPs on metal clusters), plus a disco DNS Record for the public host. The external IP is sourced from global.barbican_external_ip with services.externalip kept as an alias. Co-authored-by: Rajiv Mucheli <rajiv.mucheli@sap.com>
Most clusters do not perform automatic IP assignment, so announce the public TLS address via Service.spec.externalIPs by default. Clusters that assign IPs via the Calico annotation set calicoLoadBalancer: true.
- Log external HTTPS requests on the Barbican TLS VirtualHost and cap the request body size, mirroring the internal VirtualHost. - Add a PodDisruptionBudget (minAvailable: 1) for the Barbican API so a single pod eviction cannot drop all external HTTPS traffic. - Pin the tls-lifecycle-manager image tag in the Keystone and Barbican chart defaults instead of tracking latest.
…ning - Refine the TLS 1.3 cipher suites to AES-GCM and AES-CCM, and disable TLS compression on both Keystone and Barbican. - Set HSTS and standard HTTP security response headers on both TLS vhosts, loading mod_headers for Barbican. - Add a TLS-gated Apache hardening snippet (suppress the version banner, disable TRACE, and turn off directory listing) for both services. - Bump Keystone to 0.14.2 and Barbican to 0.9.3.
The TLS rotation failure alerts for Keystone and Barbican lacked no_alert_on_absence, unlike the other TLS alerts. When TLS is disabled the rotation jobs do not exist and the metric is absent, so add the label to avoid spurious absence alerts.
…one external IP Rebuild and sanitize the client-certificate issuer and subject DN request headers on the Keystone TLS vhost so tokenless x509 auth reads verified values and clients cannot inject them. Collapse the Barbican TLS vhost onto the single barbican-api WSGI daemon group to keep one PKCS#11 session per pod and honor api.threads. Restore the services.externalip entry on the plain Keystone API Service.
The TLS vhost CSP default-src 'self' blocked the inline auto-submit script in the WebSSO callback, leaving browser federation logins stuck. Allow just that script via its sha256 hash, and guard the pinned hash at render time so a change to the callback template fails loudly instead of silently breaking WebSSO.
mod_auth_openidc's default server-cache session lives in per-pod shared memory, so the post-callback request had to return to the same replica. Without the ingress cookie affinity that pod-level TLS removes, that request can hit any of the keystone replicas and lose the session. Store the session in the client cookie (encrypted with the already-shared crypto passphrase) so any replica can complete the WebSSO login.
Spell out the ServiceAccount subject namespace on the tls-manager RoleBinding so it matches the convention used by the other charts and is explicit about which ServiceAccount it binds. Co-authored-by: Rajiv Mucheli <rajiv.mucheli@sap.com>
The empty apache-sites volume that blanks out sites-enabled to suppress the default Apache site is only needed when Apache terminates TLS in the pod. Gate the mount and volume on tls.enabled so deployments without pod-level TLS keep the site config shipped in the image unchanged. Co-authored-by: Rajiv Mucheli <rajiv.mucheli@sap.com>
The tls-hardening snippet is mounted into conf-enabled and is already loaded automatically at server scope before the virtual host is parsed. Including it again re-declares the SSL session cache, which is redundant and can emit warnings, so rely on the automatic load only. Co-authored-by: Rajiv Mucheli <rajiv.mucheli@sap.com>
The TLS config validation previously only ran from the bootstrap hook, so rendering with hooks disabled skipped it and a later change to the rotation CronJob was not checked. Call the validation from the main Deployment and the rotation CronJob so an invalid TLS configuration is caught on every install and upgrade. Co-authored-by: Rajiv Mucheli <rajiv.mucheli@sap.com>
The rotation CronJob schedule was hard-coded to the same time for both services, so barbican and keystone rotated together and could hit issuer rate limits at once. Add a tls.rotationSchedule value and stagger the defaults (barbican Monday 03:00, keystone Monday 04:00) so operators can further adjust the timing per service. Co-authored-by: Rajiv Mucheli <rajiv.mucheli@sap.com>
When the pod terminates TLS itself there is no trusted proxy in front, so disable oslo proxy-header parsing in that mode to stop clients spoofing Forwarded/X-Forwarded-* (client address, host, scheme). Also strip the external-auth trust-key header at the TLS vhost, matching what the ingress did when it fronted the endpoint.
The external TLS (:443) vhost previously defined its own 8-process keystone-tls daemon group, so enabling pod-level TLS doubled the worker count to 16 under the same memory limit and left the TLS workers outside the metrics sampler. Define the keystone-public group once at server scope and have both vhosts use it, keeping the worker count and memory footprint fixed and letting the sampler cover every worker.
The alert matched any failed rotation Job still retained by kube-state-metrics. With failedJobsHistoryLimit set, a single old failed Job kept the alert firing for weeks, even after a later weekly run succeeded. Restrict the expression to the most recently created rotation Job so it clears as soon as a newer Job appears and still fires immediately on a first-run failure.
This alert is out of scope for the TLS work and is broken: it queries apache_error_log_saml_failures_total, a metric no component in the repo produces (there is no log-to-metric exporter, and the statsd sidecar only maps the WSGI worker gauge), so it can never fire and gives false confidence in SAML monitoring. Remove it.
A hung rotation Job with concurrencyPolicy Forbid would block every subsequent weekly run until manually killed. Cap the Job at 600s so a stuck run fails (DeadlineExceeded) and surfaces via the rotation-failed alert instead of silently wedging the schedule.
The alert queries kube_job_status_failed, a kube-state-metrics series only present in the kubernetes Prometheus, but living under alerts/openstack routed it (prometheus: openstack) to the openstack Prometheus where the metric never exists, so it could never fire. Relocate it next to the rotation-stale alert, which already uses kube_cronjob metrics.
Turning tls.enabled off tears down the public TLS LoadBalancer and the disco Record that pins the identity/key-manager endpoint to the external IP. That name is deliberately not disco-managed (disco authenticates through identity-3 itself), so it stops resolving until the region's baseline DNS is restored. A comment in this repo would not reach whoever flips the flag in the secrets repo, so guard it in the chart instead. When tls.enabled is off but the public TLS Service still exists, the render fails early (before helm upgrade touches the cluster) unless tls.allowDisable is set to acknowledge the teardown. To keep that flag a one-shot, also fail when it is left set while tls.enabled is true.
This reverts commit be04e53. The tls.allowDisable flag and its teardown guard are replaced by an explicit tls.state machine in a following commit, so the flag and its checks are removed first.
Replace the tls.enabled boolean with tls.state, which has three ordered values: disabled, transition, and enabled. The chart moves only between neighbouring states and refuses a direct jump between disabled and enabled, so an operator must pass through transition in both directions. In transition the nginx ingress and the pod TLS LoadBalancer both serve the endpoint, so switching direction causes no downtime while DNS propagates. Moving forward creates the disco Record that points the endpoint at the TLS LoadBalancer; moving back to disabled removes it and the operator repoints the endpoint DNS to the ingress by hand. The current state is read from the live cluster (the TLS LoadBalancer and the ingress) during upgrade; a fresh install is unconstrained. tls.enabled is removed entirely.
bbobrov
force-pushed
the
007-wp4-implementation
branch
from
September 12, 2026 11:31
14dc096 to
5566246
Compare
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
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.
Terminates TLS inside the API pod with Apache (mod_ssl) instead of at the
ingress. The API is exposed on port 443 through a LoadBalancer Service that
pins a configured public IP, with a matching DNS A record. All behavior is
gated behind
tls.enabledand is off by default, so existing deploymentsrender unchanged.
Changes
a2enmod),with TLS 1.2/1.3 cipher hardening included outside the VirtualHost and the
packaged default site dropped.
X-Content-Type-Options, Referrer-Policy) on the TLS endpoint, and stop
trusting client-supplied
Forwarded/X-Forwarded-*and trust-key headerswhen the pod terminates TLS directly.
cc_x509)from the verified TLS session and mount the client-cert trust anchor, so
client-certificate auth keeps working in the pod.
works across multiple replicas.
type: LoadBalancerService on port 443 that pins aconfigured public IP (via
spec.externalIPsby default, or the CalicoloadBalancerIPsannotation whencalicoLoadBalancer: true), plus a DNS Arecord. The public ingress renders only when TLS is disabled.
by cert-manager (issuer kind configurable, defaults to
ClusterIssuer), withTLS-manager RBAC as a pre-install hook and
fsGroupso the non-root tls-initcontainer can read the Secret.
validated in-chart, with an
allowInsecureStorageescape hatch fornon-production.
for certificate expiry, stale rotation, and rotation failures.
Upgrade note
Keystone now stores the OIDC federation session in the client cookie so it works
across multiple replicas. On upgrade, existing federated users re-authenticate
once. Operators with unusually large OIDC claim sets should confirm the ingress
large_client_header_buffersis adequate.