mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
iptables_state: fix per-table initialization command (#2525)
* 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>
This commit is contained in:
parent
f6db0745fc
commit
2c1ab2d384
3 changed files with 37 additions and 37 deletions
|
@ -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)."
|
|
@ -7,7 +7,7 @@ __metaclass__ = type
|
|||
import time
|
||||
|
||||
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.display import Display
|
||||
|
||||
|
@ -46,7 +46,7 @@ class ActionModule(ActionBase):
|
|||
the async wrapper results (those with the ansible_job_id key).
|
||||
'''
|
||||
# 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(
|
||||
module_name='ansible.builtin.async_status',
|
||||
module_args=module_args,
|
||||
|
@ -76,7 +76,6 @@ class ActionModule(ActionBase):
|
|||
task_async = self._task.async_val
|
||||
check_mode = self._play_context.check_mode
|
||||
max_timeout = self._connection._play_context.timeout
|
||||
module_name = self._task.action
|
||||
module_args = self._task.args
|
||||
|
||||
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
|
||||
# command before finding (and deleting) the 'starter' cookie on
|
||||
# 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
|
||||
# 'finished' means 'failed before main command be executed'.
|
||||
|
@ -143,7 +142,7 @@ class ActionModule(ActionBase):
|
|||
except AttributeError:
|
||||
pass
|
||||
|
||||
for x in range(max_timeout):
|
||||
for dummy in range(max_timeout):
|
||||
time.sleep(1)
|
||||
remaining_time -= 1
|
||||
# - AnsibleConnectionFailure covers rejected requests (i.e.
|
||||
|
@ -151,7 +150,7 @@ class ActionModule(ActionBase):
|
|||
# - ansible_timeout is able to cover dropped requests (due
|
||||
# to a rule or policy DROP) if not lower than async_val.
|
||||
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
|
||||
except AnsibleConnectionFailure:
|
||||
continue
|
||||
|
@ -164,12 +163,12 @@ class ActionModule(ActionBase):
|
|||
del result[key]
|
||||
|
||||
if result.get('invocation', {}).get('module_args'):
|
||||
if '_timeout' in result['invocation']['module_args']:
|
||||
del result['invocation']['module_args']['_back']
|
||||
del result['invocation']['module_args']['_timeout']
|
||||
for key in ('_back', '_timeout', '_async_dir', 'jid'):
|
||||
if result['invocation']['module_args'].get(key):
|
||||
del result['invocation']['module_args'][key]
|
||||
|
||||
async_status_args['mode'] = 'cleanup'
|
||||
garbage = self._execute_module(
|
||||
dummy = self._execute_module(
|
||||
module_name='ansible.builtin.async_status',
|
||||
module_args=async_status_args,
|
||||
task_vars=task_vars,
|
||||
|
|
|
@ -232,7 +232,7 @@ import filecmp
|
|||
import shutil
|
||||
|
||||
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(
|
||||
|
@ -262,7 +262,7 @@ def read_state(b_path):
|
|||
lines = text.splitlines()
|
||||
while '' in lines:
|
||||
lines.remove('')
|
||||
return (lines)
|
||||
return lines
|
||||
|
||||
|
||||
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:
|
||||
try:
|
||||
os.makedirs(b_destdir)
|
||||
except Exception as e:
|
||||
except Exception as err:
|
||||
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)
|
||||
changed = True
|
||||
|
||||
|
@ -295,10 +295,10 @@ def write_state(b_path, lines, changed):
|
|||
if changed and not module.check_mode:
|
||||
try:
|
||||
shutil.copyfile(tmpfile, b_path)
|
||||
except Exception as e:
|
||||
except Exception as err:
|
||||
path = to_native(b_path, errors='surrogate_or_strict')
|
||||
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)
|
||||
|
||||
return changed
|
||||
|
@ -313,14 +313,11 @@ def initialize_from_null_state(initializer, initcommand, table):
|
|||
if table is None:
|
||||
table = 'filter'
|
||||
|
||||
tmpfd, tmpfile = tempfile.mkstemp()
|
||||
with os.fdopen(tmpfd, 'w') as f:
|
||||
f.write('*%s\nCOMMIT\n' % table)
|
||||
|
||||
initializer.append(tmpfile)
|
||||
(rc, out, err) = module.run_command(initializer, check_rc=True)
|
||||
commandline = list(initializer)
|
||||
commandline += ['-t', table]
|
||||
(rc, out, err) = module.run_command(commandline, 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):
|
||||
|
@ -328,13 +325,13 @@ def filter_and_format_state(string):
|
|||
Remove timestamps to ensure idempotence between runs. Also remove counters
|
||||
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']:
|
||||
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()
|
||||
while '' in lines:
|
||||
lines.remove('')
|
||||
return (lines)
|
||||
return lines
|
||||
|
||||
|
||||
def per_table_state(command, state):
|
||||
|
@ -347,14 +344,14 @@ def per_table_state(command, state):
|
|||
COMMAND = list(command)
|
||||
if '*%s' % t in state.splitlines():
|
||||
COMMAND.extend(['--table', t])
|
||||
(rc, out, err) = module.run_command(COMMAND, check_rc=True)
|
||||
out = re.sub('(^|\n)(# Generated|# Completed|[*]%s|COMMIT)[^\n]*' % t, '', out)
|
||||
out = re.sub(' *[[][0-9]+:[0-9]+[]] *', '', out)
|
||||
dummy, out, dummy = module.run_command(COMMAND, check_rc=True)
|
||||
out = re.sub(r'(^|\n)(# Generated|# Completed|[*]%s|COMMIT)[^\n]*' % t, r'', out)
|
||||
out = re.sub(r' *\[[0-9]+:[0-9]+\] *', r'', out)
|
||||
table = out.splitlines()
|
||||
while '' in table:
|
||||
table.remove('')
|
||||
tables[t] = table
|
||||
return (tables)
|
||||
return tables
|
||||
|
||||
|
||||
def main():
|
||||
|
@ -402,7 +399,7 @@ def main():
|
|||
changed = False
|
||||
COMMANDARGS = []
|
||||
INITCOMMAND = [bin_iptables_save]
|
||||
INITIALIZER = [bin_iptables_restore]
|
||||
INITIALIZER = [bin_iptables, '-L', '-n']
|
||||
TESTCOMMAND = [bin_iptables_restore, '--test']
|
||||
|
||||
if counters:
|
||||
|
@ -502,7 +499,7 @@ def main():
|
|||
|
||||
if _back is not None:
|
||||
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.append(_back)
|
||||
|
||||
|
@ -559,9 +556,7 @@ def main():
|
|||
if os.path.exists(b_starter):
|
||||
os.remove(b_starter)
|
||||
break
|
||||
else:
|
||||
time.sleep(0.01)
|
||||
continue
|
||||
|
||||
(rc, stdout, stderr) = module.run_command(MAINCOMMAND)
|
||||
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)
|
||||
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:
|
||||
changed = True
|
||||
else:
|
||||
|
@ -609,7 +604,7 @@ def main():
|
|||
# timeout
|
||||
# * task attribute 'poll' equals 0
|
||||
#
|
||||
for x in range(_timeout):
|
||||
for dummy in range(_timeout):
|
||||
if os.path.exists(b_back):
|
||||
time.sleep(1)
|
||||
continue
|
||||
|
|
Loading…
Reference in a new issue