ansible/community.general
1
0
Fork 0
mirror of https://github.com/ansible-collections/community.general.git synced 2024-09-14 20:13:21 +02:00
Code Releases Activity

iptables_state: fix per-table initialization command (#2525) (#2553)

Browse source 
* refactor initialize_from_null_state()

* Use a more neutral command (iptables -L) to load per-table needed modules.

* fix 'FutureWarning: Possible nested set at position ...' (re.sub)

* fix pylints (module + action plugin)

* unsubscriptable-object
* superfluous-parens
* consider-using-in
* unused-variable
* unused-import
* no-else-break

* cleanup other internal module_args if they exist

* add changelog fragment

* Apply suggestions from code review (changelog fragment)

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove useless plugin type in changelog fragment

Co-authored-by: Amin Vakil <info@aminvakil.com>

Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Amin Vakil <info@aminvakil.com>
(cherry picked from commit 2c1ab2d384)

Co-authored-by: quidame <quidame@poivron.org>
This commit is contained in:
patchback[bot] 2021-05-18 12:27:31 +02:00 committed by GitHub
parent f1dab6d4a7
commit 62043463f3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
changelogs/fragments/2525-iptables_state-fix-initialization-command.yml Normal file
View file

@ -0,0 +1,6 @@
---
bugfixes:
- "iptables_state - fix initialization of iptables from null state when adressing
more than one table (https://github.com/ansible-collections/community.general/issues/2523)."
- "iptables_state - fix a 'FutureWarning' in a regex and do some basic code clean up
(https://github.com/ansible-collections/community.general/pull/2525)."

19
plugins/action/system/iptables_state.py
View file

@ -7,7 +7,7 @@ __metaclass__ = type
import time import time
from ansible.plugins.action import ActionBase from ansible.plugins.action import ActionBase
from ansible.errors import AnsibleError, AnsibleActionFail, AnsibleConnectionFailure from ansible.errors import AnsibleActionFail, AnsibleConnectionFailure
from ansible.utils.vars import merge_hash from ansible.utils.vars import merge_hash
from ansible.utils.display import Display from ansible.utils.display import Display
@ -46,7 +46,7 @@ class ActionModule(ActionBase):
the async wrapper results (those with the ansible_job_id key). the async wrapper results (those with the ansible_job_id key).
''' '''
# At least one iteration is required, even if timeout is 0. # At least one iteration is required, even if timeout is 0.
for i in range(max(1, timeout)): for dummy in range(max(1, timeout)):
async_result = self._execute_module( async_result = self._execute_module(
module_name='ansible.builtin.async_status', module_name='ansible.builtin.async_status',
module_args=module_args, module_args=module_args,
@ -76,7 +76,6 @@ class ActionModule(ActionBase):
task_async = self._task.async_val task_async = self._task.async_val
check_mode = self._play_context.check_mode check_mode = self._play_context.check_mode
max_timeout = self._connection._play_context.timeout max_timeout = self._connection._play_context.timeout
module_name = self._task.action
module_args = self._task.args module_args = self._task.args
if module_args.get('state', None) == 'restored': if module_args.get('state', None) == 'restored':
@ -133,7 +132,7 @@ class ActionModule(ActionBase):
# The module is aware to not process the main iptables-restore # The module is aware to not process the main iptables-restore
# command before finding (and deleting) the 'starter' cookie on # command before finding (and deleting) the 'starter' cookie on
# the host, so the previous query will not reach ssh timeout. # the host, so the previous query will not reach ssh timeout.
garbage = self._low_level_execute_command(starter_cmd, sudoable=self.DEFAULT_SUDOABLE) dummy = self._low_level_execute_command(starter_cmd, sudoable=self.DEFAULT_SUDOABLE)
# As the main command is not yet executed on the target, here # As the main command is not yet executed on the target, here
# 'finished' means 'failed before main command be executed'. # 'finished' means 'failed before main command be executed'.
@ -143,7 +142,7 @@ class ActionModule(ActionBase):
except AttributeError: except AttributeError:
pass pass
for x in range(max_timeout): for dummy in range(max_timeout):
time.sleep(1) time.sleep(1)
remaining_time -= 1 remaining_time -= 1
# - AnsibleConnectionFailure covers rejected requests (i.e. # - AnsibleConnectionFailure covers rejected requests (i.e.
@ -151,7 +150,7 @@ class ActionModule(ActionBase):
# - ansible_timeout is able to cover dropped requests (due # - ansible_timeout is able to cover dropped requests (due
# to a rule or policy DROP) if not lower than async_val. # to a rule or policy DROP) if not lower than async_val.
try: try:
garbage = self._low_level_execute_command(confirm_cmd, sudoable=self.DEFAULT_SUDOABLE) dummy = self._low_level_execute_command(confirm_cmd, sudoable=self.DEFAULT_SUDOABLE)
break break
except AnsibleConnectionFailure: except AnsibleConnectionFailure:
continue continue
@ -164,12 +163,12 @@ class ActionModule(ActionBase):
del result[key] del result[key]
if result.get('invocation', {}).get('module_args'): if result.get('invocation', {}).get('module_args'):
if '_timeout' in result['invocation']['module_args']: for key in ('_back', '_timeout', '_async_dir', 'jid'):
del result['invocation']['module_args']['_back'] if result['invocation']['module_args'].get(key):
del result['invocation']['module_args']['_timeout'] del result['invocation']['module_args'][key]
async_status_args['mode'] = 'cleanup' async_status_args['mode'] = 'cleanup'
garbage = self._execute_module( dummy = self._execute_module(
module_name='ansible.builtin.async_status', module_name='ansible.builtin.async_status',
module_args=async_status_args, module_args=async_status_args,
task_vars=task_vars, task_vars=task_vars,

47
plugins/modules/system/iptables_state.py
View file

@ -232,7 +232,7 @@ import filecmp
import shutil import shutil
from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils._text import to_bytes, to_native, to_text from ansible.module_utils._text import to_bytes, to_native
IPTABLES = dict( IPTABLES = dict(
@ -262,7 +262,7 @@ def read_state(b_path):
lines = text.splitlines() lines = text.splitlines()
while '' in lines: while '' in lines:
lines.remove('') lines.remove('')
return (lines) return lines
def write_state(b_path, lines, changed): def write_state(b_path, lines, changed):
@ -282,9 +282,9 @@ def write_state(b_path, lines, changed):
if b_destdir and not os.path.exists(b_destdir) and not module.check_mode: if b_destdir and not os.path.exists(b_destdir) and not module.check_mode:
try: try:
os.makedirs(b_destdir) os.makedirs(b_destdir)
except Exception as e: except Exception as err:
module.fail_json( module.fail_json(
msg='Error creating %s. Error code: %s. Error description: %s' % (destdir, e[0], e[1]), msg='Error creating %s: %s' % (destdir, to_native(err)),
initial_state=lines) initial_state=lines)
changed = True changed = True
@ -295,10 +295,10 @@ def write_state(b_path, lines, changed):
if changed and not module.check_mode: if changed and not module.check_mode:
try: try:
shutil.copyfile(tmpfile, b_path) shutil.copyfile(tmpfile, b_path)
except Exception as e: except Exception as err:
path = to_native(b_path, errors='surrogate_or_strict') path = to_native(b_path, errors='surrogate_or_strict')
module.fail_json( module.fail_json(
msg='Error saving state into %s. Error code: %s. Error description: %s' % (path, e[0], e[1]), msg='Error saving state into %s: %s' % (path, to_native(err)),
initial_state=lines) initial_state=lines)
return changed return changed
@ -313,14 +313,11 @@ def initialize_from_null_state(initializer, initcommand, table):
if table is None: if table is None:
table = 'filter' table = 'filter'
tmpfd, tmpfile = tempfile.mkstemp() commandline = list(initializer)
with os.fdopen(tmpfd, 'w') as f: commandline += ['-t', table]
f.write('*%s\nCOMMIT\n' % table) (rc, out, err) = module.run_command(commandline, check_rc=True)
initializer.append(tmpfile)
(rc, out, err) = module.run_command(initializer, check_rc=True)
(rc, out, err) = module.run_command(initcommand, check_rc=True) (rc, out, err) = module.run_command(initcommand, check_rc=True)
return (rc, out, err) return rc, out, err
def filter_and_format_state(string): def filter_and_format_state(string):
@ -328,13 +325,13 @@ def filter_and_format_state(string):
Remove timestamps to ensure idempotence between runs. Also remove counters Remove timestamps to ensure idempotence between runs. Also remove counters
by default. And return the result as a list. by default. And return the result as a list.
''' '''
string = re.sub('((^|\n)# (Generated|Completed)[^\n]*) on [^\n]*', '\\1', string) string = re.sub(r'((^|\n)# (Generated|Completed)[^\n]*) on [^\n]*', r'\1', string)
if not module.params['counters']: if not module.params['counters']:
string = re.sub('[[][0-9]+:[0-9]+[]]', '[0:0]', string) string = re.sub(r'\[[0-9]+:[0-9]+\]', r'[0:0]', string)
lines = string.splitlines() lines = string.splitlines()
while '' in lines: while '' in lines:
lines.remove('') lines.remove('')
return (lines) return lines
def per_table_state(command, state): def per_table_state(command, state):
@ -347,14 +344,14 @@ def per_table_state(command, state):
COMMAND = list(command) COMMAND = list(command)
if '*%s' % t in state.splitlines(): if '*%s' % t in state.splitlines():
COMMAND.extend(['--table', t]) COMMAND.extend(['--table', t])
(rc, out, err) = module.run_command(COMMAND, check_rc=True) dummy, out, dummy = module.run_command(COMMAND, check_rc=True)
out = re.sub('(^|\n)(# Generated|# Completed|[*]%s|COMMIT)[^\n]*' % t, '', out) out = re.sub(r'(^|\n)(# Generated|# Completed|[*]%s|COMMIT)[^\n]*' % t, r'', out)
out = re.sub(' *[[][0-9]+:[0-9]+[]] *', '', out) out = re.sub(r' *\[[0-9]+:[0-9]+\] *', r'', out)
table = out.splitlines() table = out.splitlines()
while '' in table: while '' in table:
table.remove('') table.remove('')
tables[t] = table tables[t] = table
return (tables) return tables
def main(): def main():
@ -402,7 +399,7 @@ def main():
changed = False changed = False
COMMANDARGS = [] COMMANDARGS = []
INITCOMMAND = [bin_iptables_save] INITCOMMAND = [bin_iptables_save]
INITIALIZER = [bin_iptables_restore] INITIALIZER = [bin_iptables, '-L', '-n']
TESTCOMMAND = [bin_iptables_restore, '--test'] TESTCOMMAND = [bin_iptables_restore, '--test']
if counters: if counters:
@ -502,7 +499,7 @@ def main():
if _back is not None: if _back is not None:
b_back = to_bytes(_back, errors='surrogate_or_strict') b_back = to_bytes(_back, errors='surrogate_or_strict')
garbage = write_state(b_back, initref_state, changed) dummy = write_state(b_back, initref_state, changed)
BACKCOMMAND = list(MAINCOMMAND) BACKCOMMAND = list(MAINCOMMAND)
BACKCOMMAND.append(_back) BACKCOMMAND.append(_back)
@ -559,9 +556,7 @@ def main():
if os.path.exists(b_starter): if os.path.exists(b_starter):
os.remove(b_starter) os.remove(b_starter)
break break
else:
time.sleep(0.01) time.sleep(0.01)
continue
(rc, stdout, stderr) = module.run_command(MAINCOMMAND) (rc, stdout, stderr) = module.run_command(MAINCOMMAND)
if 'Another app is currently holding the xtables lock' in stderr: if 'Another app is currently holding the xtables lock' in stderr:
@ -579,7 +574,7 @@ def main():
(rc, stdout, stderr) = module.run_command(SAVECOMMAND, check_rc=True) (rc, stdout, stderr) = module.run_command(SAVECOMMAND, check_rc=True)
restored_state = filter_and_format_state(stdout) restored_state = filter_and_format_state(stdout)
if restored_state != initref_state and restored_state != initial_state: if restored_state not in (initref_state, initial_state):
if module.check_mode: if module.check_mode:
changed = True changed = True
else: else:
@ -609,7 +604,7 @@ def main():
# timeout # timeout
# * task attribute 'poll' equals 0 # * task attribute 'poll' equals 0
# #
for x in range(_timeout): for dummy in range(_timeout):
if os.path.exists(b_back): if os.path.exists(b_back):
time.sleep(1) time.sleep(1)
continue continue