feat: add --serverless-rules to sam validate --lint #7950

shashax42 · 2025-03-24T16:21:35Z

Which issue(s) does this change fix?

This PR adds a new feature and is not associated with a specific issue number.

Why is this change necessary?

The AWS SAM CLI's validate --lint command is useful for validating CloudFormation templates, but currently lacks the ability to apply rules specific to Serverless applications. Serverless applications have different characteristics compared to general CloudFormation templates, requiring additional validation rules tailored to their specific needs.

How does it address the issue?

This PR adds a --serverless-rules option to the sam validate --lint command, allowing users to leverage rules from the cfn-lint-serverless package. This enables users to perform additional validations specific to Serverless applications. When enabled, this option validates Serverless application best practices such as Lambda function memory size, timeout, log retention, API Gateway stage logging, and throttling settings.

What side effects does this change have?

This change does not affect existing functionality. The --serverless-rules option is only activated when explicitly specified.
If the cfn-lint-serverless package is not installed, users receive a clear error message prompting them to install the package.
When running in debug mode, additional information about Serverless Rules application and related settings is displayed.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

Add input/output type hints to new functions/methods
Write design document if needed (Do I need to write a design document?)
Write/update unit tests
Write/update integration tests
Write/update functional tests if needed
make pr passes
make update-reproducible-reqs if dependencies were changed
Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

vicheey · 2025-03-26T17:31:08Z

samcli/commands/validate/validate.py

@@ -57,20 +57,27 @@ 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,


Any particular region to enable this flag by default?

@vicheey No, there's no particular region requirement. You can enable this flag by default in all regions.

valerena · 2025-04-07T19:13:53Z

Two comments.

One thing to check here is if this works with SAM CLI installed without pip (like installed using the actual installer). I feel like that won't work (because SAM CLI uses their own Python environment, so it won't find cfn-lint-serverless installed). If that's the case (I haven't been able to test it yet to confirm), then this would be a functionality only for a particular set up and not for all SAM CLI customers. We can still launch it like that, but we need to be aware of that, for documentation purposes mainly.
I feel like it would be a better approach to just pass "extra rules" to cfn-lint), and users will have to call it like:

sam validate --lint --extra-lint-rules "cfn_lint_serverless.rules"

(I don't know if that's the best name for the parameter, but that's the idea)

We would then keep this part of the code

linter_config["append_rules"] = extra_cfn_lint_config

but we won't do any special checks specifically for cfn-lint-serverless, and it would be more of a general purpose config. (Honestly, I don't know how many other extra "rules" exist for cfn-lint, but I imagine there are others that could be added to the validation).

- 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

valerena · 2025-04-24T23:50:18Z

samcli/commands/validate/validate.py

+    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",
+)


But this was never released in any official version of SAM CLI, so it doesn't make sense to add it as "DEPRECATED", since it never really existed.

valerena · 2025-04-24T23:51:41Z

samcli/commands/validate/validate.py

 @save_params_option
 @pass_context
 @track_command
 @check_newer_version
 @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, extra_lint_rules):


Please, remove all references to --serverless-rules, since it doesn't really exist and it never existed officially. (Not only in this section but further down below too)

valerena · 2025-04-25T00:06:39Z

samcli/commands/validate/validate.py

@@ -170,20 +199,97 @@ def _lint(ctx: Context, template: str, template_path: str) -> None:
        cfn_lint_logger.propagate = True
        cfn_lint_logger.setLevel(logging.DEBUG)

+    print(f"Debug info: initial linter_config = {linter_config}")


Don't use print for debugging strings, use the logging module instead.

import logging LOG = logging.getLogger(__name__) ... LOG.debug("Debug info: initial linter_config %s", linter_config)

Basic usage on this other file:

aws-sam-cli/samcli/lib/utils/codeuri.py

Lines 5 to 8 in bdf9a7f

import logging

import os

LOG = logging.getLogger(__name__)

aws-sam-cli/samcli/lib/utils/codeuri.py

Line 30 in bdf9a7f

LOG.debug("Resolving code path. Cwd=%s, CodeUri=%s", cwd, codeuri)

Also, prefer using a message with the string format only, and the values as parameters of debug (like "Debug info: initial linter_config %s", linter_config) instead of formatting in the string directly (f"Debug info: initial linter_config = {linter_config}"). When f" {parameter}" is used for logging, the values are formatted even when logging is off, just adding extra processing that's not needed. (full details about how to use it in here: https://docs.python.org/3/library/logging.html#logging.Logger.debug)

valerena · 2025-04-25T00:07:06Z

samcli/commands/validate/validate.py

+    # Support for previous serverless_rules option (deprecated)
+    if serverless_rules:


Just to emphasize what I mentioned before, don't add any of the serverless_rules code.

valerena · 2025-04-25T00:14:56Z

samcli/commands/validate/validate.py

+    help="Specify additional lint rules to be used with cfn-lint. "
+         "Format: module.path (e.g. 'cfn_lint_serverless.rules')",
+    default=None
+)


I think you need to pass multiple=True to allow the parameter to be passed multiple times (https://click.palletsprojects.com/en/stable/options/#multiple-options and it will be stored as a list).

You mentioned in the documentation that that is actually supported, did you test passing it multiple times? (I imagine the last one will just replace any other one if we don't add the multiple=True)

valerena · 2025-04-25T00:17:31Z

samcli/commands/validate/validate.py


    if not matches:
+        print(f"Debug info: template validation successful")


I don't think we need all these debugging messages though. Some make sense, but this part is already printing something else already.

valerena · 2025-04-25T00:21:11Z

tests/unit/commands/validate/test_cli.py


 ctx_mock = namedtuple("ctx_mock", ["profile", "region"])
 ctx_lint_mock = namedtuple("ctx_lint_mock", ["debug", "region"])


 class TestValidateCli(TestCase):
+    def setUp(self):
+        # datetime.utcnow() 사용에 대한 경고 무시


Please include comments in English, thanks.

feat: add --serverless-rules to sam validate --lint

b524fb7

shashax42 requested a review from a team as a code owner March 24, 2025 16:21

github-actions bot added area/validate sam validate command area/schema JSON schema file pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Mar 24, 2025

shashax42 force-pushed the feature/serverless-rules-lint branch from 26e31e0 to b524fb7 Compare March 25, 2025 01:06

Merge branch 'develop' into feature/serverless-rules-lint

ee659b9

vicheey reviewed Mar 26, 2025

View reviewed changes

vicheey added need-customer-response Waiting for customer response and removed stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Mar 26, 2025

shashax42 requested a review from vicheey April 4, 2025 08:51

shashax42 added 4 commits April 14, 2025 13:53

Merge branch 'develop' into feature/serverless-rules-lint

ae6b24c

docs: Convert Korean comments to English for better readability

cd68913

Merge branch 'aws:develop' into feature/serverless-rules-lint

3f44d85

valerena requested changes Apr 25, 2025

View reviewed changes

Merge branch 'develop' into feature/serverless-rules-lint

bd7404d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add --serverless-rules to sam validate --lint #7950

feat: add --serverless-rules to sam validate --lint #7950

shashax42 commented Mar 24, 2025 •

edited

Loading

vicheey Mar 26, 2025

shashax42 Mar 27, 2025

valerena commented Apr 7, 2025

valerena Apr 24, 2025

valerena Apr 24, 2025

valerena Apr 25, 2025

valerena Apr 25, 2025

valerena Apr 25, 2025

valerena Apr 25, 2025

valerena Apr 25, 2025

		# Support for previous serverless_rules option (deprecated)
		if serverless_rules:


		if not matches:
		print(f"Debug info: template validation successful")

feat: add --serverless-rules to sam validate --lint #7950

Are you sure you want to change the base?

feat: add --serverless-rules to sam validate --lint #7950

Conversation

shashax42 commented Mar 24, 2025 • edited Loading

Which issue(s) does this change fix?

Why is this change necessary?

How does it address the issue?

What side effects does this change have?

Mandatory Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valerena commented Apr 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashax42 commented Mar 24, 2025 •

edited

Loading