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

Creation of regional clients modifies default botocore session's region #190

Closed
polamayster opened this issue Sep 17, 2019 · 3 comments
Closed
Labels

Comments

@polamayster
Copy link
Contributor

polamayster commented Sep 17, 2019

Let's say we have app runing in ECS(lots of gunicorn workers) using IAM role and want to reuse botocore session for all boto3 clients to not query for credentials each time and to not hit throttling limits with botocore.credentials.ContainerProvider:

import botocore.session
import boto3

botocore_session = botocore.session.get_session()
botocore_session._credentials = RefreshableCredentials(...)
boto3.setup_default_session(botocore_session=botocore_session)

Code at https://github.com/aws/aws-encryption-sdk-python/blob/master/src/aws_encryption_sdk/key_providers/kms.py#L164

if region_name not in self._regional_clients:
    session = boto3.session.Session(region_name=region_name, botocore_session=self.config.botocore_session)
    client = session.client("kms", config=self._user_agent_adding_config)
    self._register_client(client, region_name)
    self._regional_clients[region_name] = client

will end up doing this inside boto3.session:

if region_name is not None:
    self._session.set_config_variable('region', region_name)

basically modifying botocore session for each unique region in key_ids supplied to KMSMasterKeyProvider for example:

kms_key_provider = aws_encryption_sdk.KMSMasterKeyProvider(
    key_ids=[
         'arn:aws:kms:eu-west-1:11111111111111:key/111111111-1111-1111-1111-1111111111111',
         'arn:aws:kms:us-east-1:2222222222222:key/22222222-2222-2222-2222-222222222222',
         'arn:aws:kms:ap-northeast-1:3333333333333:key/33333333-3333-3333-3333-333333333333'
     ],
     botocore_session=botocore_session
)

it will be setting region variable for botocore_session multiple times and last one ap-northeast-1 will become new region of default session, so any new boto3 client without explicitly set region_name will be created for this region e.g.

sns = boto3.client('sns')
vars(sns)

{
    '_serializer': <botocore.validate.ParamValidationDecorator at 0x7fecb6516978>,
    '_endpoint': sns(https://sns.ap-northeast-1.amazonaws.com),
    ....
}

which is not desired behavior and of course could be fixed by setting region_name explicitly (everywhere) but could be fixed more easily by:

if region_name not in self._regional_clients:
   session = boto3.session.Session(botocore_session=self.config.botocore_session)
   client = session.client("kms", region_name=region_name, config=self._user_agent_adding_config)
    self._register_client(client, region_name)
    self._regional_clients[region_name] = client

instead without modifying default botocore session and achieving same result.

Please let me know if I miss the reason for actually setting region_name for a session as it is right now or use KMSMasterKeyProvider incorrectly otherwise please consider adding this change to KMSMasterKeyProvider.

@mattsb42-aws
Copy link
Member

Thanks for finding this, @polamayster. That is definitely not the behavior we intended.

I think that the right solution for us is to do what you suggested and move the region specification to the client rather than the boto3 session.

This behavior is sufficiently magic that it makes me wonder if this was the intended behavior for the boto3 session, though, so I'm also going to open an issue with that project making sure that this is intentional.

@polamayster
Copy link
Contributor Author

Hi @mattsb42-aws,
Thanks for quick reply. Should I open PR with a change?

@mattsb42-aws
Copy link
Member

Should I open PR with a change?

That would be awesome, yes!

Please include a test that checks for this issue that you found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants