From 3e08389a0ffea99a6c31046b96123a7a6ceab5e1 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. --- 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 cf466ff9..6e391942 100644 --- a/docs/lorax-composer.rst +++ b/docs/lorax-composer.rst @@ -297,6 +297,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:: @@ -304,7 +307,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 610eb17f..ff6be0b0 100644 --- a/src/pylorax/api/compose.py +++ b/src/pylorax/api/compose.py @@ -298,9 +298,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 531c5891..b7c269c2 100644 --- a/tests/pylorax/test_compose.py +++ b/tests/pylorax/test_compose.py @@ -435,20 +435,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 853beb60..67b18c24 100644 --- a/tests/pylorax/test_recipes.py +++ b/tests/pylorax/test_recipes.py @@ -538,7 +538,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, []) @@ -553,7 +553,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): @@ -569,8 +569,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):