Skip to content

Commit

Permalink
fix(layers): Move boto3 client creation to a computed property (#1059)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfuss committed May 23, 2019
1 parent e23c7cf commit 287781c
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 28 deletions.
7 changes: 6 additions & 1 deletion samcli/commands/local/cli_common/invoke_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ def __init__(self, # pylint: disable=R0914
parameter_overrides=None,
layer_cache_basedir=None,
force_image_build=None,
aws_region=None):
aws_region=None,
aws_profile=None,
):
"""
Initialize the context
Expand Down Expand Up @@ -109,6 +111,7 @@ def __init__(self, # pylint: disable=R0914
self._layer_cache_basedir = layer_cache_basedir
self._force_image_build = force_image_build
self._aws_region = aws_region
self._aws_profile = aws_profile

self._template_dict = None
self._function_provider = None
Expand Down Expand Up @@ -197,6 +200,8 @@ def local_lambda_runner(self):
return LocalLambdaRunner(local_runtime=lambda_runtime,
function_provider=self._function_provider,
cwd=self.get_cwd(),
aws_profile=self._aws_profile,
aws_region=self._aws_region,
env_vars_values=self._env_vars_value,
debug_context=self._debug_context)

Expand Down
3 changes: 2 additions & 1 deletion samcli/commands/local/invoke/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ def do_cli(ctx, function_identifier, template, event, no_event, env_vars, debug_
parameter_overrides=parameter_overrides,
layer_cache_basedir=layer_cache_basedir,
force_image_build=force_image_build,
aws_region=ctx.region) as context:
aws_region=ctx.region,
aws_profile=ctx.profile) as context:

# Invoke the function
context.local_lambda_runner.invoke(context.function_name,
Expand Down
15 changes: 8 additions & 7 deletions samcli/commands/local/lib/local_lambda.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ def __init__(self,
local_runtime,
function_provider,
cwd,
aws_profile=None,
aws_region=None,
env_vars_values=None,
debug_context=None):
"""
Expand All @@ -43,6 +45,8 @@ def __init__(self,
self.local_runtime = local_runtime
self.provider = function_provider
self.cwd = cwd
self.aws_profile = aws_profile
self.aws_region = aws_region
self.env_vars_values = env_vars_values or {}
self.debug_context = debug_context

Expand Down Expand Up @@ -203,13 +207,10 @@ def get_aws_creds(self):
result = {}

# to pass command line arguments for region & profile to setup boto3 default session
if boto3.DEFAULT_SESSION:
session = boto3.DEFAULT_SESSION
else:
session = boto3.session.Session()

profile_name = session.profile_name if session else None
LOG.debug("Loading AWS credentials from session with profile '%s'", profile_name)
LOG.debug("Loading AWS credentials from session with profile '%s'", self.aws_profile)
# boto3.session.Session is not thread safe. To ensure we do not run into a race condition with start-lambda
# or start-api, we create the session object here on every invoke.
session = boto3.session.Session(profile_name=self.aws_profile, region_name=self.aws_region)

if not session:
return result
Expand Down
3 changes: 2 additions & 1 deletion samcli/commands/local/start_api/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ def do_cli(ctx, host, port, static_dir, template, env_vars, debug_port, debug_ar
parameter_overrides=parameter_overrides,
layer_cache_basedir=layer_cache_basedir,
force_image_build=force_image_build,
aws_region=ctx.region) as invoke_context:
aws_region=ctx.region,
aws_profile=ctx.profile) as invoke_context:

service = LocalApiService(lambda_invoke_context=invoke_context,
port=port,
Expand Down
3 changes: 2 additions & 1 deletion samcli/commands/local/start_lambda/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ def do_cli(ctx, host, port, template, env_vars, debug_port, debug_args, # pylin
parameter_overrides=parameter_overrides,
layer_cache_basedir=layer_cache_basedir,
force_image_build=force_image_build,
aws_region=ctx.region) as invoke_context:
aws_region=ctx.region,
aws_profile=ctx.profile) as invoke_context:

service = LocalLambdaService(lambda_invoke_context=invoke_context,
port=port,
Expand Down
7 changes: 6 additions & 1 deletion samcli/local/layers/layer_downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ def __init__(self, layer_cache, cwd, lambda_client=None):
"""
self._layer_cache = layer_cache
self.cwd = cwd
self.lambda_client = lambda_client or boto3.client('lambda')
self._lambda_client = lambda_client

@property
def lambda_client(self):
self._lambda_client = self._lambda_client or boto3.client('lambda')
return self._lambda_client

@property
def layer_cache(self):
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/local/start_lambda/test_start_lambda.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def setUp(self):
self.url = "http://127.0.0.1:{}".format(self.port)
self.lambda_client = boto3.client('lambda',
endpoint_url=self.url,
region_name='us-east-1',
use_ssl=False,
verify=False,
config=Config(signature_version=UNSIGNED,
Expand Down Expand Up @@ -52,6 +53,7 @@ def setUp(self):
self.url = "http://127.0.0.1:{}".format(self.port)
self.lambda_client = boto3.client('lambda',
endpoint_url=self.url,
region_name='us-east-1',
use_ssl=False,
verify=False,
config=Config(signature_version=UNSIGNED,
Expand All @@ -69,6 +71,7 @@ def setUp(self):
self.url = "http://127.0.0.1:{}".format(self.port)
self.lambda_client = boto3.client('lambda',
endpoint_url=self.url,
region_name='us-east-1',
use_ssl=False,
verify=False,
config=Config(signature_version=UNSIGNED,
Expand Down Expand Up @@ -110,6 +113,7 @@ def setUp(self):
self.url = "http://127.0.0.1:{}".format(self.port)
self.lambda_client = boto3.client('lambda',
endpoint_url=self.url,
region_name='us-east-1',
use_ssl=False,
verify=False,
config=Config(signature_version=UNSIGNED,
Expand Down
14 changes: 10 additions & 4 deletions tests/unit/commands/local/cli_common/test_invoke_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ def test_must_read_from_necessary_files(self, SamFunctionProviderMock):
debugger_path="path-to-debugger",
debug_args='args',
parameter_overrides={},
aws_region="region")
aws_region="region",
aws_profile="profile")

template_dict = "template_dict"
invoke_context._get_template_data = Mock()
Expand Down Expand Up @@ -159,7 +160,8 @@ def test_must_work_in_with_statement(self, ExitMock, EnterMock):
skip_pull_image=True,
debug_port=1111,
debugger_path="path-to-debugger",
debug_args='args') as context:
debug_args='args',
aws_profile="profile") as context:
self.assertEquals(context_obj, context)

EnterMock.assert_called_with()
Expand Down Expand Up @@ -207,7 +209,9 @@ def setUp(self):
force_image_build=True,
debug_port=1111,
debugger_path="path-to-debugger",
debug_args='args')
debug_args='args',
aws_profile="profile",
aws_region="region")

@patch("samcli.commands.local.cli_common.invoke_context.LambdaImage")
@patch("samcli.commands.local.cli_common.invoke_context.LayerDownloader")
Expand Down Expand Up @@ -256,7 +260,9 @@ def test_must_create_runner(self,
function_provider=ANY,
cwd=cwd,
debug_context=None,
env_vars_values=ANY)
env_vars_values=ANY,
aws_profile="profile",
aws_region="region")


class TestInvokeContext_stdout_property(TestCase):
Expand Down
13 changes: 11 additions & 2 deletions tests/unit/commands/local/invoke/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def setUp(self):
self.layer_cache_basedir = "/some/layers/path"
self.force_image_build = True
self.region_name = "region"
self.profile = "profile"

@patch("samcli.commands.local.invoke.cli.InvokeContext")
@patch("samcli.commands.local.invoke.cli._get_event")
Expand All @@ -47,6 +48,7 @@ def test_cli_must_setup_context_and_invoke(self, get_event_mock, InvokeContextMo

ctx_mock = Mock()
ctx_mock.region = self.region_name
ctx_mock.profile = self.profile

# Mock the __enter__ method to return a object inside a context manager
context_mock = Mock()
Expand Down Expand Up @@ -82,7 +84,8 @@ def test_cli_must_setup_context_and_invoke(self, get_event_mock, InvokeContextMo
parameter_overrides=self.parameter_overrides,
layer_cache_basedir=self.layer_cache_basedir,
force_image_build=self.force_image_build,
aws_region=self.region_name)
aws_region=self.region_name,
aws_profile=self.profile)

context_mock.local_lambda_runner.invoke.assert_called_with(context_mock.function_name,
event=event_data,
Expand All @@ -97,6 +100,7 @@ def test_cli_must_invoke_with_no_event(self, get_event_mock, InvokeContextMock):

ctx_mock = Mock()
ctx_mock.region = self.region_name
ctx_mock.profile = self.profile

# Mock the __enter__ method to return a object inside a context manager
context_mock = Mock()
Expand Down Expand Up @@ -131,7 +135,8 @@ def test_cli_must_invoke_with_no_event(self, get_event_mock, InvokeContextMock):
parameter_overrides=self.parameter_overrides,
layer_cache_basedir=self.layer_cache_basedir,
force_image_build=self.force_image_build,
aws_region=self.region_name)
aws_region=self.region_name,
aws_profile=self.profile)

context_mock.local_lambda_runner.invoke.assert_called_with(context_mock.function_name,
event="{}",
Expand All @@ -146,6 +151,7 @@ def test_must_raise_user_exception_on_no_event_and_event(self, get_event_mock, I

ctx_mock = Mock()
ctx_mock.region = self.region_name
ctx_mock.profile = self.profile

with self.assertRaises(UserException) as ex_ctx:

Expand Down Expand Up @@ -185,6 +191,7 @@ def test_must_raise_user_exception_on_function_not_found(self,

ctx_mock = Mock()
ctx_mock.region = self.region_name
ctx_mock.profile = self.profile

# Mock the __enter__ method to return a object inside a context manager
context_mock = Mock()
Expand Down Expand Up @@ -231,6 +238,7 @@ def test_must_raise_user_exception_on_invalid_sam_template(self,

ctx_mock = Mock()
ctx_mock.region = self.region_name
ctx_mock.profile = self.profile

InvokeContextMock.side_effect = exeception_to_raise

Expand Down Expand Up @@ -264,6 +272,7 @@ def test_must_raise_user_exception_on_invalid_env_vars(self, get_event_mock, Inv

ctx_mock = Mock()
ctx_mock.region = self.region_name
ctx_mock.profile = self.profile

InvokeContextMock.side_effect = OverridesNotWellDefinedError("bad env vars")

Expand Down
17 changes: 9 additions & 8 deletions tests/unit/commands/local/lib/test_local_lambda.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ def setUp(self):
self.function_provider_mock,
self.cwd,
env_vars_values=self.env_vars_values,
debug_context=self.debug_context)
debug_context=self.debug_context,
aws_profile=self.aws_profile,
aws_region=self.aws_region)

@patch("samcli.commands.local.lib.local_lambda.boto3")
def test_must_get_from_boto_session(self, boto3_mock):
Expand All @@ -43,7 +45,7 @@ def test_must_get_from_boto_session(self, boto3_mock):
mock_session = Mock()
mock_session.region_name = self.region

boto3_mock.DEFAULT_SESSION = mock_session
boto3_mock.session.Session.return_value = mock_session
mock_session.get_credentials.return_value = creds

expected = {
Expand All @@ -56,6 +58,8 @@ def test_must_get_from_boto_session(self, boto3_mock):
actual = self.local_lambda.get_aws_creds()
self.assertEquals(expected, actual)

boto3_mock.session.Session.assert_called_with(profile_name=self.aws_profile, region_name=self.aws_region)

@patch("samcli.commands.local.lib.local_lambda.boto3")
def test_must_work_with_no_region_name(self, boto3_mock):
creds = Mock()
Expand All @@ -66,7 +70,6 @@ def test_must_work_with_no_region_name(self, boto3_mock):
mock_session = Mock()
del mock_session.region_name # Ask mock to return AttributeError when 'region_name' is accessed

boto3_mock.DEFAULT_SESSION = None
boto3_mock.session.Session.return_value = mock_session
mock_session.get_credentials.return_value = creds

Expand All @@ -79,7 +82,7 @@ def test_must_work_with_no_region_name(self, boto3_mock):
actual = self.local_lambda.get_aws_creds()
self.assertEquals(expected, actual)

boto3_mock.session.Session.assert_called_with()
boto3_mock.session.Session.assert_called_with(profile_name=self.aws_profile, region_name=self.aws_region)

@patch("samcli.commands.local.lib.local_lambda.boto3")
def test_must_work_with_no_access_key(self, boto3_mock):
Expand All @@ -91,7 +94,6 @@ def test_must_work_with_no_access_key(self, boto3_mock):
mock_session = Mock()
mock_session.region_name = self.region

boto3_mock.DEFAULT_SESSION = None
boto3_mock.session.Session.return_value = mock_session
mock_session.get_credentials.return_value = creds

Expand All @@ -104,7 +106,7 @@ def test_must_work_with_no_access_key(self, boto3_mock):
actual = self.local_lambda.get_aws_creds()
self.assertEquals(expected, actual)

boto3_mock.session.Session.assert_called_with()
boto3_mock.session.Session.assert_called_with(profile_name=self.aws_profile, region_name=self.aws_region)

@patch("samcli.commands.local.lib.local_lambda.boto3")
def test_must_work_with_no_secret_key(self, boto3_mock):
Expand All @@ -116,7 +118,6 @@ def test_must_work_with_no_secret_key(self, boto3_mock):
mock_session = Mock()
mock_session.region_name = self.region

boto3_mock.DEFAULT_SESSION = None
boto3_mock.session.Session.return_value = mock_session
mock_session.get_credentials.return_value = creds

Expand All @@ -129,7 +130,7 @@ def test_must_work_with_no_secret_key(self, boto3_mock):
actual = self.local_lambda.get_aws_creds()
self.assertEquals(expected, actual)

boto3_mock.session.Session.assert_called_with()
boto3_mock.session.Session.assert_called_with(profile_name=self.aws_profile, region_name=self.aws_region)

@patch("samcli.commands.local.lib.local_lambda.boto3")
def test_must_work_with_no_session_token(self, boto3_mock):
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/commands/local/start_api/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ def setUp(self):
self.layer_cache_basedir = "/some/layers/path"
self.force_image_build = True
self.region_name = "region"
self.profile = "profile"

self.ctx_mock = Mock()
self.ctx_mock.region = self.region_name
self.ctx_mock.profile = self.profile

self.host = "host"
self.port = 123
Expand Down Expand Up @@ -65,7 +67,8 @@ def test_cli_must_setup_context_and_start_service(self, local_api_service_mock,
parameter_overrides=self.parameter_overrides,
layer_cache_basedir=self.layer_cache_basedir,
force_image_build=self.force_image_build,
aws_region=self.region_name)
aws_region=self.region_name,
aws_profile=self.profile)

local_api_service_mock.assert_called_with(lambda_invoke_context=context_mock,
port=self.port,
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/commands/local/start_lambda/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ def setUp(self):
self.layer_cache_basedir = "/some/layers/path"
self.force_image_build = True
self.region_name = "region"
self.profile = "profile"

self.ctx_mock = Mock()
self.ctx_mock.region = self.region_name
self.ctx_mock.profile = self.profile

self.host = "host"
self.port = 123
Expand Down Expand Up @@ -60,7 +62,8 @@ def test_cli_must_setup_context_and_start_service(self, local_lambda_service_moc
parameter_overrides=self.parameter_overrides,
layer_cache_basedir=self.layer_cache_basedir,
force_image_build=self.force_image_build,
aws_region=self.region_name)
aws_region=self.region_name,
aws_profile=self.profile)

local_lambda_service_mock.assert_called_with(lambda_invoke_context=context_mock,
port=self.port,
Expand Down

0 comments on commit 287781c

Please sign in to comment.