cmueller/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Backport/2.7/47695 - pamd fixes (#48307)

Browse source 
* pamd: fixes for multiple issues (#47695)

* Providing fix for #47083 in pamd.py

* Providing fix for #47197

* Fixing pep8 errors

* update regex to account for leading dash and VALID_TYPES with dashes as well

* use a results dictionary and clean up unnecessary items

* remove unnessecary return value. action is already reported in invocation output

* make naming consistent across action returns

* fix comparison so it checks equality instead of identity and indentation in update_rule()

* make sure file always has EOF newline

* updated regex to skip spacing between path and args and add rule arg regex to capture complex args

* new module argument parsing code in function and DRY changes

* remove unused has_rule method on PamdService class

* fix error in parse_module_arguments()

* updated args_present action to make it handle key value args and fail on complex bracketed arguments

* pep8 and other fixes so units still work

* suggested change - make version removed 2.8

Co-Authored-By: shepdelacreme <shepdelacreme@users.noreply.github.com>

* add more error proof test to if statement

(cherry picked from commit ef690e928f)

* add changelog fragment for backport

* add action return value back for backport
This commit is contained in:
Daniel Shepherd 2018-11-12 20:36:55 -05:00 committed by Toshio Kuratomi
parent f027f44ce4
commit 1abd90e3e6
2 changed files with 156 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
changelogs/fragments/47695-pamd-fix-idempotence-and-parsing-issues.yml Normal file
View file

@ -0,0 +1,5 @@
---
bugfixes:
- "pamd: update regex to allow leading dash and retain EOF newline (see https://github.com/ansible/ansible/issues/47418)"
- "pamd: fix state: updated idempotence (see https://github.com/ansible/ansible/issues/47083)"
- "pamd: fix state: args_present idempotence (see https://github.com/ansible/ansible/issues/47197)"

202
lib/ansible/modules/system/pamd.py
View file

@ -277,10 +277,14 @@ from tempfile import NamedTemporaryFile
from datetime import datetime from datetime import datetime
RULE_REGEX = re.compile(r"""(?P<rule_type>auth|account|session|password)\s+ RULE_REGEX = re.compile(r"""(?P<rule_type>-?(?:auth|account|session|password))\s+
(?P<control>\[.*\]|\S*)\s+ (?P<control>\[.*\]|\S*)\s+
(?P<path>\S*)\s? (?P<path>\S*)\s*
(?P<args>.*)""", re.X) (?P<args>.*)\s*""", re.X)
RULE_ARG_REGEX = re.compile(r"""(\[.*\]|\S*)""")
VALID_TYPES = ['account', '-account', 'auth', '-auth', 'password', '-password', 'session', '-session']
class PamdLine(object): class PamdLine(object):
@ -334,8 +338,7 @@ class PamdInclude(PamdLine):
class PamdRule(PamdLine): class PamdRule(PamdLine):
valid_types = ['account', 'auth', 'password', 'session'] valid_simple_controls = ['required', 'requisite', 'sufficient', 'optional', 'include', 'substack', 'definitive']
valid_simple_controls = ['required', 'requisite', 'sufficient', 'optional', 'include', 'substack']
valid_control_values = ['success', 'open_err', 'symbol_err', 'service_err', 'system_err', 'buf_err', valid_control_values = ['success', 'open_err', 'symbol_err', 'service_err', 'system_err', 'buf_err',
'perm_denied', 'auth_err', 'cred_insufficient', 'authinfo_unavail', 'user_unknown', 'perm_denied', 'auth_err', 'cred_insufficient', 'authinfo_unavail', 'user_unknown',
'maxtries', 'new_authtok_reqd', 'acct_expired', 'session_err', 'cred_unavail', 'maxtries', 'new_authtok_reqd', 'acct_expired', 'session_err', 'cred_unavail',
@ -364,8 +367,9 @@ class PamdRule(PamdLine):
@classmethod @classmethod
def rule_from_string(cls, line): def rule_from_string(cls, line):
match = RULE_REGEX.search(line) rule_match = RULE_REGEX.search(line)
return cls(match.group('rule_type'), match.group('control'), match.group('path'), match.group('args')) rule_args = parse_module_arguments(rule_match.group('args'))
return cls(rule_match.group('rule_type'), rule_match.group('control'), rule_match.group('path'), rule_args)
def __str__(self): def __str__(self):
if self.rule_args: if self.rule_args:
@ -394,11 +398,7 @@ class PamdRule(PamdLine):
@rule_args.setter @rule_args.setter
def rule_args(self, args): def rule_args(self, args):
if isinstance(args, str): self._args = parse_module_arguments(args)
args = args.replace(" = ", "=")
self._args = args.split(" ")
else:
self._args = args
@property @property
def line(self): def line(self):
@ -422,7 +422,7 @@ class PamdRule(PamdLine):
def validate(self): def validate(self):
# Validate the rule type # Validate the rule type
if self.rule_type not in PamdRule.valid_types: if self.rule_type not in VALID_TYPES:
return False, "Rule type, " + self.rule_type + ", is not valid in rule " + self.line return False, "Rule type, " + self.rule_type + ", is not valid in rule " + self.line
# Validate the rule control # Validate the rule control
if isinstance(self._control, str) and self.rule_control not in PamdRule.valid_simple_controls: if isinstance(self._control, str) and self.rule_control not in PamdRule.valid_simple_controls:
@ -507,26 +507,38 @@ class PamdService(object):
# Get a list of rules we want to change # Get a list of rules we want to change
rules_to_find = self.get(rule_type, rule_control, rule_path) rules_to_find = self.get(rule_type, rule_control, rule_path)
new_args = parse_module_arguments(new_args)
changes = 0
for current_rule in rules_to_find: for current_rule in rules_to_find:
rule_changed = False
if new_type: if new_type:
if(current_rule.rule_type != new_type):
rule_changed = True
current_rule.rule_type = new_type current_rule.rule_type = new_type
if new_control: if new_control:
if(current_rule.rule_control != new_control):
rule_changed = True
current_rule.rule_control = new_control current_rule.rule_control = new_control
if new_path: if new_path:
if(current_rule.rule_path != new_path):
rule_changed = True
current_rule.rule_path = new_path current_rule.rule_path = new_path
if new_args: if new_args:
if isinstance(new_args, str): if(current_rule.rule_args != new_args):
new_args = new_args.replace(" = ", "=") rule_changed = True
new_args = new_args.split(' ')
current_rule.rule_args = new_args current_rule.rule_args = new_args
return len(rules_to_find) if rule_changed:
changes += 1
return changes
def insert_before(self, rule_type, rule_control, rule_path, def insert_before(self, rule_type, rule_control, rule_path,
new_type=None, new_control=None, new_path=None, new_args=None): new_type=None, new_control=None, new_path=None, new_args=None):
# Get a list of rules we want to change # Get a list of rules we want to change
rules_to_find = self.get(rule_type, rule_control, rule_path) rules_to_find = self.get(rule_type, rule_control, rule_path)
changed = 0 changes = 0
# There are two cases to consider. # There are two cases to consider.
# 1. The new rule doesn't exist before the existing rule # 1. The new rule doesn't exist before the existing rule
# 2. The new rule exists # 2. The new rule exists
@ -552,7 +564,7 @@ class PamdService(object):
# Fourth, set the current rule's previous to the new_rule # Fourth, set the current rule's previous to the new_rule
current_rule.prev = new_rule current_rule.prev = new_rule
changed += 1 changes += 1
# Handle the case where it is the first rule in the list. # Handle the case where it is the first rule in the list.
elif previous_rule is None: elif previous_rule is None:
@ -566,15 +578,15 @@ class PamdService(object):
new_rule.prev = current_rule.prev new_rule.prev = current_rule.prev
new_rule.next = current_rule new_rule.next = current_rule
current_rule.prev = new_rule current_rule.prev = new_rule
changed += 1 changes += 1
return changed return changes
def insert_after(self, rule_type, rule_control, rule_path, def insert_after(self, rule_type, rule_control, rule_path,
new_type=None, new_control=None, new_path=None, new_args=None): new_type=None, new_control=None, new_path=None, new_args=None):
# Get a list of rules we want to change # Get a list of rules we want to change
rules_to_find = self.get(rule_type, rule_control, rule_path) rules_to_find = self.get(rule_type, rule_control, rule_path)
changed = 0 changes = 0
# There are two cases to consider. # There are two cases to consider.
# 1. The new rule doesn't exist after the existing rule # 1. The new rule doesn't exist after the existing rule
# 2. The new rule exists # 2. The new rule exists
@ -600,7 +612,7 @@ class PamdService(object):
# Fifth, set the current rule's next to the new_rule # Fifth, set the current rule's next to the new_rule
current_rule.next = new_rule current_rule.next = new_rule
changed += 1 changes += 1
# This is the case where the current_rule is the last in the list # This is the case where the current_rule is the last in the list
elif next_rule is None: elif next_rule is None:
@ -610,44 +622,88 @@ class PamdService(object):
self._tail = new_rule self._tail = new_rule
current_rule.next = new_rule current_rule.next = new_rule
changed += 1 changes += 1
return changed return changes
def add_module_arguments(self, rule_type, rule_control, rule_path, args_to_add): def add_module_arguments(self, rule_type, rule_control, rule_path, args_to_add):
# Get a list of rules we want to change # Get a list of rules we want to change
rules_to_find = self.get(rule_type, rule_control, rule_path) rules_to_find = self.get(rule_type, rule_control, rule_path)
changed = 0 args_to_add = parse_module_arguments(args_to_add)
changes = 0
for current_rule in rules_to_find: for current_rule in rules_to_find:
if isinstance(args_to_add, str): rule_changed = False
args_to_add = args_to_add.replace(" = ", "=")
args_to_add = args_to_add.split(' ') # create some structures to evaluate the situation
if not args_to_add: simple_new_args = set()
args_to_add = [] key_value_new_args = dict()
# Create a list of new args that aren't already present
new_args = [arg for arg in args_to_add if arg not in current_rule.rule_args] for arg in args_to_add:
# If there aren't any new args to add, we'll move on to the next rule if arg.startswith("["):
if not new_args:
continue continue
elif "=" in arg:
key, value = arg.split("=")
key_value_new_args[key] = value
else:
simple_new_args.add(arg)
current_rule.rule_args = current_rule.rule_args + new_args key_value_new_args_set = set(key_value_new_args)
changed += 1 simple_current_args = set()
key_value_current_args = dict()
return changed for arg in current_rule.rule_args:
if arg.startswith("["):
continue
elif "=" in arg:
key, value = arg.split("=")
key_value_current_args[key] = value
else:
simple_current_args.add(arg)
key_value_current_args_set = set(key_value_current_args)
new_args_to_add = list()
# Handle new simple arguments
if simple_new_args.difference(simple_current_args):
for arg in simple_new_args.difference(simple_current_args):
new_args_to_add.append(arg)
# Handle new key value arguments
if key_value_new_args_set.difference(key_value_current_args_set):
for key in key_value_new_args_set.difference(key_value_current_args_set):
new_args_to_add.append(key + '=' + key_value_new_args[key])
if new_args_to_add:
current_rule.rule_args += new_args_to_add
rule_changed = True
# Handle existing key value arguments when value is not equal
if key_value_new_args_set.intersection(key_value_current_args_set):
for key in key_value_new_args_set.intersection(key_value_current_args_set):
if key_value_current_args[key] != key_value_new_args[key]:
arg_index = current_rule.rule_args.index(key + '=' + key_value_current_args[key])
current_rule.rule_args[arg_index] = str(key + '=' + key_value_new_args[key])
rule_changed = True
if rule_changed:
changes += 1
return changes
def remove_module_arguments(self, rule_type, rule_control, rule_path, args_to_remove): def remove_module_arguments(self, rule_type, rule_control, rule_path, args_to_remove):
# Get a list of rules we want to change # Get a list of rules we want to change
rules_to_find = self.get(rule_type, rule_control, rule_path) rules_to_find = self.get(rule_type, rule_control, rule_path)
changed = 0 args_to_remove = parse_module_arguments(args_to_remove)
changes = 0
for current_rule in rules_to_find: for current_rule in rules_to_find:
if isinstance(args_to_remove, str):
args_to_remove = args_to_remove.replace(" = ", "=")
args_to_remove = args_to_remove.split(' ')
if not args_to_remove: if not args_to_remove:
args_to_remove = [] args_to_remove = []
@ -660,9 +716,9 @@ class PamdService(object):
# to remove. # to remove.
current_rule.rule_args = [arg for arg in current_rule.rule_args if arg not in args_to_remove] current_rule.rule_args = [arg for arg in current_rule.rule_args if arg not in args_to_remove]
changed += 1 changes += 1
return changed return changes
def validate(self): def validate(self):
current_line = self._head current_line = self._head
@ -686,7 +742,28 @@ class PamdService(object):
lines.insert(1, "# Updated by Ansible - " + datetime.now().isoformat()) lines.insert(1, "# Updated by Ansible - " + datetime.now().isoformat())
return '\n'.join(lines) return '\n'.join(lines) + '\n'
def parse_module_arguments(module_arguments):
# Return empty list if we have no args to parse
if not module_arguments:
return []
elif isinstance(module_arguments, list) and len(module_arguments) == 1 and not module_arguments[0]:
return []
if not isinstance(module_arguments, list):
module_arguments = [module_arguments]
parsed_args = list()
for arg in module_arguments:
for item in filter(None, RULE_ARG_REGEX.findall(arg)):
if not item.startswith("["):
re.sub("\\s*=\\s*", "=", item)
parsed_args.append(item)
return parsed_args
def main(): def main():
@ -695,13 +772,11 @@ def main():
argument_spec=dict( argument_spec=dict(
name=dict(required=True, type='str'), name=dict(required=True, type='str'),
type=dict(required=True, type=dict(required=True,
choices=['account', 'auth', choices=VALID_TYPES),
'password', 'session']),
control=dict(required=True, type='str'), control=dict(required=True, type='str'),
module_path=dict(required=True, type='str'), module_path=dict(required=True, type='str'),
new_type=dict(required=False, new_type=dict(required=False,
choices=['account', 'auth', choices=VALID_TYPES),
'password', 'session']),
new_control=dict(required=False, type='str'), new_control=dict(required=False, type='str'),
new_module_path=dict(required=False, type='str'), new_module_path=dict(required=False, type='str'),
module_arguments=dict(required=False, type='list'), module_arguments=dict(required=False, type='list'),
@ -726,7 +801,7 @@ def main():
) )
content = str() content = str()
fname = os.path.join(module.params["path"], module.params["name"]) fname = os.path.join(module.params["path"], module.params["name"])
backupdest = ""
# Open the file and read the content or fail # Open the file and read the content or fail
try: try:
with open(fname, 'r') as service_file_obj: with open(fname, 'r') as service_file_obj:
@ -742,6 +817,8 @@ def main():
# Set the action # Set the action
action = module.params['state'] action = module.params['state']
changes = 0
# Take action # Take action
if action == 'updated': if action == 'updated':
changes = service.update_rule(module.params['type'], module.params['control'], module.params['module_path'], changes = service.update_rule(module.params['type'], module.params['control'], module.params['module_path'],
@ -759,6 +836,9 @@ def main():
changes = service.remove_module_arguments(module.params['type'], module.params['control'], module.params['module_path'], changes = service.remove_module_arguments(module.params['type'], module.params['control'], module.params['module_path'],
module.params['module_arguments']) module.params['module_arguments'])
elif action == 'args_present': elif action == 'args_present':
if [arg for arg in parse_module_arguments(module.params['module_arguments']) if arg.startswith("[")]:
module.fail_json(msg="Unable to process bracketed '[' complex arguments with 'args_present'. Please use 'updated'.")
changes = service.add_module_arguments(module.params['type'], module.params['control'], module.params['module_path'], changes = service.add_module_arguments(module.params['type'], module.params['control'], module.params['module_path'],
module.params['module_arguments']) module.params['module_arguments'])
elif action == 'absent': elif action == 'absent':
@ -770,12 +850,18 @@ def main():
if not valid: if not valid:
module.fail_json(msg=msg) module.fail_json(msg=msg)
result = dict(
changed=(changes > 0),
change_count=changes,
backupdest='',
action=action,
)
# If not check mode and something changed, backup the original if necessary then write out the file or fail # If not check mode and something changed, backup the original if necessary then write out the file or fail
if not module.check_mode and changes > 0: if not module.check_mode and result['changed']:
pamd_file = os.path.realpath(fname)
# First, create a backup if desired. # First, create a backup if desired.
if module.params['backup']: if module.params['backup']:
backupdest = module.backup_local(fname) result['backupdest'] = module.backup_local(fname)
try: try:
temp_file = NamedTemporaryFile(mode='w', dir=module.tmpdir, delete=False) temp_file = NamedTemporaryFile(mode='w', dir=module.tmpdir, delete=False)
with open(temp_file.name, 'w') as fd: with open(temp_file.name, 'w') as fd:
@ -785,15 +871,9 @@ def main():
module.fail_json(msg='Unable to create temporary \ module.fail_json(msg='Unable to create temporary \
file %s' % temp_file) file %s' % temp_file)
module.atomic_move(temp_file.name, pamd_file) module.atomic_move(temp_file.name, os.path.realpath(fname))
facts = {} module.exit_json(**result)
facts['pamd'] = {'changed': changes > 0,
'change_count': changes,
'action': action,
'backupdest': backupdest}
module.exit_json(changed=changes > 0, ansible_facts=facts)
if __name__ == '__main__': if __name__ == '__main__':