diff --git a/docs/extra-lint-rules.md b/docs/extra-lint-rules.md new file mode 100644 index 0000000000..d69b574ae9 --- /dev/null +++ b/docs/extra-lint-rules.md @@ -0,0 +1,55 @@ +# SAM CLI Extra Lint Rules Usage Guide + +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" +``` + +## Considerations when Installing SAM CLI with the Installer + +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. **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 + +### Using Serverless Rules (cfn-lint-serverless) + +```bash +# First, install the package +pip install cfn-lint-serverless + +# Run SAM template validation +sam validate --lint --extra-lint-rules="cfn_lint_serverless.rules" +``` + +### Using Multiple Rule Modules + +#### 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" +``` + +Each module is automatically separated and passed to cfn-lint. + +#### Method 2: Use the Option Multiple Times + +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 + +* 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 cf317b8bfe..551145f0dc 100644 --- a/samcli/commands/validate/validate.py +++ b/samcli/commands/validate/validate.py @@ -57,6 +57,14 @@ class SamTemplate: "Create a cfnlintrc config file to specify additional parameters. " "For more information, see: https://github.com/aws-cloudformation/cfn-lint", ) +@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'). " + "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 @track_command @@ -64,13 +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): +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 - - do_cli(ctx, template_file, lint) # pragma: no cover + do_cli(ctx, template_file, lint, extra_lint_rules) # pragma: no cover -def do_cli(ctx, template, lint): +def do_cli(ctx, template, lint, extra_lint_rules=None): """ 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, extra_lint_rules) else: iam_client = boto3.client("iam") validator = SamTemplateValidator( @@ -136,54 +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) -> 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 + 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 - cfn_lint_logger = logging.getLogger(CNT_LINT_LOGGER_NAME) - cfn_lint_logger.propagate = False + # To track events, we need to enable telemetry + from samcli.lib.telemetry.event import EventTracker - EventTracker.track_event("UsedFeature", "CFNLint") + LOG = logging.getLogger(__name__) + LOG.debug("Starting template validation with linting") + + # 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) - - config = ManualArgs(**linter_config) + + # Process extra lint rules if provided + rules_to_append = [] + if extra_lint_rules: + # Track usage of Extra Lint Rules + EventTracker.track_event("UsedFeature", "ExtraLintRules") + + # 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) + + 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: + # Create linter configuration and execute linting + config = ManualArgs(**linter_config) + LOG.debug("Executing linting with configuration") matches = lint(template, config=config) + + 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: + 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: + LOG.debug("Unexpected exception during linting: %s", e) + raise - if not matches: - click.secho("{} is a valid SAM Template".format(template_path), fg="green") - return - 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/schema/samcli.json b/schema/samcli.json index 25cad510ce..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* 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": { @@ -230,6 +230,16 @@ "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": "[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", "type": "boolean", diff --git a/tests/unit/commands/validate/test_cli.py b/tests/unit/commands/validate/test_cli.py index f77a4711e9..b3a8727f2d 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): + # Ignore warnings about datetime.utcnow() usage + 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): @@ -80,12 +85,12 @@ 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") @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={}) @@ -104,7 +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, + template_path=template_path ) track_patch.assert_called_with("UsedFeature", "CFNLint") @@ -118,9 +123,103 @@ 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 ) self.assertEqual( ex.exception.message, "Linting failed. At least one linting rule was matched to the provided template." ) + + @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={}) + + # Test the do_cli function directly + with patch("samcli.commands.validate.validate._lint") as mock_lint: + # Call do_cli + do_cli( + ctx=ctx_mock(profile="profile", region="region"), + template=template, + lint=True, + extra_lint_rules=extra_lint_rules + ) + + # 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 parameter + self.assertEqual(args[3], 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",) # Tuple format for multiple=True + lint_patch.return_value = [] + + # 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: + # 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, + template_path=template_path, + extra_lint_rules=extra_lint_rules + ) + + # Verify event tracking - confirm ExtraLintRules event is being tracked + track_patch.assert_any_call("UsedFeature", "CFNLint") + track_patch.assert_any_call("UsedFeature", "ExtraLintRules") + + # 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) + # 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") + 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 = [] + + # 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, + template_path=template_path, + extra_lint_rules=extra_lint_rules + ) + + # 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)) + + # Removed test_serverless_rules_deprecated_with_extra_lint_rules as the feature is no longer supported