From 9b0f8af70b1b300d96003420bcbcf5beceda2ed0 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 5 Mar 2019 15:39:28 +0200 Subject: [PATCH] Address review comments --- certbot-apache/certbot_apache/override_centos.py | 9 ++++----- certbot-apache/certbot_apache/parser.py | 4 ++-- certbot-apache/certbot_apache/tests/centos6_test.py | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py index 2b1623671..04425741b 100644 --- a/certbot-apache/certbot_apache/override_centos.py +++ b/certbot-apache/certbot_apache/override_centos.py @@ -73,16 +73,16 @@ class CentOSConfigurator(configurator.ApacheConfigurator): for m in loadmods: if "/conf.d/ssl.conf/" in m: # Strip "arg[1]" off from the end - sslconf_loadmod_path = m[:-7] + sslconf_loadmod_path = m.rpartition("/")[0] # Use the preconfigured LoadModule values from ssl.conf loadmod_args = self.parser.get_all_args(sslconf_loadmod_path) elif self.parser.loc["default"] in m: return - rootconf_ifmod = self.parser._get_ifmod( # pylint: disable=protected-access + rootconf_ifmod = self.parser.get_ifmod( parser.get_aug_path(self.parser.loc["default"]), "!mod_ssl.c", beginning=True) - # parser._get_ifmod returns a path postfixed with "/", remove that + # parser.get_ifmod returns a path postfixed with "/", remove that self.parser.add_dir(rootconf_ifmod[:-1], "LoadModule", loadmod_args) self.save_notes += "Added LoadModule ssl_module to main configuration.\n" @@ -94,8 +94,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator): self.aug.remove(sslconf_loadmod_path) # Create a new IfModule !mod_ssl.c - ssl_ifmod = self.parser._get_ifmod( # pylint: disable=protected-access - sslconf_path, "!mod_ssl.c", beginning=True) + ssl_ifmod = self.parser.get_ifmod(sslconf_path, "!mod_ssl.c", beginning=True) self.parser.add_dir(ssl_ifmod[:-1], "LoadModule", loadmod_args) self.save_notes += ("Wrapped ssl.conf LoadModule ssl_module inside " diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index 8afd403b6..171928626 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -286,7 +286,7 @@ class ApacheParser(object): """ # TODO: Add error checking code... does the path given even exist? # Does it throw exceptions? - if_mod_path = self._get_ifmod(aug_conf_path, "mod_ssl.c") + if_mod_path = self.get_ifmod(aug_conf_path, "mod_ssl.c") # IfModule can have only one valid argument, so append after self.aug.insert(if_mod_path + "arg", "directive", False) nvh_path = if_mod_path + "directive[1]" @@ -297,7 +297,7 @@ class ApacheParser(object): for i, arg in enumerate(args): self.aug.set("%s/arg[%d]" % (nvh_path, i + 1), arg) - def _get_ifmod(self, aug_conf_path, mod, beginning=False): + def get_ifmod(self, aug_conf_path, mod, beginning=False): """Returns the path to and creates one if it doesn't exist. :param str aug_conf_path: Augeas configuration path diff --git a/certbot-apache/certbot_apache/tests/centos6_test.py b/certbot-apache/certbot_apache/tests/centos6_test.py index 7d3b117cd..8f717496d 100644 --- a/certbot-apache/certbot_apache/tests/centos6_test.py +++ b/certbot-apache/certbot_apache/tests/centos6_test.py @@ -96,7 +96,7 @@ class CentOS6Tests(util.ApacheTest): def test_loadmod_rootconf_exists(self): sslmod_args = ["ssl_module", "modules/uniquename.so"] - rootconf_ifmod = self.config.parser._get_ifmod( # pylint: disable=protected-access + rootconf_ifmod = self.config.parser.get_ifmod( parser.get_aug_path(self.config.parser.loc["default"]), "!mod_ssl.c", beginning=True) self.config.parser.add_dir(rootconf_ifmod[:-1], "LoadModule", sslmod_args)