feat: path preservation for routing in Olla by thushan · Pull Request #81 · thushan/olla

thushan · 2025-11-06T10:13:15Z

This PR addresses a few things that was raised in #80 around the use of Docker Model Runner and Olla.

OpenAI profile wasn't working
Paths failed to translate due to the path DMR creates (we mostly deal with unpathed endpoints in commercial/enterprise workloads).

This PR adds preserve_path to the endpoint config so we can map cleanly (and also support existing endpoints).

      - url: "http://localhost:12434/engines/llama.cpp/"
        name: "local-docker"
        type: "openai-compatible"
        priority: 100
        preserve_path: true
        model_url: "/models"
        health_check_url: "/"
        check_interval: 2s
        check_timeout: 1s
      - url: "http://localhost:11434"
        name: "local-ollama"
        type: "ollama"
        priority: 100
        preserve_path: true
        model_url: "/api/tags"
        health_check_url: "/"
        check_interval: 2s
        check_timeout: 1s

Summary by CodeRabbit

Release Notes

New Features
- Added OpenAI-compatible inference platform profile for seamless API integration
- Introduced path preservation setting for flexible endpoint routing and base path handling
- Added support for newer model patterns including GPT-5 and GPT-4o variants
Documentation
- Added comprehensive path preservation guide with real-world examples and troubleshooting steps
- Enhanced configuration reference with detailed endpoint options

fix broken tests add proxy specific path testing move target path resolution too & update tests

yaml formatting issue (thanks vscode)

coderabbitai · 2025-11-06T10:13:23Z

Walkthrough

This pull request introduces a path preservation feature for proxy endpoints, enabling base URL paths to be preserved during request proxying. It adds new OpenAI-compatible profiles, extends configuration types with PreservePath fields, implements centralised URL-building logic, and refactors proxy services to use the new builder. Comprehensive documentation and tests are included.

Changes

Cohort / File(s)	Summary
OpenAI-Compatible Profiles `config/profiles/openai-compatible.yaml`, `config/profiles/openai.yaml`	New OpenAI-compatible profile created; existing openai.yaml profile name normalised, display name and description updated, new `openai_compatible` API flag added, and context patterns expanded for newer GPT-5 and GPT-4 model variants (e.g., gpt-5-thinking, gpt-4.1, gpt-4o).
Path Preservation Documentation `docs/content/configuration/examples.md`, `docs/content/configuration/reference.md`, `docs/content/troubleshooting/path-preservation.md`	New documentation sections added explaining path preservation feature, including configuration examples for Docker model runners, path-based gateways, and microservices; reference documentation for the new `preserve_path` endpoint field; and troubleshooting guide with debugging steps and decision trees.
URL Builder Core Implementation `internal/adapter/proxy/common/url_builder.go`, `internal/adapter/proxy/common/url_builder_test.go`	New centralised URL-building logic introduced with `BuildTargetURL()` function and helper utilities for detecting path traversal attempts; comprehensive test suite validates preserve_path behaviour across edge cases, encoding scenarios, and real-world provider configurations.
Proxy Service Refactoring – Olla `internal/adapter/proxy/olla/service.go`, `internal/adapter/proxy/olla/service_retry.go`, `internal/adapter/proxy/olla/service_preserve_path_test.go`	Proxy services refactored to delegate URL construction to centralised `BuildTargetURL()` function; extensive test file added covering preserve_path scenarios, provider-specific configurations, and edge cases.
Proxy Service Refactoring – Sherpa `internal/adapter/proxy/sherpa/service_retry.go`, `internal/adapter/proxy/sherpa/service_preserve_path_test.go`	Proxy retry logic updated to use centralised URL builder; comprehensive test suite added for Sherpa service path preservation with mock backend validation across diverse provider scenarios.
Configuration Type Extensions `internal/config/types.go`, `internal/core/domain/endpoint.go`	New `PreservePath` boolean field added to `EndpointConfig` and `Endpoint` structs (YAML tag: `preserve_path`).
Repository and Discovery Updates `internal/adapter/discovery/repository.go`	`LoadFromConfig()` method updated to propagate `PreservePath` from configuration to in-memory endpoint representation.
Package Maintenance `internal/integration/network_integration_test.go`	Test file package declaration updated from `util` to `integration`.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ProxyService as Proxy Service<br/>(Olla/Sherpa)
    participant URLBuilder as URL Builder<br/>(common.BuildTargetURL)
    participant Endpoint as Endpoint<br/>(PreservePath flag)
    participant Backend as Backend<br/>Service

    Client->>ProxyService: HTTP Request<br/>/prefix/path?query=value
    ProxyService->>URLBuilder: BuildTargetURL(request,<br/>endpoint, proxyPrefix)
    
    alt PreservePath = true
        URLBuilder->>Endpoint: Check PreservePath flag
        Endpoint-->>URLBuilder: true
        URLBuilder->>URLBuilder: Join base path<br/>+ request path
        Note over URLBuilder: Result: base_path/path?query=value
    else PreservePath = false
        URLBuilder->>Endpoint: Check PreservePath flag
        Endpoint-->>URLBuilder: false
        URLBuilder->>URLBuilder: Strip proxy prefix,<br/>clean path,<br/>resolve against base
        Note over URLBuilder: Result: /path?query=value
    end
    
    URLBuilder-->>ProxyService: Target URL
    ProxyService->>Backend: Proxied Request
    Backend-->>ProxyService: Response
    ProxyService-->>Client: Response

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

URL builder logic complexity: The BuildTargetURL() function implements path traversal detection and handles both encoded and unencoded variants; review should verify correctness of containsDotDot() and containsEncodedDotDot() helper functions, particularly edge cases around percent-encoding.
Test coverage breadth: Multiple test files cover similar scenarios (preserve_path, edge cases, provider configs) across different proxy services; ensure tests are not redundant and sufficiently validate the feature without duplication.
Proxy service refactoring scope: Two proxy implementations (Olla, Sherpa) and retry logic replaced with centralised builder; verify all call sites correctly pass proxy prefix and that previous fast-path optimisations are maintained or appropriately sacrificed.
Configuration propagation: Ensure PreservePath field flows correctly from config types through discovery repository to endpoint domain objects without data loss.

Possibly related PRs

feat: olla profile #32: Overlapping profile and config loader changes for OpenAI-compatible profiles.
fixes: October 2025 performance improvements #71: Related modifications to proxy URL-building logic in internal/adapter/proxy/olla/service.go.
feat: Improved health recovery #50: Prior proxy URL construction and service wiring changes in proxy layer.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly and accurately describes the main feature being introduced—path preservation for routing in Olla. It is concise, specific, and directly reflects the primary change across the multiple file modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feature/preserve-path

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

docs/content/troubleshooting/path-preservation.md (1)
19-23: Consider adding language specifiers to fenced code blocks.

Several code blocks lack language identifiers, which would improve syntax highlighting and readability. Consider adding appropriate identifiers (e.g., text, plaintext, or console for output examples).

Apply these changes:
-```
+```text
 Endpoint URL: http://localhost:8080/api/v1
 Client request: POST /v1/chat/completions
 Proxied to: http://localhost:8080/v1/chat/completions
-```
+```
And similarly for the other blocks at lines 31-35, 152-155, and 298-325.

Also applies to: 31-35, 152-155, 298-325
config/profiles/openai-compatible.yaml (1)

1-127: LGTM! Comprehensive OpenAI-compatible profile.

The profile configuration is well-structured and covers all necessary aspects: routing, API paths, model patterns, context windows, and metrics extraction. The context patterns comprehensively cover GPT model families.

Minor: Consider adding a trailing newline at the end of the file to follow standard YAML conventions.
internal/adapter/proxy/common/url_builder.go (1)
38-38: Typo in comment.

"perbent-encoded" should be "percent-encoded".

Apply this diff:
-	// Non-preserve mode: guard against traversal, including perbent-encoded dot segments.
+	// Non-preserve mode: guard against traversal, including percent-encoded dot segments.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df959f0 and 43756ee.

📒 Files selected for processing (16)

config/profiles/openai-compatible.yaml (1 hunks)
config/profiles/openai.yaml (2 hunks)
docs/content/configuration/examples.md (1 hunks)
docs/content/configuration/reference.md (2 hunks)
docs/content/troubleshooting/path-preservation.md (1 hunks)
internal/adapter/discovery/repository.go (1 hunks)
internal/adapter/proxy/common/url_builder.go (1 hunks)
internal/adapter/proxy/common/url_builder_test.go (1 hunks)
internal/adapter/proxy/olla/service.go (1 hunks)
internal/adapter/proxy/olla/service_preserve_path_test.go (1 hunks)
internal/adapter/proxy/olla/service_retry.go (1 hunks)
internal/adapter/proxy/sherpa/service_preserve_path_test.go (1 hunks)
internal/adapter/proxy/sherpa/service_retry.go (1 hunks)
internal/config/types.go (1 hunks)
internal/core/domain/endpoint.go (1 hunks)
internal/integration/network_integration_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{go,md}