If lineages are in an inconsistent (non-deployed) state, deploy them (#3533)

* If lineages are in an inconsistent (non-deployed) state, deploy them

* Test new _handle_identical_cert case

* Lint

* Fix find & replace SNAFU

certbot/main.py

@@ -155,27 +155,32 @@ def _handle_subset_cert_request(config, domains, cert):
             "reinvoke the client.")
 
 
-def _handle_identical_cert_request(config, cert):
-    """Figure out what to do if a cert has the same names as a previously obtained one
+def _handle_identical_cert_request(config, lineage):
+    """Figure out what to do if a lineage has the same names as a previously obtained one
 
-    :param storage.RenewableCert cert:
+    :param storage.RenewableCert lineage:
 
     :returns: Tuple of (str action, cert_or_None) as per _treat_as_renewal
               action can be: "newcert" | "renew" | "reinstall"
     :rtype: tuple
 
     """
-    if renewal.should_renew(config, cert):
-        return "renew", cert
+    if lineage.has_pending_deployment():
+        logger.warn("Found a new cert /archive/ that was not linked to in /live/; "
+                    "fixing and reinstalling..")
+        lineage.update_all_links_to(lineage.latest_common_version())
+        return "reinstall", lineage
+    if renewal.should_renew(config, lineage):
+        return "renew", lineage
     if config.reinstall:
         # Set with --reinstall, force an identical certificate to be
         # reinstalled without further prompting.
-        return "reinstall", cert
+        return "reinstall", lineage
     question = (
         "You have an existing certificate that contains exactly the same "
         "domains you requested and isn't close to expiry."
         "{br}(ref: {0}){br}{br}What would you like to do?"
-    ).format(cert.configfile.filename, br=os.linesep)
+    ).format(lineage.configfile.filename, br=os.linesep)
 
     if config.verb == "run":
         keep_opt = "Attempt to reinstall this existing certificate"
@@ -193,9 +198,9 @@ def _handle_identical_cert_request(config, cert):
             "User chose to cancel the operation and may "
             "reinvoke the client.")
     elif response[1] == 0:
-        return "reinstall", cert
+        return "reinstall", lineage
     elif response[1] == 1:
-        return "renew", cert
+        return "renew", lineage
     else:
         assert False, "This is impossible"
 

certbot/tests/cli_test.py

@@ -579,6 +579,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem'
         mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path)
         mock_lineage.should_autorenew.return_value = due_for_renewal
+        mock_lineage.has_pending_deployment.return_value = False
         mock_certr = mock.MagicMock()
         mock_key = mock.MagicMock(pem='pem_key')
         mock_client = mock.MagicMock()

certbot/tests/main_test.py

@@ -11,6 +11,23 @@ from certbot import configuration
 from certbot import errors
 from certbot.plugins import disco as plugins_disco
 
+class MainTest(unittest.TestCase):
+    def setUp(self):
+        pass
+    def tearDown(self):
+        pass
+
+    @mock.patch("certbot.main.logger")
+    def test_handle_identical_cert_request_pending(self, _mock_logger):
+        # For now, just test has_pending_deployment_branch; other
+        # coverage is in cli_test.py...
+        from certbot import main
+        mock_lineage = mock.Mock()
+        mock_lineage.has_pending_deployment.return_value = True
+        # pylint: disable=protected-access
+        ret = main._handle_identical_cert_request(mock.Mock(), mock_lineage)
+        self.assertEqual(ret, ("reinstall", mock_lineage))
+        self.assertEqual(mock_lineage.update_all_links_to.call_count, 1)
 
 class ObtainCertTest(unittest.TestCase):
     """Tests for certbot.main.obtain_cert."""