Skip to content

chore: move to rustfs for local dev - #1427

Open
TylerHillery wants to merge 7 commits into
masterfrom
tyler/chore/switch-to-rustfs
Open

TylerHillery wants to merge 7 commits into
masterfrom
tyler/chore/switch-to-rustfs

Conversation

@TylerHillery

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Chore to replace minio with rustfs for local development.

What is the current behavior?

Currently we use minio

What is the new behavior?

Now we use rustfs

@coveralls

coveralls commented Sep 21, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 35874167664

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.09%) to 83.42%

Details

  • Coverage increased (+0.09%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 34 coverage regressions across 6 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

34 previously-covered lines in 6 files lost coverage.

File Lines Losing Coverage Coverage
src/storage/backend/file.ts 13 88.2%
src/http/routes/s3/commands/get-object.ts 6 74.19%
src/http/routes/s3/index.ts 6 88.96%
src/http/routes/s3/commands/create-multipart-upload.ts 4 42.11%
src/storage/renderer/renderer.ts 4 94.39%
src/http/routes/s3/commands/complete-multipart-upload.ts 1 75.0%

Coverage Stats

Coverage Status
Relevant Lines: 14214
Covered Lines: 12283
Line Coverage: 86.41%
Relevant Branches: 8705
Covered Branches: 6836
Branch Coverage: 78.53%
Branches in Coverage %: Yes
Coverage Strength: 768.44 hits per line

💛 - Coveralls

Comment on lines 134 to 556
@@ -371,7 +367,7 @@ export class S3Backend implements StorageBackendAdapter {
const copyMetadata = options?.copyMetadata ?? !metadata
const command = new CopyObjectCommand({
Bucket: bucket,
CopySource: encodeCopySource(bucket, source, version),
CopySource: encodeURIComponent(`${bucket}/${withOptionalVersion(source, version)}`),
Key: withOptionalVersion(destination, destinationVersion),
CopySourceIfMatch: conditions?.ifMatch,
CopySourceIfNoneMatch: conditions?.ifNoneMatch,
@@ -556,7 +552,7 @@ export class S3Backend implements StorageBackendAdapter {
const data = await this.client.send(command)
return {
cacheControl: data.CacheControl || 'no-cache',
mimetype: data.ContentType || 'application/octet-stream',
mimetype: data.ContentType || 'binary/octet-stream',
eTag: data.ETag || '',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default fallback value for S3 is binary/octet-stream. You can verify this against a real S3 bucket:

echo "hello" > /tmp/s3-src.txt
aws s3api put-object --bucket "$BUCKET" --key "$KEY_SRC" --body /tmp/s3-src.txt
aws s3api head-object --bucket "$BUCKET" --key "$KEY_SRC"
aws s3api copy-object --bucket "$BUCKET" --key "$KEY_DST" \
  --copy-source "$BUCKET/$KEY_SRC" \
  --metadata-directive REPLACE --cache-control "max-age=2009"
aws s3api head-object --bucket "$BUCKET" --key "$KEY_DST"

Both head-object calls return binary/octet-stream

This fallback only fires when the backend returns no ContentType at all. MinIO and real AWS S3 always set one, so we never hit our own fallback there but RustFS has a bug where CopyObject with MetadataDirective=REPLACE doesn't set a content type when none is passed through, which is what surfaced this. I'm working on an upstream fix to RustFS, but the fallback here should be updated, since it's correct behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the upstream fix: rustfs/rustfs#8073

@TylerHillery
TylerHillery marked this pull request as ready for review September 22, 2026 19:38
@TylerHillery
TylerHillery requested a review from a team as a code owner September 22, 2026 19:38
Copilot AI lite review requested due to automatic review settings September 22, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved access-policy, startup-order, and MIME-fallback issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity

Open (3)
What changed in this PR

Replaces MinIO with RustFS for local development, updating infrastructure, endpoints, tests, documentation, and S3 MIME handling.

Changes:

  • Replaces local MinIO services and bootstrap configuration with RustFS.
  • Updates tests, environment samples, and acceptance documentation.
  • Adjusts S3 content-type fallback behavior.
File Summary
src/​test/​s3-locker.test.ts Updates provider references in comments.
src/​test/​iceberg.test.ts Renames the local S3 client.
src/​storage/​backend/​s3/​adapter.ts Changes MIME fallbacks; retain application/octet-stream.
src/​storage/​backend/​s3/​adapter.test.ts Updates endpoints and MIME expectations.
docker-compose.yml Uses RustFS for single-tenant development.
docker-compose-multi-tenant.yml Uses RustFS; wait for the healthy service before setup.
acceptance/​README.md Updates RustFS acceptance instructions.
acceptance/​API_COVERAGE.md Updates backend coverage terminology.
.env.sample Sets the RustFS private asset endpoint.
.docker/​docker-compose-infra.yml Defines RustFS and bucket setup; keep anonymous warehouse access read-only.
.docker/​docker-compose-infra-multigres-override.yml Updates infrastructure documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .docker/docker-compose-infra.yml Outdated
Comment thread docker-compose-multi-tenant.yml Outdated
Comment thread src/storage/backend/s3/adapter.ts

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

Comment thread src/storage/backend/s3/adapter.ts
Comment thread .docker/docker-compose-infra.yml Outdated
Comment thread .docker/docker-compose-infra.yml Outdated
for b in supa-storage-bucket warehouse--table-s3 supa-storage-profiles; do
curl -fsS --aws-sigv4 'aws:amz:us-east-1:s3' -u supa-storage:secret1234 -X PUT http://rustfs:9000/\$$b;
done;
curl -fsS --aws-sigv4 'aws:amz:us-east-1:s3' -u supa-storage:secret1234 -X PUT 'http://rustfs:9000/warehouse--table-s3?policy' -d '{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:DeleteObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::warehouse--table-s3\",\"arn:aws:s3:::warehouse--table-s3/*\"]}]}';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this policy needed?

size: data.size,
cacheControl: cacheControl || 'no-cache',
mimetype: contentType || 'application/octet-stream',
mimetype: contentType || 'binary/octet-stream',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like upload case is missing

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and it also means we need a small test becase there is no test update which is missing

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants