Support module paths in append_rules (#1216)

* Add RulesCollection.create_from_module
* Support appending rules from a module
* Update README w/ new append_rules module support

README.md

@@ -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

src/cfnlint/core.py

@@ -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
 

src/cfnlint/helpers.py

@@ -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 = []
@@ -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()

src/cfnlint/rules/__init__.py

@@ -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
@@ -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)
 
 

test/fixtures/rules/__init__.py

@@ -0,0 +1,4 @@
+"""
+Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""

test/fixtures/rules/custom1.py

@@ -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']

test/unit/module/core/test_get_rules.py

@@ -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
@@ -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))

test/unit/module/helpers/test_create_rules.py

@@ -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))

test/unit/module/helpers/test_load_plugins.py

@@ -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))

test/unit/module/test_rules_collections.py

@@ -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))