Skip to content

Commit b0c1f10

Browse files
authored
Merge pull request #1024 from constructive-io/devin/1776785805-s3-utils-consolidation
feat: consolidate S3 client factory + presigned URL helpers into s3-utils
2 parents 88698ea + 0c37809 commit b0c1f10

14 files changed

Lines changed: 574 additions & 85 deletions

File tree

graphile/graphile-settings/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
"@constructive-io/graphql-env": "workspace:^",
3535
"@constructive-io/graphql-types": "workspace:^",
3636
"@constructive-io/s3-streamer": "workspace:^",
37+
"@constructive-io/s3-utils": "workspace:^",
3738
"@constructive-io/upload-names": "workspace:^",
3839
"@dataplan/json": "1.0.0",
3940
"@dataplan/pg": "1.0.0",

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Follows the same lazy-init pattern as upload-resolver.ts.
1212
*/
1313

14-
import { S3Client } from '@aws-sdk/client-s3';
14+
import { createS3Client } from '@constructive-io/s3-utils';
1515
import { getEnvOptions } from '@constructive-io/graphql-env';
1616
import { Logger } from '@pgpmjs/logger';
1717
import type { S3Config, BucketNameResolver, EnsureBucketProvisioned } from 'graphile-presigned-url-plugin';
@@ -66,10 +66,12 @@ export function getPresignedUrlS3Config(): S3Config {
6666
`[presigned-url-resolver] Initializing: bucket=${bucketName} endpoint=${endpoint}`,
6767
);
6868

69-
const client = new S3Client({
69+
const client = createS3Client({
70+
provider: (cdn.provider || 'minio') as any,
7071
region: awsRegion,
71-
credentials: { accessKeyId: awsAccessKey, secretAccessKey: awsSecretKey },
72-
...(endpoint ? { endpoint, forcePathStyle: true } : {}),
72+
accessKeyId: awsAccessKey,
73+
secretAccessKey: awsSecretKey,
74+
...(endpoint ? { endpoint } : {}),
7375
});
7476

7577
s3Config = {

graphql/server/src/scripts/create-bucket.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
// Minimal script to create a bucket in MinIO/S3 using @constructive-io/s3-utils
2-
// Avoid strict type coupling between different @aws-sdk/client-s3 versions
32

4-
import { S3Client } from '@aws-sdk/client-s3';
5-
import { createS3Bucket } from '@constructive-io/s3-utils';
3+
import { createS3Client, createS3Bucket } from '@constructive-io/s3-utils';
4+
import type { StorageProvider } from '@constructive-io/s3-utils';
65
import { getEnvOptions } from '@constructive-io/graphql-env';
76
import { Logger } from '@pgpmjs/logger';
87

@@ -13,22 +12,19 @@ const log = new Logger('create-bucket');
1312
const opts = getEnvOptions();
1413
const { cdn } = opts;
1514

16-
const provider = cdn?.provider || 'minio';
17-
const isMinio = provider === 'minio';
18-
15+
const provider = (cdn?.provider || 'minio') as StorageProvider;
1916
const bucket = cdn?.bucketName || 'test-bucket';
2017
const region = cdn?.awsRegion || 'us-east-1';
2118
const accessKey = cdn?.awsAccessKey || 'minioadmin';
2219
const secretKey = cdn?.awsSecretKey || 'minioadmin';
2320
const endpoint = cdn?.endpoint || 'http://localhost:9000';
2421

25-
const client: any = new S3Client({
22+
const client = createS3Client({
23+
provider,
2624
region,
27-
credentials: { accessKeyId: accessKey, secretAccessKey: secretKey },
28-
...(isMinio ? {
29-
endpoint,
30-
forcePathStyle: true,
31-
} : {}),
25+
accessKeyId: accessKey,
26+
secretAccessKey: secretKey,
27+
...(endpoint ? { endpoint } : {}),
3228
});
3329

3430
const res = await createS3Bucket(client as any, bucket, { provider });

packages/bucket-provisioner/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
"test:watch": "jest --watch"
3030
},
3131
"dependencies": {
32-
"@aws-sdk/client-s3": "^3.1009.0"
32+
"@aws-sdk/client-s3": "^3.1009.0",
33+
"@constructive-io/s3-utils": "workspace:^"
3334
},
3435
"devDependencies": {
3536
"makage": "^0.3.0"
Lines changed: 19 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,34 @@
11
/**
22
* S3 client factory.
33
*
4-
* Creates a configured S3Client from a StorageConnectionConfig.
5-
* Handles provider-specific settings (path-style for MinIO, etc.).
4+
* Delegates to @constructive-io/s3-utils for the actual S3Client creation.
5+
* Re-exports createS3Client so existing consumers of bucket-provisioner
6+
* continue to work without changes.
67
*/
78

8-
import { S3Client } from '@aws-sdk/client-s3';
9+
import { createS3Client as createS3ClientFromUtils, S3ConfigError } from '@constructive-io/s3-utils';
10+
import type { S3Client } from '@aws-sdk/client-s3';
911
import type { StorageConnectionConfig } from './types';
1012
import { ProvisionerError } from './types';
13+
import type { ProvisionerErrorCode } from './types';
1114

1215
/**
1316
* Create an S3Client from a storage connection config.
1417
*
15-
* Provider-specific defaults:
16-
* - `minio`: forces path-style URLs (required by MinIO)
17-
* - `r2`: forces path-style URLs (required by Cloudflare R2)
18-
* - `s3`: uses virtual-hosted style (AWS default)
19-
* - `gcs`: forces path-style URLs (GCS S3-compatible API)
20-
* - `spaces`: uses virtual-hosted style (DigitalOcean default)
18+
* Delegates to @constructive-io/s3-utils/createS3Client.
19+
* This wrapper exists for backward compatibility — new code should
20+
* import createS3Client from @constructive-io/s3-utils directly.
21+
*
22+
* Catches S3ConfigError from s3-utils and re-throws as ProvisionerError
23+
* so existing consumers that catch ProvisionerError continue to work.
2124
*/
2225
export function createS3Client(config: StorageConnectionConfig): S3Client {
23-
if (!config.accessKeyId || !config.secretAccessKey) {
24-
throw new ProvisionerError(
25-
'INVALID_CONFIG',
26-
'accessKeyId and secretAccessKey are required',
27-
);
28-
}
29-
30-
if (!config.region) {
31-
throw new ProvisionerError(
32-
'INVALID_CONFIG',
33-
'region is required',
34-
);
26+
try {
27+
return createS3ClientFromUtils(config);
28+
} catch (err) {
29+
if (err instanceof S3ConfigError) {
30+
throw new ProvisionerError(err.code as ProvisionerErrorCode, err.message);
31+
}
32+
throw err;
3533
}
36-
37-
// Providers that require path-style URLs
38-
const pathStyleProviders = new Set(['minio', 'r2', 'gcs']);
39-
const forcePathStyle = config.forcePathStyle ?? pathStyleProviders.has(config.provider);
40-
41-
// Non-AWS providers require an endpoint
42-
if (config.provider !== 's3' && !config.endpoint) {
43-
throw new ProvisionerError(
44-
'INVALID_CONFIG',
45-
`endpoint is required for provider '${config.provider}'`,
46-
);
47-
}
48-
49-
return new S3Client({
50-
region: config.region,
51-
endpoint: config.endpoint,
52-
forcePathStyle,
53-
credentials: {
54-
accessKeyId: config.accessKeyId,
55-
secretAccessKey: config.secretAccessKey,
56-
},
57-
});
5834
}

pnpm-lock.yaml

Lines changed: 12 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

uploads/s3-streamer/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
"test:watch": "jest --watch"
3030
},
3131
"devDependencies": {
32-
"@constructive-io/s3-utils": "workspace:^",
3332
"@pgpmjs/env": "workspace:^",
3433
"glob": "^13.0.6",
3534
"makage": "^0.3.0"
@@ -38,6 +37,7 @@
3837
"@aws-sdk/client-s3": "^3.1009.0",
3938
"@aws-sdk/lib-storage": "^3.1009.0",
4039
"@constructive-io/content-type-stream": "workspace:^",
40+
"@constructive-io/s3-utils": "workspace:^",
4141
"@pgpmjs/types": "workspace:^"
4242
},
4343
"keywords": [

uploads/s3-streamer/src/s3.ts

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { S3Client, S3ClientConfig } from '@aws-sdk/client-s3';
1+
import { createS3Client } from '@constructive-io/s3-utils';
2+
import type { StorageProvider } from '@constructive-io/s3-utils';
3+
import type { S3Client } from '@aws-sdk/client-s3';
24
import type { BucketProvider } from '@pgpmjs/types';
35

46
interface S3Options {
@@ -10,23 +12,11 @@ interface S3Options {
1012
}
1113

1214
export default function getS3(opts: S3Options): S3Client {
13-
const awsConfig: S3ClientConfig = {
15+
return createS3Client({
16+
provider: (opts.provider as StorageProvider) || 'minio',
1417
region: opts.awsRegion,
15-
...(opts.awsAccessKey && opts.awsSecretKey
16-
? {
17-
credentials: {
18-
accessKeyId: opts.awsAccessKey,
19-
secretAccessKey: opts.awsSecretKey,
20-
},
21-
}
22-
: {}),
23-
...(opts.endpoint
24-
? {
25-
endpoint: opts.endpoint,
26-
forcePathStyle: true,
27-
}
28-
: {}),
29-
};
30-
31-
return new S3Client(awsConfig);
18+
accessKeyId: opts.awsAccessKey,
19+
secretAccessKey: opts.awsSecretKey,
20+
...(opts.endpoint ? { endpoint: opts.endpoint } : {}),
21+
});
3222
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* Tests for the unified S3 client factory.
3+
*/
4+
5+
import { createS3Client, S3ConfigError } from '../src/client';
6+
import type { StorageConnectionConfig } from '../src/client';
7+
8+
describe('createS3Client', () => {
9+
const baseConfig: StorageConnectionConfig = {
10+
provider: 's3',
11+
region: 'us-east-1',
12+
accessKeyId: 'AKIATEST',
13+
secretAccessKey: 'secrettest',
14+
};
15+
16+
it('creates client for AWS S3', () => {
17+
const client = createS3Client(baseConfig);
18+
expect(client).toBeDefined();
19+
expect(typeof client.send).toBe('function');
20+
});
21+
22+
it('creates client for MinIO with endpoint', () => {
23+
const client = createS3Client({
24+
...baseConfig,
25+
provider: 'minio',
26+
endpoint: 'http://minio:9000',
27+
});
28+
expect(client).toBeDefined();
29+
});
30+
31+
it('creates client for R2 with endpoint', () => {
32+
const client = createS3Client({
33+
...baseConfig,
34+
provider: 'r2',
35+
endpoint: 'https://account.r2.cloudflarestorage.com',
36+
});
37+
expect(client).toBeDefined();
38+
});
39+
40+
it('creates client for GCS with endpoint', () => {
41+
const client = createS3Client({
42+
...baseConfig,
43+
provider: 'gcs',
44+
endpoint: 'https://storage.googleapis.com',
45+
});
46+
expect(client).toBeDefined();
47+
});
48+
49+
it('creates client for DO Spaces with endpoint', () => {
50+
const client = createS3Client({
51+
...baseConfig,
52+
provider: 'spaces',
53+
endpoint: 'https://nyc3.digitaloceanspaces.com',
54+
});
55+
expect(client).toBeDefined();
56+
});
57+
58+
it('throws S3ConfigError on missing accessKeyId', () => {
59+
expect(() =>
60+
createS3Client({ ...baseConfig, accessKeyId: '' }),
61+
).toThrow(S3ConfigError);
62+
});
63+
64+
it('throws S3ConfigError on missing secretAccessKey', () => {
65+
expect(() =>
66+
createS3Client({ ...baseConfig, secretAccessKey: '' }),
67+
).toThrow(S3ConfigError);
68+
});
69+
70+
it('throws S3ConfigError on missing region', () => {
71+
expect(() =>
72+
createS3Client({ ...baseConfig, region: '' }),
73+
).toThrow(S3ConfigError);
74+
});
75+
76+
it('throws on non-AWS provider without endpoint', () => {
77+
expect(() =>
78+
createS3Client({ ...baseConfig, provider: 'minio' }),
79+
).toThrow(S3ConfigError);
80+
expect(() =>
81+
createS3Client({ ...baseConfig, provider: 'minio' }),
82+
).toThrow("endpoint is required for provider 'minio'");
83+
});
84+
85+
it('does not throw on AWS S3 without endpoint', () => {
86+
expect(() => createS3Client(baseConfig)).not.toThrow();
87+
});
88+
89+
it('respects explicit forcePathStyle override', () => {
90+
const client = createS3Client({
91+
...baseConfig,
92+
forcePathStyle: true,
93+
});
94+
expect(client).toBeDefined();
95+
});
96+
});
97+
98+
describe('S3ConfigError', () => {
99+
it('has correct name and code', () => {
100+
const err = new S3ConfigError('INVALID_CONFIG', 'test message');
101+
expect(err.name).toBe('S3ConfigError');
102+
expect(err.code).toBe('INVALID_CONFIG');
103+
expect(err.message).toBe('test message');
104+
expect(err).toBeInstanceOf(Error);
105+
});
106+
});

0 commit comments

Comments
 (0)