diff --git a/bin/pungi-config-validate b/bin/pungi-config-validate index 851412b3..8dedf44f 100755 --- a/bin/pungi-config-validate +++ b/bin/pungi-config-validate @@ -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) diff --git a/bin/pungi-koji b/bin/pungi-koji index 23f70462..683e4f87 100755 --- a/bin/pungi-koji +++ b/bin/pungi-koji @@ -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: diff --git a/pungi/checks.py b/pungi/checks.py index 436477dd..90544f9e 100644 --- a/pungi/checks.py +++ b/pungi/checks.py @@ -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#", diff --git a/tests/helpers.py b/tests/helpers.py index c707f314..53e3898d 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -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): diff --git a/tests/test_config.py b/tests/test_config.py index 7f49f5de..8b22e45a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -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__': diff --git a/tests/test_imagebuildphase.py b/tests/test_imagebuildphase.py index 00cba88b..609dcb38 100644 --- a/tests/test_imagebuildphase.py +++ b/tests/test_imagebuildphase.py @@ -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' diff --git a/tests/test_liveimagesphase.py b/tests/test_liveimagesphase.py index e3be6902..6840d857 100644 --- a/tests/test_liveimagesphase.py +++ b/tests/test_liveimagesphase.py @@ -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) diff --git a/tests/test_livemediaphase.py b/tests/test_livemediaphase.py index 7e4bc266..61c37ada 100644 --- a/tests/test_livemediaphase.py +++ b/tests/test_livemediaphase.py @@ -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'