-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-3740: Add native support for AWS IAM Roles for service accounts, EKS in particular. #947
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
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why are we running
run-mongodb-aws-test.sh
twice, once withoutAWS_ROLE_SESSION_NAME
set and once with it set? If we are testing two different scenarios, we should probably separate this into two separate functions. Otherwise if one fails, it will not be immediately obvious which one. We would have to check whetherAWS_ROLE_SESSION_NAME
is set or not to understand which test case failed. 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.
these cases are very similar (but different) because if AWS_ROLE_SESSION_NAME is missed, it will be added automatically by sdk. Also the reason why we configure both cases is requirement in the spec here (supporting both cases where AWS_ROLE_SESSION_NAME is added and where not)
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.
both of these steps require the same precondition in the first step, so I would say it's not too wrong running these ones in the same scope.
given that it's almost the same scenarios, I would not expect failing them differently one from another. But even if so, EG provides a way to know what particular step was failed. For example pay attention on
(step 9.2..
hereThere 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.
Fair enough.