Feature/required args (#66)

Author: alanag13

CHANGELOG.md

@@ -10,6 +10,14 @@ how a consumer would use the library (e.g. adding unit tests, updating documenta
 
 ## Unreleased
 
+### Changed
+
+- `code42 profile create` now uses required `--name`, `--server` and `--username` flags instead of positional arguments.
+
+- `code42 high-risk-employee add-risk-tags` now uses required `--username` and `--tag` flags instead of positional arguments.
+
+- `code42 high-risk-employee remove-risk-tags` now uses required `--username` and `--tag` flags instead of positional arguments.
+
 ### Added
 
 - `code42 alerts`:

src/code42cli/args.py

@@ -38,7 +38,7 @@ def add_short_option_name(self, short_name):
     def as_multi_val_param(self, nargs=u"+"):
         self._settings[u"nargs"] = nargs
 
-    def set_required(self, required=False):
+    def set_required(self, required):
         self._settings[u"required"] = required
 
 
@@ -61,51 +61,51 @@ def get_auto_arg_configs(handler):
     """Looks at the parameter names of `handler` and builds an `ArgConfigCollection` containing
     `argparse` parameters based on them."""
     arg_configs = ArgConfigCollection()
+    excluded_args = [SDK_ARG_NAME, u"profile", u"args", u"kwargs", u"self"]
     if callable(handler):
-        # get the number of positional and keyword args
         argspec = inspect.getargspec(handler)
-        num_args = len(argspec.args)
-        num_kw_args = len(argspec.defaults) if argspec.defaults else 0
-
-        for arg_position, key in enumerate(argspec.args):
-            # do not create cli parameters for arguments named "sdk", "args", or "kwargs"
-            if not key in [SDK_ARG_NAME, u"args", u"kwargs", u"self"]:
-                arg_config = _create_auto_args_config(
-                    arg_position, key, argspec, num_args, num_kw_args
-                )
-                _set_smart_defaults(arg_config)
-                arg_configs.append(key, arg_config)
+        filtered_argspec = {
+            key: position for position, key in enumerate(argspec.args) if key not in excluded_args
+        }
+        num_optional_args = len(argspec.defaults) if argspec.defaults else 0
+        num_positional_args = len(argspec.args) - num_optional_args
+        num_required_cli_args = len(filtered_argspec) - num_optional_args
+
+        for key in filtered_argspec:
+            arg_config = _create_auto_args_config(
+                key, filtered_argspec[key], num_positional_args, num_required_cli_args, argspec
+            )
+            _set_smart_defaults(arg_config)
+            arg_configs.append(key, arg_config)
 
         if SDK_ARG_NAME in argspec.args:
             _build_sdk_arg_configs(arg_configs)
 
     return arg_configs
 
 
-def _create_auto_args_config(arg_position, key, argspec, num_args, num_kw_args):
+def _create_auto_args_config(key, position, num_positional_args, num_required_cli_args, argspec):
     default = None
+    required = None
     param_name = key.replace(u"_", u"-")
-    difference = num_args - num_kw_args
-    last_positional_arg_idx = difference - 1
+    last_positional_arg_idx = num_positional_args - 1
+    option_names = [u"--{}".format(param_name)]
     # positional arguments will come first, so if the arg position
     # is greater than the index of the last positional arg, it's a kwarg.
-    if arg_position > last_positional_arg_idx:
+    if position > last_positional_arg_idx:
         # this is a keyword arg, treat it as an optional cli arg.
-        default_value = argspec.defaults[arg_position - difference]
-        option_names = [u"--{}".format(param_name)]
+        default_value = argspec.defaults[position - num_positional_args]
         default = default_value
-    else:
+    elif num_required_cli_args > 1:
         # this is a positional arg, treat it as a required cli arg.
+        required = True
+    else:
         option_names = [param_name]
-    return ArgConfig(*option_names, default=default)
+    return ArgConfig(*option_names, default=default, required=required)
 
 
 def _set_smart_defaults(arg_config):
     default = arg_config.settings.get(u"default")
-    # make a parameter allow lists as input if its default value is a list,
-    # e.g. --my-param one two three four
-    nargs = u"+" if type(default) == list else None
-    arg_config.settings[u"nargs"] = nargs
     # make the param not require a value (e.g. --enable) if the default value of
     # the param is a bool.
     if type(default) == bool:

src/code42cli/cmds/alerts/rules/commands.py

@@ -6,26 +6,22 @@
     get_rules,
     add_bulk_users,
     remove_bulk_users,
-    show_rules,
+    show_rule,
 )
 
 
 def _customize_add_arguments(argument_collection):
     rule_id = argument_collection.arg_configs[u"rule_id"]
     rule_id.set_help(u"Observer ID of the rule to be updated. Required.")
-    rule_id.set_required(True)
     username = argument_collection.arg_configs[u"username"]
     username.set_help(u"The username of the user to add to the alert rule. Required.")
-    username.set_required(True)
 
 
 def _customize_remove_arguments(argument_collection):
     rule_id = argument_collection.arg_configs[u"rule_id"]
     rule_id.set_help(u"Observer ID of the rule to be updated.")
-    rule_id.set_required(True)
     username = argument_collection.arg_configs[u"username"]
     username.set_help(u"The username of the user to remove from the alert rule.")
-    username.set_required(True)
 
 
 def _customize_list_arguments(argument_collection):
@@ -131,7 +127,7 @@ def load_subcommands():
             u"show",
             u"Fetch configured alert-rules against the rule ID.",
             u"{} show <rule-id>".format(usage_prefix),
-            handler=show_rules,
+            handler=show_rule,
             arg_customizer=_customize_list_arguments,
         )
 

src/code42cli/cmds/alerts/rules/user_rule.py

@@ -17,12 +17,12 @@
 }
 
 
-def add_user(sdk, profile, rule_id=None, username=None):
+def add_user(sdk, profile, rule_id, username):
     user_id = get_user_id(sdk, username)
     sdk.alerts.rules.add_user(rule_id, user_id)
 
 
-def remove_user(sdk, profile, rule_id=None, username=None):
+def remove_user(sdk, profile, rule_id, username):
     if username:
         user_id = get_user_id(sdk, username)
         sdk.alerts.rules.remove_user(rule_id, user_id)
@@ -58,7 +58,7 @@ def remove_bulk_users(sdk, profile, file_name):
     )
 
 
-def show_rules(sdk, profile, rule_id):
+def show_rule(sdk, profile, rule_id):
     selected_rule = _get_rules_metadata(sdk, rule_id)
     rule_detail = None
     if len(selected_rule):

src/code42cli/cmds/detectionlists/high_risk_employee.py

@@ -24,12 +24,14 @@ def load_subcommands():
             Command(
                 u"add-risk-tags",
                 u"Associates risk tags with a user.",
+                u"code42 high-risk-employee add-risk-tags --username <username> --tag <risk-tags>",
                 handler=add_risk_tags,
                 arg_customizer=_load_risk_tag_mgmt_descriptions,
             ),
             Command(
                 u"remove-risk-tags",
                 u"Disassociates risk tags from a user.",
+                u"code42 high-risk-employee remove-risk-tags --username <username> --tag <risk-tags>",
                 handler=remove_risk_tags,
                 arg_customizer=_load_risk_tag_mgmt_descriptions,
             ),
@@ -44,14 +46,14 @@ def _create_handlers():
     )
 
 
-def add_risk_tags(sdk, profile, username, risk_tag):
-    risk_tag = _handle_list_args(risk_tag)
+def add_risk_tags(sdk, profile, username, tag):
+    risk_tag = _handle_list_args(tag)
     user_id = get_user_id(sdk, username)
     try_add_risk_tags(sdk, user_id, risk_tag)
 
 
-def remove_risk_tags(sdk, profile, username, risk_tag):
-    risk_tag = _handle_list_args(risk_tag)
+def remove_risk_tags(sdk, profile, username, tag):
+    risk_tag = _handle_list_args(tag)
     user_id = get_user_id(sdk, username)
     try_remove_risk_tags(sdk, user_id, risk_tag)
 
@@ -92,7 +94,10 @@ def remove_high_risk_employee(sdk, profile, username):
 
 
 def _load_risk_tag_description(argument_collection):
-    risk_tag = argument_collection.arg_configs[DetectionListUserKeys.RISK_TAG]
+    risk_tag = (
+        argument_collection.arg_configs.get(DetectionListUserKeys.RISK_TAG)
+        or argument_collection.arg_configs[u"tag"]
+    )
     risk_tag.as_multi_val_param()
     tags = u", ".join(list(RiskTags()))
     risk_tag.set_help(

src/code42cli/cmds/profile.py

@@ -47,7 +47,10 @@ def load_subcommands():
     create = Command(
         u"create",
         u"Create profile settings. The first profile created will be the default.",
-        u"{} {}".format(usage_prefix, u"create <profile-name> <server-address> <username>"),
+        u"{} {}".format(
+            usage_prefix,
+            u"create --name <profile-name> --server <server-address> --username <username>",
+        ),
         handler=create_profile,
         arg_customizer=_load_profile_create_descriptions,
     )
@@ -90,10 +93,10 @@ def show_profile(name=None):
     logger.print_info(u"")
 
 
-def create_profile(profile, server, username, disable_ssl_errors=False):
-    cliprofile.create_profile(profile, server, username, disable_ssl_errors)
-    _prompt_for_allow_password_set(profile)
-    get_main_cli_logger().print_info(u"Successfully created profile '{}'.".format(profile))
+def create_profile(name, server, username, disable_ssl_errors=False):
+    cliprofile.create_profile(name, server, username, disable_ssl_errors)
+    _prompt_for_allow_password_set(name)
+    get_main_cli_logger().print_info(u"Successfully created profile '{}'.".format(name))
 
 
 def update_profile(name=None, server=None, username=None, disable_ssl_errors=None):
@@ -172,7 +175,7 @@ def _load_optional_profile_description(argument_collection):
 
 
 def _load_profile_create_descriptions(argument_collection):
-    profile = argument_collection.arg_configs[PROFILE_ARG_NAME]
+    profile = argument_collection.arg_configs[u"name"]
     profile.set_help(PROFILE_HELP)
     _load_profile_settings_descriptions(argument_collection)
 

src/code42cli/parser.py

@@ -46,8 +46,9 @@ def error(self, message):
 
     def _load_argparse_config(self, command, command_parser):
         arg_configs = command.get_arg_configs()
+        required_group = command_parser.add_argument_group(u"required arguments")
         for arg in arg_configs:
-            _add_argument(command_parser, arg_configs[arg].settings)
+            _add_argument(command_parser, arg_configs[arg].settings, required_group)
 
     def _get_parser(self, command, path_parts):
         usage = command.usage or SUPPRESS
@@ -84,10 +85,12 @@ def _get_parent_subparser(path_parts, part, subparsers):
     return parent_subparser
 
 
-def _add_argument(parser, arg_settings):
+def _add_argument(parser, arg_settings, required_group):
     # register the settings of an ArgConfig object to an argparse parser
     options_list = arg_settings.pop(u"options_list")
     arg_settings = {key: arg_settings[key] for key in arg_settings if arg_settings[key] is not None}
+    if arg_settings.get(u"required"):
+        parser = required_group
     parser.add_argument(*options_list, **arg_settings)
 
 

tests/cmds/alerts/rules/test_user_rule.py

@@ -1,6 +1,6 @@
 import pytest
 
-from code42cli.cmds.alerts.rules.user_rule import add_user, remove_user, get_rules, show_rules
+from code42cli.cmds.alerts.rules.user_rule import add_user, remove_user, get_rules, show_rule
 
 TEST_RULE_ID = u"rule-id"
 TEST_USER_ID = u"test-user-id"
@@ -30,28 +30,26 @@ def test_remove_user_removes_user_list_from_alert_rules(alert_rules_sdk, profile
     alert_rules_sdk.users.get_by_username.return_value = {u"users": [{u"userUid": TEST_USER_ID}]}
     remove_user(alert_rules_sdk, profile, TEST_RULE_ID, TEST_USERNAME)
     alert_rules_sdk.alerts.rules.remove_user.assert_called_once_with(TEST_RULE_ID, TEST_USER_ID)
-    remove_user(alert_rules_sdk, profile, TEST_RULE_ID)
-    alert_rules_sdk.alerts.rules.remove_all_users.assert_called_once_with(TEST_RULE_ID)
 
 
 def test_get_rules_gets_alert_rules(alert_rules_sdk, profile):
     get_rules(alert_rules_sdk, profile)
     assert alert_rules_sdk.alerts.rules.get_all.call_count == 1
 
 
-def test_show_rules_calls_correct_rule_property(alert_rules_sdk, profile):
+def test_show_rule_calls_correct_rule_property(alert_rules_sdk, profile):
     alert_rules_sdk.alerts.rules.get_all.return_value = TEST_GET_ALL_RESPONSE_EXFILTRATION
-    show_rules(alert_rules_sdk, profile, TEST_RULE_ID)
+    show_rule(alert_rules_sdk, profile, TEST_RULE_ID)
     alert_rules_sdk.alerts.rules.exfiltration.get.assert_called_once_with(TEST_RULE_ID)
 
 
-def test_show_rules_calls_correct_rule_property_cloud_share(alert_rules_sdk, profile):
+def test_show_rule_calls_correct_rule_property_cloud_share(alert_rules_sdk, profile):
     alert_rules_sdk.alerts.rules.get_all.return_value = TEST_GET_ALL_RESPONSE_CLOUD_SHARE
-    show_rules(alert_rules_sdk, profile, TEST_RULE_ID)
+    show_rule(alert_rules_sdk, profile, TEST_RULE_ID)
     alert_rules_sdk.alerts.rules.cloudshare.get.assert_called_once_with(TEST_RULE_ID)
 
 
-def test_show_rules_calls_correct_rule_property_file_type_mismatch(alert_rules_sdk, profile):
+def test_show_rule_calls_correct_rule_property_file_type_mismatch(alert_rules_sdk, profile):
     alert_rules_sdk.alerts.rules.get_all.return_value = TEST_GET_ALL_RESPONSE_FILE_TYPE_MISMATCH
-    show_rules(alert_rules_sdk, profile, TEST_RULE_ID)
+    show_rule(alert_rules_sdk, profile, TEST_RULE_ID)
     alert_rules_sdk.alerts.rules.filetypemismatch.get.assert_called_once_with(TEST_RULE_ID)

tests/conftest.py

@@ -131,6 +131,14 @@ def func_keyword_args(one=None, two=None, three=None, default="testdefault", nar
     pass
 
 
+def func_single_positional_arg(one):
+    pass
+
+
+def func_single_positional_arg_many_optional_args(one, two=None, three=None, four=None):
+    pass
+
+
 def func_positional_args(one, two, three):
     pass
 
@@ -143,6 +151,12 @@ def func_with_sdk(sdk, one, two, three=None, four=None):
     pass
 
 
+def func_single_positional_arg_with_sdk_and_profile(
+    sdk, profile, one, two=None, three=None, four=None
+):
+    pass
+
+
 def func_with_args(args):
     pass