Skip to content

Commit

Permalink
fix(makeBucket): pass region when creating a bucket
Browse files Browse the repository at this point in the history
  • Loading branch information
TillaTheHun0 committed Apr 30, 2024
1 parent df4878a commit 418652a
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 9 deletions.
8 changes: 5 additions & 3 deletions adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ const { prop, map, always, identity } = R
*/

/**
* @param {{ minio: any, bucketPrefix: string, useNamespacedBucket: boolean }} config
* @param {{ minio: any, bucketPrefix: string, region?: string, useNamespacedBucket: boolean }} config
* @returns hyper storage terminating adapter impl
*/
export default function (config) {
const { bucketPrefix, useNamespacedBucket, minio } = config
const { bucketPrefix, region, useNamespacedBucket, minio } = config

const lib = useNamespacedBucket ? Namespaced(bucketPrefix) : Multi(bucketPrefix)
const lib = useNamespacedBucket
? Namespaced({ bucketPrefix, region })
: Multi({ bucketPrefix, region })

const client = minioClientSchema.parse({
makeBucket: lib.makeBucket(minio),
Expand Down
1 change: 1 addition & 0 deletions adapter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Deno.test('adapter', async (t) => {
const a = adapter({
minio: happyMinio,
bucketPrefix: 'test',
region: 'us-east-2',
useNamespacedBucket: false,
})

Expand Down
18 changes: 16 additions & 2 deletions lib/multi.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,28 @@ const { prop, map } = R

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

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

const bucketWithPrefix = createPrefix(bucketPrefix)

const makeBucket = (minio) => {
return asyncifyHandle((name) =>
minio.makeBucket(bucketWithPrefix(name)).catch((err) => {
/**
* When using with AWS s3 and coupled with the fact that LocationConstraint
* is not required _only_ for us-east-1 (https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucketConfiguration.html),
* this can cause some hair-pulling gotchas when creating a bucket and not passing region -- the bucket will automatically
* be attempted to be created in us-east-1, REGARDLESS of whichever region used to instantiate the MinIO Client.
*
* 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,
* you will simply get an opaque S3 AccessDenied error when creating the bucket -- your IAM Role might be constrained to only
* 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
* will simply produce a seemingly incoherent "AccessDenied". -_______-
*
* SO, we MUST pass region here that is provided to the adapter, to ensure the bucket is created in the desired region,
* and any credentials imbued by IAM or the VPC are used.
*/
minio.makeBucket(bucketWithPrefix(name), region).catch((err) => {
if (isBucketExistsErr(err)) throw HyperErr({ status: 409, msg: 'bucket already exists' })
throw err // some other err
})
Expand Down
12 changes: 11 additions & 1 deletion lib/multi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Multi } from './multi.js'
const { Async } = crocks

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

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

await t.step('it should make the bucket in the region', () => {
return multi.makeBucket({
makeBucket: (_name, region) => {
assertEquals(region, 'us-east-2')
return Promise.resolve()
},
})('foo')
.toPromise()
})

await t.step('it should throw a HyperErr if the bucket exists', () => {
return multi.makeBucket({
makeBucket: () => {
Expand Down
4 changes: 2 additions & 2 deletions lib/namespaced.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ const checkNamespaceExists = (meta, name) => {
))
}

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

const multi = Multi(bucketPrefix)
const multi = Multi({ bucketPrefix, region })
/**
* The single bucket used for all objects
*
Expand Down
2 changes: 1 addition & 1 deletion lib/namespaced.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Namespaced } from './namespaced.js'
const { Async } = crocks

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

const META = {
createdAt: new Date().toJSON(),
Expand Down
1 change: 1 addition & 0 deletions mod.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export default ({ url, region, bucketPrefix, useNamespacedBucket }) => {
link: (config) => (_) => {
return createAdapter({
minio: config.minio,
region: config.region,
bucketPrefix: config.bucketPrefix,
useNamespacedBucket: config.useNamespacedBucket,
})
Expand Down

0 comments on commit 418652a

Please sign in to comment.