Avoid to delete both webroot_map and webroot_path
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3cc3ff41f1f..8669d34a50f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,12 +16,14 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
-*
+* Renewal parameter `webroot_path` is always saved, avoiding some regressions
+  when `webroot` authenticator plugin is invoked with no challenge to perform.
 
 Despite us having broken lockstep, we are continuing to release new versions of
 all Certbot components during releases for the time being, however, the only
 package with changes other than its version number was:
 
+* certbot
 * certbot-dns-rfc2136
 
 More details about these changes can be found on our GitHub repo.
diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py
index 3a902b91fa3..bca1045d8fe 100644
--- a/certbot/plugins/webroot_test.py
+++ b/certbot/plugins/webroot_test.py
@@ -295,6 +295,19 @@ def test_multiwebroot(self):
         self.assertEqual(
             config.webroot_map[self.achall.domain], self.path)
 
+    def test_webroot_map_partial_without_perform(self):
+        # This test acknowledges the fact that webroot_map content will be partial if webroot
+        # plugin perform method is not invoked (corner case when all auths are already valid).
+        # To not be a problem, the webroot_path must always been conserved during renew.
+        # This condition is challenged by:
+        # certbot.tests.renewal_tests::RenewalTest::test_webroot_params_conservation
+        # See https://github.com/certbot/certbot/pull/7095 for details.
+        other_webroot_path = tempfile.mkdtemp()
+        args = self.parser.parse_args("-w {0} -d {1} -w {2} -d bar".format(
+            self.path, self.achall.domain, other_webroot_path).split())
+        self.assertEqual(args.webroot_map, {self.achall.domain: self.path})
+        self.assertEqual(args.webroot_path, [self.path, other_webroot_path])
+
     def _get_config_after_perform(self, config):
         from certbot.plugins.webroot import Authenticator
         auth = Authenticator(config, "webroot")
diff --git a/certbot/renewal.py b/certbot/renewal.py
index f932269b68b..4b4a5a08204 100644
--- a/certbot/renewal.py
+++ b/certbot/renewal.py
@@ -106,11 +106,11 @@ def _restore_webroot_config(config, renewalparams):
     restoring logic is not able to correctly parse it from the serialized
     form.
     """
-    if "webroot_map" in renewalparams:
-        if not cli.set_by_cli("webroot_map"):
-            config.webroot_map = renewalparams["webroot_map"]
-    elif "webroot_path" in renewalparams:
-        logger.debug("Ancient renewal conf file without webroot-map, restoring webroot-path")
+    if "webroot_map" in renewalparams and not cli.set_by_cli("webroot_map"):
+        config.webroot_map = renewalparams["webroot_map"]
+    # To understand why webroot_path and webroot_map processing are not mutually exclusive,
+    # see https://github.com/certbot/certbot/pull/7095
+    if "webroot_path" in renewalparams and not cli.set_by_cli("webroot_path"):
         wp = renewalparams["webroot_path"]
         if isinstance(wp, six.string_types):  # prior to 0.1.0, webroot_path was a string
             wp = [wp]
diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py
index dac585239dc..e33869e13ea 100644
--- a/certbot/tests/renewal_test.py
+++ b/certbot/tests/renewal_test.py
@@ -28,6 +28,29 @@ def test_ancient_webroot_renewal_conf(self, mock_set_by_cli):
         renewal._restore_webroot_config(config, renewalparams)
         self.assertEqual(config.webroot_path, ['/var/www/'])
 
+    @mock.patch('certbot.renewal.cli.set_by_cli')
+    def test_webroot_params_conservation(self, mock_set_by_cli):
+        # For more details about why this test is important, see:
+        # certbot.plugins.webroot_test::WebrootActionTest::test_webroot_map_partial_without_perform
+        from certbot import renewal
+        mock_set_by_cli.return_value = False
+
+        renewalparams = {
+            'webroot_map': {'test.example.com': '/var/www/test'},
+            'webroot_path': ['/var/www/test', '/var/www/other'],
+        }
+        renewal._restore_webroot_config(self.config, renewalparams)  # pylint: disable=protected-access
+        self.assertEqual(self.config.webroot_map, {'test.example.com': '/var/www/test'})
+        self.assertEqual(self.config.webroot_path, ['/var/www/test', '/var/www/other'])
+
+        renewalparams = {
+            'webroot_map': {},
+            'webroot_path': '/var/www/test',
+        }
+        renewal._restore_webroot_config(self.config, renewalparams)  # pylint: disable=protected-access
+        self.assertEqual(self.config.webroot_map, {})
+        self.assertEqual(self.config.webroot_path, ['/var/www/test'])
+
 
 class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase):
     """Tests for certbot.renewal.restore_required_config_elements."""
@@ -89,5 +112,6 @@ def test_must_staple_failure(self, mock_set_by_cli):
         self.assertRaises(
             errors.Error, self._call, self.config, renewalparams)
 
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

Fixes #7048.

This PR "partially" correct #7048. By "partially" here, I mean that the issue is effectively corrected, but not revert to the previous behavior, as this would require more in-depth modifications of the plugins API and behavior.

The idea is that in the specific corner case where `certbot certonly` is invoked two times using `webroot` plugin in a row for a given domain, the second call does not in fact perform a new challenge, but instead reuse the still valid authorization from ACME server. Indeed, authorizations remain valid for a couple of time.

However, this means that the `perform` method of `webroot` here is not invoked. But this method contains some logic to fill the `webroot_map` values in the renewal configuration file, that is not invoked then. If it is not invoked, it means that the last value in `webroot_path` is not associated to the given domains that are not already in `webroot_map`. In particular, this means in the case of a single provided domain that the webroot_map is empty `{}`.

Here it becomes tricky. If we invoke `certbot renew` after that, the method `_restore_webroot_config` is puzzled: it sees that a `webroot_map` is set, and so save its empty value. Then it does not copy `webroot_path`, as there was a value for `webroot_map`, and `webroot_path` is itself a corner case from ancient values ...

Finally, we just lost both `webroot_path` values and `webroot_map`, and the renewal configuration is broken from now on.

In the case of more than one given domain, this broken behavior leads to at least one missing association webroot -> domain in the `webmap_root`.

This PR fixes that by copying the `webroot_path` value if it exists and not explicitly set by CLI. With this, even if `webroot_map` is lost, the renewal configuration remains functional with `webroot_path` using the latest value in it for all missing domains in `webroot_map`, and matches functionally the previous behavior, even if the renewal configuration file remains impacted.

I pass the lool to @bmw.