From 287781ca9aa0f5ce9f8cd3036645e22cf0d0a1fd Mon Sep 17 00:00:00 2001 From: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Date: Thu, 23 May 2019 08:59:36 -0700 Subject: [PATCH] fix(layers): Move boto3 client creation to a computed property (#1059) --- .../commands/local/cli_common/invoke_context.py | 7 ++++++- samcli/commands/local/invoke/cli.py | 3 ++- samcli/commands/local/lib/local_lambda.py | 15 ++++++++------- samcli/commands/local/start_api/cli.py | 3 ++- samcli/commands/local/start_lambda/cli.py | 3 ++- samcli/local/layers/layer_downloader.py | 7 ++++++- .../local/start_lambda/test_start_lambda.py | 4 ++++ .../local/cli_common/test_invoke_context.py | 14 ++++++++++---- tests/unit/commands/local/invoke/test_cli.py | 13 +++++++++++-- .../commands/local/lib/test_local_lambda.py | 17 +++++++++-------- tests/unit/commands/local/start_api/test_cli.py | 5 ++++- .../commands/local/start_lambda/test_cli.py | 5 ++++- 12 files changed, 68 insertions(+), 28 deletions(-) diff --git a/samcli/commands/local/cli_common/invoke_context.py b/samcli/commands/local/cli_common/invoke_context.py index 9e6bc99371..342f9df179 100644 --- a/samcli/commands/local/cli_common/invoke_context.py +++ b/samcli/commands/local/cli_common/invoke_context.py @@ -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 @@ -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 @@ -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) diff --git a/samcli/commands/local/invoke/cli.py b/samcli/commands/local/invoke/cli.py index 50c96e4d2a..d8c875eee4 100644 --- a/samcli/commands/local/invoke/cli.py +++ b/samcli/commands/local/invoke/cli.py @@ -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, diff --git a/samcli/commands/local/lib/local_lambda.py b/samcli/commands/local/lib/local_lambda.py index 029e9f5856..ac401fe6d5 100644 --- a/samcli/commands/local/lib/local_lambda.py +++ b/samcli/commands/local/lib/local_lambda.py @@ -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): """ @@ -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 @@ -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 diff --git a/samcli/commands/local/start_api/cli.py b/samcli/commands/local/start_api/cli.py index b4928de7e6..fb832efaaf 100644 --- a/samcli/commands/local/start_api/cli.py +++ b/samcli/commands/local/start_api/cli.py @@ -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, diff --git a/samcli/commands/local/start_lambda/cli.py b/samcli/commands/local/start_lambda/cli.py index b27c479151..4bd7ba1129 100644 --- a/samcli/commands/local/start_lambda/cli.py +++ b/samcli/commands/local/start_lambda/cli.py @@ -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, diff --git a/samcli/local/layers/layer_downloader.py b/samcli/local/layers/layer_downloader.py index 0967f8a548..da20d0a2a2 100644 --- a/samcli/local/layers/layer_downloader.py +++ b/samcli/local/layers/layer_downloader.py @@ -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): diff --git a/tests/integration/local/start_lambda/test_start_lambda.py b/tests/integration/local/start_lambda/test_start_lambda.py index ca66961abb..c931cfd735 100644 --- a/tests/integration/local/start_lambda/test_start_lambda.py +++ b/tests/integration/local/start_lambda/test_start_lambda.py @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/tests/unit/commands/local/cli_common/test_invoke_context.py b/tests/unit/commands/local/cli_common/test_invoke_context.py index 597f7a4130..48f35b2023 100644 --- a/tests/unit/commands/local/cli_common/test_invoke_context.py +++ b/tests/unit/commands/local/cli_common/test_invoke_context.py @@ -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() @@ -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() @@ -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") @@ -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): diff --git a/tests/unit/commands/local/invoke/test_cli.py b/tests/unit/commands/local/invoke/test_cli.py index 8f6b514b36..e762a434cc 100644 --- a/tests/unit/commands/local/invoke/test_cli.py +++ b/tests/unit/commands/local/invoke/test_cli.py @@ -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") @@ -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() @@ -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, @@ -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() @@ -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="{}", @@ -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: @@ -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() @@ -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 @@ -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") diff --git a/tests/unit/commands/local/lib/test_local_lambda.py b/tests/unit/commands/local/lib/test_local_lambda.py index 5cfb92c266..bdc4e14143 100644 --- a/tests/unit/commands/local/lib/test_local_lambda.py +++ b/tests/unit/commands/local/lib/test_local_lambda.py @@ -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): @@ -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 = { @@ -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() @@ -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 @@ -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): @@ -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 @@ -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): @@ -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 @@ -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): diff --git a/tests/unit/commands/local/start_api/test_cli.py b/tests/unit/commands/local/start_api/test_cli.py index 8dbb129ddb..7b09b1ed1f 100644 --- a/tests/unit/commands/local/start_api/test_cli.py +++ b/tests/unit/commands/local/start_api/test_cli.py @@ -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 @@ -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, diff --git a/tests/unit/commands/local/start_lambda/test_cli.py b/tests/unit/commands/local/start_lambda/test_cli.py index 3656f8245c..dfc02cc3c0 100644 --- a/tests/unit/commands/local/start_lambda/test_cli.py +++ b/tests/unit/commands/local/start_lambda/test_cli.py @@ -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 @@ -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,