From 9bd5b3dda2ca606614892cb178feeab8efe9a1f9 Mon Sep 17 00:00:00 2001
From: dschlessman <36820111+dschlessman@users.noreply.github.com>
Date: Tue, 15 May 2018 13:40:32 -0400
Subject: [PATCH] Issue 5951/check untyped defs apache (#5989)

* resolved mypy untyped defs in parser.py

* resolved mypy untyped defs in obj.py

* removed unused imports

* resolved mypy untyped defs in http_01.py

* resolved mypy untyped defs in tls_sni_01.py

* resolved mypy untyped defs in configurator.py

* address mypy too-many-arguments error in override_centos.py

* resolved mypy untyped defs in http_01_test.py

* removed unused 'conf' argument that was causing mypy method assignment error

* address mypy error where same variable reassigned to different type

* address pylint and coverage issues

* one character space change for formatting

* fix required acme version for certbot-apache
---
 certbot-apache/certbot_apache/configurator.py | 24 ++++++++++---------
 certbot-apache/certbot_apache/http_01.py      |  5 ++--
 certbot-apache/certbot_apache/obj.py          |  5 ++--
 .../certbot_apache/override_centos.py         |  4 ++--
 certbot-apache/certbot_apache/parser.py       | 11 +++++----
 .../certbot_apache/tests/configurator_test.py |  9 +++----
 .../certbot_apache/tests/http_01_test.py      |  4 ++--
 certbot-apache/certbot_apache/tests/util.py   |  5 ----
 certbot-apache/certbot_apache/tls_sni_01.py   |  3 ++-
 certbot-apache/local-oldest-requirements.txt  |  2 +-
 certbot-apache/setup.py                       |  2 +-
 mypy.ini                                      |  3 +++
 12 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 03ba05bb0..bb82a9d3f 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -13,11 +13,13 @@ import zope.component
 import zope.interface
 
 from acme import challenges
+from acme.magic_typing import DefaultDict, Dict, List, Set  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import errors
 from certbot import interfaces
 from certbot import util
 
+from certbot.achallenges import KeyAuthorizationAnnotatedChallenge  # pylint: disable=unused-import
 from certbot.plugins import common
 from certbot.plugins.util import path_surgery
 
@@ -150,14 +152,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         super(ApacheConfigurator, self).__init__(*args, **kwargs)
 
         # Add name_server association dict
-        self.assoc = dict()
+        self.assoc = dict()  # type: Dict[str, obj.VirtualHost]
         # Outstanding challenges
-        self._chall_out = set()
+        self._chall_out = set()  # type: Set[KeyAuthorizationAnnotatedChallenge]
         # List of vhosts configured per wildcard domain on this run.
         # used by deploy_cert() and enhance()
-        self._wildcard_vhosts = dict()
+        self._wildcard_vhosts = dict()  # type: Dict[str, List[obj.VirtualHost]]
         # Maps enhancements to vhosts we've enabled the enhancement for
-        self._enhanced_vhosts = defaultdict(set)
+        self._enhanced_vhosts = defaultdict(set)  # type: DefaultDict[str, Set[obj.VirtualHost]]
 
         # These will be set in the prepare function
         self.parser = None
@@ -659,7 +661,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :rtype: set
 
         """
-        all_names = set()
+        all_names = set()  # type: Set[str]
 
         vhost_macro = []
 
@@ -800,8 +802,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
         # Search base config, and all included paths for VirtualHosts
-        file_paths = {}
-        internal_paths = defaultdict(set)
+        file_paths = {}  # type: Dict[str, str]
+        internal_paths = defaultdict(set)  # type: DefaultDict[str, Set[str]]
         vhs = []
         # Make a list of parser paths because the parser_paths
         # dictionary may be modified during the loop.
@@ -1239,7 +1241,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             if not self.parser.parsed_in_current(ssl_fp):
                 self.parser.parse_file(ssl_fp)
         except IOError:
-            logger.fatal("Error writing/reading to file in make_vhost_ssl")
+            logger.critical("Error writing/reading to file in make_vhost_ssl", exc_info=True)
             raise errors.PluginError("Unable to write/read in make_vhost_ssl")
 
         if sift:
@@ -1327,7 +1329,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         try:
             span_val = self.aug.span(vhost.path)
         except ValueError:
-            logger.fatal("Error while reading the VirtualHost %s from "
+            logger.critical("Error while reading the VirtualHost %s from "
                          "file %s", vhost.name, vhost.filep, exc_info=True)
             raise errors.PluginError("Unable to read VirtualHost from file")
         span_filep = span_val[0]
@@ -1770,7 +1772,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # There can be other RewriteRule directive lines in vhost config.
         # rewrite_args_dict keys are directive ids and the corresponding value
         # for each is a list of arguments to that directive.
-        rewrite_args_dict = defaultdict(list)
+        rewrite_args_dict = defaultdict(list)  # type: DefaultDict[str, List[str]]
         pat = r'(.*directive\[\d+\]).*'
         for match in rewrite_path:
             m = re.match(pat, match)
@@ -1864,7 +1866,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if ssl_vhost.aliases:
             serveralias = "ServerAlias " + " ".join(ssl_vhost.aliases)
 
-        rewrite_rule_args = []
+        rewrite_rule_args = []  # type: List[str]
         if self.get_version() >= (2, 3, 9):
             rewrite_rule_args = constants.REWRITE_HTTPS_ARGS_WITH_END
         else:
diff --git a/certbot-apache/certbot_apache/http_01.py b/certbot-apache/certbot_apache/http_01.py
index cce93a646..37545e9cc 100644
--- a/certbot-apache/certbot_apache/http_01.py
+++ b/certbot-apache/certbot_apache/http_01.py
@@ -2,9 +2,10 @@
 import logging
 import os
 
+from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
-
 from certbot.plugins import common
+from certbot_apache.obj import VirtualHost  # pylint: disable=unused-import
 
 logger = logging.getLogger(__name__)
 
@@ -51,7 +52,7 @@ class ApacheHttp01(common.TLSSNI01):
         self.challenge_dir = os.path.join(
             self.configurator.config.work_dir,
             "http_challenges")
-        self.moded_vhosts = set()
+        self.moded_vhosts = set()  # type: Set[VirtualHost]
 
     def perform(self):
         """Perform all HTTP-01 challenges."""
diff --git a/certbot-apache/certbot_apache/obj.py b/certbot-apache/certbot_apache/obj.py
index fcf3bfe08..290979f27 100644
--- a/certbot-apache/certbot_apache/obj.py
+++ b/certbot-apache/certbot_apache/obj.py
@@ -1,6 +1,7 @@
 """Module contains classes used by the Apache Configurator."""
 import re
 
+from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module
 from certbot.plugins import common
 
 
@@ -140,7 +141,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
 
     def get_names(self):
         """Return a set of all names."""
-        all_names = set()
+        all_names = set()  # type: Set[str]
         all_names.update(self.aliases)
         # Strip out any scheme:// and <port> field from servername
         if self.name is not None:
@@ -251,7 +252,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
 
         # already_found acts to keep everything very conservative.
         # Don't allow multiple ip:ports in same set.
-        already_found = set()
+        already_found = set()  # type: Set[str]
 
         for addr in vhost.addrs:
             for local_addr in self.addrs:
diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py
index 6e75e361d..0b6b12b96 100644
--- a/certbot-apache/certbot_apache/override_centos.py
+++ b/certbot-apache/certbot_apache/override_centos.py
@@ -47,10 +47,10 @@ class CentOSParser(parser.ApacheParser):
         self.sysconfig_filep = "/etc/sysconfig/httpd"
         super(CentOSParser, self).__init__(*args, **kwargs)
 
-    def update_runtime_variables(self, *args, **kwargs):
+    def update_runtime_variables(self):
         """ Override for update_runtime_variables for custom parsing """
         # Opportunistic, works if SELinux not enforced
-        super(CentOSParser, self).update_runtime_variables(*args, **kwargs)
+        super(CentOSParser, self).update_runtime_variables()
         self.parse_sysconfig_var()
 
     def parse_sysconfig_var(self):
diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py
index d7da1e55e..43878eda2 100644
--- a/certbot-apache/certbot_apache/parser.py
+++ b/certbot-apache/certbot_apache/parser.py
@@ -9,6 +9,7 @@ import sys
 
 import six
 
+from acme.magic_typing import Dict, List, Set  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 
 logger = logging.getLogger(__name__)
@@ -38,9 +39,9 @@ class ApacheParser(object):
         # issues with aug.load() after adding new files / defines to parse tree
         self.configurator = configurator
 
-        self.modules = set()
-        self.parser_paths = {}
-        self.variables = {}
+        self.modules = set()  # type: Set[str]
+        self.parser_paths = {}  # type: Dict[str, List[str]]
+        self.variables = {}  # type: Dict[str, str]
 
         self.aug = aug
         # Find configuration root and make sure augeas can parse it.
@@ -119,7 +120,7 @@ class ApacheParser(object):
             the iteration issue.  Else... parse and enable mods at same time.
 
         """
-        mods = set()
+        mods = set()  # type: Set[str]
         matches = self.find_dir("LoadModule")
         iterator = iter(matches)
         # Make sure prev_size != cur_size for do: while: iteration
@@ -408,7 +409,7 @@ class ApacheParser(object):
         else:
             arg_suffix = "/*[self::arg=~regexp('%s')]" % case_i(arg)
 
-        ordered_matches = []
+        ordered_matches = []  # type: List[str]
 
         # TODO: Wildcards should be included in alphabetical order
         # https://httpd.apache.org/docs/2.4/mod/core.html#include
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index e33e16843..23c1ee82b 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -353,14 +353,11 @@ class MultipleVhostsTest(util.ApacheTest):
 
         self.config.parser.find_dir = mock_find_dir
         mock_add.reset_mock()
-
         self.config._add_dummy_ssl_directives(self.vh_truth[0])  # pylint: disable=protected-access
-        tried_to_add = []
         for a in mock_add.call_args_list:
-            tried_to_add.append(a[0][1] == "Include" and
-                                a[0][2] == self.config.mod_ssl_conf)
-        # Include shouldn't be added, as patched find_dir "finds" existing one
-        self.assertFalse(any(tried_to_add))
+            if a[0][1] == "Include" and a[0][2] == self.config.mod_ssl_conf:
+                self.fail("Include shouldn't be added, as patched find_dir 'finds' existing one") \
+                    # pragma: no cover
 
     def test_deploy_cert(self):
         self.config.parser.modules.add("ssl_module")
diff --git a/certbot-apache/certbot_apache/tests/http_01_test.py b/certbot-apache/certbot_apache/tests/http_01_test.py
index dc1ca34d6..98bf412ae 100644
--- a/certbot-apache/certbot_apache/tests/http_01_test.py
+++ b/certbot-apache/certbot_apache/tests/http_01_test.py
@@ -4,12 +4,12 @@ import os
 import unittest
 
 from acme import challenges
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import achallenges
 from certbot import errors
 
 from certbot.tests import acme_util
-
 from certbot_apache.tests import util
 
 
@@ -23,7 +23,7 @@ class ApacheHttp01Test(util.ApacheTest):
         super(ApacheHttp01Test, self).setUp(*args, **kwargs)
 
         self.account_key = self.rsa512jwk
-        self.achalls = []
+        self.achalls = []  # type: List[achallenges.KeyAuthorizationAnnotatedChallenge]
         vh_truth = util.get_vh_truth(
             self.temp_dir, "debian_apache_2_4/multiple_vhosts")
         # Takes the vhosts for encryption-example.demo, certbot.demo, and
diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py
index 1daaa00c5..6d3cfa109 100644
--- a/certbot-apache/certbot_apache/tests/util.py
+++ b/certbot-apache/certbot_apache/tests/util.py
@@ -87,7 +87,6 @@ class ParserTest(ApacheTest):
 def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-locals
         config_path, vhost_path,
         config_dir, work_dir, version=(2, 4, 7),
-        conf=None,
         os_info="generic",
         conf_vhost_path=None):
     """Create an Apache Configurator with the specified options.
@@ -133,10 +132,6 @@ def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-loc
                         config_class = configurator.ApacheConfigurator
                     config = config_class(config=mock_le_config, name="apache",
                         version=version)
-                    # This allows testing scripts to set it a bit more
-                    # quickly
-                    if conf is not None:
-                        config.conf = conf  # pragma: no cover
 
                     config.prepare()
     return config
diff --git a/certbot-apache/certbot_apache/tls_sni_01.py b/certbot-apache/certbot_apache/tls_sni_01.py
index 549feb17d..65230cdcb 100644
--- a/certbot-apache/certbot_apache/tls_sni_01.py
+++ b/certbot-apache/certbot_apache/tls_sni_01.py
@@ -3,6 +3,7 @@
 import os
 import logging
 
+from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
 from certbot.plugins import common
 from certbot.errors import PluginError, MissingCommandlineFlag
 
@@ -93,7 +94,7 @@ class ApacheTlsSni01(common.TLSSNI01):
         :rtype: set
 
         """
-        addrs = set()
+        addrs = set()  # type: Set[obj.Addr]
         config_text = "<IfModule mod_ssl.c>\n"
 
         for achall in self.achalls:
diff --git a/certbot-apache/local-oldest-requirements.txt b/certbot-apache/local-oldest-requirements.txt
index 8368d266e..724b61d3f 100644
--- a/certbot-apache/local-oldest-requirements.txt
+++ b/certbot-apache/local-oldest-requirements.txt
@@ -1,2 +1,2 @@
-acme[dev]==0.21.1
+-e acme[dev]
 certbot[dev]==0.21.1
diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py
index f0c20da7c..0e4304300 100644
--- a/certbot-apache/setup.py
+++ b/certbot-apache/setup.py
@@ -9,7 +9,7 @@ version = '0.25.0.dev0'
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.21.1',
+    'acme>0.24.0',
     'certbot>=0.21.1',
     'mock',
     'python-augeas',
diff --git a/mypy.ini b/mypy.ini
index 9709fa5ed..7e08a1f99 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -5,6 +5,9 @@ ignore_missing_imports = True
 [mypy-acme.*]
 check_untyped_defs = True
 
+[mypy-certbot_apache.*]
+check_untyped_defs = True
+
 [mypy-certbot_dns_dnsimple.*]
 check_untyped_defs = True