Merge pull request #208 from SimonThalvorsen/main

Changed how the arg_parse/validation of `cf-remote` functions is done and sets req cf-remote version >= 9.4

Author: larsewi

pyproject.toml

@@ -10,7 +10,7 @@ readme = "README.md"
 license = {file = "LICENSE"}
 requires-python = ">=3.10"
 dependencies = [
-    "cf-remote>=0.7.3",
+    "cf-remote>=0.9.4",
     "cfbs>=5.5.0",
     "tree-sitter-cfengine>=1.1.12",
     "tree-sitter>=0.25",

src/cfengine_cli/cfengine_wrapper/arg_parse.py

@@ -1,27 +1,21 @@
 import argparse
 
+from cf_remote.args import (
+    add_save_args,
+    add_deploy_args,
+    add_install_args,
+    add_uninstall_args,
+    add_spawn_args,
+    add_destroy_args,
+)
+
 
 def parse_wrapper_args(subp: argparse._SubParsersAction):
 
-    sp = subp.add_parser(
-        "save", help="Save host(s) with a group name to use in other commands"
-    )
-    sp.add_argument(
-        "--role",
-        help="Role of the hosts",
-        choices=["hub", "hubs", "client", "clients"],
-        required=True,
-    )
-    sp.add_argument(
-        "--name",
-        help="Name of the group of hosts (can be used in other commands)",
-        required=True,
-    )
-    sp.add_argument(
-        "--hosts",
-        "-H",
-        help="SSH usernames and IPs for SSH and CFEngine in the form of user@ip",
-        required=True,
+    add_save_args(
+        subp.add_parser(
+            "save", help="Save host(s) with a group name to use in other commands"
+        )
     )
 
     sp = subp.add_parser(
@@ -40,18 +34,13 @@ def parse_wrapper_args(subp: argparse._SubParsersAction):
         help="""Build a policy set from a CFEngine Build project.
 A wrapper around the cfbs `build`-function.""",
     )
-    sp = subp.add_parser(
+
+    deploy_parser = subp.add_parser(
         "deploy",
         help="""Deploy policy-set (masterfiles) to hub.
 A wrapper around the cf-remote `deploy`-function with some added niceties.""",
     )
-    sp.add_argument("--hub", help="Hub(s) to deploy to", type=str)
-    sp.add_argument(
-        "masterfiles",
-        help="Policy-set location (tarball URL or local path to tarball / directory)",
-        type=str,
-        nargs="?",
-    )
+    add_deploy_args(deploy_parser)
 
     install_parser = subp.add_parser(
         "install",
@@ -64,78 +53,14 @@ def parse_wrapper_args(subp: argparse._SubParsersAction):
         help="Specify version",
         type=str,
     )
-    # install_parser._option_string_actions.get("--version").help = "absdfsf"
-    # TODO: Update cf-remote/cfbs to have more modular arg-parsing, then we can import
-    # and override any differences? technically illegal since _option_string_actions,
-    # but will save ~ 200-1000 loc depending on how much we import into cfengine-cli
-
-    install_parser.add_argument(
-        "--edition",
-        "-E",
-        choices=["community", "enterprise"],
-        help="Enterprise or community packages",
-        type=str,
-    )
-    install_parser.add_argument(
-        "--package", help="Local path to package or URL to download", type=str
-    )
-    install_parser.add_argument(
-        "--hub-package",
-        help="Local path to package or URL to download for --hub",
-        type=str,
-    )
-    install_parser.add_argument(
-        "--client-package",
-        help="Local path to package or URL to download for --clients",
-        type=str,
-    )
-    install_parser.add_argument(
-        "--bootstrap", "-B", help="cf-agent --bootstrap argument", type=str
-    )
-    install_parser.add_argument(
-        "--clients", "-c", help="Where to install client package", type=str
-    )
-    install_parser.add_argument("--hub", help="Where to install hub package", type=str)
-    install_parser.add_argument(
-        "--demo",
-        help="Use defaults to make demos smoother (NOT secure)",
-        action="store_true",
-    )
-    install_parser.add_argument(
-        "--call-collect",
-        help="Enable call collect in --demo def.json",
-        action="store_true",
-    )
-    install_parser.add_argument(
-        "--remote-download",
-        help="Package will be downloaded directly to the target machine",
-        action="store_true",
-    )
-    install_parser.add_argument(
-        "--trust-keys",
-        help="Comma-separated list of paths to keys hosts should trust"
-        + " (implies '--trust-server no' when boostraping)",
-        type=str,
-    )
-    install_parser.add_argument(
-        "--insecure",
-        help="Ignore mismatching checksums when downloading urls",
-        action="store_true",
-    )
+    add_install_args(install_parser)
 
     uninstall_parser = subp.add_parser(
         "uninstall",
         help="Uninstall CFEngine on the given hosts",
         description="A wrapper around the cf-remote `uninstall` function",
     )
-    uninstall_parser.add_argument(
-        "--purge", help="Complete uninstallation", action="store_true"
-    )
-    uninstall_parser.add_argument(
-        "--clients", "-c", help="Where to uninstall", type=str
-    )
-    uninstall_parser.add_argument("--hub", help="Where to uninstall", type=str)
-    uninstall_parser.add_argument("--hosts", "-H", help="Where to uninstall", type=str)
+    add_uninstall_args(uninstall_parser)
 
     report_parser = subp.add_parser(
         "report",
@@ -184,73 +109,19 @@ def parse_wrapper_args(subp: argparse._SubParsersAction):
         "If omitted and multiple installations are found, you'll be prompted.",
     )
 
-    sp = subp.add_parser(
+    spawn_parser = subp.add_parser(
         "spawn",
         help="Spawn hosts in the clouds",
         description="A wrapper around the cf-remote `spawn`-function",
     )
-    sp.add_argument(
-        "--list-platforms", help="List supported platforms", action="store_true"
-    )
-    sp.add_argument(
-        "--list-boxes", help="List installed vagrant boxes", action="store_true"
-    )
-    sp.add_argument(
-        "--init-config",
-        help="Initialize configuration file for spawn functionality",
-        action="store_true",
-    )
-    sp.add_argument("--platform", help="Platform or vagrant box to use", type=str)
-    sp.add_argument("--count", default=1, help="How many hosts to spawn", type=int)
-    sp.add_argument(
-        "--role", help="Role of the hosts", choices=["hub", "hubs", "client", "clients"]
-    )
-    sp.add_argument(
-        "--name", help="Name of the group of hosts (can be used in other commands)"
-    )
-    sp.add_argument(
-        "--append",
-        help="Append the new VMs to a pre-existing group",
-        action="store_true",
-    )
-    sp.add_argument(
-        "--provider",
-        help="VM provider",
-        type=str,
-        default="aws",
-        choices=["aws", "gcp", "vagrant"],
-    )
-    sp.add_argument("--cpus", help="Number of CPUs of the vagrant instances", type=int)
-    sp.add_argument(
-        "--sync-folder",
-        help="Root folder of synchronized folders of vagrant instance",
-        type=str,
-    )
-    sp.add_argument(
-        "--provision",
-        help="full path to provision shell script for Vagrant VM",
-        type=str,
-    )
-    sp.add_argument("--size", help="Size/type of the instances", type=str)
-    sp.add_argument(
-        "--network", help="network/subnet to assign the VMs to (GCP only)", type=str
-    )
-    sp.add_argument(
-        "--no-public-ip",
-        help="No public IP needed (GCP only; WARNING: The VMs will only be accessible"
-        + " from some other VM in the same cloud/network!)",
-        action="store_true",
-    )
+    add_spawn_args(spawn_parser)
 
-    dp = subp.add_parser(
+    destroy_parser = subp.add_parser(
         "destroy",
         help="Destroy hosts spawned in the clouds",
         description="A wrapper around the cf-remote `destroy`-function",
     )
-    dp.add_argument(
-        "--all", help="Destroy all hosts spawned in the clouds", action="store_true"
-    )
-    dp.add_argument("name", help="Name of the group of hosts to destroy", nargs="?")
+    add_destroy_args(destroy_parser)
 
     profile_parser = subp.add_parser(
         "profile", help="Parse CFEngine profiling output (cf-agent -Kp)"

src/cfengine_cli/main.py

@@ -7,7 +7,7 @@
 
 from cf_remote import log
 from cf_remote.main import resolve_hosts
-from cf_remote.utils import is_package_url, strip_user
+from cf_remote.utils import strip_user, CFRExitError
 from cfengine_cli.cfengine_wrapper import cfengine_commands
 from cfengine_cli.cfengine_wrapper.arg_parse import parse_wrapper_args
 from cfengine_cli.version import cfengine_cli_version_string
@@ -22,6 +22,14 @@
     uninstall,
 )
 from cf_remote.spawn import CFRUserError, Providers
+from cf_remote.validate import (
+    validate_edition_args,
+    validate_install_args,
+    validate_uninstall_args,
+    validate_spawn_args,
+    validate_deploy_args,
+    validate_destroy_args,
+)
 from cfbs.utils import CFBSProgrammerError
 
 
@@ -323,39 +331,19 @@ def run_command_with_args(args) -> int:
 def validate_args(args):
     if args.command == "dev" and args.dev_command is None:
         raise UserError("Missing subcommand - cfengine dev <subcommand>")
-    if (
-        args.command == "spawn"
-        and not args.list_platforms
-        and not args.init_config
-        and not args.list_boxes
-    ):
-        # The above options don't require any other options/arguments (TODO:
-        # --provider), but otherwise all have to be given
-        if not args.platform:
-            raise UserError("--platform needs to be specified")
-        if not args.count:
-            raise UserError("--count needs to be specified")
-        if not args.role:
-            raise UserError("--role needs to be specified")
-        if not args.name:
-            raise UserError("--name needs to be specified")
+
+    if args.command == "spawn":
+        validate_spawn_args(args)
 
     if args.command == "destroy":
-        if not args.all and not args.name:
-            raise UserError("Either '--all' or 'NAME' must be specified for destroy")
+        validate_destroy_args(args)
         if args.all and args.name:
             raise UserError(
                 "Only one of '--all' or 'NAME' may be specified for destruction"
             )
-    if args.command in ["install"]:  # , "packages", "list", "download"]:
-        if args.edition:
-            args.edition = args.edition.lower()
-            if args.edition == "core":
-                args.edition = "community"
-            if args.edition not in ["enterprise", "community"]:
-                raise UserError("--edition must be either community or enterprise")
-        else:
-            args.edition = "enterprise"
+
+    if args.command == "install":
+        validate_edition_args(args)
 
     if "hosts" in args and args.hosts:
         log.debug(f"validate_args, hosts in args, args.hosts='{args.hosts}'")
@@ -371,33 +359,14 @@ def validate_args(args):
         log.debug(f"validate_args, hubs in args, args.hub='{args.hub}'")
         args.hub = resolve_hosts(args.hub)
 
-    if args.command in ["uninstall"] and not (args.hosts or args.hub or args.clients):
-        raise UserError("Use --hosts, --hub or --clients to specify remote hosts")
+    if args.command == "uninstall":
+        validate_uninstall_args(args)
 
     if args.command == "install":
-        if args.call_collect and not args.demo:
-            raise UserError("--call-collect must be used with --demo")
-        if not args.clients and not args.hub:
-            raise UserError("Specify hosts using --hub and --clients")
-        if args.hub and args.clients and args.package:
-            raise UserError(
-                "Use --hub-package / --client-package instead to distinguish between hosts"
-            )
-        if args.package and (args.hub_package or args.client_package):
-            raise UserError(
-                "--package cannot be used in combination with --hub-package / --client-package"
-            )
-        if args.package and not is_package_url(args.package):
-            if not os.path.exists(os.path.expanduser(args.package)):
-                raise UserError("Package/directory '%s' does not exist" % args.package)
-        if args.hub_package and not is_package_url(args.hub_package):
-            if not os.path.isfile(args.hub_package):
-                raise UserError("Hub package '%s' does not exist" % args.hub_package)
-        if args.client_package and not is_package_url(args.client_package):
-            if not os.path.isfile(args.client_package):
-                raise UserError(
-                    "Client package '%s' does not exist" % args.client_package
-                )
+        validate_install_args(args)
+
+    if args.command == "deploy":
+        validate_deploy_args(args)
 
 
 def _main():
@@ -416,7 +385,7 @@ def main():
         exit_code = _main()
         assert type(exit_code) is int
         sys.exit(exit_code)
-    except (UserError, CFRUserError) as e:
+    except (UserError, CFRUserError, CFRExitError) as e:
         print(str(e))
         sys.exit(-1)
     # Exceptions below are not expected, print extra info: