Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump aws-sdk to v3 for s3 #10363

Merged
merged 2 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"@tensorflow/tfjs-node": "4.2.0"
},
"dependencies": {
"@aws-sdk/client-s3": "^3.294.0",
"@aws-sdk/lib-storage": "^3.294.0",
"@aws-sdk/node-http-handler": "^3.292.0",
"@bull-board/api": "5.0.0",
"@bull-board/fastify": "5.0.0",
"@bull-board/ui": "5.0.0",
Expand All @@ -59,7 +62,6 @@
"ajv": "8.12.0",
"archiver": "5.3.1",
"autwh": "0.1.0",
"aws-sdk": "2.1318.0",
"bcryptjs": "2.4.3",
"blurhash": "2.0.5",
"bull": "4.10.4",
Expand Down Expand Up @@ -190,6 +192,7 @@
"@types/ws": "8.5.4",
"@typescript-eslint/eslint-plugin": "5.54.1",
"@typescript-eslint/parser": "5.54.1",
"aws-sdk-client-mock": "^2.1.1",
"cross-env": "7.0.3",
"eslint": "8.35.0",
"eslint-plugin-import": "2.27.5",
Expand Down
53 changes: 25 additions & 28 deletions packages/backend/src/core/DriveService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { v4 as uuid } from 'uuid';
import sharp from 'sharp';
import { sharpBmp } from 'sharp-read-bmp';
import { IsNull } from 'typeorm';
import { DeleteObjectCommandInput, PutObjectCommandInput, NoSuchKey } from '@aws-sdk/client-s3';
import { DI } from '@/di-symbols.js';
import type { DriveFilesRepository, UsersRepository, DriveFoldersRepository, UserProfilesRepository } from '@/models/index.js';
import type { Config } from '@/config.js';
Expand Down Expand Up @@ -36,7 +37,6 @@ import { bindThis } from '@/decorators.js';
import { RoleService } from '@/core/RoleService.js';
import { correctFilename } from '@/misc/correct-filename.js';
import { isMimeImage } from '@/misc/is-mime-image.js';
import type S3 from 'aws-sdk/clients/s3.js';

type AddFileArgs = {
/** User who wish to add file */
Expand Down Expand Up @@ -81,6 +81,7 @@ type UploadFromUrlArgs = {
export class DriveService {
private registerLogger: Logger;
private downloaderLogger: Logger;
private deleteLogger: Logger;

constructor(
@Inject(DI.config)
Expand Down Expand Up @@ -118,6 +119,7 @@ export class DriveService {
const logger = new Logger('drive', 'blue');
this.registerLogger = logger.createSubLogger('register', 'yellow');
this.downloaderLogger = logger.createSubLogger('downloader');
this.deleteLogger = logger.createSubLogger('delete');
}

/***
Expand Down Expand Up @@ -368,7 +370,7 @@ export class DriveService {
Body: stream,
ContentType: type,
CacheControl: 'max-age=31536000, immutable',
} as S3.PutObjectRequest;
} as PutObjectCommandInput;

if (filename) params.ContentDisposition = contentDisposition(
'inline',
Expand All @@ -378,21 +380,16 @@ export class DriveService {
);
if (meta.objectStorageSetPublicRead) params.ACL = 'public-read';

const s3 = this.s3Service.getS3(meta);

const upload = s3.upload(params, {
partSize: s3.endpoint.hostname === 'storage.googleapis.com' ? 500 * 1024 * 1024 : 8 * 1024 * 1024,
});

await upload.promise()
await this.s3Service.upload(meta, params)
.then(
result => {
if (result) {
if ('Bucket' in result) { // CompleteMultipartUploadCommandOutput
this.registerLogger.debug(`Uploaded: ${result.Bucket}/${result.Key} => ${result.Location}`);
} else {
this.registerLogger.error(`Upload Result Empty: key = ${key}, filename = ${filename}`);
} else { // AbortMultipartUploadCommandOutput
this.registerLogger.error(`Upload Result Aborted: key = ${key}, filename = ${filename}`);
}
},
})
.catch(
err => {
this.registerLogger.error(`Upload Failed: key = ${key}, filename = ${filename}`, err);
},
Expand Down Expand Up @@ -528,10 +525,10 @@ export class DriveService {
};

const properties: {
width?: number;
height?: number;
orientation?: number;
} = {};
width?: number;
height?: number;
orientation?: number;
} = {};

if (info.width) {
properties['width'] = info.width;
Expand Down Expand Up @@ -720,22 +717,22 @@ export class DriveService {
@bindThis
public async deleteObjectStorageFile(key: string) {
const meta = await this.metaService.fetch();

const s3 = this.s3Service.getS3(meta);

try {
await s3.deleteObject({
Bucket: meta.objectStorageBucket!,
const param = {
Bucket: meta.objectStorageBucket,
Key: key,
}).promise();
} as DeleteObjectCommandInput;

await this.s3Service.delete(meta, param);
} catch (err: any) {
if (err.code === 'NoSuchKey') {
console.warn(`The object storage had no such key to delete: ${key}. Skipping this.`, err);
if (err.name === 'NoSuchKey') {
this.deleteLogger.warn(`The object storage had no such key to delete: ${key}. Skipping this.`, err as Error);
return;
} else {
throw new Error(`Failed to delete the file from the object storage with the given key: ${key}`, {
cause: err,
});
}
throw new Error(`Failed to delete the file from the object storage with the given key: ${key}`, {
cause: err,
});
}
}

Expand Down
61 changes: 44 additions & 17 deletions packages/backend/src/core/S3Service.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { URL } from 'node:url';
import * as http from 'node:http';
import * as https from 'node:https';
import { Inject, Injectable } from '@nestjs/common';
import S3 from 'aws-sdk/clients/s3.js';
import { DeleteObjectCommand, S3Client } from '@aws-sdk/client-s3';
import { Upload } from '@aws-sdk/lib-storage';
import { NodeHttpHandler, NodeHttpHandlerOptions } from '@aws-sdk/node-http-handler';
import { DI } from '@/di-symbols.js';
import type { Config } from '@/config.js';
import type { Meta } from '@/models/entities/Meta.js';
import { HttpRequestService } from '@/core/HttpRequestService.js';
import { bindThis } from '@/decorators.js';
import type { DeleteObjectCommandInput, PutObjectCommandInput } from '@aws-sdk/client-s3';

@Injectable()
export class S3Service {
Expand All @@ -18,25 +23,47 @@ export class S3Service {
}

@bindThis
public getS3(meta: Meta) {
public getS3Client(meta: Meta): S3Client {
const u = meta.objectStorageEndpoint
? `${meta.objectStorageUseSSL ? 'https://' : 'http://'}${meta.objectStorageEndpoint}`
: `${meta.objectStorageUseSSL ? 'https://' : 'http://'}example.net`;
? `${meta.objectStorageUseSSL ? 'https' : 'http'}://${meta.objectStorageEndpoint}`
: `${meta.objectStorageUseSSL ? 'https' : 'http'}://example.net`; // dummy url to select http(s) agent

return new S3({
endpoint: meta.objectStorageEndpoint && meta.objectStorageEndpoint.length > 0
? meta.objectStorageEndpoint
: undefined,
accessKeyId: meta.objectStorageAccessKey!,
secretAccessKey: meta.objectStorageSecretKey!,
const agent = this.httpRequestService.getAgentByUrl(new URL(u), !meta.objectStorageUseProxy);
const handlerOption: NodeHttpHandlerOptions = {};
if (meta.objectStorageUseSSL) {
handlerOption.httpsAgent = agent as https.Agent;
} else {
handlerOption.httpAgent = agent as http.Agent;
}

return new S3Client({
endpoint: meta.objectStorageEndpoint ? u : undefined,
credentials: (meta.objectStorageAccessKey !== null && meta.objectStorageSecretKey !== null) ? {
accessKeyId: meta.objectStorageAccessKey,
secretAccessKey: meta.objectStorageSecretKey,
} : undefined,
region: meta.objectStorageRegion ?? undefined,
sslEnabled: meta.objectStorageUseSSL,
s3ForcePathStyle: !meta.objectStorageEndpoint // AWS with endPoint omitted
? false
: meta.objectStorageS3ForcePathStyle,
httpOptions: {
agent: this.httpRequestService.getAgentByUrl(new URL(u), !meta.objectStorageUseProxy),
},
tls: meta.objectStorageUseSSL,
forcePathStyle: meta.objectStorageEndpoint ? meta.objectStorageS3ForcePathStyle : false, // AWS with endPoint omitted
requestHandler: new NodeHttpHandler(handlerOption),
});
}

@bindThis
public async upload(meta: Meta, input: PutObjectCommandInput) {
const client = this.getS3Client(meta);
return new Upload({
client,
params: input,
partSize: (client.config.endpoint && (await client.config.endpoint()).hostname === 'storage.googleapis.com')
? 500 * 1024 * 1024
: 8 * 1024 * 1024,
}).done();
}

@bindThis
public delete(meta: Meta, input: DeleteObjectCommandInput) {
const client = this.getS3Client(meta);
return client.send(new DeleteObjectCommand(input));
}
}
61 changes: 31 additions & 30 deletions packages/backend/test/unit/DriveService.ts
Original file line number Diff line number Diff line change
@@ -1,55 +1,56 @@
process.env.NODE_ENV = 'test';

import { jest } from '@jest/globals';
import { Test } from '@nestjs/testing';
import { DeleteObjectCommandOutput, DeleteObjectCommand, NoSuchKey, InvalidObjectState, S3Client } from '@aws-sdk/client-s3';
import { mockClient } from 'aws-sdk-client-mock';
import { GlobalModule } from '@/GlobalModule.js';
import { DriveService } from '@/core/DriveService.js';
import { CoreModule } from '@/core/CoreModule.js';
import { S3Service } from '@/core/S3Service';
import type { Meta } from '@/models';
import type { DeleteObjectOutput } from 'aws-sdk/clients/s3';
import type { AWSError } from 'aws-sdk/lib/error';
import type { PromiseResult, Request } from 'aws-sdk/lib/request';
import type { TestingModule } from '@nestjs/testing';

describe('DriveService', () => {
let app: TestingModule;
let driveService: DriveService;
const s3Mock = mockClient(S3Client);

beforeEach(async () => {
beforeAll(async () => {
app = await Test.createTestingModule({
imports: [GlobalModule, CoreModule],
providers: [DriveService, S3Service],
providers: [DriveService],
}).compile();
app.enableShutdownHooks();
driveService = app.get<DriveService>(DriveService);
});

const s3Service = app.get<S3Service>(S3Service);
const s3 = s3Service.getS3({} as Meta);

// new S3() surprisingly does not return an instance of class S3.
// Let's use getPrototypeOf here to get a real prototype, since spying on S3.prototype doesn't work.
// TODO: Use `aws-sdk-client-mock` package when upgrading to AWS SDK v3.
jest.spyOn(Object.getPrototypeOf(s3), 'deleteObject').mockImplementation(() => {
// Roughly mock AWS request object
return {
async promise(): Promise<PromiseResult<DeleteObjectOutput, AWSError>> {
const err = new Error('mock') as AWSError;
err.code = 'NoSuchKey';
throw err;
},
} as Request<DeleteObjectOutput, AWSError>;
});
beforeEach(async () => {
s3Mock.reset();
});

afterAll(async () => {
await app.close();
});

describe('Object storage', () => {
test('delete a file', async () => {
s3Mock.on(DeleteObjectCommand)
.resolves({} as DeleteObjectCommandOutput);

await driveService.deleteObjectStorageFile('peace of the world');
});

test('delete a file then unexpected error', async () => {
s3Mock.on(DeleteObjectCommand)
.rejects(new InvalidObjectState({ $metadata: {}, message: '' }));

await expect(driveService.deleteObjectStorageFile('unexpected')).rejects.toThrowError(Error);
});

test('delete a file with no valid key', async () => {
try {
await driveService.deleteObjectStorageFile('lol no way');
} catch (err: any) {
console.log(err.cause);
throw err;
}
// Some S3 implementations returns 404 Not Found on deleting with a non-existent key
s3Mock.on(DeleteObjectCommand)
.rejects(new NoSuchKey({ $metadata: {}, message: 'allowed error.' }));

await driveService.deleteObjectStorageFile('lol no way');
});
});
});
77 changes: 77 additions & 0 deletions packages/backend/test/unit/S3Service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
process.env.NODE_ENV = 'test';

import { Test } from '@nestjs/testing';
import { UploadPartCommand, CompleteMultipartUploadCommand, CreateMultipartUploadCommand, S3Client, PutObjectCommand } from '@aws-sdk/client-s3';
import { mockClient } from 'aws-sdk-client-mock';
import { GlobalModule } from '@/GlobalModule.js';
import { CoreModule } from '@/core/CoreModule.js';
import { S3Service } from '@/core/S3Service';
import { Meta } from '@/models';
import type { TestingModule } from '@nestjs/testing';

describe('S3Service', () => {
let app: TestingModule;
let s3Service: S3Service;
const s3Mock = mockClient(S3Client);

beforeAll(async () => {
app = await Test.createTestingModule({
imports: [GlobalModule, CoreModule],
providers: [S3Service],
}).compile();
app.enableShutdownHooks();
s3Service = app.get<S3Service>(S3Service);
});

beforeEach(async () => {
s3Mock.reset();
});

afterAll(async () => {
await app.close();
});

describe('upload', () => {
test('upload a file', async () => {
s3Mock.on(PutObjectCommand).resolves({});

await s3Service.upload({ objectStorageRegion: 'us-east-1' } as Meta, {
Bucket: 'fake',
Key: 'fake',
Body: 'x',
});
});

test('upload a large file', async () => {
s3Mock.on(CreateMultipartUploadCommand).resolves({ UploadId: '1' });
s3Mock.on(UploadPartCommand).resolves({ ETag: '1' });
s3Mock.on(CompleteMultipartUploadCommand).resolves({ Bucket: 'fake', Key: 'fake' });

await s3Service.upload({} as Meta, {
Bucket: 'fake',
Key: 'fake',
Body: 'x'.repeat(8 * 1024 * 1024 + 1), // デフォルトpartSizeにしている 8 * 1024 * 1024 を越えるサイズ
});
});

test('upload a file error', async () => {
s3Mock.on(PutObjectCommand).rejects({ name: 'Fake Error' });

await expect(s3Service.upload({ objectStorageRegion: 'us-east-1' } as Meta, {
Bucket: 'fake',
Key: 'fake',
Body: 'x',
})).rejects.toThrowError(Error);
});

test('upload a large file error', async () => {
s3Mock.on(UploadPartCommand).rejects();

await expect(s3Service.upload({} as Meta, {
Bucket: 'fake',
Key: 'fake',
Body: 'x'.repeat(8 * 1024 * 1024 + 1), // デフォルトpartSizeにしている 8 * 1024 * 1024 を越えるサイズ
})).rejects.toThrowError(Error);
});
});
});
Loading