Skip to content

Commit

Permalink
Fix CLI help messages
Browse files Browse the repository at this point in the history 
Previously if a user called pki-server CLI with a wrong
sub-command the ArgumentParser would show an auto-generated
error message which looks significantly different from the
help message already defined in print_help().

To fix the issue the PKIServerCLI.create_parser() and
execute() have been modified to use remainder instead of
subparsers, and the create_parser() in the sub-commands has
been modified to create a regular ArgumentParser instead of
a subparser.

Similar changes were made to the CLI base class to fix the
help messages for sub-commands (e.g. pki-server ca).

The pki-server has also been modified to provide a --version
option to show the version number of the tool.

A new CLIException has been added to distinguish a normal
CLI error (which will generate a simple error message such
as "Invalid module") from an unexpected exception which will
generate a full stack trace.

Resolves: #4932
  • Loading branch information
@edewata
edewata committed Feb 4, 2025
1 parent 0a88a9b commit 5a47222
Show file tree
Hide file tree
Showing 33 changed files with 310 additions and 200 deletions.
75 changes: 61 additions & 14 deletions base/common/python/pki/cli/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
# All rights reserved.
#

import argparse
import collections
import logging
from six import itervalues
Expand Down Expand Up @@ -73,7 +74,7 @@ def print_help(self):
continue

full_name = module.get_full_name()
print(' {:30}{:30}'.format(full_name, module.description))
print(' {:32}{:30}'.format(full_name, module.description))

first = True

Expand All @@ -88,7 +89,7 @@ def print_help(self):
first = False

full_name = module.get_full_name()
print(' {:30}{:30}'.format(full_name, module.description))
print(' {:32}{:30}'.format(full_name, module.description))

def find_module(self, command):

Expand All @@ -102,6 +103,29 @@ def find_module(self, command):

def create_parser(self, subparsers=None):

if not self.parser:
# create default parser
self.parser = argparse.ArgumentParser(
prog=self.name,
add_help=False)

# add basic arguments
self.parser.add_argument(
'-v',
'--verbose',
action='store_true')
self.parser.add_argument(
'--debug',
action='store_true')
self.parser.add_argument(
'--help',
action='store_true')

# capture sub-command and args
self.parser.add_argument(
'remainder',
nargs=argparse.REMAINDER)

for module in self.modules.values():
module.create_parser(subparsers=subparsers)

Expand Down Expand Up @@ -162,31 +186,54 @@ def parse_command(self, command):

return (module, sub_command)

def parse_args(self, args):
def execute(self, argv, args=None):
'''
:param argv: Argument values
:param args: Parsed arguments
'''

if not args:
args = self.parser.parse_args(args=argv)

if args.help:
self.print_help()
return

if args.debug:
logging.getLogger().setLevel(logging.DEBUG)

elif args.verbose:
logging.getLogger().setLevel(logging.INFO)

command = None
if len(args.remainder) > 0:
command = args.remainder[0]
logger.debug('Command: %s', command)

if not command:
self.print_help()
return

command = args[0]
(module, sub_command) = self.parse_command(command)

if not module:
raise Exception('Invalid module "%s".' % command)

logger.debug('Module: %s', module.get_full_name())

# Prepare module arguments.
if sub_command:
# If module command exists, include it as arguments:
# <module command> <args>...
module_args = [sub_command] + args[1:]
module_args = [sub_command] + args.remainder[1:]

else:
# Otherwise, pass the original arguments: <args>...
module_args = args[1:]
module_args = args.remainder[1:]

return (module, module_args)
module.execute(module_args)

def execute(self, argv, args=None): # pylint: disable=W0613
'''
:param argv: Argument values
:param args: Parsed arguments
'''
(module, module_argv) = self.parse_args(argv)

module.execute(module_argv)
class CLIException(Exception):

pass
16 changes: 11 additions & 5 deletions base/common/python/pki/cli/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ def print_help(self):
'certificate validity statuses')
print(' --ignore-banner Ignore banner')
print()
print(' -v, --verbose Run in verbose mode.')
print(' --debug Show debug messages.')
print(' --help Show help message.')
print(' --version Show version number.')
print(' -v, --verbose Run in verbose mode.')
print(' --debug Show debug messages.')
print(' --help Show help message.')
print(' --version Show version number.')
print()

super(PKICLI, self).print_help()
Expand Down Expand Up @@ -358,9 +358,15 @@ def execute(self, argv, args=None):
self.ignore_cert_status = args.ignore_cert_status
self.ignore_banner = args.ignore_banner

command = args.remainder[0]
command = None
if len(args.remainder) > 0:
command = args.remainder[0]
logger.debug('Command: %s', command)

if not command:
self.print_help()
return

if client_type == 'python' or command in PYTHON_COMMANDS:
module = self.find_module(command)
logger.debug('Module: %s', module.get_full_name())
Expand Down
62 changes: 43 additions & 19 deletions base/server/python/pki/server/cli/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ def create_parser(self, subparsers=None):
self.parser = argparse.ArgumentParser(
prog=self.name,
add_help=False)
self.parser.add_argument(
'-i',
'--instance',
default='pki-tomcat')
self.parser.add_argument(
'-v',
'--verbose',
Expand All @@ -118,45 +114,73 @@ def create_parser(self, subparsers=None):
self.parser.add_argument(
'--help',
action='store_true')
self.parser.add_argument(
'--version',
action='store_true')

self.parser.add_argument(
'remainder',
nargs=argparse.REMAINDER)

# create subparsers
subparsers = self.parser.add_subparsers(dest='command')
super().create_parser(subparsers=subparsers)
# create parsers in modules
super().create_parser()

def get_full_module_name(self, module_name):
return module_name

def print_help(self):
print('Usage: pki-server [OPTIONS]')
print()
print(' -v, --verbose Run in verbose mode.')
print(' --debug Show debug messages.')
print(' --help Show help message.')
print(' -v, --verbose Run in verbose mode.')
print(' --debug Show debug messages.')
print(' --help Show help message.')
print(' --version Show version number.')
print()

super().print_help()

def print_version(self):
print('PKI Server Command-Line Interface %s' % pki.implementation_version())

def execute(self, argv, args=None):

if not args:
args = self.parser.parse_args(args=argv)

if args.help:
self.print_help()
return

if args.version:
self.print_version()
return

if args.debug:
logging.getLogger().setLevel(logging.DEBUG)

elif args.verbose:
logging.getLogger().setLevel(logging.INFO)

command = args.command
command = None
if len(args.remainder) > 0:
command = args.remainder[0]
logger.debug('Command: %s', command)

if not command:
self.print_help()
return

module = self.find_module(command)

if not module:
raise pki.cli.CLIException('Invalid module "%s".' % command)

logger.debug('Module: %s', module.get_full_name())

module.execute(argv, args=args)
module_args = args.remainder[1:]
logger.debug('Arguments: %s', ' '.join(module_args))

module.execute(module_args)

@staticmethod
def print_status(instance):
Expand Down Expand Up @@ -312,7 +336,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument('--user')
Expand Down Expand Up @@ -409,7 +433,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument(
Expand Down Expand Up @@ -488,7 +512,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument(
Expand Down Expand Up @@ -549,7 +573,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument(
Expand Down Expand Up @@ -628,7 +652,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument(
Expand Down Expand Up @@ -707,7 +731,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument(
Expand Down Expand Up @@ -782,7 +806,7 @@ def __init__(self):

def create_parser(self, subparsers=None):

self.parser = subparsers.add_parser(
self.parser = argparse.ArgumentParser(
self.get_full_name(),
add_help=False)
self.parser.add_argument(
Expand Down
Loading

0 comments on commit 5a47222

Please sign in to comment.