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

@keystone/file-adapter's S3Adapter incorrectly requires constructor parameters #2965

Closed
intmainvoid opened this issue May 14, 2020 · 1 comment

Comments

@intmainvoid
Copy link
Contributor

Bug report

Describe the bug

Creating an instance of S3Adapter (from @keystone/file-adapters) without providing accessKeyId, secretAccessKey and region throws an error and terminates Keystone's Node.js instance. The error is thrown with the following message:

S3Adapter requires accessKeyId, secretAccessKey, region, bucket, folder.

To Reproduce

Create an instance of the S3Adapter as follows:

const fileAdapter = new S3Adapter({
	// accessKeyId // not provided
	// secretAccessKey: // not provided
	// region: // not provided
	bucket: 'website-submission-attachments',
	folder: 'test',
	s3Options: {
		apiVersion: '2006-03-01',
	},
	uploadParams: ({ filename, id, mimetype, encoding }: UploadParams) => {
		return {
			Metadata: {
				keystone_id: String(id),
			},
		};
	},
});

Expected behaviour

The S3Adapter connects to the S3 bucket regardless of the presence of the 3 missing parameters and that no attempt is made by the S3Adapter to pass the the three missing parameters on to the AWS.S3 constructor params.

Screenshots

N/A

System information

macOS

Additional context

@keystone/file-adapter's S3Adapter requires - among others - the following parameters in its constructor which are then passed to the AWS SDK constructor: AWS.S3:

    accessKeyId,
    secretAccessKey,
    region,

There are many common use cases where creating an S3 instance without these parameters are valid - and in fact - accessKeyId and secretAccessKey are deprecated in the AWS SDK.

Proposed solution

Remove the hard requirement for these parameters to be provided and instead simply allow the user to provide them as required in the s3Options object.

@MadeByMike
Copy link
Contributor

Fixed in #2967

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants