pylint_plugins: add forbidden import checker

Add new pylint AST checker plugin which implements a check for imports
forbidden in IPA. Which imports are forbidden is configurable in pylintrc.

Provide default forbidden import configuration and disable the check for
existing forbidden imports in our code base.

Reviewed-By: Martin Basti <mbasti@redhat.com>

16 files changed, 135 insertions, 11 deletions

diff --git a/Makefile.am b/Makefile.am
index 0c8f32ac1..5d41e4af9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -180,7 +180,9 @@ pylint: $(top_builddir)/ipapython/version.py ipasetup.py
 		-type f -exec grep -qsm1 '^#!.*\bpython' '{}' \; -print`; \
 	echo "Pylint is running, please wait ..."; \
 	PYTHONPATH=$(top_srcdir) $(PYTHON) -m pylint \
-		--rcfile=$(top_srcdir)/pylintrc $${FILES}
+		--rcfile=$(top_srcdir)/pylintrc \
+		--load-plugins pylint_plugins \
+		$${FILES}
 
 .PHONY: jslint jslint-ui jslint-ui-test jslint-html \
 	$(top_builddir)/install/ui/src/libs/loader.js
diff --git a/ipaclient/install/ipa_certupdate.py b/ipaclient/install/ipa_certupdate.py
index 75c5d97df..d6ffbde19 100644
--- a/ipaclient/install/ipa_certupdate.py
+++ b/ipaclient/install/ipa_certupdate.py
@@ -100,9 +100,9 @@ class CertUpdate(admintool.AdminTool):
         if server_fstore.has_files():
             self.update_server(certs)
             try:
-                # pylint: disable=import-error
+                # pylint: disable=import-error,ipa-forbidden-import
                 from ipaserver.install import cainstance
-                # pylint: enable=import-error
+                # pylint: enable=import-error,ipa-forbidden-import
                 cainstance.add_lightweight_ca_tracking_requests(
                     self.log, lwcas)
             except Exception:
diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py
index da7004d62..037dd6f67 100644
--- a/ipaclient/remote_plugins/__init__.py
+++ b/ipaclient/remote_plugins/__init__.py
@@ -109,7 +109,9 @@ class ServerInfo(collections.MutableMapping):
 
 def get_package(api):
     if api.env.in_tree:
-        from ipaserver import plugins  # pylint: disable=import-error
+        # pylint: disable=import-error,ipa-forbidden-import
+        from ipaserver import plugins
+        # pylint: enable=import-error,ipa-forbidden-import
     else:
         try:
             plugins = api._remote_plugins
diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 544fcf2c8..16f90c3bb 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -935,7 +935,9 @@ class API(plugable.API):
     @property
     def packages(self):
         if self.env.in_server:
-            import ipaserver.plugins  # pylint: disable=import-error
+            # pylint: disable=import-error,ipa-forbidden-import
+            import ipaserver.plugins
+            # pylint: enable=import-error,ipa-forbidden-import
             result = (
                 ipaserver.plugins,
             )
@@ -948,7 +950,9 @@ class API(plugable.API):
             )
 
         if self.env.context in ('installer', 'updates'):
-            import ipaserver.install.plugins  # pylint: disable=import-error
+            # pylint: disable=import-error,ipa-forbidden-import
+            import ipaserver.install.plugins
+            # pylint: enable=import-error,ipa-forbidden-import
             result += (ipaserver.install.plugins,)
 
         return result
diff --git a/ipalib/config.py b/ipalib/config.py
index 45052ad5b..e2b48360e 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -48,7 +48,9 @@ from ipalib.constants import (
 )
 from ipalib import errors
 try:
+    # pylint: disable=ipa-forbidden-import
     from ipaplatform.tasks import tasks
+    # pylint: enable=ipa-forbidden-import
 except ImportError:
     tasks = None
 
diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index 8149ff1ef..068b9723c 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -93,11 +93,13 @@ class PlatformService(object):
     """
 
     def __init__(self, service_name, api=None):
+        # pylint: disable=ipa-forbidden-import
+        import ipalib  # FixMe: break import cycle
+        # pylint: enable=ipa-forbidden-import
         self.service_name = service_name
         if api is not None:
             self.api = api
         else:
-            import ipalib  # FixMe: break import cycle
             self.api = ipalib.api
             warnings.warn(
                 "{s.__class__.__name__}('{s.service_name}', api=None) "
diff --git a/ipaplatform/debian/services.py b/ipaplatform/debian/services.py
index 82a74e2d6..85fba56e2 100644
--- a/ipaplatform/debian/services.py
+++ b/ipaplatform/debian/services.py
@@ -166,7 +166,9 @@ def debian_service_class_factory(name, api=None):
 
 class DebianServices(base_services.KnownServices):
     def __init__(self):
+        # pylint: disable=ipa-forbidden-import
         import ipalib  # FixMe: break import cycle
+        # pylint: enable=ipa-forbidden-import
         services = dict()
         for s in base_services.wellknownservices:
             services[s] = self.service_class_factory(s, ipalib.api)
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 5d8e1ecaa..8fae1f3cc 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -253,7 +253,9 @@ def redhat_service_class_factory(name, api=None):
 
 class RedHatServices(base_services.KnownServices):
     def __init__(self):
+        # pylint: disable=ipa-forbidden-import
         import ipalib  # FixMe: break import cycle
+        # pylint: enable=ipa-forbidden-import
         services = dict()
         for s in base_services.wellknownservices:
             services[s] = self.service_class_factory(s, ipalib.api)
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 67cb02196..c1b574e06 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -50,7 +50,9 @@ from ipaplatform.paths import paths
 from ipaplatform.redhat.authconfig import RedHatAuthConfig
 from ipaplatform.base.tasks import BaseTaskNamespace
 
+# pylint: disable=ipa-forbidden-import
 from ipalib.constants import IPAAPI_USER
+# pylint: enable=ipa-forbidden-import
 
 _ffi = FFI()
 _ffi.cdef("""
@@ -235,8 +237,10 @@ class RedHatTaskNamespace(BaseTaskNamespace):
             return True
 
     def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
+        # pylint: disable=ipa-forbidden-import
         from ipalib import x509  # FixMe: break import cycle
         from ipalib.errors import CertificateError
+        # pylint: enable=ipa-forbidden-import
 
         new_cacert_path = paths.SYSTEMWIDE_IPA_CA_CRT
 
diff --git a/ipapython/certdb.py b/ipapython/certdb.py
index 5389e6344..6c89e7780 100644
--- a/ipapython/certdb.py
+++ b/ipapython/certdb.py
@@ -32,10 +32,12 @@ from nss.error import NSPRError
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import root_logger
 from ipapython import ipautil
-from ipalib import x509
+from ipalib import x509     # pylint: disable=ipa-forbidden-import
 
 try:
-    from ipaplatform.paths import paths  # pylint: disable=import-error
+    # pylint: disable=import-error,ipa-forbidden-import
+    from ipaplatform.paths import paths
+    # pylint: enable=import-error,ipa-forbidden-import
 except ImportError:
     CERTUTIL = '/usr/bin/certutil'
     PK12UTIL = '/usr/bin/pk12util'
diff --git a/ipapython/config.py b/ipapython/config.py
index b983a720c..9db2dcd4d 100644
--- a/ipapython/config.py
+++ b/ipapython/config.py
@@ -35,7 +35,9 @@ from six.moves.urllib.parse import urlsplit
 from ipapython.dn import DN
 
 try:
+    # pylint: disable=ipa-forbidden-import
     from ipaplatform.paths import paths
+    # pylint: enable=ipa-forbidden-import
 except ImportError:
     IPA_DEFAULT_CONF = '/etc/ipa/default.conf'
 else:
diff --git a/ipapython/cookie.py b/ipapython/cookie.py
index 9797fc184..4d8ef2c1a 100644
--- a/ipapython/cookie.py
+++ b/ipapython/cookie.py
@@ -599,7 +599,9 @@ class Cookie(object):
             # FIXME: At the moment we can't import from ipalib at the
             # module level because of a dependency loop (cycle) in the
             # import. Our module layout needs to be refactored.
+            # pylint: disable=ipa-forbidden-import
             from ipalib.util import validate_domain_name
+            # pylint: enable=ipa-forbidden-import
             try:
                 validate_domain_name(url_domain)
             except Exception:
diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index ff7dc4464..48232a9d1 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -25,10 +25,12 @@ import six
 from six.moves.urllib.parse import urlencode
 # pylint: enable=import-error
 
+# pylint: disable=ipa-forbidden-import
 from ipalib import api, errors
 from ipalib.util import create_https_connection
 from ipalib.errors import NetworkError
 from ipalib.text import _
+# pylint: enable=ipa-forbidden-import
 from ipapython import ipautil
 from ipapython.ipa_log_manager import root_logger
 
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index b15859892..1b0aaddd6 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -39,8 +39,10 @@ import ldap.filter
 from ldap.controls import SimplePagedResultsControl
 import six
 
+# pylint: disable=ipa-forbidden-import
 from ipalib import errors, _
 from ipalib.constants import LDAP_GENERALIZED_TIME_FORMAT
+# pylint: enable=ipa-forbidden-import
 from ipapython.ipautil import format_netloc, CIDict
 from ipapython.ipa_log_manager import log_mgr
 from ipapython.dn import DN
diff --git a/pylint_plugins.py b/pylint_plugins.py
index 45adf7132..db80efeba 100644
--- a/pylint_plugins.py
+++ b/pylint_plugins.py
@@ -5,14 +5,18 @@
 from __future__ import print_function
 
 import copy
+import os.path
 import sys
 
 from astroid import MANAGER
 from astroid import scoped_nodes
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import check_messages
+from pylint.interfaces import IAstroidChecker
 
 
 def register(linter):
-    pass
+    linter.register_checker(IPAChecker(linter))
 
 
 def _warning_already_exists(cls, member):
@@ -252,3 +256,80 @@ def fix_ipa_classes(cls):
         fake_class(cls, ipa_class_members[class_name_with_module])
 
 MANAGER.register_transform(scoped_nodes.Class, fix_ipa_classes)
+
+
+class IPAChecker(BaseChecker):
+    __implements__ = IAstroidChecker
+
+    name = 'ipa'
+    msgs = {
+        'W9901': (
+            'Forbidden import %s (can\'t import from %s in %s)',
+            'ipa-forbidden-import',
+            'Used when an forbidden import is detected.',
+        ),
+    }
+    options = (
+        (
+            'forbidden-imports',
+            {
+                'default': '',
+                'type': 'csv',
+                'metavar': '<path>[:<module>[:<module>...]][,<path>...]',
+                'help': 'Modules which are forbidden to be imported in the '
+                        'given paths',
+            },
+        ),
+    )
+    priority = -1
+
+    def open(self):
+        self._dir = os.path.abspath(os.path.dirname(__file__))
+
+        self._forbidden_imports = {self._dir: []}
+        for forbidden_import in self.config.forbidden_imports:
+            forbidden_import = forbidden_import.split(':')
+            path = os.path.join(self._dir, forbidden_import[0])
+            path = os.path.abspath(path)
+            modules = forbidden_import[1:]
+            self._forbidden_imports[path] = modules
+
+        self._forbidden_imports_stack = []
+
+    def _get_forbidden_import_rule(self, node):
+        path = node.source_file
+        if path:
+            path = os.path.abspath(path)
+            while path.startswith(self._dir):
+                if path in self._forbidden_imports:
+                    return path
+                path = os.path.dirname(path)
+        return self._dir
+
+    def visit_module(self, node):
+        self._forbidden_imports_stack.append(
+            self._get_forbidden_import_rule(node))
+
+    def leave_module(self, node):
+        self._forbidden_imports_stack.pop()
+
+    def _check_forbidden_imports(self, node, names):
+        path = self._forbidden_imports_stack[-1]
+        relpath = os.path.relpath(path, self._dir)
+        modules = self._forbidden_imports[path]
+        for module in modules:
+            module_prefix = module + '.'
+            for name in names:
+                if name == module or name.startswith(module_prefix):
+                    self.add_message('ipa-forbidden-import',
+                                     args=(name, module, relpath), node=node)
+
+    @check_messages('ipa-forbidden-import')
+    def visit_import(self, node):
+        names = [n[0] for n in node.names]
+        self._check_forbidden_imports(node, names)
+
+    @check_messages('ipa-forbidden-import')
+    def visit_importfrom(self, node):
+        names = ['{}.{}'.format(node.modname, n[0]) for n in node.names]
+        self._check_forbidden_imports(node, names)
diff --git a/pylintrc b/pylintrc
index 93075f54b..bff23059d 100644
--- a/pylintrc
+++ b/pylintrc
@@ -4,7 +4,9 @@ persistent=no
 
 # List of plugins (as comma separated values of python modules names) to load,
 # usually to register additional checkers.
-load-plugins=pylint_plugins
+# FIXME: has to be specified on the command line otherwise pylint fails with
+#        DuplicateSectionError for the IPA section
+#load-plugins=pylint_plugins
 
 # Use multiple processes to speed up Pylint.
 jobs=0
@@ -103,3 +105,14 @@ msg-template='{path}:{line}: [{msg_id}({symbol}), {obj}] {msg})'
 
 [VARIABLES]
 dummy-variables-rgx=_.+
+
+
+[IPA]
+forbidden-imports=
+    client/:ipaserver,
+    ipaclient/:ipaclient.install:ipalib.install:ipaplatform:ipaserver,
+    ipaclient/install/:ipaserver,
+    ipalib/:ipaclient.install:ipalib.install:ipaplatform:ipaserver,
+    ipalib/install/:ipaserver,
+    ipaplatform/:ipaclient:ipalib:ipaserver,
+    ipapython/:ipaclient:ipalib:ipaplatform:ipaserver