Use static directory under workdir for HTTP challenges (#5428)

* Use static directory under workdir for HTTP challenges

* Handle the reverter file registration before opening file handle

certbot-apache/certbot_apache/configurator.py

@@ -1956,7 +1956,6 @@ def cleanup(self, achalls):
             self.revert_challenge_config()
             self.restart()
             self.parser.reset_modules()
-            self.http_doer.cleanup()
 
     def install_ssl_options_conf(self, options_ssl, options_ssl_digest):
         """Copy Certbot's SSL options file into the system's config dir if required."""

certbot-apache/certbot_apache/http_01.py

@@ -1,8 +1,6 @@
 """A class that performs HTTP-01 challenges for Apache"""
 import logging
 import os
-import shutil
-import tempfile
 
 from certbot.plugins import common
 
@@ -35,7 +33,9 @@ def __init__(self, *args, **kwargs):
         self.challenge_conf = os.path.join(
             self.configurator.conf("challenge-location"),
             "le_http_01_challenge.conf")
-        self.challenge_dir = None
+        self.challenge_dir = os.path.join(
+            self.configurator.config.work_dir,
+            "http_challenges")
 
     def perform(self):
         """Perform all HTTP-01 challenges."""
@@ -56,12 +56,6 @@ def perform(self):
 
         return responses
 
-    def cleanup(self):
-        """Cleanup the challenge directory."""
-        if self.challenge_dir:
-            shutil.rmtree(self.challenge_dir, ignore_errors=True)
-        self.challenge_dir = None
-
     def prepare_http01_modules(self):
         """Make sure that we have the needed modules available for http01"""
 
@@ -92,8 +86,9 @@ def _mod_config(self):
             new_conf.write(config_text)
 
     def _set_up_challenges(self):
-        self.challenge_dir = tempfile.mkdtemp()
-        os.chmod(self.challenge_dir, 0o755)
+        if not os.path.isdir(self.challenge_dir):
+            os.makedirs(self.challenge_dir)
+            os.chmod(self.challenge_dir, 0o755)
 
         responses = []
         for achall in self.achalls:
@@ -105,6 +100,8 @@ def _set_up_challenge(self, achall):
         response, validation = achall.response_and_validation()
 
         name = os.path.join(self.challenge_dir, achall.chall.encode("token"))
+
+        self.configurator.reverter.register_file_creation(True, name)
         with open(name, 'wb') as f:
             f.write(validation.encode())
         os.chmod(name, 0o644)

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -747,7 +747,6 @@ def test_perform(self, mock_restart, mock_tls_perform, mock_http_perform):
     def test_cleanup(self, mock_cfg, mock_restart):
         mock_cfg.return_value = ""
         _, achalls = self.get_key_and_achalls()
-        self.config.http_doer = mock.MagicMock()
 
         for achall in achalls:
             self.config._chall_out.add(achall)  # pylint: disable=protected-access
@@ -756,10 +755,8 @@ def test_cleanup(self, mock_cfg, mock_restart):
             self.config.cleanup([achall])
             if i == len(achalls) - 1:
                 self.assertTrue(mock_restart.called)
-                self.assertTrue(self.config.http_doer.cleanup.called)
             else:
                 self.assertFalse(mock_restart.called)
-                self.assertFalse(self.config.http_doer.cleanup.called)
 
     @mock.patch("certbot_apache.configurator.ApacheConfigurator.restart")
     @mock.patch("certbot_apache.parser.ApacheParser._get_runtime_cfg")
@@ -773,11 +770,9 @@ def test_cleanup_no_errors(self, mock_cfg, mock_restart):
 
         self.config.cleanup([achalls[-1]])
         self.assertFalse(mock_restart.called)
-        self.assertFalse(self.config.http_doer.cleanup.called)
 
         self.config.cleanup(achalls)
         self.assertTrue(mock_restart.called)
-        self.assertTrue(self.config.http_doer.cleanup.called)
 
     @mock.patch("certbot.util.run_script")
     def test_get_version(self, mock_script):

certbot-apache/certbot_apache/tests/http_01_test.py

@@ -100,6 +100,8 @@ def common_enable_modules_test(self, mock_enmod):
 
     def common_perform_test(self, achalls):
         """Tests perform with the given achalls."""
+        challenge_dir = self.http.challenge_dir
+        self.assertFalse(os.path.exists(challenge_dir))
         for achall in achalls:
             self.http.add_chall(achall)
 
@@ -114,9 +116,7 @@ def common_perform_test(self, achalls):
         for achall in achalls:
             self._test_challenge_file(achall)
 
-        challenge_dir = self.http.challenge_dir
-        self.http.cleanup()
-        self.assertFalse(os.path.exists(challenge_dir))
+        self.assertTrue(os.path.exists(challenge_dir))
 
     def _test_challenge_conf(self):
         self.assertEqual(