diff --git a/leapp/actors/__init__.py b/leapp/actors/__init__.py index 0828aba6..c15691ed 100644 --- a/leapp/actors/__init__.py +++ b/leapp/actors/__init__.py @@ -483,7 +483,7 @@ def get_actor_metadata(actor): _get_attribute(actor, 'description', _is_type(string_types), required=False, default_value=actor.__doc__ or 'There has been no description provided for this actor.'), _get_attribute(actor, 'config_schemas', _is_type(string_types), required=False, - default_value=actor.__doc__ or 'Description of the configuration used by this actor.') + default_value=actor.__doc__ or 'Description of the configuration used by this actor.'), _get_attribute(actor, 'apis', _is_api_tuple, required=False, default_value=()) ]) diff --git a/leapp/actors/config.py b/leapp/actors/config.py index 4ee119ce..75930efd 100644 --- a/leapp/actors/config.py +++ b/leapp/actors/config.py @@ -20,10 +20,8 @@ import abc import glob -import importlib import logging import os.path -import pkgutil from collections import defaultdict import six @@ -52,6 +50,9 @@ class ValidationError(Exception): """ +# pylint: disable=deprecated-decorator +# @abc.abstractproperty is deprecated in newer Python3 versions but it's +# necessary for Python <= 3.3 (including 2.7) @six.add_metaclass(abc.ABCMeta) class Config: """ @@ -96,10 +97,12 @@ def to_dict(cls): '{0}_description__'.format(cls.name): cls.description } } - ### TODO: Retrieve the default values from the type field. - # representation[cls.section][cls.name] = cls.type_.get_default() + # TODO: Retrieve the default values from the type field. + # Something like this maybe: + # representation[cls.section][cls.name] = cls.type_.get_default() return representation +# pylint: enable=deprecated-decorator def _merge_config(configuration, new_config): @@ -132,7 +135,7 @@ def _get_config(config_dir='/etc/leapp/actor_conf.d'): parsed_config = yaml.load(raw_cfg, SafeLoader) except Exception as e: log.warning("Warning: unparsable yaml file %s in the config directory." - " Error: %s", filename, str(e)) + " Error: %s", config_file, str(e)) raise _merge_config(configuration, parsed_config) @@ -140,7 +143,7 @@ def _get_config(config_dir='/etc/leapp/actor_conf.d'): return configuration -def _normalize_schemas(schemas): +def normalize_schemas(schemas): """ Merge all schemas into a single dictionary and validate them for errors we can detect. """ @@ -152,8 +155,13 @@ def _normalize_schemas(schemas): # Error if the field has been added by another schema if unique_name in added_fields and added_fields[unique_name] != field: - # TODO: Also have information on what Actor contains the conflicting fields - message = "Two actors added incompatible values for {name}".format(name=unique_name)) + # TODO: Also include information on what Actor contains the + # conflicting fields but that information isn't passed into + # this function right now. + message = ('Two actors added incompatible configuration items' + ' with the same name for Section: {section},' + ' Field: {field}'.format(section=field.section, + field=field.name)) log.error(message) raise SchemaError(message) @@ -175,21 +183,43 @@ def _validate_field_type(field_type, field_value): # with. This might not work right or there might be a much better way. try: field_type.create(field_value) - except Exception: + except Exception as e: # pylint: disable=broad-exception-caught + # Any problems mean that the field did not validate. + log.info("Configuration value failed to validate with:" + " %{exc}".format(exc=str(e))) return False return True def _normalize_config(actor_config, schema): + # Validate that the config values read from the config files obey the known + # structure. for section_name, section in actor_config.items(): + if section_name not in schema: + # TODO: Also have information about which config file contains the unknown field. + message = "A config file contained an unknown section: {section}".format(section=section_name) + log.warning(message) + continue + for field_name in actor_config: - if (section_name, field_name) not in added_fields: + # Any field names which end in "__" are reserved for LEAPP to use + # for its purposes. In particular, it places documentation of + # a field's value into these reserved field names. + if field_name.endswith("__"): + continue + + if field_name not in schema[section_name]: # TODO: Also have information about which config file contains the unknown field. - message = "A config file contained an unknown field: {name}".format(name=(section_name, field_name)) + message = ("A config file contained an unknown field: (Section:" + " {section}, Field: {field})".format( + section=section_name, field=field_name) + ) log.warning(message) + # Do several things: + # * Validate that the config values are of the proper types. + # * Add default values where no config value was provided. normalized_actor_config = {} - for section_name, section in schema.items(): for field_name, field in section.items(): # TODO: We might be able to do this using the default piece of @@ -209,10 +239,14 @@ def _normalize_config(actor_config, schema): # May need to deepcopy default if these values are modified. # However, it's probably an error if they are modified and we # should possibly look into disallowing that. - value = section[field_name] = schema[section_name][field_name].default + value = section[field_name] = field.default - if not _validate_field(schema[section_name][field_name].type_, value): - raise ValidationError("Config value for {name} is not of the correct type".format(name=(section_name, field_name))) + if not _validate_field_type(field.type_, value): + raise ValidationError("Config value for (Section: {section}," + " Field: {field}) is not of the correct" + " type".format(section=section_name, + field=field_name) + ) normalized_section = normalized_actor_config.get(section_name, {}) normalized_section[field_name] = value @@ -236,11 +270,8 @@ def load(config_dir, schemas): if _ACTOR_CONFIG: return _ACTOR_CONFIG - # TODO: Move this to the caller - schema = _normalize_schemas(schemas) - # End TODO config = _get_config(config_dir) - config = _normalize_config(config, schema) + config = _normalize_config(config, schemas) _ACTOR_CONFIG = config return _ACTOR_CONFIG @@ -248,63 +279,24 @@ def load(config_dir, schemas): def retrieve_config(schema): """Called by the actor to retrieve the actor configuration specific to this actor.""" - # TODO: This isn't good API. Since this function is called by the Actors, - # we *know* that this is okay to do (as the configuration will have already - # been loaded.) However, there's nothing in the API that ensures that this - # is the case. Need to redesign this. Can't think of how it should look - # right now because loading requires information that the Actor doesn't - # know. - global _ACTOR_CONFIG - all_actor_config = _ACTOR_CONFIG + # TODO: The use of _ACTOR_CONFIG isn't good API. Since this function is + # called by the Actors, we *know* that this is okay to do (as the + # configuration will have already been loaded.) However, there's nothing in + # the API that ensures that this is the case. Need to redesign this. + # Can't think of how it should look right now because loading requires + # information that the Actor doesn't know. configuration = defaultdict(dict) for field in schema: - configuration[field.section][field.name] = all_actor_config[field.section][field.name] + configuration[field.section][field.name] = _ACTOR_CONFIG[field.section][field.name] return configuration # -# From this point down hasn't been re-evaluated to see if it's needed or how it -# fits into the bigger picture. Some of it definitely has been implemented in -# a different way above but not all of it. +# The function after this needs some work to be ready. It isn't part of the +# upgrade or preupgrade workflows so we don't have to get it finished yet. # -def parse_repo_config_files(): - repo_config = {} - for config in all_repository_config_schemas(): - section_name = config.section - - if section_name not in repo_config: - repo_config.update(config.to_dict()) - else: - if '{0}_description__'.format(config_item.name) in repo_config[config.section]: - raise Exception("Error: Two configuration items are declared with the same name Section: {0}, Key: {1}".format(config.section, config.name)) - - repo_config[config.section].update(config.to_dict()[config.section]) - - return repo_config - - -def parse_config_files(config_dir): - """ - Parse all configuration and return a dict with those values. - """ - config = parse_repo_config_files() - system_config = parse_system_config_files(config_dir) - - for section, config_items in system_config.items(): - if section not in config: - print('WARNING: config file contains an unused section: Section: {0}'.format(section)) - config.update[section] = config_items - else: - for key, value in config_items: - if '{0}_description__'.format(key) not in config[section]: - print('WARNING: config file contains an unused config entry: Section: {0}, Key{1}'.format(section, key)) - - config[section][key] = value - - return config - def format_config(): """ @@ -330,8 +322,6 @@ def format_config(): - leapp-repository - snactor """ - return SafeDumper(yaml.dump(parse_config_files(), dumper=SafeDumper)) - if schemas != actor_config: - raise Exception("Invalid entries in the actor config files") - - global_ _ACTOR_CONFIG_VALIDATED = True + # TODO: This is just a placeholder. We need to do some additional + # formatting that includes the documentation, not just return it as is. + return yaml.dump(_ACTOR_CONFIG, SafeDumper) diff --git a/leapp/repository/actor_definition.py b/leapp/repository/actor_definition.py index 2f06f6e7..bab23e22 100644 --- a/leapp/repository/actor_definition.py +++ b/leapp/repository/actor_definition.py @@ -369,6 +369,13 @@ def files(self): """ return tuple(self._definitions.get(DefinitionKind.FILES, ())) + @property + def configs(self): + """ + :return: Tuple with path to the configs folder of the actor, empty tuple if none + """ + return tuple(self._definitions.get(DefinitionKind.CONFIGS, ())) + @property def tests(self): """ diff --git a/leapp/repository/definition.py b/leapp/repository/definition.py index e853a6a5..344ec77e 100644 --- a/leapp/repository/definition.py +++ b/leapp/repository/definition.py @@ -14,8 +14,9 @@ def __init__(self, kind): LIBRARIES = _Kind('libraries') TOOLS = _Kind('tools') FILES = _Kind('files') + CONFIGS = _Kind('configs') TESTS = _Kind('tests') API = _Kind('api') - REPO_WHITELIST = (ACTOR, API, MODEL, TOPIC, TAG, WORKFLOW, TOOLS, LIBRARIES, FILES) - ACTOR_WHITELIST = (TOOLS, LIBRARIES, FILES, TESTS) + REPO_WHITELIST = (ACTOR, API, MODEL, TOPIC, TAG, WORKFLOW, TOOLS, LIBRARIES, FILES, CONFIGS) + ACTOR_WHITELIST = (TOOLS, LIBRARIES, FILES, CONFIGS, TESTS) diff --git a/leapp/repository/manager.py b/leapp/repository/manager.py index b1725263..7d6cb227 100644 --- a/leapp/repository/manager.py +++ b/leapp/repository/manager.py @@ -139,12 +139,6 @@ def actors(self): """ return tuple(itertools.chain(*[repo.actors for repo in self._repos.values()])) - @property - def config_schemas(self): - """ - """ - return tuple(itertools.chain(*[repo.config_schemas for repo in self._repos.values()])) - @property def topics(self): """ diff --git a/packaging/leapp.spec b/packaging/leapp.spec index ab38d207..29ece534 100644 --- a/packaging/leapp.spec +++ b/packaging/leapp.spec @@ -134,6 +134,7 @@ Provides: leapp-framework-dependencies = %{framework_dependencies} Requires: python-six Requires: python-setuptools Requires: python-requests +Requires: PyYAML %else # <> rhel 7 # for Fedora & RHEL 8+ deliver just python3 stuff # NOTE: requirement on python3 refers to the general version of Python @@ -144,6 +145,7 @@ Requires: python3 Requires: python3-six Requires: python3-setuptools Requires: python3-requests +Requires: python3-PyYAML %endif Requires: findutils ################################################## diff --git a/requirements.txt b/requirements.txt index ec831ad6..2951f23d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,3 @@ six==1.12 requests +PyYAML