feat: RAG service API key file management#302
feat: RAG service API key file management#302tsivaprasad wants to merge 7 commits intoPLAT-489-rag-service-service-user-provisioningfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes introduce a new RAGServiceKeysResource to manage RAG API keys on the host filesystem. The orchestrator now parses RAG service configs, generates key resources that write authentication files to a configurable host path, and integrates them into the resource orchestration chain. Container specifications now support mounting these key directories read-only. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/internal/orchestrator/swarm/rag_service_keys_resource.go`:
- Around line 121-127: extractRAGAPIKeys currently uses pipeline names directly
to build filenames (e.g., keys[p.Name+"_embedding.key"]) and writeKeyFiles
writes them to disk, allowing path traversal via crafted pipeline names; fix by
sanitizing/validating pipeline names before using them in filenames (e.g.,
reject names containing path separators or non-alphanumeric chars, or replace
unsafe chars with underscores) and build file paths with filepath.Join then
verify the cleaned path is inside KeysDir (ensure filepath.Clean(joinedPath) has
KeysDir as a prefix) before writing; apply the same hardening wherever p.Name is
used to form filenames in extractRAGAPIKeys and writeKeyFiles.
- Around line 76-80: The Refresh loop in Refresh is only checking
os.IsNotExist(err) and ignores other stat errors; update the loop over r.Keys
(using r.KeysDir and filepath.Join) so that after calling os.Stat you: if err ==
nil continue; if os.IsNotExist(err) return resource.ErrNotFound; otherwise
return or wrap the encountered err (preserving context such as the key name) so
permission/I/O errors are surfaced instead of treated as present. Ensure you
reference the os.Stat call and return the real error for non-NotExist cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7d38e6a-f745-4663-993e-e43256518cb2
📒 Files selected for processing (6)
server/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/rag_service_keys_resource.goserver/internal/orchestrator/swarm/rag_service_keys_resource_test.goserver/internal/orchestrator/swarm/rag_service_user_role_test.goserver/internal/orchestrator/swarm/resources.goserver/internal/orchestrator/swarm/service_spec.go
…r-provisioning' into PLAT-490-rag-service-api-key-file-management
| } | ||
|
|
||
| func (r *RAGServiceKeysResource) Dependencies() []resource.Identifier { | ||
| return nil |
There was a problem hiding this comment.
Could you please make this depend on the service data directory? If you look at the pattern that we use in other file resources like Etcd creds, postgres creds, patroni/pgbackrest config, etc. you'll see that they all have a ParentID attribute that they use to compute the identifier for their parent directory.
| } | ||
|
|
||
| func (r *RAGServiceKeysResource) writeKeyFiles() error { | ||
| for name, key := range r.Keys { |
There was a problem hiding this comment.
How does this handle when a user removes a key?
| return resource.ErrNotFound | ||
| } | ||
|
|
||
| for name := range r.Keys { |
There was a problem hiding this comment.
How does this handle when a user updates a key?
tsivaprasad
force-pushed
the
PLAT-489-rag-service-service-user-provisioning
branch
from
9fcf32a to
bdc9b3d
Compare
2 participants
Summary
This PR adds
RAGServiceKeysResource, a resource that stores provider API keys (OpenAI, Anthropic, etc.) as files on the host filesystem and mounts them into the RAG container in read-only mode. The keys are not exposed via Docker Swarm configs or API responses.Changes
RAGServiceKeysResource(swarm.rag_service_keys) — manages the{DataDir}/services/{instanceID}/keys/directory: Create writes key files at mode 0600 in a 0700 directory, Update rewrites them for key rotation, Delete removes the directory via os.RemoveAllextractRAGAPIKeyshelper that extracts API keys from a parsedRAGServiceConfiginto a{pipeline_name}_embedding.key / {pipeline_name}_rag.keyfilename mapServiceContainerSpecOptionswithKeysPath; when non-empty a read-only bind mount to/app/keysis added to the container specRAGServiceKeysResourceintogenerateRAGInstanceResourcesafterRAGServiceUserRole; RAG config is now parsed at resource-generation timeRegisterResourceTypesTesting
api_keysin responseChecklist
Notes for Reviewers
PLAT-490