Models endpoint#38
Conversation
WalkthroughThis update appends new PCR history records to both development and production JSON files and introduces a new asynchronous proxy endpoint Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebServer
participant OpenAI API
Client->>WebServer: GET /v1/models (with headers, session, user)
WebServer->>WebServer: Check user authentication (deny guest)
WebServer->>WebServer: Check cache for models list
alt Cache hit
WebServer-->>Client: Return cached encrypted JSON response
else Cache miss or expired
WebServer->>OpenAI API: GET /v1/models (proxy, with headers)
OpenAI API-->>WebServer: Response (status, body)
alt Success
WebServer->>WebServer: Parse and encrypt JSON response
WebServer->>WebServer: Update cache with fresh data
WebServer-->>Client: JSON (encrypted)
else Failure
WebServer->>WebServer: Log error details
WebServer-->>Client: Internal server error
end
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 100000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/web/openai.rs (1)
327-432: Consider refactoring to reduce code duplication.The
proxy_modelsfunction is well-implemented with proper access control, error handling, and encryption. However, there's significant code duplication between this function andproxy_openai:
- Guest user validation (lines 336-342 vs 52-58)
- OpenAI API key handling (lines 345-355 vs 90-100)
- HTTP client creation (lines 358-361 vs 103-106)
- Header forwarding logic (lines 373-382 vs 124-133)
- Error handling patterns (lines 397-413 vs 148-164)
Consider extracting common functionality into helper functions:
// Helper for guest user validation fn validate_non_guest_user(user: &User) -> Result<(), ApiError> { if user.is_guest() { error!("Guest user attempted to access restricted endpoint: {}", user.uuid); return Err(ApiError::Unauthorized); } Ok(()) } // Helper for OpenAI client setup fn create_openai_client(state: &AppState) -> Result<(Client<HttpsConnector<hyper::client::HttpConnector>, Body>, &str, &str), ApiError> { let openai_api_key = match &state.openai_api_key { Some(key) if !key.is_empty() => key, _ => { if is_default_openai_domain(&state.openai_api_base) { error!("OpenAI API key is required for OpenAI domain"); return Err(ApiError::InternalServerError); } "" } }; let https = HttpsConnector::new(); let client = Client::builder() .pool_idle_timeout(Duration::from_secs(15)) .build::<_, Body>(https); Ok((client, openai_api_key, &state.openai_api_base)) } // Helper for forwarding headers fn forward_request_headers(req_builder: hyper::http::request::Builder, headers: &HeaderMap) -> hyper::http::request::Builder { let mut req = req_builder; for (key, value) in headers.iter() { if key != header::HOST && key != header::AUTHORIZATION && key != header::CONTENT_LENGTH { if let (Ok(name), Ok(val)) = ( HeaderName::from_bytes(key.as_ref()), HeaderValue::from_str(value.to_str().unwrap_or_default()), ) { req = req.header(name, val); } } } req }This would improve maintainability and reduce the risk of inconsistencies between the two proxy functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pcrDev.jsonis excluded by!pcrDev.jsonpcrProd.jsonis excluded by!pcrProd.json
📒 Files selected for processing (3)
pcrDevHistory.json(1 hunks)pcrProdHistory.json(1 hunks)src/web/openai.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 100000ms (2)
- GitHub Check: Greptile Review
- GitHub Check: Development Reproducible Build
🔇 Additional comments (5)
pcrDevHistory.json (1)
51-57: LGTM! Consistent PCR history entry structure.The new PCR history entry follows the established format with proper hash values, timestamp, and signature. The structure is consistent with existing entries, maintaining data integrity for attestation purposes.
pcrProdHistory.json (1)
51-57: LGTM! Production PCR entry matches expected format.The new production PCR history entry maintains consistency with the development environment structure and follows the established pattern. The timestamp difference from the dev environment is expected and appropriate.
src/web/openai.rs (3)
5-5: LGTM! Appropriate import addition.The addition of
encrypt_responseandEncryptedResponseimports is necessary for the new models endpoint functionality.
11-12: LGTM! Required routing imports added.The addition of
getandJsonimports is appropriate for implementing the new GET endpoint.
34-34: LGTM! Models endpoint route properly registered.The new
/v1/modelsGET route is correctly added to the router with the appropriate handler function.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements a new /v1/models endpoint for OpenAI integration and updates PCR measurements across development and production environments.
- Added
/v1/modelsGET endpoint insrc/web/openai.rsto proxy OpenAI model listing requests - Implemented guest user access prevention and API key validation for models endpoint
- Excluded billing checks for model listing requests unlike chat completions endpoint
- Updated PCR measurements in both dev (timestamp 1748624627) and prod (timestamp 1748624758) environments
- Maintained consistent PCR1 values while updating PCR0/PCR2 to reflect new system state
5 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
AnthonyRonning
force-pushed
the
models-endpoint
branch
from
8b63995 to
329d97e
Compare
There was a problem hiding this comment.
PR Summary
(updates since last review)
Added caching mechanism for OpenAI models endpoint with a 5-minute TTL using ModelsCache struct and RwLock. The implementation includes proper concurrency handling and error recovery.
- Added
ModelsCachestruct withis_valid(),set(), andget()methods for managing cached model data - Implemented thread-safe global cache using
std::sync::OnceLock<RwLock<ModelsCache>> - Added graceful cache miss handling with fallback to direct API calls
- Included proper error handling for cache read/write failures
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
AnthonyRonning
force-pushed
the
models-endpoint
branch
from
329d97e to
173fc84
Compare
1 participant
Summary by CodeRabbit