refactor: load available CLI commands dynamically

quipucordsctl/commands/__init__.py

@@ -0,0 +1,43 @@
+"""Package of mostly self-contained commands.
+
+The CLI loads the modules in this package dynamically at startup.
+Valid command modules in this package should implement an interface like the following:
+
+    __doc__ = "Perform magic."
+
+    def setup_parser(parser: argparse.ArgumentParser) -> None:
+        # Optional additions to this command's argparse subparser.
+        # For example:
+        parser.add_argument("-x", action="store_true", help="Enable effects")
+
+    def run(args: argparse.Namespace) -> None:
+        # Implementation of this command's functionality.
+        # For example"
+        shell_utils.run_command(["echo", "hello"])
+
+The module's name will be the CLI's positional argument to invoke the command.
+The module's __doc__ will be the help text for the CLI's help command.
+For example, invoking `quipucordsctl --help` with the example code above in
+a module file named `magic.py` may produce output like the following:
+
+    $ quipucordsctl --help
+    usage: quipucordsctl [-h] {install,magic} ...
+
+    positional arguments:
+      {install,magic}
+        install        Install the Quipucords server.
+        magic          Perform magic.
+
+    options:
+      -h, --help       show this help message and exit
+
+    $ quipucordsctl magic --help
+    usage: quipucordsctl magic [-h] [-x]
+
+    options:
+      -h, --help  show this help message and exit
+      -x          Enable effects
+
+If the module has attribute `NOT_A_COMMAND=True` set, it will not be included
+by argparse as a valid positional argument.
+"""

quipucordsctl/commands/install.py

@@ -1,12 +1,15 @@
-"""Logic for the "install" command."""
+"""Install the server."""
 
+import argparse
 import itertools
 import logging
 import shutil
 
 from .. import settings, shell_utils
 from . import reset_django_secret, reset_server_password
 
+__doc__ = f"Install the {settings.SERVER_SOFTWARE_NAME} server."
+
 DATA_DIRS = ("data", "db", "log", "sshkeys")
 SYSTEMCTL_USER_RESET_FAILED_CMD = ["systemctl", "--user", "reset-failed"]
 SYSTEMCTL_USER_DAEMON_RELOAD_CMD = ["systemctl", "--user", "daemon-reload"]
@@ -68,16 +71,16 @@ def systemctl_reload():
     shell_utils.run_command(SYSTEMCTL_USER_DAEMON_RELOAD_CMD)
 
 
-def run(override_conf_dir: str | None = None):
+def run(args: argparse.Namespace) -> None:
     """Install the server, ensuring requirements are met."""
     logger.info("Starting install command")
-    if override_conf_dir:
+    if args.override_conf_dir:
         raise NotImplementedError
 
     if not reset_server_password.server_password_is_set():
-        reset_server_password.run()
+        reset_server_password.run(args)
     if not reset_django_secret.django_secret_is_set():
-        reset_django_secret.run()
+        reset_django_secret.run(args)
 
     write_config_files()
     systemctl_reload()

quipucordsctl/commands/reset_django_secret.py

@@ -1,4 +1,10 @@
-"""Logic for the "reset_django_secret" command."""
+"""Reset the Django secret key."""
+
+import argparse
+import logging
+
+logger = logging.getLogger(__name__)
+NOT_A_COMMAND = True  # Until we complete the implementation.
 
 
 def django_secret_is_set() -> bool:
@@ -7,8 +13,9 @@ def django_secret_is_set() -> bool:
     return False
 
 
-def run():
+def run(args: argparse.Namespace) -> None:
     """Reset the server password."""
+    logger.warning("%s is not yet implemented.", __name__)
     # TODO Implement this.
     # TODO Should this also conditionally restart the server?
     # Old bash installer did the following:

quipucordsctl/commands/reset_server_password.py

@@ -1,4 +1,10 @@
-"""Logic for the "reset_server_password" command."""
+"""Reset the admin password."""
+
+import argparse
+import logging
+
+logger = logging.getLogger(__name__)
+NOT_A_COMMAND = True  # Until we complete the implementation.
 
 
 def server_password_is_set() -> bool:
@@ -7,8 +13,9 @@ def server_password_is_set() -> bool:
     return False
 
 
-def run():
+def run(args: argparse.Namespace) -> None:
     """Reset the server password."""
+    logger.warning("%s is not yet implemented.", __name__)
     # TODO implement this.
     # TODO Should this also conditionally restart the server?
     # Old bash installer did the following:

quipucordsctl/commands/uninstall.py

@@ -1 +1,17 @@
-"""Logic for the "uninstall" command."""
+"""Uninstall the server."""
+
+import argparse
+import logging
+
+from .. import settings
+
+__doc__ = f"Uninstall the {settings.SERVER_SOFTWARE_NAME} server."
+
+logger = logging.getLogger(__name__)
+NOT_A_COMMAND = True  # Until we complete the implementation.
+
+
+def run(args: argparse.Namespace) -> None:
+    """Uninstall the server."""
+    logger.warning("%s is not yet implemented.", __name__)
+    # TODO Implement this.

quipucordsctl/main.py

@@ -1,15 +1,27 @@
 """Main command-line entrypoint."""
 
 import argparse
+import importlib
 import logging
+import pkgutil
+from types import ModuleType
 
 from . import settings
-from .commands import install
 
 logger = logging.getLogger(__name__)
 
 
-def create_parser() -> argparse.ArgumentParser:
+def load_commands() -> dict[str, ModuleType]:
+    """Dynamically load command modules."""
+    commands = {}
+    for _, module_name, _ in pkgutil.iter_modules([settings.COMMANDS_PACKAGE_PATH]):
+        module = importlib.import_module(f"quipucordsctl.commands.{module_name}")
+        if not getattr(module, "NOT_A_COMMAND", False):
+            commands[module_name] = module
+    return commands
+
+
+def create_parser(commands: dict[str, ModuleType]) -> argparse.ArgumentParser:
     """Create the argument parser for the CLI."""
     parser = argparse.ArgumentParser(prog=settings.PROGRAM_NAME)
     parser.add_argument(
@@ -38,17 +50,12 @@ def create_parser() -> argparse.ArgumentParser:
     )
 
     subparsers = parser.add_subparsers(dest="command")
-    # TODO load subparsers dynamically from commands package.
-
-    subparsers.add_parser(
-        "install", help=f"Install the {settings.SERVER_SOFTWARE_NAME} server"
-    )
-    # TODO more arguments for this subparser.
-
-    subparsers.add_parser(
-        "uninstall", help=f"Uninstall the {settings.SERVER_SOFTWARE_NAME} server"
-    )
-    # TODO more arguments for this subparser.
+    for command_name, command_module in commands.items():
+        command_parser = subparsers.add_parser(
+            command_name, help=command_module.__doc__
+        )
+        if hasattr(command_module, "setup_parser"):
+            command_module.setup_parser(command_parser)
 
     return parser
 
@@ -74,15 +81,13 @@ def configure_logging(verbosity: int = 0, quiet: bool = False) -> int:
 
 def main():
     """Run the program with arguments from the CLI."""
-    parser = create_parser()
+    commands = load_commands()
+    parser = create_parser(commands)
     args = parser.parse_args()
     configure_logging(args.verbosity, args.quiet)
 
-    # TODO load commands dynamically from commands package.
-    if args.command == "install":
-        install.run(override_conf_dir=args.override_conf_dir)
-    elif args.command == "uninstall":
-        raise NotImplementedError
+    if args.command in commands:
+        commands[args.command].run(args)
     else:
         parser.print_help()
 

quipucordsctl/settings.py

@@ -7,6 +7,8 @@
 SERVER_SOFTWARE = "quipucords"
 SERVER_SOFTWARE_NAME = "Quipucords"
 
+COMMANDS_PACKAGE_PATH = str(pathlib.Path(__file__).parent.resolve() / "commands")
+
 DEFAULT_LOG_LEVEL = logging.WARNING
 
 _home = pathlib.Path.home()

tests/commands/test_install.py

@@ -39,6 +39,8 @@ def mock_shell_utils():
 
 def test_install_run(temp_config_directories):
     """Test the install command happy path."""
+    mock_args = mock.Mock()
+    mock_args.override_conf_dir = None
     data_dir, env_dir, systemd_dir = temp_config_directories
     with (
         mock.patch.object(install, "reset_django_secret") as reset_django_secret,
@@ -48,7 +50,7 @@ def test_install_run(temp_config_directories):
         reset_django_secret.django_secret_is_set.return_value = False
         reset_server_password.server_password_is_set.return_value = False
 
-        install.run()
+        install.run(mock_args)
 
         # Spot-check only a few paths that should now exist.
         assert (pathlib.Path(data_dir) / "data").is_dir()

tests/commands/test_reset_django_secret.py

@@ -1,6 +1,8 @@
 """Test the "reset_django_secret" command."""
-# TODO FIXME Implement the rest of these tests.
 
+from unittest import mock
+
+# TODO FIXME Implement the rest of these tests.
 from quipucordsctl.commands import reset_django_secret
 
 
@@ -11,4 +13,5 @@ def test_django_secret_is_set():
 
 def test_reset_django_secret_run():
     """Test placeholder for reset_django_secret.run."""
-    assert reset_django_secret.run() is None
+    mock_args = mock.Mock()
+    assert reset_django_secret.run(mock_args) is None

tests/commands/test_reset_server_password.py

@@ -1,6 +1,8 @@
 """Test the "reset_server_password" command."""
-# TODO FIXME Implement the rest of these tests.
 
+from unittest import mock
+
+# TODO FIXME Implement the rest of these tests.
 from quipucordsctl.commands import reset_server_password
 
 
@@ -11,4 +13,5 @@ def test_server_password_is_set():
 
 def test_reset_django_secret_run():
     """Test placeholder for reset_server_password.run."""
-    assert reset_server_password.run() is None
+    mock_args = mock.Mock()
+    assert reset_server_password.run(mock_args) is None

tests/test_main.py

@@ -1,15 +1,37 @@
 """Test the main module."""
 
 import logging
+from unittest import mock
 
 import pytest
 
 from quipucordsctl import main
 
 
-def test_create_parser_and_parse():
+def test_load_commands():
+    """Test some known commands are loaded and returned."""
+    from quipucordsctl.commands import install as install_module
+
+    commands = main.load_commands()
+    assert "install" in commands
+    assert commands["install"] == install_module
+
+    assert "__init__" not in commands
+
+    # For now, even thought "uninstall.py" exists, we skip loading it.
+    # This will change/break later when we implement the uninstall logic,
+    # and this test will need to be updated.
+    assert "uninstall" not in commands
+
+
+def test_create_parser_and_parse(faker):
     """Test the constructed argument parser."""
-    parser = main.create_parser()
+    mock_command_name = faker.slug()
+    mock_command_doc = faker.sentence()
+    mock_command = mock.Mock()
+    mock_command.__doc__ = mock_command_doc
+
+    parser = main.create_parser({mock_command_name: mock_command})
 
     # Simplest no-arg invocation.
     parsed_args = parser.parse_args([])
@@ -20,7 +42,7 @@ def test_create_parser_and_parse():
     # Many-args invocation
     # TODO use a TemporaryDirectory and assert bogus paths raise errors.
     override_conf_dir = "/bogus/path"
-    command = "install"
+    command = mock_command_name
     parsed_args = parser.parse_args(["-vv", "-q", "-c", override_conf_dir, command])
     assert parsed_args.verbosity == 2
     assert parsed_args.quiet
@@ -107,3 +129,17 @@ def test_configure_logging(
                 assert message in caplog.messages
             else:
                 assert message not in caplog.messages
+
+
+def test_main_without_command():
+    """Test the main CLI entry function when no command is given."""
+    with mock.patch.object(main, "create_parser") as mock_create_parser:
+        mock_parser = mock_create_parser.return_value
+        mock_args = mock_parser.parse_args.return_value
+        mock_args.verbosity = 0
+        mock_args.quiet = True
+        mock_args.command = None
+
+        main.main()
+
+        mock_parser.print_help.assert_called_once()