From 832b9d3bced8e31ab3fb19583b2821d2202a59f7 Mon Sep 17 00:00:00 2001 From: OniriCorpe Date: Mon, 20 May 2024 23:58:50 +0200 Subject: [PATCH 1/5] provision: reload the firewall only once --- src/utils/resources.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/utils/resources.py b/src/utils/resources.py index e2400e91d..0232cc7fa 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -1153,7 +1153,9 @@ class AptDependenciesAppResource(AppResource): for key, values in self.extras.items(): if isinstance(values.get("packages"), str): - values["packages"] = [value.strip() for value in values["packages"].split(",")] # type: ignore + values["packages"] = [ + value.strip() for value in values["packages"].split(",") + ] # type: ignore if isinstance(values.get("packages_from_raw_bash"), str): out, err = self.check_output_bash_snippet( @@ -1164,7 +1166,9 @@ class AptDependenciesAppResource(AppResource): f"Error while running apt resource packages_from_raw_bash snippet for '{key}' extras:" ) logger.error(err) - values["packages"] = values.get("packages", []) + [value.strip() for value in out.split("\n")] # type: ignore + values["packages"] = values.get("packages", []) + [ + value.strip() for value in out.split("\n") + ] # type: ignore if ( not isinstance(values.get("repo"), str) @@ -1291,7 +1295,14 @@ class PortsResource(AppResource): return used_by_process or used_by_app or used_by_self_provisioning def provision_or_update(self, context: Dict = {}): - from yunohost.firewall import firewall_allow, firewall_disallow + from yunohost.firewall import ( + firewall_allow, + firewall_disallow, + firewall_list, + firewall_reload, + ) + + previous_ports = firewall_list(raw=True) for name, infos in self.ports.items(): setting_name = f"port_{name}" if name != "main" else "port" @@ -1322,11 +1333,12 @@ class PortsResource(AppResource): self.set_setting(setting_name, port_value) if infos["exposed"]: - firewall_allow(infos["exposed"], port_value, reload_only_if_change=True) + firewall_allow(infos["exposed"], port_value, no_reload=True) else: - firewall_disallow( - infos["exposed"], port_value, reload_only_if_change=True - ) + firewall_disallow(infos["exposed"], port_value, no_reload=True) + + if firewall_list(raw=True) != previous_ports: + firewall_reload() def deprovision(self, context: Dict = {}): from yunohost.firewall import firewall_disallow From 6b564ef9f4214070f302597d664a1ec6afb4cba0 Mon Sep 17 00:00:00 2001 From: OniriCorpe Date: Tue, 21 May 2024 00:14:34 +0200 Subject: [PATCH 2/5] same for deprovisionning: reload once --- src/utils/resources.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/utils/resources.py b/src/utils/resources.py index 0232cc7fa..52d8edbbf 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -1341,16 +1341,19 @@ class PortsResource(AppResource): firewall_reload() def deprovision(self, context: Dict = {}): - from yunohost.firewall import firewall_disallow + from yunohost.firewall import firewall_disallow, firewall_list, firewall_reload + + previous_ports = firewall_list(raw=True) for name, infos in self.ports.items(): setting_name = f"port_{name}" if name != "main" else "port" value = self.get_setting(setting_name) self.delete_setting(setting_name) if value and str(value).strip(): - firewall_disallow( - infos["exposed"], int(value), reload_only_if_change=True - ) + firewall_disallow(infos["exposed"], int(value), no_reload=True) + + if firewall_list(raw=True) != previous_ports: + firewall_reload() class DatabaseAppResource(AppResource): From b8b683bd660d36866f31329ac33b46225df07a51 Mon Sep 17 00:00:00 2001 From: OniriCorpe Date: Tue, 21 May 2024 00:22:36 +0200 Subject: [PATCH 3/5] undo an autoformater edit --- src/utils/resources.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/utils/resources.py b/src/utils/resources.py index 52d8edbbf..2abb4d603 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -1153,9 +1153,7 @@ class AptDependenciesAppResource(AppResource): for key, values in self.extras.items(): if isinstance(values.get("packages"), str): - values["packages"] = [ - value.strip() for value in values["packages"].split(",") - ] # type: ignore + values["packages"] = [value.strip() for value in values["packages"].split(",")] # type: ignore if isinstance(values.get("packages_from_raw_bash"), str): out, err = self.check_output_bash_snippet( @@ -1166,9 +1164,7 @@ class AptDependenciesAppResource(AppResource): f"Error while running apt resource packages_from_raw_bash snippet for '{key}' extras:" ) logger.error(err) - values["packages"] = values.get("packages", []) + [ - value.strip() for value in out.split("\n") - ] # type: ignore + values["packages"] = values.get("packages", []) + [value.strip() for value in out.split("\n")] # type: ignore if ( not isinstance(values.get("repo"), str) From d354c75dbf9f667960caa97d19f46aaebd18950a Mon Sep 17 00:00:00 2001 From: OniriCorpe Date: Tue, 21 May 2024 22:50:53 +0200 Subject: [PATCH 4/5] remove now unused 'reload_only_if_change' stuff --- src/firewall.py | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/firewall.py b/src/firewall.py index 3ed33da61..c5873bf36 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -38,7 +38,6 @@ def firewall_allow( ipv6_only=False, no_upnp=False, no_reload=False, - reload_only_if_change=False, ): """ Allow connections on a port @@ -86,10 +85,7 @@ def firewall_allow( changed = True else: ipv = "IPv%s" % i[3] - if not reload_only_if_change: - logger.warning( - m18n.n("port_already_opened", port=port, ip_version=ipv) - ) + logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv)) # Add port forwarding with UPnP if not no_upnp and port not in firewall["uPnP"][p]: firewall["uPnP"][p].append(port) @@ -101,9 +97,7 @@ def firewall_allow( # Update and reload firewall _update_firewall_file(firewall) - if (not reload_only_if_change and not no_reload) or ( - reload_only_if_change and changed - ): + if (not no_reload) or (changed): return firewall_reload() @@ -114,7 +108,6 @@ def firewall_disallow( ipv6_only=False, upnp_only=False, no_reload=False, - reload_only_if_change=False, ): """ Disallow connections on a port @@ -169,10 +162,7 @@ def firewall_disallow( changed = True else: ipv = "IPv%s" % i[3] - if not reload_only_if_change: - logger.warning( - m18n.n("port_already_closed", port=port, ip_version=ipv) - ) + logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv)) # Remove port forwarding with UPnP if upnp and port in firewall["uPnP"][p]: firewall["uPnP"][p].remove(port) @@ -182,9 +172,7 @@ def firewall_disallow( # Update and reload firewall _update_firewall_file(firewall) - if (not reload_only_if_change and not no_reload) or ( - reload_only_if_change and changed - ): + if (not no_reload) or (changed): return firewall_reload() From 33cb89918f835f2b1a4cccd69fbf14a596452ad6 Mon Sep 17 00:00:00 2001 From: OniriCorpe Date: Tue, 21 May 2024 23:04:52 +0200 Subject: [PATCH 5/5] remove now unused 'changed' stuff --- src/firewall.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/firewall.py b/src/firewall.py index c5873bf36..7dcd1a190 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -75,14 +75,11 @@ def firewall_allow( "ipv6", ] - changed = False - for p in protocols: # Iterate over IP versions to add port for i in ipvs: if port not in firewall[i][p]: firewall[i][p].append(port) - changed = True else: ipv = "IPv%s" % i[3] logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv)) @@ -97,7 +94,7 @@ def firewall_allow( # Update and reload firewall _update_firewall_file(firewall) - if (not no_reload) or (changed): + if not no_reload: return firewall_reload() @@ -152,14 +149,11 @@ def firewall_disallow( elif upnp_only: ipvs = [] - changed = False - for p in protocols: # Iterate over IP versions to remove port for i in ipvs: if port in firewall[i][p]: firewall[i][p].remove(port) - changed = True else: ipv = "IPv%s" % i[3] logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv)) @@ -172,7 +166,7 @@ def firewall_disallow( # Update and reload firewall _update_firewall_file(firewall) - if (not no_reload) or (changed): + if not no_reload: return firewall_reload()