Skip to content

feat: wire per-database S3 bucket name and publicUrlPrefix into presigned URL plugin#968

Merged
pyramation merged 1 commit intomainfrom
feat/per-database-s3-bucket
Apr 9, 2026
Merged

feat: wire per-database S3 bucket name and publicUrlPrefix into presigned URL plugin#968
pyramation merged 1 commit intomainfrom
feat/per-database-s3-bucket

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

Summary

The storage_module table already has endpoint, public_url_prefix, and provider columns per-database, but the presigned URL plugin runtime ignored them — all S3 operations used the single global S3Config from env vars (BUCKET_NAME, CDN_ENDPOINT, CDN_PUBLIC_URL_PREFIX).

This PR wires up per-database bucket isolation:

  • New BucketNameResolver type + resolveBucketName option on PresignedUrlPluginOptions — a function (databaseId) => s3BucketName that the plugin calls on every request
  • New resolveS3ForDatabase() helper — overlays per-database bucket name (from resolveBucketName) and publicUrlPrefix (from storageConfig) onto the global S3Config. S3 credentials/client remain shared.
  • requestUploadUrl now generates presigned PUT URLs against the per-database bucket
  • confirmUpload now verifies uploads (HeadObject) against the per-database bucket
  • downloadUrl field restructured: resolves storageConfig before building URLs, so public files use the per-database publicUrlPrefix and private files use the per-database bucket for presigned GET URLs
  • createBucketNameResolver() in presigned-url-resolver.ts derives bucket names as {BUCKET_NAME}-{databaseId}
  • Wired into ConstructivePreset

Review & Testing Checklist for Human

  • createBucketNameResolver always appends -{databaseId} — the JSDoc claims it "returns the prefix as-is when it looks like a local dev bucket" but the implementation does ${prefix}-${databaseId} unconditionally. This will break local MinIO dev (bucket test-bucket-{uuid} won't exist). Either the code or the comment needs fixing — likely needs a guard for local dev or the MinIO setup needs to auto-create per-database buckets.
  • resolveS3ForDatabase is duplicated in both plugin.ts and download-url-field.ts. Verify both copies stay in sync, or extract to a shared module.
  • Performance regression for public file URLs: Previously, public files with a publicUrlPrefix returned immediately without any DB query. Now ALL files trigger a withPgClient call to resolve per-database config. Verify this is acceptable for your traffic patterns.
  • Bucket naming convention alignment: The bucket provisioner uses {prefix}-{bucketKey} (e.g., myapp-public), but this resolver uses {prefix}-{databaseId}. Verify these are intentionally different naming schemes, or reconcile them.
  • End-to-end test: Upload a file via requestUploadUrl, confirm via confirmUpload, then fetch downloadUrl — verify the presigned URLs and public URLs target the correct per-database bucket. Test with at least two databases to confirm isolation.

Notes

  • S3 endpoint is NOT yet per-database — the S3 client (and thus the endpoint it connects to) remains global. The storageConfig.endpoint column exists in the DB but is not used to create per-database S3 clients. This would require an S3 client pool, deferred to future work.
  • No DB schema changes are included — bucket names are derived by convention, not stored in storage_module.

Link to Devin session: https://app.devin.ai/sessions/e47513cf8b974ae6985c42c0a657e4d7
Requested by: @pyramation

…gned URL plugin

The storage_module table already has endpoint, public_url_prefix, and
provider columns per-database, but the presigned URL plugin runtime
never used them — all S3 operations went through the single global
S3Config (from env vars).

This commit:

1. Adds BucketNameResolver type + resolveBucketName option to
   PresignedUrlPluginOptions — lets each database resolve to its own
   S3 bucket name while sharing a single S3 client (credentials).

2. Adds resolveS3ForDatabase() helper in both plugin.ts and
   download-url-field.ts that overlays per-database bucket name
   (from resolveBucketName) and publicUrlPrefix (from storageConfig)
   onto the global S3Config.

3. Updates requestUploadUrl to generate presigned PUT URLs against
   the per-database S3 bucket.

4. Updates confirmUpload to verify uploads (HeadObject) against the
   per-database S3 bucket.

5. Updates downloadUrl field to:
   - Always resolve storageConfig before building URLs
   - Use per-database publicUrlPrefix for public file CDN URLs
   - Use per-database bucket for presigned GET URLs

6. Adds createBucketNameResolver() in presigned-url-resolver.ts
   that derives bucket names as {BUCKET_NAME}-{databaseId}.

7. Wires resolveBucketName into the ConstructivePreset.

S3 credentials (AWS_ACCESS_KEY/AWS_SECRET_KEY) remain global —
only the bucket name and publicUrlPrefix are per-database.
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@pyramation pyramation merged commit e6ec421 into main Apr 9, 2026
49 checks passed
@pyramation pyramation deleted the feat/per-database-s3-bucket branch April 9, 2026 20:27
devin-ai-integration Bot pushed a commit that referenced this pull request Apr 10, 2026
Adds a new integration test that exercises the full upload pipeline
for both public and private files using real MinIO:

- requestUploadUrl → PUT to presigned URL → confirmUpload
- Tests public bucket (is_public=true) and private bucket (is_public=false)
- Tests content-hash deduplication
- Uses lazy S3 bucket provisioning (from PR #969)
- Uses per-database bucket naming (from PR #968)

Includes seed fixtures (simple-seed-storage) that create:
- jwt_private schema with current_database_id() function
- metaschema tables (database, schema, table, field)
- services tables (apis, domains, api_schemas)
- storage_module config row
- storage tables (buckets, files, upload_requests)
- Two buckets: public and private
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.

1 participant