Skip to content

chore: [Running GitHub actions for #9378]#9846

Open
justin-tahara wants to merge 5 commits intomainfrom
run-ci/9378
Open

chore: [Running GitHub actions for #9378]#9846
justin-tahara wants to merge 5 commits intomainfrom
run-ci/9378

Conversation

@justin-tahara
Copy link
Copy Markdown
Contributor

@justin-tahara justin-tahara commented Apr 2, 2026

This PR runs GitHub Actions CI for #9378.

  • Override Linear Check

This PR should be closed (not merged) after CI completes.


Summary by cubic

Adds an opt-in api.runUpdateCaCertificates flag to run update-ca-certificates before the API starts, enabling custom CA bundles from mounted volumes. Supports Linear #9378; default is false, so no behavior change.

  • New Features

    • Entrypoint runs update-ca-certificates when api.runUpdateCaCertificates: true.
    • Bumped Helm chart version to 0.4.36.
  • Migration

    • When enabling, mount CAs to /usr/local/share/ca-certificates/*.crt and run the container as root.
    • For Python clients (requests, httpx), set REQUESTS_CA_BUNDLE and SSL_CERT_FILE to /etc/ssl/certs/ca-certificates.crt.

Written for commit 41f057e. Summary will update on new commits.

@justin-tahara justin-tahara requested a review from a team as a code owner April 2, 2026 00:30
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR adds a new opt-in api.runUpdateCaCertificates Helm value (default false) that runs update-ca-certificates before the API server starts, enabling users to inject custom CA certificates via volume mounts for internal HTTPS connectivity. The Chart version is bumped from 0.4.35 to 0.4.36.

Key changes:

  • values.yaml: Introduces api.runUpdateCaCertificates: false with detailed inline documentation covering the root-user requirement, cert file placement, and the need to set REQUESTS_CA_BUNDLE/SSL_CERT_FILE env vars so Python HTTP clients bypass certifi's bundle.
  • api-deployment.yaml: Wraps update-ca-certificates && in a Helm conditional that is prepended to the existing alembic upgrade head && uvicorn … startup command.
  • Chart.yaml: Patch version bump to 0.4.36.

Issue found:

  • The feature only applies to the API deployment. Celery workers (celery-worker-docfetching, celery-worker-docprocessing, celery-worker-primary, etc.) also make outbound HTTPS calls to indexed data sources and share the same image, but their startup scripts never run update-ca-certificates. Users who rely on this feature for connecting to custom-CA-protected data sources will find that background indexing still fails even with the flag enabled and env vars set.

Confidence Score: 4/5

  • The PR is a CI-test-only run for feat(helm): add optional CA certificate update step to api-server startup #9378 and the changes themselves are safe by default (flag defaults to false), but the feature as implemented has an incomplete scope that would leave Celery workers unable to use the custom CA even when the feature is enabled.
  • One P1 finding: the runUpdateCaCertificates feature is limited to the API deployment, so the primary use case (indexing from internal HTTPS sources requiring custom CAs) would still be broken for Celery workers. The code change is otherwise correct and the default-off flag prevents any regression.
  • deployment/helm/charts/onyx/templates/api-deployment.yaml — the feature should be extended to Celery worker templates to be functionally complete.

Important Files Changed

Filename Overview
deployment/helm/charts/onyx/Chart.yaml Routine Helm chart version bump from 0.4.35 to 0.4.36 to reflect the new feature.
deployment/helm/charts/onyx/templates/api-deployment.yaml Adds conditional update-ca-certificates step to the API container startup command; feature is complete for the API server but does not extend to Celery workers that also make outbound HTTPS calls.
deployment/helm/charts/onyx/values.yaml Adds api.runUpdateCaCertificates flag (default false) with thorough documentation on root requirement, cert placement, and Python env-var overrides needed for requests/httpx.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Container Starts] --> B{api.runUpdateCaCertificates?}
    B -- true --> C[update-ca-certificates\nupdates /etc/ssl/certs/ca-certificates.crt]
    B -- false --> D[alembic upgrade head]
    C --> D
    D --> E[uvicorn starts API Server]

    subgraph gap[⚠️ Not covered by this PR]
        F[Celery Worker Containers\ndocfetching / docprocessing / primary / etc.]
        F --> G[Make outbound HTTPS calls\nto indexed data sources]
        G --> H[❌ /etc/ssl/certs/ca-certificates.crt\nnever updated — custom CA missing]
    end

    subgraph pyclient[Python HTTP Client Behaviour]
        I[requests / httpx\nuse certifi bundle by default]
        I --> J{REQUESTS_CA_BUNDLE\nor SSL_CERT_FILE set?}
        J -- yes --> K[Use system CA bundle\nCustom CA works ✅]
        J -- no --> L[Use certifi bundle\nCustom CA missing ❌]
    end

    E --> I
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: deployment/helm/charts/onyx/templates/api-deployment.yaml
Line: 70-72

Comment:
**Custom CA update limited to API server only**

`update-ca-certificates` is only wired into the API deployment, but the Celery workers (`celery-worker-docfetching`, `celery-worker-docprocessing`, `celery-worker-primary`, etc.) also run Python and make outbound HTTPS requests to user-configured data sources during indexing. If a user mounts a custom CA and enables `runUpdateCaCertificates: true`, the system CA bundle at `/etc/ssl/certs/ca-certificates.crt` is updated **only** in the API container. The Celery worker containers share the same image but never run `update-ca-certificates`, so their CA bundle is unchanged.

Combined with the documented requirement to set `REQUESTS_CA_BUNDLE`/`SSL_CERT_FILE` (which point to the system bundle), this creates a situation where:
- **API server**: custom CA works ✅
- **Celery workers**: env vars point to system bundle, but system bundle was never updated → HTTPS calls to custom-CA-protected endpoints still fail ❌

The feature needs to be extended to the Celery worker templates (and potentially `mcp-server-deployment.yaml`) to be functionally complete for the primary use case (indexing from internal HTTPS sources).

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Update deployment/helm/charts/onyx/value..." | Re-trigger Greptile

Comment on lines +70 to +72
{{- if .Values.api.runUpdateCaCertificates }}
update-ca-certificates &&
{{- end }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Custom CA update limited to API server only

update-ca-certificates is only wired into the API deployment, but the Celery workers (celery-worker-docfetching, celery-worker-docprocessing, celery-worker-primary, etc.) also run Python and make outbound HTTPS requests to user-configured data sources during indexing. If a user mounts a custom CA and enables runUpdateCaCertificates: true, the system CA bundle at /etc/ssl/certs/ca-certificates.crt is updated only in the API container. The Celery worker containers share the same image but never run update-ca-certificates, so their CA bundle is unchanged.

Combined with the documented requirement to set REQUESTS_CA_BUNDLE/SSL_CERT_FILE (which point to the system bundle), this creates a situation where:

  • API server: custom CA works ✅
  • Celery workers: env vars point to system bundle, but system bundle was never updated → HTTPS calls to custom-CA-protected endpoints still fail ❌

The feature needs to be extended to the Celery worker templates (and potentially mcp-server-deployment.yaml) to be functionally complete for the primary use case (indexing from internal HTTPS sources).

Prompt To Fix With AI
This is a comment left during a code review.
Path: deployment/helm/charts/onyx/templates/api-deployment.yaml
Line: 70-72

Comment:
**Custom CA update limited to API server only**

`update-ca-certificates` is only wired into the API deployment, but the Celery workers (`celery-worker-docfetching`, `celery-worker-docprocessing`, `celery-worker-primary`, etc.) also run Python and make outbound HTTPS requests to user-configured data sources during indexing. If a user mounts a custom CA and enables `runUpdateCaCertificates: true`, the system CA bundle at `/etc/ssl/certs/ca-certificates.crt` is updated **only** in the API container. The Celery worker containers share the same image but never run `update-ca-certificates`, so their CA bundle is unchanged.

Combined with the documented requirement to set `REQUESTS_CA_BUNDLE`/`SSL_CERT_FILE` (which point to the system bundle), this creates a situation where:
- **API server**: custom CA works ✅
- **Celery workers**: env vars point to system bundle, but system bundle was never updated → HTTPS calls to custom-CA-protected endpoints still fail ❌

The feature needs to be extended to the Celery worker templates (and potentially `mcp-server-deployment.yaml`) to be functionally complete for the primary use case (indexing from internal HTTPS sources).

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants