Skip to content

Commit

Permalink
Fix iptables fixture and delete action
Browse files Browse the repository at this point in the history
Iptables check does not work when having a chain of sources
When we insert a rule with multiple source, iptables split the list
and adding each source with same conditions

On check action iptables wont catch case when having a mix of source
when 1 is not in the table. looks like iptables check only one.
In case check action returned wrong result add/delete rule chnaged and
may trigger failures due to unexpected results

Also deletion , iptables delete only the first match when having duplicate
rules.

Example:
iptables --insert FORWARD -p tcp -j DROP -s 192.168.128.10,192.168.128.13
results with:
-A FORWARD -s 192.168.128.13/32 -p tcp -m tcp --dport 80 -j DROP
-A FORWARD -s 192.168.128.10/32 -p tcp -m tcp --dport 80 -j DROP

We will get different result from iptables for:
iptables --check FORWARD -p tcp -j DROP -s 7.7.7.7,192.168.128.10
iptables --check FORWARD -p tcp -j DROP -s 192.168.128.10,7.7.7.7

Means that only 1 is the best approch here.
Run delete rules till all duplicate rules gone
  • Loading branch information
bkopilov committed Dec 15, 2022
1 parent 9ac7b5e commit 562ebc0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
5 changes: 3 additions & 2 deletions src/assisted_test_infra/test_infra/controllers/iptables.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ def insert(self) -> None:
utils.run_command(insert_rule, shell=True)

def delete(self) -> None:
if self._does_rule_exist():
delete_rule = self._build_command_string(IpTableCommandOption.DELETE)
delete_rule = self._build_command_string(IpTableCommandOption.DELETE)
# delete duplicate rules - iptable deletes only first hit
while self._does_rule_exist():
log.info(f"Removing iptable rule: {delete_rule}")
utils.run_command(delete_rule, shell=True)
16 changes: 13 additions & 3 deletions src/tests/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,19 @@ def set_iptables_rules_for_nodes(
log.info(f"Given node ips: {given_node_ips}")

for _rule in iptables_rules:
_rule.add_sources(given_node_ips)
rules.append(_rule)
_rule.insert()
if not given_node_ips:
# in case a rule without given_node_ips
rules.append(_rule)
_rule.insert()
continue
for source in given_node_ips:
# iptables check broken for list of sources
# copy base rule and add a single source.
copy_rule = deepcopy(_rule)
copy_rule.add_sources([source])
log.info(f"Adding rule with source: {copy_rule}")
rules.append(copy_rule)
copy_rule.insert()

yield set_iptables_rules_for_nodes
log.info("---TEARDOWN iptables ---")
Expand Down

0 comments on commit 562ebc0

Please sign in to comment.