From bee95254d61adbf18f261abdf2062bbb11798d0e Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Fri, 21 Jul 2023 15:50:42 -0700 Subject: [PATCH 1/2] fix: remove resolve-image-repos error message from the commands it is not supported --- samcli/commands/deploy/command.py | 2 +- samcli/commands/package/command.py | 2 +- samcli/commands/sync/command.py | 2 +- .../image_repository_validation.py | 152 ++++++++++-------- .../test_image_repository_validation.py | 42 +++-- 5 files changed, 121 insertions(+), 79 deletions(-) diff --git a/samcli/commands/deploy/command.py b/samcli/commands/deploy/command.py index 557a601261..dee019f117 100644 --- a/samcli/commands/deploy/command.py +++ b/samcli/commands/deploy/command.py @@ -154,7 +154,7 @@ @capabilities_option @aws_creds_options @common_options -@image_repository_validation +@image_repository_validation() @pass_context @track_command @check_newer_version diff --git a/samcli/commands/package/command.py b/samcli/commands/package/command.py index 151661c91e..0b1efb6fce 100644 --- a/samcli/commands/package/command.py +++ b/samcli/commands/package/command.py @@ -90,7 +90,7 @@ def resources_and_properties_help_string(): @no_progressbar_option @common_options @aws_creds_options -@image_repository_validation +@image_repository_validation(support_resolve_image_repos=False) @pass_context @track_command @check_newer_version diff --git a/samcli/commands/sync/command.py b/samcli/commands/sync/command.py index ddc5e2a165..bb1e24c7fd 100644 --- a/samcli/commands/sync/command.py +++ b/samcli/commands/sync/command.py @@ -175,7 +175,7 @@ @pass_context @track_command @track_long_event("SyncUsed", "Start", "SyncUsed", "End") -@image_repository_validation +@image_repository_validation(support_resolve_image_repos=False) @track_template_warnings([CodeDeployWarning.__name__, CodeDeployConditionWarning.__name__]) @check_newer_version @print_cmdline_args diff --git a/samcli/lib/cli_validation/image_repository_validation.py b/samcli/lib/cli_validation/image_repository_validation.py index 277e6321f6..8d26ff558e 100644 --- a/samcli/lib/cli_validation/image_repository_validation.py +++ b/samcli/lib/cli_validation/image_repository_validation.py @@ -15,83 +15,103 @@ from samcli.lib.utils.packagetype import IMAGE -def image_repository_validation(func): +def image_repository_validation(support_resolve_image_repos=True): """ Wrapper Validation function that will run last after the all cli parmaters have been loaded to check for conditions surrounding `--image-repository`, `--image-repositories`, and `--resolve-image-repos`. The reason they are done last instead of in callback functions, is because the options depend on each other, and this breaks cyclic dependencies. - :param func: Click command function - :return: Click command function after validation + Parameters + ---------- + support_resolve_image_repos: bool + If it is True, it will be adding `--resolve-image-repos` related error messages below. Default is True. """ - def wrapped(*args, **kwargs): - ctx = click.get_current_context() - guided = ctx.params.get("guided", False) or ctx.params.get("g", False) - image_repository = ctx.params.get("image_repository", False) - image_repositories = ctx.params.get("image_repositories", False) or {} - resolve_image_repos = ctx.params.get("resolve_image_repos", False) - parameters_overrides = ctx.params.get("parameters_overrides", {}) - template_file = ( - ctx.params.get("t", False) or ctx.params.get("template_file", False) or ctx.params.get("template", False) - ) - - # Check if `--image-repository`, `--image-repositories`, or `--resolve-image-repos` are required by - # looking for resources that have an IMAGE based packagetype. - - required = any( - [ - _template_artifact == IMAGE - for _template_artifact in get_template_artifacts_format(template_file=template_file) - ] - ) - - validators = [ - Validator( - validation_function=lambda: bool(image_repository) - + bool(image_repositories) - + bool(resolve_image_repos) - > 1, - exception=click.BadOptionUsage( - option_name="--image-repositories", - ctx=ctx, - message="Only one of the following can be provided: '--image-repositories', " - "'--image-repository', or '--resolve-image-repos'. " - "Do you have multiple specified in the command or in a configuration file?", - ), - ), - Validator( - validation_function=lambda: not guided - and not (image_repository or image_repositories or resolve_image_repos) - and required, - exception=click.BadOptionUsage( - option_name="--image-repositories", - ctx=ctx, - message="Missing option '--image-repository', '--image-repositories', or '--resolve-image-repos'", + def decorator(func): + """ + Actual decorator implementation for the validation functionality + + :param func: Click command function + :return: Click command function after validation + """ + + def wrapped(*args, **kwargs): + ctx = click.get_current_context() + guided = ctx.params.get("guided", False) or ctx.params.get("g", False) + image_repository = ctx.params.get("image_repository", False) + image_repositories = ctx.params.get("image_repositories", False) or {} + resolve_image_repos = ctx.params.get("resolve_image_repos", False) + parameters_overrides = ctx.params.get("parameters_overrides", {}) + template_file = ( + ctx.params.get("t", False) + or ctx.params.get("template_file", False) + or ctx.params.get("template", False) + ) + + # Check if `--image-repository`, `--image-repositories`, or `--resolve-image-repos` are required by + # looking for resources that have an IMAGE based packagetype. + + required = any( + [ + _template_artifact == IMAGE + for _template_artifact in get_template_artifacts_format(template_file=template_file) + ] + ) + + available_options = "'--image-repositories', '--image-repository'" + if support_resolve_image_repos: + available_options += ", '--resolve-image-repos'" + + image_repos_error_msg = "Incomplete list of function logical ids specified for '--image-repositories'." + if support_resolve_image_repos: + image_repos_error_msg += ( + "You can also add --resolve-image-repos to automatically create missing " "repositories." + ) + + validators = [ + Validator( + validation_function=lambda: bool(image_repository) + + bool(image_repositories) + + bool(resolve_image_repos) + > 1, + exception=click.BadOptionUsage( + option_name="--image-repositories", + ctx=ctx, + message=f"Only one of the following can be provided: {available_options}. " + "Do you have multiple specified in the command or in a configuration file?", + ), ), - ), - Validator( - validation_function=lambda: not guided - and ( - image_repositories - and not resolve_image_repos - and not _is_all_image_funcs_provided(template_file, image_repositories, parameters_overrides) + Validator( + validation_function=lambda: not guided + and not (image_repository or image_repositories or resolve_image_repos) + and required, + exception=click.BadOptionUsage( + option_name="--image-repositories", + ctx=ctx, + message=f"Missing option {available_options}", + ), ), - exception=click.BadOptionUsage( - option_name="--image-repositories", - ctx=ctx, - message="Incomplete list of function logical ids specified for '--image-repositories'. " - "You can also add --resolve-image-repos to automatically create missing repositories.", + Validator( + validation_function=lambda: not guided + and ( + image_repositories + and not resolve_image_repos + and not _is_all_image_funcs_provided(template_file, image_repositories, parameters_overrides) + ), + exception=click.BadOptionUsage( + option_name="--image-repositories", ctx=ctx, message=image_repos_error_msg + ), ), - ), - ] - for validator in validators: - validator.validate() - # Call Original function after validation. - return func(*args, **kwargs) - - return wrapped + ] + for validator in validators: + validator.validate() + # Call Original function after validation. + return func(*args, **kwargs) + + return wrapped + + return decorator def _is_all_image_funcs_provided(template_file, image_repositories, parameters_overrides): diff --git a/tests/unit/lib/cli_validation/test_image_repository_validation.py b/tests/unit/lib/cli_validation/test_image_repository_validation.py index c312a4993f..cb60f78933 100644 --- a/tests/unit/lib/cli_validation/test_image_repository_validation.py +++ b/tests/unit/lib/cli_validation/test_image_repository_validation.py @@ -3,7 +3,7 @@ import click import posixpath -from parameterized import parameterized +from parameterized import parameterized, parameterized_class from samcli.lib.cli_validation.image_repository_validation import ( image_repository_validation, @@ -14,9 +14,18 @@ from samcli.lib.utils.packagetype import ZIP, IMAGE +@parameterized_class( + ("support_resolve_image_repos"), + [ + (True,), + (False,), + ], +) class TestImageRepositoryValidation(TestCase): + support_resolve_image_repos = False + def setUp(self): - @image_repository_validation + @image_repository_validation(self.support_resolve_image_repos) def foo(): pass @@ -102,10 +111,16 @@ def test_image_repository_validation_failure_IMAGE_image_repositories_and_image_ with self.assertRaises(click.BadOptionUsage) as ex: self.foobar() - self.assertIn( - "Only one of the following can be provided: '--image-repositories', '--image-repository', or '--resolve-image-repos'. ", - ex.exception.message, - ) + if self.support_resolve_image_repos: + self.assertIn( + "Only one of the following can be provided: '--image-repositories', '--image-repository', '--resolve-image-repos'.", + ex.exception.message, + ) + else: + self.assertIn( + "Only one of the following can be provided: '--image-repositories', '--image-repository'.", + ex.exception.message, + ) @patch("samcli.lib.cli_validation.image_repository_validation.click") @patch("samcli.lib.cli_validation.image_repository_validation._is_all_image_funcs_provided") @@ -147,10 +162,17 @@ def test_image_repository_validation_failure_IMAGE_missing_image_repositories( with self.assertRaises(click.BadOptionUsage) as ex: self.foobar() - self.assertIn( - "Missing option '--image-repository', '--image-repositories', or '--resolve-image-repos'", - ex.exception.message, - ) + if self.support_resolve_image_repos: + self.assertIn( + "Missing option '--image-repositories', '--image-repository', '--resolve-image-repos'", + ex.exception.message, + ) + else: + self.assertIn( + "Missing option '--image-repositories', '--image-repository'", + ex.exception.message, + ) + @patch("samcli.lib.cli_validation.image_repository_validation.click") @patch("samcli.lib.cli_validation.image_repository_validation._is_all_image_funcs_provided") From b18168e2b2fc5dfde8340002938833e076205a49 Mon Sep 17 00:00:00 2001 From: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Date: Fri, 21 Jul 2023 17:11:59 -0700 Subject: [PATCH 2/2] formatting --- .../unit/lib/cli_validation/test_image_repository_validation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/lib/cli_validation/test_image_repository_validation.py b/tests/unit/lib/cli_validation/test_image_repository_validation.py index cb60f78933..37f00b1f87 100644 --- a/tests/unit/lib/cli_validation/test_image_repository_validation.py +++ b/tests/unit/lib/cli_validation/test_image_repository_validation.py @@ -173,7 +173,6 @@ def test_image_repository_validation_failure_IMAGE_missing_image_repositories( ex.exception.message, ) - @patch("samcli.lib.cli_validation.image_repository_validation.click") @patch("samcli.lib.cli_validation.image_repository_validation._is_all_image_funcs_provided") @patch("samcli.lib.cli_validation.image_repository_validation.get_template_artifacts_format")