-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add support for sso-session credentials #2812
Conversation
fdada5c
to
673d59f
Compare
Codecov ReportBase: 93.45% // Head: 93.45% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #2812 +/- ##
========================================
Coverage 93.45% 93.45%
========================================
Files 63 63
Lines 13298 13337 +39
========================================
+ Hits 12427 12464 +37
- Misses 871 873 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
79fb260
to
020cd4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine. Just had comments on the tests.
with open(self.config_file, 'w') as f: | ||
f.write(config) | ||
|
||
def test_token_chosen_from_provider(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great if we added test cases for some of the error cases such as:
- Mismatching start url and sso region
- SSO session not being defined
I'm also wondering if we should add a case for having an expired SSO token in the cache and needing the token provider to refresh first. However, I'm not sure how straightforward that will be to add. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first two are super reasonable, I attempted the last one earlier but the MockCache gets very complicated to do this without the ability to change inputs.
020cd4c
to
694165a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 🚢
The updates allow using sso-session configuration as part of the SSO credential provider. This was ported from this PR: boto/botocore#2812
The updates allow using sso-session configuration as part of the SSO credential provider. This was ported from this PR: boto/botocore#2812
This PR will add support for loading credentials via a configured
sso-session
in your AWS config file.