URL: https://github.com/freeipa/freeipa/pull/463
Author: HonzaCholasta
Title: #463: pylint_plugins: add forbidden import checker
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/463/head:pr463
git checkout pr463
From 82af6b07e922f4cad625ab31b91f65cf804a9858 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 14 Feb 2017 09:58:44 +0100
Subject: [PATCH] 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.
---
Makefile.am | 4 +-
ipaclient/csrgen.py | 2 +-
ipaclient/install/ipa_certupdate.py | 4 +-
ipaclient/remote_plugins/__init__.py | 4 +-
ipalib/__init__.py | 8 +++-
ipaplatform/base/services.py | 4 +-
ipaplatform/debian/services.py | 2 +
ipaplatform/redhat/services.py | 2 +
ipaplatform/redhat/tasks.py | 2 +
ipapython/certdb.py | 6 ++-
ipapython/cookie.py | 2 +
ipapython/dogtag.py | 2 +
ipapython/ipaldap.py | 2 +
pylint_plugins.py | 82 +++++++++++++++++++++++++++++++++++-
pylintrc | 15 ++++++-
15 files changed, 129 insertions(+), 12 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 9bfc899..bb6e480 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -164,7 +164,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/csrgen.py b/ipaclient/csrgen.py
index 96100ae..828ab43 100644
--- a/ipaclient/csrgen.py
+++ b/ipaclient/csrgen.py
@@ -15,7 +15,7 @@
from ipalib import errors
from ipalib.text import _
-from ipaplatform.paths import paths
+from ipaplatform.paths import paths # pylint: disable=ipa-forbidden-import
from ipapython.ipa_log_manager import log_mgr
if six.PY3:
diff --git a/ipaclient/install/ipa_certupdate.py b/ipaclient/install/ipa_certupdate.py
index 75c5d97..d6ffbde 100644
--- a/ipaclient/install/ipa_certupdate.py
+++ b/ipaclient/install/ipa_certupdate.py
@@ -100,9 +100,9 @@ def run(self):
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 da7004d..037dd6f 100644
--- a/ipaclient/remote_plugins/__init__.py
+++ b/ipaclient/remote_plugins/__init__.py
@@ -109,7 +109,9 @@ def is_valid(self):
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 544fcf2..16f90c3 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 @@ def packages(self):
)
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/ipaplatform/base/services.py b/ipaplatform/base/services.py
index 9c9a5ae..ae7c777 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 82a74e2..85fba56 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 cc5d674..3b69bfa 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -252,7 +252,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 9dd71b4..3bfa337 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -218,7 +218,9 @@ def reload_systemwide_ca_store(self):
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
+ # pylint: enable=ipa-forbidden-import
new_cacert_path = paths.SYSTEMWIDE_IPA_CA_CRT
diff --git a/ipapython/certdb.py b/ipapython/certdb.py
index 9481326..90c8adc 100644
--- a/ipapython/certdb.py
+++ b/ipapython/certdb.py
@@ -29,10 +29,12 @@
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/cookie.py b/ipapython/cookie.py
index 8abd961..6f4ba8b 100644
--- a/ipapython/cookie.py
+++ b/ipapython/cookie.py
@@ -596,7 +596,9 @@ def domain_valid(url_domain, cookie_domain):
# 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 01fc5cb..aadea8e 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -26,9 +26,11 @@
from six.moves.urllib.parse import urlencode
# pylint: enable=import-error
+# pylint: disable=ipa-forbidden-import
from ipalib import api, errors
from ipalib.errors import NetworkError
from ipalib.text import _
+# pylint: enable=ipa-forbidden-import
from ipapython import nsslib, ipautil
from ipapython.ipa_log_manager import root_logger
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 82d45b9..6ac34dd 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -39,8 +39,10 @@
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 fc2ce9b..b5c6577 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):
@@ -255,3 +259,79 @@ 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 = {
+ 'E9901': (
+ 'Forbidden import %s',
+ 'ipa-forbidden-import',
+ 'Used when an import forbidden in IPA is detected.',
+ ),
+ }
+ options = (
+ (
+ 'forbidden-imports',
+ {
+ 'default': '',
+ 'type': 'csv',
+ 'metavar': '<path>[:<module>[:<module>...]][,<path>...]',
+ 'help': 'Modules which are forbidden to import in the given '
+ 'paths',
+ },
+ ),
+ )
+ priority = -1
+
+ def open(self):
+ self._dir = os.path.abspath(os.path.dirname(__file__))
+
+ self._forbidden_imports = {}
+ 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_modules(self, node):
+ path = node.source_file
+ if path:
+ path = os.path.abspath(path)
+ while path.startswith(self._dir):
+ try:
+ return self._forbidden_imports[path]
+ except KeyError:
+ pass
+ path = os.path.dirname(path)
+ return []
+
+ def visit_module(self, node):
+ self._forbidden_imports_stack.append(self._get_forbidden_modules(node))
+
+ def leave_module(self, node):
+ self._forbidden_imports_stack.pop()
+
+ def _check_forbidden_imports(self, node, names):
+ modules = self._forbidden_imports_stack[-1]
+ 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, 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 93075f5..bff2305 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
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
