From 3e3f82c461a69fb35654e9e29b321fdf83bd75f0 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 14:25:07 -0300 Subject: [PATCH 01/33] Fix pylint warning W0613: unused-argument. --- .../module_utils/ansible_freeipa_module.py | 6 ++-- plugins/modules/ipadnszone.py | 12 +++---- plugins/modules/ipahost.py | 4 +-- plugins/modules/ipatrust.py | 2 +- plugins/modules/ipauser.py | 19 +++++------ plugins/modules/ipavault.py | 32 +++++++++---------- 6 files changed, 35 insertions(+), 40 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 6ce9749e..9a040ad0 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -202,11 +202,11 @@ else: if not backend.isconnected(): backend.connect(ccache=os.environ.get('KRB5CCNAME', None)) - def api_command(module, command, name, args): + def api_command(_module, command, name, args): """Call ipa.Command.""" return api.Command[command](name, **args) - def api_command_no_name(module, command, args): + def api_command_no_name(_module, command, args): """Call ipa.Command without a name.""" return api.Command[command](**args) @@ -805,7 +805,7 @@ else: self.process_command_result(name, command, args, result) self.get_command_errors(command, result) - def process_command_result(self, name, command, args, result): + def process_command_result(self, _name, _command, _args, result): """ Process an API command result. diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index 8500eb14..dbaa331b 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -298,7 +298,7 @@ class DNSZoneModule(FreeIPABaseModule): return True - def get_ipa_nsec3paramrecord(self, **kwargs): + def get_ipa_nsec3paramrecord(self, **_kwargs): nsec3param_rec = self.ipa_params.nsec3param_rec if nsec3param_rec is not None: error_msg = ( @@ -310,7 +310,7 @@ class DNSZoneModule(FreeIPABaseModule): self.fail_json(msg=error_msg) return nsec3param_rec - def get_ipa_idnsforwarders(self, **kwargs): + def get_ipa_idnsforwarders(self, **_kwargs): if self.ipa_params.forwarders is not None: forwarders = [] for forwarder in self.ipa_params.forwarders: @@ -334,14 +334,14 @@ class DNSZoneModule(FreeIPABaseModule): return forwarders - def get_ipa_idnsallowtransfer(self, **kwargs): + def get_ipa_idnsallowtransfer(self, **_kwargs): if self.ipa_params.allow_transfer is not None: error_msg = "Invalid ip_address for DNS allow_transfer: %s" self.validate_ips(self.ipa_params.allow_transfer, error_msg) return (";".join(self.ipa_params.allow_transfer) or "none") + ";" - def get_ipa_idnsallowquery(self, **kwargs): + def get_ipa_idnsallowquery(self, **_kwargs): if self.ipa_params.allow_query is not None: error_msg = "Invalid ip_address for DNS allow_query: %s" self.validate_ips(self.ipa_params.allow_query, error_msg) @@ -364,13 +364,13 @@ class DNSZoneModule(FreeIPABaseModule): return ".".join((name, domain)) - def get_ipa_idnssoarname(self, **kwargs): + def get_ipa_idnssoarname(self, **_kwargs): if self.ipa_params.admin_email is not None: return DNSName( self._replace_at_symbol_in_rname(self.ipa_params.admin_email) ) - def get_ipa_idnssoamname(self, **kwargs): + def get_ipa_idnssoamname(self, **_kwargs): if self.ipa_params.name_server is not None: return DNSName(self.ipa_params.name_server) diff --git a/plugins/modules/ipahost.py b/plugins/modules/ipahost.py index 4bf7c47c..5864e180 100644 --- a/plugins/modules/ipahost.py +++ b/plugins/modules/ipahost.py @@ -466,7 +466,7 @@ def show_host(module, name): def gen_args(description, locality, location, platform, os, password, random, mac_address, sshpubkey, userclass, auth_ind, requires_pre_auth, - ok_as_delegate, ok_to_auth_as_delegate, force, reverse, + ok_as_delegate, ok_to_auth_as_delegate, force, _reverse, ip_address, update_dns): # certificate, managedby_host, principal, create_keytab_* and # allow_retrieve_keytab_* are not handled here @@ -529,7 +529,7 @@ def gen_dnsrecord_args(module, ip_address, reverse): return _args -def check_parameters( +def check_parameters( # pylint: disable=unused-argument module, state, action, description, locality, location, platform, os, password, random, certificate, managedby_host, principal, allow_create_keytab_user, diff --git a/plugins/modules/ipatrust.py b/plugins/modules/ipatrust.py index c48dcb41..80eb3ea8 100644 --- a/plugins/modules/ipatrust.py +++ b/plugins/modules/ipatrust.py @@ -153,7 +153,7 @@ def add_trust(module, realm, args): def gen_args(trust_type, admin, password, server, trust_secret, base_id, - range_size, range_type, two_way, external): + range_size, _range_type, two_way, external): _args = {} if trust_type is not None: _args["trust_type"] = trust_type diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index 52463abb..2e80811a 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -599,17 +599,14 @@ def gen_args(first, last, fullname, displayname, initials, homedir, shell, return _args -def check_parameters(module, state, action, - first, last, fullname, displayname, initials, homedir, - shell, email, principal, principalexpiration, - passwordexpiration, password, random, uid, gid, city, - phone, mobile, pager, fax, orgunit, title, manager, - carlicense, sshpubkey, userauthtype, userclass, radius, - radiususer, departmentnumber, employeenumber, - employeetype, preferredlanguage, certificate, - certmapdata, noprivate, nomembers, preserve, - update_password): - +def check_parameters( # pylint: disable=unused-argument + module, state, action, first, last, fullname, displayname, initials, + homedir, shell, email, principal, principalexpiration, + passwordexpiration, password, random, uid, gid, city, phone, mobile, + pager, fax, orgunit, title, manager, carlicense, sshpubkey, + userauthtype, userclass, radius, radiususer, departmentnumber, + employeenumber, employeetype, preferredlanguage, certificate, + certmapdata, noprivate, nomembers, preserve, update_password): if state == "present": if action == "member": invalid = ["first", "last", "fullname", "displayname", "initials", diff --git a/plugins/modules/ipavault.py b/plugins/modules/ipavault.py index 1a4efb4b..df4bdf28 100644 --- a/plugins/modules/ipavault.py +++ b/plugins/modules/ipavault.py @@ -348,9 +348,9 @@ def find_vault(module, name, username, service, shared): return None -def gen_args(description, username, service, shared, vault_type, salt, - password, password_file, public_key, public_key_file, vault_data, - datafile_in, datafile_out): +def gen_args( + description, username, service, shared, vault_type, salt, + public_key, public_key_file): _args = {} vault_type = vault_type or to_text("symmetric") @@ -443,12 +443,12 @@ def data_storage_args(vault_type, args, data, password, password_file, return _args -def check_parameters(module, state, action, description, username, service, - shared, users, groups, services, owners, ownergroups, - ownerservices, vault_type, salt, password, password_file, - public_key, public_key_file, private_key, - private_key_file, vault_data, datafile_in, datafile_out, - new_password, new_password_file): +def check_parameters( # pylint: disable=unused-argument + module, state, action, description, username, service, shared, users, + groups, services, owners, ownergroups, ownerservices, vault_type, salt, + password, password_file, public_key, public_key_file, private_key, + private_key_file, vault_data, datafile_in, datafile_out, new_password, + new_password_file): invalid = [] if state == "present": invalid = ['datafile_out'] @@ -491,11 +491,11 @@ def check_parameters(module, state, action, description, username, service, "action '%s'" % (arg, state, action)) -def check_encryption_params(module, state, action, vault_type, salt, - password, password_file, public_key, - public_key_file, private_key, private_key_file, - vault_data, datafile_in, datafile_out, - new_password, new_password_file, res_find): +def check_encryption_params( # pylint: disable=unused-argument + module, state, action, vault_type, salt, password, password_file, + public_key, public_key_file, private_key, private_key_file, vault_data, + datafile_in, datafile_out, new_password, new_password_file, res_find): + """Check parameters used for (de)vault data encryption.""" vault_type_invalid = [] existing_type = None @@ -757,9 +757,7 @@ def main(): # Generate args args = gen_args(description, username, service, shared, vault_type, - salt, password, password_file, public_key, - public_key_file, vault_data, datafile_in, - datafile_out) + salt, public_key, public_key_file) pwdargs = None # Create command From 935956b6106327b53851b61b0cc05fec88311872 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 16:22:05 -0300 Subject: [PATCH 02/33] Fix pylint's warning `invalid-name`. --- .../module_utils/ansible_freeipa_module.py | 4 +-- plugins/modules/ipaconfig.py | 25 ++++++++----------- plugins/modules/ipadnsrecord.py | 2 +- plugins/modules/ipauser.py | 2 +- setup.cfg | 15 +++++++++++ 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 9a040ad0..a725af01 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -57,7 +57,7 @@ else: # FreeIPA releases. import re - class version: + class version: # pylint: disable=invalid-name @staticmethod def parse(version_str): """ @@ -460,7 +460,7 @@ else: cert = load_certificate(cert.encode('utf-8')) return cert - def DN_x500_text(text): + def DN_x500_text(text): # pylint: disable=invalid-name if hasattr(DN, "x500_text"): return DN(text).x500_text() else: diff --git a/plugins/modules/ipaconfig.py b/plugins/modules/ipaconfig.py index e97e3647..5e4a1bae 100644 --- a/plugins/modules/ipaconfig.py +++ b/plugins/modules/ipaconfig.py @@ -264,10 +264,7 @@ def config_show(module): def gen_args(params): - _args = {} - for k, v in params.items(): - if v is not None: - _args[k] = v + _args = {k: v for k, v in params.items() if v is not None} return _args @@ -434,7 +431,7 @@ def main(): rawresult = api_command_no_name(ansible_module, "config_show", {}) result = rawresult['result'] del result['dn'] - for key, v in result.items(): + for key, value in result.items(): k = reverse_field_map.get(key, key) if ansible_module.argument_spec.get(k): if k == 'ipaselinuxusermaporder': @@ -449,20 +446,20 @@ def main(): elif k == 'groupsearch': exit_args['groupsearch'] = \ result.get(key)[0].split(',') - elif isinstance(v, str) and \ + elif isinstance(value, str) and \ ansible_module.argument_spec[k]['type'] == "list": - exit_args[k] = [v] - elif isinstance(v, list) and \ + exit_args[k] = [value] + elif isinstance(value, list) and \ ansible_module.argument_spec[k]['type'] == "str": - exit_args[k] = ",".join(v) - elif isinstance(v, list) and \ + exit_args[k] = ",".join(value) + elif isinstance(value, list) and \ ansible_module.argument_spec[k]['type'] == "int": - exit_args[k] = ",".join(v) - elif isinstance(v, list) and \ + exit_args[k] = ",".join(value) + elif isinstance(value, list) and \ ansible_module.argument_spec[k]['type'] == "bool": - exit_args[k] = (v[0] == "TRUE") + exit_args[k] = (value[0] == "TRUE") else: - exit_args[k] = v + exit_args[k] = value except ipalib_errors.EmptyModlist: changed = False except Exception as e: diff --git a/plugins/modules/ipadnsrecord.py b/plugins/modules/ipadnsrecord.py index 0b998be6..1741a7a7 100644 --- a/plugins/modules/ipadnsrecord.py +++ b/plugins/modules/ipadnsrecord.py @@ -1377,7 +1377,7 @@ def define_commands_for_present_state(module, zone_name, entry, res_find): _args['idnsname'] = name _commands.append([zone_name, 'dnsrecord_add', _args]) # clean used fields from args - for f in part_fields: + for f in part_fields: # pylint: disable=invalid-name if f in args: del args[f] else: diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index 2e80811a..576fc59b 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -712,7 +712,7 @@ def check_certmapdata(data): return False i = data.find("", 4) - s = data.find("", i) + s = data.find("", i) # pylint: disable=invalid-name issuer = data[i+3:s] subject = data[s+3:] diff --git a/setup.cfg b/setup.cfg index 4d60e31f..d06c1495 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,3 +31,18 @@ per-file-ignores = [pydocstyle] inherit = false ignore = D1,D212,D203 +[pylint.BASIC] +good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode + +[pylint.IMPORTS] +ignored-modules = + ansible.module_utils.ansible_freeipa_module, + ipalib, ipalib.config, ipalib.constants, ipalib.krb_utils, ipalib.errors, + ipapython.ipautil, ipapython.dn, ipapython.version, ipapython.dnsutil, + ipaplatform.paths + +[pylint.REFACTORING] +max-nested-blocks = 9 + +[pylint.FORMAT] +max-line-length = 80 From efce0bdc057246fa3929b344f6a1db4a66d1e2a1 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 17:45:20 -0300 Subject: [PATCH 03/33] Disable pylint warnings for missing docstrings. --- setup.cfg | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/setup.cfg b/setup.cfg index d06c1495..061bc6be 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,6 +31,12 @@ per-file-ignores = [pydocstyle] inherit = false ignore = D1,D212,D203 +[pylint.MASTER] +disable = + missing-module-docstring, + missing-class-docstring, + missing-function-docstring, + [pylint.BASIC] good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode From fa9e11363a2d6676e9f22bf6272a4c2c2a614f19 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 17:45:48 -0300 Subject: [PATCH 04/33] Disable pylint warning for wrong import position. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 061bc6be..d415e999 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,6 +36,7 @@ disable = missing-module-docstring, missing-class-docstring, missing-function-docstring, + wrong-import-position [pylint.BASIC] good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode From b37045bd415a4dd48703d34acfab58b4ed7b5af8 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 17:57:29 -0300 Subject: [PATCH 05/33] Disable pylint duplicate code verification. Although it is an interesting setup, it currently has too many false positives, disable comments are not working for duplicate-code, and there are some expected duplications in the modules. --- setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index d415e999..aa6ed6c0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,8 @@ disable = missing-module-docstring, missing-class-docstring, missing-function-docstring, - wrong-import-position + wrong-import-position, + duplicate-code [pylint.BASIC] good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode From b3a6c9ebe1c928bc61fa99750c2855fc65ac3603 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:01:02 -0300 Subject: [PATCH 06/33] Disable pylint `broad-except` warning. This should be enabled in the future, but currently, nearly all modules rely on `Exception`, and the changes would be too invasive. --- setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index aa6ed6c0..cddb81dc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -37,7 +37,8 @@ disable = missing-class-docstring, missing-function-docstring, wrong-import-position, - duplicate-code + duplicate-code, + broad-except [pylint.BASIC] good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode From 0dabcd402fea674ac159fe4991697bf4e8347eb8 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:04:24 -0300 Subject: [PATCH 07/33] Disable pylint's `too-many-locals` and `too-many-branches`. Although both warnings are relevant, the code style choosen for ansible-freeipa currently require them to be disable. --- setup.cfg | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index cddb81dc..cbf283d1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -38,7 +38,9 @@ disable = missing-function-docstring, wrong-import-position, duplicate-code, - broad-except + broad-except, + too-many-branches, + too-many-locals [pylint.BASIC] good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode From 482bd05b629464adf0246daf5a271e1ef39454ec Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:06:52 -0300 Subject: [PATCH 08/33] Disable pylint's `protected-access` warning. Protected access is required for AnsibleModule. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index cbf283d1..aa45b22b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -37,6 +37,7 @@ disable = missing-class-docstring, missing-function-docstring, wrong-import-position, + protected-access, duplicate-code, broad-except, too-many-branches, From 59d4d1b146aa05a3c9b9b21ecbaeea5cf0ece05d Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:27:33 -0300 Subject: [PATCH 09/33] Fix or disable pylint warnings for inconsistent return. In some places, disabling the warnings rather than fixing it required less changes, without compromising readability. --- plugins/module_utils/ansible_freeipa_module.py | 7 +++++-- plugins/modules/ipadnsforwardzone.py | 4 ++-- plugins/modules/ipadnszone.py | 18 +++++++++--------- plugins/modules/ipagroup.py | 4 ++-- plugins/modules/ipahbacrule.py | 4 ++-- plugins/modules/ipahbacsvc.py | 4 ++-- plugins/modules/ipahbacsvcgroup.py | 4 ++-- plugins/modules/ipahostgroup.py | 4 ++-- plugins/modules/ipapwpolicy.py | 4 ++-- plugins/modules/ipasudocmd.py | 4 ++-- plugins/modules/ipasudorule.py | 4 ++-- plugins/modules/ipatopologysegment.py | 8 ++++---- plugins/modules/ipatrust.py | 8 ++------ plugins/modules/ipauser.py | 5 ++--- 14 files changed, 40 insertions(+), 42 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index a725af01..a1f14020 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -258,6 +258,8 @@ else: finally: temp_kdestroy(ccache_dir, ccache_name) + # fix pylint inconsistent return + return None def date_format(value): accepted_date_formats = [ @@ -536,8 +538,9 @@ else: def __getitem__(self, key): param = self.mapping[key] - if param is not None: - return _afm_convert(param) + if param is None: + return None + return _afm_convert(param) def __iter__(self): return iter(self.mapping) diff --git a/plugins/modules/ipadnsforwardzone.py b/plugins/modules/ipadnsforwardzone.py index ffebc3d2..5df7a1e8 100644 --- a/plugins/modules/ipadnsforwardzone.py +++ b/plugins/modules/ipadnsforwardzone.py @@ -135,8 +135,8 @@ def find_dnsforwardzone(module, name): msg="There is more than one dnsforwardzone '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(forwarders, forwardpolicy, skip_overlap_check): diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index dbaa331b..b9601c6a 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -298,7 +298,7 @@ class DNSZoneModule(FreeIPABaseModule): return True - def get_ipa_nsec3paramrecord(self, **_kwargs): + def get_ipa_nsec3paramrecord(self, **_kwargs): # pylint: disable=R1710 nsec3param_rec = self.ipa_params.nsec3param_rec if nsec3param_rec is not None: error_msg = ( @@ -310,7 +310,7 @@ class DNSZoneModule(FreeIPABaseModule): self.fail_json(msg=error_msg) return nsec3param_rec - def get_ipa_idnsforwarders(self, **_kwargs): + def get_ipa_idnsforwarders(self, **_kwargs): # pylint: disable=R1710 if self.ipa_params.forwarders is not None: forwarders = [] for forwarder in self.ipa_params.forwarders: @@ -334,14 +334,14 @@ class DNSZoneModule(FreeIPABaseModule): return forwarders - def get_ipa_idnsallowtransfer(self, **_kwargs): + def get_ipa_idnsallowtransfer(self, **_kwargs): # pylint: disable=R1710 if self.ipa_params.allow_transfer is not None: error_msg = "Invalid ip_address for DNS allow_transfer: %s" self.validate_ips(self.ipa_params.allow_transfer, error_msg) return (";".join(self.ipa_params.allow_transfer) or "none") + ";" - def get_ipa_idnsallowquery(self, **_kwargs): + def get_ipa_idnsallowquery(self, **_kwargs): # pylint: disable=R1710 if self.ipa_params.allow_query is not None: error_msg = "Invalid ip_address for DNS allow_query: %s" self.validate_ips(self.ipa_params.allow_query, error_msg) @@ -364,27 +364,27 @@ class DNSZoneModule(FreeIPABaseModule): return ".".join((name, domain)) - def get_ipa_idnssoarname(self, **_kwargs): + def get_ipa_idnssoarname(self, **_kwargs): # pylint: disable=R1710 if self.ipa_params.admin_email is not None: return DNSName( self._replace_at_symbol_in_rname(self.ipa_params.admin_email) ) - def get_ipa_idnssoamname(self, **_kwargs): + def get_ipa_idnssoamname(self, **_kwargs): # pylint: disable=R1710 if self.ipa_params.name_server is not None: return DNSName(self.ipa_params.name_server) - def get_ipa_skip_overlap_check(self, **kwargs): + def get_ipa_skip_overlap_check(self, **kwargs): # pylint: disable=R1710 zone = kwargs.get('zone') if not zone and self.ipa_params.skip_overlap_check is not None: return self.ipa_params.skip_overlap_check - def get_ipa_skip_nameserver_check(self, **kwargs): + def get_ipa_skip_nameserver_check(self, **kwargs): # pylint: disable=R1710 zone = kwargs.get('zone') if not zone and self.ipa_params.skip_nameserver_check is not None: return self.ipa_params.skip_nameserver_check - def __reverse_zone_name(self, ipaddress): + def __reverse_zone_name(self, ipaddress): # pylint: disable=R1710 """ Infer reverse zone name from an ip address. diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py index e7072f08..bfddebf4 100644 --- a/plugins/modules/ipagroup.py +++ b/plugins/modules/ipagroup.py @@ -201,8 +201,8 @@ def find_group(module, name): msg="There is more than one group '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description, gid, nomembers): diff --git a/plugins/modules/ipahbacrule.py b/plugins/modules/ipahbacrule.py index 12fad26c..010f68a9 100644 --- a/plugins/modules/ipahbacrule.py +++ b/plugins/modules/ipahbacrule.py @@ -175,8 +175,8 @@ def find_hbacrule(module, name): msg="There is more than one hbacrule '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description, usercategory, hostcategory, servicecategory, diff --git a/plugins/modules/ipahbacsvc.py b/plugins/modules/ipahbacsvc.py index 1ead6d53..26e6a957 100644 --- a/plugins/modules/ipahbacsvc.py +++ b/plugins/modules/ipahbacsvc.py @@ -89,8 +89,8 @@ def find_hbacsvc(module, name): msg="There is more than one hbacsvc '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description): diff --git a/plugins/modules/ipahbacsvcgroup.py b/plugins/modules/ipahbacsvcgroup.py index 6676bca7..560ebc56 100644 --- a/plugins/modules/ipahbacsvcgroup.py +++ b/plugins/modules/ipahbacsvcgroup.py @@ -121,8 +121,8 @@ def find_hbacsvcgroup(module, name): msg="There is more than one hbacsvcgroup '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description, nomembers): diff --git a/plugins/modules/ipahostgroup.py b/plugins/modules/ipahostgroup.py index 8c594353..ff13cc59 100644 --- a/plugins/modules/ipahostgroup.py +++ b/plugins/modules/ipahostgroup.py @@ -157,8 +157,8 @@ def find_hostgroup(module, name): msg="There is more than one hostgroup '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description, nomembers, rename): diff --git a/plugins/modules/ipapwpolicy.py b/plugins/modules/ipapwpolicy.py index 77fa023d..c9509f4d 100644 --- a/plugins/modules/ipapwpolicy.py +++ b/plugins/modules/ipapwpolicy.py @@ -130,8 +130,8 @@ def find_pwpolicy(module, name): msg="There is more than one pwpolicy '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(maxlife, minlife, history, minclasses, minlength, priority, diff --git a/plugins/modules/ipasudocmd.py b/plugins/modules/ipasudocmd.py index ca484ba9..08344f41 100644 --- a/plugins/modules/ipasudocmd.py +++ b/plugins/modules/ipasudocmd.py @@ -90,8 +90,8 @@ def find_sudocmd(module, name): msg="There is more than one sudocmd '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description): diff --git a/plugins/modules/ipasudorule.py b/plugins/modules/ipasudorule.py index 47735480..332423f4 100644 --- a/plugins/modules/ipasudorule.py +++ b/plugins/modules/ipasudorule.py @@ -206,8 +206,8 @@ def find_sudorule(module, name): msg="There is more than one sudorule '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def gen_args(description, usercat, hostcat, cmdcat, runasusercat, diff --git a/plugins/modules/ipatopologysegment.py b/plugins/modules/ipatopologysegment.py index 4d522bda..d2909025 100644 --- a/plugins/modules/ipatopologysegment.py +++ b/plugins/modules/ipatopologysegment.py @@ -132,8 +132,8 @@ def find_left_right(module, suffix, left, right): "not unique for suffix '%s'" % (left, right, suffix)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def find_cn(module, suffix, name): @@ -147,8 +147,8 @@ def find_cn(module, suffix, name): msg="CN '%s' is not unique for suffix '%s'" % (name, suffix)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def find_left_right_cn(module, suffix, left, right, name): diff --git a/plugins/modules/ipatrust.py b/plugins/modules/ipatrust.py index 80eb3ea8..3c14077b 100644 --- a/plugins/modules/ipatrust.py +++ b/plugins/modules/ipatrust.py @@ -125,8 +125,8 @@ def find_trust(module, realm): module.fail_json(msg="There is more than one realm '%s'" % (realm)) elif len(_result["result"]) == 1: return _result["result"][0] - else: - return None + + return None def del_trust(module, realm): @@ -136,8 +136,6 @@ def del_trust(module, realm): if len(_result["result"]["failed"]) > 0: module.fail_json( msg="Trust deletion has failed for '%s'" % (realm)) - else: - return None def add_trust(module, realm, args): @@ -148,8 +146,6 @@ def add_trust(module, realm, args): if "cn" not in _result["result"]: module.fail_json( msg="Trust add has failed for '%s'" % (realm)) - else: - return None def gen_args(trust_type, admin, password, server, trust_secret, base_id, diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index 576fc59b..23b9710e 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -512,10 +512,9 @@ def find_user(module, name, preserved=False): if certs is not None: _result["usercertificate"] = [encode_certificate(x) for x in certs] - return _result - else: - return None + + return None def gen_args(first, last, fullname, displayname, initials, homedir, shell, From f1ecc5d986cc733b0335bc984045404688aa0dc0 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:32:07 -0300 Subject: [PATCH 10/33] Disable pylint error `no-name-in-module`. All instances related to `ansible.module_utils.ansible_freeipa_module`, which works. Future occurrences, if they happen, will likely not to be a problem. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index aa45b22b..4d7011c7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -38,6 +38,7 @@ disable = missing-function-docstring, wrong-import-position, protected-access, + no-name-in-module, duplicate-code, broad-except, too-many-branches, From 87504eaa2cb54524ea9e49fae212b54cb5abba02 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:34:18 -0300 Subject: [PATCH 11/33] Disable pylint's `too-many-statements`. This is expected for most modules `main()` function. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 4d7011c7..8fd2726f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -39,6 +39,7 @@ disable = wrong-import-position, protected-access, no-name-in-module, + too-many-statements, duplicate-code, broad-except, too-many-branches, From 07abd6c12e565dd48442c170e74fe94206d9bcce Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:36:41 -0300 Subject: [PATCH 12/33] Disable pylint's `too-many-arguments`. This is a style decision for ansible-freeipa, and in use by most modules. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 8fd2726f..7609b1a5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -39,6 +39,7 @@ disable = wrong-import-position, protected-access, no-name-in-module, + too-many-arguments, too-many-statements, duplicate-code, broad-except, From 43c4a6d91fab31c9f665d60a391e70d11ce6048d Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:46:13 -0300 Subject: [PATCH 13/33] Fix or disable pylint's `no-else-return`. Fixed usage of `else` right after return, or disable pylint evaluation when it would play against code readability. --- .../module_utils/ansible_freeipa_module.py | 21 ++++++++----------- plugins/modules/ipadnsconfig.py | 4 ++-- plugins/modules/ipadnszone.py | 5 ++--- plugins/modules/ipahbacsvcgroup.py | 2 +- plugins/modules/ipaservice.py | 4 ++-- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index a1f14020..3dffd44a 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -315,7 +315,7 @@ else: filtered_args = [key for key in args if key not in ignore] for key in filtered_args: - if key not in ipa: + if key not in ipa: # pylint: disable=no-else-return module.debug( base_debug_msg + "Command key not present in IPA: %s" % key ) @@ -367,15 +367,13 @@ else: if value is not None: if isinstance(value, list): return [_afm_convert(x) for x in value] - elif isinstance(value, dict): + if isinstance(value, dict): return {_afm_convert(k): _afm_convert(v) for k, v in value.items()} - elif isinstance(value, str): + if isinstance(value, str): return to_text(value) - else: - return value - else: - return value + + return value def module_params_get(module, name): return _afm_convert(module.params.get(name)) @@ -465,11 +463,10 @@ else: def DN_x500_text(text): # pylint: disable=invalid-name if hasattr(DN, "x500_text"): return DN(text).x500_text() - else: - # Emulate x500_text - dn = DN(text) - dn.rdns = reversed(dn.rdns) - return str(dn) + # Emulate x500_text + dn = DN(text) + dn.rdns = reversed(dn.rdns) + return str(dn) def is_valid_port(port): if not isinstance(port, int): diff --git a/plugins/modules/ipadnsconfig.py b/plugins/modules/ipadnsconfig.py index a0f2bc56..8d705807 100644 --- a/plugins/modules/ipadnsconfig.py +++ b/plugins/modules/ipadnsconfig.py @@ -114,8 +114,8 @@ def find_dnsconfig(module): if _result["result"].get('idnsforwarders', None) is None: _result["result"]['idnsforwarders'] = [''] return _result["result"] - else: - module.fail_json(msg="Could not retrieve current DNS configuration.") + + module.fail_json(msg="Could not retrieve current DNS configuration.") return None diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index b9601c6a..4ed9a9d2 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -404,10 +404,9 @@ class DNSZoneModule(FreeIPABaseModule): ip_version = ip.version if ip_version == 4: return u'.'.join(items[4 - prefixlen // 8:]) - elif ip_version == 6: + if ip_version == 6: return u'.'.join(items[32 - prefixlen // 4:]) - else: - self.fail_json(msg="Invalid IP version for reverse zone.") + self.fail_json(msg="Invalid IP version for reverse zone.") def get_zone(self, zone_name): get_zone_args = {"idnsname": zone_name, "all": True} diff --git a/plugins/modules/ipahbacsvcgroup.py b/plugins/modules/ipahbacsvcgroup.py index 560ebc56..141fa539 100644 --- a/plugins/modules/ipahbacsvcgroup.py +++ b/plugins/modules/ipahbacsvcgroup.py @@ -121,7 +121,7 @@ def find_hbacsvcgroup(module, name): msg="There is more than one hbacsvcgroup '%s'" % (name)) elif len(_result["result"]) == 1: return _result["result"][0] - + return None diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 77b934c1..cd55c76b 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -250,8 +250,8 @@ def find_service(module, name): _res["usercertificate"] = [encode_certificate(cert) for cert in certs] return _res - else: - return None + + return None def gen_args(pac_type, auth_ind, skip_host_check, force, requires_pre_auth, From b5429618f16a32ccfe87d11d56cc758721841a65 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:48:07 -0300 Subject: [PATCH 14/33] Disable pylint's warnings on import order ang grouping. --- setup.cfg | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup.cfg b/setup.cfg index 7609b1a5..c6ef7173 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,6 +36,8 @@ disable = missing-module-docstring, missing-class-docstring, missing-function-docstring, + wrong-import-order, + ungrouped-imports, wrong-import-position, protected-access, no-name-in-module, From afb64419d5f6744449000bbbb929dea3ad5cc426 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:50:08 -0300 Subject: [PATCH 15/33] Disable pylint's `too-many-lines` for modules. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index c6ef7173..4525555f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -43,6 +43,7 @@ disable = no-name-in-module, too-many-arguments, too-many-statements, + too-many-lines, duplicate-code, broad-except, too-many-branches, From e7b9e97a84dd446380059b145138967c2ffcaf6e Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 18:54:33 -0300 Subject: [PATCH 16/33] Fix pylint warnings for name redefinition. --- plugins/module_utils/ansible_freeipa_module.py | 4 ++-- plugins/modules/ipaconfig.py | 6 +++--- plugins/modules/ipadnsforwardzone.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 3dffd44a..cd0a5b08 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -271,9 +271,9 @@ else: '%Y-%m-%d %H:%MZ', # non-ISO 8601, minute precision ] - for date_format in accepted_date_formats: + for _date_format in accepted_date_formats: try: - return datetime.strptime(value, date_format) + return datetime.strptime(value, _date_format) except ValueError: pass raise ValueError("Invalid date '%s'" % value) diff --git a/plugins/modules/ipaconfig.py b/plugins/modules/ipaconfig.py index 5e4a1bae..b557baf5 100644 --- a/plugins/modules/ipaconfig.py +++ b/plugins/modules/ipaconfig.py @@ -399,11 +399,11 @@ def main(): ("ipasearchrecordslimit", -1, 2147483647), ("ipapwdexpadvnotify", 0, 2147483647), ] - for arg, min, max in args_with_limits: - if arg in params and (params[arg] > max or params[arg] < min): + for arg, minimum, maximum in args_with_limits: + if arg in params and (params[arg] > maximum or params[arg] < minimum): ansible_module.fail_json( msg="Argument '%s' must be between %d and %d." - % (arg, min, max)) + % (arg, minimum, maximum)) changed = False exit_args = {} diff --git a/plugins/modules/ipadnsforwardzone.py b/plugins/modules/ipadnsforwardzone.py index 5df7a1e8..1c2a22e5 100644 --- a/plugins/modules/ipadnsforwardzone.py +++ b/plugins/modules/ipadnsforwardzone.py @@ -386,8 +386,8 @@ def main(): **exit_args) # Execute commands - for name, command, args in commands: - api_command(ansible_module, command, name, args) + for _name, command, args in commands: + api_command(ansible_module, command, _name, args) changed = True except Exception as e: From 544474a5931d01bacfeff8483b2e94f76d8be943 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:03:54 -0300 Subject: [PATCH 17/33] Disable pylint's `super-with-arguments`. We still need to support Python 2. --- plugins/module_utils/ansible_freeipa_module.py | 1 + plugins/modules/ipadnszone.py | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index cd0a5b08..9fbd5229 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -622,6 +622,7 @@ else: ipa_param_mapping = None def __init__(self, *args, **kwargs): + # pylint: disable=super-with-arguments super(FreeIPABaseModule, self).__init__(*args, **kwargs) # Attributes to store kerberos credentials (if needed) diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index 4ed9a9d2..e0dc56c9 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -504,6 +504,7 @@ class DNSZoneModule(FreeIPABaseModule): self.add_ipa_command("dnszone_mod", zone_name, args) def process_command_result(self, name, command, args, result): + # pylint: disable=super-with-arguments super(DNSZoneModule, self).process_command_result( name, command, args, result ) From 121dbe69252009c2d37a3b9d39c19823c4f20c6f Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:07:16 -0300 Subject: [PATCH 18/33] Fix pylint warning `consider-merging-isinstance`. --- plugins/modules/ipahost.py | 2 +- plugins/modules/ipauser.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ipahost.py b/plugins/modules/ipahost.py index 5864e180..0bd5b1b4 100644 --- a/plugins/modules/ipahost.py +++ b/plugins/modules/ipahost.py @@ -862,7 +862,7 @@ def main(): ok_to_auth_as_delegate, force, reverse, ip_address, update_dns, update_password) - elif isinstance(host, str) or isinstance(host, unicode): + elif isinstance(host, (str, unicode)): name = host else: ansible_module.fail_json(msg="Host '%s' is not valid" % diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index 23b9710e..0a104cb9 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -1029,7 +1029,7 @@ def main(): email = extend_emails(email, default_email_domain) - elif isinstance(user, str) or isinstance(user, unicode): + elif isinstance(user, (str, unicode)): name = user else: ansible_module.fail_json(msg="User '%s' is not valid" % From 3acb9333f4d798e75d71c78aca96bdbd6bcb100d Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:09:07 -0300 Subject: [PATCH 19/33] Disable pylint's `c-extension-no-member`. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 4525555f..dccb9fd9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -33,6 +33,7 @@ inherit = false ignore = D1,D212,D203 [pylint.MASTER] disable = + c-extension-no-member, missing-module-docstring, missing-class-docstring, missing-function-docstring, From 4f2b8000ce4e8a761a10b71b9b28056885a2dec9 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:10:28 -0300 Subject: [PATCH 20/33] Fix usage of superfluous parens. --- plugins/module_utils/ansible_freeipa_module.py | 2 +- plugins/modules/ipadnszone.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 9fbd5229..338e0f7c 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -233,7 +233,7 @@ else: "!=": operator.ne, } operation = oper_map.get(oper) - if not(operation): + if not operation: raise NotImplementedError("Invalid operator: %s" % oper) return operation(version.parse(VERSION), version.parse(requested_version)) diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index e0dc56c9..67287950 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -315,7 +315,7 @@ class DNSZoneModule(FreeIPABaseModule): forwarders = [] for forwarder in self.ipa_params.forwarders: ip_address = forwarder.get("ip_address") - if not (is_ip_address(ip_address)): + if not is_ip_address(ip_address): self.fail_json( msg="Invalid IP for DNS forwarder: %s" % ip_address ) From 14c4b60aae70374e7080f635d2f447ecf7d3ee82 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:22:57 -0300 Subject: [PATCH 21/33] Disable pylint warnings we don't care. --- setup.cfg | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index dccb9fd9..4b9ed4db 100644 --- a/setup.cfg +++ b/setup.cfg @@ -45,10 +45,12 @@ disable = too-many-arguments, too-many-statements, too-many-lines, + raise-missing-from, duplicate-code, broad-except, too-many-branches, - too-many-locals + too-many-locals, + fixme [pylint.BASIC] good-names = ex, i, j, k, Run, _, e, x, dn, cn, ip, os, unicode From b61028595849ddb85953259263df658e0c8475e8 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:39:24 -0300 Subject: [PATCH 22/33] Disable pylint warning `no-self-use` for `is_valid_nsec3param_rec`. --- plugins/modules/ipadnszone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index 67287950..07b1df81 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -263,7 +263,7 @@ class DNSZoneModule(FreeIPABaseModule): if any(invalid_ips): self.fail_json(msg=error_msg % invalid_ips) - def is_valid_nsec3param_rec(self, nsec3param_rec): + def is_valid_nsec3param_rec(self, nsec3param_rec): # pylint: disable=R0201 try: part1, part2, part3, part4 = nsec3param_rec.split(" ") except ValueError: From 95cdd43a0af38550a4309a0341a1feb01da9e502 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:56:44 -0300 Subject: [PATCH 23/33] Fix iteration over dictionaire to not use "keys()" method. --- plugins/modules/ipaconfig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ipaconfig.py b/plugins/modules/ipaconfig.py index b557baf5..dad9bd9d 100644 --- a/plugins/modules/ipaconfig.py +++ b/plugins/modules/ipaconfig.py @@ -365,7 +365,7 @@ def main(): reverse_field_map = {v: k for k, v in field_map.items()} params = {} - for x in field_map.keys(): + for x in field_map: val = module_params_get(ansible_module, x) if val is not None: From 2545f9702b46f247f308857310f7a70464428cf6 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:57:23 -0300 Subject: [PATCH 24/33] Fix excessive number of returns. --- plugins/module_utils/ansible_freeipa_module.py | 10 +--------- plugins/modules/ipadnszone.py | 16 ++++++---------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 338e0f7c..a18a89f7 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -294,16 +294,8 @@ else: # If both args and ipa are None, return there's no difference. # If only one is None, return there is a difference. # This tests avoid unecessary invalid access to attributes. - if args is None and ipa is None: - return True if args is None or ipa is None: - module.debug( - base_debug_msg + "args is%s None an ipa is%s None" % ( - "" if args is None else " not", - "" if ipa is None else " not", - ) - ) - return False + return args is None and ipa is None # Fail if args or ipa are not dicts. if not (isinstance(args, dict) and isinstance(ipa, dict)): diff --git a/plugins/modules/ipadnszone.py b/plugins/modules/ipadnszone.py index 07b1df81..ae8ed93a 100644 --- a/plugins/modules/ipadnszone.py +++ b/plugins/modules/ipadnszone.py @@ -269,16 +269,12 @@ class DNSZoneModule(FreeIPABaseModule): except ValueError: return False - if not all([part1.isdigit(), part2.isdigit(), part3.isdigit()]): - return False - - if not 0 <= int(part1) <= 255: - return False - - if not 0 <= int(part2) <= 255: - return False - - if not 0 <= int(part3) <= 65535: + if ( + not all([part1.isdigit(), part2.isdigit(), part3.isdigit()]) + or not 0 <= int(part1) <= 255 + or not 0 <= int(part2) <= 255 + or not 0 <= int(part3) <= 65535 + ): return False try: From 61c6680fdccec92010da05468e40e12e81b26b23 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 19:57:48 -0300 Subject: [PATCH 25/33] Fix unnecessary usage of `if`. --- plugins/modules/ipaservice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index cd55c76b..50081be5 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -771,7 +771,7 @@ def main(): elif state == "absent": if action == "service": if res_find is not None: - args = {'continue': True if delete_continue else False} + args = {'continue': delete_continue} commands.append([name, 'service_del', args]) elif action == "member": From 3beb041ec1d96ecb102815bec2f513b26fdcca12 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 20:00:18 -0300 Subject: [PATCH 26/33] Fix setup.cfg formatting. --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 4b9ed4db..a470e2f9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,6 +31,7 @@ per-file-ignores = [pydocstyle] inherit = false ignore = D1,D212,D203 + [pylint.MASTER] disable = c-extension-no-member, From dc9bb626f00efc5fd57250e8e4c6fb363060b490 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 20:28:23 -0300 Subject: [PATCH 27/33] Add pre-commit configuration for pylint. --- .pre-commit-config.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ec90f12a..24404caa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,6 +21,13 @@ repos: rev: 5.1.1 hooks: - id: pydocstyle +- repo: https://github.com/pycqa/pylint + rev: v2.8.2 + hooks: + - id: pylint + args: + - --disable=import-error + files: \.py$ - repo: local hooks: - id: ansible-doc-test From 9e00273864ce80e53450a5f09ad68cc57b55ac6d Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 29 Apr 2021 20:31:40 -0300 Subject: [PATCH 28/33] Add pylint to Github lint workflow. --- .github/workflows/lint.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3ddc5bd6..62564979 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -31,3 +31,8 @@ jobs: - name: Run Python linters uses: rjeffman/python-lint-action@v2 + + - name: Run pylint + run: | + pip install pylint==2.8.2 + pylint plugins --disable=import-error From a12275bc0e01cf514ec97614c1c40d80e396817a Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 18 May 2021 21:18:34 -0300 Subject: [PATCH 29/33] Fix, by disabling, pylint's error too-many-function-args (E1121). --- plugins/module_utils/ansible_freeipa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index a18a89f7..1d1b8aae 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -778,7 +778,7 @@ else: ) if len(errors) > 0: - self.fail_json(", ".join("errors")) + self.fail_json(", ".join("errors")) # pylint: disable=E1121 def add_ipa_command(self, command, name=None, args=None): """Add a command to the list of commands to be executed.""" From 9c591de3cdeda28c3b901eba257d4914307f122f Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 18 May 2021 21:27:47 -0300 Subject: [PATCH 30/33] Fix anomalous use of '\' in reguluar expression. --- plugins/module_utils/ansible_freeipa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 1d1b8aae..9d31d6bb 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -65,7 +65,7 @@ else: This will not work for `rc`, `dev` or similar version string. """ - return tuple(re.split("[-_\.]", version_str)) # noqa: W605 + return tuple(re.split("[-_.]", version_str)) # noqa: W605 from ipalib import api from ipalib import errors as ipalib_errors # noqa From bf30d4b5f8a1728a94aa10cb97ee6aae6eb83daf Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 18 May 2021 21:29:05 -0300 Subject: [PATCH 31/33] Fix, by disabling, pylint's warning on too few public methods. --- plugins/module_utils/ansible_freeipa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 9d31d6bb..f8f1e2bb 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -57,7 +57,7 @@ else: # FreeIPA releases. import re - class version: # pylint: disable=invalid-name + class version: # pylint: disable=invalid-name, too-few-public-methods @staticmethod def parse(version_str): """ From 967f9c747466951bf4a26debe768c126a225c925 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 18 May 2021 21:30:28 -0300 Subject: [PATCH 32/33] Fix, by disabling, pylint's warning on unnecessary pass. --- plugins/module_utils/ansible_freeipa_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index f8f1e2bb..71ce4063 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -693,7 +693,7 @@ else: def check_ipa_params(self): """Validate ipa_params before command is called.""" - pass + pass # pylint: disable=unnecessary-pass def define_ipa_commands(self): """Define commands that will be run in IPA server.""" From f7698271bd0a5ed18be7127ff1e994dc36a90dcc Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 25 May 2021 18:42:02 -0300 Subject: [PATCH 33/33] Enable pylint in utils/lint_check.sh The script utils/lint_check.sh should be used before push commits to the repository. This change enables pylint to be executed by the script. --- utils/lint_check.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/lint_check.sh b/utils/lint_check.sh index eba2c1d5..d7bfadc3 100755 --- a/utils/lint_check.sh +++ b/utils/lint_check.sh @@ -4,6 +4,7 @@ topdir="`dirname $(dirname $0)`" flake8 . pydocstyle . +pylint plugins ANSIBLE_LIBRARY=${ANSIBLE_LIBRARY:-"${topdir}/plugins/modules"} ANSIBLE_MODULE_UTILS=${ANSIBLE_MODULE_UTILS:-"${topdir}/plugins/module_utils"} @@ -24,5 +25,5 @@ done for dir in "${yaml_dirs[@]}" do - find "${dir}" -type f -name "*.yml" | xargs yamllint + find "${dir}" -type f -name "*.yml" | xargs yamllint done