Skip to content
Permalink
Browse files

Add pylint checker to verify our module layering

This checker is verifying that the modules of the different components
(cmk_base, cmk.gui, cmk.ec and so on) do not cross import the code of
the other components. Only import of cmk and cmk.utils is allowed from
all components.

Change-Id: I7ce6e4dcbabe431093d3779bc244b8083bc0af04
  • Loading branch information
LarsMichelsen committed Oct 10, 2019
1 parent d2cbde5 commit df37ef9b4ba787523d5e21fcc5939c6fc53c2934
@@ -17,7 +17,11 @@ init-hook=
tests_dir = tests_dir3 if sys.version.split('.')[0] == '3' else tests_dir2
sys.path.insert(0, os.environ.get("TEST_PATH", tests_dir))
import conftest
load-plugins=testlib.pylint_cmk,testlib.pylint_checker_localization,pylint.extensions.bad_builtin
load-plugins=
testlib.pylint_cmk,
testlib.pylint_checker_localization,
testlib.pylint_checker_cmk_module_layers,
pylint.extensions.bad_builtin
jobs=0
# TODO: Why do we need persistence?
persistent=yes
@@ -28,7 +28,7 @@ import argparse
import sys
import urlparse
import requests
import urllib3
import urllib3 # type: ignore
from cmk.utils.defines import service_state_name
from cmk.utils.exceptions import MKGeneralException
import cmk.utils.password_store
@@ -35,8 +35,9 @@
import livestatus

import cmk.utils.paths
import cmk.ec.settings
import cmk.ec.export
# It's OK to import centralized config load logic
import cmk.ec.settings # pylint: disable=cmk-module-layer-violation
import cmk.ec.export # pylint: disable=cmk-module-layer-violation
import cmk.utils.store
import cmk.utils

@@ -51,8 +51,9 @@
import cmk.utils.store as store
import cmk.utils.render

import cmk.ec.export as ec
import cmk.ec.defaults
# It's OK to import centralized config load logic
import cmk.ec.export as ec # pylint: disable=cmk-module-layer-violation
import cmk.ec.defaults # pylint: disable=cmk-module-layer-violation

if cmk.is_managed_edition():
import cmk.gui.cme.managed as managed
@@ -42,7 +42,9 @@
ConfigHostname,
)

import cmk_base.export
# Tolerate this for 1.6. Should be cleaned up in future versions,
# e.g. by trying to move the common code to a common place
import cmk_base.export # pylint: disable=cmk-module-layer-violation


@mode_registry.register
@@ -64,8 +64,9 @@
import cmk.utils
import cmk.utils.store as store
import cmk.utils.render as render
import cmk.ec.defaults
import cmk.ec.export
# It's OK to import centralized config load logic
import cmk.ec.defaults # pylint: disable=cmk-module-layer-violation
import cmk.ec.export # pylint: disable=cmk-module-layer-violation
import cmk.utils.regex
import cmk.utils.plugin_registry

@@ -54,7 +54,9 @@
NEGATE,
)

import cmk_base.export
# Tolerate this for 1.6. Should be cleaned up in future versions,
# e.g. by trying to move the common code to a common place
import cmk_base.export # pylint: disable=cmk-module-layer-violation

# This macro is needed to make the to_config() methods be able to use native
# pprint/repr for the ruleset data structures. Have a look at
@@ -50,22 +50,26 @@
import logging
from werkzeug.test import create_environ

import cmk_base.autochecks
# This special script needs persistence and conversion code from different
# places of Checkmk. We may centralize the conversion and move the persistance
# to a specific layer in the future, but for the the moment we need to deal
# with it.
import cmk_base.autochecks # pylint: disable=cmk-module-layer-violation

import cmk.utils.log as log
from cmk.utils.log import VERBOSE
import cmk.utils.debug
import cmk.utils.paths
import cmk.utils
import cmk.gui.watolib.tags
import cmk.gui.watolib.hosts_and_folders
import cmk.gui.watolib.rulesets
import cmk.gui.modules
import cmk.gui.config
import cmk.gui.utils
import cmk.gui.htmllib as htmllib
from cmk.gui.globals import AppContext, RequestContext
from cmk.gui.http import Request, Response
import cmk.gui.watolib.tags # pylint: disable=cmk-module-layer-violation
import cmk.gui.watolib.hosts_and_folders # pylint: disable=cmk-module-layer-violation
import cmk.gui.watolib.rulesets # pylint: disable=cmk-module-layer-violation
import cmk.gui.modules # pylint: disable=cmk-module-layer-violation
import cmk.gui.config # pylint: disable=cmk-module-layer-violation
import cmk.gui.utils # pylint: disable=cmk-module-layer-violation
import cmk.gui.htmllib as htmllib # pylint: disable=cmk-module-layer-violation
from cmk.gui.globals import AppContext, RequestContext # pylint: disable=cmk-module-layer-violation
from cmk.gui.http import Request, Response # pylint: disable=cmk-module-layer-violation


# TODO: Better make our application available?
@@ -37,7 +37,8 @@
from typing import NamedTuple, List # pylint: disable=unused-import
from pathlib2 import Path

import cmk.ec.export
# It's OK to import centralized config load logic
import cmk.ec.export # pylint: disable=cmk-module-layer-violation
from cmk.utils.log import VERBOSE
import cmk.utils.paths
import cmk.utils.tty as tty
@@ -34,7 +34,9 @@
from typing import Tuple, Union, Dict, Pattern, Optional # pylint: disable=unused-import

try:
from cmk.gui.i18n import _
# Tolerate this violation for the moment. It's OK to have unlocalized
# texts when used in other components than our GUI.
from cmk.gui.i18n import _ # pylint: disable=cmk-module-layer-violation
except ImportError:
_ = lambda x: x

@@ -0,0 +1,144 @@
"""Checker to prevent disallowed imports of modules
See chapter "Module hierarchy" in coding_guidelines_python in wiki
for further information.
"""

import os.path
from pylint.checkers import BaseChecker, utils # type: ignore
from pylint.interfaces import IAstroidChecker # type: ignore
from testlib import cmk_path


def register(linter):
linter.register_checker(CMKModuleLayerChecker(linter))


_COMPONENTS = (
"cmk_base",
"cmk.gui",
"cmk.ec",
"cmk.notification_plugins",
"cmk.special_agents",
"cmk.update_config",
"cmk.cee.dcd",
"cmk.cee.mknotifyd",
"cmk.cee.liveproxy",
"cmk.cee.notification_plugins",
)

_EXPLICIT_FILE_TO_COMPONENT = {
"web/app/index.wsgi": "cmk.gui",
"bin/update_rrd_fs_names.py": "cmk_base",
"bin/check_mk": "cmk_base",
"bin/cmk-update-config": "cmk.update_config",
"bin/mkeventd": "cmk.ec",
"enterprise/bin/liveproxyd": "cmk.cee.liveproxy",
"enterprise/bin/mknotifyd": "cmk.cee.mknotifyd",
"enterprise/bin/dcd": "cmk.cee.dcd",
# CEE specific notification plugins
"notifications/servicenow": "cmk.cee.notification_plugins",
"notifications/jira_issues": "cmk.cee.notification_plugins",
}


class CMKModuleLayerChecker(BaseChecker):
__implements__ = IAstroidChecker

name = 'cmk-module-layer-violation'
msgs = {
'C8410': ('Import of %r not allowed in module %r', 'cmk-module-layer-violation', 'whoop?'),
}

@utils.check_messages('cmk-module-layer-violation')
def visit_import(self, node):
names = [name for name, _ in node.names]
for name in names:
self._check_import(node, name)

@utils.check_messages('cmk-module-layer-violation')
def visit_importfrom(self, node):
self._check_import(node, node.modname)

def _check_import(self, node, import_modname):
file_path = node.root().file

if not import_modname.startswith("cmk"):
return # We only care about our own modules, ignore this

file_path = file_path[len(cmk_path()) + 1:] # Make relative

# Pylint fails to detect the correct module path here. Instead of realizing that the file
# cmk_base/automations/cee.py is cmk_base.automations.cee it thinks the module is "cee".
# We can silently ignore these files because the linked files at enterprise/... are checked.
if os.path.islink(file_path):
return # Ignore symlinked files instead of checking them twice, ignore this

mod_name = self._get_module_name_of_file(node, file_path)

if not self._is_import_allowed(file_path, mod_name, import_modname):
self.add_message("cmk-module-layer-violation",
node=node,
args=(import_modname, mod_name))

def _get_module_name_of_file(self, node, file_path):
"""Fixup managed and enterprise module names
astroid does not produce correct module names, because it does not know
that we link/copy our CEE/CME parts to the cmk.* module in the site.
Fake the final module name here.
"""
if file_path.startswith("enterprise/cmk") or file_path.startswith("managed/cmk"):
return self._get_module_path_from_shadowed_file_path(file_path)
return node.root().name

# Only works for our enterprise / managed directories
def _get_module_path_from_shadowed_file_path(self, file_path):
without_ext = file_path[:-3]
parts = without_ext.split("/")[1:]
return ".".join(parts)

def _is_import_allowed(self, file_path, mod_name, import_modname):
for component in _COMPONENTS:
if self._is_part_of_component(mod_name, file_path, component):
if self._is_import_in_component(import_modname, component):
return True

elif self._is_import_in_cee_component_part(mod_name, import_modname, component):
return True

return self._is_utility_import(import_modname)

def _is_part_of_component(self, mod_name, file_path, component):
if self._is_import_in_component(mod_name, component):
return True

explicit_component = _EXPLICIT_FILE_TO_COMPONENT.get(file_path)
if explicit_component is not None:
return explicit_component == component

# The check and bakery plugins are all compiled together by tests/pylint/test_pylint.py.
# They clearly belong to the cmk_base component.
if component == "cmk_base" and mod_name.startswith("cmk_pylint"):
return True

if component == "cmk.notification_plugins" and file_path.startswith("notifications/"):
return True

if component == "cmk.special_agents" and file_path.startswith("agents/special/"):
return True

return False

def _is_import_in_component(self, import_modname, component):
return import_modname == component or import_modname.startswith(component + ".")

def _is_import_in_cee_component_part(self, mod_name, import_modname, component):
"""If a module is split into cmk.cee.[mod] and cmk.[mod] it's allowed
to import non-cee parts in the cee part."""
return mod_name.startswith("cmk.cee.") and self._is_import_in_component(
import_modname, component)

def _is_utility_import(self, import_modname):
"""cmk and cmk.utils are allowed to be imported from all over the place"""
return import_modname in {"cmk", "cmk.utils"} or import_modname.startswith("cmk.utils.")

0 comments on commit df37ef9

Please sign in to comment.
You can’t perform that action at this time.