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.

Sign up for GitHub

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

Jump to bottom

fix: botocore sessions across samcli #1693

Merged
merged 1 commit into from Jan 3, 2020
Merged

fix: botocore sessions across samcli #1693

merged 1 commit into from Jan 3, 2020

Conversation

sriram-mv
Copy link
Contributor

@sriram-mv sriram-mv commented Dec 25, 2019 •
edited

fix: botocore sessions across samcli

Why is this change necessary?

  • This change is necessary to address MFA usecases when caching of credentials is essential. There have been multiple reports in github issues: 1682, 1623

How does it address the issue?

  • Use botocore's native json file cache manager.

What side effects does this change have?

  • Sessions are managed at the cli context level and not anywhere else.

TODO?

  • Need to check entire codebase for session management.

Depends on:

@sriram-mv
Copy link
Contributor Author

will rebase once #1681 is merged.

sanathkr
sanathkr reviewed Dec 26, 2019
View reviewed changes
designs/botocore_sessions.md
What is the problem?
--------------------

SAM CLI does not have first class support for MFA based authentication, which leads to multiple requests for such tokens degrading the overall CLI experience.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding a design doc!

@sriram-mv sriram-mv merged commit 2d71af8 into aws:develop Jan 3, 2020
sriram-mv added a commit to sriram-mv/aws-sam-cli that referenced this pull request Jan 7, 2020
@sriram-mv
Revert "fix: botocore session cache across samcli (aws#1693)"
b8adf29 
This reverts commit 2d71af8.
sriram-mv added a commit that referenced this pull request Jan 7, 2020
@sriram-mv
Revert "fix: botocore session cache across samcli (#1693)" (#1717)
d72e1ea 
This reverts commit 2d71af8.
sriram-mv
sriram-mv commented Jan 23, 2020
View reviewed changes
samcli/cli/context.py
@@ -144,6 +147,11 @@ def _refresh_session(self):
"""
try:
boto3.setup_default_session(region_name=self._aws_region, profile_name=self._aws_profile)
# get botocore session and setup caching for MFA based
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code changes

sriram-mv added a commit that referenced this pull request Jan 23, 2020
@sriram-mv
Revert "Revert "fix: botocore session cache across samcli (#1693)" (#…
2b9cf65 
…1717)"

This reverts commit d72e1ea.
sriram-mv added a commit to sriram-mv/aws-sam-cli that referenced this pull request Jan 23, 2020
@sriram-mv
Revert "Revert "fix: botocore session cache across samcli (aws#1693)" (…
d4b9bff 
…aws#1717)"

This reverts commit d72e1ea.
sriram-mv added a commit that referenced this pull request Jan 28, 2020
@sriram-mv @awood45
fix: cache MFA based credentials (#1744)
125f0b0 
* Revert "Revert "fix: botocore session cache across samcli (#1693)" (#1717)"

This reverts commit d72e1ea.

* fix: do not rely on boto private methods for session

* chore: run black autoformatter

* tests(context): assert botocore session

Co-authored-by: Alex Wood <awood45@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanathkr sanathkr sanathkr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sriram-mv @sanathkr