From 3b9a87cd2fd2d565c002dde37419a6f8624b7a20 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 23 Sep 2016 17:48:50 -0700 Subject: [PATCH] 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 | 23 ++++++++++++++--------- certbot/tests/cli_test.py | 1 + certbot/tests/main_test.py | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/certbot/main.py b/certbot/main.py index 675d10640..3557f65b3 100644 --- a/certbot/main.py +++ b/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" diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index fdfb9dcc8..6cbbea631 100644 --- a/certbot/tests/cli_test.py +++ b/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() diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 32df525f0..69291871a 100644 --- a/certbot/tests/main_test.py +++ b/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."""