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

Added support for AWS Sigv4 for UrlLib3. #547

Merged
merged 13 commits into from
Oct 23, 2023

Conversation

dblock
Copy link
Member

@dblock dblock commented Oct 20, 2023

Description

  • Adds SigV4 support for Urllib3HttpConnection.
  • Deprecates AWSV4SignerAuth in favor of RequestsAWSV4SignerAuth
  • Uses Urllib3AWSV4SignerAuth in the documentation since that transport is faster.

Testing

AOS

export SERVICE=es
export ENDPOINT=https://xyz.us-west-2.aoss.amazonaws.com
poetry run python sigv4/urllib3-hello.py 

INFO:Found credentials in environment variables.
INFO:GET https://search-dblock-test-opensearch-27-xyz.us-west-2.es.amazonaws.com:443/ [status:200 request:0.768s]
opensearch: 2.7.0
INFO:PUT https://search-dblock-test-opensearch-27-xyz.us-west-2.es.amazonaws.com:443/movies [status:200 request:2.298s]
INFO:PUT https://search-dblock-test-opensearch-27-xyz.us-west-2.es.amazonaws.com:443/movies/_doc/1 [status:201 request:0.500s]
INFO:POST https://search-dblock-test-opensearch-27-xyz.us-west-2.es.amazonaws.com:443/_search [status:200 request:0.213s]
{'director': 'Bennett Miller', 'title': 'Moneyball', 'year': 2011}
INFO:DELETE https://search-dblock-test-opensearch-27-xyz.us-west-2.es.amazonaws.com:443/movies/_doc/1 [status:200 request:0.108s]
INFO:DELETE https://search-dblock-test-opensearch-27-xyz.us-west-2.es.amazonaws.com:443/movies [status:200 request:0.223s]

AOSS

export SERVICE=aoss
export ENDPOINT=https://xyz.us-west-2.aoss.amazonaws.com
poetry run python sigv4/urllib3-hello.py 
INFO:Found credentials in environment variables.
INFO:PUT https://xyz.us-west-2.aoss.amazonaws.com:443/movies [status:200 request:4.173s]
INFO:PUT https://xyz.us-west-2.aoss.amazonaws.com:443/movies/_doc/1 [status:201 request:3.117s]
INFO:POST https://xyz.us-west-2.aoss.amazonaws.com:443/_search [status:200 request:4.916s]
{'director': 'Bennett Miller', 'title': 'Moneyball', 'year': 2011}
INFO:DELETE https://xyz.us-west-2.aoss.amazonaws.com:443/movies/_doc/1 [status:200 request:1.249s]
INFO:DELETE https://xyz.us-west-2.aoss.amazonaws.com:443/movies [status:200 request:0.168s]

Issues Resolved

Closes #546

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: dblock <dblock@amazon.com>
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #547 (284a5fd) into main (fa8f3a7) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #547      +/-   ##
==========================================
+ Coverage   70.63%   70.66%   +0.02%     
==========================================
  Files          83       83              
  Lines        7857     7877      +20     
==========================================
+ Hits         5550     5566      +16     
- Misses       2307     2311       +4     
Files Coverage Δ
opensearchpy/__init__.py 92.85% <100.00%> (ø)
opensearchpy/connection/http_urllib3.py 82.00% <100.00%> (-4.03%) ⬇️
opensearchpy/helpers/__init__.py 100.00% <100.00%> (ø)
opensearchpy/helpers/signer.py 98.03% <100.00%> (+3.30%) ⬆️

... and 2 files with indirect coverage changes

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
@dblock dblock changed the title WIP: Added support for AWS Sigv4 for UrlLib3. Added support for AWS Sigv4 for UrlLib3. Oct 20, 2023
@dblock dblock force-pushed the urllib3-aws-sig-v4 branch 2 times, most recently from c9f113f to fb9e1b9 Compare October 20, 2023 21:43
@dblock dblock marked this pull request as ready for review October 20, 2023 21:44
@dblock
Copy link
Member Author

dblock commented Oct 20, 2023

@harshavamsi would appreciate your CR since you did support for the requests connection class.

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
# Modifications Copyright OpenSearch Contributors. See
# GitHub history for details.
#
# Licensed to Elasticsearch B.V. under one or more contributor
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this license here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this code was refactored out of test_connection.py, so let's keep it.

guides/auth.md Outdated
@@ -9,24 +9,24 @@ OpenSearch allows you to use different methods for the authentication via `conne

## IAM Authentication

Opensearch-py supports IAM-based authentication via `AWSV4SignerAuth`, which uses `RequestHttpConnection` as the transport class for communicating with OpenSearch clusters running in Amazon Managed OpenSearch and OpenSearch Serverless, and works in conjunction with [botocore](https://pypi.org/project/botocore/).
Opensearch-py supports IAM-based authentication via `RequestsAWSV4SignerAuth` and `Urllib3AWSV4SignerAuth`, which use `RequestHttpConnection` and `Urllib3HttpConnection` respectively, as the transport classes for communicating with OpenSearch clusters running in Amazon Managed OpenSearch and OpenSearch Serverless, and works in conjunction with [botocore](https://pypi.org/project/botocore/).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we recommend Urllib3AWSV4SignerAuth here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've clarified the documentation in cb76874.

self.signer = AWSV4Signer(credentials, region, service)

def __call__(self, method: str, url: str, body: Any) -> Dict[str, str]:
return self.signer.sign(method, url, body)
Copy link
Collaborator

Choose a reason for hiding this comment

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

RequestsAWSV4SignerAuth returns prepared request while Urllib3AWSV4SignerAuth returns the headers, should we keep them consistent?

Copy link
Member Author

Choose a reason for hiding this comment

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

The requests library expect a callable that derives from requests.auth.AuthBase and receives a request object. It's called inside the requests library.

The urllib3 library doesn't support such an interface. It actually splits the client constructor and exposes a perform_request method. That's where we have the method, URL, and body, and call auth. There's no "request" object here.

We could make a similar interface to AuthBase for urllib3, but it cannot be the same interface because that one exists in the requests library. Since these signer classes aren't interchangeable at all, they don't need to derive from the same class, and I already moved the common parts into AWSV4Signer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense, thank you!

…uestHttpConnection.

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Copy link
Member Author

dblock commented Oct 23, 2023

@VachaShah I wrote some more tests and addressed your comments. LMK if there's anything else needed for this one

Copy link
Collaborator

@VachaShah VachaShah left a comment

Choose a reason for hiding this comment

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

LGTM!

@dblock dblock merged commit a1f942b into opensearch-project:main Oct 23, 2023
53 checks passed
@dblock dblock deleted the urllib3-aws-sig-v4 branch October 23, 2023 23:46
Djcarrillo6 added a commit to Djcarrillo6/opensearch-py that referenced this pull request Oct 25, 2023
Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Added a guide on making raw JSON REST requests. (opensearch-project#542)

Signed-off-by: dblock <dblock@amazon.com>

Added document lifecycle guide & sample code.

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Updated CHANGELOG

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Added support for AWS Sigv4 for UrlLib3. (opensearch-project#547)

* WIP: Added support for AWS Sigv4 for UrlLib3.

Signed-off-by: dblock <dblock@amazon.com>

* Refactored common implementation.

Signed-off-by: dblock <dblock@amazon.com>

* Added sigv4 samples.

Signed-off-by: dblock <dblock@amazon.com>

* Updated CHANGELOG.

Signed-off-by: dblock <dblock@amazon.com>

* Add documentation.

Signed-off-by: dblock <dblock@amazon.com>

* Use the correct class in tests.

Signed-off-by: dblock <dblock@amazon.com>

* Renamed samples.

Signed-off-by: dblock <dblock@amazon.com>

* Split up requests and urllib3 unit tests.

Signed-off-by: dblock <dblock@amazon.com>

* Rename AWSV4Signer.

Signed-off-by: dblock <dblock@amazon.com>

* Clarified documentation of when to use Urllib3AWSV4SignerAuth vs. RequestHttpConnection.

Signed-off-by: dblock <dblock@amazon.com>

* Move fetch_url inside the signer class.

Signed-off-by: dblock <dblock@amazon.com>

* Added unit test for Urllib3AWSV4SignerAuth adding headers.

Signed-off-by: dblock <dblock@amazon.com>

* Added unit test for signing to include query string.

Signed-off-by: dblock <dblock@amazon.com>

---------

Signed-off-by: dblock <dblock@amazon.com>

Remove support for Python 2.x. (opensearch-project#548)

Signed-off-by: dblock <dblock@amazon.com>

Fixed guide & added link in USER_GUIDE.md

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>

Added support for AWS Sigv4 for UrlLib3. (opensearch-project#547)

* WIP: Added support for AWS Sigv4 for UrlLib3.

Signed-off-by: dblock <dblock@amazon.com>

* Refactored common implementation.

Signed-off-by: dblock <dblock@amazon.com>

* Added sigv4 samples.

Signed-off-by: dblock <dblock@amazon.com>

* Updated CHANGELOG.

Signed-off-by: dblock <dblock@amazon.com>

* Add documentation.

Signed-off-by: dblock <dblock@amazon.com>

* Use the correct class in tests.

Signed-off-by: dblock <dblock@amazon.com>

* Renamed samples.

Signed-off-by: dblock <dblock@amazon.com>

* Split up requests and urllib3 unit tests.

Signed-off-by: dblock <dblock@amazon.com>

* Rename AWSV4Signer.

Signed-off-by: dblock <dblock@amazon.com>

* Clarified documentation of when to use Urllib3AWSV4SignerAuth vs. RequestHttpConnection.

Signed-off-by: dblock <dblock@amazon.com>

* Move fetch_url inside the signer class.

Signed-off-by: dblock <dblock@amazon.com>

* Added unit test for Urllib3AWSV4SignerAuth adding headers.

Signed-off-by: dblock <dblock@amazon.com>

* Added unit test for signing to include query string.

Signed-off-by: dblock <dblock@amazon.com>

---------

Signed-off-by: dblock <dblock@amazon.com>

Remove support for Python 2.x. (opensearch-project#548)

Signed-off-by: dblock <dblock@amazon.com>

Fixed guide & added link in USER_GUIDE.md opensearch-project#3

Signed-off-by: Djcarrillo6 <djcarrillo6@yahoo.com>
roma2023 pushed a commit to roma2023/opensearch-py that referenced this pull request Dec 28, 2023
* WIP: Added support for AWS Sigv4 for UrlLib3.

Signed-off-by: dblock <dblock@amazon.com>

* Refactored common implementation.

Signed-off-by: dblock <dblock@amazon.com>

* Added sigv4 samples.

Signed-off-by: dblock <dblock@amazon.com>

* Updated CHANGELOG.

Signed-off-by: dblock <dblock@amazon.com>

* Add documentation.

Signed-off-by: dblock <dblock@amazon.com>

* Use the correct class in tests.

Signed-off-by: dblock <dblock@amazon.com>

* Renamed samples.

Signed-off-by: dblock <dblock@amazon.com>

* Split up requests and urllib3 unit tests.

Signed-off-by: dblock <dblock@amazon.com>

* Rename AWSV4Signer.

Signed-off-by: dblock <dblock@amazon.com>

* Clarified documentation of when to use Urllib3AWSV4SignerAuth vs. RequestHttpConnection.

Signed-off-by: dblock <dblock@amazon.com>

* Move fetch_url inside the signer class.

Signed-off-by: dblock <dblock@amazon.com>

* Added unit test for Urllib3AWSV4SignerAuth adding headers.

Signed-off-by: dblock <dblock@amazon.com>

* Added unit test for signing to include query string.

Signed-off-by: dblock <dblock@amazon.com>

---------

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: roma2023 <romasaparhan19@gmail.com>
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

Successfully merging this pull request may close these issues.

[FEATURE] Add support for AWSV4SignerAuth for Urllib3HttpConnection
2 participants