diff --git a/changelogs/fragments/86147-fix-iptables-extension-handling.yml b/changelogs/fragments/86147-fix-iptables-extension-handling.yml new file mode 100644 index 00000000000..d93daa7e211 --- /dev/null +++ b/changelogs/fragments/86147-fix-iptables-extension-handling.yml @@ -0,0 +1,6 @@ +--- +bugfixes: + - >- + iptables - The module can now detect when a extensions added with the module ``match`` argument have + been automatically imported by other module arguments such as ``uid_owner`` and prevents duplicate + extension imports which previously caused an error (https://github.com/ansible/ansible/issues/84387). diff --git a/lib/ansible/modules/iptables.py b/lib/ansible/modules/iptables.py index 9502dcad2cb..09350185726 100644 --- a/lib/ansible/modules/iptables.py +++ b/lib/ansible/modules/iptables.py @@ -136,6 +136,8 @@ options: - The set of matches makes up the condition under which a target is invoked. - Matches are evaluated first to last if specified as an array and work in short-circuit fashion, in other words if one extension yields false, the evaluation will stop. + - The module automatically adds necessary extensions (for example, O(uid_owner=1) corresponds + to the command flags C(-m owner --uid-owner 1)), so it is often unnecessary to use the O(match) argument. type: list elements: str default: [] @@ -598,9 +600,10 @@ def append_csv(rule, param, flag): rule.extend([flag, ','.join(param)]) -def append_match(rule, param, match): - if param: +def append_match(rule, param, match, loaded_extensions): + if param and match not in loaded_extensions: rule.extend(['-m', match]) + loaded_extensions.add(match) def append_jump(rule, param, jump): @@ -619,6 +622,7 @@ def construct_rule(params): append_param(rule, params['source'], '-s', False) append_param(rule, params['destination'], '-d', False) append_param(rule, params['match'], '-m', True) + loaded_extensions = set(params['match']) # Keep track of the above extensions append_tcp_flags(rule, params['tcp_flags'], '--tcp-flags') append_param(rule, params['jump'], '-j', False) if params.get('jump') and params['jump'].lower() == 'tee': @@ -626,7 +630,7 @@ def construct_rule(params): append_param(rule, params['log_prefix'], '--log-prefix', False) append_param(rule, params['log_level'], '--log-level', False) append_param(rule, params['to_destination'], '--to-destination', False) - append_match(rule, params['destination_ports'], 'multiport') + append_match(rule, params['destination_ports'], 'multiport', loaded_extensions) append_csv(rule, params['destination_ports'], '--dports') append_param(rule, params['to_source'], '--to-source', False) append_param(rule, params['goto'], '-g', False) @@ -654,29 +658,29 @@ def construct_rule(params): elif 'state' in params['match']: append_csv(rule, params['ctstate'], '--state') elif params['ctstate']: - append_match(rule, params['ctstate'], 'conntrack') + append_match(rule, params['ctstate'], 'conntrack', loaded_extensions) append_csv(rule, params['ctstate'], '--ctstate') if 'iprange' in params['match']: append_param(rule, params['src_range'], '--src-range', False) append_param(rule, params['dst_range'], '--dst-range', False) elif params['src_range'] or params['dst_range']: - append_match(rule, params['src_range'] or params['dst_range'], 'iprange') + append_match(rule, params['src_range'] or params['dst_range'], 'iprange', loaded_extensions) append_param(rule, params['src_range'], '--src-range', False) append_param(rule, params['dst_range'], '--dst-range', False) if 'set' in params['match']: append_param(rule, params['match_set'], '--match-set', False) append_match_flag(rule, 'match', params['match_set_flags'], False) elif params['match_set']: - append_match(rule, params['match_set'], 'set') + append_match(rule, params['match_set'], 'set', loaded_extensions) append_param(rule, params['match_set'], '--match-set', False) append_match_flag(rule, 'match', params['match_set_flags'], False) - append_match(rule, params['limit'] or params['limit_burst'], 'limit') + append_match(rule, params['limit'] or params['limit_burst'], 'limit', loaded_extensions) append_param(rule, params['limit'], '--limit', False) append_param(rule, params['limit_burst'], '--limit-burst', False) - append_match(rule, params['uid_owner'], 'owner') + append_match(rule, params['uid_owner'], 'owner', loaded_extensions) append_match_flag(rule, params['uid_owner'], '--uid-owner', True) append_param(rule, params['uid_owner'], '--uid-owner', False) - append_match(rule, params['gid_owner'], 'owner') + append_match(rule, params['gid_owner'], 'owner', loaded_extensions) append_match_flag(rule, params['gid_owner'], '--gid-owner', True) append_param(rule, params['gid_owner'], '--gid-owner', False) if params['jump'] is None: @@ -690,7 +694,7 @@ def construct_rule(params): params['icmp_type'], ICMP_TYPE_OPTIONS[params['ip_version']], False) - append_match(rule, params['comment'], 'comment') + append_match(rule, params['comment'], 'comment', loaded_extensions) append_param(rule, params['comment'], '--comment', False) return rule diff --git a/test/integration/targets/iptables/tasks/main.yml b/test/integration/targets/iptables/tasks/main.yml index cc3ba808273..aef2e34adf6 100644 --- a/test/integration/targets/iptables/tasks/main.yml +++ b/test/integration/targets/iptables/tasks/main.yml @@ -35,4 +35,19 @@ # prevent attempts to upgrade the kernel and install kernel modules for a non-running kernel version exclude: "{{ 'kernel-core' if ansible_distribution == 'RedHat' else omit }}" +- name: Use iptables with unnecessary extension match + iptables: + chain: INPUT + source: 8.8.8.8 + jump: DROP + match: comment + comment: Here to include an extension + register: unnecessary_extension + +- name: Assert success + assert: + that: + - unnecessary_extension is success + - unnecessary_extension.rule.count('-m comment') == 1 + - import_tasks: chain_management.yml