From 3e9ebe961ffabd7eb5f3a8bba155c519085f8010 Mon Sep 17 00:00:00 2001 From: Kyle Laker Date: Wed, 2 Dec 2020 20:38:11 -0500 Subject: [PATCH] Fix schema loading when no default profile The region_name value when a profile does not specify a region or when there is not a 'default' profile resulted in a crash since _get_aws_region_choice attempted to perform concatenation using the `+` operator (which is undefined for str + NoneType). Using an actual string formatting method fixes that issue. Additionally, it is useless to prompt the user whether not to use the 'default' profile since doing so is impossible. They need to be prompted. Additionally, it is slightly more 'intelligent' to create a new session after the user has chosen a new profile so that a better default region can be chosen. --- samcli/lib/schemas/schemas_aws_config.py | 13 +++-- .../lib/schemas/test_schemas_aws_config.py | 48 +++++++++++++++++-- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/samcli/lib/schemas/schemas_aws_config.py b/samcli/lib/schemas/schemas_aws_config.py index d845f6e482..37be478773 100644 --- a/samcli/lib/schemas/schemas_aws_config.py +++ b/samcli/lib/schemas/schemas_aws_config.py @@ -15,15 +15,18 @@ def get_aws_configuration_choice(): profile = session.profile_name region = session.region_name - message = "\nDo you want to use the default AWS profile [%s] and region [%s]?" % (profile, region) - choice = click.confirm(message, default=True) + message = f"\nDo you want to use the default AWS profile [{profile}] and region [{region}]?" + # Avoid prompting if there isn't a region, the user must supply the information + needs_edit = (not region) or (not click.confirm(message, default=True)) schemas_available_regions_name = session.get_available_regions("schemas") - if not choice: + if needs_edit: available_profiles = session.available_profiles profile = _get_aws_profile_choice(available_profiles) - region = _get_aws_region_choice(schemas_available_regions_name, region) + # Reinitialize the session with the new profile to get the new region + session = Session(profile_name=profile) + region = _get_aws_region_choice(schemas_available_regions_name, session.region_name) else: # session.profile_name will return 'default' if no profile is found, # but botocore itself will fail if you pass it in, when one is not configured @@ -74,7 +77,7 @@ def _get_aws_region_choice(available_regions_name, region): msg = cli_display_regions[cli_display_region] click.echo("# " + msg) - region_choice = click.prompt("Region " + "[" + region + "]", type=str, show_choices=False) + region_choice = click.prompt(f"Region [{region}]", type=str, show_choices=False) return region_choice diff --git a/tests/unit/lib/schemas/test_schemas_aws_config.py b/tests/unit/lib/schemas/test_schemas_aws_config.py index 8755ad6a43..25e7f6ce74 100644 --- a/tests/unit/lib/schemas/test_schemas_aws_config.py +++ b/tests/unit/lib/schemas/test_schemas_aws_config.py @@ -1,7 +1,7 @@ """ Test AWS configuration """ from unittest import TestCase -from unittest.mock import patch, ANY +from unittest.mock import patch, ANY, Mock from samcli.commands.local.cli_common.user_exceptions import ResourceNotFound, NotAvailableInRegion from samcli.lib.schemas.schemas_aws_config import get_aws_configuration_choice @@ -15,7 +15,7 @@ def test_get_aws_configuration_profile_is_set_to_none_for_default_selection(self session_mock.return_value.profile_name = "default" session_mock.return_value.region_name = "us-west-2" aws_configuration_choice = get_aws_configuration_choice() - self.assertEqual(aws_configuration_choice["profile"], None), + self.assertEqual(aws_configuration_choice["profile"], None) self.assertEqual(aws_configuration_choice["region"], "us-west-2") confirm_mock.assert_any_call( "\nDo you want to use the default AWS profile [default] and region [us-west-2]?", default=True @@ -38,7 +38,7 @@ def test_get_aws_configuration_choice_selected(self, prompt_mock, confirm_mock, "ap-northeast-1", ] aws_configuration_choice = get_aws_configuration_choice() - self.assertEqual(aws_configuration_choice["profile"], "test-profile"), + self.assertEqual(aws_configuration_choice["profile"], "test-profile") self.assertEqual(aws_configuration_choice["region"], "us-east-2") confirm_mock.assert_any_call( "\nDo you want to use the default AWS profile [default] and region [us-west-2]?", default=True @@ -75,10 +75,50 @@ def test_get_aws_configuration_allow_free_text_region_value(self, prompt_mock, c "ap-northeast-1", ] aws_configuration_choice = get_aws_configuration_choice() - self.assertEqual(aws_configuration_choice["profile"], "test-profile"), + self.assertEqual(aws_configuration_choice["profile"], "test-profile") self.assertEqual(aws_configuration_choice["region"], "random-region") confirm_mock.assert_any_call( "\nDo you want to use the default AWS profile [default] and region [us-west-2]?", default=True ) prompt_mock.assert_any_call("Profile", type=ANY, show_choices=False) prompt_mock.assert_any_call("Region [us-west-2]", type=ANY, show_choices=False) + + @patch("samcli.lib.schemas.schemas_aws_config.Session") + @patch("click.confirm") + @patch("click.prompt") + def test_get_aws_configuration_succeeds_with_default(self, prompt_mock, confirm_mock, session_mock): + region = "us-east-2" + confirm_mock.side_effect = [True] + prompt_mock.side_effect = ["1", region] + + def profile_mock(**kwargs): + session = Mock() + session.profile_name = "default" + session.available_profiles = ["test-profile-1", "test-profile-2"] + session.get_available_regions.return_value = [ + "us-east-1", + "us-east-2", + "us-west-2", + "eu-west-1", + "ap-northeast-1", + ] + if "profile_name" in kwargs: + session.profile_name = kwargs["profile_name"] + session.region_name = region + else: + session.profile_name = "default" + session.region_name = None + return session + + session_mock.side_effect = profile_mock + + aws_configuration_choice = get_aws_configuration_choice() + self.assertEqual(aws_configuration_choice["profile"], "test-profile-1") + self.assertEqual(aws_configuration_choice["region"], region) + + # Since the region will be None, the user should not get prompted to confirm + # whether to choose a different profile. + self.assertFalse(confirm_mock.called) + + prompt_mock.assert_any_call("Profile", type=ANY, show_choices=False) + prompt_mock.assert_any_call(f"Region [{region}]", type=ANY, show_choices=False)