config: Don't abort on deprecated options

These options are in fact removed and have no effect anymore. This patch
changes the validation to print a warning that the option was removed
and what should be done instead. It no longer stops the whole compose.

The validation script still rejects configuration files with these
removed keys.

This change means we no longer check these removals with the JSON schema
(as that makes it hard to determine where exactly the problem is).

Fixes: #438
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

bin/pungi-config-validate

@@ -65,8 +65,9 @@ def run(config, topdir, has_old):
     conf.load_from_file(config)
 
     errors = pungi.checks.validate(conf)
-    if errors:
+    warnings = list(pungi.checks.report_removed(conf))
-        for error in errors:
+    if errors or warnings:
+        for error in errors + warnings:
             print(error)
         sys.exit(1)
 

bin/pungi-koji

@@ -174,6 +174,8 @@ def main():
         sys.exit(1)
     pungi.checks.check_umask(logger)
     errors = pungi.checks.validate(conf)
+    for warning in pungi.checks.report_removed(conf):
+        print >>sys.stderr, warning
     if errors:
         for error in errors:
             print >>sys.stderr, error

pungi/checks.py

@@ -180,24 +180,29 @@ def validate(config):
                                   'regex': (str, unicode)})
     errors = []
     for error in validator.iter_errors(config):
-        if isinstance(error, ConfigDeprecation):
+        if not error.path and error.validator == 'additionalProperties':
-            errors.append(DEPRECATED.format('.'.join(error.path), error.message))
+            allowed_keys = set(error.schema['properties'].keys())
+            used_keys = set(error.instance.keys())
+            for key in used_keys - allowed_keys:
+                suggestion = _get_suggestion(key, allowed_keys)
+                if suggestion:
+                    errors.append(UNKNOWN_SUGGEST.format(key, suggestion))
+                else:
+                    errors.append(UNKNOWN.format(key))
         else:
-            if not error.path and error.validator == 'additionalProperties':
+            errors.append('Failed validation in %s: %s' % (
-                allowed_keys = set(error.schema['properties'].keys())
+                '.'.join([str(x) for x in error.path]), error.message))
-                used_keys = set(error.instance.keys())
-                for key in used_keys - allowed_keys:
-                    suggestion = _get_suggestion(key, allowed_keys)
-                    if suggestion:
-                        errors.append(UNKNOWN_SUGGEST.format(key, suggestion))
-                    else:
-                        errors.append(UNKNOWN.format(key))
-            else:
-                errors.append('Failed validation in %s: %s' % (
-                    '.'.join([str(x) for x in error.path]), error.message))
     return errors + _validate_requires(schema, config, CONFIG_DEPS)
 
 
+def report_removed(config):
+    schema = _make_schema()
+    for key in config:
+        msg = schema['properties'].get(key, {}).get('deprecated')
+        if msg:
+            yield REMOVED.format(key, msg)
+
+
 def _get_suggestion(desired, names):
     """Find a value in ``names`` that is the closest match for ``desired``.
 
@@ -214,11 +219,11 @@ def _get_suggestion(desired, names):
     return closest
 
 
-CONFLICTS = 'Config option {0}={1} conflicts with option {2}.'
+CONFLICTS = 'ERROR: Config option {0}={1} conflicts with option {2}.'
-REQUIRES = 'Config option {0}={1} requires {2} which is not set.'
+REQUIRES = 'ERROR: Config option {0}={1} requires {2} which is not set.'
-DEPRECATED = 'Deprecated config option: {0}; {1}.'
+REMOVED = 'WARNING: Config option {0} was removed and has no effect; {1}.'
-UNKNOWN = 'Unrecognized config option: {0}.'
+UNKNOWN = 'ERROR: Unrecognized config option: {0}.'
-UNKNOWN_SUGGEST = 'Unrecognized config option: {0}. Did you mean {1}?'
+UNKNOWN_SUGGEST = 'ERROR: Unrecognized config option: {0}. Did you mean {1}?'
 
 
 def _extend_with_default(validator_class):
@@ -237,12 +242,6 @@ def _extend_with_default(validator_class):
         for error in validate_properties(validator, properties, instance, schema):
             yield error
 
-    def error_on_deprecated(validator, properties, instance, schema):
-        """Unconditionally raise deprecation error if encountered."""
-        yield ConfigDeprecation(
-            'use %s instead' % properties
-        )
-
     def validate_regex_type(validator, properties, instance, schema):
         """
         Extend standard type validation to check correctness in regular
@@ -262,15 +261,10 @@ def _extend_with_default(validator_class):
 
     return jsonschema.validators.extend(
         validator_class, {"properties": set_defaults,
-                          "deprecated": error_on_deprecated,
                           "type": validate_regex_type},
     )
 
 
-class ConfigDeprecation(jsonschema.exceptions.ValidationError):
-    pass
-
-
 def _make_schema():
     return {
         "$schema": "http://json-schema.org/draft-04/schema#",
@@ -588,7 +582,7 @@ def _make_schema():
                 "default": True,
             },
             "keep_original_comps": {
-                "deprecated": "no <groups> tag for respective variant in variants XML"
+                "deprecated": "remove <groups> tag from respective variant in variants XML"
             },
 
             "link_type": {
@@ -822,25 +816,25 @@ def _make_schema():
 
             # Deprecated options
             "multilib_arches": {
-                "deprecated": "multilib"
+                "deprecated": "use multilib instead"
             },
             "multilib_methods": {
-                "deprecated": "multilib"
+                "deprecated": "use multilib instead"
             },
             "additional_packages_multiarch": {
-                "deprecated": "multilib_whitelist"
+                "deprecated": "use multilib_whitelist instead"
             },
             "filter_packages_multiarch": {
-                "deprecated": "multilib_blacklist"
+                "deprecated": "use multilib_blacklist instead"
             },
             "buildinstall_upgrade_image": {
-                "deprecated": "lorax_options"
+                "deprecated": "use lorax_options instead"
             },
             "pkgset_koji_path_prefix": {
-                "deprecated": "koji_profile",
+                "deprecated": "use koji_profile instead",
             },
             "pkgset_koji_url": {
-                "deprecated": "koji_profile",
+                "deprecated": "use koji_profile instead",
             },
         },
 

tests/test_config.py

@@ -194,9 +194,11 @@ class BuildinstallConfigTestCase(unittest.TestCase):
             buildinstall_upgrade_image=True,
         )
 
+        self.assertItemsEqual(checks.validate(cfg), [])
+
         self.assertItemsEqual(
-            checks.validate(cfg),
+            checks.report_removed(cfg),
-            [checks.DEPRECATED.format('buildinstall_upgrade_image', 'use lorax_options instead')]
+            [checks.REMOVED.format('buildinstall_upgrade_image', 'use lorax_options instead')]
         )