BF: apache cfg parsing - relax assumption that value cannot contain = (#8930)

* BF: apache cfg parsing - relax assumption that value cannot contain =

* Remove failing test_update_runtime_vars_bad_output

* Add test Define statements: with = in value, and an empty value

* update CHANGELOG

Co-authored-by: Alex Zorin <alex@zorin.id.au>

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -153,13 +153,10 @@ def parse_defines(apachectl):
         return {}
 
     for match in matches:
-        if match.count("=") > 1:
-            logger.error("Unexpected number of equal signs in "
-                         "runtime config dump.")
-            raise errors.PluginError(
-                "Error parsing Apache runtime variables")
-        parts = match.partition("=")
-        variables[parts[0]] = parts[2]
+        # Value could also contain = so split only once
+        parts = match.split('=', 1)
+        value = parts[1] if len(parts) == 2 else ''
+        variables[parts[0]] = value
 
     return variables
 

certbot-apache/tests/parser_test.py

@@ -188,6 +188,8 @@ class BasicParserTest(util.ParserTest):
             'Define: DUMP_RUN_CFG\n'
             'Define: U_MICH\n'
             'Define: TLS=443\n'
+            'Define: WITH_ASSIGNMENT=URL=http://example.com\n'
+            'Define: EMPTY=\n'
             'Define: example_path=Documents/path\n'
             'User: name="www-data" id=33 not_used\n'
             'Group: name="www-data" id=33 not_used\n'
@@ -266,7 +268,10 @@ class BasicParserTest(util.ParserTest):
         mock_cfg.side_effect = mock_get_vars
 
         expected_vars = {"TEST": "", "U_MICH": "", "TLS": "443",
-                         "example_path": "Documents/path"}
+                         "example_path": "Documents/path",
+                         "WITH_ASSIGNMENT": "URL=http://example.com",
+                         "EMPTY": "",
+                         }
 
         self.parser.modules = {}
         with mock.patch(
@@ -301,15 +306,6 @@ class BasicParserTest(util.ParserTest):
             # path derived from root configuration Include statements
             self.assertEqual(mock_parse.call_count, 1)
 
-    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
-    def test_update_runtime_vars_bad_output(self, mock_cfg):
-        mock_cfg.return_value = "Define: TLS=443=24"
-        self.parser.update_runtime_variables()
-
-        mock_cfg.return_value = "Define: DUMP_RUN_CFG\nDefine: TLS=443=24"
-        self.assertRaises(
-            errors.PluginError, self.parser.update_runtime_variables)
-
     @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
     def test_update_runtime_vars_bad_ctl(self, mock_run):
         mock_run.side_effect = OSError

certbot/CHANGELOG.md

@@ -14,7 +14,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
-*
+* Fixed parsing of `Define`d values in the Apache plugin to allow for `=` in the value.
 
 More details about these changes can be found on our GitHub repo.