Skip to content

Commit 03486de

Browse files
pyramationpyramation
authored
Merge pull request #1147 from constructive-io/fix/bucket-name-resolver-scoping
fix: scope BucketNameResolver to (databaseId, bucketKey)
2 parents a19d207 + 304d201 commit 03486de

4 files changed

Lines changed: 48 additions & 23 deletions

File tree

‎graphile/graphile-presigned-url-plugin/src/download-url-field.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ function resolveS3ForDatabase(
6060
options: PresignedUrlPluginOptions,
6161
storageConfig: StorageModuleConfig,
6262
databaseId: string,
63+
bucketKey: string,
6364
): S3Config {
6465
const globalS3 = resolveS3(options);
6566
const bucket = options.resolveBucketName
66-
? options.resolveBucketName(databaseId)
67+
? options.resolveBucketName(databaseId, bucketKey)
6768
: globalS3.bucket;
6869
const publicUrlPrefix = storageConfig.publicUrlPrefix ?? globalS3.publicUrlPrefix;
6970

@@ -127,6 +128,7 @@ export function createDownloadUrlPlugin(
127128
const $key = $parent.get('key');
128129
const $isPublic = $parent.get('is_public');
129130
const $filename = $parent.get('filename');
131+
const $bucketId = $parent.get('bucket_id');
130132

131133
const $withPgClient = (grafastContext() as any).get('withPgClient');
132134
const $pgSettings = (grafastContext() as any).get('pgSettings');
@@ -135,11 +137,12 @@ export function createDownloadUrlPlugin(
135137
key: $key,
136138
isPublic: $isPublic,
137139
filename: $filename,
140+
bucketId: $bucketId,
138141
withPgClient: $withPgClient,
139142
pgSettings: $pgSettings,
140143
});
141144

142-
return lambda($combined, async ({ key, isPublic, filename, withPgClient, pgSettings }: any) => {
145+
return lambda($combined, async ({ key, isPublic, filename, bucketId, withPgClient, pgSettings }: any) => {
143146
if (!key) return null;
144147

145148
let s3ForDb = resolveS3(options);
@@ -155,11 +158,24 @@ export function createDownloadUrlPlugin(
155158
const allConfigs = await loadAllStorageModules(pgClient, databaseId);
156159
const config = resolveStorageConfigFromCodec(capturedCodec, allConfigs);
157160
if (!config) return null;
158-
return { config, databaseId };
161+
162+
// Look up the bucket key for scoped S3 resolution
163+
let bucketKey = 'public';
164+
if (bucketId) {
165+
const bucketResult = await pgClient.query({
166+
text: `SELECT key FROM ${config.bucketsQualifiedName} WHERE id = $1 LIMIT 1`,
167+
values: [bucketId],
168+
});
169+
if (bucketResult.rows[0]?.key) {
170+
bucketKey = bucketResult.rows[0].key;
171+
}
172+
}
173+
174+
return { config, databaseId, bucketKey };
159175
});
160176
if (resolved) {
161177
downloadUrlExpirySeconds = resolved.config.downloadUrlExpirySeconds;
162-
s3ForDb = resolveS3ForDatabase(options, resolved.config, resolved.databaseId);
178+
s3ForDb = resolveS3ForDatabase(options, resolved.config, resolved.databaseId, resolved.bucketKey);
163179
}
164180
}
165181
} catch {

‎graphile/graphile-presigned-url-plugin/src/plugin.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,11 @@ function resolveS3ForDatabase(
9292
options: PresignedUrlPluginOptions,
9393
storageConfig: StorageModuleConfig,
9494
databaseId: string,
95+
bucketKey: string,
9596
): S3Config {
9697
const globalS3 = resolveS3(options);
9798
const bucket = options.resolveBucketName
98-
? options.resolveBucketName(databaseId)
99+
? options.resolveBucketName(databaseId, bucketKey)
99100
: globalS3.bucket;
100101
const publicUrlPrefix = storageConfig.publicUrlPrefix ?? globalS3.publicUrlPrefix;
101102

@@ -282,7 +283,7 @@ export function createPresignedUrlPlugin(
282283
);
283284
if (!bucket) throw new Error('BUCKET_NOT_FOUND');
284285

285-
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId);
286+
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId, bucket.key);
286287
await ensureS3BucketExists(options, s3ForDb.bucket, bucket, databaseId, storageConfig.allowedOrigins);
287288

288289
return processSingleFile(options, txClient, storageConfig, databaseId, bucket, s3ForDb, {
@@ -397,7 +398,7 @@ export function createPresignedUrlPlugin(
397398
);
398399
}
399400

400-
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId);
401+
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId, bucket.key);
401402
await ensureS3BucketExists(options, s3ForDb.bucket, bucket, databaseId, storageConfig.allowedOrigins);
402403

403404
const results = [];
@@ -533,7 +534,17 @@ export function createPresignedUrlPlugin(
533534
}
534535

535536
// No other references — attempt sync S3 delete
536-
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId);
537+
// Look up the bucket key for scoped S3 resolution
538+
const bucketResult = await pgClient.query({
539+
text: `SELECT key FROM ${storageConfig.bucketsQualifiedName} WHERE id = $1 LIMIT 1`,
540+
values: [fileRow!.bucket_id],
541+
});
542+
const bucketKey = bucketResult.rows[0]?.key;
543+
if (!bucketKey) {
544+
log.warn(`Bucket not found for bucket_id=${fileRow!.bucket_id}; skipping S3 delete`);
545+
return;
546+
}
547+
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId, bucketKey);
537548
await deleteS3Object(s3ForDb, fileRow!.key);
538549
log.info(`Sync S3 delete succeeded for key=${fileRow!.key}`);
539550
});

‎graphile/graphile-presigned-url-plugin/src/types.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,17 @@ export interface S3Config {
149149
export type S3ConfigOrGetter = S3Config | (() => S3Config);
150150

151151
/**
152-
* Function to derive the actual S3 bucket name for a given database.
152+
* Function to derive the actual S3 bucket name for a given database and bucket key.
153153
*
154154
* When provided, the presigned URL plugin calls this on every request
155-
* to determine which S3 bucket to use — enabling per-database bucket
155+
* to determine which S3 bucket to use — enabling per-(database, bucketKey)
156156
* isolation. If not provided, falls back to `s3Config.bucket` (global).
157157
*
158158
* @param databaseId - The metaschema database UUID
159-
* @returns The S3 bucket name for this database
159+
* @param bucketKey - The logical bucket key (e.g., "public", "private")
160+
* @returns The S3 bucket name for this database + bucket key
160161
*/
161-
export type BucketNameResolver = (databaseId: string) => string;
162+
export type BucketNameResolver = (databaseId: string, bucketKey: string) => string;
162163

163164
/**
164165
* Callback to lazily provision an S3 bucket on first use.

‎graphile/graphile-settings/src/presigned-url-resolver.ts‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,24 +86,21 @@ export function getPresignedUrlS3Config(): S3Config {
8686
}
8787

8888
/**
89-
* Create a per-database bucket name resolver.
89+
* Create a per-(database, bucketKey) bucket name resolver.
9090
*
91-
* Uses the BUCKET_NAME env var as a prefix. For each database, the S3 bucket
92-
* name becomes `{prefix}-{databaseId}` (e.g., "myapp-abc123def456").
91+
* Uses the BUCKET_NAME env var as a prefix. For each (database, bucketKey)
92+
* pair, the S3 bucket name becomes `{prefix}-{bucketKey}-{databaseId}`
93+
* (e.g., "myapp-public-abc123def456").
9394
*
94-
* In local development with MinIO (default BUCKET_NAME="test-bucket"),
95-
* all databases share the same bucket for simplicity — the resolver
96-
* returns the prefix as-is when it looks like a local dev bucket.
97-
*
98-
* In production, set BUCKET_NAME to your org prefix (e.g., "myapp")
99-
* and each database gets its own isolated S3 bucket.
95+
* This aligns with the bucket provisioner plugin which creates separate
96+
* S3 buckets per logical bucket key.
10097
*/
10198
export function createBucketNameResolver(): BucketNameResolver {
10299
const { cdn } = getEnvOptions();
103100
const prefix = cdn?.bucketName || 'test-bucket';
104101

105-
return (databaseId: string): string => {
106-
return `${prefix}-${databaseId}`;
102+
return (databaseId: string, bucketKey: string): string => {
103+
return `${prefix}-${bucketKey}-${databaseId}`;
107104
};
108105
}
109106

0 commit comments

Comments
 (0)