From c3c4c6c632f25002b7ec8a8911844392b46505f8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 25 Nov 2015 21:24:13 -0800 Subject: [PATCH 01/35] Begin work on a noninteractive iDisplay --- letsencrypt/display/util.py | 255 ++++++++++++++++++++++++++++++++++++ 1 file changed, 255 insertions(+) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 01a8cbc92..303c078c3 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -403,6 +403,261 @@ class FileDisplay(object): return OK, selection +class NoninteractiveDisplay(object): + """File-based display.""" + + zope.interface.implements(interfaces.IDisplay) + + def __init__(self, outfile): + super(FileDisplay, self).__init__() + self.outfile = outfile + + def _interaction_fail(self, message, extra=""): + "Error out in case of an attempt to interact in noninteractive mode" + msg ="Missing command line flag or config entry for this setting:\n" + msg += message + if extra: + msg += "\n" + extra + raise MissingCommandlineFlag, msg + + def notification(self, message, height=10, pause=True): + # pylint: disable=unused-argument + """Displays a notification and waits for user acceptance. + + :param str message: Message to display to stdout + :param int height: No effect for NoninteractiveDisplay + :param bool pause: The NoninteractiveDisplay waits for no keyboard + + """ + side_frame = "-" * 79 + message = self._wrap_lines(message) + self.outfile.write( + "{line}{frame}{line}{msg}{line}{frame}{line}".format( + line=os.linesep, frame=side_frame, msg=message)) + + def menu(self, message, choices, ok_label="", cancel_label="", + help_label="", default=None): + # pylint: disable=unused-argument + """Display a menu. + + .. todo:: This doesn't enable the help label/button (I wasn't sold on + any interface I came up with for this). It would be a nice feature + + :param str message: title of menu + :param choices: Menu lines, len must be > 0 + :type choices: list of tuples (tag, item) or + list of descriptions (tags will be enumerated) + + :returns: tuple of the form (code, tag) where + code - int display exit code + tag - str corresponding to the item chosen + :rtype: tuple + :raises errors.MissingCommandlineFlag: if there was no default + + """ + if default is None: + self._interaction_fail(message, "Choices: " + repr(choices)) + if default == None: + msg ="Missing command line flag or config entry for this choice:\n" + msg += message + msg += "\nChoices: " + repr(choices) + raise MissingCommandlineFlag, msg + + return OK, choices.index(default) + + def input(self, message, default=None): + # pylint: disable=no-self-use + """Accept input from the user. + + :param str message: message to display to the user + + :returns: tuple of (`code`, `input`) where + `code` - str display exit code + `input` - str of the user's input + :rtype: tuple + + """ + ans = raw_input( + textwrap.fill("%s (Enter 'c' to cancel): " % message, 80)) + + if ans == "c" or ans == "C": + return CANCEL, "-1" + else: + return OK, ans + + def yesno(self, message, yes_label="Yes", no_label="No"): + """Query the user with a yes/no question. + + Yes and No label must begin with different letters, and must contain at + least one letter each. + + :param str message: question for the user + :param str yes_label: Label of the "Yes" parameter + :param str no_label: Label of the "No" parameter + + :returns: True for "Yes", False for "No" + :rtype: bool + + """ + side_frame = ("-" * 79) + os.linesep + + message = self._wrap_lines(message) + + self.outfile.write("{0}{frame}{msg}{0}{frame}".format( + os.linesep, frame=side_frame, msg=message)) + + while True: + ans = raw_input("{yes}/{no}: ".format( + yes=_parens_around_char(yes_label), + no=_parens_around_char(no_label))) + + # Couldn't get pylint indentation right with elif + # elif doesn't matter in this situation + if (ans.startswith(yes_label[0].lower()) or + ans.startswith(yes_label[0].upper())): + return True + if (ans.startswith(no_label[0].lower()) or + ans.startswith(no_label[0].upper())): + return False + + def checklist(self, message, tags, default_status=True): + # pylint: disable=unused-argument + """Display a checklist. + + :param str message: Message to display to user + :param list tags: `str` tags to select, len(tags) > 0 + :param bool default_status: Not used for FileDisplay + + :returns: tuple of (`code`, `tags`) where + `code` - str display exit code + `tags` - list of selected tags + :rtype: tuple + + """ + while True: + self._print_menu(message, tags) + + code, ans = self.input("Select the appropriate numbers separated " + "by commas and/or spaces") + + if code == OK: + indices = separate_list_input(ans) + selected_tags = self._scrub_checklist_input(indices, tags) + if selected_tags: + return code, selected_tags + else: + self.outfile.write( + "** Error - Invalid selection **%s" % os.linesep) + else: + return code, [] + + def _scrub_checklist_input(self, indices, tags): + # pylint: disable=no-self-use + """Validate input and transform indices to appropriate tags. + + :param list indices: input + :param list tags: Original tags of the checklist + + :returns: valid tags the user selected + :rtype: :class:`list` of :class:`str` + + """ + # They should all be of type int + try: + indices = [int(index) for index in indices] + except ValueError: + return [] + + # Remove duplicates + indices = list(set(indices)) + + # Check all input is within range + for index in indices: + if index < 1 or index > len(tags): + return [] + # Transform indices to appropriate tags + return [tags[index - 1] for index in indices] + + def _print_menu(self, message, choices): + """Print a menu on the screen. + + :param str message: title of menu + :param choices: Menu lines + :type choices: list of tuples (tag, item) or + list of descriptions (tags will be enumerated) + + """ + # Can take either tuples or single items in choices list + if choices and isinstance(choices[0], tuple): + choices = ["%s - %s" % (c[0], c[1]) for c in choices] + + # Write out the message to the user + self.outfile.write( + "{new}{msg}{new}".format(new=os.linesep, msg=message)) + side_frame = ("-" * 79) + os.linesep + self.outfile.write(side_frame) + + # Write out the menu choices + for i, desc in enumerate(choices, 1): + self.outfile.write( + textwrap.fill("{num}: {desc}".format(num=i, desc=desc), 80)) + + # Keep this outside of the textwrap + self.outfile.write(os.linesep) + + self.outfile.write(side_frame) + + def _wrap_lines(self, msg): # pylint: disable=no-self-use + """Format lines nicely to 80 chars. + + :param str msg: Original message + + :returns: Formatted message respecting newlines in message + :rtype: str + + """ + lines = msg.splitlines() + fixed_l = [] + for line in lines: + fixed_l.append(textwrap.fill(line, 80)) + + return os.linesep.join(fixed_l) + + def _get_valid_int_ans(self, max_): + """Get a numerical selection. + + :param int max: The maximum entry (len of choices), must be positive + + :returns: tuple of the form (`code`, `selection`) where + `code` - str display exit code ('ok' or cancel') + `selection` - int user's selection + :rtype: tuple + + """ + selection = -1 + if max_ > 1: + input_msg = ("Select the appropriate number " + "[1-{max_}] then [enter] (press 'c' to " + "cancel): ".format(max_=max_)) + else: + input_msg = ("Press 1 [enter] to confirm the selection " + "(press 'c' to cancel): ") + while selection < 1: + ans = raw_input(input_msg) + if ans.startswith("c") or ans.startswith("C"): + return CANCEL, -1 + try: + selection = int(ans) + if selection < 1 or selection > max_: + selection = -1 + raise ValueError + + except ValueError: + self.outfile.write( + "{0}** Invalid input **{0}".format(os.linesep)) + + return OK, selection + def separate_list_input(input_): """Separate a comma or space separated list. From 4f33a4dbb5e761f0f21b14c835cee2ff760f487a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 27 Dec 2015 15:24:52 -0800 Subject: [PATCH 02/35] Noninteractive iDisplay basic implementation (no tests or hooks, yet) --- letsencrypt/display/util.py | 244 +++++-------------------- letsencrypt/errors.py | 5 + letsencrypt/tests/display/util_test.py | 2 +- 3 files changed, 55 insertions(+), 196 deletions(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 303c078c3..0c7926bd4 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -6,7 +6,7 @@ import dialog import zope.interface from letsencrypt import interfaces - +from letsencrypt import errors WIDTH = 72 HEIGHT = 20 @@ -21,6 +21,20 @@ CANCEL = "cancel" HELP = "help" """Display exit code when for when the user requests more help.""" +def _wrap_lines(msg): # pylint: disable=no-self-use + """Format lines nicely to 80 chars. + + :param str msg: Original message + + :returns: Formatted message respecting newlines in message + :rtype: str + + """ + lines = msg.splitlines() + fixed_l = [] + for line in lines: + fixed_l.append(textwrap.fill(line, 80)) + return os.linesep.join(fixed_l) class NcursesDisplay(object): """Ncurses-based display.""" @@ -178,7 +192,7 @@ class FileDisplay(object): """ side_frame = "-" * 79 - message = self._wrap_lines(message) + message = _wrap_lines(message) self.outfile.write( "{line}{frame}{line}{msg}{line}{frame}{line}".format( line=os.linesep, frame=side_frame, msg=message)) @@ -246,7 +260,7 @@ class FileDisplay(object): """ side_frame = ("-" * 79) + os.linesep - message = self._wrap_lines(message) + message = _wrap_lines(message) self.outfile.write("{0}{frame}{msg}{0}{frame}".format( os.linesep, frame=side_frame, msg=message)) @@ -352,21 +366,6 @@ class FileDisplay(object): self.outfile.write(side_frame) - def _wrap_lines(self, msg): # pylint: disable=no-self-use - """Format lines nicely to 80 chars. - - :param str msg: Original message - - :returns: Formatted message respecting newlines in message - :rtype: str - - """ - lines = msg.splitlines() - fixed_l = [] - for line in lines: - fixed_l.append(textwrap.fill(line, 80)) - - return os.linesep.join(fixed_l) def _get_valid_int_ans(self, max_): """Get a numerical selection. @@ -409,20 +408,22 @@ class NoninteractiveDisplay(object): zope.interface.implements(interfaces.IDisplay) def __init__(self, outfile): - super(FileDisplay, self).__init__() + super(NoninteractiveDisplay, self).__init__() self.outfile = outfile - def _interaction_fail(self, message, extra=""): + def _interaction_fail(self, message, cli_flag, extra=""): "Error out in case of an attempt to interact in noninteractive mode" - msg ="Missing command line flag or config entry for this setting:\n" + msg = "Missing command line flag or config entry for this setting:\n" msg += message if extra: msg += "\n" + extra - raise MissingCommandlineFlag, msg + if cli_flag: + msg += "\n\n(You can set this with the {0} flag)".format(cli_flag) + raise errors.MissingCommandlineFlag, msg - def notification(self, message, height=10, pause=True): + def notification(self, message, height=10, pause=False): # pylint: disable=unused-argument - """Displays a notification and waits for user acceptance. + """Displays a notification without waiting for user acceptance. :param str message: Message to display to stdout :param int height: No effect for NoninteractiveDisplay @@ -430,23 +431,20 @@ class NoninteractiveDisplay(object): """ side_frame = "-" * 79 - message = self._wrap_lines(message) + message = _wrap_lines(message) self.outfile.write( "{line}{frame}{line}{msg}{line}{frame}{line}".format( line=os.linesep, frame=side_frame, msg=message)) - def menu(self, message, choices, ok_label="", cancel_label="", - help_label="", default=None): + def menu(self, message, choices, default=None, cli_flag=None, **kwargs): # pylint: disable=unused-argument - """Display a menu. - - .. todo:: This doesn't enable the help label/button (I wasn't sold on - any interface I came up with for this). It would be a nice feature + """Avoid displaying a menu. :param str message: title of menu :param choices: Menu lines, len must be > 0 :type choices: list of tuples (tag, item) or list of descriptions (tags will be enumerated) + :param dict kwargs: absorbs various irrelevant labelling arguments :returns: tuple of the form (code, tag) where code - int display exit code @@ -456,17 +454,11 @@ class NoninteractiveDisplay(object): """ if default is None: - self._interaction_fail(message, "Choices: " + repr(choices)) - if default == None: - msg ="Missing command line flag or config entry for this choice:\n" - msg += message - msg += "\nChoices: " + repr(choices) - raise MissingCommandlineFlag, msg + self._interaction_fail(message, cli_flag, "Choices: " + repr(choices)) return OK, choices.index(default) - def input(self, message, default=None): - # pylint: disable=no-self-use + def input(self, message, default=None, cli_flag=None): """Accept input from the user. :param str message: message to display to the user @@ -475,58 +467,39 @@ class NoninteractiveDisplay(object): `code` - str display exit code `input` - str of the user's input :rtype: tuple + :raises errors.MissingCommandlineFlag: if there was no default """ - ans = raw_input( - textwrap.fill("%s (Enter 'c' to cancel): " % message, 80)) - - if ans == "c" or ans == "C": - return CANCEL, "-1" + if default is None: + self._interaction_fail(message, cli_flag) else: - return OK, ans + return OK, default - def yesno(self, message, yes_label="Yes", no_label="No"): - """Query the user with a yes/no question. - Yes and No label must begin with different letters, and must contain at - least one letter each. + def yesno(self, message, default=None, cli_flag=None, **kwargs): + # pylint: disable=unused-argument + """Decide Yes or No, without asking anybody :param str message: question for the user - :param str yes_label: Label of the "Yes" parameter - :param str no_label: Label of the "No" parameter + :param dict kwargs: absorbs yes_label, no_label + :raises errors.MissingCommandlineFlag: if there was no default :returns: True for "Yes", False for "No" :rtype: bool """ - side_frame = ("-" * 79) + os.linesep + if default is None: + self._interaction_fail(message, cli_flag) + else: + return OK, default - message = self._wrap_lines(message) - - self.outfile.write("{0}{frame}{msg}{0}{frame}".format( - os.linesep, frame=side_frame, msg=message)) - - while True: - ans = raw_input("{yes}/{no}: ".format( - yes=_parens_around_char(yes_label), - no=_parens_around_char(no_label))) - - # Couldn't get pylint indentation right with elif - # elif doesn't matter in this situation - if (ans.startswith(yes_label[0].lower()) or - ans.startswith(yes_label[0].upper())): - return True - if (ans.startswith(no_label[0].lower()) or - ans.startswith(no_label[0].upper())): - return False - - def checklist(self, message, tags, default_status=True): + def checklist(self, message, tags, default=None, cli_flag=None, **kwargs): # pylint: disable=unused-argument """Display a checklist. :param str message: Message to display to user :param list tags: `str` tags to select, len(tags) > 0 - :param bool default_status: Not used for FileDisplay + :param dict kwargs: absorbs default_status arg :returns: tuple of (`code`, `tags`) where `code` - str display exit code @@ -534,129 +507,10 @@ class NoninteractiveDisplay(object): :rtype: tuple """ - while True: - self._print_menu(message, tags) - - code, ans = self.input("Select the appropriate numbers separated " - "by commas and/or spaces") - - if code == OK: - indices = separate_list_input(ans) - selected_tags = self._scrub_checklist_input(indices, tags) - if selected_tags: - return code, selected_tags - else: - self.outfile.write( - "** Error - Invalid selection **%s" % os.linesep) - else: - return code, [] - - def _scrub_checklist_input(self, indices, tags): - # pylint: disable=no-self-use - """Validate input and transform indices to appropriate tags. - - :param list indices: input - :param list tags: Original tags of the checklist - - :returns: valid tags the user selected - :rtype: :class:`list` of :class:`str` - - """ - # They should all be of type int - try: - indices = [int(index) for index in indices] - except ValueError: - return [] - - # Remove duplicates - indices = list(set(indices)) - - # Check all input is within range - for index in indices: - if index < 1 or index > len(tags): - return [] - # Transform indices to appropriate tags - return [tags[index - 1] for index in indices] - - def _print_menu(self, message, choices): - """Print a menu on the screen. - - :param str message: title of menu - :param choices: Menu lines - :type choices: list of tuples (tag, item) or - list of descriptions (tags will be enumerated) - - """ - # Can take either tuples or single items in choices list - if choices and isinstance(choices[0], tuple): - choices = ["%s - %s" % (c[0], c[1]) for c in choices] - - # Write out the message to the user - self.outfile.write( - "{new}{msg}{new}".format(new=os.linesep, msg=message)) - side_frame = ("-" * 79) + os.linesep - self.outfile.write(side_frame) - - # Write out the menu choices - for i, desc in enumerate(choices, 1): - self.outfile.write( - textwrap.fill("{num}: {desc}".format(num=i, desc=desc), 80)) - - # Keep this outside of the textwrap - self.outfile.write(os.linesep) - - self.outfile.write(side_frame) - - def _wrap_lines(self, msg): # pylint: disable=no-self-use - """Format lines nicely to 80 chars. - - :param str msg: Original message - - :returns: Formatted message respecting newlines in message - :rtype: str - - """ - lines = msg.splitlines() - fixed_l = [] - for line in lines: - fixed_l.append(textwrap.fill(line, 80)) - - return os.linesep.join(fixed_l) - - def _get_valid_int_ans(self, max_): - """Get a numerical selection. - - :param int max: The maximum entry (len of choices), must be positive - - :returns: tuple of the form (`code`, `selection`) where - `code` - str display exit code ('ok' or cancel') - `selection` - int user's selection - :rtype: tuple - - """ - selection = -1 - if max_ > 1: - input_msg = ("Select the appropriate number " - "[1-{max_}] then [enter] (press 'c' to " - "cancel): ".format(max_=max_)) + if default is None: + self._interaction_fail(message, cli_flag, "? ".join(tags)) else: - input_msg = ("Press 1 [enter] to confirm the selection " - "(press 'c' to cancel): ") - while selection < 1: - ans = raw_input(input_msg) - if ans.startswith("c") or ans.startswith("C"): - return CANCEL, -1 - try: - selection = int(ans) - if selection < 1 or selection > max_: - selection = -1 - raise ValueError - - except ValueError: - self.outfile.write( - "{0}** Invalid input **{0}".format(os.linesep)) - - return OK, selection + return OK, default def separate_list_input(input_): diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py index 1358d1048..99bb29d9d 100644 --- a/letsencrypt/errors.py +++ b/letsencrypt/errors.py @@ -102,3 +102,8 @@ class StandaloneBindError(Error): class ConfigurationError(Error): """Configuration sanity error.""" + +# NoninteractiveDisplay iDisplay plugin error: + +class MissingCommandlineFlag(Error): + """A command line argument was missing in noninteractive usage""" diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index 001a9e578..1d0c10d31 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -250,7 +250,7 @@ class FileOutputDisplayTest(unittest.TestCase): "This function is only meant to be for easy viewing{0}" "Test a really really really really really really really really " "really really really really long line...".format(os.linesep)) - text = self.displayer._wrap_lines(msg) + text = display_util._wrap_lines(msg) self.assertEqual(text.count(os.linesep), 3) From d471169b6b2fafade218251897a35f8fab72d7a9 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 28 Dec 2015 23:47:32 -0800 Subject: [PATCH 03/35] Add default-handling to the iDisplay interface (and the other iDisplay implementations) --- letsencrypt/display/util.py | 28 ++++++++++++++++++---------- letsencrypt/interfaces.py | 20 +++++++++++++------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 0c7926bd4..0d2dbc9d1 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -63,8 +63,8 @@ class NcursesDisplay(object): """ self.dialog.msgbox(message, height, width=self.width) - def menu(self, message, choices, - ok_label="OK", cancel_label="Cancel", help_label=""): + def menu(self, message, choices, ok_label="OK", cancel_label="Cancel", + help_label="", **_kwargs): """Display a menu. :param str message: title of menu @@ -75,6 +75,7 @@ class NcursesDisplay(object): :param str ok_label: label of the OK button :param str help_label: label of the help button + :param dict _kwargs: absorbs default / cli_args :returns: tuple of the form (`code`, `tag`) where `code` - `str` display_util exit code @@ -119,10 +120,11 @@ class NcursesDisplay(object): return code, int(tag) - 1 - def input(self, message): + def input(self, message, **_kwargs): """Display an input box to the user. :param str message: Message to display that asks for input. + :param dict _kwargs: absorbs default / cli_args :returns: tuple of the form (code, string) where `code` - int display exit code @@ -136,7 +138,7 @@ class NcursesDisplay(object): return self.dialog.inputbox(message, width=self.width, height=height) - def yesno(self, message, yes_label="Yes", no_label="No"): + def yesno(self, message, yes_label="Yes", no_label="No", **_kwargs): """Display a Yes/No dialog box. Yes and No label must begin with different letters. @@ -144,6 +146,7 @@ class NcursesDisplay(object): :param str message: message to display to user :param str yes_label: label on the "yes" button :param str no_label: label on the "no" button + :param dict _kwargs: absorbs default / cli_args :returns: if yes_label was selected :rtype: bool @@ -153,13 +156,14 @@ class NcursesDisplay(object): message, self.height, self.width, yes_label=yes_label, no_label=no_label) - def checklist(self, message, tags, default_status=True): + def checklist(self, message, tags, default_status=True, **_kwargs): """Displays a checklist. :param message: Message to display before choices :param list tags: where each is of type :class:`str` len(tags) > 0 :param bool default_status: If True, items are in a selected state by default. + :param dict _kwargs: absorbs default / cli_args :returns: tuple of the form (code, list_tags) where @@ -199,8 +203,8 @@ class FileDisplay(object): if pause: raw_input("Press Enter to Continue") - def menu(self, message, choices, - ok_label="", cancel_label="", help_label=""): + def menu(self, message, choices, ok_label="", cancel_label="", + help_label="", **_kwargs): # pylint: disable=unused-argument """Display a menu. @@ -211,6 +215,7 @@ class FileDisplay(object): :param choices: Menu lines, len must be > 0 :type choices: list of tuples (tag, item) or list of descriptions (tags will be enumerated) + :param dict _kwargs: absorbs default / cli_args :returns: tuple of the form (code, tag) where code - int display exit code @@ -224,11 +229,12 @@ class FileDisplay(object): return code, selection - 1 - def input(self, message): + def input(self, message, **_kwargs): # pylint: disable=no-self-use """Accept input from the user. :param str message: message to display to the user + :param dict _kwargs: absorbs default / cli_args :returns: tuple of (`code`, `input`) where `code` - str display exit code @@ -244,7 +250,7 @@ class FileDisplay(object): else: return OK, ans - def yesno(self, message, yes_label="Yes", no_label="No"): + def yesno(self, message, yes_label="Yes", no_label="No", **_kwargs): """Query the user with a yes/no question. Yes and No label must begin with different letters, and must contain at @@ -253,6 +259,7 @@ class FileDisplay(object): :param str message: question for the user :param str yes_label: Label of the "Yes" parameter :param str no_label: Label of the "No" parameter + :param dict _kwargs: absorbs default / cli_args :returns: True for "Yes", False for "No" :rtype: bool @@ -279,13 +286,14 @@ class FileDisplay(object): ans.startswith(no_label[0].upper())): return False - def checklist(self, message, tags, default_status=True): + def checklist(self, message, tags, default_status=True, **_kwargs): # pylint: disable=unused-argument """Display a checklist. :param str message: Message to display to user :param list tags: `str` tags to select, len(tags) > 0 :param bool default_status: Not used for FileDisplay + :param dict _kwargs: absorbs default / cli_args :returns: tuple of (`code`, `tags`) where `code` - str display exit code diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index c8a725fde..02d2802ed 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -365,8 +365,8 @@ class IDisplay(zope.interface.Interface): """ - def menu(message, choices, - ok_label="OK", cancel_label="Cancel", help_label=""): + def menu(message, choices, ok_label="OK", # pylint: disable=too-many-arguments + cancel_label="Cancel", help_label="", default=None, cli_flag=None): """Displays a generic menu. :param str message: message to display @@ -377,6 +377,8 @@ class IDisplay(zope.interface.Interface): :param str ok_label: label for OK button :param str cancel_label: label for Cancel button :param str help_label: label for Help button + :param str default: default (non-interactive) choice from the menu + :param str cli_flag: to automate choice from the menu, eg "--keep" :returns: tuple of (`code`, `index`) where `code` - str display exit code @@ -384,7 +386,7 @@ class IDisplay(zope.interface.Interface): """ - def input(message): + def input(message, default=None, cli_args=None): """Accept input from the user. :param str message: message to display to the user @@ -396,25 +398,29 @@ class IDisplay(zope.interface.Interface): """ - def yesno(message, yes_label="Yes", no_label="No"): + def yesno(message, yes_label="Yes", no_label="No", default=None, + cli_args=None): """Query the user with a yes/no question. Yes and No label must begin with different letters. :param str message: question for the user + :param str default: default (non-interactive) choice from the menu + :param str cli_flag: to automate choice from the menu, eg "--redirect / --no-redirect" :returns: True for "Yes", False for "No" :rtype: bool """ - def checklist(message, tags, default_state): + def checklist(message, tags, default_state, default=None, cli_args=None): """Allow for multiple selections from a menu. :param str message: message to display to the user :param list tags: where each is of type :class:`str` len(tags) > 0 - :param bool default_status: If True, items are in a selected state by - default. + :param bool default_status: If True, items are in a selected state by default. + :param str default: default (non-interactive) state of the checklist + :param str cli_flag: to automate choice from the menu, eg "--domains" """ From 6daaa7a7630f22d59b554ff80524ebf629b0acd2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 00:03:57 -0800 Subject: [PATCH 04/35] Add defaults / cli_flags for yesno() input --- letsencrypt/cli.py | 5 +++-- letsencrypt/display/ops.py | 3 ++- letsencrypt/plugins/manual.py | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index aba9116f9..5536d122f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -156,7 +156,7 @@ def _determine_account(args, config): "server at {1}".format( regr.terms_of_service, config.server)) return zope.component.getUtility(interfaces.IDisplay).yesno( - msg, "Agree", "Cancel") + msg, "Agree", "Cancel", cli_flag="--agree-tos") try: acc, acme = client.register( @@ -315,7 +315,8 @@ def _handle_subset_cert_request(config, domains, cert): ", ".join(domains), br=os.linesep) if config.expand or config.renew_by_default or zope.component.getUtility( - interfaces.IDisplay).yesno(question, "Expand", "Cancel"): + interfaces.IDisplay).yesno(question, "Expand", "Cancel", + default=True): return "renew", cert else: reporter_util = zope.component.getUtility(interfaces.IReporter) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 5ceb7fcfc..08ce0a0b3 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -197,7 +197,8 @@ def choose_names(installer): "specify ServerNames in your config files in order to allow for " "accurate installation of your certificate.{0}" "If you do use the default vhost, you may specify the name " - "manually. Would you like to continue?{0}".format(os.linesep)) + "manually. Would you like to continue?{0}".format(os.linesep), + default=False, cli_flag="--domains") if manual: return _choose_names_manually() diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 793285e62..7f782a41b 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -165,7 +165,8 @@ s.serve_forever()" """ else: if not self.conf("public-ip-logging-ok"): if not zope.component.getUtility(interfaces.IDisplay).yesno( - self.IP_DISCLAIMER, "Yes", "No"): + self.IP_DISCLAIMER, "Yes", "No", + cli_flag="--manual-public-ip-logging-ok"): raise errors.PluginError("Must agree to IP logging to proceed") self._notify_and_wait(self.MESSAGE_TEMPLATE.format( From 430604b63f336296b9149799878f60161fd1b8c5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 00:12:08 -0800 Subject: [PATCH 05/35] Add flag to enable non-interactivity --- letsencrypt/cli.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5536d122f..4a2ad5e85 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -947,6 +947,12 @@ def prepare_and_parse_args(plugins, args): helpful.add( None, "-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") + helpful.add( + None, "-n", "--non-interactive", "--noninteractive", + dest="noninteractive_mode", action="store_true", + help="Run without ever asking for user input. This may require " + "additional command line flags; the client will try to explain " + "which ones are required if it finds one missing") helpful.add( None, "--register-unsafely-without-email", action="store_true", help="Specifying this flag enables registering an account with no " @@ -1374,7 +1380,9 @@ def main(cli_args=sys.argv[1:]): sys.excepthook = functools.partial(_handle_exception, args=args) # Displayer - if args.text_mode: + if args.noninteractive_mode: + displayer = display_util.NoninteractiveDisplay(sys.stdout) + elif args.text_mode: displayer = display_util.FileDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() From 78f188968afc462002be0b7097f9e587bdbbc88c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 13:24:23 -0800 Subject: [PATCH 06/35] Update the plugin help to say something that always makes sense (Even if you do --help plugins) This can change again when #1460 is done --- letsencrypt/cli.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4a2ad5e85..62357a191 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1166,9 +1166,8 @@ def _plugins_parsing(helpful, plugins): "plugins", description="Let's Encrypt client supports an " "extensible plugins architecture. See '%(prog)s plugins' for a " "list of all installed plugins and their names. You can force " - "a particular plugin by setting options provided below. Further " - "down this help message you will find plugin-specific options " - "(prefixed by --{plugin_name}).") + "a particular plugin by setting options provided below. Running " + "--help will list flags specific to that plugin.") helpful.add( "plugins", "-a", "--authenticator", help="Authenticator plugin name.") helpful.add( From 4e0b010d3deaf043978ec50bea49095e8f0baa03 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 14:06:40 -0800 Subject: [PATCH 07/35] Improve interface docstrings --- letsencrypt/interfaces.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 02d2802ed..ced84bc54 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -377,13 +377,16 @@ class IDisplay(zope.interface.Interface): :param str ok_label: label for OK button :param str cancel_label: label for Cancel button :param str help_label: label for Help button - :param str default: default (non-interactive) choice from the menu + :param int default: default (non-interactive) choice from the menu :param str cli_flag: to automate choice from the menu, eg "--keep" :returns: tuple of (`code`, `index`) where `code` - str display exit code `index` - int index of the user's selection + :raises errors.MissingCommandlineFlag: if called in non-interactive + mode without a default set + """ def input(message, default=None, cli_args=None): @@ -411,6 +414,9 @@ class IDisplay(zope.interface.Interface): :returns: True for "Yes", False for "No" :rtype: bool + :raises errors.MissingCommandlineFlag: if called in non-interactive + mode without a default set + """ def checklist(message, tags, default_state, default=None, cli_args=None): @@ -422,6 +428,14 @@ class IDisplay(zope.interface.Interface): :param str default: default (non-interactive) state of the checklist :param str cli_flag: to automate choice from the menu, eg "--domains" + :returns: tuple of the form (code, list_tags) where + `code` - int display exit code + `list_tags` - list of str tags selected by the user + :rtype: tuple + + :raises errors.MissingCommandlineFlag: if called in non-interactive + mode without a default set + """ From 762697524833c00283feb089edf11da7f5a34f9d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 14:21:05 -0800 Subject: [PATCH 08/35] Make iDisplay.menu() calls non-interactive where possible - And provide helpful errors where they're not --- .../letsencrypt_apache/display_ops.py | 16 +++++++++++----- letsencrypt/cli.py | 2 +- letsencrypt/display/enhancements.py | 2 +- letsencrypt/display/ops.py | 12 ++++++++++-- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/display_ops.py b/letsencrypt-apache/letsencrypt_apache/display_ops.py index 45c55f49a..f4ce14a24 100644 --- a/letsencrypt-apache/letsencrypt_apache/display_ops.py +++ b/letsencrypt-apache/letsencrypt_apache/display_ops.py @@ -78,11 +78,17 @@ def _vhost_menu(domain, vhosts): name_size=disp_name_size) ) - code, tag = zope.component.getUtility(interfaces.IDisplay).menu( - "We were unable to find a vhost with a ServerName or Address of {0}.{1}" - "Which virtual host would you like to choose?".format( - domain, os.linesep), - choices, help_label="More Info", ok_label="Select") + try: + code, tag = zope.component.getUtility(interfaces.IDisplay).menu( + "We were unable to find a vhost with a ServerName or Address of {0}.{1}" + "Which virtual host would you like to choose?".format(domain, os.linesep), + choices, help_label="More Info", ok_label="Select") + except errors.MissingCommandlineFlag, e: + msg = ("Failed to run Apache plugin non-interactively{1}{0}{1}" + "(The best solution is to add ServerName or ServerAlias entries to " + "the VirtualHost directives of your apache configuration files.)".format(e, + os.linesep)) + raise errors.MissingCommandlineFlag, msg return code, tag diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 62357a191..0e77d65e4 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -280,7 +280,7 @@ def _handle_identical_cert_request(config, cert): "Cancel this operation and do nothing"] display = zope.component.getUtility(interfaces.IDisplay) - response = display.menu(question, choices, "OK", "Cancel") + response = display.menu(question, choices, "OK", "Cancel", default=0) if response[0] == "cancel" or response[1] == 2: # TODO: Add notification related to command-line options for # skipping the menu for this case. diff --git a/letsencrypt/display/enhancements.py b/letsencrypt/display/enhancements.py index c56198161..39def1651 100644 --- a/letsencrypt/display/enhancements.py +++ b/letsencrypt/display/enhancements.py @@ -48,7 +48,7 @@ def redirect_by_default(): code, selection = util(interfaces.IDisplay).menu( "Please choose whether HTTPS access is required or optional.", - choices) + choices, default=0, cli_flag="--redirect / --no-redirect") if code != display_util.OK: return False diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 08ce0a0b3..fde9c62d0 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -31,8 +31,16 @@ def choose_plugin(prepared, question): for plugin_ep in prepared] while True: - code, index = util(interfaces.IDisplay).menu( - question, opts, help_label="More Info") + disp = util(interfaces.IDisplay) + try: + code, index = disp.menu(question, opts, help_label="More Info") + except errors.MissingCommandlineFlag: + # use a custom message for this case + raise errors.MissingCommandlineFlag, ("Missing command line flags. For non-interactive " + "execution, you will need to specify a plugin on the command line. Run with " + "'--help plugins' to see a list of options, and see " + " https://eff.org/letsencrypt-plugins for more detail on what the plugins " + "do and how to use them.") if code == display_util.OK: plugin_ep = prepared[index] From 5ed9ac5ae6558280525d30a42ddedfb25fa76165 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 20:02:33 -0800 Subject: [PATCH 09/35] Handle non-interactivity of iDisplay.input() --- letsencrypt/display/ops.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index fde9c62d0..147a8b245 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -151,7 +151,12 @@ def get_email(more=False, invalid=False): msg += ('\n\nIf you really want to skip this, you can run the client with ' '--register-unsafely-without-email but make sure you backup your ' 'account key from /etc/letsencrypt/accounts\n\n') - code, email = zope.component.getUtility(interfaces.IDisplay).input(msg) + try: + code, email = zope.component.getUtility(interfaces.IDisplay).input(msg) + except errors.MissingCommandlineFlag, e: + msg = ("You should register before running non-interactively, or provide --agree-tos" + " and --email flags") + raise errors.MissingCommandlineFlag, msg if code == display_util.OK: if le_util.safe_email(email): @@ -259,7 +264,8 @@ def _choose_names_manually(): """Manually input names for those without an installer.""" code, input_ = util(interfaces.IDisplay).input( - "Please enter in your domain name(s) (comma and/or space separated) ") + "Please enter in your domain name(s) (comma and/or space separated) ", + cli_flag="--domains") if code == display_util.OK: invalid_domains = dict() From 7daf773c7325699d7cc2c535b37bd5e5160903d7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 20:23:23 -0800 Subject: [PATCH 10/35] Handle noninteractiv calls to iDisplay.checklist() --- letsencrypt/display/ops.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 147a8b245..aef8f65e6 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -211,7 +211,7 @@ def choose_names(installer): "accurate installation of your certificate.{0}" "If you do use the default vhost, you may specify the name " "manually. Would you like to continue?{0}".format(os.linesep), - default=False, cli_flag="--domains") + default=True) if manual: return _choose_names_manually() @@ -256,7 +256,7 @@ def _filter_names(names): """ code, names = util(interfaces.IDisplay).checklist( "Which names would you like to activate HTTPS for?", - tags=names) + tags=names, cli_flag="--domains") return code, [str(s) for s in names] From 548ba6b6552747fc9edfb42d67b92003ecd70c56 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 29 Dec 2015 20:25:07 -0800 Subject: [PATCH 11/35] lint --- letsencrypt-apache/letsencrypt_apache/display_ops.py | 1 + letsencrypt/display/ops.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/display_ops.py b/letsencrypt-apache/letsencrypt_apache/display_ops.py index f4ce14a24..73fec220e 100644 --- a/letsencrypt-apache/letsencrypt_apache/display_ops.py +++ b/letsencrypt-apache/letsencrypt_apache/display_ops.py @@ -4,6 +4,7 @@ import os import zope.component +from letsencrypt import errors from letsencrypt import interfaces import letsencrypt.display.util as display_util diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index aef8f65e6..90d3d97c3 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -153,7 +153,7 @@ def get_email(more=False, invalid=False): 'account key from /etc/letsencrypt/accounts\n\n') try: code, email = zope.component.getUtility(interfaces.IDisplay).input(msg) - except errors.MissingCommandlineFlag, e: + except errors.MissingCommandlineFlag: msg = ("You should register before running non-interactively, or provide --agree-tos" " and --email flags") raise errors.MissingCommandlineFlag, msg From 50fa6c7f22093a14c93fef79cf7f0c875ea8e044 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 30 Dec 2015 15:10:02 -0800 Subject: [PATCH 12/35] Error helpfully on "letsencrypt --standalone" - Also refactor choose_configurator_plugins a bit --- letsencrypt/cli.py | 79 ++++++++++++++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 0e77d65e4..32833e51a 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -432,21 +432,6 @@ def _avoid_invalidating_lineage(config, lineage, original_server): "a test certificate (domains: {0}). We will not do that " "unless you use the --break-my-certs flag!".format(names)) -def set_configurator(previously, now): - """ - Setting configurators multiple ways is okay, as long as they all agree - :param str previously: previously identified request for the installer/authenticator - :param str requested: the request currently being processed - """ - if now is None: - # we're not actually setting anything - return previously - if previously: - if previously != now: - msg = "Too many flags setting configurators/installers/authenticators {0} -> {1}" - raise errors.PluginSelectionError(msg.format(repr(previously), repr(now))) - return now - def diagnose_configurator_problem(cfg_type, requested, plugins): """ @@ -480,22 +465,28 @@ def diagnose_configurator_problem(cfg_type, requested, plugins): raise errors.PluginSelectionError(msg) -def choose_configurator_plugins(args, config, plugins, verb): # pylint: disable=too-many-branches +def set_configurator(previously, now): """ - Figure out which configurator we're going to use - :raises error.PluginSelectionError if there was a problem + Setting configurators multiple ways is okay, as long as they all agree + :param str previously: previously identified request for the installer/authenticator + :param str requested: the request currently being processed """ + if now is None: + # we're not actually setting anything + return previously + if previously: + if previously != now: + msg = "Too many flags setting configurators/installers/authenticators {0} -> {1}" + raise errors.PluginSelectionError(msg.format(repr(previously), repr(now))) + return now - # Which plugins do we need? - need_inst = need_auth = (verb == "run") - if verb == "certonly": - need_auth = True - if verb == "install": - need_inst = True - if args.authenticator: - logger.warn("Specifying an authenticator doesn't make sense in install mode") +def cli_plugin_requests(args): + """ + Figure out which plugins the user requested with CLI and config options - # Which plugins did the user request? + :returns: (requested authenticator string or None, requested installer string or None) + :rtype: tuple + """ req_inst = req_auth = args.configurator req_inst = set_configurator(req_inst, args.installer) req_auth = set_configurator(req_auth, args.authenticator) @@ -512,6 +503,40 @@ def choose_configurator_plugins(args, config, plugins, verb): # pylint: disable if args.manual: req_auth = set_configurator(req_auth, "manual") logger.debug("Requested authenticator %s and installer %s", req_auth, req_inst) + return req_auth, req_inst + + +noninstaller_plugins = ["webroot", "manual", "standalone"] + +def choose_configurator_plugins(args, config, plugins, verb): + """ + Figure out which configurator we're going to use + :raises errors.PluginSelectionError if there was a problem + """ + + req_auth, req_inst = cli_plugin_requests(args) + + # Which plugins do we need? + if verb == "run": + need_inst = need_auth = True + if req_auth in noninstaller_plugins and not req_inst: + msg = ('With the {0} plugin, you probably want to use the "certonly" command, eg:{1}' + '{1} {2} certonly --{0}{1}{1}' + '(Alternatively, add a --installer flag. See https://eff.org/letsencrypt-plugins' + '{1} and "--help plugins" for more information.)'.format( + req_auth, os.linesep, cmd)) + + raise errors.MissingCommandlineFlag, msg + else: + need_inst = need_auth = False + if verb == "certonly": + need_auth = True + if verb == "install": + need_inst = True + if args.authenticator: + logger.warn("Specifying an authenticator doesn't make sense in install mode") + + # Try to meet the user's request and/or ask them to pick plugins authenticator = installer = None From 833e61a411fb983992b54c998aa12addd1fe82c3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 30 Dec 2015 15:11:27 -0800 Subject: [PATCH 13/35] Update usage to be letsencrypt-auto or letsencrypt, as appropriate --- letsencrypt/cli.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 32833e51a..196c64f98 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -44,6 +44,11 @@ from letsencrypt.plugins import disco as plugins_disco logger = logging.getLogger(__name__) +# For help strings, figure out how the user ran us. +# When invoked from letsencrypt-auto, sys.argv[0] is something like: +# /home/user/.local/share/letsencrypt/bin/letsencrypt" +fragment = os.path.join(".local", "share", "letsencrypt") +cli_command = "letsencrypt-auto" if fragment in sys.argv[0] else "letsencrypt" # Argparse's help formatting has a lot of unhelpful peculiarities, so we want # to replace as much of it as we can... @@ -51,7 +56,7 @@ logger = logging.getLogger(__name__) # This is the stub to include in help generated by argparse SHORT_USAGE = """ - letsencrypt [SUBCOMMAND] [options] [-d domain] [-d domain] ... + {0} [SUBCOMMAND] [options] [-d domain] [-d domain] ... The Let's Encrypt agent can obtain and install HTTPS/TLS/SSL certificates. By default, it will attempt to use a webserver both for obtaining and installing @@ -65,7 +70,7 @@ the cert. Major SUBCOMMANDS are: config_changes Show changes made to server config during installation plugins Display information about installed plugins -""" +""".format(cli_command) # This is the short help for letsencrypt --help, where we disable argparse # altogether From ef09362a9ff52a53ea5fc148a651bba105bc81a2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 30 Dec 2015 15:48:59 -0800 Subject: [PATCH 14/35] bugfix --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 196c64f98..037188bb2 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -529,7 +529,7 @@ def choose_configurator_plugins(args, config, plugins, verb): '{1} {2} certonly --{0}{1}{1}' '(Alternatively, add a --installer flag. See https://eff.org/letsencrypt-plugins' '{1} and "--help plugins" for more information.)'.format( - req_auth, os.linesep, cmd)) + req_auth, os.linesep, cli_command)) raise errors.MissingCommandlineFlag, msg else: From d358c8d1c0a037cedb18b88f077533e60a84be34 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 30 Dec 2015 15:52:51 -0800 Subject: [PATCH 15/35] Correct docstrings & associated typing confusion --- letsencrypt/display/util.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 0d2dbc9d1..a0ebaffa0 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -77,9 +77,9 @@ class NcursesDisplay(object): :param str help_label: label of the help button :param dict _kwargs: absorbs default / cli_args - :returns: tuple of the form (`code`, `tag`) where - `code` - `str` display_util exit code - `tag` - `int` index corresponding to the item chosen + :returns: tuple of the form (code, tag) where + code - int display exit code + tag - str corresponding to the item chosen :rtype: tuple """ @@ -217,9 +217,10 @@ class FileDisplay(object): list of descriptions (tags will be enumerated) :param dict _kwargs: absorbs default / cli_args - :returns: tuple of the form (code, tag) where - code - int display exit code - tag - str corresponding to the item chosen + :returns: tuple of (`code`, `index`) where + `code` - str display exit code + `index` - int index of the user's selection + :rtype: tuple """ @@ -452,11 +453,12 @@ class NoninteractiveDisplay(object): :param choices: Menu lines, len must be > 0 :type choices: list of tuples (tag, item) or list of descriptions (tags will be enumerated) + :param int default: the default choice :param dict kwargs: absorbs various irrelevant labelling arguments - :returns: tuple of the form (code, tag) where - code - int display exit code - tag - str corresponding to the item chosen + :returns: tuple of (`code`, `index`) where + `code` - str display exit code + `index` - int index of the user's selection :rtype: tuple :raises errors.MissingCommandlineFlag: if there was no default @@ -464,7 +466,7 @@ class NoninteractiveDisplay(object): if default is None: self._interaction_fail(message, cli_flag, "Choices: " + repr(choices)) - return OK, choices.index(default) + return OK, default def input(self, message, default=None, cli_flag=None): """Accept input from the user. From b5828d92ad2c33d58ab9b24c9a6e42c1f4265200 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 30 Dec 2015 16:07:25 -0800 Subject: [PATCH 16/35] Test cases for NoninteractiveDisplay (and one of the associated bugfixes :) --- letsencrypt/display/util.py | 2 +- letsencrypt/tests/display/util_test.py | 36 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index a0ebaffa0..d02f74681 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -501,7 +501,7 @@ class NoninteractiveDisplay(object): if default is None: self._interaction_fail(message, cli_flag) else: - return OK, default + return default def checklist(self, message, tags, default=None, cli_flag=None, **kwargs): # pylint: disable=unused-argument diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index 1d0c10d31..8759a7faa 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -4,6 +4,8 @@ import unittest import mock +import letsencrypt.errors as errors + from letsencrypt.display import util as display_util @@ -278,6 +280,40 @@ class FileOutputDisplayTest(unittest.TestCase): self.displayer._get_valid_int_ans(3), (display_util.CANCEL, -1)) +class NoninteractiveDisplayTest(unittest.TestCase): + """Test non-interactive display. + + These tests are pretty easy! + + """ + def setUp(self): + super(NoninteractiveDisplayTest, self).setUp() + self.mock_stdout = mock.MagicMock() + self.displayer = display_util.NoninteractiveDisplay(self.mock_stdout) + + def test_input(self): + d = "an incomputable value" + ret = self.displayer.input("message", default=d) + self.assertEqual(ret, (display_util.OK, d)) + self.assertRaises(errors.MissingCommandlineFlag, self.displayer.input, "message") + + def test_menu(self): + ret = self.displayer.menu("message", CHOICES, default=1) + self.assertEqual(ret, (display_util.OK, 1)) + self.assertRaises(errors.MissingCommandlineFlag, self.displayer.menu, "message", CHOICES) + + def test_yesno(self): + d = False + ret = self.displayer.yesno("message", default=d) + self.assertEqual(ret, d) + self.assertRaises(errors.MissingCommandlineFlag, self.displayer.yesno, "message") + + def test_checklist(self): + d = [1, 3] + ret = self.displayer.menu("message", TAGS, default=d) + self.assertEqual(ret, (display_util.OK, d)) + self.assertRaises(errors.MissingCommandlineFlag, self.displayer.checklist, "message", TAGS) + class SeparateListInputTest(unittest.TestCase): """Test Module functions.""" From 96f704f5770a422ce6ad1b88e1330326540aa9a0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 1 Jan 2016 15:29:34 -0800 Subject: [PATCH 17/35] Test cases for various error cases (and associtated bugfixes) --- letsencrypt/cli.py | 27 +++++++++++++++++---------- letsencrypt/display/util.py | 2 +- letsencrypt/tests/cli_test.py | 30 +++++++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 037188bb2..18e7a96b2 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -160,12 +160,14 @@ def _determine_account(args, config): "must agree in order to register with the ACME " "server at {1}".format( regr.terms_of_service, config.server)) - return zope.component.getUtility(interfaces.IDisplay).yesno( - msg, "Agree", "Cancel", cli_flag="--agree-tos") + obj = zope.component.getUtility(interfaces.IDisplay) + return obj.yesno(msg, "Agree", "Cancel", cli_flag="--agree-tos") try: acc, acme = client.register( config, account_storage, tos_cb=_tos_cb) + except errors.MissingCommandlineFlag: + raise except errors.Error as error: logger.debug(error, exc_info=True) raise errors.Error( @@ -636,6 +638,8 @@ def obtain_cert(args, config, plugins): def install(args, config, plugins): """Install a previously obtained cert in a server.""" # XXX: Update for renewer/RenewableCert + # FIXME: be consistent about whether errors are raised or returned from + # this function ... try: installer, _ = choose_configurator_plugins(args, config, @@ -643,14 +647,17 @@ def install(args, config, plugins): except errors.PluginSelectionError, e: return e.message - domains = _find_domains(args, installer) - le_client = _init_le_client( - args, config, authenticator=None, installer=installer) - assert args.cert_path is not None # required=True in the subparser - le_client.deploy_certificate( - domains, args.key_path, args.cert_path, args.chain_path, - args.fullchain_path) - le_client.enhance_config(domains, config) + try: + domains = _find_domains(args, installer) + le_client = _init_le_client( + args, config, authenticator=None, installer=installer) + assert args.cert_path is not None # required=True in the subparser + le_client.deploy_certificate( + domains, args.key_path, args.cert_path, args.chain_path, + args.fullchain_path) + le_client.enhance_config(domains, config) + except errors.MissingCommandlineFlag, e: + return e.message def revoke(args, config, unused_plugins): # TODO: coop with renewal config diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index d02f74681..2ea6b9bfe 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -486,7 +486,7 @@ class NoninteractiveDisplay(object): return OK, default - def yesno(self, message, default=None, cli_flag=None, **kwargs): + def yesno(self, message, yes_label=None, no_label=None, default=None, cli_flag=None): # pylint: disable=unused-argument """Decide Yes or No, without asking anybody diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ccf16f5b5..688e92ad3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -81,7 +81,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(1, mock_run.call_count) def _help_output(self, args): - "Run a help command, and return the help string for scrutiny" + "Run a command, and return the ouput string for scrutiny" output = StringIO.StringIO() with mock.patch('letsencrypt.cli.sys.stdout', new=output): self.assertRaises(SystemExit, self._call_stdout, args) @@ -105,6 +105,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue("--checkpoints" not in out) out = self._help_output(['-h']) + self.assertTrue("letsencrypt-auto" not in out) # test cli.cli_command if "nginx" in plugins: self.assertTrue("Use the Nginx plugin" in out) else: @@ -130,6 +131,31 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods out = self._help_output(['-h']) self.assertTrue(cli.usage_strings(plugins)[0] in out) + + def _cli_missing_flag(self, args, message): + "Ensure that a particular error raises a missing cli flag error containing message" + exc = None + try: + #self._call_no_clientmock(args) + with mock.patch('letsencrypt.cli.sys.stderr') as stderr: + out = cli.main(self.standard_args + args[:]) # NOTE: parser can alter its args! + #out = self._help_output(args) + print out + except errors.MissingCommandlineFlag, exc: + #print "checking for " + message + " in\n"+ str(exc) + self.assertTrue(message in str(exc)) + self.assertTrue(exc is not None) + + def test_noninteractive(self): + args = ['-n', 'certonly'] + self._cli_missing_flag(args, "specify a plugin") + args.extend(['--apache', '-d', 'eg.is']) + self._cli_missing_flag(args, "register before running") + with mock.patch('letsencrypt.cli._auth_from_domains'): + with mock.patch('letsencrypt.cli.client.acme_from_config_key'): + args.extend(['--email', 'io@io.is']) + self._cli_missing_flag(args, "--agree-tos") + @mock.patch('letsencrypt.cli.client.acme_client.Client') @mock.patch('letsencrypt.cli._determine_account') @mock.patch('letsencrypt.cli.client.Client.obtain_and_enroll_certificate') @@ -209,6 +235,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods ret, _, _, _ = self._call(args) self.assertTrue("--webroot-path must be set" in ret) + self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably") + with mock.patch("letsencrypt.cli._init_le_client") as mock_init: with mock.patch("letsencrypt.cli._auth_from_domains"): self._call(["certonly", "--manual", "-d", "foo.bar"]) From 59f68d074fb16877ee7a70411e315c70cf8ac778 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 1 Jan 2016 15:31:43 -0800 Subject: [PATCH 18/35] Clean-ups & lint chasing --- letsencrypt/tests/cli_test.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 688e92ad3..5c5e7f8bd 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -136,13 +136,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods "Ensure that a particular error raises a missing cli flag error containing message" exc = None try: - #self._call_no_clientmock(args) - with mock.patch('letsencrypt.cli.sys.stderr') as stderr: + with mock.patch('letsencrypt.cli.sys.stderr'): out = cli.main(self.standard_args + args[:]) # NOTE: parser can alter its args! - #out = self._help_output(args) print out except errors.MissingCommandlineFlag, exc: - #print "checking for " + message + " in\n"+ str(exc) self.assertTrue(message in str(exc)) self.assertTrue(exc is not None) From 77ec944da11eebb659f755e4508d838b65551ab0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 1 Jan 2016 17:10:57 -0800 Subject: [PATCH 19/35] One more apache unit test --- .../letsencrypt_apache/tests/display_ops_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py index 6db319d87..590144372 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py @@ -6,6 +6,7 @@ import mock import zope.component from letsencrypt.display import util as display_util +from letsencrypt import errors from letsencrypt_apache import obj @@ -31,6 +32,14 @@ class SelectVhostTest(unittest.TestCase): mock_util().menu.return_value = (display_util.OK, 3) self.assertEqual(self.vhosts[3], self._call(self.vhosts)) + @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") + def test_noninteractive(self, mock_util): + mock_util().menu.side_effect = errors.MissingCommandlineFlag("no vhost default") + try: + self._call(self.vhosts) + except errors.MissingCommandlineFlag, e: + self.assertTrue("VirtualHost directives" in e.message) + @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") def test_more_info_cancel(self, mock_util): mock_util().menu.side_effect = [ From 1f45c2ca5ce31d6ded34bf8d4ea277b145e1227b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 9 Jan 2016 15:10:06 -0800 Subject: [PATCH 20/35] s/--apache/--standalong in non-interactive unit tests Since apache may not be installed in travis or other test envs --- letsencrypt/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 5c5e7f8bd..0775bc349 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -146,7 +146,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_noninteractive(self): args = ['-n', 'certonly'] self._cli_missing_flag(args, "specify a plugin") - args.extend(['--apache', '-d', 'eg.is']) + args.extend(['--standalone', '-d', 'eg.is']) self._cli_missing_flag(args, "register before running") with mock.patch('letsencrypt.cli._auth_from_domains'): with mock.patch('letsencrypt.cli.client.acme_from_config_key'): From 74f09fb7bdd7930f843c62618a41b8cf27a85e63 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 9 Jan 2016 15:43:58 -0800 Subject: [PATCH 21/35] Never auto-select plugins in non-interactive mode * We really want the user to pick one, so that the later addition of a second option doesn't cause -n mode to fail. --- letsencrypt/display/ops.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 90d3d97c3..5568e24b6 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -32,15 +32,7 @@ def choose_plugin(prepared, question): while True: disp = util(interfaces.IDisplay) - try: - code, index = disp.menu(question, opts, help_label="More Info") - except errors.MissingCommandlineFlag: - # use a custom message for this case - raise errors.MissingCommandlineFlag, ("Missing command line flags. For non-interactive " - "execution, you will need to specify a plugin on the command line. Run with " - "'--help plugins' to see a list of options, and see " - " https://eff.org/letsencrypt-plugins for more detail on what the plugins " - "do and how to use them.") + code, index = disp.menu(question, opts, help_label="More Info") if code == display_util.OK: plugin_ep = prepared[index] @@ -82,6 +74,16 @@ def pick_plugin(config, default, plugins, question, ifaces): # throw more UX-friendly error if default not in plugins filtered = plugins.filter(lambda p_ep: p_ep.name == default) else: + if config.noninteractive_mode: + # it's really bad to auto-select the single available plugin in + # non-interactive mode, because an update could later add a second + # available plugin + raise errors.MissingCommandlineFlag, ("Missing command line flags. For non-interactive " + "execution, you will need to specify a plugin on the command line. Run with " + "'--help plugins' to see a list of options, and see " + " https://eff.org/letsencrypt-plugins for more detail on what the plugins " + "do and how to use them.") + filtered = plugins.visible().ifaces(ifaces) filtered.init(config) From 3c70af7da5ab37e2bbe9093ab3d5b9193e18b94e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 9 Jan 2016 15:55:54 -0800 Subject: [PATCH 22/35] Avoid accidentally mocking noninteractivity --- letsencrypt/tests/display/ops_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 31db47cce..8db751e34 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -69,7 +69,7 @@ class PickPluginTest(unittest.TestCase): """Tests for letsencrypt.display.ops.pick_plugin.""" def setUp(self): - self.config = mock.Mock() + self.config = mock.Mock(noninteractive_mode=False) self.default = None self.reg = mock.MagicMock() self.question = "Question?" From e112e2ce6126845afb06028abdc81008cbbde507 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 20 Jan 2016 17:16:19 -0800 Subject: [PATCH 23/35] Remove pylint disable --- letsencrypt/display/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 2ea6b9bfe..602c246de 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -21,7 +21,7 @@ CANCEL = "cancel" HELP = "help" """Display exit code when for when the user requests more help.""" -def _wrap_lines(msg): # pylint: disable=no-self-use +def _wrap_lines(msg): """Format lines nicely to 80 chars. :param str msg: Original message From 22dccf0adb30369351a7c062f90934073de9451a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 20 Jan 2016 17:16:32 -0800 Subject: [PATCH 24/35] Use sphinx backticks more consistently --- letsencrypt/display/util.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 602c246de..47f54cd5f 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -77,9 +77,9 @@ class NcursesDisplay(object): :param str help_label: label of the help button :param dict _kwargs: absorbs default / cli_args - :returns: tuple of the form (code, tag) where - code - int display exit code - tag - str corresponding to the item chosen + :returns: tuple of the form (`code`, `tag`) where + `code` - int display exit code + `tag` - str corresponding to the item chosen :rtype: tuple """ @@ -126,7 +126,7 @@ class NcursesDisplay(object): :param str message: Message to display that asks for input. :param dict _kwargs: absorbs default / cli_args - :returns: tuple of the form (code, string) where + :returns: tuple of the form (`code`, `string`) where `code` - int display exit code `string` - input entered by the user @@ -166,7 +166,7 @@ class NcursesDisplay(object): :param dict _kwargs: absorbs default / cli_args - :returns: tuple of the form (code, list_tags) where + :returns: tuple of the form (`code`, `list_tags`) where `code` - int display exit code `list_tags` - list of str tags selected by the user From 410bd227936b481e41a648ec1b1ce264c856f63b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 20 Jan 2016 17:21:40 -0800 Subject: [PATCH 25/35] As previously implemented, iDisplay.menu() returns an index, not a tag --- letsencrypt/display/util.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index 47f54cd5f..dde00584f 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -77,9 +77,9 @@ class NcursesDisplay(object): :param str help_label: label of the help button :param dict _kwargs: absorbs default / cli_args - :returns: tuple of the form (`code`, `tag`) where + :returns: tuple of the form (`code`, `index`) where `code` - int display exit code - `tag` - str corresponding to the item chosen + `int` - index of the selected item :rtype: tuple """ @@ -112,12 +112,12 @@ class NcursesDisplay(object): (str(i), choice) for i, choice in enumerate(choices, 1) ] # pylint: disable=star-args - code, tag = self.dialog.menu(message, **menu_options) + code, index = self.dialog.menu(message, **menu_options) if code == CANCEL: return code, -1 - return code, int(tag) - 1 + return code, int(index) - 1 def input(self, message, **_kwargs): From 8c9757a06255c4486608956fa5226d8812a1888d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 20 Jan 2016 17:55:45 -0800 Subject: [PATCH 26/35] Correct docstring --- letsencrypt/display/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index dde00584f..a91c5bab1 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -412,7 +412,7 @@ class FileDisplay(object): return OK, selection class NoninteractiveDisplay(object): - """File-based display.""" + """An iDisplay implementation that never asks for interactive user input""" zope.interface.implements(interfaces.IDisplay) From b75235b3dc80bd79a06a9800d956403a4517e823 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jan 2016 15:27:23 -0800 Subject: [PATCH 27/35] Close test coverage gaps (And fix a bug in one test...) --- letsencrypt/tests/display/util_test.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index 8759a7faa..a16eb544e 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -291,6 +291,12 @@ class NoninteractiveDisplayTest(unittest.TestCase): self.mock_stdout = mock.MagicMock() self.displayer = display_util.NoninteractiveDisplay(self.mock_stdout) + def test_notification_no_pause(self): + self.displayer.notification("message", 10) + string = self.mock_stdout.write.call_args[0][0] + + self.assertTrue("message" in string) + def test_input(self): d = "an incomputable value" ret = self.displayer.input("message", default=d) @@ -310,7 +316,7 @@ class NoninteractiveDisplayTest(unittest.TestCase): def test_checklist(self): d = [1, 3] - ret = self.displayer.menu("message", TAGS, default=d) + ret = self.displayer.checklist("message", TAGS, default=d) self.assertEqual(ret, (display_util.OK, d)) self.assertRaises(errors.MissingCommandlineFlag, self.displayer.checklist, "message", TAGS) From 467e8fdb049395f624c51cbb0b18c8d8935c562b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jan 2016 15:38:38 -0800 Subject: [PATCH 28/35] [interfaces.py] add missing :raises: for iDisplay.input --- letsencrypt/interfaces.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index ced84bc54..db5d2c5e8 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -399,6 +399,9 @@ class IDisplay(zope.interface.Interface): `input` - str of the user's input :rtype: tuple + :raises errors.MissingCommandlineFlag: if called in non-interactive + mode without a default set + """ def yesno(message, yes_label="Yes", no_label="No", default=None, From 81d9ed220c009126f7e2ad74f4838ab7dce74a25 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jan 2016 15:44:37 -0800 Subject: [PATCH 29/35] Less elaborate exception processing --- letsencrypt/cli.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4eaadb5f6..0eb9595ca 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -647,17 +647,14 @@ def install(args, config, plugins): except errors.PluginSelectionError, e: return e.message - try: - domains = _find_domains(args, installer) - le_client = _init_le_client( - args, config, authenticator=None, installer=installer) - assert args.cert_path is not None # required=True in the subparser - le_client.deploy_certificate( - domains, args.key_path, args.cert_path, args.chain_path, - args.fullchain_path) - le_client.enhance_config(domains, config) - except errors.MissingCommandlineFlag, e: - return e.message + domains = _find_domains(args, installer) + le_client = _init_le_client( + args, config, authenticator=None, installer=installer) + assert args.cert_path is not None # required=True in the subparser + le_client.deploy_certificate( + domains, args.key_path, args.cert_path, args.chain_path, + args.fullchain_path) + le_client.enhance_config(domains, config) def revoke(args, config, unused_plugins): # TODO: coop with renewal config From 9a1199ed24b3346689b0a3e7eec8815ae0f1e9c4 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jan 2016 15:50:45 -0800 Subject: [PATCH 30/35] Wrangle a lot of **_kwargs --- letsencrypt/display/util.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index a91c5bab1..12c32ff05 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -64,7 +64,7 @@ class NcursesDisplay(object): self.dialog.msgbox(message, height, width=self.width) def menu(self, message, choices, ok_label="OK", cancel_label="Cancel", - help_label="", **_kwargs): + help_label="", **unused_kwargs): """Display a menu. :param str message: title of menu @@ -75,7 +75,7 @@ class NcursesDisplay(object): :param str ok_label: label of the OK button :param str help_label: label of the help button - :param dict _kwargs: absorbs default / cli_args + :param dict unused_kwargs: absorbs default / cli_args :returns: tuple of the form (`code`, `index`) where `code` - int display exit code @@ -120,7 +120,7 @@ class NcursesDisplay(object): return code, int(index) - 1 - def input(self, message, **_kwargs): + def input(self, message, **unused_kwargs): """Display an input box to the user. :param str message: Message to display that asks for input. @@ -138,7 +138,7 @@ class NcursesDisplay(object): return self.dialog.inputbox(message, width=self.width, height=height) - def yesno(self, message, yes_label="Yes", no_label="No", **_kwargs): + def yesno(self, message, yes_label="Yes", no_label="No", **unused_kwargs): """Display a Yes/No dialog box. Yes and No label must begin with different letters. @@ -156,7 +156,7 @@ class NcursesDisplay(object): message, self.height, self.width, yes_label=yes_label, no_label=no_label) - def checklist(self, message, tags, default_status=True, **_kwargs): + def checklist(self, message, tags, default_status=True, **unused_kwargs): """Displays a checklist. :param message: Message to display before choices @@ -204,7 +204,7 @@ class FileDisplay(object): raw_input("Press Enter to Continue") def menu(self, message, choices, ok_label="", cancel_label="", - help_label="", **_kwargs): + help_label="", **unused_kwargs): # pylint: disable=unused-argument """Display a menu. @@ -230,7 +230,7 @@ class FileDisplay(object): return code, selection - 1 - def input(self, message, **_kwargs): + def input(self, message, **unused_kwargs): # pylint: disable=no-self-use """Accept input from the user. @@ -251,7 +251,7 @@ class FileDisplay(object): else: return OK, ans - def yesno(self, message, yes_label="Yes", no_label="No", **_kwargs): + def yesno(self, message, yes_label="Yes", no_label="No", **unused_kwargs): """Query the user with a yes/no question. Yes and No label must begin with different letters, and must contain at @@ -287,7 +287,7 @@ class FileDisplay(object): ans.startswith(no_label[0].upper())): return False - def checklist(self, message, tags, default_status=True, **_kwargs): + def checklist(self, message, tags, default_status=True, **unused_kwargs): # pylint: disable=unused-argument """Display a checklist. @@ -445,8 +445,9 @@ class NoninteractiveDisplay(object): "{line}{frame}{line}{msg}{line}{frame}{line}".format( line=os.linesep, frame=side_frame, msg=message)) - def menu(self, message, choices, default=None, cli_flag=None, **kwargs): - # pylint: disable=unused-argument + def menu(self, message, choices, ok_label=None, cancel_label=None, + default=None, cli_flag=None): + # pylint: disable=unused-argument,too-many-arguments """Avoid displaying a menu. :param str message: title of menu From e6c608b0a69ecd87318f2eef13874a5be94a8fe1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 21 Jan 2016 15:55:44 -0800 Subject: [PATCH 31/35] Do not autoexpand --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 0eb9595ca..ab04b906d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -323,7 +323,7 @@ def _handle_subset_cert_request(config, domains, cert): br=os.linesep) if config.expand or config.renew_by_default or zope.component.getUtility( interfaces.IDisplay).yesno(question, "Expand", "Cancel", - default=True): + cli_flag="--expand (or in some cases, --duplicate)"): return "renew", cert else: reporter_util = zope.component.getUtility(interfaces.IReporter) From 904f44864e3b1721b52ac1ce0b0088fef0b13d19 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 22 Jan 2016 16:44:54 -0800 Subject: [PATCH 32/35] rm stray print --- letsencrypt/tests/cli_test.py | 1 - tests/letstest/scripts/test_leauto_upgrades.sh | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 75c69b502..bc0fa6892 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -138,7 +138,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods try: with mock.patch('letsencrypt.cli.sys.stderr'): out = cli.main(self.standard_args + args[:]) # NOTE: parser can alter its args! - print out except errors.MissingCommandlineFlag, exc: self.assertTrue(message in str(exc)) self.assertTrue(exc is not None) diff --git a/tests/letstest/scripts/test_leauto_upgrades.sh b/tests/letstest/scripts/test_leauto_upgrades.sh index b7849755a..262839ab1 100755 --- a/tests/letstest/scripts/test_leauto_upgrades.sh +++ b/tests/letstest/scripts/test_leauto_upgrades.sh @@ -13,7 +13,7 @@ unset PIP_INDEX_URL export PIP_EXTRA_INDEX_URL="$SAVE" -if ! ./letsencrypt-auto -v --debug --version | grep 0.1.1 ; then +if ! ./letsencrypt-auto -v --debug --version | grep 0.2.0 ; then echo upgrade appeared to fail exit 1 fi From 260534d1c3b91ead22ea14c91f79f978410e97a5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 22 Jan 2016 17:23:13 -0800 Subject: [PATCH 33/35] Moderate warning label for cli.cli_command --- letsencrypt/cli.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ab04b906d..f1bf00058 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -46,7 +46,11 @@ logger = logging.getLogger(__name__) # For help strings, figure out how the user ran us. # When invoked from letsencrypt-auto, sys.argv[0] is something like: -# /home/user/.local/share/letsencrypt/bin/letsencrypt" +# "/home/user/.local/share/letsencrypt/bin/letsencrypt" +# Note that this won't work if the user set VENV_PATH or XDG_DATA_HOME before running +# letsencrypt-auto (and sudo stops us from seing if they did), so it should only be used +# for purposes where inability to detect letsencrypt-auto fails safely + fragment = os.path.join(".local", "share", "letsencrypt") cli_command = "letsencrypt-auto" if fragment in sys.argv[0] else "letsencrypt" From 58b50ba0083e809cf4da762172b4b6792e281145 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 22 Jan 2016 17:27:45 -0800 Subject: [PATCH 34/35] fix typo --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f1bf00058..1a1d09274 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -48,7 +48,7 @@ logger = logging.getLogger(__name__) # When invoked from letsencrypt-auto, sys.argv[0] is something like: # "/home/user/.local/share/letsencrypt/bin/letsencrypt" # Note that this won't work if the user set VENV_PATH or XDG_DATA_HOME before running -# letsencrypt-auto (and sudo stops us from seing if they did), so it should only be used +# letsencrypt-auto (and sudo stops us from seeing if they did), so it should only be used # for purposes where inability to detect letsencrypt-auto fails safely fragment = os.path.join(".local", "share", "letsencrypt") From b7c9ed1b996e4cb216bd37487e39efc10cc66be7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 25 Jan 2016 15:36:00 -0800 Subject: [PATCH 35/35] lintmonster --- letsencrypt/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index bc0fa6892..be7b8acda 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -137,7 +137,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods exc = None try: with mock.patch('letsencrypt.cli.sys.stderr'): - out = cli.main(self.standard_args + args[:]) # NOTE: parser can alter its args! + cli.main(self.standard_args + args[:]) # NOTE: parser can alter its args! except errors.MissingCommandlineFlag, exc: self.assertTrue(message in str(exc)) self.assertTrue(exc is not None)