From b524fb7afa0099c0862ced57aed2045e88a13cf9 Mon Sep 17 00:00:00 2001 From: shashax42 Date: Mon, 24 Mar 2025 18:56:43 +0900 Subject: [PATCH 1/4] feat: add --serverless-rules to sam validate --lint --- samcli/commands/validate/validate.py | 52 ++++++++++++++++++++--- schema/samcli.json | 7 ++- tests/unit/commands/validate/test_cli.py | 54 ++++++++++++++++++++++-- 3 files changed, 104 insertions(+), 9 deletions(-) diff --git a/samcli/commands/validate/validate.py b/samcli/commands/validate/validate.py index cf317b8bfe..1dc9ac808a 100644 --- a/samcli/commands/validate/validate.py +++ b/samcli/commands/validate/validate.py @@ -57,6 +57,13 @@ class SamTemplate: "Create a cfnlintrc config file to specify additional parameters. " "For more information, see: https://github.com/aws-cloudformation/cfn-lint", ) +@click.option( + "--serverless-rules", + is_flag=True, + help="Enable Serverless Rules for linting validation. " + "Requires the cfn-lint-serverless package to be installed. " + "For more information, see: https://github.com/awslabs/serverless-rules", +) @save_params_option @pass_context @track_command @@ -64,13 +71,13 @@ class SamTemplate: @print_cmdline_args @unsupported_command_cdk(alternative_command="cdk doctor") @command_exception_handler -def cli(ctx, template_file, config_file, config_env, lint, save_params): +def cli(ctx, template_file, config_file, config_env, lint, save_params, serverless_rules): # All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing - do_cli(ctx, template_file, lint) # pragma: no cover + do_cli(ctx, template_file, lint, serverless_rules) # pragma: no cover -def do_cli(ctx, template, lint): +def do_cli(ctx, template, lint, serverless_rules): """ Implementation of the ``cli`` method, just separated out for unit testing purposes """ @@ -84,7 +91,7 @@ def do_cli(ctx, template, lint): sam_template = _read_sam_file(template) if lint: - _lint(ctx, sam_template.serialized, template) + _lint(ctx, sam_template.serialized, template, serverless_rules) else: iam_client = boto3.client("iam") validator = SamTemplateValidator( @@ -136,7 +143,7 @@ def _read_sam_file(template) -> SamTemplate: return SamTemplate(serialized=template_string, deserialized=sam_template) -def _lint(ctx: Context, template: str, template_path: str) -> None: +def _lint(ctx: Context, template: str, template_path: str, serverless_rules: bool) -> None: """ Parses provided SAM template and maps errors from CloudFormation template back to SAM template. @@ -153,6 +160,8 @@ def _lint(ctx: Context, template: str, template_path: str) -> None: Contents of sam template as a string template_path Path to the sam template + serverless_rules + Flag to enable Serverless Rules for linting """ from cfnlint.api import ManualArgs, lint @@ -170,6 +179,39 @@ def _lint(ctx: Context, template: str, template_path: str) -> None: cfn_lint_logger.propagate = True cfn_lint_logger.setLevel(logging.DEBUG) + # Add Serverless Rules if enabled + if serverless_rules: + try: + # Track usage of Serverless Rules + EventTracker.track_event("UsedFeature", "ServerlessRules") + + # Check if cfn-lint-serverless is installed + import importlib.util + if importlib.util.find_spec("cfn_lint_serverless") is None: + click.secho( + "Serverless Rules package (cfn-lint-serverless) is not installed. " + "Please install it using: pip install cfn-lint-serverless", + fg="red", + ) + raise UserException( + "Serverless Rules package (cfn-lint-serverless) is not installed. " + "Please install it using: pip install cfn-lint-serverless" + ) + + # Add Serverless Rules to the linter config + linter_config["append_rules"] = ["cfn_lint_serverless.rules"] + click.secho("Serverless Rules enabled for linting", fg="green") + except ImportError: + click.secho( + "Serverless Rules package (cfn-lint-serverless) is not installed. " + "Please install it using: pip install cfn-lint-serverless", + fg="red", + ) + raise UserException( + "Serverless Rules package (cfn-lint-serverless) is not installed. " + "Please install it using: pip install cfn-lint-serverless" + ) + config = ManualArgs(**linter_config) try: diff --git a/schema/samcli.json b/schema/samcli.json index 28bc9afc5c..d3d0a625cd 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -194,7 +194,7 @@ "properties": { "parameters": { "title": "Parameters for the validate command", - "description": "Available parameters for the validate command:\n* template_file:\nAWS SAM template file.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* lint:\nRun linting validation on template through cfn-lint. Create a cfnlintrc config file to specify additional parameters. For more information, see: https://github.com/aws-cloudformation/cfn-lint\n* save_params:\nSave the parameters provided via the command line to the configuration file.", + "description": "Available parameters for the validate command:\n* template_file:\nAWS SAM template file.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* lint:\nRun linting validation on template through cfn-lint. Create a cfnlintrc config file to specify additional parameters. For more information, see: https://github.com/aws-cloudformation/cfn-lint\n* serverless_rules:\nEnable Serverless Rules for linting validation. Requires the cfn-lint-serverless package to be installed. For more information, see: https://github.com/awslabs/serverless-rules\n* save_params:\nSave the parameters provided via the command line to the configuration file.", "type": "object", "properties": { "template_file": { @@ -228,6 +228,11 @@ "type": "boolean", "description": "Run linting validation on template through cfn-lint. Create a cfnlintrc config file to specify additional parameters. For more information, see: https://github.com/aws-cloudformation/cfn-lint" }, + "serverless_rules": { + "title": "serverless_rules", + "type": "boolean", + "description": "Enable Serverless Rules for linting validation. Requires the cfn-lint-serverless package to be installed. For more information, see: https://github.com/awslabs/serverless-rules" + }, "save_params": { "title": "save_params", "type": "boolean", diff --git a/tests/unit/commands/validate/test_cli.py b/tests/unit/commands/validate/test_cli.py index f77a4711e9..7e75818b5a 100644 --- a/tests/unit/commands/validate/test_cli.py +++ b/tests/unit/commands/validate/test_cli.py @@ -85,13 +85,13 @@ def test_template_passes_validation(self, patched_boto, read_sam_file_patch, cli @patch("samcli.commands.validate.validate._read_sam_file") @patch("samcli.commands.validate.validate.click") @patch("samcli.commands.validate.validate._lint") - def test_lint_template_passes(self, click_patch, lint_patch, read_sam_file_patch): + def test_lint_template_passes(self, lint_patch, click_patch, read_sam_file_patch): template_path = "path_to_template" read_sam_file_patch.return_value = SamTemplate(serialized="{}", deserialized={}) lint_patch.return_value = True - do_cli(ctx=ctx_lint_mock(debug=False, region="region"), template=template_path, lint=True) + do_cli(ctx=ctx_lint_mock(debug=False, region="region"), template=template_path, lint=True, serverless_rules=False) @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") @@ -105,6 +105,7 @@ def test_lint_event_recorded(self, click_patch, lint_patch): ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, template_path=template_path, + serverless_rules=False ) track_patch.assert_called_with("UsedFeature", "CFNLint") @@ -118,9 +119,56 @@ def test_linter_raises_exception_if_matches_found(self, click_patch, lint_patch) with self.assertRaises(LinterRuleMatchedException) as ex: _lint( - ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, template_path=template_path + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=False ) self.assertEqual( ex.exception.message, "Linting failed. At least one linting rule was matched to the provided template." ) + + @patch("cfnlint.api.lint") + @patch("samcli.commands.validate.validate.click") + @patch("importlib.util.find_spec") + def test_serverless_rules_enabled(self, find_spec_mock, click_patch, lint_patch): + template_path = "path_to_template" + template_contents = "{}" + find_spec_mock.return_value = True + lint_patch.return_value = [] + + with patch("samcli.lib.telemetry.event.EventTracker.track_event") as track_patch: + _lint( + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=True + ) + # Check that both CFNLint and ServerlessRules events are tracked + track_patch.assert_any_call("UsedFeature", "CFNLint") + track_patch.assert_any_call("UsedFeature", "ServerlessRules") + + # Check that the lint function was called with the serverless rules + lint_patch.assert_called_once() + args, kwargs = lint_patch.call_args + self.assertEqual(args[0], template_contents) + self.assertEqual(kwargs["config"].append_rules, ["cfn_lint_serverless.rules"]) + + @patch("cfnlint.api.lint") + @patch("samcli.commands.validate.validate.click") + @patch("importlib.util.find_spec") + def test_serverless_rules_package_not_installed(self, find_spec_mock, click_patch, lint_patch): + template_path = "path_to_template" + template_contents = "{}" + find_spec_mock.return_value = None + + with self.assertRaises(UserException) as ex: + _lint( + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=True + ) + + self.assertIn("Serverless Rules package (cfn-lint-serverless) is not installed", ex.exception.message) From dbdff604676304f506acc35004e183a09aeb76d6 Mon Sep 17 00:00:00 2001 From: shashax42 Date: Tue, 15 Apr 2025 01:28:01 +0900 Subject: [PATCH 2/4] feat: Add --extra-lint-rules option for flexible linting rules support - Mark existing --serverless-rules option as deprecated - Add more flexible --extra-lint-rules option - Implement support for comma-separated multiple rule modules - Design to work regardless of installation environment - Update documentation and schema accordingly --- docs/extra-lint-rules.md | 55 +++++++ samcli/commands/validate/validate.py | 114 +++++++++++---- schema/samcli.json | 9 +- tests/unit/commands/validate/test_cli.py | 178 ++++++++++++++++++++--- 4 files changed, 309 insertions(+), 47 deletions(-) create mode 100644 docs/extra-lint-rules.md diff --git a/docs/extra-lint-rules.md b/docs/extra-lint-rules.md new file mode 100644 index 0000000000..1623d18f76 --- /dev/null +++ b/docs/extra-lint-rules.md @@ -0,0 +1,55 @@ +# SAM CLI 추가 Lint 규칙 사용 가이드 + +AWS SAM CLI의 `validate` 명령어는 템플릿 검증을 위해 [cfn-lint](https://github.com/aws-cloudformation/cfn-lint)를 사용합니다. +이제 SAM CLI는 `--extra-lint-rules` 옵션을 통해 추가 lint 규칙을 지원합니다. + +## 사용 방법 + +```bash +sam validate --lint --extra-lint-rules="cfn_lint_serverless.rules" +``` + +## 인스톨러로 SAM CLI 설치 시 고려사항 + +SAM CLI를 인스톨러(설치 프로그램)로 설치한 경우, SAM CLI는 자체 Python 환경을 사용합니다. 이 경우 추가 규칙 모듈이 해당 환경에 설치되어 있어야 합니다. 이 때 두 가지 접근 방식이 있습니다: + +1. **인스톨러 Python 환경에 패키지 설치**: 인스톨러의 Python 환경에 필요한 패키지를 설치합니다. +2. **모듈 경로를 전체 경로로 지정**: 사용자 환경에 설치된 패키지의 전체 경로를 지정합니다. + +## 사용 예제 + +### 서버리스 규칙 사용 (cfn-lint-serverless) + +```bash +# 먼저 패키지 설치 +pip install cfn-lint-serverless + +# SAM 템플릿 검증 실행 +sam validate --lint --extra-lint-rules="cfn_lint_serverless.rules" +``` + +### 여러 규칙 모듈 사용 + +#### 방법 1: 콤마(,)로 구분하여 지정 + +여러 규칙 모듈을 콤마(,)로 구분하여 한 번의 옵션으로 지정할 수 있습니다: + +```bash +sam validate --lint --extra-lint-rules="module1.rules,module2.rules,module3.rules" +``` + +각 모듈은 자동으로 분리되어 cfn-lint에 전달됩니다. + +#### 방법 2: 옵션을 여러 번 사용 + +`--extra-lint-rules` 옵션을 여러 번 사용하여 여러 규칙 모듈을 지정할 수도 있습니다: + +```bash +sam validate --lint --extra-lint-rules="module1.rules" --extra-lint-rules="module2.rules" +``` + +## 참고사항 + +* 과거에 사용하던 `--serverless-rules` 옵션은 deprecated 되었습니다. +* 새로운 `--extra-lint-rules` 옵션을 사용하는 것이 좋습니다. +* 인스톨러로 SAM CLI를 설치한 경우 추가 규칙이 작동하지 않으면 인스톨러의 Python 환경에 패키지가 설치되어 있는지 확인하세요. diff --git a/samcli/commands/validate/validate.py b/samcli/commands/validate/validate.py index 1dc9ac808a..e701363161 100644 --- a/samcli/commands/validate/validate.py +++ b/samcli/commands/validate/validate.py @@ -60,10 +60,17 @@ class SamTemplate: @click.option( "--serverless-rules", is_flag=True, - help="Enable Serverless Rules for linting validation. " + help="[DEPRECATED] Enable Serverless Rules for linting validation. " "Requires the cfn-lint-serverless package to be installed. " + "Use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead. " "For more information, see: https://github.com/awslabs/serverless-rules", ) +@click.option( + "--extra-lint-rules", + help="Specify additional lint rules to be used with cfn-lint. " + "Format: module.path (e.g. 'cfn_lint_serverless.rules')", + default=None +) @save_params_option @pass_context @track_command @@ -71,13 +78,22 @@ class SamTemplate: @print_cmdline_args @unsupported_command_cdk(alternative_command="cdk doctor") @command_exception_handler -def cli(ctx, template_file, config_file, config_env, lint, save_params, serverless_rules): +def cli(ctx, template_file, config_file, config_env, lint, save_params, serverless_rules, extra_lint_rules): # All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing - do_cli(ctx, template_file, lint, serverless_rules) # pragma: no cover + # serverless_rules 옵션이 사용되면 경고 표시 및 extra_lint_rules로 변환 + if serverless_rules and not extra_lint_rules: + click.secho( + "Warning: --serverless-rules is deprecated. Please use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead.", + fg="yellow" + ) + # 이전 옵션을 새 옵션으로 변환 + extra_lint_rules = "cfn_lint_serverless.rules" + + do_cli(ctx, template_file, lint, serverless_rules, extra_lint_rules) # pragma: no cover -def do_cli(ctx, template, lint, serverless_rules): +def do_cli(ctx, template, lint, serverless_rules, extra_lint_rules=None): """ Implementation of the ``cli`` method, just separated out for unit testing purposes """ @@ -91,7 +107,7 @@ def do_cli(ctx, template, lint, serverless_rules): sam_template = _read_sam_file(template) if lint: - _lint(ctx, sam_template.serialized, template, serverless_rules) + _lint(ctx, sam_template.serialized, template, serverless_rules, extra_lint_rules) else: iam_client = boto3.client("iam") validator = SamTemplateValidator( @@ -143,7 +159,7 @@ def _read_sam_file(template) -> SamTemplate: return SamTemplate(serialized=template_string, deserialized=sam_template) -def _lint(ctx: Context, template: str, template_path: str, serverless_rules: bool) -> None: +def _lint(ctx: Context, template: str, template_path: str, serverless_rules: bool = False, extra_lint_rules: str = None) -> None: """ Parses provided SAM template and maps errors from CloudFormation template back to SAM template. @@ -166,6 +182,10 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo from cfnlint.api import ManualArgs, lint from cfnlint.runner import InvalidRegionException + from samcli.lib.telemetry.event import EventTracker + + # 디버그 정보 추가 + print(f"디버그 정보: serverless_rules 옵션 값 = {serverless_rules}") cfn_lint_logger = logging.getLogger(CNT_LINT_LOGGER_NAME) cfn_lint_logger.propagate = False @@ -179,29 +199,44 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo cfn_lint_logger.propagate = True cfn_lint_logger.setLevel(logging.DEBUG) - # Add Serverless Rules if enabled + print(f"디버그 정보: linter_config 초기값 = {linter_config}") + + # 두 옵션을 함께 처리하기 위한 변수 초기화 + rules_to_append = [] + + # 이전 serverless_rules 옵션 지원 (deprecated) if serverless_rules: - try: - # Track usage of Serverless Rules - EventTracker.track_event("UsedFeature", "ServerlessRules") + print("디버그 정보: serverless_rules 옵션이 활성화되었습니다.") + # Track usage of Serverless Rules + EventTracker.track_event("UsedFeature", "ServerlessRules") + + # Check if cfn-lint-serverless is installed + import importlib.util + serverless_spec = importlib.util.find_spec("cfn_lint_serverless") + print(f"디버그 정보: cfn_lint_serverless 패키지 설치 여부 = {serverless_spec is not None}") + + if serverless_spec is None: + print("디버그 정보: cfn_lint_serverless 패키지가 설치되어 있지 않습니다.") + click.secho( + "Serverless Rules package (cfn-lint-serverless) is not installed. " + "Please install it using: pip install cfn-lint-serverless", + fg="red", + ) + raise UserException( + "Serverless Rules package (cfn-lint-serverless) is not installed. " + "Please install it using: pip install cfn-lint-serverless" + ) - # Check if cfn-lint-serverless is installed - import importlib.util - if importlib.util.find_spec("cfn_lint_serverless") is None: - click.secho( - "Serverless Rules package (cfn-lint-serverless) is not installed. " - "Please install it using: pip install cfn-lint-serverless", - fg="red", - ) - raise UserException( - "Serverless Rules package (cfn-lint-serverless) is not installed. " - "Please install it using: pip install cfn-lint-serverless" - ) + try: + # Try to import the package + import cfn_lint_serverless + print("디버그 정보: cfn_lint_serverless 패키지 임포트 성공") - # Add Serverless Rules to the linter config - linter_config["append_rules"] = ["cfn_lint_serverless.rules"] + # Serverless Rules를 규칙 목록에 추가 + rules_to_append.append("cfn_lint_serverless.rules") click.secho("Serverless Rules enabled for linting", fg="green") - except ImportError: + except ImportError as e: + print(f"디버그 정보: cfn_lint_serverless 임포트 오류 = {e}") click.secho( "Serverless Rules package (cfn-lint-serverless) is not installed. " "Please install it using: pip install cfn-lint-serverless", @@ -211,21 +246,50 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo "Serverless Rules package (cfn-lint-serverless) is not installed. " "Please install it using: pip install cfn-lint-serverless" ) + + # 새로운 extra_lint_rules 옵션 지원 + if extra_lint_rules: + print(f"디버그 정보: extra_lint_rules 옵션이 활성화되었습니다. 값: {extra_lint_rules}") + # Track usage of Extra Lint Rules + EventTracker.track_event("UsedFeature", "ExtraLintRules") + + # 콤마로 구분된 여러 규칙 모듈을 파싱 + modules = [module.strip() for module in extra_lint_rules.split(',') if module.strip()] + print(f"디버그 정보: 파싱된 규칙 모듈 목록 = {modules}") + + # 각 모듈을 규칙 목록에 추가 + rules_to_append.extend(modules) + click.secho(f"Extra lint rules enabled: {extra_lint_rules}", fg="green") + + # 규칙이 있으면 linter_config에 추가 + if rules_to_append: + print(f"디버그 정보: 추가할 규칙 목록 = {rules_to_append}") + linter_config["append_rules"] = rules_to_append + print(f"디버그 정보: linter_config 업데이트 = {linter_config}") config = ManualArgs(**linter_config) + print(f"디버그 정보: config 생성 완료") try: + print(f"디버그 정보: lint 함수 호출 시작") matches = lint(template, config=config) + print(f"디버그 정보: lint 함수 호출 완료, matches = {matches}") except InvalidRegionException as ex: + print(f"디버그 정보: InvalidRegionException 발생 = {ex}") raise UserException( f"AWS Region was not found. Please configure your region through the --region option.\n{ex}", wrapped_from=ex.__class__.__name__, ) from ex + except Exception as e: + print(f"디버그 정보: 예외 발생 = {e}") + raise if not matches: + print(f"디버그 정보: 템플릿 검증 성공") click.secho("{} is a valid SAM Template".format(template_path), fg="green") return + print(f"디버그 정보: 템플릿 검증 실패, matches = {matches}") click.secho(matches) raise LinterRuleMatchedException("Linting failed. At least one linting rule was matched to the provided template.") diff --git a/schema/samcli.json b/schema/samcli.json index 51cf1f4751..cfa9f90dec 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -196,7 +196,7 @@ "properties": { "parameters": { "title": "Parameters for the validate command", - "description": "Available parameters for the validate command:\n* template_file:\nAWS SAM template file.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* lint:\nRun linting validation on template through cfn-lint. Create a cfnlintrc config file to specify additional parameters. For more information, see: https://github.com/aws-cloudformation/cfn-lint\n* serverless_rules:\nEnable Serverless Rules for linting validation. Requires the cfn-lint-serverless package to be installed. For more information, see: https://github.com/awslabs/serverless-rules\n* save_params:\nSave the parameters provided via the command line to the configuration file.", + "description": "Available parameters for the validate command:\n* template_file:\nAWS SAM template file.\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* lint:\nRun linting validation on template through cfn-lint. Create a cfnlintrc config file to specify additional parameters. For more information, see: https://github.com/aws-cloudformation/cfn-lint\n* serverless_rules:\n[DEPRECATED] Enable Serverless Rules for linting validation. Use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead. Requires the cfn-lint-serverless package to be installed. For more information, see: https://github.com/awslabs/serverless-rules\n* extra_lint_rules:\nSpecify additional lint rules to be used with cfn-lint. Format: module.path (e.g. 'cfn_lint_serverless.rules')\n* save_params:\nSave the parameters provided via the command line to the configuration file.", "type": "object", "properties": { "template_file": { @@ -233,7 +233,12 @@ "serverless_rules": { "title": "serverless_rules", "type": "boolean", - "description": "Enable Serverless Rules for linting validation. Requires the cfn-lint-serverless package to be installed. For more information, see: https://github.com/awslabs/serverless-rules" + "description": "[DEPRECATED] Enable Serverless Rules for linting validation. Use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead. Requires the cfn-lint-serverless package to be installed. For more information, see: https://github.com/awslabs/serverless-rules" + }, + "extra_lint_rules": { + "title": "extra_lint_rules", + "type": "string", + "description": "Specify additional lint rules to be used with cfn-lint. Format: module.path (e.g. 'cfn_lint_serverless.rules')" }, "save_params": { "title": "save_params", diff --git a/tests/unit/commands/validate/test_cli.py b/tests/unit/commands/validate/test_cli.py index 7e75818b5a..22f98ca2c5 100644 --- a/tests/unit/commands/validate/test_cli.py +++ b/tests/unit/commands/validate/test_cli.py @@ -1,19 +1,24 @@ from unittest import TestCase from unittest.mock import Mock, patch from collections import namedtuple +import warnings from botocore.exceptions import NoCredentialsError from samcli.commands.exceptions import UserException, LinterRuleMatchedException from samcli.commands.local.cli_common.user_exceptions import SamTemplateNotFoundException, InvalidSamTemplateException from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException -from samcli.commands.validate.validate import do_cli, _read_sam_file, _lint, SamTemplate +from samcli.commands.validate.validate import do_cli, _read_sam_file, _lint, SamTemplate, cli ctx_mock = namedtuple("ctx_mock", ["profile", "region"]) ctx_lint_mock = namedtuple("ctx_lint_mock", ["debug", "region"]) class TestValidateCli(TestCase): + def setUp(self): + # datetime.utcnow() 사용에 대한 경고 무시 + warnings.filterwarnings("ignore", category=DeprecationWarning, message="datetime.datetime.utcnow()") + @patch("samcli.commands.validate.validate.click") @patch("samcli.commands.validate.validate.os.path.exists") def test_file_not_found(self, path_exists_patch, click_patch): @@ -51,7 +56,7 @@ def test_template_fails_validation(self, patched_boto, read_sam_file_patch, clic template_validator.return_value = get_translated_template_if_valid_mock with self.assertRaises(InvalidSamTemplateException): - do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False) + do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False, serverless_rules=False) @patch("samcli.lib.translate.sam_template_validator.SamTemplateValidator") @patch("samcli.commands.validate.validate.click") @@ -66,7 +71,7 @@ def test_no_credentials_provided(self, patched_boto, read_sam_file_patch, click_ template_validator.return_value = get_translated_template_if_valid_mock with self.assertRaises(UserException): - do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False) + do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False, serverless_rules=False) @patch("samcli.lib.translate.sam_template_validator.SamTemplateValidator") @patch("samcli.commands.validate.validate.click") @@ -80,7 +85,7 @@ def test_template_passes_validation(self, patched_boto, read_sam_file_patch, cli get_translated_template_if_valid_mock.get_translated_template_if_valid.return_value = True template_validator.return_value = get_translated_template_if_valid_mock - do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False) + do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False, serverless_rules=False) @patch("samcli.commands.validate.validate._read_sam_file") @patch("samcli.commands.validate.validate.click") @@ -138,23 +143,30 @@ def test_serverless_rules_enabled(self, find_spec_mock, click_patch, lint_patch) find_spec_mock.return_value = True lint_patch.return_value = [] + # ManualArgs 클래스를 모킹하여 append_rules 속성이 올바르게 설정되도록 함 with patch("samcli.lib.telemetry.event.EventTracker.track_event") as track_patch: - _lint( - ctx=ctx_lint_mock(debug=False, region="region"), - template=template_contents, - template_path=template_path, - serverless_rules=True - ) - # Check that both CFNLint and ServerlessRules events are tracked - track_patch.assert_any_call("UsedFeature", "CFNLint") - track_patch.assert_any_call("UsedFeature", "ServerlessRules") - - # Check that the lint function was called with the serverless rules - lint_patch.assert_called_once() - args, kwargs = lint_patch.call_args - self.assertEqual(args[0], template_contents) - self.assertEqual(kwargs["config"].append_rules, ["cfn_lint_serverless.rules"]) - + with patch("cfnlint.api.ManualArgs") as manual_args_mock: + # ManualArgs 객체가 append_rules 속성을 가지도록 설정 + manual_args_instance = Mock() + manual_args_mock.return_value = manual_args_instance + + _lint( + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=True + ) + + # Check that both CFNLint and ServerlessRules events are tracked + track_patch.assert_any_call("UsedFeature", "CFNLint") + track_patch.assert_any_call("UsedFeature", "ServerlessRules") + + # Check that the ManualArgs was called with the serverless rules + manual_args_mock.assert_called_once() + args, kwargs = manual_args_mock.call_args + self.assertIn("append_rules", kwargs) + self.assertEqual(kwargs["append_rules"], ["cfn_lint_serverless.rules"]) + @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") @patch("importlib.util.find_spec") @@ -172,3 +184,129 @@ def test_serverless_rules_package_not_installed(self, find_spec_mock, click_patc ) self.assertIn("Serverless Rules package (cfn-lint-serverless) is not installed", ex.exception.message) + + @patch("samcli.commands.validate.validate._read_sam_file") + def test_cli_with_extra_lint_rules(self, mock_read_sam_file): + # 테스트 준비 + template = "template" + extra_lint_rules = "cfn_lint_serverless.rules" + mock_read_sam_file.return_value = SamTemplate(serialized="", deserialized={}) + + # do_cli 함수를 직접 테스트 + with patch("samcli.commands.validate.validate._lint") as mock_lint: + # do_cli 호출 + do_cli( + ctx=ctx_mock(profile="profile", region="region"), + template=template, + lint=True, + serverless_rules=False, + extra_lint_rules=extra_lint_rules + ) + + # _lint이 올바른 파라미터로 호출되는지 확인 + mock_lint.assert_called_once() + args, kwargs = mock_lint.call_args + self.assertEqual(args[2], template) # template_path 파라미터 + self.assertEqual(args[3], False) # serverless_rules 파라미터 + self.assertEqual(args[4], extra_lint_rules) # extra_lint_rules 파라미터 + + @patch("cfnlint.api.lint") + @patch("samcli.commands.validate.validate.click") + def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): + # 테스트 준비 + template_path = "path_to_template" + template_contents = "{}" + extra_lint_rules = "custom.rules.module" + lint_patch.return_value = [] + + # ManualArgs 클래스를 모킹하여 append_rules 속성이 올바르게 설정되는지 확인 + with patch("samcli.lib.telemetry.event.EventTracker.track_event") as track_patch: + with patch("cfnlint.api.ManualArgs") as manual_args_mock: + # ManualArgs 객체가 append_rules 속성을 가지도록 설정 + manual_args_instance = Mock() + manual_args_mock.return_value = manual_args_instance + + # 테스트 실행 + _lint( + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=False, + extra_lint_rules=extra_lint_rules + ) + + # 이벤트 추적 확인 - ExtraLintRules 이벤트가 추적되는지 확인 + track_patch.assert_any_call("UsedFeature", "CFNLint") + track_patch.assert_any_call("UsedFeature", "ExtraLintRules") + + # ManualArgs가 올바른 인자와 함께 호출되는지 확인 + manual_args_mock.assert_called_once() + args, kwargs = manual_args_mock.call_args + self.assertIn("append_rules", kwargs) + self.assertEqual(kwargs["append_rules"], [extra_lint_rules]) + + @patch("cfnlint.api.lint") + @patch("samcli.commands.validate.validate.click") + def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, lint_patch): + # 테스트 준비 + template_path = "path_to_template" + template_contents = "{}" + # 콤마로 구분된 여러 규칙 모듈 지정 + extra_lint_rules = "module1.rules,module2.rules,module3.rules" + lint_patch.return_value = [] + + # ManualArgs 클래스를 모킹하여 append_rules 속성이 올바르게 설정되는지 확인 + with patch("samcli.lib.telemetry.event.EventTracker.track_event"): + with patch("cfnlint.api.ManualArgs") as manual_args_mock: + manual_args_instance = Mock() + manual_args_mock.return_value = manual_args_instance + + # 테스트 실행 + _lint( + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=False, + extra_lint_rules=extra_lint_rules + ) + + # ManualArgs가 올바른 인자와 함께 호출되는지 확인 + manual_args_mock.assert_called_once() + args, kwargs = manual_args_mock.call_args + self.assertIn("append_rules", kwargs) + # 콤마로 구분된 각 모듈이 분리되어 리스트에 추가되는지 확인 + expected_rules = ["module1.rules", "module2.rules", "module3.rules"] + self.assertEqual(set(kwargs["append_rules"]), set(expected_rules)) + + @patch("cfnlint.api.lint") + @patch("samcli.commands.validate.validate.click") + @patch("importlib.util.find_spec") + def test_serverless_rules_deprecated_with_extra_lint_rules(self, find_spec_mock, click_patch, lint_patch): + # 테스트 준비 + template_path = "path_to_template" + template_contents = "{}" + find_spec_mock.return_value = True + lint_patch.return_value = [] + + # 두 옵션이 모두 제공되었을 때 extra_lint_rules가 우선되는지 확인 + with patch("samcli.lib.telemetry.event.EventTracker.track_event"): + with patch("cfnlint.api.ManualArgs") as manual_args_mock: + manual_args_instance = Mock() + manual_args_mock.return_value = manual_args_instance + + # 테스트 실행 - 두 옵션 모두 사용 + _lint( + ctx=ctx_lint_mock(debug=False, region="region"), + template=template_contents, + template_path=template_path, + serverless_rules=True, + extra_lint_rules="custom.rules.module" + ) + + # 검증 - 두 규칙이 모두 추가되는지 확인 + manual_args_mock.assert_called_once() + args, kwargs = manual_args_mock.call_args + self.assertIn("append_rules", kwargs) + self.assertEqual(len(kwargs["append_rules"]), 2) + self.assertIn("cfn_lint_serverless.rules", kwargs["append_rules"]) + self.assertIn("custom.rules.module", kwargs["append_rules"]) From cd689135c1470fb8c7954379fcb969cec717a451 Mon Sep 17 00:00:00 2001 From: shashax42 Date: Tue, 15 Apr 2025 03:10:48 +0900 Subject: [PATCH 3/4] docs: Convert Korean comments to English for better readability --- docs/extra-lint-rules.md | 44 +++++++++---------- samcli/commands/validate/validate.py | 56 ++++++++++++------------ tests/unit/commands/validate/test_cli.py | 46 +++++++++---------- 3 files changed, 73 insertions(+), 73 deletions(-) diff --git a/docs/extra-lint-rules.md b/docs/extra-lint-rules.md index 1623d18f76..d69b574ae9 100644 --- a/docs/extra-lint-rules.md +++ b/docs/extra-lint-rules.md @@ -1,55 +1,55 @@ -# SAM CLI 추가 Lint 규칙 사용 가이드 +# SAM CLI Extra Lint Rules Usage Guide -AWS SAM CLI의 `validate` 명령어는 템플릿 검증을 위해 [cfn-lint](https://github.com/aws-cloudformation/cfn-lint)를 사용합니다. -이제 SAM CLI는 `--extra-lint-rules` 옵션을 통해 추가 lint 규칙을 지원합니다. +The AWS SAM CLI's `validate` command uses [cfn-lint](https://github.com/aws-cloudformation/cfn-lint) for template validation. +SAM CLI now supports additional lint rules through the `--extra-lint-rules` option. -## 사용 방법 +## Usage ```bash sam validate --lint --extra-lint-rules="cfn_lint_serverless.rules" ``` -## 인스톨러로 SAM CLI 설치 시 고려사항 +## Considerations when Installing SAM CLI with the Installer -SAM CLI를 인스톨러(설치 프로그램)로 설치한 경우, SAM CLI는 자체 Python 환경을 사용합니다. 이 경우 추가 규칙 모듈이 해당 환경에 설치되어 있어야 합니다. 이 때 두 가지 접근 방식이 있습니다: +When SAM CLI is installed using the installer, it uses its own Python environment. In this case, additional rule modules must be installed in that environment. There are two approaches: -1. **인스톨러 Python 환경에 패키지 설치**: 인스톨러의 Python 환경에 필요한 패키지를 설치합니다. -2. **모듈 경로를 전체 경로로 지정**: 사용자 환경에 설치된 패키지의 전체 경로를 지정합니다. +1. **Install packages in the installer's Python environment**: Install the required packages in the installer's Python environment. +2. **Specify the full path to the module**: Specify the full path to the package installed in the user's environment. -## 사용 예제 +## Usage Examples -### 서버리스 규칙 사용 (cfn-lint-serverless) +### Using Serverless Rules (cfn-lint-serverless) ```bash -# 먼저 패키지 설치 +# First, install the package pip install cfn-lint-serverless -# SAM 템플릿 검증 실행 +# Run SAM template validation sam validate --lint --extra-lint-rules="cfn_lint_serverless.rules" ``` -### 여러 규칙 모듈 사용 +### Using Multiple Rule Modules -#### 방법 1: 콤마(,)로 구분하여 지정 +#### Method 1: Specify Multiple Modules Separated by Commas -여러 규칙 모듈을 콤마(,)로 구분하여 한 번의 옵션으로 지정할 수 있습니다: +You can specify multiple rule modules separated by commas in a single option: ```bash sam validate --lint --extra-lint-rules="module1.rules,module2.rules,module3.rules" ``` -각 모듈은 자동으로 분리되어 cfn-lint에 전달됩니다. +Each module is automatically separated and passed to cfn-lint. -#### 방법 2: 옵션을 여러 번 사용 +#### Method 2: Use the Option Multiple Times -`--extra-lint-rules` 옵션을 여러 번 사용하여 여러 규칙 모듈을 지정할 수도 있습니다: +You can also specify multiple rule modules by using the `--extra-lint-rules` option multiple times: ```bash sam validate --lint --extra-lint-rules="module1.rules" --extra-lint-rules="module2.rules" ``` -## 참고사항 +## Notes -* 과거에 사용하던 `--serverless-rules` 옵션은 deprecated 되었습니다. -* 새로운 `--extra-lint-rules` 옵션을 사용하는 것이 좋습니다. -* 인스톨러로 SAM CLI를 설치한 경우 추가 규칙이 작동하지 않으면 인스톨러의 Python 환경에 패키지가 설치되어 있는지 확인하세요. +* The previously used `--serverless-rules` option is deprecated. +* It is recommended to use the new `--extra-lint-rules` option. +* If you installed SAM CLI using the installer and additional rules are not working, check if the package is installed in the installer's Python environment. diff --git a/samcli/commands/validate/validate.py b/samcli/commands/validate/validate.py index e701363161..54a091d058 100644 --- a/samcli/commands/validate/validate.py +++ b/samcli/commands/validate/validate.py @@ -81,13 +81,13 @@ class SamTemplate: def cli(ctx, template_file, config_file, config_env, lint, save_params, serverless_rules, extra_lint_rules): # All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing - # serverless_rules 옵션이 사용되면 경고 표시 및 extra_lint_rules로 변환 + # Show warning and convert to extra_lint_rules if serverless_rules is used if serverless_rules and not extra_lint_rules: click.secho( "Warning: --serverless-rules is deprecated. Please use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead.", fg="yellow" ) - # 이전 옵션을 새 옵션으로 변환 + # Convert old option to new option extra_lint_rules = "cfn_lint_serverless.rules" do_cli(ctx, template_file, lint, serverless_rules, extra_lint_rules) # pragma: no cover @@ -184,8 +184,8 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo from cfnlint.runner import InvalidRegionException from samcli.lib.telemetry.event import EventTracker - # 디버그 정보 추가 - print(f"디버그 정보: serverless_rules 옵션 값 = {serverless_rules}") + # Add debug information + print(f"Debug info: serverless_rules option value = {serverless_rules}") cfn_lint_logger = logging.getLogger(CNT_LINT_LOGGER_NAME) cfn_lint_logger.propagate = False @@ -199,24 +199,24 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo cfn_lint_logger.propagate = True cfn_lint_logger.setLevel(logging.DEBUG) - print(f"디버그 정보: linter_config 초기값 = {linter_config}") + print(f"Debug info: initial linter_config = {linter_config}") - # 두 옵션을 함께 처리하기 위한 변수 초기화 + # Initialize variable to handle both options together rules_to_append = [] - # 이전 serverless_rules 옵션 지원 (deprecated) + # Support for previous serverless_rules option (deprecated) if serverless_rules: - print("디버그 정보: serverless_rules 옵션이 활성화되었습니다.") + print("Debug info: serverless_rules option is activated.") # Track usage of Serverless Rules EventTracker.track_event("UsedFeature", "ServerlessRules") # Check if cfn-lint-serverless is installed import importlib.util serverless_spec = importlib.util.find_spec("cfn_lint_serverless") - print(f"디버그 정보: cfn_lint_serverless 패키지 설치 여부 = {serverless_spec is not None}") + print(f"Debug info: cfn_lint_serverless package installed = {serverless_spec is not None}") if serverless_spec is None: - print("디버그 정보: cfn_lint_serverless 패키지가 설치되어 있지 않습니다.") + print("Debug info: cfn_lint_serverless package is not installed.") click.secho( "Serverless Rules package (cfn-lint-serverless) is not installed. " "Please install it using: pip install cfn-lint-serverless", @@ -230,13 +230,13 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo try: # Try to import the package import cfn_lint_serverless - print("디버그 정보: cfn_lint_serverless 패키지 임포트 성공") + print("Debug info: cfn_lint_serverless package import successful") - # Serverless Rules를 규칙 목록에 추가 + # Add Serverless Rules to the rule list rules_to_append.append("cfn_lint_serverless.rules") click.secho("Serverless Rules enabled for linting", fg="green") except ImportError as e: - print(f"디버그 정보: cfn_lint_serverless 임포트 오류 = {e}") + print(f"Debug info: cfn_lint_serverless import error = {e}") click.secho( "Serverless Rules package (cfn-lint-serverless) is not installed. " "Please install it using: pip install cfn-lint-serverless", @@ -247,49 +247,49 @@ def _lint(ctx: Context, template: str, template_path: str, serverless_rules: boo "Please install it using: pip install cfn-lint-serverless" ) - # 새로운 extra_lint_rules 옵션 지원 + # Support for the new extra_lint_rules option if extra_lint_rules: - print(f"디버그 정보: extra_lint_rules 옵션이 활성화되었습니다. 값: {extra_lint_rules}") + print(f"Debug info: extra_lint_rules option is activated. Value: {extra_lint_rules}") # Track usage of Extra Lint Rules EventTracker.track_event("UsedFeature", "ExtraLintRules") - # 콤마로 구분된 여러 규칙 모듈을 파싱 + # Parse comma-separated rule modules modules = [module.strip() for module in extra_lint_rules.split(',') if module.strip()] - print(f"디버그 정보: 파싱된 규칙 모듈 목록 = {modules}") + print(f"Debug info: parsed rule modules list = {modules}") - # 각 모듈을 규칙 목록에 추가 + # Add each module to the rule list rules_to_append.extend(modules) click.secho(f"Extra lint rules enabled: {extra_lint_rules}", fg="green") - # 규칙이 있으면 linter_config에 추가 + # Add rules to linter_config if any exist if rules_to_append: - print(f"디버그 정보: 추가할 규칙 목록 = {rules_to_append}") + print(f"Debug info: rules to append = {rules_to_append}") linter_config["append_rules"] = rules_to_append - print(f"디버그 정보: linter_config 업데이트 = {linter_config}") + print(f"Debug info: updated linter_config = {linter_config}") config = ManualArgs(**linter_config) - print(f"디버그 정보: config 생성 완료") + print(f"Debug info: config creation completed") try: - print(f"디버그 정보: lint 함수 호출 시작") + print(f"Debug info: starting lint function call") matches = lint(template, config=config) - print(f"디버그 정보: lint 함수 호출 완료, matches = {matches}") + print(f"Debug info: lint function call completed, matches = {matches}") except InvalidRegionException as ex: - print(f"디버그 정보: InvalidRegionException 발생 = {ex}") + print(f"Debug info: InvalidRegionException occurred = {ex}") raise UserException( f"AWS Region was not found. Please configure your region through the --region option.\n{ex}", wrapped_from=ex.__class__.__name__, ) from ex except Exception as e: - print(f"디버그 정보: 예외 발생 = {e}") + print(f"Debug info: exception occurred = {e}") raise if not matches: - print(f"디버그 정보: 템플릿 검증 성공") + print(f"Debug info: template validation successful") click.secho("{} is a valid SAM Template".format(template_path), fg="green") return - print(f"디버그 정보: 템플릿 검증 실패, matches = {matches}") + print(f"Debug info: template validation failed, matches = {matches}") click.secho(matches) raise LinterRuleMatchedException("Linting failed. At least one linting rule was matched to the provided template.") diff --git a/tests/unit/commands/validate/test_cli.py b/tests/unit/commands/validate/test_cli.py index 22f98ca2c5..c6fc09f8d9 100644 --- a/tests/unit/commands/validate/test_cli.py +++ b/tests/unit/commands/validate/test_cli.py @@ -187,14 +187,14 @@ def test_serverless_rules_package_not_installed(self, find_spec_mock, click_patc @patch("samcli.commands.validate.validate._read_sam_file") def test_cli_with_extra_lint_rules(self, mock_read_sam_file): - # 테스트 준비 + # Prepare test template = "template" extra_lint_rules = "cfn_lint_serverless.rules" mock_read_sam_file.return_value = SamTemplate(serialized="", deserialized={}) - # do_cli 함수를 직접 테스트 + # Test the do_cli function directly with patch("samcli.commands.validate.validate._lint") as mock_lint: - # do_cli 호출 + # Call do_cli do_cli( ctx=ctx_mock(profile="profile", region="region"), template=template, @@ -203,30 +203,30 @@ def test_cli_with_extra_lint_rules(self, mock_read_sam_file): extra_lint_rules=extra_lint_rules ) - # _lint이 올바른 파라미터로 호출되는지 확인 + # Verify that _lint is called with the correct parameters mock_lint.assert_called_once() args, kwargs = mock_lint.call_args - self.assertEqual(args[2], template) # template_path 파라미터 - self.assertEqual(args[3], False) # serverless_rules 파라미터 - self.assertEqual(args[4], extra_lint_rules) # extra_lint_rules 파라미터 + self.assertEqual(args[2], template) # template_path parameter + self.assertEqual(args[3], False) # serverless_rules parameter + self.assertEqual(args[4], extra_lint_rules) # extra_lint_rules parameter @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): - # 테스트 준비 + # Prepare test template_path = "path_to_template" template_contents = "{}" extra_lint_rules = "custom.rules.module" lint_patch.return_value = [] - # ManualArgs 클래스를 모킹하여 append_rules 속성이 올바르게 설정되는지 확인 + # Mock ManualArgs class to verify that append_rules property is set correctly with patch("samcli.lib.telemetry.event.EventTracker.track_event") as track_patch: with patch("cfnlint.api.ManualArgs") as manual_args_mock: - # ManualArgs 객체가 append_rules 속성을 가지도록 설정 + # Set up ManualArgs object to have append_rules property manual_args_instance = Mock() manual_args_mock.return_value = manual_args_instance - # 테스트 실행 + # Run test _lint( ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, @@ -235,11 +235,11 @@ def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): extra_lint_rules=extra_lint_rules ) - # 이벤트 추적 확인 - ExtraLintRules 이벤트가 추적되는지 확인 + # Verify event tracking - confirm ExtraLintRules event is being tracked track_patch.assert_any_call("UsedFeature", "CFNLint") track_patch.assert_any_call("UsedFeature", "ExtraLintRules") - # ManualArgs가 올바른 인자와 함께 호출되는지 확인 + # Verify ManualArgs is called with correct arguments manual_args_mock.assert_called_once() args, kwargs = manual_args_mock.call_args self.assertIn("append_rules", kwargs) @@ -248,20 +248,20 @@ def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, lint_patch): - # 테스트 준비 + # Prepare test template_path = "path_to_template" template_contents = "{}" - # 콤마로 구분된 여러 규칙 모듈 지정 + # Specify multiple rule modules separated by commas extra_lint_rules = "module1.rules,module2.rules,module3.rules" lint_patch.return_value = [] - # ManualArgs 클래스를 모킹하여 append_rules 속성이 올바르게 설정되는지 확인 + # Mock ManualArgs class to verify that append_rules property is set correctly with patch("samcli.lib.telemetry.event.EventTracker.track_event"): with patch("cfnlint.api.ManualArgs") as manual_args_mock: manual_args_instance = Mock() manual_args_mock.return_value = manual_args_instance - # 테스트 실행 + # Run test _lint( ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, @@ -270,11 +270,11 @@ def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, extra_lint_rules=extra_lint_rules ) - # ManualArgs가 올바른 인자와 함께 호출되는지 확인 + # Verify ManualArgs is called with correct arguments manual_args_mock.assert_called_once() args, kwargs = manual_args_mock.call_args self.assertIn("append_rules", kwargs) - # 콤마로 구분된 각 모듈이 분리되어 리스트에 추가되는지 확인 + # Verify each comma-separated module is split and added to the list expected_rules = ["module1.rules", "module2.rules", "module3.rules"] self.assertEqual(set(kwargs["append_rules"]), set(expected_rules)) @@ -282,19 +282,19 @@ def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, @patch("samcli.commands.validate.validate.click") @patch("importlib.util.find_spec") def test_serverless_rules_deprecated_with_extra_lint_rules(self, find_spec_mock, click_patch, lint_patch): - # 테스트 준비 + # Prepare test template_path = "path_to_template" template_contents = "{}" find_spec_mock.return_value = True lint_patch.return_value = [] - # 두 옵션이 모두 제공되었을 때 extra_lint_rules가 우선되는지 확인 + # Verify when both options are provided, both rules are included with patch("samcli.lib.telemetry.event.EventTracker.track_event"): with patch("cfnlint.api.ManualArgs") as manual_args_mock: manual_args_instance = Mock() manual_args_mock.return_value = manual_args_instance - # 테스트 실행 - 두 옵션 모두 사용 + # Run test - use both options _lint( ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, @@ -303,7 +303,7 @@ def test_serverless_rules_deprecated_with_extra_lint_rules(self, find_spec_mock, extra_lint_rules="custom.rules.module" ) - # 검증 - 두 규칙이 모두 추가되는지 확인 + # Verify both rules are added manual_args_mock.assert_called_once() args, kwargs = manual_args_mock.call_args self.assertIn("append_rules", kwargs) From f75e51dd717b24474cd0156d0d4f9c67290318be Mon Sep 17 00:00:00 2001 From: shashax42 Date: Wed, 7 May 2025 23:42:16 +0900 Subject: [PATCH 4/4] refactor: improve extra-lint-rules feature based on review feedback --- samcli/commands/validate/validate.py | 273 +++++++++++++---------- tests/unit/commands/validate/test_cli.py | 113 ++-------- 2 files changed, 168 insertions(+), 218 deletions(-) diff --git a/samcli/commands/validate/validate.py b/samcli/commands/validate/validate.py index 54a091d058..551145f0dc 100644 --- a/samcli/commands/validate/validate.py +++ b/samcli/commands/validate/validate.py @@ -57,19 +57,13 @@ class SamTemplate: "Create a cfnlintrc config file to specify additional parameters. " "For more information, see: https://github.com/aws-cloudformation/cfn-lint", ) -@click.option( - "--serverless-rules", - is_flag=True, - help="[DEPRECATED] Enable Serverless Rules for linting validation. " - "Requires the cfn-lint-serverless package to be installed. " - "Use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead. " - "For more information, see: https://github.com/awslabs/serverless-rules", -) @click.option( "--extra-lint-rules", help="Specify additional lint rules to be used with cfn-lint. " - "Format: module.path (e.g. 'cfn_lint_serverless.rules')", - default=None + "Format: module.path (e.g. 'cfn_lint_serverless.rules'). " + "Multiple rule modules can be specified by separating with commas or using this option multiple times.", + default=None, + multiple=True ) @save_params_option @pass_context @@ -78,22 +72,12 @@ class SamTemplate: @print_cmdline_args @unsupported_command_cdk(alternative_command="cdk doctor") @command_exception_handler -def cli(ctx, template_file, config_file, config_env, lint, save_params, serverless_rules, extra_lint_rules): +def cli(ctx, template_file, config_file, config_env, lint, save_params, extra_lint_rules): # All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing - - # Show warning and convert to extra_lint_rules if serverless_rules is used - if serverless_rules and not extra_lint_rules: - click.secho( - "Warning: --serverless-rules is deprecated. Please use --extra-lint-rules=\"cfn_lint_serverless.rules\" instead.", - fg="yellow" - ) - # Convert old option to new option - extra_lint_rules = "cfn_lint_serverless.rules" - - do_cli(ctx, template_file, lint, serverless_rules, extra_lint_rules) # pragma: no cover + do_cli(ctx, template_file, lint, extra_lint_rules) # pragma: no cover -def do_cli(ctx, template, lint, serverless_rules, extra_lint_rules=None): +def do_cli(ctx, template, lint, extra_lint_rules=None): """ Implementation of the ``cli`` method, just separated out for unit testing purposes """ @@ -107,7 +91,7 @@ def do_cli(ctx, template, lint, serverless_rules, extra_lint_rules=None): sam_template = _read_sam_file(template) if lint: - _lint(ctx, sam_template.serialized, template, serverless_rules, extra_lint_rules) + _lint(ctx, sam_template.serialized, template, extra_lint_rules) else: iam_client = boto3.client("iam") validator = SamTemplateValidator( @@ -159,137 +143,190 @@ def _read_sam_file(template) -> SamTemplate: return SamTemplate(serialized=template_string, deserialized=sam_template) -def _lint(ctx: Context, template: str, template_path: str, serverless_rules: bool = False, extra_lint_rules: str = None) -> None: +def _lint(ctx: Context, template: str, template_path: str, extra_lint_rules=None): """ Parses provided SAM template and maps errors from CloudFormation template back to SAM template. Cfn-lint loggers are added to the SAM cli logging hierarchy which at the root logger - configures with INFO level logging and a different formatting. This exposes and duplicates - some cfn-lint logs that are not typically shown to customers. Explicitly setting the level to - WARNING and propagate to be False remediates these issues. + formatter and handlers are defined. This ensures that logging is output correctly when used from SAM cli + for CLI consumers. Parameters - ----------- + ---------- ctx - Click context object + Click Context template - Contents of sam template as a string + SAM template contents template_path Path to the sam template - serverless_rules - Flag to enable Serverless Rules for linting + extra_lint_rules + List of additional rule modules to apply """ + import logging + import importlib.util + import cfnlint - from cfnlint.api import ManualArgs, lint + from cfnlint.api import lint, ManualArgs from cfnlint.runner import InvalidRegionException + # Import only what is necessary + + # To track events, we need to enable telemetry from samcli.lib.telemetry.event import EventTracker - # Add debug information - print(f"Debug info: serverless_rules option value = {serverless_rules}") + LOG = logging.getLogger(__name__) + LOG.debug("Starting template validation with linting") - cfn_lint_logger = logging.getLogger(CNT_LINT_LOGGER_NAME) - cfn_lint_logger.propagate = False + # Set up cfnlint logger verbosity using context provided + cfnlint_logger = logging.getLogger(CNT_LINT_LOGGER_NAME) + cfnlint_logger.propagate = False + + if ctx and ctx.debug: + cfnlint_logger.propagate = True + cfnlint_logger.setLevel(logging.DEBUG) + else: + cfnlint_logger.setLevel(logging.INFO) + # Track linting in telemetry EventTracker.track_event("UsedFeature", "CFNLint") - + + # Create linter configuration linter_config = {} if ctx.region: linter_config["regions"] = [ctx.region] - if ctx.debug: - cfn_lint_logger.propagate = True - cfn_lint_logger.setLevel(logging.DEBUG) - - print(f"Debug info: initial linter_config = {linter_config}") - - # Initialize variable to handle both options together - rules_to_append = [] - # Support for previous serverless_rules option (deprecated) - if serverless_rules: - print("Debug info: serverless_rules option is activated.") - # Track usage of Serverless Rules - EventTracker.track_event("UsedFeature", "ServerlessRules") - - # Check if cfn-lint-serverless is installed - import importlib.util - serverless_spec = importlib.util.find_spec("cfn_lint_serverless") - print(f"Debug info: cfn_lint_serverless package installed = {serverless_spec is not None}") - - if serverless_spec is None: - print("Debug info: cfn_lint_serverless package is not installed.") - click.secho( - "Serverless Rules package (cfn-lint-serverless) is not installed. " - "Please install it using: pip install cfn-lint-serverless", - fg="red", - ) - raise UserException( - "Serverless Rules package (cfn-lint-serverless) is not installed. " - "Please install it using: pip install cfn-lint-serverless" - ) - - try: - # Try to import the package - import cfn_lint_serverless - print("Debug info: cfn_lint_serverless package import successful") - - # Add Serverless Rules to the rule list - rules_to_append.append("cfn_lint_serverless.rules") - click.secho("Serverless Rules enabled for linting", fg="green") - except ImportError as e: - print(f"Debug info: cfn_lint_serverless import error = {e}") - click.secho( - "Serverless Rules package (cfn-lint-serverless) is not installed. " - "Please install it using: pip install cfn-lint-serverless", - fg="red", - ) - raise UserException( - "Serverless Rules package (cfn-lint-serverless) is not installed. " - "Please install it using: pip install cfn-lint-serverless" - ) - - # Support for the new extra_lint_rules option + # Process extra lint rules if provided + rules_to_append = [] if extra_lint_rules: - print(f"Debug info: extra_lint_rules option is activated. Value: {extra_lint_rules}") # Track usage of Extra Lint Rules EventTracker.track_event("UsedFeature", "ExtraLintRules") - # Parse comma-separated rule modules - modules = [module.strip() for module in extra_lint_rules.split(',') if module.strip()] - print(f"Debug info: parsed rule modules list = {modules}") + # Process each rule option (multiple=True gives us a list) + for rule_option in extra_lint_rules: + # Handle comma-separated rule modules + for module in rule_option.split(','): + module = module.strip() + if not module: + continue + + LOG.debug("Processing lint rule module: %s", module) + if _is_module_available(module): + rules_to_append.append(module) + LOG.debug("Module %s is available and will be used", module) + else: + module_name = module.split('.')[0].replace('_', '-') + _handle_missing_module(module_name, + f"The rule module '{module}' was specified but is not available.", + ctx.debug) - # Add each module to the rule list - rules_to_append.extend(modules) - click.secho(f"Extra lint rules enabled: {extra_lint_rules}", fg="green") - - # Add rules to linter_config if any exist - if rules_to_append: - print(f"Debug info: rules to append = {rules_to_append}") - linter_config["append_rules"] = rules_to_append - print(f"Debug info: updated linter_config = {linter_config}") - - config = ManualArgs(**linter_config) - print(f"Debug info: config creation completed") + if rules_to_append: + module_names = ', '.join(rules_to_append) + click.secho(f"Extra lint rules enabled: {module_names}", fg="green") + linter_config["append_rules"] = rules_to_append + LOG.debug("Linter configuration updated with rules: %s", rules_to_append) try: - print(f"Debug info: starting lint function call") + # Create linter configuration and execute linting + config = ManualArgs(**linter_config) + LOG.debug("Executing linting with configuration") matches = lint(template, config=config) - print(f"Debug info: lint function call completed, matches = {matches}") + + if not matches: + click.secho("{} is a valid SAM Template".format(template_path), fg="green") + return + + # Display validation failures + click.secho(matches) + raise LinterRuleMatchedException("Linting failed. At least one linting rule was matched to the provided template.") + except InvalidRegionException as ex: - print(f"Debug info: InvalidRegionException occurred = {ex}") + LOG.debug("Region validation failed: %s", ex) raise UserException( f"AWS Region was not found. Please configure your region through the --region option.\n{ex}", wrapped_from=ex.__class__.__name__, ) from ex except Exception as e: - print(f"Debug info: exception occurred = {e}") + LOG.debug("Unexpected exception during linting: %s", e) raise - if not matches: - print(f"Debug info: template validation successful") - click.secho("{} is a valid SAM Template".format(template_path), fg="green") - return - print(f"Debug info: template validation failed, matches = {matches}") - click.secho(matches) +def _is_module_available(module_path: str) -> bool: + """ + Check if a module is available for import. + Works with both standard pip installations and installer-based SAM CLI. + + Parameters + ---------- + module_path + Full module path (e.g. 'cfn_lint_serverless.rules') + + Returns + ------- + bool + True if module can be imported, False otherwise + """ + LOG = logging.getLogger(__name__) + + # Try using importlib.util which is safer + try: + root_module = module_path.split('.')[0] + spec = importlib.util.find_spec(root_module) + if spec is None: + LOG.debug("Module %s not found with importlib.util.find_spec", root_module) + return False + + # For deeper paths, try actually importing + try: + __import__(module_path) + return True + except (ImportError, ModuleNotFoundError) as e: + LOG.debug("Could not import module %s: %s", module_path, e) + return False + except Exception as e: + LOG.debug("Unexpected error checking for module %s: %s", module_path, e) + # Fallback to direct import attempt + try: + __import__(module_path) + return True + except (ImportError, ModuleNotFoundError): + return False + - raise LinterRuleMatchedException("Linting failed. At least one linting rule was matched to the provided template.") +def _handle_missing_module(package_name: str, error_context: str, debug_mode: bool = False): + """ + Handle missing module by providing appropriate error message that works + in both pip and installer environments. + + Parameters + ---------- + package_name + Name of the package (for pip install instructions) + error_context + Contextual message describing what feature requires this package + debug_mode + Whether to include detailed instructions for different install methods + + Raises + ------ + UserException + With appropriate error message + """ + LOG = logging.getLogger(__name__) + LOG.debug("Module %s is missing: %s", package_name, error_context) + + base_message = error_context + install_instruction = f"Please install it using: pip install {package_name}" + + if debug_mode: + # In debug mode, provide more comprehensive instructions + message = ( + f"{base_message}\n\n" + f"The package '{package_name}' is not available. Installation options:\n" + f"1. If using pip-installed SAM CLI: {install_instruction}\n" + f"2. If using installer-based SAM CLI: You need to install the package in the same Python environment\n" + f" that SAM CLI uses. Check the SAM CLI installation documentation for details." + ) + else: + message = f"{base_message}\n\n{package_name} package is not installed. {install_instruction}" + + click.secho(message, fg="red") + raise UserException(message) diff --git a/tests/unit/commands/validate/test_cli.py b/tests/unit/commands/validate/test_cli.py index c6fc09f8d9..b3a8727f2d 100644 --- a/tests/unit/commands/validate/test_cli.py +++ b/tests/unit/commands/validate/test_cli.py @@ -16,7 +16,7 @@ class TestValidateCli(TestCase): def setUp(self): - # datetime.utcnow() 사용에 대한 경고 무시 + # Ignore warnings about datetime.utcnow() usage warnings.filterwarnings("ignore", category=DeprecationWarning, message="datetime.datetime.utcnow()") @patch("samcli.commands.validate.validate.click") @@ -56,7 +56,7 @@ def test_template_fails_validation(self, patched_boto, read_sam_file_patch, clic template_validator.return_value = get_translated_template_if_valid_mock with self.assertRaises(InvalidSamTemplateException): - do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False, serverless_rules=False) + do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False) @patch("samcli.lib.translate.sam_template_validator.SamTemplateValidator") @patch("samcli.commands.validate.validate.click") @@ -71,7 +71,7 @@ def test_no_credentials_provided(self, patched_boto, read_sam_file_patch, click_ template_validator.return_value = get_translated_template_if_valid_mock with self.assertRaises(UserException): - do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False, serverless_rules=False) + do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path, lint=False) @patch("samcli.lib.translate.sam_template_validator.SamTemplateValidator") @patch("samcli.commands.validate.validate.click") @@ -96,7 +96,7 @@ def test_lint_template_passes(self, lint_patch, click_patch, read_sam_file_patch read_sam_file_patch.return_value = SamTemplate(serialized="{}", deserialized={}) lint_patch.return_value = True - do_cli(ctx=ctx_lint_mock(debug=False, region="region"), template=template_path, lint=True, serverless_rules=False) + do_cli(ctx=ctx_lint_mock(debug=False, region="region"), template=template_path, lint=True) @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") @@ -109,8 +109,7 @@ def test_lint_event_recorded(self, click_patch, lint_patch): _lint( ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, - template_path=template_path, - serverless_rules=False + template_path=template_path ) track_patch.assert_called_with("UsedFeature", "CFNLint") @@ -126,70 +125,18 @@ def test_linter_raises_exception_if_matches_found(self, click_patch, lint_patch) _lint( ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, - template_path=template_path, - serverless_rules=False + template_path=template_path ) self.assertEqual( ex.exception.message, "Linting failed. At least one linting rule was matched to the provided template." ) - @patch("cfnlint.api.lint") - @patch("samcli.commands.validate.validate.click") - @patch("importlib.util.find_spec") - def test_serverless_rules_enabled(self, find_spec_mock, click_patch, lint_patch): - template_path = "path_to_template" - template_contents = "{}" - find_spec_mock.return_value = True - lint_patch.return_value = [] - - # ManualArgs 클래스를 모킹하여 append_rules 속성이 올바르게 설정되도록 함 - with patch("samcli.lib.telemetry.event.EventTracker.track_event") as track_patch: - with patch("cfnlint.api.ManualArgs") as manual_args_mock: - # ManualArgs 객체가 append_rules 속성을 가지도록 설정 - manual_args_instance = Mock() - manual_args_mock.return_value = manual_args_instance - - _lint( - ctx=ctx_lint_mock(debug=False, region="region"), - template=template_contents, - template_path=template_path, - serverless_rules=True - ) - - # Check that both CFNLint and ServerlessRules events are tracked - track_patch.assert_any_call("UsedFeature", "CFNLint") - track_patch.assert_any_call("UsedFeature", "ServerlessRules") - - # Check that the ManualArgs was called with the serverless rules - manual_args_mock.assert_called_once() - args, kwargs = manual_args_mock.call_args - self.assertIn("append_rules", kwargs) - self.assertEqual(kwargs["append_rules"], ["cfn_lint_serverless.rules"]) - - @patch("cfnlint.api.lint") - @patch("samcli.commands.validate.validate.click") - @patch("importlib.util.find_spec") - def test_serverless_rules_package_not_installed(self, find_spec_mock, click_patch, lint_patch): - template_path = "path_to_template" - template_contents = "{}" - find_spec_mock.return_value = None - - with self.assertRaises(UserException) as ex: - _lint( - ctx=ctx_lint_mock(debug=False, region="region"), - template=template_contents, - template_path=template_path, - serverless_rules=True - ) - - self.assertIn("Serverless Rules package (cfn-lint-serverless) is not installed", ex.exception.message) - @patch("samcli.commands.validate.validate._read_sam_file") def test_cli_with_extra_lint_rules(self, mock_read_sam_file): # Prepare test template = "template" - extra_lint_rules = "cfn_lint_serverless.rules" + extra_lint_rules = ("cfn_lint_serverless.rules",) # 튜플 형태로 변경 mock_read_sam_file.return_value = SamTemplate(serialized="", deserialized={}) # Test the do_cli function directly @@ -199,7 +146,6 @@ def test_cli_with_extra_lint_rules(self, mock_read_sam_file): ctx=ctx_mock(profile="profile", region="region"), template=template, lint=True, - serverless_rules=False, extra_lint_rules=extra_lint_rules ) @@ -207,8 +153,7 @@ def test_cli_with_extra_lint_rules(self, mock_read_sam_file): mock_lint.assert_called_once() args, kwargs = mock_lint.call_args self.assertEqual(args[2], template) # template_path parameter - self.assertEqual(args[3], False) # serverless_rules parameter - self.assertEqual(args[4], extra_lint_rules) # extra_lint_rules parameter + self.assertEqual(args[3], extra_lint_rules) # extra_lint_rules parameter @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") @@ -216,7 +161,7 @@ def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): # Prepare test template_path = "path_to_template" template_contents = "{}" - extra_lint_rules = "custom.rules.module" + extra_lint_rules = ("custom.rules.module",) # Tuple format for multiple=True lint_patch.return_value = [] # Mock ManualArgs class to verify that append_rules property is set correctly @@ -231,7 +176,6 @@ def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, template_path=template_path, - serverless_rules=False, extra_lint_rules=extra_lint_rules ) @@ -243,7 +187,8 @@ def test_lint_with_extra_lint_rules(self, click_patch, lint_patch): manual_args_mock.assert_called_once() args, kwargs = manual_args_mock.call_args self.assertIn("append_rules", kwargs) - self.assertEqual(kwargs["append_rules"], [extra_lint_rules]) + # We expect the first item from the tuple to be processed + self.assertEqual(kwargs["append_rules"], ["custom.rules.module"]) @patch("cfnlint.api.lint") @patch("samcli.commands.validate.validate.click") @@ -252,7 +197,7 @@ def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, template_path = "path_to_template" template_contents = "{}" # Specify multiple rule modules separated by commas - extra_lint_rules = "module1.rules,module2.rules,module3.rules" + extra_lint_rules = ("module1.rules,module2.rules,module3.rules",) lint_patch.return_value = [] # Mock ManualArgs class to verify that append_rules property is set correctly @@ -266,7 +211,6 @@ def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, ctx=ctx_lint_mock(debug=False, region="region"), template=template_contents, template_path=template_path, - serverless_rules=False, extra_lint_rules=extra_lint_rules ) @@ -278,35 +222,4 @@ def test_lint_with_multiple_comma_separated_extra_lint_rules(self, click_patch, expected_rules = ["module1.rules", "module2.rules", "module3.rules"] self.assertEqual(set(kwargs["append_rules"]), set(expected_rules)) - @patch("cfnlint.api.lint") - @patch("samcli.commands.validate.validate.click") - @patch("importlib.util.find_spec") - def test_serverless_rules_deprecated_with_extra_lint_rules(self, find_spec_mock, click_patch, lint_patch): - # Prepare test - template_path = "path_to_template" - template_contents = "{}" - find_spec_mock.return_value = True - lint_patch.return_value = [] - - # Verify when both options are provided, both rules are included - with patch("samcli.lib.telemetry.event.EventTracker.track_event"): - with patch("cfnlint.api.ManualArgs") as manual_args_mock: - manual_args_instance = Mock() - manual_args_mock.return_value = manual_args_instance - - # Run test - use both options - _lint( - ctx=ctx_lint_mock(debug=False, region="region"), - template=template_contents, - template_path=template_path, - serverless_rules=True, - extra_lint_rules="custom.rules.module" - ) - - # Verify both rules are added - manual_args_mock.assert_called_once() - args, kwargs = manual_args_mock.call_args - self.assertIn("append_rules", kwargs) - self.assertEqual(len(kwargs["append_rules"]), 2) - self.assertIn("cfn_lint_serverless.rules", kwargs["append_rules"]) - self.assertIn("custom.rules.module", kwargs["append_rules"]) + # Removed test_serverless_rules_deprecated_with_extra_lint_rules as the feature is no longer supported