Skip to content
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

Support module paths in append_rules #1216

Merged
merged 6 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Optional parameters:
| -r, --regions | regions | [REGIONS [REGIONS ...]], ALL_REGIONS | Test the template against many regions. [Supported regions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html) |
| -b, --ignore-bad-template | ignore_bad_template | | Ignores bad template errors |
| --ignore-templates | IGNORE_TEMPLATES [IGNORE_TEMPLATES ...] | Ignore templates from being scanned
| -a, --append-rules | append_rules | [RULESDIR [RULESDIR ...]] | Specify one or more rules directories using one or more --append-rules arguments. |
| -a, --append-rules | append_rules | [RULESPATH [RULESPATH ...]] | Specify one or more rules paths using one or more --append-rules arguments. Each path can be either a directory containing python files, or an import path to a module. |
| -i, --ignore-checks | ignore_checks | [IGNORE_CHECKS [IGNORE_CHECKS ...]] | Only check rules whose ID do not match or prefix these values. Examples: <br />- A value of `W` will disable all warnings<br />- `W2` disables all Warnings for Parameter rules.<br />- `W2001` will disable rule `W2001` |
| -e, --include-experimental | include_experimental | | Whether rules that still in an experimental state should be included in the checks |
| -c, --include-checks | INCLUDE_CHECKS [INCLUDE_CHECKS ...] | Include rules whose id match these values
Expand Down
13 changes: 8 additions & 5 deletions src/cfnlint/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,17 @@ def get_formatter(fmt):
return formatter


def get_rules(rulesdir, ignore_rules, include_rules, configure_rules=None, include_experimental=False):
def get_rules(append_rules, ignore_rules, include_rules, configure_rules=None, include_experimental=False):
"""Get rules"""
rules = RulesCollection(ignore_rules, include_rules, configure_rules, include_experimental)
rules_dirs = [DEFAULT_RULESDIR] + rulesdir
rules_paths = [DEFAULT_RULESDIR] + append_rules
try:
for rules_dir in rules_dirs:
rules.create_from_directory(rules_dir)
except OSError as e:
for rules_path in rules_paths:
if rules_path and os.path.isdir(os.path.expanduser(rules_path)):
rules.create_from_directory(rules_path)
else:
rules.create_from_module(rules_path)
except (OSError, ImportError) as e:
raise UnexpectedRuleException('Tried to append rules but got an error: %s' % str(e), 1)
return rules

Expand Down
20 changes: 14 additions & 6 deletions src/cfnlint/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,19 @@ def converter(o): # pylint: disable=R1710
return json.dumps(json_string, indent=2, sort_keys=True, separators=(',', ': '), default=converter)


def create_rules(mod):
"""Create and return an instance of each CloudFormationLintRule subclass
from the given module."""
result = []
for _, clazz in inspect.getmembers(mod, inspect.isclass):
method_resolution = inspect.getmro(clazz)
if [clz for clz in method_resolution[1:] if clz.__module__ in ('cfnlint', 'cfnlint.rules') and clz.__name__ == 'CloudFormationLintRule']:
# create and instance of subclasses of CloudFormationLintRule
obj = clazz()
result.append(obj)
return result


def load_plugins(directory):
"""Load plugins"""
result = []
Expand All @@ -282,12 +295,7 @@ def onerror(os_error):
try:
fh, filename, desc = imp.find_module(pluginname, [root])
mod = imp.load_module(pluginname, fh, filename, desc)
for _, clazz in inspect.getmembers(mod, inspect.isclass):
method_resolution = inspect.getmro(clazz)
if [clz for clz in method_resolution[1:] if clz.__module__ in ('cfnlint', 'cfnlint.rules') and clz.__name__ == 'CloudFormationLintRule']:
# create and instance of subclasses of CloudFormationLintRule
obj = clazz()
result.append(obj)
result.extend(create_rules(mod))
finally:
if fh:
fh.close()
Expand Down
11 changes: 6 additions & 5 deletions src/cfnlint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import logging
from datetime import datetime
import importlib
import traceback
import cfnlint.helpers
from cfnlint.decode.node import TemplateAttributeError
Expand Down Expand Up @@ -341,16 +342,16 @@ def run(self, filename, cfn):

return matches

def create_from_module(self, modpath):
"""Create rules from a module import path"""
mod = importlib.import_module(modpath)
self.extend(cfnlint.helpers.create_rules(mod))

def create_from_directory(self, rulesdir):
"""Create rules from directory"""
result = []
if rulesdir != '':
result = cfnlint.helpers.load_plugins(os.path.expanduser(rulesdir))

for rule in result:
if rule.id in self.configure_rules:
rule.configure(self.configure_rules[rule.id])

self.extend(result)


Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/rules/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"""
Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
13 changes: 13 additions & 0 deletions test/fixtures/rules/custom1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
from cfnlint.rules import CloudFormationLintRule # pylint: disable=E0401

class CustomRule1(CloudFormationLintRule):
""" Def Rule """
id = 'E9001'
shortdesc = 'Custom Rule 1'
description = 'Test Rule Description'
source_url = 'https://github.com/aws-cloudformation/cfn-python-lint/'
tags = ['resources']
18 changes: 18 additions & 0 deletions test/unit/module/core/test_get_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import os

from test.testlib.testcase import BaseTestCase
import cfnlint.core
import cfnlint.helpers # pylint: disable=E0401
Expand All @@ -18,3 +20,19 @@ def test_invalid_rule(self):
except cfnlint.core.UnexpectedRuleException as e:
err = e
assert (type(err) == cfnlint.core.UnexpectedRuleException)

def test_append_module(self):
"""test appending rules from a module"""
rules = cfnlint.core.get_rules(["test.fixtures.rules.custom1"], [], [], [])
self.assertIn("E9001", (r.id for r in rules))
# Make sure the default rules are there too.
self.assertIn("E1001", (r.id for r in rules))

def test_append_directory(self):
"""test appending rules from a directory"""
import test.fixtures.rules
path = os.path.dirname(test.fixtures.rules.__file__)
rules = cfnlint.core.get_rules([path], [], [], [])
self.assertIn("E9001", (r.id for r in rules))
# Make sure the default rules are there too.
self.assertIn("E1001", (r.id for r in rules))
19 changes: 19 additions & 0 deletions test/unit/module/helpers/test_create_rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
"""
Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import os

from test.testlib.testcase import BaseTestCase
from cfnlint.helpers import create_rules
from cfnlint.rules import CloudFormationLintRule


class TestCreateRules(BaseTestCase):
"""Test creating rules from a module."""

def testBase(self):
from cfnlint.rules.templates import Base
rules = create_rules(Base)
self.assertTrue(all(isinstance(r, CloudFormationLintRule) for r in rules))
self.assertTrue('E1001' in (r.id for r in rules))
31 changes: 31 additions & 0 deletions test/unit/module/helpers/test_load_plugins.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""
Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""
import os

from test.testlib.testcase import BaseTestCase
from cfnlint.helpers import load_plugins
from cfnlint.rules import CloudFormationLintRule
from cfnlint.core import DEFAULT_RULESDIR # pylint: disable=E0401


class TestLoadPlugins(BaseTestCase):
"""Test loading rules."""

def testFromDefaultDirectory(self):
rules = load_plugins(DEFAULT_RULESDIR)

self.assertTrue(all(isinstance(r, CloudFormationLintRule) for r in rules))
# From templates/Base.py
self.assertTrue('E1001' in (r.id for r in rules))
# From resources/Name.py
self.assertTrue('E3006' in (r.id for r in rules))

def testFromSubDirectory(self):
path = os.path.join(DEFAULT_RULESDIR, "templates")
rules = load_plugins(path)

self.assertTrue(all(isinstance(r, CloudFormationLintRule) for r in rules))
self.assertTrue('E1001' in (r.id for r in rules))
self.assertFalse('E3006' in (r.id for r in rules))
10 changes: 10 additions & 0 deletions test/unit/module/test_rules_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,13 @@ class rule_e0002(CloudFormationLintRule):
self.assertEqual(len(rules), 2)
for rule in rules:
self.assertIn(rule.id, ['E0000', 'E0010'])


class TestCreateFromModule(BaseTestCase):
"""Test loading a rules collection from a module"""

def test_create_from_module(self):
"""Load rules from a module"""
rules = RulesCollection()
rules.create_from_module("cfnlint.rules.templates.Base")
self.assertIn('E1001', (r.id for r in rules))