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

Replace requests-auth-aws-sigv4 with boto3 signing #1077

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

AndreKurait
Copy link
Member

@AndreKurait AndreKurait commented Oct 16, 2024

Description

Replace requests-auth-aws-sigv4 with boto3 signing

  • Category: Bug fix
  • Why these changes are required? better security with sigv4 by using trusted library and bug fixes
  • What is the old behavior before changes and new behavior after changes? Some calls would fail that included ?v or * in url

Issues Resolved

MIGRATIONS-2099

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Added unit tests

Tested locally against an aws cluster with sigv4

(.venv) bash-5.2# console clusters clear-indices --cluster target --acknowledge-risk
Performing clear indices operation...
(.venv) bash-5.2# console clusters curl -XGET target_cluster /_cat/indices?v
health status index                     uuid                   pri rep docs.count docs.deleted store.size pri.store.size
green  open   .opensearch-observability pfTL2UhYS_WCRVfqJTx6Bg   1   2          0            0       624b           208b
green  open   .plugins-ml-config        4ox10VJMTFSXHTBERLp-Lg   1   1          1            0      7.8kb          3.9kb
green  open   .ql-datasources           iQ014_ShS4eYj9e-RSMZpg   1   2          0            0       624b           208b
green  open   .kibana_1                 OqJWIofcRUeXYvf8YDS0OA   1   2          1            0     15.6kb          5.2kb

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.21%. Comparing base (d0a0b6f) to head (97b6525).
Report is 19 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1077      +/-   ##
============================================
+ Coverage     80.19%   80.21%   +0.02%     
- Complexity     2863     2866       +3     
============================================
  Files           383      383              
  Lines         14333    14358      +25     
  Branches        988      988              
============================================
+ Hits          11494    11517      +23     
- Misses         2245     2247       +2     
  Partials        594      594              
Flag Coverage Δ
gradle-test 78.20% <ø> (-0.01%) ⬇️
python-test 90.33% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AndreKurait AndreKurait marked this pull request as ready for review October 16, 2024 23:06
@AndreKurait AndreKurait force-pushed the boto3Sigv4 branch 2 times, most recently from a7a3586 to 590f8a2 Compare October 16, 2024 23:09
Signed-off-by: Andre Kurait <akurait@amazon.com>
Signed-off-by: Andre Kurait <akurait@amazon.com>
Copy link
Collaborator

@mikaylathompson mikaylathompson left a comment

Choose a reason for hiding this comment

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

That is a fantastic test. Thanks for making it so comprehensive. I'm impressed how surgical this change is overall.

# requests to AWS services using SigV4.


class SigV4AuthPlugin(requests.auth.AuthBase):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This integrates it so elegantly! I like it!

@AndreKurait AndreKurait merged commit b6683c8 into opensearch-project:main Oct 18, 2024
14 checks passed
@AndreKurait AndreKurait deleted the boto3Sigv4 branch October 18, 2024 18:09
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.

2 participants