almalinux
/
pungi
15
1
Fork 0
Code Issues Pull Requests Releases Activity

config: Report unknown options as warnings 

Given the way config files can include other files, it is entirely
possible to use the same config file for both Pungi and Distill. Pungi
will however complain about unknown options.

This patch reverts part of c38bb480 and moves deprecation handling back
into schema validation. The validation method then returns a list of
errors and a list of warnings.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář 2016-12-07 15:57:35 +01:00
parent ec6206b064
commit c338219ef0
8 changed files with 121 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
bin/pungi-config-validate
View File

@ -64,8 +64,7 @@ def run(config, topdir, has_old):
conf = kobo.conf.PyConfigParser()
conf.load_from_file(config)
errors = pungi.checks.validate(conf)
warnings = list(pungi.checks.report_removed(conf))
errors, warnings = pungi.checks.validate(conf)
if errors or warnings:
for error in errors + warnings:
print(error)

4
bin/pungi-koji
View File

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

33
pungi/checks.py
View File

@ -192,28 +192,24 @@ def validate(config):
{'array': (tuple, list),
'regex': (str, unicode)})
errors = []
warnings = []
for error in validator.iter_errors(config):
if not error.path and error.validator == 'additionalProperties':
if isinstance(error, ConfigDeprecation):
warnings.append(REMOVED.format('.'.join(error.path), error.message))
elif not error.path and error.validator == 'additionalProperties':
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))
warnings.append(UNKNOWN_SUGGEST.format(key, suggestion))
else:
errors.append(UNKNOWN.format(key))
warnings.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)
return (errors + _validate_requires(schema, config, CONFIG_DEPS),
warnings)
def _get_suggestion(desired, names):
@ -235,8 +231,8 @@ def _get_suggestion(desired, names):
CONFLICTS = 'ERROR: Config option {0}={1} conflicts with option {2}.'
REQUIRES = 'ERROR: Config option {0}={1} requires {2} which is not set.'
REMOVED = 'WARNING: Config option {0} was removed and has no effect; {1}.'
UNKNOWN = 'ERROR: Unrecognized config option: {0}.'
UNKNOWN_SUGGEST = 'ERROR: Unrecognized config option: {0}. Did you mean {1}?'
UNKNOWN = 'WARNING: Unrecognized config option: {0}.'
UNKNOWN_SUGGEST = 'WARNING: Unrecognized config option: {0}. Did you mean {1}?'
def _extend_with_default(validator_class):
@ -255,6 +251,10 @@ 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(properties)
def validate_regex_type(validator, properties, instance, schema):
"""
Extend standard type validation to check correctness in regular
@ -274,10 +274,15 @@ 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#",

3
tests/helpers.py
View File

@ -25,6 +25,9 @@ class PungiTestCase(unittest.TestCase):
if err.errno != errno.ENOENT:
raise
def assertValidConfig(self, conf):
self.assertEqual(checks.validate(conf), ([], []))
class MockVariant(mock.Mock):
def __str__(self):

150
tests/test_config.py
View File

@ -16,7 +16,14 @@ from pungi import checks
from tests.helpers import load_config, PKGSET_REPOS
class PkgsetConfigTestCase(unittest.TestCase):
class ConfigTestCase(unittest.TestCase):
def assertValidation(self, cfg, errors=[], warnings=[]):
actual_errors, actual_warnings = checks.validate(cfg)
self.assertItemsEqual(errors, actual_errors)
self.assertEqual(warnings, actual_warnings)
class PkgsetConfigTestCase(ConfigTestCase):
def test_validate_minimal_pkgset_koji(self):
cfg = load_config(
@ -24,7 +31,7 @@ class PkgsetConfigTestCase(unittest.TestCase):
pkgset_koji_tag="f25",
)
self.assertEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_validate_minimal_pkgset_repos(self):
cfg = load_config(
@ -32,7 +39,7 @@ class PkgsetConfigTestCase(unittest.TestCase):
pkgset_repos={'x86_64': '/first', 'ppc64': '/second'},
)
self.assertEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_pkgset_mismatch_repos(self):
cfg = load_config(
@ -41,8 +48,8 @@ class PkgsetConfigTestCase(unittest.TestCase):
pkgset_koji_inherit=False,
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.REQUIRES.format('pkgset_source', 'repos', 'pkgset_repos'),
checks.CONFLICTS.format('pkgset_source', 'repos', 'pkgset_koji_tag'),
checks.CONFLICTS.format('pkgset_source', 'repos', 'pkgset_koji_inherit')])
@ -53,21 +60,21 @@ class PkgsetConfigTestCase(unittest.TestCase):
pkgset_repos={'whatever': '/foo'},
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.REQUIRES.format('pkgset_source', 'koji', 'pkgset_koji_tag'),
checks.CONFLICTS.format('pkgset_source', 'koji', 'pkgset_repos')])
class ReleaseConfigTestCase(unittest.TestCase):
class ReleaseConfigTestCase(ConfigTestCase):
def test_layered_without_base_product(self):
cfg = load_config(
PKGSET_REPOS,
release_is_layered=True
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.REQUIRES.format('release_is_layered', 'True', 'base_product_name'),
checks.REQUIRES.format('release_is_layered', 'True', 'base_product_short'),
checks.REQUIRES.format('release_is_layered', 'True', 'base_product_version')])
@ -81,23 +88,23 @@ class ReleaseConfigTestCase(unittest.TestCase):
base_product_type='updates',
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_name'),
checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_short'),
checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_type'),
checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_version')])
class RunrootConfigTestCase(unittest.TestCase):
class RunrootConfigTestCase(ConfigTestCase):
def test_runroot_without_deps(self):
cfg = load_config(
PKGSET_REPOS,
runroot=True,
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.REQUIRES.format('runroot', 'True', 'koji_profile'),
checks.REQUIRES.format('runroot', 'True', 'runroot_tag'),
checks.REQUIRES.format('runroot', 'True', 'runroot_channel')])
@ -111,23 +118,22 @@ class RunrootConfigTestCase(unittest.TestCase):
runroot_channel='compose',
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.CONFLICTS.format('runroot', 'False', 'runroot_tag'),
checks.CONFLICTS.format('runroot', 'False', 'runroot_channel')])
class BuildinstallConfigTestCase(unittest.TestCase):
class BuildinstallConfigTestCase(ConfigTestCase):
def test_bootable_without_method(self):
cfg = load_config(
PKGSET_REPOS,
bootable=True,
)
self.assertItemsEqual(
checks.validate(cfg),
[checks.REQUIRES.format('bootable', 'True', 'buildinstall_method')]
)
self.assertValidation(
cfg,
[checks.REQUIRES.format('bootable', 'True', 'buildinstall_method')])
def test_non_bootable_with_method(self):
cfg = load_config(
@ -136,10 +142,9 @@ class BuildinstallConfigTestCase(unittest.TestCase):
buildinstall_method='lorax',
)
self.assertItemsEqual(
checks.validate(cfg),
[checks.CONFLICTS.format('bootable', 'False', 'buildinstall_method')]
)
self.assertValidation(
cfg,
[checks.CONFLICTS.format('bootable', 'False', 'buildinstall_method')])
def test_buildinstall_method_without_bootable(self):
cfg = load_config(
@ -147,10 +152,9 @@ class BuildinstallConfigTestCase(unittest.TestCase):
buildinstall_method='lorax',
)
self.assertItemsEqual(
checks.validate(cfg),
[checks.CONFLICTS.format('bootable', 'False', 'buildinstall_method')]
)
self.assertValidation(
cfg,
[checks.CONFLICTS.format('bootable', 'False', 'buildinstall_method')])
def test_buildinstall_with_lorax_options(self):
cfg = load_config(
@ -160,10 +164,9 @@ class BuildinstallConfigTestCase(unittest.TestCase):
lorax_options=[('^Server$', {})]
)
self.assertItemsEqual(
checks.validate(cfg),
[checks.CONFLICTS.format('buildinstall_method', 'buildinstall', 'lorax_options')]
)
self.assertValidation(
cfg,
[checks.CONFLICTS.format('buildinstall_method', 'buildinstall', 'lorax_options')])
def test_lorax_with_lorax_options(self):
cfg = load_config(
@ -173,7 +176,7 @@ class BuildinstallConfigTestCase(unittest.TestCase):
lorax_options=[]
)
self.assertItemsEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_lorax_options_without_bootable_and_method(self):
cfg = load_config(
@ -182,11 +185,10 @@ class BuildinstallConfigTestCase(unittest.TestCase):
buildinstall_kickstart='foo',
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.CONFLICTS.format('buildinstall_method', 'None', 'lorax_options'),
checks.CONFLICTS.format('buildinstall_method', 'None', 'buildinstall_kickstart')]
)
checks.CONFLICTS.format('buildinstall_method', 'None', 'buildinstall_kickstart')])
def test_deprecated(self):
cfg = load_config(
@ -194,15 +196,13 @@ class BuildinstallConfigTestCase(unittest.TestCase):
buildinstall_upgrade_image=True,
)
self.assertItemsEqual(checks.validate(cfg), [])
self.assertItemsEqual(
checks.report_removed(cfg),
self.assertValidation(
cfg, [],
[checks.REMOVED.format('buildinstall_upgrade_image', 'use lorax_options instead')]
)
class CreaterepoConfigTestCase(unittest.TestCase):
class CreaterepoConfigTestCase(ConfigTestCase):
def test_validate_minimal_pkgset_koji(self):
cfg = load_config(
pkgset_source='koji',
@ -210,13 +210,12 @@ class CreaterepoConfigTestCase(unittest.TestCase):
product_id_allow_missing=True,
)
self.assertEqual(
checks.validate(cfg),
[checks.CONFLICTS.format('product_id', 'None', 'product_id_allow_missing')]
)
self.assertValidation(
cfg,
[checks.CONFLICTS.format('product_id', 'None', 'product_id_allow_missing')])
class GatherConfigTestCase(unittest.TestCase):
class GatherConfigTestCase(ConfigTestCase):
def test_source_comps_requires_comps(self):
cfg = load_config(
pkgset_source='koji',
@ -225,11 +224,10 @@ class GatherConfigTestCase(unittest.TestCase):
gather_source_mapping='foo'
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.REQUIRES.format('gather_source', 'comps', 'comps_file'),
checks.CONFLICTS.format('gather_source', 'comps', 'gather_source_mapping')]
)
checks.CONFLICTS.format('gather_source', 'comps', 'gather_source_mapping')])
def test_source_json_requires_mapping(self):
cfg = load_config(
@ -239,14 +237,13 @@ class GatherConfigTestCase(unittest.TestCase):
comps_file='comps',
)
self.assertItemsEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
[checks.REQUIRES.format('gather_source', 'json', 'gather_source_mapping'),
checks.CONFLICTS.format('gather_source', 'json', 'comps_file')]
)
checks.CONFLICTS.format('gather_source', 'json', 'comps_file')])
class OSBSConfigTestCase(unittest.TestCase):
class OSBSConfigTestCase(ConfigTestCase):
def test_validate(self):
cfg = load_config(
PKGSET_REPOS,
@ -256,7 +253,7 @@ class OSBSConfigTestCase(unittest.TestCase):
}}
)
self.assertItemsEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_validate_bad_conf(self):
cfg = load_config(
@ -264,10 +261,10 @@ class OSBSConfigTestCase(unittest.TestCase):
osbs='yes please'
)
self.assertNotEqual(checks.validate(cfg), [])
self.assertNotEqual(checks.validate(cfg), ([], []))
class OstreeConfigTestCase(unittest.TestCase):
class OstreeConfigTestCase(ConfigTestCase):
def test_validate(self):
cfg = load_config(
PKGSET_REPOS,
@ -283,7 +280,7 @@ class OstreeConfigTestCase(unittest.TestCase):
]
)
self.assertEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_validate_bad_conf(self):
cfg = load_config(
@ -291,10 +288,10 @@ class OstreeConfigTestCase(unittest.TestCase):
ostree='yes please'
)
self.assertNotEqual(checks.validate(cfg), [])
self.assertNotEqual(checks.validate(cfg), ([], []))
class OstreeInstallerConfigTestCase(unittest.TestCase):
class OstreeInstallerConfigTestCase(ConfigTestCase):
def test_validate(self):
cfg = load_config(
PKGSET_REPOS,
@ -321,7 +318,7 @@ class OstreeInstallerConfigTestCase(unittest.TestCase):
]
)
self.assertEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_validate_bad_conf(self):
cfg = load_config(
@ -348,10 +345,10 @@ class OstreeInstallerConfigTestCase(unittest.TestCase):
]
)
self.assertNotEqual(checks.validate(cfg), [])
self.assertNotEqual(checks.validate(cfg), ([], []))
class LiveMediaConfigTestCase(unittest.TestCase):
class LiveMediaConfigTestCase(ConfigTestCase):
def test_global_config_validation(self):
cfg = load_config(
PKGSET_REPOS,
@ -361,7 +358,7 @@ class LiveMediaConfigTestCase(unittest.TestCase):
live_media_version='Rawhide',
)
self.assertEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
def test_global_config_null_release(self):
cfg = load_config(
@ -369,28 +366,27 @@ class LiveMediaConfigTestCase(unittest.TestCase):
live_media_release=None,
)
self.assertEqual(checks.validate(cfg), [])
self.assertValidation(cfg)
class TestSuggestions(unittest.TestCase):
class TestSuggestions(ConfigTestCase):
def test_with_a_typo(self):
cfg = load_config(PKGSET_REPOS,
product_pid=None)
self.assertEqual(
checks.validate(cfg),
[checks.UNKNOWN_SUGGEST.format('product_pid', 'product_id')])
self.assertValidation(cfg, [], [checks.UNKNOWN_SUGGEST.format('product_pid', 'product_id')])
class TestRegexValidation(unittest.TestCase):
class TestRegexValidation(ConfigTestCase):
def test_incorrect_regular_expression(self):
cfg = load_config(PKGSET_REPOS,
multilib=[('^*$', {'*': []})])
self.assertEqual(
checks.validate(cfg),
self.assertValidation(
cfg,
['Failed validation in multilib.0.0: incorrect regular '
'expression: nothing to repeat'])
'expression: nothing to repeat'],
[])
if __name__ == '__main__':

19
tests/test_imagebuildphase.py
View File

@ -14,7 +14,6 @@ import sys
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from pungi.phases.image_build import ImageBuildPhase, CreateImageBuildThread
from pungi.checks import validate
from tests.helpers import DummyCompose, PungiTestCase, boom
@ -47,7 +46,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -132,7 +131,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -249,7 +248,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -284,7 +283,7 @@ class TestImageBuildPhase(PungiTestCase):
})
compose.setup_optional()
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -347,7 +346,7 @@ class TestImageBuildPhase(PungiTestCase):
})
compose.setup_optional()
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -411,7 +410,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -473,7 +472,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -511,7 +510,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = ImageBuildPhase(compose)
@ -549,7 +548,7 @@ class TestImageBuildPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'git://git.fedorahosted.org/git/spin-kickstarts.git?#BEEFCAFE'

17
tests/test_liveimagesphase.py
View File

@ -11,7 +11,6 @@ import sys
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from pungi.phases.live_images import LiveImagesPhase, CreateLiveImageThread
from pungi.checks import validate
from tests.helpers import DummyCompose, PungiTestCase, boom
@ -33,7 +32,7 @@ class TestLiveImagesPhase(PungiTestCase):
})
compose.setup_optional()
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveImagesPhase(compose)
@ -85,7 +84,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveImagesPhase(compose)
@ -133,7 +132,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveImagesPhase(compose)
@ -183,7 +182,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveImagesPhase(compose)
@ -253,7 +252,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'https://git.example.com/kickstarts.git?#CAFEBABE'
@ -308,7 +307,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'https://git.example.com/kickstarts.git?#CAFEBABE'
@ -363,7 +362,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'https://git.example.com/kickstarts.git?#CAFEBABE'
@ -415,7 +414,7 @@ class TestLiveImagesPhase(PungiTestCase):
],
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveImagesPhase(compose)

13
tests/test_livemediaphase.py
View File

@ -10,7 +10,6 @@ import os
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from pungi.phases.livemedia_phase import LiveMediaPhase, LiveMediaThread
from pungi.checks import validate
from tests.helpers import DummyCompose, PungiTestCase, boom
@ -33,7 +32,7 @@ class TestLiveMediaPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveMediaPhase(compose)
@ -91,7 +90,7 @@ class TestLiveMediaPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'git://example.com/repo.git#BEEFCAFE'
@ -192,7 +191,7 @@ class TestLiveMediaPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'git://example.com/repo.git#BEEFCAFE'
@ -280,7 +279,7 @@ class TestLiveMediaPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveMediaPhase(compose)
@ -305,7 +304,7 @@ class TestLiveMediaPhase(PungiTestCase):
'koji_profile': 'koji',
})
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
phase = LiveMediaPhase(compose)
@ -340,7 +339,7 @@ class TestLiveMediaPhase(PungiTestCase):
})
compose.setup_optional()
self.assertEqual(validate(compose.conf), [])
self.assertValidConfig(compose.conf)
resolve_git_url.return_value = 'resolved'