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 13d7be7a60e72905403c4ecbddd905cbd955def3 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/install/ipa_certupdate.py | 4 +-
ipaclient/remote_plugins/__init__.py | 4 +-
ipalib/__init__.py | 8 +++-
ipalib/config.py | 2 +
ipaplatform/base/services.py | 4 +-
ipaplatform/debian/services.py | 2 +
ipaplatform/redhat/services.py | 2 +
ipaplatform/redhat/tasks.py | 4 ++
ipapython/certdb.py | 6 ++-
ipapython/config.py | 2 +
ipapython/cookie.py | 2 +
ipapython/dogtag.py | 2 +
ipapython/ipaldap.py | 2 +
pylint_plugins.py | 83 +++++++++++++++++++++++++++++++++++-
pylintrc | 15 ++++++-
16 files changed, 135 insertions(+), 11 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 0c8f32a..5d41e4a 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 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/ipalib/config.py b/ipalib/config.py
index 45052ad..e2b4836 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -48,7 +48,9 @@
)
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 8149ff1..068b972 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 5d8e1ec..8fae1f3 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 67cb021..c1b574e 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -50,7 +50,9 @@
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 @@ 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
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 5389e63..6c89e77 100644
--- a/ipapython/certdb.py
+++ b/ipapython/certdb.py
@@ -32,10 +32,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/config.py b/ipapython/config.py
index b983a72..9db2dcd 100644
--- a/ipapython/config.py
+++ b/ipapython/config.py
@@ -35,7 +35,9 @@
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 9797fc1..4d8ef2c 100644
--- a/ipapython/cookie.py
+++ b/ipapython/cookie.py
@@ -599,7 +599,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 ff7dc44..48232a9 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -25,10 +25,12 @@
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 b158598..1b0aadd 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 45adf71..db80efe 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 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
