From b4a95b8d2b1d6c08c80aabcbfb5595a28b8e9dea Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Wed, 17 Apr 2019 11:08:16 -0700 Subject: [PATCH] Change customizations.firewall to append items instead of replace To maintain consistency with the other options this changes firewall to combine the existing settings from the image template with the settings from the blueprint. Also updated the docs, added a new test for it, and sorted the output for consistency. (cherry picked from commit 3e08389a0ffea99a6c31046b96123a7a6ceab5e1) Related: rhbz#1718473 --- docs/lorax-composer.rst | 5 ++++- src/pylorax/api/compose.py | 6 +++--- tests/pylorax/test_compose.py | 11 ++++++++--- tests/pylorax/test_recipes.py | 8 ++++---- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/lorax-composer.rst b/docs/lorax-composer.rst index 39f68804..f666e301 100644 --- a/docs/lorax-composer.rst +++ b/docs/lorax-composer.rst @@ -286,6 +286,9 @@ the port:protocol format:: Numeric ports, or their names from ``/etc/services`` can be used in the ``ports`` enabled/disabled lists. +The blueprint settings extend any existing settings in the image templates, so if ``sshd`` is +already enabled it will extend the list of ports with the ones listed by the blueprint. + If the distribution uses ``firewalld`` you can specify services listed by ``firewall-cmd --get-services`` in a ``customizations.firewall.services`` section:: @@ -293,7 +296,7 @@ in a ``customizations.firewall.services`` section:: enabled = ["ftp", "ntp", "dhcp"] disabled = ["telnet"] -Note that these are different from the names in ``/etc/services``, and only ``enabled`` is supported. +Remember that the ``firewall.services`` are different from the names in ``/etc/services``. Both are optional, if they are not used leave them out or set them to an empty list ``[]``. If you only want the default firewall setup this section can be omitted from the blueprint. diff --git a/src/pylorax/api/compose.py b/src/pylorax/api/compose.py index b8c09166..7ca5f9ba 100644 --- a/src/pylorax/api/compose.py +++ b/src/pylorax/api/compose.py @@ -304,9 +304,9 @@ def firewall_cmd(line, settings): # Do not override firewall --disabled if ks.handler.firewall.enabled != False and settings: - ks.handler.firewall.ports = settings["ports"] - ks.handler.firewall.services = settings["enabled"] - ks.handler.firewall.remove_services = settings["disabled"] + ks.handler.firewall.ports = sorted(set(settings["ports"] + ks.handler.firewall.ports)) + ks.handler.firewall.services = sorted(set(settings["enabled"] + ks.handler.firewall.services)) + ks.handler.firewall.remove_services = sorted(set(settings["disabled"] + ks.handler.firewall.remove_services)) # Converting back to a string includes a comment, return just the keyboard line return str(ks.handler.firewall).splitlines()[-1] diff --git a/tests/pylorax/test_compose.py b/tests/pylorax/test_compose.py index 6e62209f..34b53b30 100644 --- a/tests/pylorax/test_compose.py +++ b/tests/pylorax/test_compose.py @@ -407,20 +407,25 @@ disabled = ["telnet"] self.assertEqual(firewall_cmd("firewall --enabled", {"ports": ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"], "enabled": [], "disabled": []}), - "firewall --enabled --port=22:tcp,80:tcp,imap:tcp,53:tcp,53:udp") + "firewall --enabled --port=22:tcp,53:tcp,53:udp,80:tcp,imap:tcp") self.assertEqual(firewall_cmd("firewall --enabled", {"ports": ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"], "enabled": ["ftp", "ntp", "dhcp"], "disabled": []}), - "firewall --enabled --port=22:tcp,80:tcp,imap:tcp,53:tcp,53:udp --service=ftp,ntp,dhcp") + "firewall --enabled --port=22:tcp,53:tcp,53:udp,80:tcp,imap:tcp --service=dhcp,ftp,ntp") self.assertEqual(firewall_cmd("firewall --enabled", {"ports": ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"], "enabled": ["ftp", "ntp", "dhcp"], "disabled": ["telnet"]}), - "firewall --enabled --port=22:tcp,80:tcp,imap:tcp,53:tcp,53:udp --service=ftp,ntp,dhcp --remove-service=telnet") + "firewall --enabled --port=22:tcp,53:tcp,53:udp,80:tcp,imap:tcp --service=dhcp,ftp,ntp --remove-service=telnet") # Make sure that --disabled overrides setting ports and services self.assertEqual(firewall_cmd("firewall --disabled", {"ports": ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"], "enabled": ["ftp", "ntp", "dhcp"], "disabled": ["telnet"]}), "firewall --disabled") + # Make sure that ports includes any existing settings from the firewall command + self.assertEqual(firewall_cmd("firewall --enabled --port=8080:tcp --service=dns --remove-service=ftp", + {"ports": ["80:tcp"], + "enabled": ["ntp"], "disabled": ["telnet"]}), + "firewall --enabled --port=8080:tcp,80:tcp --service=dns,ntp --remove-service=ftp,telnet") def test_get_services(self): """Test get_services function""" diff --git a/tests/pylorax/test_recipes.py b/tests/pylorax/test_recipes.py index 7d91fe09..8d39ae1e 100644 --- a/tests/pylorax/test_recipes.py +++ b/tests/pylorax/test_recipes.py @@ -509,7 +509,7 @@ ports = ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"] self.assertEqual(ks.handler.firewall.remove_services, []) ks = self._blueprint_to_ks(blueprint2_data) - self.assertEqual(ks.handler.firewall.ports, ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"]) + self.assertEqual(ks.handler.firewall.ports, ["22:tcp", "53:tcp", "53:udp", "80:tcp", "imap:tcp"]) self.assertEqual(ks.handler.firewall.services, []) self.assertEqual(ks.handler.firewall.remove_services, []) @@ -524,7 +524,7 @@ disabled = ["telnet"] """ ks = self._blueprint_to_ks(blueprint_data) self.assertEqual(ks.handler.firewall.ports, []) - self.assertEqual(ks.handler.firewall.services, ["ftp", "ntp", "dhcp"]) + self.assertEqual(ks.handler.firewall.services, ["dhcp", "ftp", "ntp"]) self.assertEqual(ks.handler.firewall.remove_services, ["telnet"]) def test_firewall(self): @@ -540,8 +540,8 @@ enabled = ["ftp", "ntp", "dhcp"] disabled = ["telnet"] """ ks = self._blueprint_to_ks(blueprint_data) - self.assertEqual(ks.handler.firewall.ports, ["22:tcp", "80:tcp", "imap:tcp", "53:tcp", "53:udp"]) - self.assertEqual(ks.handler.firewall.services, ["ftp", "ntp", "dhcp"]) + self.assertEqual(ks.handler.firewall.ports, ["22:tcp", "53:tcp", "53:udp", "80:tcp", "imap:tcp"]) + self.assertEqual(ks.handler.firewall.services, ["dhcp", "ftp", "ntp"]) self.assertEqual(ks.handler.firewall.remove_services, ["telnet"]) def test_services(self):