feat: add support for passing arbitrary headers to Ollama via OLLAMA_HEADERS#938
Open
zebrapurring wants to merge 2 commits into
Open
feat: add support for passing arbitrary headers to Ollama via OLLAMA_HEADERS#938zebrapurring wants to merge 2 commits into
OLLAMA_HEADERS#938zebrapurring wants to merge 2 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds optional OLLAMA_HEADERS env var parsing into comma-separated Key=Value pairs, provides an exported OllamaHTTPClient() that returns an *http.Client with a RoundTripper that sets those headers, wires that client into Ollama client creation when present, and documents the new env var and troubleshooting guidance. ChangesOLLAMA headers and wiring
Sequence Diagram(s)sequenceDiagram
participant App as "App (createLLM / createVisionLLM)"
participant Client as "Custom http.Client\n(ollamaHeaderTransport)"
participant Proxy as "Reverse Proxy (auth)"
participant Ollama as "Ollama Server"
App->>Client: call OllamaHTTPClient() and build client with OLLAMA_HEADERS
App->>Ollama: create Ollama client with injected Client
App->>Ollama: send request via Ollama client
Client->>Proxy: outbound HTTP request with injected headers
Proxy->>Ollama: forward request after auth
Ollama->>App: response returned to App
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
main.go (1)
1144-1167: Consider extracting sharedollamaHTTPClient()to avoid code duplication.This function is duplicated in
ocr/llm_provider.gowith a slightly different implementation (different struct name, usesHeader.SetvsHeader.Add, and clones the request). Maintaining two implementations increases the risk of divergence and bugs.Additionally, the parsing doesn't trim whitespace from individual keys/values. Input like
"Key = Value"would result in key"Key "and value" Value".💡 Suggested improvements
Extract to a shared package (e.g.,
internal/httpclient) and import in both locations.Trim individual key/value whitespace:
for _, pair := range strings.Split(raw, ",") { parts := strings.SplitN(strings.TrimSpace(pair), "=", 2) - if len(parts) == 2 && parts[0] != "" { - headers[parts[0]] = parts[1] + if len(parts) == 2 && strings.TrimSpace(parts[0]) != "" { + headers[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.go` around lines 1144 - 1167, The ollamaHTTPClient implementation is duplicated and has parsing issues; extract a single shared helper (e.g., package internal/httpclient) used by both ollamaHTTPClient in main.go and the variant in ocr/llm_provider.go, unify on one headerTransport type/method that clones requests and uses Header.Set (not Add) when injecting headers, and update the parsing to TrimSpace both key and value (use strings.TrimSpace on parts[0] and parts[1]) so inputs like "Key = Value" produce "Key":"Value"; replace both local implementations with calls to the new shared constructor returning *http.Client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@main.go`:
- Around line 1144-1167: The ollamaHTTPClient implementation is duplicated and
has parsing issues; extract a single shared helper (e.g., package
internal/httpclient) used by both ollamaHTTPClient in main.go and the variant in
ocr/llm_provider.go, unify on one headerTransport type/method that clones
requests and uses Header.Set (not Add) when injecting headers, and update the
parsing to TrimSpace both key and value (use strings.TrimSpace on parts[0] and
parts[1]) so inputs like "Key = Value" produce "Key":"Value"; replace both local
implementations with calls to the new shared constructor returning *http.Client.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6dbeff47-25ee-4873-98c4-6e5da865dd67
📒 Files selected for processing (3)
README.mdmain.goocr/llm_provider.go
zebrapurring
force-pushed
the
ollama-headers
branch
from
24266ad to
7c870fb
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ocr/llm_provider.go (1)
212-235: Consider trimming keys and values individually for robustness.The current implementation trims the full pair but not the individual key and value. If users configure
OLLAMA_HEADERS="Authorization = Bearer token", the key becomes"Authorization "(with trailing space) which may cause header matching issues on the server side.Also, consider adding debug logging when headers are successfully parsed to aid troubleshooting.
♻️ Proposed fix
func OllamaHTTPClient() *http.Client { raw := os.Getenv("OLLAMA_HEADERS") if raw == "" { return nil } headers := map[string]string{} for _, pair := range strings.Split(raw, ",") { parts := strings.SplitN(strings.TrimSpace(pair), "=", 2) if len(parts) == 2 && parts[0] != "" { - headers[parts[0]] = parts[1] + headers[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) } } if len(headers) == 0 { return nil } + log.WithField("header_count", len(headers)).Debug("Configured Ollama HTTP headers from OLLAMA_HEADERS") return &http.Client{ Transport: &ollamaHeaderTransport{ base: http.DefaultTransport, headers: headers, }, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ocr/llm_provider.go` around lines 212 - 235, Trim the individual key and value when parsing OLLAMA_HEADERS inside OllamaHTTPClient (i.e., after SplitN on "=") so keys and values get strings.TrimSpace applied separately before inserting into the headers map to avoid trailing/leading whitespace in header names/values; also add a debug/info log (using the existing logger or standard log) after successful parsing to list which header keys were loaded (without printing sensitive values) to aid troubleshooting; update references to the headers map used by ollamaHeaderTransport accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ocr/llm_provider.go`:
- Around line 212-235: Trim the individual key and value when parsing
OLLAMA_HEADERS inside OllamaHTTPClient (i.e., after SplitN on "=") so keys and
values get strings.TrimSpace applied separately before inserting into the
headers map to avoid trailing/leading whitespace in header names/values; also
add a debug/info log (using the existing logger or standard log) after
successful parsing to list which header keys were loaded (without printing
sensitive values) to aid troubleshooting; update references to the headers map
used by ollamaHeaderTransport accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a49a7b0-e5bf-46de-b548-11c01f05f363
📒 Files selected for processing (3)
README.mdmain.goocr/llm_provider.go
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 573: Update the README entry for OLLAMA_HEADERS to explicitly state that
values cannot contain commas (there is no escaping for commas), so header values
with commas (e.g., "Accept=text/html, application/json") are not supported; also
clarify parsing semantics that keys/values are parsed by splitting on the first
'=' (so values may contain '=' characters such as Base64 tokens), and give a
short example of a valid header string and a recommended workaround (e.g., use a
reverse proxy or alternate env var) to avoid silent misconfiguration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| | `VISION_LLM_TEMPERATURE` | Sampling temperature for Vision OCR generation. Lower is more deterministic. Important: For OpenAI GPT-5 it must be explicitly set to `1.0`. | No | | | ||
| | `OLLAMA_CONTEXT_LENGTH` | (Ollama only) Integer. Sets NumCtx (context window) for the Ollama runner. If unset or 0, the model default is used. | No | | | ||
| | `OLLAMA_OCR_TOP_K` | (Ollama only) Top-k token sampling for Vision OCR. Lower favors more likely tokens; higher increases diversity. | No | | | ||
| | `OLLAMA_HEADERS` | (Ollama only) Comma-separated `Key=Value` pairs added as HTTP headers to every Ollama request. Useful for authorization when Ollama is behind a reverse proxy (e.g. `Authorization=Bearer mytoken`). | No | | |
Contributor
There was a problem hiding this comment.
Document the comma-in-value limitation of OLLAMA_HEADERS.
The current description implies a simple Key=Value,Key2=Value2 split. Two edge cases users may hit:
- Comma in value – e.g.
Accept=text/html, application/jsonis unparseable with this format; no escaping mechanism is mentioned. =in value – e.g. a Base64 Basic auth token (Authorization=Basic dXNlcjpwYXNz) must be parsed by splitting on the first=only.
Mentioning at least the comma limitation prevents silent misconfiguration.
📝 Suggested documentation addition
-| `OLLAMA_HEADERS` | (Ollama only) Comma-separated `Key=Value` pairs added as HTTP headers to every Ollama request. Useful for authorization when Ollama is behind a reverse proxy (e.g. `Authorization=Bearer mytoken`). | No | |
+| `OLLAMA_HEADERS` | (Ollama only) Comma-separated `Key=Value` pairs added as HTTP headers to every Ollama request. Useful for authorization when Ollama is behind a reverse proxy (e.g. `Authorization=Bearer mytoken`). Each pair is split on the **first** `=`; header values must not contain commas. | No | |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `OLLAMA_HEADERS` | (Ollama only) Comma-separated `Key=Value` pairs added as HTTP headers to every Ollama request. Useful for authorization when Ollama is behind a reverse proxy (e.g. `Authorization=Bearer mytoken`). | No | | | |
| | `OLLAMA_HEADERS` | (Ollama only) Comma-separated `Key=Value` pairs added as HTTP headers to every Ollama request. Useful for authorization when Ollama is behind a reverse proxy (e.g. `Authorization=Bearer mytoken`). Each pair is split on the **first** `=`; header values must not contain commas. | No | | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 573, Update the README entry for OLLAMA_HEADERS to
explicitly state that values cannot contain commas (there is no escaping for
commas), so header values with commas (e.g., "Accept=text/html,
application/json") are not supported; also clarify parsing semantics that
keys/values are parsed by splitting on the first '=' (so values may contain '='
characters such as Base64 tokens), and give a short example of a valid header
string and a recommended workaround (e.g., use a reverse proxy or alternate env
var) to avoid silent misconfiguration.
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.
This feature allows providing an authentication header for Ollama instances secured behind a reverse proxy (related to #3).
Summary by CodeRabbit
Documentation
New Features