Conversation
WalkthroughThis PR migrates object storage from MinIO to RustFS: environment variables, Docker Compose service definitions, and bucket-init scripts are updated to use RustFS and AWS CLI; web upload URL handling is adjusted to include an uploaded object key when available. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as "docker-compose (orchestrator)"
participant RustFS as "RustFS (storage)"
participant Init as "create-buckets (AWS CLI)"
participant Services as "gopie / companion / others"
participant Web as "Web UI (upload flow)"
rect rgba(200,230,201,0.25)
Dev->>RustFS: start rustfs service (image rustfs/rustfs:latest)
RustFS-->>Dev: listen on http://rustfs:9000 (S3), :9001 (console)
end
rect rgba(200,221,242,0.25)
Dev->>Init: start create-buckets (depends_on rustfs)
Init->>RustFS: (after sleep) AWS CLI create-bucket --endpoint-url http://rustfs:9000
RustFS-->>Init: bucket created (200/201)
Init-->>Dev: success
end
rect rgba(255,243,205,0.25)
Dev->>Services: start application services (depend on rustfs)
Services->>RustFS: use RUSTFS_* creds/endpoint for S3 operations
end
rect rgba(255,238,230,0.12)
Web->>Services: upload request -> service returns uploadResponse (may include body.key)
alt upload URL has single path part
Web->>Web: if uploadResponse.body.key exists, construct s3://bucket/key
else
Web->>Web: fallback to s3://bucket
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
Claude finished @elliot14A's task —— View job Pull Request ReviewI've completed my analysis of the rustfs integration changes. Here's my comprehensive review: Todo List:
✅ Positive ChangesClean Migration Strategy
Simplified Configuration
|
shreeharsha-factly
requested review from
surajmn1
and removed request for
shreeharsha-factly
![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
🧹 Nitpick comments (2)
docker-compose-auth.yaml (1)
121-138: Consider adding a healthcheck to improve reliability.The
create-bucketsservice relies on a fixed 5-second sleep before attempting bucket creation. Without a healthcheck on therustfsservice, this approach is fragile and may fail if RustFS takes longer to start.🔎 Add healthcheck to rustfs service:
rustfs: image: rustfs/rustfs:latest restart: always ports: - "9000:9000" # S3 API - "9001:9001" # RustFS Console environment: - RUSTFS_ACCESS_KEY=rustfsadmin - RUSTFS_SECRET_KEY=rustfsadmin - RUSTFS_CONSOLE_ENABLE=true - RUSTFS_ADDRESS=:9000 volumes: - ./volumes/rustfs/data:/data + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:9000/minio/health/live"] + interval: 5s + timeout: 5s + retries: 5 networks: - gopieThen update create-buckets to use condition:
create-buckets: image: amazon/aws-cli:latest depends_on: - - rustfs + rustfs: + condition: service_healthyThis eliminates the need for the sleep and makes startup more reliable.
docker-compose-noauth.yaml (1)
94-111: Add healthcheck for improved reliability.Similar to
docker-compose-auth.yaml, this relies on a fixed sleep instead of a proper healthcheck. Consider adding a healthcheck to therustfsservice and usingcondition: service_healthyin thecreate-bucketsdependency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
config-auth.env.example(3 hunks)config-noauth.env.example(3 hunks)docker-compose-auth.yaml(2 hunks)docker-compose-noauth.yaml(2 hunks)docker-compose.yaml(4 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: factly/gopie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-29T09:36:39.908Z
Learning: Applies to server/**/*.go : Store dataset files in S3-compatible storage (MinIO) in the Go backend server
📚 Learning: 2025-07-29T09:36:39.908Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-29T09:36:39.908Z
Learning: Applies to config-noauth.env : Use `config-noauth.env` for no-auth development setup
Applied to files:
config-noauth.env.exampleconfig-auth.env.example
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Configure environment variables including NEXT_PUBLIC_COMPANION_URL, NEXT_PUBLIC_GOPIE_API_URL, GOPIE_API_URL, and NEXT_PUBLIC_ENABLE_AUTH
Applied to files:
config-noauth.env.exampleconfig-auth.env.exampledocker-compose.yaml
📚 Learning: 2025-07-29T09:36:39.908Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-29T09:36:39.908Z
Learning: Applies to server/**/*.go : Store dataset files in S3-compatible storage (MinIO) in the Go backend server
Applied to files:
config-noauth.env.exampleconfig-auth.env.example
📚 Learning: 2025-07-29T09:36:39.908Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-29T09:36:39.908Z
Learning: Applies to docker-compose-noauth.yaml : Use `docker-compose-noauth.yaml` for no-auth development setup
Applied to files:
docker-compose-auth.yamldocker-compose-noauth.yaml
📚 Learning: 2025-12-02T11:48:34.955Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: chat-server/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:34.955Z
Learning: Applies to chat-server/**/docker-compose.yaml : Use docker-compose for orchestrating service dependencies in development
Applied to files:
docker-compose-auth.yamldocker-compose.yaml
📚 Learning: 2025-07-29T09:36:39.908Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-29T09:36:39.908Z
Learning: Applies to chat-server/{pyproject.toml,requirements.txt} : Use `uv sync` to install dependencies in the chat server
Applied to files:
docker-compose-auth.yamldocker-compose.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (5)
config-auth.env.example (1)
53-55: LGTM! Credentials and endpoints consistently updated.The migration from MinIO to RustFS credentials and endpoints is correctly applied across all three service configurations (GOPIE, COMPANION, and CHAT server).
Also applies to: 92-93, 212-212, 214-215
config-noauth.env.example (1)
27-29: LGTM! RustFS configuration correctly applied.The credentials and endpoints are consistently updated across all service configurations, matching the changes in
config-auth.env.example.Also applies to: 44-45, 172-172, 174-175
docker-compose-noauth.yaml (1)
78-93: Verify RustFS configuration and consider using a specific version tag.Same concerns as in
docker-compose-auth.yaml:
- Verify the RustFS Docker image and environment variables are correct
- Consider using a specific version tag instead of
:latestfor reproducible deploymentsdocker-compose.yaml (2)
163-177: Verify RustFS configuration and use a specific version tag.Same concerns as the other docker-compose files regarding the RustFS Docker image, environment variables, and the use of
:latesttag.
197-204: Remove this comment. The config.env file is gitignored and therefore not included in the PR—it's a local deployment concern, not something a reviewer can or should verify. The example configuration files (config-auth.env.example and config-noauth.env.example) already specify RustFS credentials consistently across all services, confirming the intended storage backend.Likely an incorrect or invalid review comment.
| rustfs: | ||
| image: rustfs/rustfs:latest | ||
| restart: always | ||
| ports: | ||
| - "9000:9000" | ||
| - "9002:9002" | ||
| volumes: | ||
| - ./volumes/minio/data:/export | ||
| - ./volumes/minio/config:/root/.minio | ||
| - "9000:9000" # S3 API | ||
| - "9001:9001" # RustFS Console | ||
| environment: | ||
| - MINIO_ACCESS_KEY=minioadmin | ||
| - MINIO_SECRET_KEY=minioadmin | ||
| - RUSTFS_ACCESS_KEY=rustfsadmin | ||
| - RUSTFS_SECRET_KEY=rustfsadmin | ||
| - RUSTFS_CONSOLE_ENABLE=true | ||
| - RUSTFS_ADDRESS=:9000 | ||
| volumes: | ||
| - ./volumes/rustfs/data:/data | ||
| networks: | ||
| - gopie | ||
| command: server -console-address :9002 /export | ||
| healthcheck: | ||
| test: ["CMD", "mc", "ready", "local"] | ||
| interval: 5s | ||
| timeout: 5s | ||
| retries: 5 | ||
|
|
There was a problem hiding this comment.
Fix RUSTFS_ADDRESS format to include IP binding.
The RUSTFS_ADDRESS environment variable should be 0.0.0.0:9000 instead of :9000 to properly bind to all interfaces. Additionally, add RUSTFS_CONSOLE_ADDRESS=0.0.0.0:9001 to explicitly configure the console binding. The official RustFS configuration uses RUSTFS_ADDRESS=0.0.0.0:9000 and RUSTFS_CONSOLE_ADDRESS=0.0.0.0:9001.
Also consider adding a healthcheck: "curl -f http://localhost:9000/health && curl -f http://localhost:9001/rustfs/console/health" to ensure proper service readiness before bucket creation attempts. The default RUSTFS_ACCESS_KEY and RUSTFS_SECRET_KEY values are intended for development only — rotate these credentials for production.
🤖 Prompt for AI Agents
In docker-compose-auth.yaml around lines 105 to 120, update the rustfs service
environment and readiness: change RUSTFS_ADDRESS from ":9000" to "0.0.0.0:9000",
add RUSTFS_CONSOLE_ADDRESS="0.0.0.0:9001" to bind the console, and add a
healthcheck that probes both endpoints (e.g. curl -f
http://localhost:9000/health && curl -f
http://localhost:9001/rustfs/console/health) so other services wait for
readiness; also note that RUSTFS_ACCESS_KEY/RUSTFS_SECRET_KEY are development
defaults and should be rotated for production.
| create-buckets: | ||
| image: amazon/aws-cli:latest | ||
| depends_on: | ||
| minio: | ||
| condition: service_healthy | ||
| - rustfs | ||
| environment: | ||
| - AWS_ACCESS_KEY_ID=rustfsadmin | ||
| - AWS_SECRET_ACCESS_KEY=rustfsadmin | ||
| - AWS_DEFAULT_REGION=us-east-1 | ||
| networks: | ||
| - gopie | ||
| entrypoint: > | ||
| /bin/sh -c " | ||
| echo 'Minio is up. Creating the bucket!!' && | ||
| /usr/bin/mc alias set myminio http://minio:9000 minioadmin minioadmin && | ||
| /usr/bin/mc mb -p myminio/gopie && | ||
| /usr/bin/mc anonymous set public myminio/gopie | ||
| sleep 5; | ||
| aws s3 mb s3://gopie --endpoint-url http://rustfs:9000 || true; | ||
| echo 'Buckets created successfully'; | ||
| exit 0; | ||
| " |
There was a problem hiding this comment.
Inconsistent bucket creation and missing healthcheck.
Two issues:
-
This compose file only creates the
gopiebucket (line 192), whiledocker-compose-auth.yamlanddocker-compose-noauth.yamlcreate bothgopieanddownloadsbuckets. Based onconfig-auth.env.exampleline 58 (GOPIE_DOWNLOADS_S3_BUCKET=downloads), thedownloadsbucket appears to be required. -
Similar to other files, consider adding a healthcheck instead of relying on
sleep 5.
🔎 Add the missing downloads bucket:
entrypoint: >
/bin/sh -c "
sleep 5;
aws s3 mb s3://gopie --endpoint-url http://rustfs:9000 || true;
+ aws s3 mb s3://downloads --endpoint-url http://rustfs:9000 || true;
echo 'Buckets created successfully';
exit 0;
"🤖 Prompt for AI Agents
In docker-compose.yaml around lines 179 to 195, the create-buckets service only
creates the gopie bucket and uses a hardcoded sleep rather than a healthcheck;
update it to also create the downloads bucket (aws s3 mb s3://downloads
--endpoint-url http://rustfs:9000) so it matches docker-compose-auth.yaml/noauth
and config-auth.env.example, and replace the sleep hack by adding a proper
healthcheck for the rustfs service (or for this container) that waits for the S3
endpoint on rustfs:9000 to be ready before running the bucket creation commands,
then run both aws s3 mb commands (each tolerant of existing buckets) and exit
successfully.
- mc: <ERROR> Unable to get bucket CORS configuration for rustfs/gopie. GetBucketCors is not implemented yet.
|
Claude finished @elliot14A's task —— View job Pull Request Review: Integrate rustfsTodo List:
✅ Strengths of the MigrationComprehensive Migration
Good Configuration Structure
🚨 Critical Security Issues1. Hardcoded Default Credentials (HIGH PRIORITY)Files: All config and docker-compose files # Found in multiple files:
RUSTFS_ACCESS_KEY=rustfsadmin
RUSTFS_SECRET_KEY=rustfsadminRisk:
Recommendation:
2. Missing Encryption ConfigurationIssue: No TLS/SSL configuration for RustFS service
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/dataset/dataset-upload-wizard.tsxweb/src/components/dataset/file-refresh-wizard.tsx
🧰 Additional context used
📓 Path-based instructions (3)
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Use Next.js 15 with App Router and React 19 for the web frontend framework
Use TypeScript as the language for the web frontend
Generate and use types from API responses for type safety
Use dynamic imports and code splitting for optimal performance
Load DuckDB WASM on-demand and use web workers for client-side SQL execution
Files:
web/src/components/dataset/dataset-upload-wizard.tsxweb/src/components/dataset/file-refresh-wizard.tsx
web/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/components/**/*.{ts,tsx}: Use TailwindCSS with Radix UI components for styling instead of custom CSS
Use React Hook Form with Zod validation for form handling
Use pre-configured queries and mutations from lib/queries and lib/mutations for API calls
Wrap features in error boundaries to handle errors gracefully
Always handle loading and error states in UI components
Use Next.js Image component for image optimization
Files:
web/src/components/dataset/dataset-upload-wizard.tsxweb/src/components/dataset/file-refresh-wizard.tsx
web/src/components/dataset/**upload**.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Use Uppy for dataset file uploads in upload components
Files:
web/src/components/dataset/dataset-upload-wizard.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-02T11:48:50.411Z
Learnt from: CR
Repo: factly/gopie PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2025-12-02T11:48:50.411Z
Learning: Applies to web/src/components/dataset/**upload**.{ts,tsx} : Use Uppy for dataset file uploads in upload components
Applied to files:
web/src/components/dataset/dataset-upload-wizard.tsxweb/src/components/dataset/file-refresh-wizard.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (1)
web/src/components/dataset/file-refresh-wizard.tsx (1)
174-184: Document the intended behavior for single-path bucket URLs and add validation if uploadResponse.body.key should always be present.The code constructs an s3 URL without a key when
uploadResponse?.body?.keyis undefined (s3://bucket). This fallback appears intentionally designed—it's implemented identically in bothfile-refresh-wizard.tsx(line 184) anddataset-upload-wizard.tsx(line 394), each with the comment "assume it's just the bucket," suggesting support for RustFS or similar storage backends that may handle bucket-only URLs.However, the lack of validation and error handling creates ambiguity: it's unclear whether this fallback is intentional and whether the backend
source/s3/refreshendpoint actually accepts key-less s3:// paths. If this fallback is intentional for RustFS:
- Document it clearly to prevent confusion
- Add a comment explaining the RustFS use case
If
uploadResponse.body.keyshould always be present for valid operations:
- Add validation with descriptive error messaging before constructing the URL
| // Non-S3 URL, might be RUSTFS, presigned URL, or proxy | ||
| // For localhost:9000 (RUSTFS) or similar, the format is usually: http://localhost:9000/bucket | ||
| // Try to extract path assuming format: /bucket | ||
| if (pathParts.length >= 2) { | ||
| const bucket = pathParts[0]; | ||
| const key = pathParts.slice(1).join("/"); | ||
| s3Url = `s3://${bucket}/${key}`; | ||
| } else if (pathParts.length === 1) { | ||
| // Only one path part, assume it's just the bucket | ||
| s3Url = `s3://${pathParts[0]}`; | ||
| const uploadResponse = useUploadStore.getState() | ||
| .uploadResponse as { body?: { key?: string } } | undefined; | ||
|
|
||
| const key = uploadResponse?.body?.key; | ||
| s3Url = `${key ? `s3://${pathParts[0]}/${key}` : `s3://${pathParts[0]}`}`; |
There was a problem hiding this comment.
Validate that the s3:// URL includes a key and consider refactoring duplicated logic.
This code has the same issue as in file-refresh-wizard.tsx: when pathParts.length === 1 and uploadResponse?.body?.key is undefined or empty, it constructs an incomplete s3 URL (s3://bucket). This will likely cause the dataset creation API call at line 425 to fail.
Additionally, this URL parsing logic is duplicated across multiple upload components, which increases maintenance burden.
🔎 Proposed fix with validation
} else {
// Non-S3 URL, might be RUSTFS, presigned URL, or proxy
// For localhost:9000 (RUSTFS) or similar, the format is usually: http://localhost:9000/bucket
// Try to extract path assuming format: /bucket
if (pathParts.length >= 2) {
const bucket = pathParts[0];
const key = pathParts.slice(1).join("/");
s3Url = `s3://${bucket}/${key}`;
} else if (pathParts.length === 1) {
- // Only one path part, assume it's just the bucket
const uploadResponse = useUploadStore.getState()
.uploadResponse as { body?: { key?: string } } | undefined;
const key = uploadResponse?.body?.key;
- s3Url = `${key ? `s3://${pathParts[0]}/${key}` : `s3://${pathParts[0]}`}`;
+ if (!key) {
+ throw new Error("Upload response missing object key for single-path URL");
+ }
+ s3Url = `s3://${pathParts[0]}/${key}`;
} else {Refactoring suggestion: Consider extracting the S3 URL parsing logic into a shared utility function (e.g., lib/utils/s3-url-parser.ts) to avoid duplication:
// lib/utils/s3-url-parser.ts
export function parseUploadUrlToS3(
uploadURL: string,
uploadResponse?: { body?: { key?: string } }
): string {
// Consolidated parsing logic here
}Then use it in both files:
s3Url = parseUploadUrlToS3(uploadURL, useUploadStore.getState().uploadResponse);🤖 Prompt for AI Agents
In web/src/components/dataset/dataset-upload-wizard.tsx around lines 381 to 394,
the code constructs s3://bucket when pathParts.length === 1 and
uploadResponse?.body?.key is missing, producing an incomplete S3 URL that will
fail later; update the logic to validate that a key exists before forming an
s3://bucket/key URL (if no key, fail early or return an explicit error/undefined
instead of s3://bucket), and refactor this parsing into a shared utility (e.g.,
lib/utils/s3-url-parser.ts with signature parseUploadUrlToS3(uploadURL,
uploadResponse)) so both this file and file-refresh-wizard.tsx call the same
function and avoid duplicated logic.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.