-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: add --serverless-rules to sam validate --lint #7950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
b524fb7
ee659b9
ae6b24c
dbdff60
cd68913
3f44d85
bd7404d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -57,20 +57,43 @@ 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", | ||||||||||||
) | ||||||||||||
Comment on lines
+63
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||||||||
@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 | ||||||||||||
) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you need to pass 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 |
||||||||||||
@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): | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, remove all references to |
||||||||||||
# 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 | ||||||||||||
# 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 | ||||||||||||
|
||||||||||||
def do_cli(ctx, template, lint): | ||||||||||||
|
||||||||||||
def do_cli(ctx, template, lint, serverless_rules, extra_lint_rules=None): | ||||||||||||
""" | ||||||||||||
Implementation of the ``cli`` method, just separated out for unit testing purposes | ||||||||||||
""" | ||||||||||||
|
@@ -84,7 +107,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, extra_lint_rules) | ||||||||||||
else: | ||||||||||||
iam_client = boto3.client("iam") | ||||||||||||
validator = SamTemplateValidator( | ||||||||||||
|
@@ -136,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) -> 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. | ||||||||||||
|
||||||||||||
|
@@ -153,10 +176,16 @@ 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 | ||||||||||||
from cfnlint.runner import InvalidRegionException | ||||||||||||
from samcli.lib.telemetry.event import EventTracker | ||||||||||||
|
||||||||||||
# 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 | ||||||||||||
|
@@ -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}") | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use
Basic usage on this other file: aws-sam-cli/samcli/lib/utils/codeuri.py Lines 5 to 8 in bdf9a7f
aws-sam-cli/samcli/lib/utils/codeuri.py Line 30 in bdf9a7f
Also, prefer using a message with the string format only, and the values as parameters of |
||||||||||||
|
||||||||||||
# Initialize variable to handle both options together | ||||||||||||
rules_to_append = [] | ||||||||||||
|
||||||||||||
# Support for previous serverless_rules option (deprecated) | ||||||||||||
if serverless_rules: | ||||||||||||
Comment on lines
+207
to
+208
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to emphasize what I mentioned before, don't add any of the |
||||||||||||
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 | ||||||||||||
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}") | ||||||||||||
|
||||||||||||
# 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") | ||||||||||||
|
||||||||||||
try: | ||||||||||||
print(f"Debug info: starting lint function call") | ||||||||||||
matches = lint(template, config=config) | ||||||||||||
print(f"Debug info: lint function call completed, matches = {matches}") | ||||||||||||
except InvalidRegionException as 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"Debug info: exception occurred = {e}") | ||||||||||||
raise | ||||||||||||
|
||||||||||||
if not matches: | ||||||||||||
print(f"Debug info: template validation successful") | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need all these debugging messages though. Some make sense, but this part is already printing something else already. |
||||||||||||
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) | ||||||||||||
|
||||||||||||
raise LinterRuleMatchedException("Linting failed. At least one linting rule was matched to the provided template.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular region to enable this flag by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vicheey No, there's no particular region requirement. You can enable this flag by default in all regions.