From c10b9a4230c4daccdf24c531083a3d44696e74f3 Mon Sep 17 00:00:00 2001
From: William Brown <firstyear@redhat.com>
Date: Wed, 10 Feb 2016 15:34:15 +1000
Subject: [PATCH] Ticket 48357 - clitools should standarise their args

Bug Description:  Ldap tools tend to follow some conventions, especialy in
389. These are
* -D for bindDN
* -Z for instance name
* -n for backend name
We should make the lib389 tools conform to this.

Fix Description:  Update the lib389 clitools to use standardised args.

https://fedorahosted.org/389/ticket/48357

Author: wibrown

Review by: ???
---
 lib389/__init__.py                               |  7 +++----
 lib389/clitools/__init__.py                      | 20 +++++++++++++++-----
 lib389/clitools/ds_monitor_backend.py            | 14 ++++++++------
 lib389/clitools/ds_monitor_server.py             |  8 +++++---
 lib389/clitools/ds_schema_attributetype_list.py  |  2 +-
 lib389/clitools/ds_schema_attributetype_query.py |  2 +-
 lib389/clitools/ds_start.py                      |  2 +-
 lib389/clitools/ds_stop.py                       |  2 +-
 lib389/clitools/krb_create_keytab.py             |  6 +++---
 lib389/clitools/krb_create_principal.py          |  6 +++---
 lib389/clitools/krb_create_realm.py              |  6 +++---
 lib389/clitools/krb_destroy_realm.py             |  6 +++---
 12 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/lib389/__init__.py b/lib389/__init__.py
index bf142a8..d4b1d50 100644
--- a/lib389/__init__.py
+++ b/lib389/__init__.py
@@ -1031,11 +1031,10 @@ class DirSrv(SimpleLDAPObject):
             @raise ValueError - if the instance has not the right state
         '''
         # check that DirSrv was in DIRSRV_STATE_ONLINE state
-        if self.state != DIRSRV_STATE_ONLINE:
-            raise ValueError("invalid state for calling close: %s" %
-                             self.state)
+        if self.state == DIRSRV_STATE_ONLINE:
+            # Don't raise an error. Just move the state and return
+            SimpleLDAPObject.unbind(self)
 
-        SimpleLDAPObject.unbind(self)
         self.state = DIRSRV_STATE_OFFLINE
 
     def start(self, timeout):
diff --git a/lib389/clitools/__init__.py b/lib389/clitools/__init__.py
index 5bd9cad..da8f49f 100644
--- a/lib389/clitools/__init__.py
+++ b/lib389/clitools/__init__.py
@@ -27,14 +27,19 @@ class CliTool(object):
         if len(insts) != 1:
             # Raise an exception here?
             self.inst = None
+            raise ValueError("No such instance %s" % instance)
         else:
             self.inst = insts[0]
 
     def get_rootdn_pass(self):
+        if self.args.binddn is None:
+            binddn = self.inst[SER_ROOT_DN]
+        else:
+            binddn = self.args.binddn
         # There is a dict get key thing somewhere ...
         if self.inst.get(SER_ROOT_PW, None) is None:
             prompt_txt = ('Enter password for %s on instance %s: ' %
-                          (self.inst[SER_ROOT_DN],
+                          (binddn,
                            self.inst[SER_SERVERID_PROP]))
             self.inst[SER_ROOT_PW] = getpass(prompt_txt)
         return
@@ -50,11 +55,16 @@ class CliTool(object):
 
 
 def _clitools_parser():
-    parser = ArgumentParser(add_help=False)
-    parser.add_argument('--instance',
-                        '-i',
+    parser = ArgumentParser()
+    parser.add_argument('-Z', '--instance',
                         help='The name of the DS instance to connect to and ' +
-                             'work upon.')
+                             'work upon.', required=True)
+    parser.add_argument('-D', '--binddn',
+                        help='The bind dn to use for operations. Defaults to ' +
+                             'rooddn', default=None)
     return parser
 
 clitools_parser = _clitools_parser()
+
+if __name__ == '__main__':
+    args = clitools_parser.parse_args()
diff --git a/lib389/clitools/ds_monitor_backend.py b/lib389/clitools/ds_monitor_backend.py
index c1e40df..02d09b3 100644
--- a/lib389/clitools/ds_monitor_backend.py
+++ b/lib389/clitools/ds_monitor_backend.py
@@ -9,7 +9,7 @@
 # --- END COPYRIGHT BLOCK ---
 
 # from clitools import clitools_parser, get_instance_dict, get_rootdn_pass
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 # from lib389 import DirSrv
 from lib389._constants import *
 from argparse import ArgumentParser
@@ -21,17 +21,19 @@ class MonitorTool(CliTool):
             self.populate_instance_dict(self.args.instance)
             self.connect()
             # This is pretty rough, it just dumps the objects
-            for monitor in self.ds.monitor.backend(self.args.backend):
-                print(monitor)
+            status = self.ds.monitor.backend(self.args.backend)
+            print ("dn: %s" % status.pop('dn'))
+            for monitor in status:
+                print("%s: %s" % (monitor, status[monitor][0]))
         finally:
             self.disconnect()
 
 if __name__ == '__main__':
     # Do some arg parse stuff
     # You can always add a child parser here too ...
-    parser = ArgumentParser(parents=[clitools_parser])
-    parser.add_argument('--backend', '-b', help='The name of the backend to ' +
+    parser = clitools_parser.add_argument_group('monitor', 'monitoring options')
+    parser.add_argument('-n', '--backend', help='The name of the backend to ' +
                         ' retrieve monitoring information for.', required=True)
-    args = parser.parse_args()
+    args = clitools_parser.parse_args()
     monitortool = MonitorTool(args)
     monitortool.monitor_backend_list()
diff --git a/lib389/clitools/ds_monitor_server.py b/lib389/clitools/ds_monitor_server.py
index 544a033..0f574cd 100644
--- a/lib389/clitools/ds_monitor_server.py
+++ b/lib389/clitools/ds_monitor_server.py
@@ -9,7 +9,7 @@
 # --- END COPYRIGHT BLOCK ---
 
 # from clitools import clitools_parser, get_instance_dict, get_rootdn_pass
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 # from lib389 import DirSrv
 from lib389._constants import *
 
@@ -20,8 +20,10 @@ class MonitorTool(CliTool):
             self.populate_instance_dict(self.args.instance)
             self.connect()
             # This is pretty rough, it just dumps the objects
-            for monitor in self.ds.monitor.server():
-                print(monitor)
+            status = self.ds.monitor.server()
+            print ("dn: %s" % status.pop('dn'))
+            for monitor in status:
+                print("%s: %s" % (monitor, status[monitor][0]))
         finally:
             self.disconnect()
 
diff --git a/lib389/clitools/ds_schema_attributetype_list.py b/lib389/clitools/ds_schema_attributetype_list.py
index 5bd6d7d..f73e331 100644
--- a/lib389/clitools/ds_schema_attributetype_list.py
+++ b/lib389/clitools/ds_schema_attributetype_list.py
@@ -9,7 +9,7 @@
 # --- END COPYRIGHT BLOCK ---
 
 # from clitools import clitools_parser, get_instance_dict, get_rootdn_pass
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 # from lib389 import DirSrv
 from lib389._constants import *
 import ldap
diff --git a/lib389/clitools/ds_schema_attributetype_query.py b/lib389/clitools/ds_schema_attributetype_query.py
index 5b4ad62..6acc93b 100644
--- a/lib389/clitools/ds_schema_attributetype_query.py
+++ b/lib389/clitools/ds_schema_attributetype_query.py
@@ -8,7 +8,7 @@
 # See LICENSE for details.
 # --- END COPYRIGHT BLOCK ---
 
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 from lib389._constants import *
 import ldap
 from argparse import ArgumentParser
diff --git a/lib389/clitools/ds_start.py b/lib389/clitools/ds_start.py
index a4927cc..143051c 100644
--- a/lib389/clitools/ds_start.py
+++ b/lib389/clitools/ds_start.py
@@ -9,7 +9,7 @@
 # --- END COPYRIGHT BLOCK ---
 
 # from clitools import clitools_parser, get_instance_dict, get_rootdn_pass
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 # from lib389 import DirSrv
 from lib389._constants import *
 from lib389.tools import DirSrvTools
diff --git a/lib389/clitools/ds_stop.py b/lib389/clitools/ds_stop.py
index e3eb37f..7988c8b 100644
--- a/lib389/clitools/ds_stop.py
+++ b/lib389/clitools/ds_stop.py
@@ -9,7 +9,7 @@
 # --- END COPYRIGHT BLOCK ---
 
 # from clitools import clitools_parser, get_instance_dict, get_rootdn_pass
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 # from lib389 import DirSrv
 from lib389._constants import *
 from lib389.tools import DirSrvTools
diff --git a/lib389/clitools/krb_create_keytab.py b/lib389/clitools/krb_create_keytab.py
index 4fc1b9f..11c345a 100644
--- a/lib389/clitools/krb_create_keytab.py
+++ b/lib389/clitools/krb_create_keytab.py
@@ -8,7 +8,7 @@
 # See LICENSE for details.
 # --- END COPYRIGHT BLOCK ---
 
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 from lib389._constants import *
 from lib389.mit_krb5 import MitKrb5
 from argparse import ArgumentParser
@@ -25,7 +25,7 @@ class MitKrb5Tool(CliTool):
 if __name__ == '__main__':
     # Do some arg parse stuff
     # You can always add a child parser here too ...
-    parser = ArgumentParser(parents=[clitools_parser])
+    parser = clitools_parser.add_argument_group('krb', 'kerberos options')
     parser.add_argument('--realm', '-r',
                         help='The name of the realm to create',
                         required=True)
@@ -35,6 +35,6 @@ if __name__ == '__main__':
     parser.add_argument('--keytab', '-k',
                         help='The path to the keytab to create',
                         required=True)
-    args = parser.parse_args()
+    args = clitools_parser.parse_args()
     mittool = MitKrb5Tool(args)
     mittool.mit_krb5_realm_create()
diff --git a/lib389/clitools/krb_create_principal.py b/lib389/clitools/krb_create_principal.py
index cee295e..f4b59b2 100644
--- a/lib389/clitools/krb_create_principal.py
+++ b/lib389/clitools/krb_create_principal.py
@@ -9,7 +9,7 @@
 # --- END COPYRIGHT BLOCK ---
 
 
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 from lib389._constants import *
 from lib389.mit_krb5 import MitKrb5
 from argparse import ArgumentParser
@@ -26,13 +26,13 @@ class MitKrb5Tool(CliTool):
 if __name__ == '__main__':
     # Do some arg parse stuff
     # You can always add a child parser here too ...
-    parser = ArgumentParser(parents=[clitools_parser])
+    parser = clitools_parser.add_argument_group('krb', 'kerberos options')
     parser.add_argument('--realm', '-r',
                         help='The name of the realm to create',
                         required=True)
     parser.add_argument('--principal', '-p',
                         help='The name of the principal to create',
                         required=True)
-    args = parser.parse_args()
+    args = clitools_parser.parse_args()
     mittool = MitKrb5Tool(args)
     mittool.mit_krb5_realm_create()
diff --git a/lib389/clitools/krb_create_realm.py b/lib389/clitools/krb_create_realm.py
index bf818e1..23aafc4 100644
--- a/lib389/clitools/krb_create_realm.py
+++ b/lib389/clitools/krb_create_realm.py
@@ -8,7 +8,7 @@
 # See LICENSE for details.
 # --- END COPYRIGHT BLOCK ---
 
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 from lib389._constants import *
 from lib389.mit_krb5 import MitKrb5
 from argparse import ArgumentParser
@@ -25,10 +25,10 @@ class MitKrb5Tool(CliTool):
 if __name__ == '__main__':
     # Do some arg parse stuff
     # You can always add a child parser here too ...
-    parser = ArgumentParser(parents=[clitools_parser])
+    parser = clitools_parser.add_argument_group('krb', 'kerberos options')
     parser.add_argument('--realm', '-r',
                         help='The name of the realm to create',
                         required=True)
-    args = parser.parse_args()
+    args = clitools_parser.parse_args()
     mittool = MitKrb5Tool(args)
     mittool.mit_krb5_realm_create()
diff --git a/lib389/clitools/krb_destroy_realm.py b/lib389/clitools/krb_destroy_realm.py
index d42d8c5..ce2938f 100644
--- a/lib389/clitools/krb_destroy_realm.py
+++ b/lib389/clitools/krb_destroy_realm.py
@@ -8,7 +8,7 @@
 # See LICENSE for details.
 # --- END COPYRIGHT BLOCK ---
 
-from clitools import CliTool, clitools_parser
+from lib389.clitools import CliTool, clitools_parser
 from lib389._constants import *
 from lib389.mit_krb5 import MitKrb5
 from argparse import ArgumentParser
@@ -25,10 +25,10 @@ class MitKrb5Tool(CliTool):
 if __name__ == '__main__':
     # Do some arg parse stuff
     # You can always add a child parser here too ...
-    parser = ArgumentParser(parents=[clitools_parser])
+    parser = clitools_parser.add_argument_group('krb', 'kerberos options')
     parser.add_argument('--realm', '-r',
                         help='The name of the realm to DESTROY',
                         required=True)
-    args = parser.parse_args()
+    args = clitools_parser.parse_args()
     mittool = MitKrb5Tool(args)
     mittool.mit_krb5_realm_destroy()
-- 
2.5.0

