Some bugfixes and cleanup for actor config code.

* Make normalize_schemas() public.  The caller (normally in the workflow) will read the schemas and
  then pass them to load() so it also needs to call normalize_schema() in between those two steps.
* Emit separate warnings for unknown section names and unknown field names
* Remove usage of added_field variable which was replaced in a refactor with using the dictionary of
  schemas (which have the same information as keys of nested dictionaries).
* Remove some functions that have been rewritten after confirming that all of their functionality
  has been ported.
* Add a comment specifically for what is happening with format_config() (Not needed for MVP).
* Remove config_schemas from the RepositoryManager class.  Instead, we will have any caller extract
  that information from RepositoryManager.actors.
* Don't warn for every field in a section that isn't recognized.
* Add both section and field to warning messages about unknown fields.
* Make use of field instead of getting it from the schema dict again.
* Fix name of the validate_field_type function.
* Enable config directory inside of repositories. Configuration schema information can live in
  repository-wide and actor-wide locations.  Try to enable both of those.
* Add pyyaml to the requirements (requirements.txt and spec file)

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=())
     ])
 

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,15 +135,15 @@ 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)
 
     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,75 +270,33 @@ 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
 
 
 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)

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):
         """

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)

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):
         """

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
 ##################################################

requirements.txt

@@ -1,2 +1,3 @@
 six==1.12
 requests
+PyYAML