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

Refactor AWSSigV4 auth to support different AWSCredentialProviders #1389

Merged
merged 2 commits into from
Mar 3, 2023

Conversation

vmmusings
Copy link
Member

@vmmusings vmmusings commented Mar 3, 2023

Description

Refactored sigv4auth support to use credential Providers.

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • 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.

@vmmusings vmmusings requested a review from a team as a code owner March 3, 2023 01:17
anirudha
anirudha previously approved these changes Mar 3, 2023
kavithacm
kavithacm previously approved these changes Mar 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2023

Codecov Report

Merging #1389 (56b32fa) into 2.x (a32bcbe) will increase coverage by 0.00%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##                2.x    #1389   +/-   ##
=========================================
  Coverage     98.38%   98.38%           
- Complexity     3698     3699    +1     
=========================================
  Files           345      345           
  Lines          9113     9119    +6     
  Branches        585      586    +1     
=========================================
+ Hits           8966     8972    +6     
  Misses          142      142           
  Partials          5        5           
Flag Coverage Δ
sql-engine 98.38% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...etheus/authinterceptors/AwsSigningInterceptor.java 100.00% <100.00%> (ø)
...l/prometheus/storage/PrometheusStorageFactory.java 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment on lines 67 to 69
LOG.info(awsCredentialsProvider.getClass());
LOG.info("Access Key: {}", awsCredentials.getAWSAccessKeyId());
LOG.info("Secret Key: {}", awsCredentials.getAWSSecretKey());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure that you want to log your passwords as plain text?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing out...I was splitting a big PR and left these lines.

@vmmusings vmmusings dismissed stale reviews from kavithacm and anirudha via a34ceec March 3, 2023 02:25
@vmmusings vmmusings force-pushed the refactor-sigv4-auth branch 2 times, most recently from a34ceec to a8dcece Compare March 3, 2023 02:28
@vmmusings vmmusings force-pushed the refactor-sigv4-auth branch from a8dcece to 93a82a8 Compare March 3, 2023 02:35
@vmmusings vmmusings self-assigned this Mar 3, 2023
@vmmusings vmmusings added backport main maintenance Improves code quality, but not the product labels Mar 3, 2023
@vmmusings vmmusings merged commit 8583fd1 into opensearch-project:2.x Mar 3, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 3, 2023
…1389)

* Refactor AWSSigV4 auth to support different AWSCredentialProviders

Signed-off-by: Vamsi Manohar <[email protected]>

* Added unit tests for sts assume role credentials provider

Signed-off-by: Vamsi Manohar <[email protected]>

---------

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 8583fd1)
vmmusings added a commit that referenced this pull request Mar 7, 2023
…1389)

* Refactor AWSSigV4 auth to support different AWSCredentialProviders

Signed-off-by: Vamsi Manohar <[email protected]>

* Added unit tests for sts assume role credentials provider

Signed-off-by: Vamsi Manohar <[email protected]>

---------

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 8583fd1)
vmmusings added a commit that referenced this pull request Mar 7, 2023
…1389) (#1396)

* Refactor AWSSigV4 auth to support different AWSCredentialProviders

Signed-off-by: Vamsi Manohar <[email protected]>

* Added unit tests for sts assume role credentials provider

Signed-off-by: Vamsi Manohar <[email protected]>

---------

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 8583fd1)

Co-authored-by: vamsi-amazon <[email protected]>
matthewryanwells pushed a commit to Bit-Quill/opensearch-project-sql that referenced this pull request Mar 10, 2023
…pensearch-project#1389) (opensearch-project#1396)

* Refactor AWSSigV4 auth to support different AWSCredentialProviders

Signed-off-by: Vamsi Manohar <[email protected]>

* Added unit tests for sts assume role credentials provider

Signed-off-by: Vamsi Manohar <[email protected]>

---------

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 8583fd1)

Co-authored-by: vamsi-amazon <[email protected]>
Signed-off-by: Matthew Wells <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport main maintenance Improves code quality, but not the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants