linux-system-roles / firewall

Configure firewalld and system-config-firewall
https://linux-system-roles.github.io/firewall/
GNU General Public License v2.0
57 stars 32 forks source link

fix: Don't install python(3)-firewall it's a dependency of firewalld #148

Closed spetrosi closed 1 year ago

spetrosi commented 1 year ago

Enhancement: The role now does not run tasks to install python-firewall or python3-firewall based on installed python version.

Reason: python-firewall or python3-firewall is pulled automatically by dnf and yum when installing firewalld. The issue is that when I install python3 on EL 7, the role then fails with "No package matching 'python3-firewall' found available, installed or updated". It sees python3 present on the system and tries to install python3-firewall, which is not available on EL 7.

Result: The role doesn't fail on EL 7 when python3 is installed on the managed node.

spetrosi commented 1 year ago

[citest]

spetrosi commented 1 year ago

I faced this on https://github.com/linux-system-roles/mssql/pull/207 where initial tests on EL 7 pass but then installing mssql-server-15 pulls python3 as a dependency. After this, firewall role tries to install python3-firewall, but it is not available. The role should continue to use python-firewall in this case.

richm commented 1 year ago

@t-woerner @vrindle @erig0 this looks correct, but there must have been some reason we explicitly installed the python firewall library - I don't want to miss some case, then push this update to customers, then find out it breaks some use case that we didn't consider

erig0 commented 1 year ago

there must have been some reason we explicitly installed the python firewall library

IMO it's the more correct dependency. Ansible is probably using the "client" classes in the python3-firewall module.

That being said, firewalld always depends upon python3-firewall (or python-firewall). Most of the "daemon" code is actually in python3-firewall; alongside the client code.

I can't come up with a reason why depending on firewalld instead of python3-firewall would cause a problem.

richm commented 1 year ago

there must have been some reason we explicitly installed the python firewall library

IMO it's the more correct dependency. Ansible is probably using the "client" classes in the python3-firewall module.

Whatever this is:

    from firewall.client import (
        FirewallClient,
        Rich_Rule,
        FirewallClientZoneSettings,
        FirewallClientServiceSettings,
    )

and

    from firewall.core.fw_nm import (
        nm_is_imported,
        nm_get_connection_of_interface,
        nm_get_zone_of_connection,
        nm_set_zone_of_connection,
        nm_get_interfaces,
        nm_get_client,
    )

That being said, firewalld always depends upon python3-firewall (or python-firewall). Most of the "daemon" code is actually in python3-firewall; alongside the client code.

I can't come up with a reason why depending on firewalld instead of python3-firewall would cause a problem.

Ok - and it looks like the unit and integration tests would agree. I think we can merge this (with the npx commitlint pipe fix). Thanks!

erig0 commented 1 year ago

I think we can merge this (with the npx commitlint pipe fix).

Agree.