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

Updates the S3 sink to use the AWS Plugin for loading AWS credentials #2787

Merged

Conversation

dlvenable
Copy link
Member

Description

Updates the s3 sink to use the AWS Plugin. This now provides the AWS credentials for this sink. This includes some related refactoring including fixing a bug where the S3 client was created anew for every object write.

Issues Resolved

Resolves #2767

Check List

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

…meterize the regions to ensure they are not variable.

Signed-off-by: David Venable <dlv@amazon.com>
@dlvenable dlvenable mentioned this pull request May 31, 2023
4 tasks
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #2787 (dcbc251) into main (269baea) will decrease coverage by 0.18%.
The diff coverage is n/a.

❗ Current head dcbc251 differs from pull request most recent head f34627a. Consider uploading reports for the commit f34627a to get more accurate results

@@             Coverage Diff              @@
##               main    #2787      +/-   ##
============================================
- Coverage     94.12%   93.94%   -0.18%     
+ Complexity     2441     2437       -4     
============================================
  Files           278      278              
  Lines          6744     6759      +15     
  Branches        551      551              
============================================
+ Hits           6348     6350       +2     
- Misses          256      269      +13     
  Partials        140      140              

see 3 files with indirect coverage changes

Comment on lines +31 to +36
public String getAwsStsRoleArn() {
return awsStsRoleArn;
}

return awsCredentialsProvider;
public Map<String, String> getAwsStsHeaderOverrides() {
return awsStsHeaderOverrides;
Copy link
Contributor

Choose a reason for hiding this comment

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

There are no unit tests in AwsAuthenticationOptionsTest.java file for these new methods

Copy link
Member Author

Choose a reason for hiding this comment

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

@cmanning09 , I just pushed a change which adds testing for these getters.

…approach to deserialize using Jackson to better simulate the final usage.

Signed-off-by: David Venable <dlv@amazon.com>
@dlvenable dlvenable merged commit 40e60fb into opensearch-project:main Jun 2, 2023
24 checks passed
@dlvenable dlvenable mentioned this pull request Jun 5, 2023
@dlvenable dlvenable deleted the 2767-s3-sink-aws-plugin branch July 13, 2023 21:44
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.

Update the S3 sink to use AWS Plugin for credentials
3 participants