-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Update Assume Role docs and tests #2956
Conversation
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. Just had some small suggestions about wordings.
awscli/topics/config-vars.rst
Outdated
* ``source_profile`` - The AWS CLI profile that contains credentials we should | ||
use for the initial ``assume-role`` call. | ||
* ``source_profile`` - The AWS CLI profile that contains credentials / | ||
configuration we should use for the initial ``assume-role`` call. This |
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.
Probably want to remove the use of "we" to make it more third person or instructive.
awscli/topics/config-vars.rst
Outdated
* ``source_profile`` - The AWS CLI profile that contains credentials / | ||
configuration we should use for the initial ``assume-role`` call. This | ||
profile may be another profile configured to use ``assume-role``, though | ||
if static credentials are present in the profile they will be preferred. |
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.
Maybe rephrase this from "preferred" to something a long the lines of taking precedence over role_arn
/credential_source
awscli/topics/config-vars.rst
Outdated
the initial ``assume-role`` call. This parameter cannot be provided | ||
alongside ``source_profile``. Valid values are: | ||
* ``Environment`` to pull source credentials from environment variables. | ||
* ``Ec2InstanceMetadta`` to use the EC2 instance role as source credentials. |
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.
Ec2InstanceMetadta -> Ec2InstanceMetadata
[profile crossaccount] | ||
role_arn=arn:aws:iam:... | ||
credential_source=Ec2InstanceMetadata | ||
|
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.
I wonder if it makes sense to explain what this example does exactly in obtaining credentials?
Codecov Report
@@ Coverage Diff @@
## develop #2956 +/- ##
========================================
Coverage 95.69% 95.69%
========================================
Files 159 159
Lines 12031 12031
========================================
Hits 11513 11513
Misses 518 518 Continue to review full report at Codecov.
|
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 to me
072fa4f
to
ea5bf09
Compare
Cool. I squashed that extra doc commit. This will be merged alongside the relevant botocore pr. |
52270dc
to
5497031
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.
Great! 🚢
* Enabled ability to provide tags as list in samconfig.toml file * Removed trailing white spaces and reformatted code * Added integration test for tags as list deploy command * Added integration test for tags as string from samconfig.toml * Fixed Appveyer error by removing s3 info Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
This updates the assume role documentation and adds tests in response
to the assume role changes in boto/botocore#1313
It additionally adds changelog entries that will result in a minor
version bump.