BeerMoneyDev / nest-aws-sdk

A thin wrapping layer around the aws-sdk package for clean NestJS dependency injection.
MIT License
92 stars 13 forks source link

region option in defaultServiceOptions is not used when initialize S3 instance #2

Closed raphaelsoul closed 3 years ago

raphaelsoul commented 3 years ago

Wonderful package with mock support, thanks

But I got some problem.

the following example, I got s3 instance with region=us-east-1. which expect to be ap-southeast-1

upload.module.ts

@Module({
    imports: [
        ConfigModule,
        AwsSdkModule.forRootAsync({
            defaultServiceOptions: {
                useFactory: (configService: ConfigService) => {
                    return {
                        region: configService.get("AWS.REGION", "ap-southeast-1"),
                    }
                },
                imports: [ConfigModule],
                inject: [ConfigService],
            },
            services: [
                {
                    service: S3,
                    serviceOptions: {
                        signatureVersion: "v4",
                        useAccelerateEndpoint: true
                    }
                }
            ],
        }),
    ],
    providers: [UploadService, ConfigService],
    controllers: [],
    exports: [UploadService]
})
export class UploadModule {}

any ideas?

raphaelsoul commented 3 years ago

And if i set accessKeyId and secretAccessKey in defaultServiceOptions also not works.

The generated s3 instance uses accessKey pair from process.env.AWS_ACCESS_KEY_ID and process.env.AWS_ACESSS_KEY_SECRET

Seems I have misunderstand the usage of defaultServiceOptions?

KerryRitter commented 3 years ago

HI @raphaelsoul,

As of now serviceOptions is a full override of defaultServiceOptions - we do not merge the two objects together. In your case, if your S3 does not have a key id or access key defined, it will default to process.env. Similarly, the region resets to the default value instead of the supplied default value.

This is a great note, perhaps we should be spreading these values. In code, we're doing something like this:

// pseudo code - how it works
optionsToUse = serviceOptions ?? defaultServiceOptions
// pseudo code - perhaps how it should work
optionsToUse = { ...defaultServiceOptions, ...serviceOptions }
raphaelsoul commented 3 years ago

HI @raphaelsoul,

As of now serviceOptions is a full override of defaultServiceOptions - we do not merge the two objects together. In your case, if your S3 does not have a key id or access key defined, it will default to process.env. Similarly, the region resets to the default value instead of the supplied default value.

This is a great note, perhaps we should be spreading these values. In code, we're doing something like this:

// pseudo code - how it works
optionsToUse = serviceOptions ?? defaultServiceOptions
// pseudo code - perhaps how it should work
optionsToUse = { ...defaultServiceOptions, ...serviceOptions }

thanks, I think the default options should be merged into serviceOptions. Otherwise I cant access ConfigService while defining serviceOption.

A better option is inject ConfigService into serviceOptions like

services: [
                {
                    service: S3,
                    // this will fully override defaultServiceOptions
                    serviceOptions(configService: ConfigService) {
                        return {
                            signatureVersion: "v4",
                            useAccelerateEndpoint: true,
                            region: configService.get("AWS.REGION", "ap-southeast-1"),
                            accessKeyId: configService.get("AWS.ACCESS_KEY_ID"),
                            secretAccessKey: configService.get("AWS.ACCESS_KEY_SECRET")
                        }
                    }
                }
            ],

Yet serviceOptions receives no params

KerryRitter commented 3 years ago

You should be able to use a useValue or useFactory provider declaration for serviceOptions to be able to do what you've outlined.

raphaelsoul commented 3 years ago

Thanks your suggest works.

example work code:

export class UploadService {

    private s3: S3

    constructor(
        // @InjectAwsService(S3) private readonly s3: S3,
        @InjectAwsDefaultOptions() readonly options: ServiceConfigurationOptions,
        private readonly factory: AwsServiceFactory,
        private readonly configService: ConfigService
    ) {
        this.s3 = this.factory.create<S3>(S3, {
            ...options,
            region: "ap-southeast-1",
            signatureVersion: "v4",
            useAccelerateEndpoint: true,
        })
    }
}