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

Add AwsSigV4 signing functionality #279

Merged
merged 26 commits into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d78ae1c
Add AwsSigV4 signing functionality
harshavamsi Aug 25, 2022
59bbbf4
Adlicense text to signer types
harshavamsi Aug 25, 2022
caf4d74
Pulling aws signer into separate namespace
harshavamsi Aug 25, 2022
7faaae1
Adding separate injection point for v4Signer
harshavamsi Aug 26, 2022
6a67179
Fix name spacing and bump version
harshavamsi Aug 26, 2022
90a443d
Typo in readme
harshavamsi Aug 26, 2022
e50e8fc
Adding 0BSD to allow license
harshavamsi Aug 29, 2022
f58e21e
Split code snippets into USER GUIDE
harshavamsi Aug 29, 2022
f1e5a16
Remove un-used package and update license
harshavamsi Aug 29, 2022
93f347b
Merge branch 'main' into awsv4signer
harshavamsi Aug 29, 2022
6d7b79c
Fix language in user guide
harshavamsi Aug 29, 2022
8505f85
Add types to dev dependencies
harshavamsi Aug 29, 2022
562d683
Update USER_GUIDE.md
harshavamsi Aug 30, 2022
341df4f
add credentials refresh options
rawpixel-vincent Aug 31, 2022
f629fa1
fix AwsSigv4Signer type with Promise
rawpixel-vincent Aug 31, 2022
c952ba0
remove JSDoc
rawpixel-vincent Aug 31, 2022
4abf7ea
update example usage
rawpixel-vincent Sep 1, 2022
48e5b3a
update credentials refresh strategy
rawpixel-vincent Sep 2, 2022
e376ef4
update credentials refresh and expiration
rawpixel-vincent Sep 3, 2022
1475bf0
fix types
rawpixel-vincent Sep 3, 2022
52ab9ad
add failure to refresh credentials test case
rawpixel-vincent Sep 3, 2022
d31eb6f
cleanup and comments
rawpixel-vincent Sep 3, 2022
f036b30
clarify code example in the docs
rawpixel-vincent Sep 6, 2022
ef2b3c7
remove explicit async from code example
rawpixel-vincent Sep 6, 2022
13d30de
remove unused credentialsState.acquiredAt
rawpixel-vincent Sep 6, 2022
b1308b9
Minor doc and misc fixes
harshavamsi Sep 7, 2022
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
87 changes: 87 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,93 @@ async function search() {
search().catch(console.log);
```

### With AWS SigV4 signing
```javascript
const host = ""; // Opensearch domain URL e.g. https://search-xxx.region.es.amazonaws.com
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
const { Client, AwsV4Signer } = require('@opensearch-project/opensearch');
const { defaultProvider } = require("@aws-sdk/credential-provider-node");

async function getClient() {
const credentials = await defaultProvider()();
var client = new Client({
...AwsV4Signer({
credentials: credentials,
region: "us-west-2",
}),
node: host,
});
return client;
}

async function search() {

var client = await getClient();

var index_name = "books-test-1";
var settings = {
settings: {
index: {
number_of_shards: 4,
number_of_replicas: 3,
},
},
};

var response = await client.indices.create({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identical to the samples above? Maybe we should break up the examples to specific sections (indexing, searching, AWS Sigv4 Signing...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why it lives in a separate code block is just because of the difference in the imports and the way the client is initialized. Because the signer needs credentials, it's an async call. Felt like maybe the two should be written separately.

index: index_name,
body: settings,
});

console.log("Creating index:");
console.log(response.body);

// Add a document to the index.
var document = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the examples here look very similar to the already existing ones. I see that initializing the client is different, can we provide may be just one example here so we don't duplicate all the test data?

Also, consider moving this to a USER_GUIDE since the number of examples is increasing, might make the README very long.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, but I also think we can break this up in this PR or do it in another PR, so no hard ask from me. FYI I do like what they have done in the .NET client: https://github.com/opensearch-project/opensearch-net/blob/main/USER_GUIDE.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VachaShah @dblock I've split the code snippets into a new USER_GUIDE. wdyt?

title: "The Outsider",
author: "Stephen King",
year: "2018",
genre: "Crime fiction",
};

var id = "1";

var response = await client.index({
id: id,
index: index_name,
body: document,
refresh: true,
});

console.log("Adding document:");
console.log(response.body);

var response = await client.bulk({ body: bulk_documents });
console.log(response.body);

// Search for the document.
var query = {
query: {
match: {
title: {
query: "The Outsider",
},
},
},
};

var response = await client.search({
index: index_name,
body: query,
});

console.log("Search results:");
console.log(response.body.hits);
}

search().catch(console.log);
```


## Project Resources

- [Project Website](https://opensearch.org/)
Expand Down
2 changes: 2 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import {
ResurrectEvent,
BasicAuth,
} from './lib/pool';
import AwsV4Signer from './lib/AwsV4Signer';
import Serializer from './lib/Serializer';
import Helpers from './lib/Helpers';
import * as errors from './lib/errors';
Expand Down Expand Up @@ -5050,4 +5051,5 @@ export {
ClientOptions,
NodeOptions,
ClientExtendsCallbackOptions,
AwsV4Signer
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
};
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ if (clientVersion.includes('-')) {
// clean prerelease
clientVersion = clientVersion.slice(0, clientVersion.indexOf('-')) + 'p';
}
const AwsV4Signer = require('./lib/AwsV4Signer');

const kInitialOptions = Symbol('opensearchjs-initial-options');
const kChild = Symbol('opensearchjs-child');
Expand Down Expand Up @@ -348,4 +349,5 @@ module.exports = {
Serializer,
events,
errors,
AwsV4Signer,
};
1 change: 1 addition & 0 deletions index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ export const Connection = mod.Connection;
export const Serializer = mod.Serializer;
export const events = mod.events;
export const errors = mod.errors;
export const awsV4Signer = mod.AwsV4Signer;
47 changes: 47 additions & 0 deletions lib/AwsV4Signer.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
* Licensed to Elasticsearch B.V. under one or more contributor
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { Credentials } from '@aws-sdk/types';
import Connection from './Connection';
import * as http from 'http';

interface AwsV4SignerOptions {
credentials: Credentials;
region: string;
}

export interface AwsV4SignerResponse {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is Response standard? If it is I'm cool with it but for me feels like Response for something that is happening pre-flight feels off

Connection: Connection;
buildSignedRequestObject(request: any): http.ClientRequestArgs;
}

export default function AwsV4Signer(opts: AwsV4SignerOptions): AwsV4SignerResponse;

export {};
62 changes: 62 additions & 0 deletions lib/AwsV4Signer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
* Licensed to Elasticsearch B.V. under one or more contributor
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

'use strict';
const Connection = require('./Connection');
const aws4 = require('aws4');
const { AwsV4SignerError } = require('./errors');

function AwsV4Signer(opts) {
if (opts && (!opts.region || opts.region === null || opts.region === '')) {
throw new AwsV4SignerError('Region cannot be empty');
}
if (opts && (!opts.credentials || opts.credentials === null || opts.credentials === '')) {
throw new AwsV4SignerError('Credentials cannot be empty');
}

function buildSignedRequestObject(request = {}) {
request.service = 'es';
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
request.region = opts.region;
request.headers = request.headers || {};
request.headers['host'] = request.hostname;
return aws4.sign(request, opts.credentials);
}
class AwsV4SignedConnection extends Connection {
buildRequestObject(params) {
const request = super.buildRequestObject(params);
return buildSignedRequestObject(request);
}
}
return {
Connection: AwsV4SignedConnection,
buildSignedRequestObject,
};
}
module.exports = AwsV4Signer;
7 changes: 7 additions & 0 deletions lib/errors.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,10 @@ export declare class NotCompatibleError<
meta: ApiResponse<TResponse, TContext>;
constructor(meta: ApiResponse);
}

export declare class AwsV4SignerError extends OpenSearchClientError {
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
name: string;
message: string;
data: any;
constructor(message: string, data: any);
}
11 changes: 11 additions & 0 deletions lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ class NotCompatibleError extends OpenSearchClientError {
}
}

class AwsV4SignerError extends OpenSearchClientError {
constructor(message, data) {
super(message, data);
Error.captureStackTrace(this, AwsV4SignerError);
this.name = 'AwsV4SignerError';
this.message = message || 'AwsV4Signer Error';
this.data = data;
}
}

module.exports = {
OpenSearchClientError,
TimeoutError,
Expand All @@ -169,4 +179,5 @@ module.exports = {
ResponseError,
RequestAbortedError,
NotCompatibleError,
AwsV4SignerError,
};
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@
"xmlbuilder2": "^2.4.1"
},
"dependencies": {
"@aws-sdk/credential-provider-node": "^3.154.0",
Copy link
Member

@dblock dblock Aug 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does this become a hard dependency for this package? Meaning if I am not doing AWS sigv4, am I still dragging this module in? Can it be avoided? Asking again because I'd like not to drag any vendor specific components in by default.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this dependency like I mentioned below. We are still depending on the aws4 library which we cannot do away(unless we want to re-write all of that implementation) if we want to support signing natively in the client.

"aws4": "^1.11.0",
"debug": "^4.3.1",
"hpagent": "^0.1.1",
"ms": "^2.1.3",
Expand Down
48 changes: 48 additions & 0 deletions test/types/awsv4signer.test-d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
* Licensed to Elasticsearch B.V. under one or more contributor
harshavamsi marked this conversation as resolved.
Show resolved Hide resolved
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { expectType } from 'tsd';
import { AwsV4Signer } from '../../';
import { AwsV4SignerResponse } from '../../lib/AwsV4Signer';

const mockCreds = {
accessKeyId: 'mockCredAccessKeyId',
secretAccessKey: 'mockCredSecretAccessKey',
};

const mockRegion = 'us-west-2';

{
const AwsV4SignerOptions = { credentials: mockCreds, region: mockRegion };

const auth = AwsV4Signer(AwsV4SignerOptions);

expectType<AwsV4SignerResponse>(auth);
}
Loading