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.

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

Move JSONFileCache to botocore #1338

Merged
merged 3 commits into from
Dec 14, 2017
Merged

Conversation

JordonPhillips
Copy link
Contributor

This moves the file cache to botocore and allows users to set their
own cache via create_credential_resolver.

Future work: allow setting this via the config file and/or some other
easy method of configuration.

Closes #1157

@codecov-io
Copy link

codecov-io commented Dec 13, 2017

Codecov Report

Merging #1338 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1338      +/-   ##
===========================================
+ Coverage    98.13%   98.13%   +<.01%     
===========================================
  Files           46       46              
  Lines         7701     7732      +31     
===========================================
+ Hits          7557     7588      +31     
  Misses         144      144
Impacted Files Coverage Δ
botocore/credentials.py 98.39% <100%> (+0.06%) ⬆️

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 f1de3d7...b915ca3. Read the comment docs.

Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks fine. I just had a question about the default location of the cache.

values can be retrieved at a later time.
"""

CACHE_DIR = os.path.expanduser(os.path.join('~', '.aws', 'cli', 'cache'))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should consider using a different directory than what the CLI uses. I have not thought through whether there will be edge cases to this, but it may be surprising if you are running a script to learn that your temp credentials were being cached under a cli directory. I wonder if it makes sense to call it boto or python instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds reasonable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@kyleknap kyleknap left a 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. Don't forget to add an enhancement changelog. 🚢

@JordonPhillips
Copy link
Contributor Author

Will do. Also gonna put up a PR for the cli to switch to using this.

@et304383
Copy link

Can we get a quick explanation of how to use this? Is it possible to cache the MFA globally yet?

@mixja
Copy link

mixja commented Feb 26, 2018

@et304383 - here is an example of leveraging the AWS CLI cache. Note you could create your own custom cache location if you wanted.

from botocore import credentials
import botocore.session
import boto3
import os

# By default the cache path is ~/.aws/boto/cache
cli_cache = os.path.join(os.path.expanduser('~'),'.aws/cli/cache')

# Construct botocore session with cache
session = botocore.session.get_session()
session.get_component('credential_provider').get_provider('assume-role').cache = credentials.JSONFileCache(cli_cache)

# Create boto3 client from session
client = boto3.Session(botocore_session=session).client('ecs')

@bgabrhelik
Copy link

If you need to specify a profile you have to specify it in botocore.Session. It's too late to specify it in boto3.Session as AssumeRoleProvider instantiates with wrong 'default' profile.
So the code above will be

from botocore import credentials
import botocore.session
import boto3
import os

# By default the cache path is ~/.aws/boto/cache
cli_cache = os.path.join(os.path.expanduser('~'),'.aws/cli/cache')

# Construct botocore session with cache
session = botocore.session.Session(profile='prod')
session.get_component('credential_provider').get_provider('assume-role').cache = credentials.JSONFileCache(cli_cache)

# Create boto3 client from session
client = boto3.Session(botocore_session=session, profile_name = 'prod').client('ecs')

@mixja
Copy link

mixja commented Feb 19, 2019

@bgabrhelik - it's generally better practice to rely on the external environment to configure your credentials, rather than specify an AWS profile name in code. This approach has benefit of allowing the code to work in EC2/ECS environments where you may be using EC2 instance profile or ECS task role.

My code snippet works in a local desktop environment assuming you have set profile name as environment variable - e.g. export AWS_PROFILE=prod

adamnovak added a commit to DataBiosphere/toil that referenced this pull request Sep 19, 2019
See boto/botocore#1157 and also
boto/botocore#1338 which actually merged.

There's no global way to say that we want Boto3 to cache credentials;
we need to do a dance for every session.

TODO: Should we use the same credentials cache as awscli? Right
now this uses the default boto3 cache location which is different.

TODO: Should we remove the custom caching at the boto2 monkey
patch level?
@mikehas
Copy link

mikehas commented Mar 22, 2024

Does anyone know if there's a feature request to implement this as a configuration option, like @JordonPhillips mentioned in initial description?

Future work: allow setting this via the config file and/or some other
easy method of configuration.

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.

8 participants