aws-sam-cli icon indicating copy to clipboard operation
aws-sam-cli copied to clipboard
verified publisher icon aws

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

Open shashax42 opened this issue 9 months ago • 3 comments

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

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

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

shashax42 avatar Mar 24 '25 16:03 shashax42

Two comments.

  1. 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.

  2. 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).

valerena avatar Apr 07 '25 19:04 valerena

@valerena

Issue: Enhance --extra-lint-rules Feature in SAM CLI (Addressing Review Feedback)

Description

This PR enhances the --extra-lint-rules feature in SAM CLI, addressing all feedback from valerena.

Addressed Feedback

  1. Added multiple=True flag to --extra-lint-rules option to allow specifying multiple rule modules
  2. Completely removed all code and tests related to the never-officially-released --serverless-rules option
  3. Replaced debug print statements with proper logging module usage
  4. Changed all comments to English for consistency
  5. Thoroughly updated test cases to reflect the new implementation

Test Code Improvements

  • Removed obsolete test_serverless_rules_deprecated_with_extra_lint_rules test
  • Updated all test cases to handle extra_lint_rules as a tuple (supporting multiple=True)
  • Removed all references to serverless_rules from test parameters
  • Fixed assertions to match the new parameter structure
  • Changed Korean test comments to English

Implementation Details

  • Users can now use the --extra-lint-rules option multiple times or specify multiple modules separated by commas
  • Code is cleaner and more maintainable
  • Test code now correctly validates the new interface

Related PR

#7950

shashax42 avatar May 07 '25 14:05 shashax42

@valerena Will this PR be merged?

shashax42 avatar Aug 10 '25 16:08 shashax42