[BUG] iptables rules (iptables.insert with save:True) added with each salt run, resulting in duplicate entries in /etc/iptables/rules.v* #60467
Comments
I am just not able to repro. iptables v1.8.2 here 🤔 Not sure why that would make a difference though. |
|
Try running check: https://docs.saltproject.io/en/latest/ref/modules/all/salt.modules.iptables.html#salt.modules.iptables.check I think it would look like |
|
I'm on a very slim, clean and vanilla debian bullseye (installed via test.sls: Output of two runs: |
|
Calling |
|
Okay that's heckin interesting! And also confusing. Because the Do you have a copy of the settings that you used to spin up the vm? |
so I'm thinking that it might be incorrectly comparing the outputs.
|
I can confirm that this is still happening with a Debian 11 salt master, and a Debian 11 salt minion. Version reports below: Master Minion: |
|
@c1pherx was this docker images or virt, or...? |
waynew
added
Confirmed
|
Okay, I was able to repro this using a vagrant box from Is the state that I used, and I set the root password to (followed by ssh 127.0.0.1 to accept the host key) allowed me to run Will report that the rule exists, so apparently somewhere in the iptables code when we do the check we're not doing it correctly. I didn't dig into that part, but when someone fixes this issue, it's going to probably be within the iptables module where it does Anyway, this is confirmed 👍 |
|
Since this is confirmed and Debian bullseye (11) is released as stable meanwhile, can we push that a little tiny bit on the agenda? :) |
|
@mirko added to our list to consider for Phosphorus. No promises, but we'll at least consider it. Of course, if you're willing to add some tests/fix, we welcome PRs! If you're not sure about writing tests, I run the Test Clinics on Twitch every Tues/Thurs, and am there to help folks get started writing tests! |
|
I had this issue on Debian bullseye and this is a fix that works with bullseye but not with buster. This bug is due to a change in the return of Following servers have this rule active in firewall: With buster: With bullseye: One way to make Line 425 in 2fed568 There are two other lines in the code with "iptables.check" that need attention. Hope it helps. |
|
Also hit this problem testing upgrade to bullseye. Looking at the code (salt & iptables) it's unclear to me it this actually might be an iptables bug. The reason is that iptables in But it does when you pass the For some reason iptables under When this option was introduced (in iptables commit d59b9db031abee37a9aa9776662dd15370faabf4) it states: Based on this I think it's safe to just check on the return code from --- iptables.py 2022-02-28 13:24:48.719229701 +0000
+++ iptables.py 2022-02-28 13:24:42.698220497 +0000
@@ -744,6 +744,7 @@
if _has_option("--check", family):
cmd = "{} -t {} -C {} {}".format(ipt_cmd, table, chain, rule)
out = __salt__["cmd.run"](cmd, output_loglevel="quiet")
+ return not __context__["retcode"]
else:
_chain_name = hex(uuid.getnode())The only downside to this is if someone actually uses the return of this command for something, but I don't think so, and I can't find a use case for the actual output in the code. |
|
As it not yet fixed in Salt: 3002.6 for Debian 11 bullseye I have honour to present ugliest workaround ever: |
Description
Having iptables rules defined via salt's iptables states, e.g.:
results in correct entries in /etc/iptables/rules.v4 after the first run.
With each further salt run, though, the same line gets appended to the existing lines in /etc/iptables/rules.v4.
After 42 runs I have 42 lines of above defined rule listed in /etc/iptables/rules.v4.
Running salt in this case means running salt-ssh.
Setup
salt-ssh on Debian/Bullseye (currently testing)
Steps to Reproduce the behavior
see above
Expected behavior
The respective iptables rule, derived from the above defined iptables salt state, being present only once in /etc/iptables/rules.v4 - no matter how often salt was run.
Versions Report
salt --versions-report
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)Additional context
Potentially relevant issues / commits:
The text was updated successfully, but these errors were encountered: