Validation of parameter skip_phases

It checks config file value 'skip_phases' for valid phases names.
Also checks command-line attribute 'skip-phase' of 'bin/pungi-koji'.

JIRA: COMPOSE-2493

Signed-off-by: Ondrej Nosek <onosek@redhat.com>

bin/pungi-koji

@@ -3,24 +3,26 @@
 
 from __future__ import print_function
 
-import os
-import sys
 import argparse
-import logging
-import locale
 import datetime
 import getpass
+import json
+import locale
+import logging
+import os
 import socket
 import signal
+import sys
 import traceback
+
 from six.moves import shlex_quote
-import json
 
 here = sys.path[0]
 if here != '/usr/bin':
     # Git checkout
     sys.path[0] = os.path.dirname(here)
 
+from pungi.phases import PHASES_NAMES
 from pungi import get_full_version
 
 
@@ -84,6 +86,7 @@ def main():
     parser.add_argument(
         "--skip-phase",
         metavar="PHASE",
+        choices=PHASES_NAMES,
         action="append",
         default=[],
         help="skip a compose phase",
@@ -91,6 +94,7 @@ def main():
     parser.add_argument(
         "--just-phase",
         metavar="PHASE",
+        choices=PHASES_NAMES,
         action="append",
         default=[],
         help="run only a specified compose phase",

pungi/checks.py

@@ -37,15 +37,17 @@ When a new config option is added, the schema must be updated (see the
 from __future__ import print_function
 
 import contextlib
+import multiprocessing
 import os.path
 import platform
 import re
+
 import jsonschema
 import six
 from kobo.shortcuts import force_list
+from pungi.phases import PHASES_NAMES
 from productmd.common import RELEASE_TYPES
 from productmd.composeinfo import COMPOSE_TYPES
-import multiprocessing
 
 from . import util
 
@@ -760,7 +762,11 @@ def make_schema():
 
             "paths_module": {"type": "string"},
             "skip_phases": {
-                "$ref": "#/definitions/list_of_strings",
+                "type": "array",
+                "items": {
+                    "type": "string",
+                    "enum": PHASES_NAMES,
+                },
                 "default": [],
             },
 

pungi/phases/__init__.py

@@ -13,6 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <https://gnu.org/licenses/>.
 
+import sys
 
 # phases in runtime order
 from .init import InitPhase  # noqa
@@ -32,3 +33,8 @@ from .livemedia_phase import LiveMediaPhase  # noqa
 from .ostree import OSTreePhase  # noqa
 from .ostree_installer import OstreeInstallerPhase  # noqa
 from .osbs import OSBSPhase  # noqa
+from .phases_metadata import gather_phases_metadata  # noqa
+
+
+this_module = sys.modules[__name__]
+PHASES_NAMES = gather_phases_metadata(this_module)

pungi/phases/phases_metadata.py

@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+
+
+from inspect import isclass
+
+from pungi.phases.base import PhaseBase
+
+
+def gather_phases_metadata(source_object):
+    """
+    Code gathers metadata from Phase classes.
+    Metadata are 'name' attributes of the corresponding classes.
+    Metadata are gathered without creating instances of Phase classes.
+    """
+
+    if not source_object:
+        raise ValueError("PhasesMetadata can not load any data - it got empty parameter")
+
+    phases = []
+    for item in dir(source_object):
+        cls = getattr(source_object, item)  # get all objects references
+        if not isclass(cls):  # filter out non-classes
+            continue
+        if issubclass(cls, PhaseBase):
+            try:
+                name_attr = getattr(cls, 'name')
+                phases.append(name_attr)
+            except AttributeError:
+                raise AttributeError("Bad phase-class format: '%s' is missing attribute 'name'" % item)
+
+    return phases

tests/test_config.py

@@ -428,5 +428,37 @@ class VariantAsLookasideTestCase(ConfigTestCase):
         self.assertValidation(cfg)
 
 
+class SkipPhasesTestCase(ConfigTestCase):
+    def test_empty(self):
+        skip_phases = []
+        cfg = load_config(
+            PKGSET_REPOS,
+            skip_phases=skip_phases,
+        )
+        self.assertValidation(cfg)
+
+    def test_basic(self):
+        skip_phases = [
+            "buildinstall",
+            "gather",
+        ]
+        cfg = load_config(
+            PKGSET_REPOS,
+            skip_phases=skip_phases,
+        )
+        self.assertValidation(cfg)
+
+    def test_bad_phase_name(self):
+        skip_phases = [
+            "gather",
+            "non-existing-phase_name",
+        ]
+        cfg = load_config(
+            PKGSET_REPOS,
+            skip_phases=skip_phases,
+        )
+        self.assertNotEqual(checks.validate(cfg), ([], []))
+
+
 if __name__ == '__main__':
     unittest.main()