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