Skip to content

Commit 9adb323

Browse files
committed
fix: Fix "interface_pci_id" role option
Cause: The introduction of this feature in commit 2b0bb1b was broken: The new `interface_pci_id` option was not passed to `firewall_lib`. The integration test case was very shallow and did not verify the success of the operation. Consequence: Calling the role with `interface_pci_id` had no effect, that setting was ignored. Fix: Pass the option to the module. Rework the test to add a positive assertion that the interface mapped to the given PCI ID actually appears in the nftables/iptables rules. Also replace the hardcoded 1af4:0001 ID in the test with runtime detection. That *happens* to be the ID of QEMU's virtio card, but this is neither obvious nor reliable nor does it apply to other environments such as Testing Farm. Fixes #277
1 parent 24620c0 commit 9adb323

File tree

2 files changed

+41
-11
lines changed

2 files changed

+41
-11
lines changed

tasks/main.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
source: "{{ item.source | default(omit) }}"
9999
destination: "{{ item.destination | default(omit) }}"
100100
interface: "{{ item.interface | default(omit) }}"
101+
interface_pci_id: "{{ item.interface_pci_id | default(omit) }}"
101102
icmp_block: "{{ item.icmp_block | default(omit) }}"
102103
icmp_block_inversion: "{{ item.icmp_block_inversion | default(omit) }}"
103104
timeout: "{{ item.timeout | default(omit) }}"

tests/tests_interface_pci.yml

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,30 @@
2626

2727
- name: Test interfaces with PCI ids
2828
block:
29+
- name: Find network interface
30+
shell: |
31+
set -euo pipefail
32+
find /sys/class/net -mindepth 1 ! -name lo | head -n1
33+
register: find_iface
34+
changed_when: false
35+
failed_when: find_iface is failed or (find_iface.stdout | trim) == ""
36+
37+
- name: Determine interface vendor/product ID
38+
shell: |
39+
set -euo pipefail
40+
VID="$(sed 's/^0x//' < {{find_iface.stdout}}/device/vendor)"
41+
PID="$(sed 's/^0x//' < {{find_iface.stdout}}/device/device)"
42+
echo "$VID:$PID"
43+
register: pci_id
44+
changed_when: false
45+
2946
- name: Add pci device ethernet controller
3047
include_role:
3148
name: linux-system-roles.firewall
3249
vars:
3350
firewall:
3451
zone: internal
35-
interface_pci_id: 1af4:0001
52+
interface_pci_id: "{{pci_id.stdout}}"
3653
state: enabled
3754
permanent: true
3855

@@ -42,31 +59,43 @@
4259
vars:
4360
firewall:
4461
zone: internal
45-
interface_pci_id: 1af4:0001
62+
interface_pci_id: "{{pci_id.stdout}}"
4663
state: enabled
4764
permanent: true
4865

49-
- name: Assert pcid not in nftable ruleset
66+
- name: Get nftable ruleset
5067
command: nft list ruleset
51-
register: result
52-
failed_when: result is failed or '1af4:0001' in result.stdout
53-
when: nftables_backend | bool
68+
register: nft_list
5469
changed_when: false
70+
when: nftables_backend | bool
5571

56-
- name: Assert pcid not in iptables rules
72+
- name: Assert that interface is in nftable ruleset
73+
assert:
74+
that:
75+
- find_iface.stdout | basename in nft_list.stdout
76+
- pci_id.stdout | trim not in nft_list.stdout
77+
when: nftables_backend | bool
78+
79+
- name: Get iptables ruleset
5780
command: iptables -S
58-
register: result
59-
failed_when: result is failed or '1af4:0001' in result.stdout
60-
when: not nftables_backend | bool
81+
register: ipt_list
6182
changed_when: false
83+
when: not nftables_backend | bool
84+
85+
- name: Assert that interface is in iptables ruleset
86+
assert:
87+
that:
88+
- find_iface.stdout | basename in ipt_list.stdout
89+
- pci_id.stdout | trim not in ipt_list.stdout
90+
when: not nftables_backend | bool
6291

6392
- name: Remove interface from internal
6493
include_role:
6594
name: linux-system-roles.firewall
6695
vars:
6796
firewall:
6897
zone: internal
69-
interface_pci_id: 1af4:0001
98+
interface_pci_id: "{{pci_id.stdout}}"
7099
state: disabled
71100
permanent: true
72101
always:

0 commit comments

Comments
 (0)