standalone 2.0: lint, docs, cleanup.

acme/acme/crypto_util.py

@@ -27,8 +27,14 @@ _DEFAULT_DVSNI_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD
 
 
 class SSLSocket(object):  # pylint: disable=too-few-public-methods
-    """SSL wrapper for sockets."""
+    """SSL wrapper for sockets.
 
+    :ivar socket sock: Original wrapped socket.
+    :ivar dict certs: Mapping from domain names (`bytes`) to
+        `OpenSSL.crypto.X509`.
+    :ivar method: See `OpenSSL.SSL.Context` for allowed values.
+
+    """
     def __init__(self, sock, certs, method=_DEFAULT_DVSNI_SSL_METHOD):
         self.sock = sock
         self.certs = certs

letsencrypt/plugins/standalone.py

@@ -6,8 +6,6 @@ import random
 import socket
 import threading
 
-from six.moves import BaseHTTPServer  # pylint: disable=import-error
-
 import OpenSSL
 import zope.interface
 
@@ -26,17 +24,39 @@ logger = logging.getLogger(__name__)
 
 
 class ServerManager(object):
-    """Standalone servers manager."""
+    """Standalone servers manager.
 
+    Manager for `ACMEServer` and `ACMETLSServer` instances.
+
+    `certs` and `simple_http_resources` correspond to
+    `acme.crypto_util.SSLSocket.certs` and
+    `acme.crypto_util.SSLSocket.simple_http_resources` respectively. All
+    created servers share the same certificates and resources, so if
+    you're running both TLS and non-TLS instances, SimpleHTTP handlers
+    will serve the same URLs!
+
+    """
     def __init__(self, certs, simple_http_resources):
-        self.servers = {}
+        self._servers = {}
         self.certs = certs
         self.simple_http_resources = simple_http_resources
 
     def run(self, port, tls):
-        """Run ACME server on specified ``port``."""
+        """Run ACME server on specified ``port``.
-        if port in self.servers:
+
-            return self.servers[port]
+        This method is idempotent, i.e. all calls with the same pair of
+        ``(port, tls)`` will reuse the same server.
+
+        :param int port: Port to run the server on.
+        :param bool tls: TLS or non-TLS?
+
+        :returns: Server instance (`ACMEServerMixin`) and the
+            corresponding (already started) thread (`threading.Thread`).
+        :rtype: tuple
+
+        """
+        if port in self._servers:
+            return self._servers[port]
 
         logger.debug("Starting new server at %s (tls=%s)", port, tls)
         handler = acme_standalone.ACMERequestHandler.partial_init(
@@ -54,23 +74,38 @@ class ServerManager(object):
             raise errors.StandaloneBindError(error, port)
 
         # if port == 0, then random free port on OS is taken
-        real_port = server.socket.getsockname()
+        # pylint: disable=no-member
+        host, real_port = server.socket.getsockname()
 
         thread = threading.Thread(target=server.serve_forever2)
+        logger.debug("Starting server at %s:%d", host, real_port)
         thread.start()
-        self.servers[real_port] = (server, thread)
+
-        return self.servers[real_port]
+        self._servers[real_port] = (server, thread)
+        return self._servers[real_port]
 
     def stop(self, port):
-        """Stop ACME server running on the specified ``port``."""
+        """Stop ACME server running on the specified ``port``.
-        server, thread = self.servers[port]
+
+        :param int port:
+
+        """
+        server, thread = self._servers[port]
         server.shutdown2()
         thread.join()
-        del self.servers[port]
+        del self._servers[port]
 
-    def items(self):
+    def running(self):
-        """Return a list of all port, server tuples."""
+        """Return all running instances.
-        return self.servers.items()
+
+        Once the server is stopped using `stop`, it will not be
+        returned.
+
+        :returns: ``(port, (server, thread))``
+        :rtype: tuple
+
+        """
+        return self._servers.items()
 
 
 class Authenticator(common.Plugin):
@@ -98,7 +133,6 @@ class Authenticator(common.Plugin):
         self.simple_http_cert = acme_crypto_util.gen_ss_cert(
             self.key, domains=["temp server"])
 
-        self.responses = {}
         self.served = collections.defaultdict(set)
 
         # Stuff below is shared across threads (i.e. servers read
@@ -117,7 +151,8 @@ class Authenticator(common.Plugin):
         if any(util.already_listening(port) for port in
                (self.config.dvsni_port, self.config.simple_http_port)):
             raise errors.MisconfigurationError(
-                "One of the (possibly) required ports is already taken.")
+                "At least one of the (possibly) required ports is "
+                "already taken.")
 
     # TODO: add --chall-pref flag
     def get_chall_pref(self, domain):
@@ -167,7 +202,6 @@ class Authenticator(common.Plugin):
                 response, cert, _ = achall.gen_cert_and_response(self.key)
                 domain = response.z_domain
             self.certs[domain] = (self.key, cert)
-            self.responses[achall] = response
             self.served[server].add(achall)
             responses.append(response)
 
@@ -179,6 +213,6 @@ class Authenticator(common.Plugin):
             for achall in achalls:
                 if achall in server_achalls:
                     server_achalls.remove(achall)
-        for port, (server, _) in self.servers.items():
+        for port, (server, _) in self.servers.running():
             if not self.served[server]:
                 self.servers.stop(port)

letsencrypt/plugins/standalone_test.py

@@ -26,19 +26,19 @@ class ServerManagerTest(unittest.TestCase):
             self.mgr.simple_http_resources is self.simple_http_resources)
 
     def test_run_stop_non_tls(self):
-        server, thread = self.mgr.run(port=0, tls=False)
+        server, _ = self.mgr.run(port=0, tls=False)
-        self.mgr.stop(port=server.socket.getsockname())
+        self.mgr.stop(port=server.socket.getsockname()[1])
 
     def test_run_stop_tls(self):
-        server, thread = self.mgr.run(port=0, tls=True)
+        server, _ = self.mgr.run(port=0, tls=True)
-        self.mgr.stop(port=server.socket.getsockname())
+        self.mgr.stop(port=server.socket.getsockname()[1])
 
     def test_run_idempotent(self):
         server, thread = self.mgr.run(port=0, tls=False)
-        port = server.socket.getsockname()
+        port = server.socket.getsockname()[1]
         server2, thread2 = self.mgr.run(port=port, tls=False)
         self.assertTrue(server is server2)
-        self.assertTrue(thread2 is thread2)
+        self.assertTrue(thread is thread2)
         self.mgr.stop(port)
 
     def test_run_bind_error(self):
@@ -48,12 +48,12 @@ class ServerManagerTest(unittest.TestCase):
         self.assertRaises(
             errors.StandaloneBindError, self.mgr.run, port, tls=False)
 
-    def test_items(self):
+    def test_running(self):
         server, thread = self.mgr.run(port=0, tls=True)
-        port = server.socket.getsockname()
+        port = server.socket.getsockname()[1]
-        self.assertEqual(port, self.mgr.items()[0][0])
+        self.assertEqual(port, self.mgr.running()[0][0])
-        self.assertTrue(self.mgr.items()[0][1][0] is server)
+        self.assertTrue(self.mgr.running()[0][1][0] is server)
-        self.assertTrue(self.mgr.items()[0][1][1] is thread)
+        self.assertTrue(self.mgr.running()[0][1][1] is thread)
         self.mgr.stop(port=port)
 
 
@@ -79,7 +79,7 @@ class AuthenticatorTest(unittest.TestCase):
                          set([challenges.SimpleHTTP, challenges.DVSNI]))
 
     @mock.patch("letsencrypt.plugins.standalone.zope.component.getUtility")
-    def test_perform(self, mock_get_utility):
+    def test_perform(self, unused_mock_get_utility):
         achalls = [1, 2, 3]
         self.auth.perform2 = mock.Mock(return_value=mock.sentinel.responses)
         self.assertEqual(mock.sentinel.responses, self.auth.perform(achalls))
@@ -87,7 +87,7 @@ class AuthenticatorTest(unittest.TestCase):
 
     @mock.patch("letsencrypt.plugins.standalone.zope.component.getUtility")
     def _test_perform_bind_errors(self, errno, achalls, mock_get_utility):
-        def _perform2(achalls):
+        def _perform2(unused_achalls):
             raise errors.StandaloneBindError(mock.Mock(errno=errno), 1234)
 
         self.auth.perform2 = mock.MagicMock(side_effect=_perform2)
@@ -98,9 +98,11 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertTrue("1234" in notification.call_args[0][0])
 
     def test_perform_eacces(self):
+        # pylint: disable=no-value-for-parameter
         self._test_perform_bind_errors(socket.errno.EACCES, [])
 
     def test_perform_eaddrinuse(self):
+        # pylint: disable=no-value-for-parameter
         self._test_perform_bind_errors(socket.errno.EADDRINUSE, [])
 
     def test_perfom_unknown_bind_error(self):
@@ -109,9 +111,8 @@ class AuthenticatorTest(unittest.TestCase):
             socket.errno.ENOTCONN, [])
 
     def test_cleanup(self):
-        servers = {1: "server1", 2: "server2"}
         self.auth.servers = mock.Mock()
-        self.auth.servers.items.return_value = [
+        self.auth.servers.running.return_value = [
             (1, ("server1", "thread1")),
             (2, ("server2", "thread2")),
         ]
@@ -123,7 +124,7 @@ class AuthenticatorTest(unittest.TestCase):
             "server1": set(), "server2": set(["chall2", "chall3"])})
         self.auth.servers.stop.assert_called_once_with(1)
 
-        self.auth.servers.items.return_value = [
+        self.auth.servers.running.return_value = [
             (2, ("server2", "thread2")),
         ]
         self.auth.cleanup(["chall2"])