diff options
author | alexzorin <alex@zorin.id.au> | 2022-01-24 20:33:11 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-01-24 20:33:11 +0300 |
commit | 7198f43008e05ecc0feb37814e0a1f2f520caf3a (patch) | |
tree | c74b5d50f8c1602be05a66820a10d8776d7c7efb | |
parent | fb564cddd9b434d482bcd8ef1d753144be1c10ed (diff) |
apache: expose aug_save errors in the debug log (#9169)
Fixes #9168.
* apache: expose aug_save errors in the debug log
* logger arguments wrong way around
* log formatting
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
-rw-r--r-- | certbot-apache/certbot_apache/_internal/parser.py | 23 | ||||
-rw-r--r-- | certbot-apache/tests/parser_test.py | 13 |
2 files changed, 29 insertions, 7 deletions
diff --git a/certbot-apache/certbot_apache/_internal/parser.py b/certbot-apache/certbot_apache/_internal/parser.py index f52cb2416..fbb22a116 100644 --- a/certbot-apache/certbot_apache/_internal/parser.py +++ b/certbot-apache/certbot_apache/_internal/parser.py @@ -199,7 +199,13 @@ class ApacheParser: """ self.configurator.save_notes = "" - self.aug.save() + + ex_errs = self.aug.match("/augeas//error") + try: + self.aug.save() + except IOError: + self._log_save_errors(ex_errs) + raise # Force reload if files were modified # This is needed to recalculate augeas directive span @@ -215,12 +221,15 @@ class ApacheParser: """ # Check for the root of save problems - new_errs = self.aug.match("/augeas//error") - # logger.error("During Save - %s", mod_conf) - logger.error("Unable to save files: %s. Attempted Save Notes: %s", - ", ".join(err[13:len(err) - 6] for err in new_errs - # Only new errors caused by recent save - if err not in ex_errs), self.configurator.save_notes) + new_errs = [e for e in self.aug.match("/augeas//error") if e not in ex_errs] + + for err in new_errs: + logger.debug( + "Error %s saving %s: %s", self.aug.get(err), err[13:len(err) - 6], + self.aug.get(f"{err}/message")) + logger.error( + "Unable to save files: %s.%s", ", ".join(err[13:len(err) - 6] for err in new_errs), + f" Save Notes: {self.configurator.save_notes}" if self.configurator.save_notes else "") def add_include(self, main_config: str, inc_path: str) -> None: """Add Include for a new configuration file if one does not exist diff --git a/certbot-apache/tests/parser_test.py b/certbot-apache/tests/parser_test.py index 8607eef11..1062156e3 100644 --- a/certbot-apache/tests/parser_test.py +++ b/certbot-apache/tests/parser_test.py @@ -35,6 +35,19 @@ class BasicParserTest(util.ParserTest): self.parser.aug.save = mock_save self.assertRaises(errors.PluginError, self.parser.unsaved_files) + @mock.patch("certbot_apache._internal.parser.logger") + def test_bad_save_errors(self, mock_logger): + nx_path = "/non/existent/path.conf" + self.parser.aug.set("/augeas/load/Httpd/incl[last()]", nx_path) + self.parser.add_dir(f"/files{nx_path}", "AddDirective", "test") + + self.assertRaises(IOError, self.parser.save, {}) + mock_logger.error.assert_called_with( + 'Unable to save files: %s.%s', '/non/existent/path.conf', mock.ANY) + mock_logger.debug.assert_called_with( + "Error %s saving %s: %s", "mk_augtemp", + "/non/existent/path.conf", "No such file or directory") + def test_aug_version(self): mock_match = mock.Mock(return_value=["something"]) self.parser.aug.match = mock_match |