From ebf9d522e05f01038ffbe948e9c23e162c57baa9 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 10 Nov 2021 19:51:11 +0100 Subject: [PATCH] Adapt diagnosers code to load them as python modules instead of hooks --- src/diagnosis.py | 68 ++++++++++++++++++----------- src/diagnosis/00-basesystem.py | 21 +++++---- src/diagnosis/10-ip.py | 13 +++--- src/diagnosis/12-dnsrecords.py | 12 +++-- src/diagnosis/14-ports.py | 6 +-- src/diagnosis/21-web.py | 6 +-- src/diagnosis/24-mail.py | 11 +++-- src/diagnosis/30-services.py | 6 +-- src/diagnosis/50-systemresources.py | 6 +-- src/diagnosis/70-regenconf.py | 6 +-- src/diagnosis/80-apps.py | 6 +-- 11 files changed, 74 insertions(+), 87 deletions(-) diff --git a/src/diagnosis.py b/src/diagnosis.py index 4ac5e2731..a7b6c725c 100644 --- a/src/diagnosis.py +++ b/src/diagnosis.py @@ -27,6 +27,8 @@ import re import os import time +import glob +from importlib import import_module from moulinette import m18n, Moulinette from moulinette.utils import log @@ -38,7 +40,6 @@ from moulinette.utils.filesystem import ( ) from yunohost.utils.error import YunohostError, YunohostValidationError -from yunohost.hook import hook_list, hook_exec logger = log.getActionLogger("yunohost.diagnosis") @@ -48,15 +49,13 @@ DIAGNOSIS_SERVER = "diagnosis.yunohost.org" def diagnosis_list(): - all_categories_names = [h for h, _ in _list_diagnosis_categories()] - return {"categories": all_categories_names} + return {"categories": _list_diagnosis_categories()} def diagnosis_get(category, item): # Get all the categories - all_categories = _list_diagnosis_categories() - all_categories_names = [c for c, _ in all_categories] + all_categories_names = _list_diagnosis_categories() if category not in all_categories_names: raise YunohostValidationError( @@ -84,8 +83,7 @@ def diagnosis_show( return # Get all the categories - all_categories = _list_diagnosis_categories() - all_categories_names = [category for category, _ in all_categories] + all_categories_names = _list_diagnosis_categories() # Check the requested category makes sense if categories == []: @@ -174,8 +172,7 @@ def diagnosis_run( return # Get all the categories - all_categories = _list_diagnosis_categories() - all_categories_names = [category for category, _ in all_categories] + all_categories_names = _list_diagnosis_categories() # Check the requested category makes sense if categories == []: @@ -192,10 +189,11 @@ def diagnosis_run( diagnosed_categories = [] for category in categories: logger.debug("Running diagnosis for %s ..." % category) - path = [p for n, p in all_categories if n == category][0] + + diagnoser = _load_diagnoser(category) try: - code, report = hook_exec(path, args={"force": force}, env=None) + code, report = diagnoser.diagnose(force=force) except Exception: import traceback @@ -275,8 +273,7 @@ def _diagnosis_ignore(add_filter=None, remove_filter=None, list=False): def validate_filter_criterias(filter_): # Get all the categories - all_categories = _list_diagnosis_categories() - all_categories_names = [category for category, _ in all_categories] + all_categories_names = _list_diagnosis_categories() # Sanity checks for the provided arguments if len(filter_) == 0: @@ -404,12 +401,8 @@ def add_ignore_flag_to_issues(report): class Diagnoser: - def __init__(self, args, env, loggers): + def __init__(self): - # FIXME ? That stuff with custom loggers is weird ... (mainly inherited from the bash hooks, idk) - self.logger_debug, self.logger_warning, self.logger_info = loggers - self.env = env - self.args = args or {} self.cache_file = Diagnoser.cache_file(self.id_) self.description = Diagnoser.get_description(self.id_) @@ -424,10 +417,10 @@ class Diagnoser: os.makedirs(DIAGNOSIS_CACHE) return write_to_json(self.cache_file, report) - def diagnose(self): + def diagnose(self, force=False): if ( - not self.args.get("force", False) + not force and self.cached_time_ago() < self.cache_duration ): self.logger_debug("Cache still valid : %s" % self.cache_file) @@ -666,13 +659,36 @@ class Diagnoser: def _list_diagnosis_categories(): - hooks_raw = hook_list("diagnosis", list_by="priority", show_info=True)["hooks"] - hooks = [] - for _, some_hooks in sorted(hooks_raw.items(), key=lambda h: int(h[0])): - for name, info in some_hooks.items(): - hooks.append((name, info["path"])) - return hooks + paths = glob.glob(os.path.dirname(__file__) + "/diagnosis/??-*.py") + names = sorted([os.path.basename(path)[: -len(".py")] for path in paths]) + + return names + + +def _load_diagnoser(diagnoser_name): + + logger.debug(f"Loading diagnoser {diagnoser_name}") + + paths = glob.glob(os.path.dirname(__file__) + f"/diagnosis/??-{diagnoser_name}.py") + + if len(paths) != 1: + raise YunohostError(f"Uhoh, found several matches (or none?) for diagnoser {diagnoser_name} : {paths}", raw_msg=True) + + module_id = os.path.basename(paths[0][: -len(".py")]) + + try: + # this is python builtin method to import a module using a name, we + # use that to import the migration as a python object so we'll be + # able to run it in the next loop + module = import_module("yunohost.diagnosis.{}".format(module_id)) + return module.MyDiagnoser() + except Exception as e: + import traceback + + traceback.print_exc() + + raise YunohostError(f"Failed to load diagnoser {diagnoser_name} : {e}", raw_msg=True) def _email_diagnosis_issues(): diff --git a/src/diagnosis/00-basesystem.py b/src/diagnosis/00-basesystem.py index b472a2d32..66a2bf47b 100644 --- a/src/diagnosis/00-basesystem.py +++ b/src/diagnosis/00-basesystem.py @@ -4,13 +4,16 @@ import os import json import subprocess +from moulinette.utils import log from moulinette.utils.process import check_output from moulinette.utils.filesystem import read_file, read_json, write_to_json from yunohost.diagnosis import Diagnoser from yunohost.utils.packages import ynh_packages_version +logger = log.getActionLogger("yunohost.diagnosis") -class BaseSystemDiagnoser(Diagnoser): + +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 600 @@ -172,7 +175,7 @@ class BaseSystemDiagnoser(Diagnoser): try: return int(n_failures) except Exception: - self.logger_warning( + logger.warning( "Failed to parse number of recent auth failures, expected an int, got '%s'" % n_failures ) @@ -196,7 +199,7 @@ class BaseSystemDiagnoser(Diagnoser): if not os.path.exists(dpkg_log) or os.path.getmtime( cache_file ) > os.path.getmtime(dpkg_log): - self.logger_debug( + logger.debug( "Using cached results for meltdown checker, from %s" % cache_file ) return read_json(cache_file)[0]["VULNERABLE"] @@ -209,7 +212,7 @@ class BaseSystemDiagnoser(Diagnoser): # example output from the script: # [{"NAME":"MELTDOWN","CVE":"CVE-2017-5754","VULNERABLE":false,"INFOS":"PTI mitigates the vulnerability"}] try: - self.logger_debug("Running meltdown vulnerability checker") + logger.debug("Running meltdown vulnerability checker") call = subprocess.Popen( "bash %s --batch json --variant 3" % SCRIPT_PATH, shell=True, @@ -231,7 +234,7 @@ class BaseSystemDiagnoser(Diagnoser): # stuff which should be the last line output = output.strip() if "\n" in output: - self.logger_debug("Original meltdown checker output : %s" % output) + logger.debug("Original meltdown checker output : %s" % output) output = output.split("\n")[-1] CVEs = json.loads(output) @@ -241,18 +244,14 @@ class BaseSystemDiagnoser(Diagnoser): import traceback traceback.print_exc() - self.logger_warning( + logger.warning( "Something wrong happened when trying to diagnose Meltdown vunerability, exception: %s" % e ) raise Exception("Command output for failed meltdown check: '%s'" % output) - self.logger_debug( + logger.debug( "Writing results from meltdown checker to cache file, %s" % cache_file ) write_to_json(cache_file, CVEs) return CVEs[0]["VULNERABLE"] - - -def main(args, env, loggers): - return BaseSystemDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/10-ip.py b/src/diagnosis/10-ip.py index bc00fe25c..4ad4cfbfc 100644 --- a/src/diagnosis/10-ip.py +++ b/src/diagnosis/10-ip.py @@ -4,6 +4,7 @@ import re import os import random +from moulinette.utils import log from moulinette.utils.network import download_text from moulinette.utils.process import check_output from moulinette.utils.filesystem import read_file @@ -11,8 +12,10 @@ from moulinette.utils.filesystem import read_file from yunohost.diagnosis import Diagnoser from yunohost.utils.network import get_network_interfaces +logger = log.getActionLogger("yunohost.diagnosis") -class IPDiagnoser(Diagnoser): + +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 600 @@ -144,7 +147,7 @@ class IPDiagnoser(Diagnoser): ) if not any(is_default_route(r) for r in routes): - self.logger_debug( + logger.debug( "No default route for IPv%s, so assuming there's no IP address for that version" % protocol ) @@ -220,11 +223,7 @@ class IPDiagnoser(Diagnoser): try: return download_text(url, timeout=30).strip() except Exception as e: - self.logger_debug( + logger.debug( "Could not get public IPv%s : %s" % (str(protocol), str(e)) ) return None - - -def main(args, env, loggers): - return IPDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/12-dnsrecords.py b/src/diagnosis/12-dnsrecords.py index d45ebd9b8..305fda79b 100644 --- a/src/diagnosis/12-dnsrecords.py +++ b/src/diagnosis/12-dnsrecords.py @@ -6,6 +6,7 @@ import re from datetime import datetime, timedelta from publicsuffix2 import PublicSuffixList +from moulinette.utils import log from moulinette.utils.process import check_output from yunohost.utils.dns import ( @@ -18,8 +19,9 @@ from yunohost.diagnosis import Diagnoser from yunohost.domain import domain_list, _get_maindomain from yunohost.dns import _build_dns_conf, _get_dns_zone_for_domain +logger = log.getActionLogger("yunohost.diagnosis") -class DNSRecordsDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 600 @@ -31,7 +33,7 @@ class DNSRecordsDiagnoser(Diagnoser): major_domains = domain_list(exclude_subdomains=True)["domains"] for domain in major_domains: - self.logger_debug("Diagnosing DNS conf for %s" % domain) + logger.debug("Diagnosing DNS conf for %s" % domain) for report in self.check_domain( domain, @@ -223,7 +225,7 @@ class DNSRecordsDiagnoser(Diagnoser): ) ) else: - self.logger_debug("Dyndns domain: %s" % (domain)) + logger.debug("Dyndns domain: %s" % (domain)) continue expire_in = expire_date - datetime.now() @@ -298,7 +300,3 @@ class DNSRecordsDiagnoser(Diagnoser): return datetime.strptime(match.group(1), "%d-%b-%Y") return "expiration_not_found" - - -def main(args, env, loggers): - return DNSRecordsDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/14-ports.py b/src/diagnosis/14-ports.py index 7581a1ac6..e339a946c 100644 --- a/src/diagnosis/14-ports.py +++ b/src/diagnosis/14-ports.py @@ -6,7 +6,7 @@ from yunohost.diagnosis import Diagnoser from yunohost.service import _get_services -class PortsDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 600 @@ -146,7 +146,3 @@ class PortsDiagnoser(Diagnoser): "diagnosis_ports_forwarding_tip", ], ) - - -def main(args, env, loggers): - return PortsDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/21-web.py b/src/diagnosis/21-web.py index 450296e7e..3d9fb9f73 100644 --- a/src/diagnosis/21-web.py +++ b/src/diagnosis/21-web.py @@ -13,7 +13,7 @@ from yunohost.utils.dns import is_special_use_tld DIAGNOSIS_SERVER = "diagnosis.yunohost.org" -class WebDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 600 @@ -198,7 +198,3 @@ class WebDiagnoser(Diagnoser): summary="diagnosis_http_partially_unreachable", details=[detail.replace("error_http_check", "diagnosis_http")], ) - - -def main(args, env, loggers): - return WebDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/24-mail.py b/src/diagnosis/24-mail.py index f29b2786d..af7af1a16 100644 --- a/src/diagnosis/24-mail.py +++ b/src/diagnosis/24-mail.py @@ -6,6 +6,7 @@ import re from subprocess import CalledProcessError +from moulinette.utils import log from moulinette.utils.process import check_output from moulinette.utils.filesystem import read_yaml @@ -16,8 +17,10 @@ from yunohost.utils.dns import dig DEFAULT_DNS_BLACKLIST = "/usr/share/yunohost/dnsbl_list.yml" +logger = log.getActionLogger("yunohost.diagnosis") -class MailDiagnoser(Diagnoser): + +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 600 @@ -42,7 +45,7 @@ class MailDiagnoser(Diagnoser): "check_queue", # i18n: diagnosis_mail_queue_ok ] for check in checks: - self.logger_debug("Running " + check) + logger.debug("Running " + check) reports = list(getattr(self, check)()) for report in reports: yield report @@ -292,7 +295,3 @@ class MailDiagnoser(Diagnoser): if global_ipv6: outgoing_ips.append(global_ipv6) return (outgoing_ipversions, outgoing_ips) - - -def main(args, env, loggers): - return MailDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/30-services.py b/src/diagnosis/30-services.py index adbcc73b9..daf86ab1e 100644 --- a/src/diagnosis/30-services.py +++ b/src/diagnosis/30-services.py @@ -6,7 +6,7 @@ from yunohost.diagnosis import Diagnoser from yunohost.service import service_status -class ServicesDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 300 @@ -41,7 +41,3 @@ class ServicesDiagnoser(Diagnoser): item["summary"] = "diagnosis_services_running" yield item - - -def main(args, env, loggers): - return ServicesDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/50-systemresources.py b/src/diagnosis/50-systemresources.py index a662e392e..265e62acb 100644 --- a/src/diagnosis/50-systemresources.py +++ b/src/diagnosis/50-systemresources.py @@ -9,7 +9,7 @@ from moulinette.utils.process import check_output from yunohost.diagnosis import Diagnoser -class SystemResourcesDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 300 @@ -214,7 +214,3 @@ def round_(n): if n > 10: n = int(round(n)) return n - - -def main(args, env, loggers): - return SystemResourcesDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/70-regenconf.py b/src/diagnosis/70-regenconf.py index 8ccbeed58..63f6f2b32 100644 --- a/src/diagnosis/70-regenconf.py +++ b/src/diagnosis/70-regenconf.py @@ -9,7 +9,7 @@ from yunohost.regenconf import _get_regenconf_infos, _calculate_hash from moulinette.utils.filesystem import read_file -class RegenconfDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 300 @@ -70,7 +70,3 @@ class RegenconfDiagnoser(Diagnoser): for path, hash_ in infos["conffiles"].items(): if hash_ != _calculate_hash(path): yield {"path": path, "category": category} - - -def main(args, env, loggers): - return RegenconfDiagnoser(args, env, loggers).diagnose() diff --git a/src/diagnosis/80-apps.py b/src/diagnosis/80-apps.py index 5aec48ed8..e62acaa1f 100644 --- a/src/diagnosis/80-apps.py +++ b/src/diagnosis/80-apps.py @@ -7,7 +7,7 @@ from yunohost.app import app_list from yunohost.diagnosis import Diagnoser -class AppDiagnoser(Diagnoser): +class MyDiagnoser(Diagnoser): id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] cache_duration = 300 @@ -90,7 +90,3 @@ class AppDiagnoser(Diagnoser): == 0 ): yield ("error", "diagnosis_apps_deprecated_practices") - - -def main(args, env, loggers): - return AppDiagnoser(args, env, loggers).diagnose()