Apply changes to cli since last merge from master (efc8d49806b14a31d88cfc0f1b6daca1dd373d8d)refactor-cli

11 files changed, 81 insertions, 100 deletions

diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index ff3061e01..126b07eec 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -13,7 +13,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* certbot._internal.cli is now a package split in submodules instead of a whole module.
 
 ### Fixed
 
diff --git a/certbot/certbot/_internal/cli/__init__.py b/certbot/certbot/_internal/cli/__init__.py
index 520c5f73d..3f799ee2a 100644
--- a/certbot/certbot/_internal/cli/__init__.py
+++ b/certbot/certbot/_internal/cli/__init__.py
@@ -5,20 +5,18 @@ import logging
 import logging.handlers
 import argparse
 import sys
-import certbot.plugins.selection as plugin_selection
-from certbot.plugins import disco as plugins_disco
+import certbot._internal.plugins.selection as plugin_selection
+from certbot._internal.plugins import disco as plugins_disco
 
-# pylint: disable=unused-import, no-name-in-module
-from acme.magic_typing import Any, Dict, Optional
-# pylint: enable=unused-import, no-name-in-module
+from acme.magic_typing import Optional
 
 import certbot
-from certbot import constants
+from certbot._internal import constants
 
 import certbot.plugins.enhancements as enhancements
 
 
-from certbot.cli.cli_constants import (
+from certbot._internal.cli.cli_constants import (
     LEAUTO,
     old_path_fragment,
     new_path_prefix,
@@ -32,7 +30,7 @@ from certbot.cli.cli_constants import (
     VAR_MODIFIERS
 )
 
-from certbot.cli.cli_utils import (
+from certbot._internal.cli.cli_utils import (
     _Default,
     read_file,
     flag_default,
@@ -52,16 +50,15 @@ from certbot.cli.cli_utils import (
 )
 
 # These imports depend on cli_constants and cli_utils.
-from certbot.cli.report_config_interaction import report_config_interaction
-from certbot.cli.warning import possible_deprecation_warning
-from certbot.cli.verb_help import VERB_HELP, VERB_HELP_MAP
-from certbot.cli.group_adder import _add_all_groups
-from certbot.cli.subparsers import _create_subparsers
-from certbot.cli.paths_parser import _paths_parser
-from certbot.cli.plugins_parsing import _plugins_parsing
+from certbot._internal.cli.report_config_interaction import report_config_interaction
+from certbot._internal.cli.verb_help import VERB_HELP, VERB_HELP_MAP
+from certbot._internal.cli.group_adder import _add_all_groups
+from certbot._internal.cli.subparsers import _create_subparsers
+from certbot._internal.cli.paths_parser import _paths_parser
+from certbot._internal.cli.plugins_parsing import _plugins_parsing
 
 # These imports depend on some or all of the submodules for cli.
-from certbot.cli.helpful import HelpfulArgumentParser
+from certbot._internal.cli.helpful import HelpfulArgumentParser
 
 
 logger = logging.getLogger(__name__)
@@ -71,7 +68,7 @@ logger = logging.getLogger(__name__)
 helpful_parser = None  # type: Optional[HelpfulArgumentParser]
 
 
-def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: disable=too-many-statements
+def prepare_and_parse_args(plugins, args, detect_defaults=False):
     """Returns parsed command line arguments.
 
     :param .PluginsRegistry plugins: available plugins
@@ -82,8 +79,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
 
     """
 
-    # pylint: disable=too-many-statements
-
     helpful = HelpfulArgumentParser(args, plugins, detect_defaults)
     _add_all_groups(helpful)
 
@@ -181,12 +176,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "certificates. Updates to the Subscriber Agreement will still "
              "affect you, and will be effective 14 days after posting an "
              "update to the web site.")
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    # delete following helpful.add
-    helpful.add(
-        "register", "--update-registration", action="store_true",
-        default=flag_default("update_registration"), dest="update_registration",
-        help=argparse.SUPPRESS)
     helpful.add(
         ["register", "update_account", "unregister", "automation"], "-m", "--email",
         default=flag_default("email"),
@@ -273,6 +262,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              " installing OS-level dependencies (default: Prompt to install "
              " OS-wide dependencies, but exit if the user says 'No')")
     helpful.add(
+        "automation", "--no-permissions-check", action="store_true",
+        default=flag_default("no_permissions_check"),
+        help="(certbot-auto only) skip the check on the file system"
+             " permissions of the certbot-auto script")
+    helpful.add(
         ["automation", "renew", "certonly", "run"],
         "-q", "--quiet", dest="quiet", action="store_true",
         default=flag_default("quiet"),
@@ -297,14 +291,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help=config_help("no_verify_ssl"),
         default=flag_default("no_verify_ssl"))
     helpful.add(
-        ["testing", "standalone", "apache", "nginx"], "--tls-sni-01-port", type=int,
-        default=flag_default("tls_sni_01_port"),
-        help=config_help("tls_sni_01_port"))
-    helpful.add(
-        ["testing", "standalone"], "--tls-sni-01-address",
-        default=flag_default("tls_sni_01_address"),
-        help=config_help("tls_sni_01_address"))
-    helpful.add(
         ["testing", "standalone", "manual"], "--http-01-port", type=int,
         dest="http01_port",
         default=flag_default("http01_port"), help=config_help("http01_port"))
@@ -313,6 +299,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         dest="http01_address",
         default=flag_default("http01_address"), help=config_help("http01_address"))
     helpful.add(
+        ["testing", "nginx"], "--https-port", type=int,
+        default=flag_default("https_port"),
+        help=config_help("https_port"))
+    helpful.add(
         "testing", "--break-my-certs", action="store_true",
         default=flag_default("break_my_certs"),
         help="Be willing to replace or renew valid certificates with invalid "
@@ -372,7 +362,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         action=_PrefChallAction, default=flag_default("pref_challs"),
         help='A sorted, comma delimited list of the preferred challenge to '
              'use during authorization with the most preferred challenge '
-             'listed first (Eg, "dns" or "tls-sni-01,http,dns"). '
+             'listed first (Eg, "dns" or "http,dns"). '
              'Not all plugins support all challenges. See '
              'https://certbot.eff.org/docs/using.html#plugins for details. '
              'ACME Challenges are versioned, but if you pick "http" rather '
@@ -440,9 +430,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         default=flag_default("autorenew"), dest="autorenew",
         help="Disable auto renewal of certificates.")
 
-    helpful.add_deprecated_argument("--agree-dev-preview", 0)
-    helpful.add_deprecated_argument("--dialog", 0)
-
     # Populate the command line parameters for new style enhancements
     enhancements.populate_cli(helpful.add)
 
diff --git a/certbot/certbot/_internal/cli/cli_constants.py b/certbot/certbot/_internal/cli/cli_constants.py
index 0b60dea4a..ed5908390 100644
--- a/certbot/certbot/_internal/cli/cli_constants.py
+++ b/certbot/certbot/_internal/cli/cli_constants.py
@@ -59,12 +59,13 @@ obtain, install, and renew certificates:
 
 manage certificates:
     certificates    Display information about certificates you have from Certbot
-    revoke          Revoke a certificate (supply --cert-path or --cert-name)
-    delete          Delete a certificate
+    revoke          Revoke a certificate (supply --cert-name or --cert-path)
+    delete          Delete a certificate (supply --cert-name)
 
-manage your account with Let's Encrypt:
-    register        Create a Let's Encrypt ACME account
-    update_account  Update a Let's Encrypt ACME account
+manage your account:
+    register        Create an ACME account
+    unregister      Deactivate an ACME account
+    update_account  Update an ACME account
   --agree-tos       Agree to the ACME server's Subscriber Agreement
    -m EMAIL         Email address for important account notifications
 """
@@ -80,7 +81,7 @@ More detailed help:
    all, automation, commands, paths, security, testing, or any of the
    subcommands or plugins (certonly, renew, install, register, nginx,
    apache, standalone, webroot, etc.)
-
+  -h all                print a detailed help page including all topics
   --version             print the version number
 """
 
diff --git a/certbot/certbot/_internal/cli/cli_utils.py b/certbot/certbot/_internal/cli/cli_utils.py
index eaf55bcbc..f9978af24 100644
--- a/certbot/certbot/_internal/cli/cli_utils.py
+++ b/certbot/certbot/_internal/cli/cli_utils.py
@@ -4,10 +4,10 @@ import copy
 import os
 
 from acme import challenges
-from certbot import constants
 from certbot import interfaces
 from certbot import util
 from certbot import errors
+from certbot._internal import constants
 
 # pylint: disable=unused-import, no-name-in-module
 import zope.interface.interface
@@ -63,12 +63,10 @@ def flag_default(name):
 
 def config_help(name, hidden=False):
     """Extract the help message for an `.IConfig` attribute."""
-    # pylint: disable=no-member
     if hidden:
         return argparse.SUPPRESS
-    else:
-        field = interfaces.IConfig.__getitem__(name) # type: zope.interface.interface.Attribute
-        return field.__doc__
+    field = interfaces.IConfig.__getitem__(name)  # type: zope.interface.interface.Attribute
+    return field.__doc__
 
 
 class HelpfulArgumentGroup(object):
@@ -175,9 +173,10 @@ def parse_preferred_challenges(pref_challs):
     :raises errors.Error: if pref_challs is invalid
 
     """
-    aliases = {"dns": "dns-01", "http": "http-01", "tls-sni": "tls-sni-01"}
+    aliases = {"dns": "dns-01", "http": "http-01"}
     challs = [c.strip() for c in pref_challs]
     challs = [aliases.get(c, c) for c in challs]
+
     unrecognized = ", ".join(name for name in challs
                              if name not in challenges.Challenge.TYPES)
     if unrecognized:
diff --git a/certbot/certbot/_internal/cli/group_adder.py b/certbot/certbot/_internal/cli/group_adder.py
index 9a1fc503e..f22fbc496 100644
--- a/certbot/certbot/_internal/cli/group_adder.py
+++ b/certbot/certbot/_internal/cli/group_adder.py
@@ -1,6 +1,6 @@
 """This module contains a function to add the groups of arguments for the help
 display"""
-from certbot.cli import VERB_HELP
+from certbot._internal.cli import VERB_HELP
 
 
 def _add_all_groups(helpful):
diff --git a/certbot/certbot/_internal/cli/helpful.py b/certbot/certbot/_internal/cli/helpful.py
index 7249bee53..097f4ffc1 100644
--- a/certbot/certbot/_internal/cli/helpful.py
+++ b/certbot/certbot/_internal/cli/helpful.py
@@ -16,16 +16,16 @@ from zope.interface import interfaces as zope_interfaces
 from acme.magic_typing import Any, Dict, Optional
 # pylint: enable=unused-import, no-name-in-module
 
-from certbot import constants
 from certbot import crypto_util
 from certbot import errors
-from certbot import hooks
 from certbot import interfaces
 from certbot import util
+from certbot._internal import constants
+from certbot._internal import hooks
 
 from certbot.display import util as display_util
 
-from certbot.cli import (
+from certbot._internal.cli import (
     SHORT_USAGE,
     CustomHelpFormatter,
     flag_default,
@@ -34,7 +34,6 @@ from certbot.cli import (
     COMMAND_OVERVIEW,
     HELP_AND_VERSION_USAGE,
     _Default,
-    possible_deprecation_warning,
     add_domains,
     EXIT_ACTIONS,
     ZERO_ARG_ACTIONS,
@@ -51,14 +50,11 @@ class HelpfulArgumentParser(object):
     'certbot --help security' for security options.
 
     """
-
-
     def __init__(self, args, plugins, detect_defaults=False):
-        from certbot import main
+        from certbot._internal import main
         self.VERBS = {
             "auth": main.certonly,
             "certonly": main.certonly,
-            "config_changes": main.config_changes,
             "run": main.run,
             "install": main.install,
             "plugins": main.plugins_cmd,
@@ -124,12 +120,13 @@ class HelpfulArgumentParser(object):
                 " and ".join(flag_default("config_files"))))
 
         # This is the only way to turn off overly verbose config flag documentation
-        self.parser._add_config_file_help = False  # pylint: disable=protected-access
+        self.parser._add_config_file_help = False
 
     # Help that are synonyms for --help subcommands
     COMMANDS_TOPICS = ["command", "commands", "subcommand", "subcommands", "verbs"]
+
     def _list_subcommands(self):
-        longest = max(len(v) for v in VERB_HELP_MAP.keys())
+        longest = max(len(v) for v in VERB_HELP_MAP)
 
         text = "The full list of available SUBCOMMANDS is:\n\n"
         for verb, props in sorted(VERB_HELP):
@@ -157,7 +154,7 @@ class HelpfulArgumentParser(object):
             apache_doc = "(the certbot apache plugin is not installed)"
 
         usage = SHORT_USAGE
-        if help_arg == True:
+        if help_arg is True:
             self.notify(usage + COMMAND_OVERVIEW % (apache_doc, nginx_doc) + HELP_AND_VERSION_USAGE)
             sys.exit(0)
         elif help_arg in self.COMMANDS_TOPICS:
@@ -235,20 +232,25 @@ class HelpfulArgumentParser(object):
             raise errors.Error(
                 "Parameters --hsts and --auto-hsts cannot be used simultaneously.")
 
-        possible_deprecation_warning(parsed_args)
-
         return parsed_args
 
     def set_test_server(self, parsed_args):
         """We have --staging/--dry-run; perform sanity check and set config.server"""
 
-        if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
-            conflicts = ["--staging"] if parsed_args.staging else []
-            conflicts += ["--dry-run"] if parsed_args.dry_run else []
-            raise errors.Error("--server value conflicts with {0}".format(
-                " and ".join(conflicts)))
+        # Flag combinations should produce these results:
+        #                             | --staging      | --dry-run   |
+        # ------------------------------------------------------------
+        # | --server acme-v02         | Use staging    | Use staging |
+        # | --server acme-staging-v02 | Use staging    | Use staging |
+        # | --server <other>          | Conflict error | Use <other> |
 
-        parsed_args.server = constants.STAGING_URI
+        default_servers = (flag_default("server"), constants.STAGING_URI)
+
+        if parsed_args.staging and parsed_args.server not in default_servers:
+            raise errors.Error("--server value conflicts with --staging")
+
+        if parsed_args.server in default_servers:
+            parsed_args.server = constants.STAGING_URI
 
         if parsed_args.dry_run:
             if self.verb not in ["certonly", "renew"]:
@@ -287,7 +289,7 @@ class HelpfulArgumentParser(object):
 
         parsed_args.actual_csr = (csr, typ)
 
-        csr_domains = set([d.lower() for d in domains])
+        csr_domains = {d.lower() for d in domains}
         config_domains = set(parsed_args.domains)
         if csr_domains != config_domains:
             raise errors.ConfigurationError(
@@ -344,9 +346,10 @@ class HelpfulArgumentParser(object):
         """Add a new command line argument.
 
         :param topics: str or [str] help topic(s) this should be listed under,
-                       or None for "always documented". The first entry
-                       determines where the flag lives in the "--help all"
-                       output (None -> "optional arguments").
+                       or None for options that don't fit under a specific
+                       topic which will only be shown in "--help all" output.
+                       The first entry determines where the flag lives in the
+                       "--help all" output (None -> "optional arguments").
         :param list *args: the names of this argument flag
         :param dict **kwargs: various argparse settings for this argument
 
@@ -459,9 +462,7 @@ class HelpfulArgumentParser(object):
             chosen_topic = "run"
         if chosen_topic == "all":
             # Addition of condition closes #6209 (removal of duplicate route53 option).
-            return dict([(t, True) if t != 'certbot-route53:auth' else (t, False)
-                         for t in self.help_topics])
+            return {t: t != 'certbot-route53:auth' for t in self.help_topics}
         elif not chosen_topic:
-            return dict([(t, False) for t in self.help_topics])
-        else:
-            return dict([(t, t == chosen_topic) for t in self.help_topics])
+            return {t: False for t in self.help_topics}
+        return {t: t == chosen_topic for t in self.help_topics}
diff --git a/certbot/certbot/_internal/cli/paths_parser.py b/certbot/certbot/_internal/cli/paths_parser.py
index 970411094..175ff7168 100644
--- a/certbot/certbot/_internal/cli/paths_parser.py
+++ b/certbot/certbot/_internal/cli/paths_parser.py
@@ -2,7 +2,7 @@
 paths for certificates"""
 import os
 
-from certbot.cli import (
+from certbot._internal.cli import (
     read_file,
     flag_default,
     config_help
diff --git a/certbot/certbot/_internal/cli/plugins_parsing.py b/certbot/certbot/_internal/cli/plugins_parsing.py
index 404428e07..9e11ad3ab 100644
--- a/certbot/certbot/_internal/cli/plugins_parsing.py
+++ b/certbot/certbot/_internal/cli/plugins_parsing.py
@@ -1,5 +1,5 @@
 """This is a module that handles parsing of plugins for the argument parser"""
-from certbot.cli import flag_default
+from certbot._internal.cli import flag_default
 
 
 def _plugins_parsing(helpful, plugins):
@@ -19,10 +19,10 @@ def _plugins_parsing(helpful, plugins):
                 help="Authenticator plugin name.")
     helpful.add("plugins", "-i", "--installer", default=flag_default("installer"),
                 help="Installer plugin name (also used to find domains).")
-    helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
+    helpful.add(["plugins", "certonly", "run", "install"],
                 "--apache", action="store_true", default=flag_default("apache"),
                 help="Obtain and install certificates using Apache")
-    helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
+    helpful.add(["plugins", "certonly", "run", "install"],
                 "--nginx", action="store_true", default=flag_default("nginx"),
                 help="Obtain and install certificates using Nginx")
     helpful.add(["plugins", "certonly"], "--standalone", action="store_true",
@@ -52,12 +52,12 @@ def _plugins_parsing(helpful, plugins):
                       "using DNSimple for DNS)."))
     helpful.add(["plugins", "certonly"], "--dns-dnsmadeeasy", action="store_true",
                 default=flag_default("dns_dnsmadeeasy"),
-                help=("Obtain certificates using a DNS TXT record (if you are"
+                help=("Obtain certificates using a DNS TXT record (if you are "
                       "using DNS Made Easy for DNS)."))
     helpful.add(["plugins", "certonly"], "--dns-gehirn", action="store_true",
                 default=flag_default("dns_gehirn"),
                 help=("Obtain certificates using a DNS TXT record "
-                     "(if you are using Gehirn Infrastracture Service for DNS)."))
+                      "(if you are using Gehirn Infrastructure Service for DNS)."))
     helpful.add(["plugins", "certonly"], "--dns-google", action="store_true",
                 default=flag_default("dns_google"),
                 help=("Obtain certificates using a DNS TXT record (if you are "
diff --git a/certbot/certbot/_internal/cli/report_config_interaction.py b/certbot/certbot/_internal/cli/report_config_interaction.py
index c664b667b..39266e776 100644
--- a/certbot/certbot/_internal/cli/report_config_interaction.py
+++ b/certbot/certbot/_internal/cli/report_config_interaction.py
@@ -1,7 +1,8 @@
 """This is a module that reports config option interaction that should be
-checled by set_by_cli"""
+checked by set_by_cli"""
 import six
-from certbot.cli import VAR_MODIFIERS
+
+from certbot._internal.cli import VAR_MODIFIERS
 
 
 def report_config_interaction(modified, modifiers):
diff --git a/certbot/certbot/_internal/cli/subparsers.py b/certbot/certbot/_internal/cli/subparsers.py
index da2649ca9..13f8705ce 100644
--- a/certbot/certbot/_internal/cli/subparsers.py
+++ b/certbot/certbot/_internal/cli/subparsers.py
@@ -1,8 +1,8 @@
 """This module creates subparsers for the argument parser"""
-from certbot import constants
 from certbot import interfaces
+from certbot._internal import constants
 
-from certbot.cli import (
+from certbot._internal.cli import (
     flag_default,
     read_file,
     CaseInsensitiveList,
@@ -12,10 +12,7 @@ from certbot.cli import (
 
 
 def _create_subparsers(helpful):
-    helpful.add("config_changes", "--num", type=int, default=flag_default("num"),
-                help="How many past revisions you want to be displayed")
-
-    from certbot.client import sample_user_agent # avoid import loops
+    from certbot._internal.client import sample_user_agent  # avoid import loops
     helpful.add(
         None, "--user-agent", default=flag_default("user_agent"),
         help='Set a custom user agent string for the client. User agent strings allow '
diff --git a/certbot/certbot/_internal/cli/verb_help.py b/certbot/certbot/_internal/cli/verb_help.py
index cc07bb541..425b80282 100644
--- a/certbot/certbot/_internal/cli/verb_help.py
+++ b/certbot/certbot/_internal/cli/verb_help.py
@@ -1,7 +1,7 @@
 """This module contain help information for verbs supported by certbot"""
 import os
 
-from certbot.cli import (
+from certbot._internal.cli import (
     SHORT_USAGE,
     flag_default
 )
@@ -77,11 +77,6 @@ VERB_HELP = [
         "usage": "\n\n  certbot install --cert-path /path/to/fullchain.pem "
         " --key-path /path/to/private-key [options]\n\n"
     }),
-    ("config_changes", {
-        "short": "Show changes that Certbot has made to server configurations",
-        "opts": "Options for controlling which changes are displayed",
-        "usage": "\n\n  certbot config_changes --num NUM [options]\n\n"
-    }),
     ("rollback", {
         "short": "Roll back server conf changes made during certificate installation",
         "opts": "Options for rolling back server configuration changes",
@@ -89,7 +84,7 @@ VERB_HELP = [
     }),
     ("plugins", {
         "short": "List plugins that are installed and available on your system",
-        "opts": 'Options for for the "plugins" subcommand',
+        "opts": 'Options for the "plugins" subcommand',
         "usage": "\n\n  certbot plugins [options]\n\n"
     }),
     ("update_symlinks", {