Refactor ArgumentParser (#53)

* Separate argparser into class

We will be using almost the same cli options for the daemon (different entrypoint) and the client, so the ArrgumentParser class has been extracted and CLI only options isolated.

* Make cli options POSIX compliant

Following from https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html

> Option names are typically one to three words long, with hyphens to separate words

* Fix bad package reference

* Extract version into single location

* Remove duplicate version option

* Add test cases

* Add more robust test

* Add a single point for the package name

* Use universally shared package name

* Fix flake8

* The versioning uses the pip package. Must install it

* Make sure we don’t run the imports

* Fix version extraction

* use literal string interpolation

* No need to install self.

This is handled by the nosetest

* Fix bad syntax

* Separate setup.py changes into PR #59

* Add `—renewSeconds` as legacy option

* Add deprecation warning.

I'm going out on a limb here saying this will be removed. I figure it should be removed in favour of the POSIX version eventually.

* Fix bad merge from master

* escape string

* daemon and client print a version

* Add testcase for legacy handler

* Add missing dependency

* Fix typo

onelogin_aws_cli/argparse.py

@@ -0,0 +1,67 @@
+"""
+CLI Argument Parser
+"""
+
+import argparse
+
+import pkg_resources
+
+
+class OneLoginAWSArgumentParser(argparse.ArgumentParser):
+    """
+    Argument Parser separated into daemon and cli tool
+    """
+
+    def __init__(self):
+        super().__init__(description='Login to AWS with OneLogin')
+
+        self.add_argument(
+            '-C', '--config-name', default='default', dest='config_name',
+            help='Switch configuration name within config file'
+        )
+
+        self.add_argument(
+            '--profile', default='', help='Specify profile name of credential'
+        )
+
+        self.add_argument(
+            '-u', '--username', default='', help='Specify OneLogin username'
+        )
+
+        version = pkg_resources.get_distribution(__package__).version
+        self.add_argument(
+            '-v', '--version', action='version',
+            version="%(prog)s " + version
+        )
+
+    def add_cli_options(self):
+        """
+        Add Argument Parser options only used in the CLI entrypoint
+        """
+
+        renew_seconds_group = self.add_mutually_exclusive_group()
+
+        renew_seconds_group.add_argument(
+            '-r', '--renew-seconds', type=int,
+            help='Auto-renew credentials after this many seconds'
+        )
+
+        renew_seconds_group.add_argument(
+            # Help is suppressed as this is replaced by the POSIX friendlier
+            # version above. This is here for legacy compliance and will
+            # be deprecated.
+            '--renewSeconds', type=int, help=argparse.SUPPRESS,
+            dest='renew_seconds_legacy'
+        )
+
+        self.add_argument(
+            '-c', '--configure', dest='configure', action='store_true',
+            help='Configure OneLogin and AWS settings'
+        )
+
+        # The `--client` option is a precursor to the daemon process in
+        # https://github.com/physera/onelogin-aws-cli/issues/36
+        # self.add_argument("--client", dest="client_mode",
+        #                   action='store_true')
+
+        return self

onelogin_aws_cli/cli.py

@@ -1,14 +1,12 @@
 """
 Collections of entrypoints
 """
-import argparse
 import signal
 import sys
 from threading import Event
 
-import pkg_resources
-
 from onelogin_aws_cli import DEFAULT_CONFIG_PATH, OneloginAWS
+from onelogin_aws_cli.argparse import OneLoginAWSArgumentParser
 from onelogin_aws_cli.configuration import ConfigurationFile
 from onelogin_aws_cli.model import SignalRepr
 
@@ -18,23 +16,8 @@ def login(args=sys.argv[1:]):
     Entrypoint for `onelogin-aws-login`
     :param args:
     """
-    version = pkg_resources.get_distribution('pip').version
-
-    parser = argparse.ArgumentParser(description="Login to AWS with Onelogin")
-    parser.add_argument("-c", "--configure", dest="configure",
-                        action="store_true",
-                        help="Configure Onelogin and AWS settings")
-    parser.add_argument("-C", "--config_name", default="default",
-                        help="Switch configuration name within config file")
-    parser.add_argument("--profile", default="",
-                        help="Specify profile name of credential")
-    parser.add_argument("-u", "--username", default="",
-                        help="Specify OneLogin username")
-    parser.add_argument("-r", "--renewSeconds", type=int,
-                        help="Auto-renew credentials after this many seconds")
-    parser.add_argument('-v', '--version', action='version',
-                        version='%(prog)s ' + version)
 
+    parser = OneLoginAWSArgumentParser().add_cli_options()
     args = parser.parse_args(args)
 
     with open(DEFAULT_CONFIG_PATH, 'a+') as fp:
@@ -50,10 +33,20 @@ def login(args=sys.argv[1:]):
         sys.exit("Configuration '{}' not defined. "
                  "Please run 'onelogin-aws-login -c'".format(args.config_name))
 
+    # Handle legacy `--renewSeconds` option while it is depecated
+    if args.renew_seconds:
+        renew_seconds = args.renew_seconds
+    elif args.renew_seconds_legacy:
+        print("WARNING: --renewSeconds is depecated in favour of " +
+              "--renew-seconds and be removed in a future version.")
+        renew_seconds = args.renew_seconds_legacy
+    else:
+        renew_seconds = None
+
     api = OneloginAWS(config_section, args)
     api.save_credentials()
 
-    if args.renewSeconds:
+    if renew_seconds:
 
         interrupted = Event()
 
@@ -73,5 +66,5 @@ def _interrupt_handler(signal_num: int, *args):
 
         interrupted.clear()
         while not interrupted.is_set():
-            interrupted.wait(args.renewSeconds)
+            interrupted.wait(renew_seconds)
             api.save_credentials()

onelogin_aws_cli/tests/test_oneLoginAWSArgumentParser.py

@@ -0,0 +1,79 @@
+import contextlib
+from unittest import TestCase
+
+import pkg_resources
+import re
+from io import StringIO
+
+from onelogin_aws_cli.argparse import OneLoginAWSArgumentParser
+
+
+class TestOneLoginAWSArgumentParser(TestCase):
+
+    def test_basic(self):
+        parser = OneLoginAWSArgumentParser()
+        args = parser.parse_args([
+            '-C', 'my_config',
+            '--profile', 'my_profile',
+            '-u', 'my_username'
+        ])
+
+        self.assertEqual(args.config_name, 'my_config')
+        self.assertEqual(args.profile, 'my_profile')
+        self.assertEqual(args.username, 'my_username')
+
+    def test_version(self):
+        parser = OneLoginAWSArgumentParser()
+        parser.add_cli_options()
+        mock_stdout = StringIO()
+
+        with self.assertRaises(SystemExit) as cm:
+            with contextlib.redirect_stdout(mock_stdout):
+                parser.parse_args([
+                    '--version'
+                ])
+
+        # This spits out the nosetest prog name.
+        # I'm ok with that, as what is important is that the version is
+        # correct
+        version = pkg_resources.get_distribution('onelogin_aws_cli').version
+        self.assertRegex(
+            mock_stdout.getvalue(),
+            re.escape(version) + r'$'
+        )
+
+        self.assertEqual(cm.exception.code, 0)
+
+    def test_add_cli_options(self):
+        parser = OneLoginAWSArgumentParser()
+        parser.add_cli_options()
+        args = parser.parse_args([
+            '-C', 'my_config',
+            '--profile', 'my_profile',
+            '-u', 'my_username',
+            '--renew-seconds', '30',
+            '-c',
+        ])
+
+        self.assertEqual(args.config_name, 'my_config')
+        self.assertEqual(args.profile, 'my_profile')
+        self.assertEqual(args.username, 'my_username')
+        self.assertEqual(args.renew_seconds, 30)
+        self.assertTrue(args.configure)
+
+    def test_legacy_renew_seconds(self):
+        parser = OneLoginAWSArgumentParser()
+        parser.add_cli_options()
+        args = parser.parse_args([
+            '--renewSeconds', '30'
+        ])
+
+        self.assertEqual(args.renew_seconds_legacy, 30)
+
+        with self.assertRaises(SystemExit) as cm:
+            parser.parse_args([
+                '--renewSeconds', '30',
+                '--renew-seconds', '30',
+            ])
+
+        self.assertEqual(cm.exception.code, 2)