From 917926e38f2195c9dd6728f3b9d656dd2c13ddfd Mon Sep 17 00:00:00 2001 From: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com> Date: Mon, 20 Mar 2023 14:07:55 -0700 Subject: [PATCH] fix: respect `--resolve-s3` and `--resolve-image-repos` in a guided context. (#4873) * fix: Do not set s3_bucket explicitly on sam deploy --guided. * fix: make deploy --guided seamless * especially when --resolve-s3 or --resolve-image-repos is set, no further questions are asked on setting up infrastructure. --- samcli/commands/deploy/command.py | 2 + samcli/commands/deploy/guided_context.py | 42 ++++-- tests/unit/commands/deploy/test_command.py | 158 ++++++++++++++++++++- 3 files changed, 188 insertions(+), 14 deletions(-) diff --git a/samcli/commands/deploy/command.py b/samcli/commands/deploy/command.py index 3125f14fce..44fc6c5aa2 100644 --- a/samcli/commands/deploy/command.py +++ b/samcli/commands/deploy/command.py @@ -271,6 +271,8 @@ def do_cli( s3_bucket=s3_bucket, image_repository=image_repository, image_repositories=image_repositories, + resolve_s3=resolve_s3, + resolve_image_repos=resolve_image_repos, s3_prefix=s3_prefix, region=region, profile=profile, diff --git a/samcli/commands/deploy/guided_context.py b/samcli/commands/deploy/guided_context.py index 4c04d611b7..4b65526d84 100644 --- a/samcli/commands/deploy/guided_context.py +++ b/samcli/commands/deploy/guided_context.py @@ -24,7 +24,7 @@ from samcli.commands.deploy.guided_config import GuidedConfig from samcli.commands.deploy.utils import sanitize_parameter_overrides from samcli.lib.bootstrap.bootstrap import manage_stack -from samcli.lib.bootstrap.companion_stack.companion_stack_manager import CompanionStackManager +from samcli.lib.bootstrap.companion_stack.companion_stack_manager import CompanionStackManager, sync_ecr_stack from samcli.lib.config.samconfig import DEFAULT_CONFIG_FILE_NAME, DEFAULT_ENV from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable from samcli.lib.package.ecr_utils import is_ecr_url @@ -49,6 +49,8 @@ def __init__( image_repository, image_repositories, s3_prefix, + resolve_s3=False, + resolve_image_repos=False, region=None, profile=None, confirm_changeset=None, @@ -83,6 +85,8 @@ def __init__( self.guided_s3_prefix = None self.guided_region = None self.guided_profile = None + self.resolve_s3 = resolve_s3 + self.resolve_image_repositories = resolve_image_repos self.signing_profiles = signing_profiles self._capabilities = None self._parameter_overrides = None @@ -177,17 +181,30 @@ def guided_prompts(self, parameter_override_keys): ) click.echo("\n\tLooking for resources needed for deployment:") - s3_bucket = manage_stack(profile=self.profile, region=region) - click.secho(f"\n\tManaged S3 bucket: {s3_bucket}", bold=True) - click.echo("\tA different default S3 bucket can be set in samconfig.toml") + managed_s3_bucket = manage_stack(profile=self.profile, region=region) + click.secho(f"\n\tManaged S3 bucket: {managed_s3_bucket}", bold=True) + click.echo( + "\tA different default S3 bucket can be set in samconfig.toml" + " and auto resolution of buckets turned off by setting resolve_s3=False" + ) - image_repositories = self.prompt_image_repository( - stack_name, stacks, self.image_repositories, region, s3_bucket, self.s3_prefix + image_repositories = ( + sync_ecr_stack( + self.template_file, stack_name, region, managed_s3_bucket, self.s3_prefix, self.image_repositories + ) + if self.resolve_image_repositories + else self.prompt_image_repository( + stack_name, stacks, self.image_repositories, region, managed_s3_bucket, self.s3_prefix + ) ) self.guided_stack_name = stack_name - self.guided_s3_bucket = s3_bucket + self.guided_s3_bucket = managed_s3_bucket self.guided_image_repositories = image_repositories + # NOTE(sriram-mv): The resultant s3 bucket is ALWAYS the managed_s3_bucket. There is no user flow to set it + # within guided. + self.resolve_s3 = True if self.guided_s3_bucket else False + self.guided_s3_prefix = stack_name self.guided_region = region self.guided_profile = self.profile @@ -403,7 +420,9 @@ def prompt_specify_repos( ) if not is_ecr_url(image_uri): raise GuidedDeployFailedError(f"Invalid Image Repository ECR URI: {image_uri}") - + # NOTE(sriram-mv): If a prompt to accept an ECR URI succeeded, then one does not any longer + # resolve image repositories automatically. + self.resolve_image_repositories = False updated_repositories[function_logical_id] = image_uri return updated_repositories @@ -510,7 +529,7 @@ def prompt_delete_unreferenced_repos( click.echo( "\t #The deployment was aborted to prevent " "unreferenced managed ECR repositories from being deleted.\n" - "\t #You may remove repositories from the SAMCLI " + "\t #You may remove repositories from the AWS SAM CLI " "managed stack to retain them and resolve this unreferenced check." ) raise GuidedDeployFailedError("Unreferenced Auto Created ECR Repos Must Be Deleted.") @@ -562,9 +581,10 @@ def run(self): self.config_env or DEFAULT_ENV, self.config_file or DEFAULT_CONFIG_FILE_NAME, stack_name=self.guided_stack_name, - s3_bucket=self.guided_s3_bucket, + resolve_s3=self.resolve_s3, s3_prefix=self.guided_s3_prefix, - image_repositories=self.guided_image_repositories, + image_repositories=self.guided_image_repositories if not self.resolve_image_repositories else None, + resolve_image_repos=self.resolve_image_repositories, region=self.guided_region, profile=self.guided_profile, confirm_changeset=self.confirm_changeset, diff --git a/tests/unit/commands/deploy/test_command.py b/tests/unit/commands/deploy/test_command.py index 4376114362..db4c22b420 100644 --- a/tests/unit/commands/deploy/test_command.py +++ b/tests/unit/commands/deploy/test_command.py @@ -221,6 +221,157 @@ def test_all_args_guided_no_to_authorization_confirmation_prompt( on_failure=self.on_failure, ) + @patch("samcli.commands.package.command.click") + @patch("samcli.commands.package.package_context.PackageContext") + @patch("samcli.commands.deploy.command.click") + @patch("samcli.commands.deploy.deploy_context.DeployContext") + @patch("samcli.commands.deploy.guided_context.manage_stack") + @patch("samcli.commands.deploy.guided_context.auth_per_resource") + @patch("samcli.commands.deploy.guided_context.get_template_parameters") + @patch("samcli.commands.deploy.guided_context.SamLocalStackProvider.get_stacks") + @patch("samcli.commands.deploy.guided_context.SamFunctionProvider") + @patch("samcli.commands.deploy.guided_context.signer_config_per_function") + @patch.object(GuidedConfig, "get_config_ctx", MagicMock(return_value=(None, get_mock_sam_config()))) + @patch("samcli.commands.deploy.guided_context.prompt") + @patch("samcli.commands.deploy.guided_context.confirm") + @patch("samcli.commands.deploy.guided_context.tag_translation") + @patch("samcli.commands.deploy.guided_context.sync_ecr_stack") + def test_all_args_guided_use_defaults( + self, + mock_sync_ecr_stack, + mock_tag_translation, + mock_confirm, + mock_prompt, + mock_signer_config_per_function, + mock_sam_function_provider, + mock_get_buildable_stacks, + mock_get_template_parameters, + mockauth_per_resource, + mock_managed_stack, + mock_deploy_context, + mock_deploy_click, + mock_package_context, + mock_package_click, + ): + mock_get_buildable_stacks.return_value = (Mock(), []) + mock_tag_translation.return_value = "helloworld-123456-v1" + + context_mock = Mock() + function_mock = MagicMock() + function_mock.packagetype = IMAGE + function_mock.imageuri = "helloworld:v1" + function_mock.full_path = "HelloWorldFunction" + mock_sam_function_provider.return_value.get_all.return_value = [function_mock] + mockauth_per_resource.return_value = [("HelloWorldResource", False)] + mock_deploy_context.return_value.__enter__.return_value = context_mock + mock_confirm.side_effect = [True, False, True, True, True, True, True] + mock_prompt.side_effect = [ + "sam-app", + "us-east-1", + "guidedParameter", + "secure", + ("CAPABILITY_IAM",), + "testconfig.toml", + "test-env", + ] + + mock_get_template_parameters.return_value = { + "Myparameter": {"Type": "String"}, + "MyNoEchoParameter": {"Type": "String", "NoEcho": True}, + } + + mock_managed_stack.return_value = "managed-s3-bucket" + mock_sync_ecr_stack.return_value = { + "HelloWorldFunction": "123456789012.dkr.ecr.us-east-1.amazonaws.com/managed-ecr" + } + + mock_signer_config_per_function.return_value = ({}, {}) + + self.resolve_s3 = True + self.resolve_image_repos = True + with patch.object(GuidedConfig, "save_config", MagicMock(return_value=True)) as mock_save_config: + do_cli( + template_file=self.template_file, + stack_name=self.stack_name, + s3_bucket=None, + image_repository=None, + image_repositories=None, + force_upload=self.force_upload, + no_progressbar=self.no_progressbar, + s3_prefix=self.s3_prefix, + kms_key_id=self.kms_key_id, + parameter_overrides=self.parameter_overrides, + capabilities=self.capabilities, + no_execute_changeset=self.no_execute_changeset, + role_arn=self.role_arn, + notification_arns=self.notification_arns, + fail_on_empty_changeset=self.fail_on_empty_changset, + tags=self.tags, + region=self.region, + profile=self.profile, + use_json=self.use_json, + metadata=self.metadata, + guided=True, + confirm_changeset=True, + signing_profiles=self.signing_profiles, + resolve_s3=self.resolve_s3, + config_env=self.config_env, + config_file=self.config_file, + resolve_image_repos=self.resolve_image_repos, + disable_rollback=self.disable_rollback, + on_failure=self.on_failure, + ) + + mock_deploy_context.assert_called_with( + template_file=ANY, + stack_name="sam-app", + s3_bucket="managed-s3-bucket", + image_repository=None, + image_repositories={"HelloWorldFunction": "123456789012.dkr.ecr.us-east-1.amazonaws.com/managed-ecr"}, + force_upload=self.force_upload, + no_progressbar=self.no_progressbar, + s3_prefix="sam-app", + kms_key_id=self.kms_key_id, + parameter_overrides={"Myparameter": "guidedParameter", "MyNoEchoParameter": "secure"}, + capabilities=self.capabilities, + no_execute_changeset=self.no_execute_changeset, + role_arn=self.role_arn, + notification_arns=self.notification_arns, + fail_on_empty_changeset=self.fail_on_empty_changset, + tags=self.tags, + region="us-east-1", + profile=self.profile, + confirm_changeset=True, + signing_profiles=self.signing_profiles, + use_changeset=self.use_changeset, + disable_rollback=True, + poll_delay=0.5, + on_failure=self.on_failure, + ) + + context_mock.run.assert_called_with() + mock_save_config.assert_called_with( + { + "Myparameter": {"Value": "guidedParameter", "Hidden": False}, + "MyNoEchoParameter": {"Value": "secure", "Hidden": True}, + }, + "test-env", + "testconfig.toml", + capabilities=("CAPABILITY_IAM",), + confirm_changeset=True, + profile=self.profile, + region="us-east-1", + resolve_s3=True, + resolve_image_repos=True, + image_repositories=None, + stack_name="sam-app", + s3_prefix="sam-app", + signing_profiles=self.signing_profiles, + disable_rollback=True, + ) + mock_managed_stack.assert_called_with(profile=self.profile, region="us-east-1") + self.assertEqual(context_mock.run.call_count, 1) + @patch("samcli.commands.package.command.click") @patch("samcli.commands.package.package_context.PackageContext") @patch("samcli.commands.deploy.command.click") @@ -354,7 +505,8 @@ def test_all_args_guided( confirm_changeset=True, profile=self.profile, region="us-east-1", - s3_bucket="managed-s3-bucket", + resolve_s3=True, + resolve_image_repos=False, image_repositories={"HelloWorldFunction": "123456789012.dkr.ecr.us-east-1.amazonaws.com/test1"}, stack_name="sam-app", s3_prefix="sam-app", @@ -501,7 +653,7 @@ def test_all_args_guided_no_save_echo_param_to_config( MOCK_SAM_CONFIG.put.call_args_list, [ call(["deploy"], "parameters", "stack_name", "sam-app", env="test-env"), - call(["deploy"], "parameters", "s3_bucket", "managed-s3-bucket", env="test-env"), + call(["deploy"], "parameters", "resolve_s3", True, env="test-env"), call(["deploy"], "parameters", "s3_prefix", "sam-app", env="test-env"), call(["deploy"], "parameters", "region", "us-east-1", env="test-env"), call(["deploy"], "parameters", "confirm_changeset", True, env="test-env"), @@ -655,7 +807,7 @@ def test_all_args_guided_no_params_save_config( MOCK_SAM_CONFIG.put.call_args_list, [ call(["deploy"], "parameters", "stack_name", "sam-app", env="test-env"), - call(["deploy"], "parameters", "s3_bucket", "managed-s3-bucket", env="test-env"), + call(["deploy"], "parameters", "resolve_s3", True, env="test-env"), call(["deploy"], "parameters", "s3_prefix", "sam-app", env="test-env"), call(["deploy"], "parameters", "region", "us-east-1", env="test-env"), call(["deploy"], "parameters", "confirm_changeset", True, env="test-env"),