From 3df9823a5db148e2345fb643bd252baf53ce3659 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Sat, 6 Sep 2025 16:13:59 +0200 Subject: [PATCH 1/4] Avoid linting failures by unconditionally defining a logger property. Recent changes to the linting checks have resulted in any changes in and around configuration fail in CI with the complaint that logger is not defined. In trying to understand what was happening it was found that some amount of confusion was occurring having both logger and logger_obj properties. Attempt to fix this by: 1) unconditionally defining both properties 2) always setting both properties 3) determining the type of logger being assigned and set the internal properties as appropriate Expanding on the latter, loggers are almost always set as assignment to .logger, but this was not always being passed the same kind of object. At times this was a bare logger (i.e. info(), .debug() etc) but sometimes it was something with .reopen() which would then simply be thrown away and thus reload() would not actually work. Fix this by detecting a .reopen() method and correctly referencing such an object. --- mig/shared/configuration.py | 57 +++++++++++++++---- mig/shared/logger.py | 17 ++++++ .../mig_shared_configuration--new.json | 2 - tests/test_mig_shared_configuration.py | 6 +- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index 91a24549a..cd5637020 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -37,6 +37,7 @@ import copy import datetime import functools +import inspect import os import pwd import re @@ -67,7 +68,7 @@ generic_valid_days, DEFAULT_USER_ID_FORMAT, valid_user_id_formats, \ valid_filter_methods, default_twofactor_auth_apps, \ mig_conf_section_dirname - from mig.shared.logger import Logger, SYSLOG_GDP + from mig.shared.logger import Logger, BareLoggerAdapter, SYSLOG_GDP from mig.shared.htmlgen import menu_items, vgrid_items from mig.shared.fileio import read_file, load_json, write_file except ImportError as ioe: @@ -693,7 +694,6 @@ def get(self, *args, **kwargs): 'logfile': '', 'loglevel': '', 'logger_obj': None, - 'logger': None, 'gdp_logger_obj': None, 'gdp_logger': None, 'auth_logger_obj': None, @@ -757,6 +757,13 @@ def __init__(self, config_file, verbose=False, skip_log=False, # Explicitly init a few helpers hot-plugged and used in ways where it's # less obvious if they are always guaranteed to already be initialized. self.default_page = None + + # internal state + self._loaded = False + + # logging related + self.logger_obj = None + self._logger = None self.auth_logger_obj = None self.gdp_logger_obj = None @@ -770,6 +777,34 @@ def __init__(self, config_file, verbose=False, skip_log=False, disable_auth_log=disable_auth_log, _config_file=config_file) + @property + def logger(self): + assert self._logger, "logging attempt prior to logger availability" + return self._logger + + @logger.setter + def logger(self, logger): + """Setter method that correctly sets logger related properties.""" + + # attempt to determine what type of objetc we were given - this logic + # exists to deal with some fallout from having both logger_obj and + # logger properties and which of them should be set + + if inspect.ismethod(getattr(logger, 'reopen', None)): + # we have a logger_obj, not a plain logger + # record it that way to ensure it could be corrctly reopened where + # otherwise refefences to the object that has it may be lost and it + # may not occur + self.logger_obj = logger + self._logger = logger.logger + elif inspect.ismethod(getattr(logger, 'info', None)): + # we have a bare logger object based on the sanity check + self._logger = logger + self.logger_obj = BareLoggerAdapter(logger) + else: + raise AssertionError("attempted assignment of unsupported logger") + + def reload_config(self, verbose, skip_log=False, disable_auth_log=False, _config_file=None): """Re-read and parse configuration file. Optional skip_log arg @@ -786,12 +821,6 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, _config_file = _config_file or self.config_file assert _config_file is not None - try: - if self.logger: - self.logger.info('reloading configuration and reopening log') - except: - pass - try: config_file_is_path = os.path.isfile(_config_file) except TypeError: @@ -841,13 +870,17 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, # reopen or initialize logger - if self.logger_obj: + if self._loaded: + self.logger.info('reloading configuration and reopening log') self.logger_obj.reopen() else: - self.logger_obj = Logger(self.loglevel, logfile=self.log_path) + self.logger = Logger(self.loglevel, logfile=self.log_path) + self.logger.info('loading configuration and opening log') + + # record that the object has been populated + self._loaded = True - logger = self.logger_obj.logger - self.logger = logger + logger = self.logger # print "logger initialized (level " + logger_obj.loglevel() + ")" # logger.debug("logger initialized") diff --git a/mig/shared/logger.py b/mig/shared/logger.py index 5b949fca0..a2244f511 100644 --- a/mig/shared/logger.py +++ b/mig/shared/logger.py @@ -67,6 +67,23 @@ def _name_to_format(name): return formats[name] +class BareLoggerAdapter: + """Small wrapper to adapt an arbitrary bare logger to the MiG Logger API""" + + def __init__(self, logger): + self._logger = logger + + @property + def logger(self): + return self._logger + + def reopen(self): + pass + + def shutdown(self): + pass + + class SysLogLibHandler(logging.Handler): """A logging handler that emits messages to syslog.syslog.""" diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json index 2d4edb02c..54b3d9673 100644 --- a/tests/fixture/mig_shared_configuration--new.json +++ b/tests/fixture/mig_shared_configuration--new.json @@ -104,8 +104,6 @@ ], "log_dir": "", "logfile": "", - "logger": null, - "logger_obj": null, "loglevel": "", "lrmstypes": [], "mig_code_base": "", diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index 7c9aef06e..655739fcd 100644 --- a/tests/test_mig_shared_configuration.py +++ b/tests/test_mig_shared_configuration.py @@ -34,6 +34,7 @@ from tests.support import MigTestCase, TEST_DATA_DIR, PY2, testmain, \ fixturefile from mig.shared.configuration import Configuration +from mig.shared.logger import null_logger def _is_method(value): @@ -42,7 +43,7 @@ def _is_method(value): def _to_dict(obj): return {k: v for k, v in inspect.getmembers(obj) - if not (k.startswith('__') or _is_method(v))} + if not (k.startswith('_') or k.startswith('logger') or _is_method(v))} class MigSharedConfiguration(MigTestCase): @@ -320,6 +321,9 @@ def test_default_object(self): 'mig_shared_configuration--new', fixture_format='json') configuration = Configuration(None) + # attach a null logger to sidestep the useful logger before available + # assertion which would otherwise blow when the object is inspected + configuration.logger = null_logger("test_configuration") # TODO: the following work-around default values set for these on the # instance that no longer make total sense but fiddling with them # is better as a follow-up. From fcf4097c8203f146432a45a71a0d9c9d06dcb3ed Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 25 Jul 2025 15:13:48 +0200 Subject: [PATCH 2/4] Add support for adding request contextual data to configuration objects. This change adds a small namespace to configuraton objects that behaves like a dictonary allowing objects to made available to e.g. request handlers. Opt to do this by specialising Configuration rather than changing it directly. The rationale is to preserve a distinction between static parts of the configuration of a dpeloyment, which do no change at runtime, versus e.g. a users locale which would be different for each request though consistent for its duration. Such functionality it needed to support templates without restorting to globals within the current system architecture; only congifuration objects are already threaded through most of the places this would need to be available with the correct semantics of an instance being created wherever it is needed. RuntimeConfiguration --- mig/shared/conf.py | 4 +- mig/shared/configuration.py | 60 ++++++++++++++++++++++++++ tests/test_mig_shared_configuration.py | 2 +- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index 0de37b321..32d49852d 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -32,6 +32,7 @@ import os import sys +from mig.shared.configuration import RuntimeConfiguration from mig.shared.defaults import MIG_ENV from mig.shared.fileio import unpickle @@ -42,7 +43,6 @@ def get_configuration_object(config_file=None, skip_log=False, and disable_auth_log arguments are passed on to allow skipping the default log initialization and disabling auth log for unit tests. """ - from mig.shared.configuration import Configuration if config_file: _config_file = config_file elif os.environ.get('MIG_CONF', None): @@ -63,7 +63,7 @@ def get_configuration_object(config_file=None, skip_log=False, skip_log = True disable_auth_log = True - configuration = Configuration(_config_file, False, skip_log, + configuration = RuntimeConfiguration(_config_file, False, skip_log, disable_auth_log) return configuration diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index cd5637020..f72565935 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -804,6 +804,25 @@ def logger(self, logger): else: raise AssertionError("attempted assignment of unsupported logger") + def context(self, namespace=None): + """Retrieve the context or a previously registered namespace. + """ + + if self._context is None: + self._context = {} + if namespace is None: + return self._context + # allow the KeyError to escape if the registered namespace is missing + return self._context[namespace] + + def context_set(self, value, namespace=None): + """Attach a value as named namespace within the active congifuration. + """ + assert namespace is not None + + context = self.context() + context[namespace] = value + return value def reload_config(self, verbose, skip_log=False, disable_auth_log=False, _config_file=None): @@ -2884,6 +2903,47 @@ def parse_peers(self, peerfile): return peers_dict +class RuntimeConfiguration(Configuration): + """A more specific version of the Configuration which additionally supports + the notion of a context. + + Contextual information that is relevant to the duration of a request is + required in certain cases e.g. to support templating. Given Configuration + objects are threaded into and throough almost all the necessary codepaths + to make this information available, they are an attractive place to put + this - but a Configuration is currently loaded from static per-site data. + + Resolv this ambiguity with this subclass - a raw Confioguration will + continute to represent the static data while a specialised but entirely + compatible object is handed to request processing codepaths. + """ + + def __init__(self, config_file, verbose=False, skip_log=False, + disable_auth_log=False): + super().__init__(config_file, verbose, skip_log, disable_auth_log) + self._context = None + + def context(self, namespace=None): + """Retrieve the context or a previously registered namespace. + """ + + if self._context is None: + self._context = {} + if namespace is None: + return self._context + # allow the KeyError to escape if the registered namespace is missing + return self._context[namespace] + + def context_set(self, value, namespace=None): + """Attach a value as named namespace within the active congifuration. + """ + assert namespace is not None + + context = self.context() + context[namespace] = value + return value + + if '__main__' == __name__: conf = Configuration(os.path.expanduser('~/mig/server/MiGserver.conf'), True) diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index 655739fcd..21cb61930 100644 --- a/tests/test_mig_shared_configuration.py +++ b/tests/test_mig_shared_configuration.py @@ -43,7 +43,7 @@ def _is_method(value): def _to_dict(obj): return {k: v for k, v in inspect.getmembers(obj) - if not (k.startswith('_') or k.startswith('logger') or _is_method(v))} + if not (k.startswith('_') or _is_method(v) or k.startswith('logger'))} class MigSharedConfiguration(MigTestCase): From 2db321f1e5965f829dc2bfa671689353ca782997 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 12 Sep 2025 13:43:52 +0200 Subject: [PATCH 3/4] fix mangled rebase --- mig/shared/configuration.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index f72565935..a5453ef26 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -804,26 +804,6 @@ def logger(self, logger): else: raise AssertionError("attempted assignment of unsupported logger") - def context(self, namespace=None): - """Retrieve the context or a previously registered namespace. - """ - - if self._context is None: - self._context = {} - if namespace is None: - return self._context - # allow the KeyError to escape if the registered namespace is missing - return self._context[namespace] - - def context_set(self, value, namespace=None): - """Attach a value as named namespace within the active congifuration. - """ - assert namespace is not None - - context = self.context() - context[namespace] = value - return value - def reload_config(self, verbose, skip_log=False, disable_auth_log=False, _config_file=None): """Re-read and parse configuration file. Optional skip_log arg From 5a36ff285ecfe35b75bf8dec09e6234077997c96 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Mon, 20 Jan 2025 20:36:31 +0100 Subject: [PATCH 4/4] Introduce support for jinja2-based templates. This addds the necessary logic both to maintain a store of templates and makiung them available for serving within any page via a new template object type. Make sure there is a separation of the test templates from the mainline stuff by wiring a TEMPLATES section into Configuration and reading the necessary paths via the loaded configuration. Doing so allows both source location and the cache directory to be optionally specified via the main ini file and thus are runtime accessible without hard-coding. Use this facility within the tests to adjust the paths. --- .gitignore | 1 + mig/assets/templates/.gitkeep | 0 mig/lib/__init__.py | 0 mig/lib/templates/__init__.py | 138 ++++++++++++++++++ mig/lib/templates/__main__.py | 78 ++++++++++ mig/shared/configuration.py | 94 ++++++++++-- mig/shared/objecttypes.py | 24 ++- mig/shared/output.py | 12 +- mig/wsgi-bin/migwsgi.py | 2 +- requirements.txt | 4 + tests/data/MiGserver--customised.conf | 2 + tests/data/templates/test_other.html.jinja | 1 + .../data/templates/test_something.html.jinja | 1 + .../test_objects_with_type_template.html | 1 + tests/test_mig_lib_templates.py | 51 +++++++ tests/test_mig_shared_configuration.py | 17 ++- tests/test_mig_wsgibin.py | 42 +++++- 17 files changed, 449 insertions(+), 19 deletions(-) create mode 100644 mig/assets/templates/.gitkeep create mode 100644 mig/lib/__init__.py create mode 100644 mig/lib/templates/__init__.py create mode 100644 mig/lib/templates/__main__.py create mode 100644 tests/data/templates/test_other.html.jinja create mode 100644 tests/data/templates/test_something.html.jinja create mode 100644 tests/snapshots/test_objects_with_type_template.html create mode 100644 tests/test_mig_lib_templates.py diff --git a/.gitignore b/.gitignore index 3cb78bc1d..910fef556 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ # Byte-compiled / optimized / DLL files +__jinja__/ __pycache__/ *.py[cod] *$py.class diff --git a/mig/assets/templates/.gitkeep b/mig/assets/templates/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/mig/lib/__init__.py b/mig/lib/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/mig/lib/templates/__init__.py b/mig/lib/templates/__init__.py new file mode 100644 index 000000000..92efdc0e6 --- /dev/null +++ b/mig/lib/templates/__init__.py @@ -0,0 +1,138 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# base - shared base helper functions +# Copyright (C) 2003-2024 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# -- END_HEADER --- +# + +import errno +from collections import ChainMap +from jinja2 import meta as jinja2_meta, select_autoescape, Environment, \ + FileSystemLoader, FileSystemBytecodeCache +import os +import weakref + +from mig.shared.compat import PY2 +from mig.shared.defaults import MIG_BASE + + +class _BonundTemplate: + def __init__(self, template, template_args): + self.tmpl = template + self.args = template_args + + + def render(self): + return self.tmpl.render(**self.args) + + +class _FormatContext: + def __init__(self, configuration): + self.output_format = None + self.configuration = configuration + self.script_map = {} + self.style_map = {} + + def __getitem__(self, key): + return self.__dict__[key] + + def __iter__(self): + return iter(self.__dict__) + + def extend(self, template, template_args): + return _BonundTemplate(template, ChainMap(template_args, self)) + + +class TemplateStore: + def __init__(self, template_dirs, cache_dir=None, extra_globals=None): + assert cache_dir is not None + + self._cache_dir = cache_dir + self._template_globals = extra_globals + self._template_environment = Environment( + loader=FileSystemLoader(template_dirs), + bytecode_cache=FileSystemBytecodeCache(cache_dir, '%s'), + autoescape=select_autoescape() + ) + + @property + def cache_dir(self): + return self._cache_dir + + @property + def context(self): + return self._template_globals + + def _get_template(self, template_fqname): + return self._template_environment.get_template(template_fqname) + + def grab_template(self, template_name, template_group, output_format, template_globals=None, **kwargs): + template_fqname = "%s_%s.%s.jinja" % ( + template_group, template_name, output_format) + return self._template_environment.get_template(template_fqname, globals=template_globals) + + def list_templates(self): + return [t for t in self._template_environment.list_templates() if t.endswith('.jinja')] + + def extract_variables(self, template_fqname): + template = self._template_environment.get_template(template_fqname) + with open(template.filename) as f: + template_source = f.read() + ast = self._template_environment.parse(template_source) + return jinja2_meta.find_undeclared_variables(ast) + + @staticmethod + def populated(template_dirs, cache_dir=None, context=None): + assert cache_dir is not None + + try: + os.mkdir(cache_dir) + except OSError as direxc: + if direxc.errno != errno.EEXIST: # FileExistsError + raise + + store = TemplateStore( + template_dirs, cache_dir=cache_dir, extra_globals=context) + + for template_fqname in store.list_templates(): + store._get_template(template_fqname) + + return store + + +def init_global_templates(configuration): + template_division = configuration.division(section_name="TEMPLATES") + + _context = configuration.context() + + try: + return configuration.context(namespace='templates') + except KeyError as exc: + pass + + store = TemplateStore.populated( + [template_division.source_dir], + cache_dir=template_division.cache_dir, + context=_FormatContext(configuration) + ) + return configuration.context_set(store, namespace='templates') diff --git a/mig/lib/templates/__main__.py b/mig/lib/templates/__main__.py new file mode 100644 index 000000000..32453b66d --- /dev/null +++ b/mig/lib/templates/__main__.py @@ -0,0 +1,78 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# base - shared base helper functions +# Copyright (C) 2003-2024 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# -- END_HEADER --- +# + +from types import SimpleNamespace +import os +import sys + +from mig.lib.templates import init_global_templates +from mig.shared.conf import get_configuration_object + + +def warn(message): + print(message, file=sys.stderr, flush=True) + + +def main(args, _print=print): + configuration = get_configuration_object(config_file=args.config_file) + template_store = init_global_templates(configuration) + + command = args.command + if command == 'show': + print(template_store.list_templates()) + elif command == 'prime': + try: + os.mkdir(template_store.cache_dir) + except FileExistsError: + pass + + for template_fqname in template_store.list_templates(): + template_store._get_template(template_fqname) + elif command == 'vars': + for template_ref in template_store.list_templates(): + _print("<%s>" % (template_ref,)) + for var in template_store.extract_variables(template_ref): + _print(" {{%s}}" % (var,)) + _print("" % (template_ref,)) + else: + raise RuntimeError("unknown command: %s" % (command,)) + + +if __name__ == '__main__': + import argparse + + parser = argparse.ArgumentParser() + parser.add_argument('-c', dest='config_file', default=None) + parser.add_argument('command') + args = parser.parse_args() + + try: + main(args) + sys.exit(0) + except Exception as exc: + warn(str(exc)) + sys.exit(1) diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index a5453ef26..b817857cd 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -44,6 +44,7 @@ import socket import sys import time +from types import SimpleNamespace # Init future py2/3 compatibility helpers @@ -60,7 +61,9 @@ # NOTE: protect migrid import from autopep8 reordering try: from mig.shared.base import force_native_str - from mig.shared.defaults import CSRF_MINIMAL, CSRF_WARN, CSRF_MEDIUM, \ + from mig.shared.base import force_native_str + from mig.shared.defaults import MIG_BASE, \ + CSRF_MINIMAL, CSRF_WARN, CSRF_MEDIUM, \ CSRF_FULL, POLICY_NONE, POLICY_WEAK, POLICY_MEDIUM, POLICY_HIGH, \ POLICY_MODERN, POLICY_CUSTOM, freeze_flavors, cert_field_order, \ default_css_filename, keyword_any, keyword_auto, keyword_all, \ @@ -180,14 +183,15 @@ def expand_external_sources(logger, val): return expanded -def fix_missing(config_file, verbose=True): - """Add missing configuration options - used by checkconf script""" +_MARKER_ADMIN_EMAIL = object() +_MARKER_FQDN = object() +_MARKER_USER = object() - config = ConfigParser() - config.read([config_file]) - fqdn = socket.getfqdn() - user = os.environ['USER'] +def _generate_fix_missing_definitions(): + fqdn = _MARKER_FQDN + user = _MARKER_USER + global_section = { 'enable_server_dist': False, 'auto_add_cert_user': False, @@ -200,7 +204,7 @@ def fix_missing(config_file, verbose=True): 'auto_add_filter_fields': '', 'server_fqdn': fqdn, 'support_email': '', - 'admin_email': '%s@%s' % (user, fqdn), + 'admin_email': _MARKER_ADMIN_EMAIL, 'admin_list': '', 'ca_fqdn': '', 'ca_smtp': '', @@ -399,7 +403,14 @@ def fix_missing(config_file, verbose=True): quota_section = {'backend': 'lustre', 'user_limit': 1024**4, 'vgrid_limit': 1024**4} - defaults = { + + default_template_source_dir = os.path.join(MIG_BASE, 'mig/assets/templates') + default_template_cache_dir = os.path.join(default_template_source_dir, '__jinja__') + + templates_section = {'source_dir': default_template_source_dir, + 'cache_dir': default_template_cache_dir} + + return { 'GLOBAL': global_section, 'SCHEDULER': scheduler_section, 'MONITOR': monitor_section, @@ -407,15 +418,38 @@ def fix_missing(config_file, verbose=True): 'FEASIBILITY': feasibility_section, 'WORKFLOWS': workflows_section, 'QUOTA': quota_section, + 'TEMPLATES': templates_section, } - for section in defaults: + + +_FIX_MISSING_DEFINITIONS = _generate_fix_missing_definitions() + + +def fix_missing(config_file, verbose=False): + """Add missing configuration options used by checkconf script""" + + fqdn = socket.getfqdn() + user = os.environ['USER'] + + _marker_substitutions = { + _MARKER_ADMIN_EMAIL: "%s@%s" % (user, fqdn), + _MARKER_FQDN: fqdn, + _MARKER_USER: user, + } + + config = ConfigParser() + config.read([config_file]) + + modified = False + for (section, settings) in _FIX_MISSING_DEFINITIONS.items(): if not section in config.sections(): config.add_section(section) - modified = False - for (section, settings) in defaults.items(): for (option, value) in settings.items(): if not config.has_option(section, option): + if value in _marker_substitutions: + value = _marker_substitutions[value] + if verbose: print('setting %s->%s to %s' % (section, option, value)) @@ -435,6 +469,11 @@ def fix_missing(config_file, verbose=True): fd.close() +def _only_valid_for_section(candididates_dict, section_name): + section_defaults = dict(_FIX_MISSING_DEFINITIONS[section_name]) + return {k: v for k, v in candididates_dict.items() if k in section_defaults} + + class NativeConfigParser(ConfigParser): """Wraps configparser.ConfigParser to force get method to return native string instead of always returning unicode. @@ -767,6 +806,9 @@ def __init__(self, config_file, verbose=False, skip_log=False, self.auth_logger_obj = None self.gdp_logger_obj = None + # structured + self._divisions = {} + configuration_options = copy.deepcopy(_CONFIGURATION_DEFAULTS) for k, v in configuration_options.items(): @@ -1038,6 +1080,9 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, pass raise Exception('Failed to parse configuration: %s' % err) + # handle structured sections + self.apply_loaded_config_to_division(config, section_name='TEMPLATES') + # Remaining options in order of importance - i.e. options needed for # later parsing must be parsed and set first. @@ -2844,6 +2889,31 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, % keyword_all) self.site_twofactor_mandatory_protos = [keyword_all] + def apply_loaded_config_to_division(self, loaded_config, section_name): + active_division = self.division(section_name=section_name) + + try: + templates_candidates = dict(loaded_config[section_name]) + templates_overrides = _only_valid_for_section(templates_candidates, section_name=section_name) + except KeyError: + templates_overrides = {} + + active_division.__dict__.update(templates_overrides) + + self._divisions[section_name] = active_division + + + def division(self, section_name): + if section_name not in _FIX_MISSING_DEFINITIONS: + raise NotImplementedError() + + try: + return self._divisions[section_name] + except KeyError: + new_division = SimpleNamespace(**_FIX_MISSING_DEFINITIONS[section_name]) + self._divisions[section_name] = new_division + return new_division + def parse_peers(self, peerfile): # read peer information from peerfile diff --git a/mig/shared/objecttypes.py b/mig/shared/objecttypes.py index 038d28ef3..52009cb9f 100644 --- a/mig/shared/objecttypes.py +++ b/mig/shared/objecttypes.py @@ -28,9 +28,13 @@ """ Defines valid objecttypes and provides a method to verify if an object is correct """ +from mig.lib.templates import init_global_templates + + start = {'object_type': 'start', 'required': [], 'optional': ['headers' ]} end = {'object_type': 'end', 'required': [], 'optional': []} +template = {'object_type': 'template'} timing_info = {'object_type': 'timing_info', 'required': [], 'optional': []} title = {'object_type': 'title', 'required': ['text'], @@ -324,6 +328,7 @@ valid_types_list = [ start, end, + template, timing_info, title, text, @@ -423,6 +428,8 @@ table_pager, ] +base_template_required = set(('template_name', 'template_group', 'template_args,')) + # valid_types_dict = {"title":title, "link":link, "header":header} # autogenerate dict based on list. Dictionary access is prefered to allow @@ -463,8 +470,8 @@ def get_object_type_info(object_type_list): return out -def validate(input_object): - """ validate input_object """ +def validate(input_object, configuration=None): + """ validate presented objects against their definitions """ if not type(input_object) == type([]): return (False, 'validate object must be a list' % ()) @@ -484,6 +491,19 @@ def validate(input_object): this_object_type = obj['object_type'] valid_object_type = valid_types_dict[this_object_type] + + if this_object_type == 'template': + # the required keys stuff below is not applicable to templates + # because templates know what they need in terms of data thus + # are self-documenting - use this fact to perform validation + #template_ref = "%s_%s.html" % (obj['template_group'], ) + store = init_global_templates(configuration) + template = store.grab_template(obj['template_name'], obj['template_group'], 'html') + valid_object_type = { + 'required': store.extract_variables(template) + } + obj = obj.get('template_args', None) + if 'required' in valid_object_type: for req in valid_object_type['required']: if req not in obj: diff --git a/mig/shared/output.py b/mig/shared/output.py index 2db2c0ca1..91a66431e 100644 --- a/mig/shared/output.py +++ b/mig/shared/output.py @@ -43,6 +43,7 @@ import time import traceback +from mig.lib.templates import init_global_templates from mig.shared import returnvalues from mig.shared.bailout import bailout_title, crash_helper, \ filter_output_objects @@ -740,6 +741,15 @@ def html_format(configuration, ret_val, ret_msg, out_obj): for i in out_obj: if i['object_type'] == 'start': pass + elif i['object_type'] == 'template': + store = init_global_templates(configuration) + template = store.grab_template( + i['template_name'], + i['template_group'], + 'html', + ) + bound = store.context.extend(template, i['template_args']) + lines.append(bound.render()) elif i['object_type'] == 'error_text': msg = "%(text)s" % i if i.get('exc', False): @@ -2818,7 +2828,7 @@ def format_output( logger = configuration.logger #logger.debug("format output to %s" % outputformat) valid_formats = get_valid_outputformats() - (val_ret, val_msg) = validate(out_obj) + (val_ret, val_msg) = validate(out_obj, configuration) if not val_ret: logger.error("%s formatting failed: %s (%s)" % (outputformat, val_msg, val_ret)) diff --git a/mig/wsgi-bin/migwsgi.py b/mig/wsgi-bin/migwsgi.py index 073958880..07b1102a8 100755 --- a/mig/wsgi-bin/migwsgi.py +++ b/mig/wsgi-bin/migwsgi.py @@ -132,7 +132,7 @@ def stub(configuration, client_id, import_path, backend, user_arguments_dict, crash_helper(configuration, backend, output_objects) return (output_objects, returnvalues.ERROR) - (val_ret, val_msg) = validate(output_objects) + (val_ret, val_msg) = validate(output_objects, configuration=configuration) if not val_ret: (ret_code, ret_msg) = returnvalues.OUTPUT_VALIDATION_ERROR bailout_helper(configuration, backend, output_objects, diff --git a/requirements.txt b/requirements.txt index 5c2b1bc8f..f04b99f31 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,6 +26,10 @@ email-validator;python_version >= "3.7" email-validator<2.0;python_version >= "3" and python_version < "3.7" email-validator<1.3;python_version < "3" +jinja2<3;python_version < "3" +jinja2==3.0.*;python_version >= "3" and python_version < "3.7" +jinja2;python_version >= "3.7" + # NOTE: additional optional dependencies depending on site conf are listed # in recommended.txt and can be installed in the same manner by pointing # pip there. diff --git a/tests/data/MiGserver--customised.conf b/tests/data/MiGserver--customised.conf index a39336d73..f5b2960fb 100644 --- a/tests/data/MiGserver--customised.conf +++ b/tests/data/MiGserver--customised.conf @@ -806,3 +806,5 @@ logo_right = /images/skin/migrid-basic/logo-right.png # Optional data safety notice and popup on Files page datasafety_link = datasafety_text = + +[TEMPLATES] diff --git a/tests/data/templates/test_other.html.jinja b/tests/data/templates/test_other.html.jinja new file mode 100644 index 000000000..f22ed0101 --- /dev/null +++ b/tests/data/templates/test_other.html.jinja @@ -0,0 +1 @@ +{{ other }} diff --git a/tests/data/templates/test_something.html.jinja b/tests/data/templates/test_something.html.jinja new file mode 100644 index 000000000..4c912a491 --- /dev/null +++ b/tests/data/templates/test_something.html.jinja @@ -0,0 +1 @@ + diff --git a/tests/snapshots/test_objects_with_type_template.html b/tests/snapshots/test_objects_with_type_template.html new file mode 100644 index 000000000..8eaf5bde5 --- /dev/null +++ b/tests/snapshots/test_objects_with_type_template.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/test_mig_lib_templates.py b/tests/test_mig_lib_templates.py new file mode 100644 index 000000000..413b4d439 --- /dev/null +++ b/tests/test_mig_lib_templates.py @@ -0,0 +1,51 @@ +import os +import shutil + +from tests.support import MigTestCase, testmain, \ + MIG_BASE, TEST_DATA_DIR, TEST_OUTPUT_DIR + +from mig.lib.templates import TemplateStore, init_global_templates + +TEST_CACHE_DIR = os.path.join(TEST_OUTPUT_DIR, '__template_cache__') +TEST_TMPL_DIR = os.path.join(TEST_DATA_DIR, 'templates') + + +class TestMigSharedTemplates_instance(MigTestCase): + def after_each(self): + shutil.rmtree(TEST_CACHE_DIR, ignore_errors=True) + + def _provide_configuration(self): + return 'testconfig' + + def test_the_creation_of_a_template_store(self): + store = TemplateStore.populated(TEST_TMPL_DIR, cache_dir=TEST_CACHE_DIR) + self.assertIsInstance(store, TemplateStore) + + def test_a_listing_all_templates(self): + store = TemplateStore.populated(TEST_TMPL_DIR, cache_dir=TEST_CACHE_DIR) + self.assertEqual(len(store.list_templates()), 2) + + def test_grab_template(self): + store = TemplateStore.populated(TEST_TMPL_DIR, cache_dir=TEST_CACHE_DIR) + template = store.grab_template('other', 'test', 'html') + pass + + def test_variables_for_remplate_ref(self): + store = TemplateStore.populated(TEST_TMPL_DIR, cache_dir=TEST_CACHE_DIR) + template_vars = store.extract_variables('test_something.html.jinja') + self.assertEqual(template_vars, set(['content'])) + + +class TestMigSharedTemplates_global(MigTestCase): + def _provide_configuration(self): + return 'testconfig' + + def test_cache_location(self): + store = init_global_templates(self.configuration) + + relative_cache_dir = os.path.relpath(store.cache_dir, MIG_BASE) + self.assertEqual(relative_cache_dir, 'mig/assets/templates/__jinja__') + + +if __name__ == '__main__': + testmain() diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index 21cb61930..64c39b5da 100644 --- a/tests/test_mig_shared_configuration.py +++ b/tests/test_mig_shared_configuration.py @@ -31,8 +31,8 @@ import os import unittest -from tests.support import MigTestCase, TEST_DATA_DIR, PY2, testmain, \ - fixturefile +from tests.support import MigTestCase, MIG_BASE, TEST_DATA_DIR, PY2, \ + testmain, fixturefile from mig.shared.configuration import Configuration from mig.shared.logger import null_logger @@ -315,6 +315,19 @@ def test_argument_include_sections_multi_ignores_other_sections(self): # TODO: rename file to valid section name we can check and enable next? # self.assertEqual(configuration.multi, 'blabla') + def test_structured_templates_defaults(self): + test_conf_file = os.path.join( + TEST_DATA_DIR, 'MiGserver--customised.conf') + + configuration = Configuration( + test_conf_file, skip_log=True, disable_auth_log=True) + + division = configuration.division(section_name='TEMPLATES') + self.assertEqual(division.__dict__, { + 'source_dir': os.path.join(MIG_BASE, 'mig/assets/templates'), + 'cache_dir': os.path.join(MIG_BASE, 'mig/assets/templates/__jinja__'), + }) + @unittest.skipIf(PY2, "Python 3 only") def test_default_object(self): prepared_fixture = self.prepareFixtureAssert( diff --git a/tests/test_mig_wsgibin.py b/tests/test_mig_wsgibin.py index 857d03b99..724331d34 100644 --- a/tests/test_mig_wsgibin.py +++ b/tests/test_mig_wsgibin.py @@ -34,7 +34,8 @@ import stat import sys -from tests.support import PY2, MIG_BASE, MigTestCase, testmain, is_path_within +from tests.support import PY2, MIG_BASE, TEST_DATA_DIR, TEST_OUTPUT_DIR, \ + MigTestCase, testmain from tests.support.snapshotsupp import SnapshotAssertMixin from tests.support.wsgisupp import prepare_wsgi, WsgiAssertMixin @@ -49,6 +50,18 @@ from html.parser import HTMLParser +def _force_test_templates(configuration): + templates_division = configuration.division('TEMPLATES') + # overwrite the template source and cache directory to known locations for + # the duration of the tests. thyis allows us to control which templates are + # available as well as ensures the template cache directory is cleaned out + # on test completion as part of the standard output directory cleanup + templates_division.__dict__.update({ + 'source_dir': os.path.join(TEST_DATA_DIR, 'templates'), + 'cache_dir': os.path.join(TEST_OUTPUT_DIR, '__jinja__'), + }) + + class DocumentBasicsHtmlParser(HTMLParser): """An HTML parser using builtin machinery to check basic html structure.""" @@ -303,6 +316,33 @@ def test_objects_with_type_text(self): output, _ = self.assertWsgiResponse(wsgi_result, self.fake_wsgi, 200) self.assertSnapshotOfHtmlContent(output) + def test_objects_with_type_template(self): + output_objects = [ + # workaround invalid HTML being generated with no title object + { + 'object_type': 'title', + 'text': 'TEST' + }, + { + 'object_type': 'template', + 'template_name': 'something', + 'template_group': 'test', + 'template_args': { + 'content': 'here!!' + } + } + ] + self.fake_backend.set_response(output_objects, returnvalues.OK) + _force_test_templates(self.configuration) + + wsgi_result = migwsgi.application( + *self.application_args, + **self.application_kwargs + ) + + output, _ = self.assertWsgiResponse(wsgi_result, self.fake_wsgi, 200) + self.assertSnapshotOfHtmlContent(output) + if __name__ == '__main__': testmain()