From d21a6aa1470cb292e0f1ce9b85e3d2ae1343db6f Mon Sep 17 00:00:00 2001 From: Yadnyawalkya Tale <10824880+Yadnyawalkya@users.noreply.github.com> Date: Tue, 7 Nov 2017 12:46:55 +0000 Subject: [PATCH] Ansible system module: sanity pep8 fixes (#32314) * Ansible system module: pep8 fixes * Clean up documentation * Blank line change not required by PEP8 * Update legacy-files.txt * Documentation updates * Update documentation * Documentation update --- lib/ansible/modules/system/aix_inittab.py | 96 ++++++++++---------- lib/ansible/modules/system/capabilities.py | 64 ++++++------- lib/ansible/modules/system/cronvar.py | 100 +++++++++------------ lib/ansible/modules/system/crypttab.py | 74 ++++++--------- test/sanity/pep8/legacy-files.txt | 4 - 5 files changed, 143 insertions(+), 195 deletions(-) diff --git a/lib/ansible/modules/system/aix_inittab.py b/lib/ansible/modules/system/aix_inittab.py index 448040caa46..5705be57347 100644 --- a/lib/ansible/modules/system/aix_inittab.py +++ b/lib/ansible/modules/system/aix_inittab.py @@ -7,18 +7,16 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type - ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} - - DOCUMENTATION = ''' --- -author: "Joris Weijters (@molekuul)" +author: +- Joris Weijters (@molekuul) module: aix_inittab -short_description: Manages the inittab on AIX. +short_description: Manages the inittab on AIX description: - Manages the inittab on AIX. version_added: "2.3" @@ -26,47 +24,47 @@ options: name: description: - Name of the inittab entry. - required: True + required: yes aliases: ['service'] runlevel: description: - Runlevel of the entry. - required: True + required: yes action: description: - Action what the init has to do with this entry. - required: True - choices: [ - 'respawn', - 'wait', - 'once', - 'boot', - 'bootwait', - 'powerfail', - 'powerwait', - 'off', - 'hold', - 'ondemand', - 'initdefault', - 'sysinit' - ] + required: yes + choices: + - boot + - bootwait + - hold + - initdefault + - off + - once + - ondemand + - powerfail + - powerwait + - respawn + - sysinit + - wait command: description: - What command has to run. - required: True + required: yes insertafter: description: - After which inittabline should the new entry inserted. state: description: - - Whether the entry should be present or absent in the inittab file - choices: [ "present", "absent" ] + - Whether the entry should be present or absent in the inittab file. + choices: [ absent, present ] default: present notes: - - The changes are persistent across reboots, you need root rights to read or adjust the inittab with the lsitab, chitab, - mkitab or rmitab commands. - - tested on AIX 7.1. -requirements: [ 'itertools'] + - The changes are persistent across reboots, you need root rights to read or adjust the inittab with the C(lsitab), chitab, + C(mkitab) or C(rmitab) commands. + - Tested on AIX 7.1. +requirements: +- itertools ''' EXAMPLES = ''' @@ -76,7 +74,7 @@ EXAMPLES = ''' name: startmyservice runlevel: 4 action: once - command: "echo hello" + command: echo hello insertafter: existingservice state: present become: yes @@ -87,17 +85,16 @@ EXAMPLES = ''' name: startmyservice runlevel: 2 action: wait - command: "echo hello" + command: echo hello state: present become: yes -# Remove inittab entry startmyservice. -- name: remove startmyservice from inittab +- name: Remove startmyservice from inittab aix_inittab: name: startmyservice runlevel: 2 action: wait - command: "echo hello" + command: echo hello state: absent become: yes ''' @@ -148,28 +145,25 @@ def main(): # initialize module = AnsibleModule( argument_spec=dict( - name=dict(required=True, type='str', aliases=['service']), - runlevel=dict(required=True, type='str'), - action=dict(choices=[ - 'respawn', - 'wait', - 'once', + name=dict(type='str', required=True, aliases=['service']), + runlevel=dict(type='str', required=True), + action=dict(type='str', choices=[ 'boot', 'bootwait', - 'powerfail', - 'powerwait', - 'off', 'hold', - 'ondemand', 'initdefault', - 'sysinit' - ], type='str'), - command=dict(required=True, type='str'), + 'off', + 'once', + 'ondemand', + 'powerfail', + 'powerwait', + 'respawn', + 'sysinit', + 'wait', + ]), + command=dict(type='str', required=True), insertafter=dict(type='str'), - state=dict(choices=[ - 'present', - 'absent', - ], required=True, type='str'), + state=dict(type='str', required=True, choices=['absent', 'present']), ), supports_check_mode=True, ) diff --git a/lib/ansible/modules/system/capabilities.py b/lib/ansible/modules/system/capabilities.py index 6c62ee520b9..022a751029f 100644 --- a/lib/ansible/modules/system/capabilities.py +++ b/lib/ansible/modules/system/capabilities.py @@ -1,18 +1,16 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# (c) 2014, Nate Coraor +# Copyright: (c) 2014, Nate Coraor # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type - ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} - DOCUMENTATION = ''' --- module: capabilities @@ -24,18 +22,16 @@ options: path: description: - Specifies the path to the file to be managed. - required: true - default: null + required: yes capability: description: - Desired capability to set (with operator and flags, if state is C(present)) or remove (if state is C(absent)) - required: true - default: null - aliases: [ 'cap' ] + required: yes + aliases: [ cap ] state: description: - Whether the entry should be present or absent in the file's capabilities. - choices: [ "present", "absent" ] + choices: [ absent, present ] default: present notes: - The capabilities system will automatically transform operators and flags @@ -43,19 +39,19 @@ notes: cap_foo+ep). This module does not attempt to determine the final operator and flags to compare, so you will want to ensure that your capabilities argument matches the final capabilities. -requirements: [] -author: "Nate Coraor (@natefoo)" +author: +- Nate Coraor (@natefoo) ''' EXAMPLES = ''' -# Set cap_sys_chroot+ep on /foo -- capabilities: +- name: Set cap_sys_chroot+ep on /foo + capabilities: path: /foo capability: cap_sys_chroot+ep state: present -# Remove cap_net_bind_service from /bar -- capabilities: +- name: Remove cap_net_bind_service from /bar + capabilities: path: /bar capability: cap_net_bind_service state: absent @@ -63,30 +59,28 @@ EXAMPLES = ''' from ansible.module_utils.basic import AnsibleModule - -OPS = ( '=', '-', '+' ) +OPS = ('=', '-', '+') class CapabilitiesModule(object): - platform = 'Linux' distribution = None def __init__(self, module): - self.module = module - self.path = module.params['path'].strip() - self.capability = module.params['capability'].strip().lower() - self.state = module.params['state'] - self.getcap_cmd = module.get_bin_path('getcap', required=True) - self.setcap_cmd = module.get_bin_path('setcap', required=True) - self.capability_tup = self._parse_cap(self.capability, op_required=self.state=='present') + self.module = module + self.path = module.params['path'].strip() + self.capability = module.params['capability'].strip().lower() + self.state = module.params['state'] + self.getcap_cmd = module.get_bin_path('getcap', required=True) + self.setcap_cmd = module.get_bin_path('setcap', required=True) + self.capability_tup = self._parse_cap(self.capability, op_required=self.state == 'present') self.run() def run(self): current = self.getcap(self.path) - caps = [ cap[0] for cap in current ] + caps = [cap[0] for cap in current] if self.state == 'present' and self.capability_tup not in current: # need to add capability @@ -96,7 +90,7 @@ class CapabilitiesModule(object): # remove from current cap list if it's already set (but op/flags differ) current = list(filter(lambda x: x[0] != self.capability_tup[0], current)) # add new cap with correct op/flags - current.append( self.capability_tup ) + current.append(self.capability_tup) self.module.exit_json(changed=True, state=self.state, msg='capabilities changed', stdout=self.setcap(self.path, current)) elif self.state == 'absent' and self.capability_tup[0] in caps: # need to remove capability @@ -130,13 +124,13 @@ class CapabilitiesModule(object): cap_group = cap.split(',') cap_group[-1], op, flags = self._parse_cap(cap_group[-1]) for subcap in cap_group: - rval.append( ( subcap, op, flags ) ) + rval.append((subcap, op, flags)) else: rval.append(self._parse_cap(cap)) return rval def setcap(self, path, caps): - caps = ' '.join([ ''.join(cap) for cap in caps ]) + caps = ' '.join([''.join(cap) for cap in caps]) cmd = "%s '%s' %s" % (self.setcap_cmd, caps, path) rc, stdout, stderr = self.module.run_command(cmd) if rc != 0: @@ -160,19 +154,19 @@ class CapabilitiesModule(object): cap, flags = cap.split(op) return (cap, op, flags) + # ============================================================== # main def main(): - # defining module module = AnsibleModule( - argument_spec = dict( - path = dict(aliases=['key'], required=True), - capability = dict(aliases=['cap'], required=True), - state = dict(default='present', choices=['present', 'absent']), + argument_spec=dict( + path=dict(type='str', required=True, aliases=['key']), + capability=dict(type='str', required=True, aliases=['cap']), + state=dict(type='str', default='present', choices=['absent', 'present']), ), - supports_check_mode=True + supports_check_mode=True, ) CapabilitiesModule(module) diff --git a/lib/ansible/modules/system/cronvar.py b/lib/ansible/modules/system/cronvar.py index 684debd8f8a..757ae2891cf 100644 --- a/lib/ansible/modules/system/cronvar.py +++ b/lib/ansible/modules/system/cronvar.py @@ -1,30 +1,25 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# -# This file is part of Ansible -# + +# Copyright: (c) 2017, Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -# # Cronvar Plugin: The goal of this plugin is to provide an idempotent # method for set cron variable values. It should play well with the # existing cron module as well as allow for manually added variables. # Each variable entered will be preceded with a comment describing the # variable so that it can be found later. This is required to be # present in order for this plugin to find/modify the variable -# + # This module is based on the crontab module. -# from __future__ import absolute_import, division, print_function __metaclass__ = type - ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} - DOCUMENTATION = """ --- module: cronvar @@ -37,69 +32,58 @@ options: name: description: - Name of the crontab variable. - default: null - required: true + required: yes value: description: - - The value to set this variable to. Required if state=present. - required: false - default: null + - The value to set this variable to. + - Required if C(state=present). insertafter: - required: false - default: null description: - - Used with C(state=present). If specified, the variable will be inserted - after the variable specified. + - If specified, the variable will be inserted after the variable specified. + - Used with C(state=present). insertbefore: - required: false - default: null description: - Used with C(state=present). If specified, the variable will be inserted just before the variable specified. state: description: - Whether to ensure that the variable is present or absent. - required: false + choices: [ absent, present ] default: present - choices: [ "present", "absent" ] user: description: - The specific user whose crontab should be modified. - required: false default: root cron_file: description: - If specified, uses this file instead of an individual user's crontab. Without a leading /, this is assumed to be in /etc/cron.d. With a leading /, this is taken as absolute. - required: false - default: null backup: description: - If set, create a backup of the crontab before it is modified. The location of the backup is returned in the C(backup) variable by this module. - required: false - default: false + type: bool + default: 'no' requirements: - cron -author: "Doug Luce (@dougluce)" +author: +- Doug Luce (@dougluce) """ EXAMPLES = ''' -# Ensure a variable exists. -# Creates an entry like "EMAIL=doug@ansibmod.con.com" -- cronvar: +- name: Ensure entry like "EMAIL=doug@ansibmod.con.com" exists + cronvar: name: EMAIL value: doug@ansibmod.con.com -# Make sure a variable is gone. This will remove any variable named -# "LEGACY" -- cronvar: +- name: Ensure a variable does not exist. This may remove any variable named "LEGACY" + cronvar: name: LEGACY state: absent -# Adds a variable to a file under /etc/cron.d -- cronvar: +- name: Add a variable to a file under /etc/cron.d + cronvar: name: LOGFILE value: /var/log/yum-autoupdate.log user: root @@ -117,7 +101,6 @@ import tempfile from ansible.module_utils.basic import AnsibleModule - CRONCMD = "/usr/bin/crontab" @@ -132,11 +115,12 @@ class CronVar(object): user - the user of the crontab (defaults to root) cron_file - a cron file under /etc/cron.d """ + def __init__(self, module, user=None, cron_file=None): self.module = module self.user = user self.lines = None - self.wordchars = ''.join(chr(x) for x in range(128) if chr(x) not in ('=', "'", '"', )) + self.wordchars = ''.join(chr(x) for x in range(128) if chr(x) not in ('=', "'", '"',)) if cron_file: self.cron_file = "" @@ -167,15 +151,14 @@ class CronVar(object): # using safely quoted shell for now, but this really should be two non-shell calls instead. FIXME (rc, out, err) = self.module.run_command(self._read_user_execute(), use_unsafe_shell=True) - if rc != 0 and rc != 1: # 1 can mean that there are no jobs. + if rc != 0 and rc != 1: # 1 can mean that there are no jobs. raise CronVarError("Unable to read crontab") lines = out.splitlines() count = 0 for l in lines: - if count > 2 or (not re.match( r'# DO NOT EDIT THIS FILE - edit the master and reinstall.', l) and - not re.match( r'# \(/tmp/.*installed on.*\)', l) and - not re.match( r'# \(.*version.*\)', l)): + if count > 2 or (not re.match(r'# DO NOT EDIT THIS FILE - edit the master and reinstall.', l + ) and not re.match(r'# \(/tmp/.*installed on.*\)', l) and not re.match(r'# \(.*version.*\)', l)): self.lines.append(l) count += 1 @@ -258,7 +241,7 @@ class CronVar(object): newlines = [] for l in self.lines: try: - (varname, _) = self.parse_for_var(l) # Throws if not a var line + (varname, _) = self.parse_for_var(l) # Throws if not a var line if varname == insertbefore: newlines.append("%s=%s" % (name, value)) newlines.append(l) @@ -266,7 +249,7 @@ class CronVar(object): newlines.append(l) newlines.append("%s=%s" % (name, value)) else: - raise CronVarError # Append. + raise CronVarError # Append. except CronVarError: newlines.append(l) @@ -279,9 +262,9 @@ class CronVar(object): newlines = [] for l in self.lines: try: - (varname, _) = self.parse_for_var(l) # Throws if not a var line + (varname, _) = self.parse_for_var(l) # Throws if not a var line if varname != name: - raise CronVarError # Append. + raise CronVarError # Append. if not remove: newlines.append("%s=%s" % (name, value)) except CronVarError: @@ -310,10 +293,10 @@ class CronVar(object): elif platform.system() == 'AIX': return "%s -l %s" % (pipes.quote(CRONCMD), pipes.quote(self.user)) elif platform.system() == 'HP-UX': - return "%s %s %s" % (CRONCMD , '-l', pipes.quote(self.user)) + return "%s %s %s" % (CRONCMD, '-l', pipes.quote(self.user)) elif pwd.getpwuid(os.getuid())[0] != self.user: user = '-u %s' % pipes.quote(self.user) - return "%s %s %s" % (CRONCMD , user, '-l') + return "%s %s %s" % (CRONCMD, user, '-l') def _write_execute(self, path): """ @@ -325,9 +308,10 @@ class CronVar(object): return "chown %s %s ; su '%s' -c '%s %s'" % (pipes.quote(self.user), pipes.quote(path), pipes.quote(self.user), CRONCMD, pipes.quote(path)) elif pwd.getpwuid(os.getuid())[0] != self.user: user = '-u %s' % pipes.quote(self.user) - return "%s %s %s" % (CRONCMD , user, pipes.quote(path)) + return "%s %s %s" % (CRONCMD, user, pipes.quote(path)) + -#================================================== +# ================================================== def main(): # The following example playbooks: @@ -346,14 +330,14 @@ def main(): module = AnsibleModule( argument_spec=dict( - name=dict(required=True), - value=dict(required=False), - user=dict(required=False), - cron_file=dict(required=False), - insertafter=dict(default=None), - insertbefore=dict(default=None), - state=dict(default='present', choices=['present', 'absent']), - backup=dict(default=False, type='bool'), + name=dict(type='str', required=True), + value=dict(type='str'), + user=dict(type='str'), + cron_file=dict(type='str'), + insertafter=dict(type='str'), + insertbefore=dict(type='str'), + state=dict(type='str', default='present', choices=['absent', 'present']), + backup=dict(type='bool', default=False), ), mutually_exclusive=[['insertbefore', 'insertafter']], supports_check_mode=False, @@ -373,7 +357,7 @@ def main(): res_args = dict() # Ensure all files generated are only writable by the owning user. Primarily relevant for the cron_file option. - os.umask(int('022',8)) + os.umask(int('022', 8)) cronvar = CronVar(module, user, cron_file) module.debug('cronvar instantiated - name: "%s"' % name) diff --git a/lib/ansible/modules/system/crypttab.py b/lib/ansible/modules/system/crypttab.py index ef6bb8e1044..92b95efe057 100644 --- a/lib/ansible/modules/system/crypttab.py +++ b/lib/ansible/modules/system/crypttab.py @@ -1,18 +1,16 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# (c) 2014, Steve +# Copyright: (c) 2014, Steve # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type - ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} - DOCUMENTATION = ''' --- module: crypttab @@ -26,9 +24,7 @@ options: - Name of the encrypted block device as it appears in the C(/etc/crypttab) file, or optionally prefixed with C(/dev/mapper/), as it appears in the filesystem. I(/dev/mapper/) will be stripped from I(name). - required: true - default: null - aliases: [] + required: yes state: description: - Use I(present) to add a line to C(/etc/crypttab) or update it's definition @@ -36,45 +32,35 @@ options: Use I(opts_present) to add options to those already present; options with different values will be updated. Use I(opts_absent) to remove options from the existing set. - required: true - choices: [ "present", "absent", "opts_present", "opts_absent"] - default: null + required: yes + choices: [ absent, opts_absent, opts_present, present ] backing_device: description: - Path to the underlying block device or file, or the UUID of a block-device - prefixed with I(UUID=) - required: false - default: null + prefixed with I(UUID=). password: description: - Encryption password, the path to a file containing the password, or - 'none' or '-' if the password should be entered at boot. - required: false - default: "none" + C(none) or C(-) if the password should be entered at boot. + default: 'none' opts: description: - A comma-delimited list of options. See C(crypttab(5) ) for details. - required: false path: description: - Path to file to use instead of C(/etc/crypttab). This might be useful in a chroot environment. - required: false default: /etc/crypttab - -notes: [] -requirements: [] -author: "Steve (@groks)" +author: +- Steve (@groks) ''' EXAMPLES = ''' - -# Since column is a special character in YAML, if your string contains a column, it's better to use quotes around the string - name: Set the options explicitly a device which must already exist crypttab: name: luks-home state: present - opts: 'discard,cipher=aes-cbc-essiv:sha256' + opts: discard,cipher=aes-cbc-essiv:sha256 - name: Add the 'discard' option to any existing options for all devices crypttab: @@ -91,30 +77,29 @@ import traceback from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_bytes, to_native -def main(): +def main(): module = AnsibleModule( - argument_spec = dict( - name = dict(required=True), - state = dict(required=True, choices=['present', 'absent', 'opts_present', 'opts_absent']), - backing_device = dict(default=None), - password = dict(default=None, type='path'), - opts = dict(default=None), - path = dict(default='/etc/crypttab', type='path') + argument_spec=dict( + name=dict(type='str', required=True), + state=dict(type='str', required=True, choices=['absent', 'opts_absent', 'opts_present', 'present']), + backing_device=dict(type='str'), + password=dict(type='path'), + opts=dict(type='str'), + path=dict(type='path', default='/etc/crypttab') ), - supports_check_mode = True + supports_check_mode=True, ) backing_device = module.params['backing_device'] - password = module.params['password'] - opts = module.params['opts'] - state = module.params['state'] - path = module.params['path'] - name = module.params['name'] + password = module.params['password'] + opts = module.params['opts'] + state = module.params['state'] + path = module.params['path'] + name = module.params['name'] if name.startswith('/dev/mapper/'): name = name[len('/dev/mapper/'):] - if state != 'absent' and backing_device is None and password is None and opts is None: module.fail_json(msg="expected one or more of 'backing_device', 'password' or 'opts'", **module.params) @@ -127,8 +112,7 @@ def main(): ('backing_device', backing_device), ('password', password), ('opts', opts)): - if (arg is not None - and (' ' in arg or '\t' in arg or arg == '')): + if (arg is not None and (' ' in arg or '\t' in arg or arg == '')): module.fail_json(msg="invalid '%s': contains white space or is empty" % arg_name, **module.params) @@ -165,7 +149,6 @@ def main(): if existing_line is not None: changed, reason = existing_line.opts.remove(opts) - if changed and not module.check_mode: try: f = open(path, 'wb') @@ -177,7 +160,6 @@ def main(): class Crypttab(object): - _lines = [] def __init__(self, path): @@ -185,7 +167,7 @@ class Crypttab(object): if not os.path.exists(path): if not os.path.exists(os.path.dirname(path)): os.makedirs(os.path.dirname(path)) - open(path,'a').close() + open(path, 'a').close() try: f = open(path, 'r') @@ -222,7 +204,6 @@ class Crypttab(object): class Line(object): - def __init__(self, line=None, name=None, backing_device=None, password=None, opts=None): self.line = line self.name = name @@ -355,8 +336,7 @@ class Options(dict): super(Options, self).__delitem__(key) def __ne__(self, obj): - return not (isinstance(obj, Options) - and sorted(self.items()) == sorted(obj.items())) + return not (isinstance(obj, Options) and sorted(self.items()) == sorted(obj.items())) def __str__(self): ret = [] diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index eaf55abbb9a..d253720f443 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -311,7 +311,3 @@ lib/ansible/modules/storage/netapp/sf_snapshot_schedule_manager.py lib/ansible/modules/storage/netapp/sf_volume_access_group_manager.py lib/ansible/modules/storage/netapp/sf_volume_manager.py lib/ansible/modules/storage/zfs/zfs.py -lib/ansible/modules/system/aix_inittab.py -lib/ansible/modules/system/capabilities.py -lib/ansible/modules/system/cronvar.py -lib/ansible/modules/system/crypttab.py