Skip to content

Commit 418652a

Browse files
committed
fix(makeBucket): pass region when creating a bucket
1 parent df4878a commit 418652a

File tree

7 files changed

+37
-9
lines changed

7 files changed

+37
-9
lines changed

adapter.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@ const { prop, map, always, identity } = R
3939
*/
4040

4141
/**
42-
* @param {{ minio: any, bucketPrefix: string, useNamespacedBucket: boolean }} config
42+
* @param {{ minio: any, bucketPrefix: string, region?: string, useNamespacedBucket: boolean }} config
4343
* @returns hyper storage terminating adapter impl
4444
*/
4545
export default function (config) {
46-
const { bucketPrefix, useNamespacedBucket, minio } = config
46+
const { bucketPrefix, region, useNamespacedBucket, minio } = config
4747

48-
const lib = useNamespacedBucket ? Namespaced(bucketPrefix) : Multi(bucketPrefix)
48+
const lib = useNamespacedBucket
49+
? Namespaced({ bucketPrefix, region })
50+
: Multi({ bucketPrefix, region })
4951

5052
const client = minioClientSchema.parse({
5153
makeBucket: lib.makeBucket(minio),

adapter.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ Deno.test('adapter', async (t) => {
5656
const a = adapter({
5757
minio: happyMinio,
5858
bucketPrefix: 'test',
59+
region: 'us-east-2',
5960
useNamespacedBucket: false,
6061
})
6162

lib/multi.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,28 @@ const { prop, map } = R
55

66
export const createPrefix = (prefix) => (str) => `${prefix}-${str}`
77

8-
export const Multi = (bucketPrefix) => {
8+
export const Multi = ({ bucketPrefix, region }) => {
99
if (!bucketPrefix) throw new Error('bucketPrefix is required')
1010

1111
const bucketWithPrefix = createPrefix(bucketPrefix)
1212

1313
const makeBucket = (minio) => {
1414
return asyncifyHandle((name) =>
15-
minio.makeBucket(bucketWithPrefix(name)).catch((err) => {
15+
/**
16+
* When using with AWS s3 and coupled with the fact that LocationConstraint
17+
* is not required _only_ for us-east-1 (https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucketConfiguration.html),
18+
* this can cause some hair-pulling gotchas when creating a bucket and not passing region -- the bucket will automatically
19+
* be attempted to be created in us-east-1, REGARDLESS of whichever region used to instantiate the MinIO Client.
20+
*
21+
* So if you're using something like IAM to securely access s3, or a VPC Endpoint for s3 in a region that is _not_ us-east-1,
22+
* you will simply get an opaque S3 AccessDenied error when creating the bucket -- your IAM Role might be constrained to only
23+
* access, say us-east-2, or your VPC Endpoint is for s3.us-east-2.amazonaws.com, and accessing us-east-1 out of the blue
24+
* will simply produce a seemingly incoherent "AccessDenied". -_______-
25+
*
26+
* SO, we MUST pass region here that is provided to the adapter, to ensure the bucket is created in the desired region,
27+
* and any credentials imbued by IAM or the VPC are used.
28+
*/
29+
minio.makeBucket(bucketWithPrefix(name), region).catch((err) => {
1630
if (isBucketExistsErr(err)) throw HyperErr({ status: 409, msg: 'bucket already exists' })
1731
throw err // some other err
1832
})

lib/multi.test.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Multi } from './multi.js'
66
const { Async } = crocks
77

88
Deno.test('Multi bucket minio client', async (t) => {
9-
const multi = Multi('test')
9+
const multi = Multi({ bucketPrefix: 'test', region: 'us-east-2' })
1010

1111
await t.step('makeBucket', async (t) => {
1212
await t.step('it should resolve if the bucket is created successfully', () => {
@@ -25,6 +25,16 @@ Deno.test('Multi bucket minio client', async (t) => {
2525
.toPromise()
2626
})
2727

28+
await t.step('it should make the bucket in the region', () => {
29+
return multi.makeBucket({
30+
makeBucket: (_name, region) => {
31+
assertEquals(region, 'us-east-2')
32+
return Promise.resolve()
33+
},
34+
})('foo')
35+
.toPromise()
36+
})
37+
2838
await t.step('it should throw a HyperErr if the bucket exists', () => {
2939
return multi.makeBucket({
3040
makeBucket: () => {

lib/namespaced.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ const checkNamespaceExists = (meta, name) => {
6060
))
6161
}
6262

63-
export const Namespaced = (bucketPrefix) => {
63+
export const Namespaced = ({ bucketPrefix, region }) => {
6464
if (!bucketPrefix) throw new Error('bucketPrefix is required')
6565

66-
const multi = Multi(bucketPrefix)
66+
const multi = Multi({ bucketPrefix, region })
6767
/**
6868
* The single bucket used for all objects
6969
*

lib/namespaced.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Namespaced } from './namespaced.js'
66
const { Async } = crocks
77

88
Deno.test('Namespaced bucket minio client', async (t) => {
9-
const namespaced = Namespaced('test')
9+
const namespaced = Namespaced({ bucketPrefix: 'test', region: 'us-east-2' })
1010

1111
const META = {
1212
createdAt: new Date().toJSON(),

mod.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export default ({ url, region, bucketPrefix, useNamespacedBucket }) => {
6060
link: (config) => (_) => {
6161
return createAdapter({
6262
minio: config.minio,
63+
region: config.region,
6364
bucketPrefix: config.bucketPrefix,
6465
useNamespacedBucket: config.useNamespacedBucket,
6566
})

0 commit comments

Comments
 (0)