Repository navigation
Enable TLS encryption in transit for all Ymir services - #864
Open
majamassarini wants to merge 8 commits into
Open
majamassarini wants to merge 8 commits into
majamassarini wants to merge 8 commits into
Conversation
PR Summary by QodoEnable TLS across Ymir database, cache, and trace services
AI Description
Diagram
High-Level Assessment
Files changed (40)
|
majamassarini
force-pushed
the
tls-compliance-remediation
branch
from
October 8, 2026 12:51
6c0297b to
28b8453
Compare
Code Review by Qodo
1.
|
![qodo-for-packit[bot]](https://avatars.githubusercontent.com/in/4066991?s=60&v=4){kind=small}
majamassarini
force-pushed
the
tls-compliance-remediation
branch
from
October 8, 2026 13:35
56f6ff0 to
df40e31
Compare
Enable HTTP Strict Transport Security on all externally exposed Routes to enforce HTTPS in browsers for 1 year, addressing a compliance gap for the Red Hat encryption-in-transit requirement. Assisted-by: Claude Haiku 4.5 <noreply@anthropic.com>
Document the encryption-in-transit posture of the Ymir deployment covering external Routes (HSTS, TLS termination), database/cache TLS, SDN isolation for internal HTTP services, and PFS cipher compliance. Assisted-by: Claude Haiku 4.5 <noreply@anthropic.com>
Configure the phoenix-db deployment to serve TLS using certificates auto-generated by the OpenShift service-ca operator. An initContainer copies the cert/key to a writable volume with correct permissions and generates a PostgreSQL SSL config snippet. The Phoenix application now connects with sslmode=verify-full to ensure encrypted and authenticated database connections. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Configure Valkey to serve TLS-only connections using certificates auto-generated by the OpenShift service-ca operator. Update REDIS_URL to use the rediss:// scheme and configure redis-commander for TLS. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Add TLS support to the trace-server Python application via environment- configurable cert/key paths. Mount OpenShift service-ca certificates in the deployment and switch both trace-server Routes from edge to reencrypt termination so router-to-pod traffic is encrypted. Update OTel collector config to export via HTTPS to the now-TLS trace-server sidecar. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add service-ca-bundle ConfigMap volume mount to all deployments - Valkey mTLS: add --tls-auth-clients no (clients have no certs) - Python Redis client: pass ssl_ca_certs from mounted CA bundle - phoenix-db init container: add ImageStream trigger for image resolution - Update all CA cert paths to /etc/pki/service-ca/service-ca.crt Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Service CA is valid 26 months and auto-rotates at 13 months with a 13-month grace period. An annual proactive rotation (delete secrets, restart pods) is simpler and more reliable than monitoring expiry thresholds. Replace the CronJob-based checker with a documented runbook tracked by a recurring Jira issue. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use FQDN (*.jotnar-ymir--jotnar-ymir.svc) in REDIS_URL, REDIS_HOST, and PHOENIX_SQL_DATABASE_URL so hostnames match the service-ca certificate SANs (redis-py 6.x checks by default) - Add TLS flags to all valkey-cli invocations in Makefile and requeue_error.py (Valkey is now TLS-only, no plaintext port) - Fix trace server TLS: defer handshake to worker threads so a stalled client cannot block all connections and health probes - Fix typographic curly quotes in docs JSONPath command - Add supervisor-processor and mcp-gateway to rotation runbook - Document service-ca trust dependency in THREAT_MODEL.md - Add TLS flags to error_list.py direct valkey-cli invocation Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
majamassarini
force-pushed
the
tls-compliance-remediation
branch
from
October 8, 2026 13:37
df40e31 to
5387283
Compare
Member
Author
|
/agentic_review |
Comment on lines
+64
to
+68
| if redis_url.startswith("rediss://"): | ||
| ca_path = os.environ.get("REDIS_TLS_CA_CERT_FILE", "/etc/pki/service-ca/service-ca.crt") | ||
| if os.path.isfile(ca_path): | ||
| ssl_kwargs["ssl_ca_certs"] = ca_path | ||
| client = redis.Redis.from_url(redis_url, socket_timeout=socket_timeout, **ssl_kwargs) |
There was a problem hiding this comment.
18. A missing ca bundle fails without explanation 🐞 Bug ◔ Observability
redis_client passes ssl_ca_certs only when os.path.isfile(ca_path) is true and otherwise quietly falls back to the system trust store, logging nothing. If a pod is deployed without the service-ca-bundle mount, or REDIS_TLS_CA_CERT_FILE is set wrong, every Valkey connection fails with a generic certificate verification error. Nothing in the logs points at the missing CA file.
Agent Prompt
## Issue description
`redis_client` silently skips the CA bundle when the file is missing, so later TLS failures are hard to diagnose.
## Fix Focus Areas
- ymir/common/base_utils.py[63-68]
## Recommended Fix
Add an `else:` branch that calls `logger.warning("Redis TLS CA bundle %s not found; falling back to system trust store", ca_path)`. Or raise an error if `REDIS_TLS_CA_CERT_FILE` was set explicitly but the file is missing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 5387283 |
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
1 participant
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
sslmode=verify-fullon the Phoenix clientrediss://with CA verificationPost-merge action: create recurring Jira ticket
Once this PR is merged, create a recurring Jira issue with:
Annual TLS certificate rotation — Ymir (jotnar-ymir)Test plan
Strict-Transport-Securityheaderoc exec deployment/phoenix-db -- psql -c "SHOW ssl"returnsonoc exec deployment/valkey -- valkey-cli --tls --cert /tls/tls.crt --key /tls/tls.key --cacert /etc/pki/service-ca/service-ca.crt pingreencryptterminationrediss://URLs🤖 Generated with Claude Code