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

Add/remove interfaces to zone using PCI device ID #89

Closed BrennanPaciorek closed 2 years ago

BrennanPaciorek commented 2 years ago
BrennanPaciorek commented 2 years ago

I'm still tweaking the test, as there will likely be variance in the PCI ids of valid interfaces for each operating system.

liangwen12year commented 2 years ago

The following is what currently in the firewall_lib.py, you may make proper changes to that if you plan to stick with interface as the key name for PCI ID.

  interface:
    description:
      List of interface name strings.
    required: false
    type: list
    elements: str
richm commented 2 years ago

[citest commit:ed1b763a125d77dec38ef56204c30e6fa542e9dc]

richm commented 2 years ago

[citest commit:ed1b763a125d77dec38ef56204c30e6fa542e9dc]

BrennanPaciorek commented 2 years ago

I need to update the test for this feature, specifically to check for dbus errors in case there is no "stdout" when fetching FIREWALL_BACKEND nevermind, its probably not an issue that an dbus error is raised, just that its not being parsed correctly

richm commented 2 years ago

CI test failures

MSG:

[Errno 2] No such file or directory: b'dbus-send' ...ignoring

TASK [get backend from result] ***** Tuesday 19 July 2022 16:15:44 +0000 (0:00:00.391) 0:00:09.307 ** fatal: [sut]: FAILED! => {}

MSG:

The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'stdout'

The error appears to be in '/WORKDIR/dist-git-firewall-gh87-jMYE8r/tests/tests_interface_pci.yml': line 21, column 7, but may be elsewhere in the file depending on the exact syntax problem.


you can ignore the other failures
BrennanPaciorek commented 2 years ago

Okay that should fix the testsuite, CI tests just need to be run to be sure. All feedback has been implemented in the same update to the branch

richm commented 2 years ago

[citest]

richm commented 2 years ago

ok - lgtm - @vrindle @liangwen12year ?

richm commented 2 years ago

ok - I think this is ready to merge, unless there are any other objections?

vrindle commented 2 years ago

LGTM