From 39263120e49125ae47f6e7a4b25c2a43270743b2 Mon Sep 17 00:00:00 2001 From: Simon Halvorsen Date: Tue, 4 Aug 2026 14:01:17 +0200 Subject: [PATCH] Changed how the arg_parse/validation of `cf-remote` functions is done Ticket: ENT-14305 Changelog: Title Signed-off-by: Simon Halvorsen --- .../cfengine_wrapper/arg_parse.py | 173 +++--------------- src/cfengine_cli/main.py | 77 +++----- 2 files changed, 45 insertions(+), 205 deletions(-) diff --git a/src/cfengine_cli/cfengine_wrapper/arg_parse.py b/src/cfengine_cli/cfengine_wrapper/arg_parse.py index 5292b29..3211243 100644 --- a/src/cfengine_cli/cfengine_wrapper/arg_parse.py +++ b/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)" diff --git a/src/cfengine_cli/main.py b/src/cfengine_cli/main.py index 4dc83d5..1f4e9a2 100644 --- a/src/cfengine_cli/main.py +++ b/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 ") - 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: