Merge pull request #242 from seocam/lints

Add flake8 and pydocstyle lints

azure-pipelines.yml

@@ -0,0 +1,22 @@
+trigger:
+- master
+
+pool:
+  vmImage: 'ubuntu-18.04'
+
+steps:
+- task: UsePythonVersion@0
+  inputs:
+    versionSpec: '3.6'
+
+- script: python -m pip install --upgrade pip setuptools wheel
+  displayName: Install tools
+
+- script: pip install pydocstyle flake8
+  displayName: Install dependencies
+
+- script: flake8 .
+  displayName: Run flake8 checks
+
+- script: pydocstyle .
+  displayName: Verify docstings

plugins/module_utils/ansible_freeipa_module.py

@@ -63,9 +63,7 @@ if six.PY3:
 
 
 def valid_creds(module, principal):  # noqa
-    """
-    Get valid credintials matching the princial, try GSSAPI first
-    """
+    """Get valid credentials matching the princial, try GSSAPI first."""
     if "KRB5CCNAME" in os.environ:
         ccache = os.environ["KRB5CCNAME"]
         module.debug('KRB5CCNAME set to %s' % ccache)
@@ -103,9 +101,7 @@ def valid_creds(module, principal):  # noqa
 
 
 def temp_kinit(principal, password):
-    """
-    kinit with password using a temporary ccache
-    """
+    """Kinit with password using a temporary ccache."""
     if not password:
         raise RuntimeError("The password is not set")
     if not principal:
@@ -123,9 +119,7 @@ def temp_kinit(principal, password):
 
 
 def temp_kdestroy(ccache_dir, ccache_name):
-    """
-    Destroy temporary ticket and remove temporary ccache
-    """
+    """Destroy temporary ticket and remove temporary ccache."""
     if ccache_name is not None:
         run([paths.KDESTROY, '-c', ccache_name], raiseonerr=False)
     if ccache_dir is not None:
@@ -134,7 +128,12 @@ def temp_kdestroy(ccache_dir, ccache_name):
 
 def api_connect(context=None):
     """
-    Create environment, initialize api and connect to ldap2
+    Initialize IPA API with the provided context.
+
+    `context` can be any of:
+        * `server` (default)
+        * `ansible-freeipa`
+        * `cli_installer`
     """
     env = Env()
     env._bootstrap()
@@ -157,28 +156,24 @@ def api_connect(context=None):
 
 
 def api_command(module, command, name, args):
-    """
-    Call ipa.Command
-    """
+    """Call ipa.Command."""
     return api.Command[command](name, **args)
 
 
 def api_command_no_name(module, command, args):
-    """
-    Call ipa.Command without a name.
-    """
+    """Call ipa.Command without a name."""
     return api.Command[command](**args)
 
 
 def api_check_param(command, name):
-    """
-    Return if param exists in command param list
-    """
+    """Check if param exists in command param list."""
     return name in api.Command[command].params
 
 
 def execute_api_command(module, principal, password, command, name, args):
     """
+    Execute an API command.
+
     Get KRB ticket if not already there, initialize api, connect,
     execute command and destroy ticket again if it has been created also.
     """
@@ -300,10 +295,7 @@ def api_get_realm():
 
 
 def gen_add_del_lists(user_list, res_list):
-    """
-    Generate the lists for the addition and removal of members using the
-    provided user and ipa settings
-    """
+    """Generate the lists for the addition and removal of members."""
     add_list = list(set(user_list or []) - set(res_list or []))
     del_list = list(set(res_list or []) - set(user_list or []))
 
@@ -312,8 +304,9 @@ def gen_add_del_lists(user_list, res_list):
 
 def encode_certificate(cert):
     """
-    Encode a certificate using base64 with also taking FreeIPA and Python
-    versions into account
+    Encode a certificate using base64.
+
+    It also takes FreeIPA and Python versions into account.
     """
     if isinstance(cert, (str, unicode, bytes)):
         encoded = base64.b64encode(cert)
@@ -335,9 +328,7 @@ def is_valid_port(port):
 
 
 def is_ipv4_addr(ipaddr):
-    """
-    Test if figen IP address is a valid IPv4 address
-    """
+    """Test if given IP address is a valid IPv4 address."""
     try:
         socket.inet_pton(socket.AF_INET, ipaddr)
     except socket.error:
@@ -346,9 +337,7 @@ def is_ipv4_addr(ipaddr):
 
 
 def is_ipv6_addr(ipaddr):
-    """
-    Test if figen IP address is a valid IPv6 address
-    """
+    """Test if given IP address is a valid IPv6 address."""
     try:
         socket.inet_pton(socket.AF_INET6, ipaddr)
     except socket.error:

plugins/modules/ipadnsconfig.py

@@ -97,11 +97,10 @@ RETURN = """
 """
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
-    temp_kdestroy, valid_creds, api_connect, api_command, \
+    temp_kdestroy, valid_creds, api_connect, \
     api_command_no_name, compare_args_ipa, module_params_get, \
-    gen_add_del_lists, is_ipv4_addr, is_ipv6_addr, ipalib_errors
+    is_ipv4_addr, is_ipv6_addr
 
 
 def find_dnsconfig(module):

plugins/modules/ipagroup.py

@@ -221,7 +221,10 @@ def main():
     # Get parameters
 
     # general
-    ipaadmin_principal = module_params_get(ansible_module, "ipaadmin_principal")
+    ipaadmin_principal = module_params_get(
+        ansible_module,
+        "ipaadmin_principal",
+    )
     ipaadmin_password = module_params_get(ansible_module, "ipaadmin_password")
     names = module_params_get(ansible_module, "name")
 

plugins/modules/ipahost.py

@@ -875,9 +875,11 @@ def main():
                 res_find_dnsrecord = find_dnsrecord(ansible_module, name)
             except ipalib_errors.NotFound as e:
                 msg = str(e)
-                if ip_address is None and \
-                   ("DNS is not configured" in msg or \
-                    "DNS zone not found" in msg):
+                dns_not_configured = "DNS is not configured" in msg
+                dns_zone_not_found = "DNS zone not found" in msg
+                if ip_address is None and (
+                    dns_not_configured or dns_zone_not_found
+                ):
                     # IP address(es) not given and no DNS support in IPA
                     # -> Ignore failure
                     # IP address(es) not given and DNS zone is not found

plugins/modules/ipauser.py

@@ -655,9 +655,9 @@ def check_parameters(module, state, action,
 
 def extend_emails(email, default_email_domain):
     if email is not None:
-        return [ "%s@%s" % (_email, default_email_domain)
-                 if "@" not in _email else _email
-                 for _email in email]
+        return ["%s@%s" % (_email, default_email_domain)
+                if "@" not in _email else _email
+                for _email in email]
     return email
 
 

roles/ipaclient/action_plugins/ipaclient_get_otp.py

@@ -33,9 +33,7 @@ from ansible.plugins.action import ActionBase
 
 
 def run_cmd(args, stdin=None):
-    """
-    Execute an external command.
-    """
+    """Execute an external command."""
     p_in = None
     p_out = subprocess.PIPE
     p_err = subprocess.PIPE
@@ -53,8 +51,10 @@ def run_cmd(args, stdin=None):
 
 def kinit_password(principal, password, ccache_name, config):
     """
-    Perform kinit using principal/password, with the specified config file
-    and store the TGT in ccache_name.
+    Perform kinit using principal/password.
+
+    It uses the specified config file to kinit and stores the TGT
+    in ccache_name.
     """
     args = ["/usr/bin/kinit", principal, '-c', ccache_name]
     old_config = os.environ.get('KRB5_CONFIG')
@@ -71,8 +71,10 @@ def kinit_password(principal, password, ccache_name, config):
 
 def kinit_keytab(principal, keytab, ccache_name, config):
     """
-    Perform kinit using principal/keytab, with the specified config file
-    and store the TGT in ccache_name.
+    Perform kinit using principal/keytab.
+
+    It uses the specified config file to kinit and stores the TGT
+    in ccache_name.
     """
     if gssapi is None:
         raise ImportError("gssapi is not available")
@@ -126,7 +128,7 @@ class ActionModule(ActionBase):
 
     def run(self, tmp=None, task_vars=None):
         """
-        handler for credential cache transfer
+        Handle credential cache transfer.
 
         ipa* commands can either provide a password or a keytab file
         in order to authenticate on the managed node with Kerberos.
@@ -142,7 +144,6 @@ class ActionModule(ActionBase):
 
         Then the IPA commands can use this credential cache file.
         """
-
         if task_vars is None:
             task_vars = dict()
 

roles/ipaclient/files/py3test.py

@@ -1,7 +1,7 @@
 #!/usr/bin/python3
 
 # Test ipaclient python3 binding
-from ipaclient.install.client import SECURE_PATH
+from ipaclient.install.client import SECURE_PATH  # noqa: F401
 
 # Check ipapython version to be >= 4.6
 from ipapython.version import NUM_VERSION, VERSION

roles/ipaclient/library/ipaclient_api.py

@@ -100,7 +100,6 @@ def main():
 
     realm = module.params.get('realm')
     hostname = module.params.get('hostname')
-    servers = module.params.get('servers')
     debug = module.params.get('debug')
 
     host_principal = 'host/%s@%s' % (hostname, realm)

roles/ipaclient/library/ipaclient_get_facts.py

@@ -13,7 +13,7 @@ from ansible.module_utils.basic import AnsibleModule
 
 # pylint: disable=unused-import
 try:
-    from ipalib import api
+    from ipalib import api  # noqa: F401
 except ImportError:
     HAS_IPALIB = False
 else:
@@ -27,7 +27,7 @@ else:
         from ipapython import sysrestore
 
 try:
-    import ipaserver
+    import ipaserver  # noqa: F401
 except ImportError:
     HAS_IPASERVER = False
 else:
@@ -41,7 +41,7 @@ VAR_LIB_PKI_TOMCAT = "/var/lib/pki/pki-tomcat"
 def is_ntpd_configured():
     # ntpd is configured when sysrestore.state contains the line
     # [ntpd]
-    ntpd_conf_section = re.compile('^\s*\[ntpd\]\s*$')
+    ntpd_conf_section = re.compile(r'^\s*\[ntpd\]\s*$')
 
     try:
         with open(SERVER_SYSRESTORE_STATE) as f:
@@ -56,7 +56,7 @@ def is_ntpd_configured():
 def is_dns_configured():
     # dns is configured when /etc/named.conf contains the line
     # dyndb "ipa" "/usr/lib64/bind/ldap.so" {
-    bind_conf_section = re.compile('^\s*dyndb\s+"ipa"\s+"[^"]+"\s+{$')
+    bind_conf_section = re.compile(r'^\s*dyndb\s+"ipa"\s+"[^"]+"\s+{$')
 
     try:
         with open(NAMED_CONF) as f:

roles/ipaclient/library/ipaclient_get_otp.py

@@ -135,8 +135,7 @@ if six.PY3:
 
 def get_host_diff(ipa_host, module_host):
     """
-    Compares two dictionaries containing host attributes and builds a dict
-    of differences.
+    Build a dict with the differences from two host dicts.
 
     :param ipa_host: the host structure seen from IPA
     :param module_host: the target host structure seen from the module params
@@ -164,7 +163,7 @@ def get_host_diff(ipa_host, module_host):
 
 def get_module_host(module):
     """
-    Creates a structure representing the host information
+    Create a structure representing the host information.
 
     Reads the module parameters and builds the host structure as expected from
     the module
@@ -189,7 +188,7 @@ def get_module_host(module):
 
 def ensure_host_present(module, api, ipahost):
     """
-    Ensures that the host exists in IPA and has the same attributes.
+    Ensure host exists in IPA and has the same attributes.
 
     :param module: the ansible module
     :param api: IPA api handle
@@ -246,7 +245,7 @@ def ensure_host_present(module, api, ipahost):
 
 def ensure_host_absent(module, api, host):
     """
-    Ensures that the host does not exist in IPA
+    Ensure host does not exist in IPA.
 
     :param module: the ansible module
     :param api: the IPA API handle
@@ -271,9 +270,7 @@ def ensure_host_absent(module, api, host):
 
 
 def main():
-    """
-    Main routine for the ansible module.
-    """
+
     module = AnsibleModule(
         argument_spec=dict(
             principal=dict(default='admin'),
@@ -288,7 +285,6 @@ def main():
         supports_check_mode=True,
     )
 
-    principal = module.params.get('principal', 'admin')
     ccache = module.params.get('ccache')
     fqdn = unicode(module.params.get('fqdn'))
     state = module.params.get('state')

roles/ipaclient/library/ipaclient_test.py

@@ -235,7 +235,6 @@ def is_client_configured():
 
     :returns: boolean
     """
-
     return (os.path.isfile(paths.IPA_DEFAULT_CONF) and
             os.path.isfile(os.path.join(paths.IPA_CLIENT_SYSRESTORE,
                                         sysrestore.SYSRESTORE_STATEFILE)))
@@ -243,11 +242,10 @@ def is_client_configured():
 
 def get_ipa_conf():
     """
-    Return IPA configuration read from /etc/ipa/default.conf
+    Return IPA configuration read from `/etc/ipa/default.conf`.
 
     :returns: dict containing key,value
     """
-
     parser = RawConfigParser()
     parser.read(paths.IPA_DEFAULT_CONF)
     result = dict()

roles/ipareplica/files/py3test.py

@@ -2,9 +2,13 @@
 
 # Test ipaerver python3 binding
 try:
-    from ipaserver.install.server.replicainstall import install_check
+    from ipaserver.install.server.replicainstall import (  # noqa: F401
+        install_check,
+    )
 except ImportError:
-    from ipaserver.install.server.replicainstall import promote_check
+    from ipaserver.install.server.replicainstall import (  # noqa: F401
+        promote_check,
+    )
 
 # Check ipapython version to be >= 4.6
 from ipapython.version import NUM_VERSION, VERSION

roles/ipaserver/files/py3test.py

@@ -1,7 +1,7 @@
 #!/usr/bin/python3
 
 # Test ipaerver python3 binding
-from ipaserver.install.server.install import install_check
+from ipaserver.install.server.install import install_check   # noqa: F401
 
 # Check ipapython version to be >= 4.6
 from ipapython.version import NUM_VERSION, VERSION

setup.cfg

@@ -20,3 +20,13 @@ data_files =
     /usr/share/ansible/roles/ipaclient = roles/ipaclient/*
     /usr/share/ansible/roles/ipaserver = roles/ipaserver/*
     /usr/share/ansible/roles/ipareplica = roles/ipareplica/*
+
+[flake8]
+extend-ignore = E203
+per-file-ignores =
+    plugins/*:E402
+    roles/*:E402
+
+[pydocstyle]
+inherit = false
+ignore = D1,D212,D203