Allow multiple interactive certname selections in certbot delete (#5133)

certbot/cert_manager.py

@@ -45,7 +45,7 @@ def rename_lineage(config):
     """
     disp = zope.component.getUtility(interfaces.IDisplay)
 
-    certname = _get_certname(config, "rename")
+    certname = _get_certnames(config, "rename")[0]
 
     new_certname = config.new_certname
     if not new_certname:
@@ -87,11 +87,12 @@ def certificates(config):
 
 def delete(config):
     """Delete Certbot files associated with a certificate lineage."""
-    certname = _get_certname(config, "delete")
+    certnames = _get_certnames(config, "delete", allow_multiple=True)
-    storage.delete_files(config, certname)
+    for certname in certnames:
-    disp = zope.component.getUtility(interfaces.IDisplay)
+        storage.delete_files(config, certname)
-    disp.notification("Deleted all files relating to certificate {0}."
+        disp = zope.component.getUtility(interfaces.IDisplay)
-        .format(certname), pause=False)
+        disp.notification("Deleted all files relating to certificate {0}."
+            .format(certname), pause=False)
 
 ###################
 # Public Helpers
@@ -146,23 +147,34 @@ def find_duplicative_certs(config, domains):
 # Private Helpers
 ###################
 
-def _get_certname(config, verb):
+def _get_certnames(config, verb, allow_multiple=False):
     """Get certname from flag, interactively, or error out.
     """
     certname = config.certname
-    if not certname:
+    if certname:
+        certnames = [certname]
+    else:
         disp = zope.component.getUtility(interfaces.IDisplay)
         filenames = storage.renewal_conf_files(config)
         choices = [storage.lineagename_for_filename(name) for name in filenames]
         if not choices:
             raise errors.Error("No existing certificates found.")
-        code, index = disp.menu("Which certificate would you like to {0}?".format(verb),
+        if allow_multiple:
-                                choices, flag="--cert-name",
+            code, certnames = disp.checklist(
-                                force_interactive=True)
+                                    "Which certificate(s) would you like to {0}?".format(verb),
-        if code != display_util.OK or not index in range(0, len(choices)):
+                                    choices, cli_flag="--cert-name",
-            raise errors.Error("User ended interaction.")
+                                    force_interactive=True)
-        certname = choices[index]
+            if code != display_util.OK:
-    return certname
+                raise errors.Error("User ended interaction.")
+        else:
+            code, index = disp.menu("Which certificate would you like to {0}?".format(verb),
+                                    choices, cli_flag="--cert-name",
+                                    force_interactive=True)
+
+            if code != display_util.OK or index not in range(0, len(choices)):
+                raise errors.Error("User ended interaction.")
+            certnames = [choices[index]]
+    return certnames
 
 def _report_lines(msgs):
     """Format a results report for a category of single-line renewal outcomes"""

certbot/tests/cert_manager_test.py

@@ -1,3 +1,4 @@
+
 """Tests for certbot.cert_manager."""
 # pylint: disable=protected-access
 import os
@@ -107,16 +108,45 @@ class UpdateLiveSymlinksTest(BaseCertManagerTest):
 class DeleteTest(storage_test.BaseRenewableCertTest):
     """Tests for certbot.cert_manager.delete
     """
+
+    def _call(self):
+        from certbot import cert_manager
+        cert_manager.delete(self.config)
+
     @test_util.patch_get_utility()
     @mock.patch('certbot.cert_manager.lineage_for_certname')
     @mock.patch('certbot.storage.delete_files')
-    def test_delete(self, mock_delete_files, mock_lineage_for_certname, unused_get_utility):
+    def test_delete_from_config(self, mock_delete_files, mock_lineage_for_certname,
+        unused_get_utility):
         """Test delete"""
         mock_lineage_for_certname.return_value = self.test_rc
         self.config.certname = "example.org"
-        from certbot import cert_manager
+        self._call()
-        cert_manager.delete(self.config)
+        mock_delete_files.assert_called_once_with(self.config, "example.org")
-        self.assertTrue(mock_delete_files.called)
+
+    @test_util.patch_get_utility()
+    @mock.patch('certbot.cert_manager.lineage_for_certname')
+    @mock.patch('certbot.storage.delete_files')
+    def test_delete_interactive_single(self, mock_delete_files, mock_lineage_for_certname,
+        mock_util):
+        """Test delete"""
+        mock_lineage_for_certname.return_value = self.test_rc
+        mock_util().checklist.return_value = (display_util.OK, ["example.org"])
+        self._call()
+        mock_delete_files.assert_called_once_with(self.config, "example.org")
+
+    @test_util.patch_get_utility()
+    @mock.patch('certbot.cert_manager.lineage_for_certname')
+    @mock.patch('certbot.storage.delete_files')
+    def test_delete_interactive_multiple(self, mock_delete_files, mock_lineage_for_certname,
+        mock_util):
+        """Test delete"""
+        mock_lineage_for_certname.return_value = self.test_rc
+        mock_util().checklist.return_value = (display_util.OK, ["example.org", "other.org"])
+        self._call()
+        mock_delete_files.assert_any_call(self.config, "example.org")
+        mock_delete_files.assert_any_call(self.config, "other.org")
+        self.assertEqual(mock_delete_files.call_count, 2)
 
 
 class CertificatesTest(BaseCertManagerTest):