Skip to content

Log a warning in case of missing sts dependency. #2169

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

Merged
merged 3 commits into from
Dec 2, 2020

Conversation

ascheja
Copy link
Contributor

@ascheja ascheja commented Nov 26, 2020

Description

Log a warning message if the sts dependecy is missing and either the necessary properties or the environment variables for web identity are present.

Motivation and Context

When using the a credentials provider chain with the intent to use web identity with a fallback to some other credential method for development the web identity credentials provider will be silently skipped if the sts dependency is missing. Also see #1915.

Testing


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • A short description of the change has been added to the CHANGELOG
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@ascheja
Copy link
Contributor Author

ascheja commented Nov 26, 2020

I'm currently having troubles running the unittests locally (even on master), I'll have to take a look at it on the weekend.

@dagnir
Copy link
Contributor

dagnir commented Dec 1, 2020

I'm currently having troubles running the unittests locally (even on master), I'll have to take a look at it on the weekend.

Should be fine, our CodeBuild job should suffice for this.

@codecov-io
Copy link

Codecov Report

Merging #2169 (807677c) into master (1d6a766) will increase coverage by 0.01%.
The diff coverage is 25.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2169      +/-   ##
============================================
+ Coverage     77.44%   77.45%   +0.01%     
  Complexity      328      328              
============================================
  Files          1222     1222              
  Lines         38301    38304       +3     
  Branches       3018     3018              
============================================
+ Hits          29661    29669       +8     
+ Misses         7184     7181       -3     
+ Partials       1456     1454       -2     
Flag Coverage Δ Complexity Δ
unittests 77.45% <25.00%> (+0.01%) 0.00 <0.00> (ø)

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

Impacted Files Coverage Δ Complexity Δ
...dentials/internal/WebIdentityCredentialsUtils.java 33.33% <25.00%> (ø) 0.00 <0.00> (ø)
...ssdk/core/internal/async/FileAsyncRequestBody.java 84.76% <0.00%> (+3.80%) 0.00% <0.00%> (ø%)
...nio/netty/internal/OldConnectionReaperHandler.java 90.90% <0.00%> (+9.09%) 0.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d6a766...807677c. Read the comment docs.

@dagnir dagnir merged commit 126f8a7 into aws:master Dec 2, 2020
@ascheja ascheja deleted the missing-sts-dependency-warning branch December 3, 2020 01:38
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.

3 participants