URL: https://github.com/freeipa/freeipa/pull/1443
Author: tiran
 Title: #1443: [Backport][ipa-4-6] Fix warnings and errors found by LGTM static 
code analyzer
Action: opened

PR body:
"""
This PR was opened automatically because PR #1429 was pushed to master and 
backport to ipa-4-6 is required.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1443/head:pr1443
git checkout pr1443
From 418ff56c4118579367b95a9a473661e2227ca1a2 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 09:59:37 +0100
Subject: [PATCH 01/10] LGTM: Silence unmatchable dollar

Silence false positive "unmatchable dollar in regular expression".

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaserver/install/dsinstance.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 9c15d721fe..892057a5dd 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -1303,4 +1303,8 @@ def write_certmap_conf(realm, ca_subject):
     shutil.copyfile(
         os.path.join(paths.USR_SHARE_IPA_DIR, "certmap.conf.template"),
         certmap_filename)
-    installutils.update_file(certmap_filename, '$ISSUER_DN', str(ca_subject))
+    installutils.update_file(
+        certmap_filename,
+        '$ISSUER_DN',  # lgtm [py/regex/unmatchable-dollar]
+        str(ca_subject)
+    )

From 462d59d53287100beb34c1b90b8ac1b6b2d4ca58 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 10:08:50 +0100
Subject: [PATCH 02/10] LGTM: Use of exit() or quit()

Replace exit() with sys.exit(). exit() or quit() may fail if the interpreter
is run with the -S option.

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipalib/cli.py                   |  6 +++---
 ipaserver/install/cainstance.py |  2 +-
 ipatests/ipa-test-config        | 26 ++++++++++++++++----------
 ipatests/ipa-test-task          |  2 +-
 makeaci                         |  2 +-
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 625096a383..2da641046f 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -956,13 +956,13 @@ def run(self, filename=None, **options):
             try:
                 script = open(filename)
             except IOError as e:
-                exit("%s: %s" % (e.filename, e.strerror))
+                sys.exit("%s: %s" % (e.filename, e.strerror))
             try:
                 with script:
                     exec(script, globals(), local)
             except Exception:
                 traceback.print_exc()
-                exit(1)
+                sys.exit(1)
         else:
             code.interact(
                 '(Custom IPA interactive Python console)',
@@ -1121,7 +1121,7 @@ def get_command(self, argv):
             self.Command.help(outfile=sys.stderr)
             print(file=sys.stderr)
             print('Error: Command not specified', file=sys.stderr)
-            exit(2)
+            sys.exit(2)
         (key, argv) = (argv[0], argv[1:])
         name = from_cli(key)
         if name not in self.Command and len(argv) == 0:
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 68cc77b9ff..20bca078c0 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1370,7 +1370,7 @@ def replica_ca_install_check(config, promote):
             'Please run copy-schema-to-ca.py on all CA masters.\n'
             'If you are certain that this is a false positive, use '
             '--skip-schema-check.')
-        exit('IPA schema missing on master CA directory server')
+        sys.exit('IPA schema missing on master CA directory server')
 
 
 def __update_entry_from_cert(make_filter, make_entry, cert):
diff --git a/ipatests/ipa-test-config b/ipatests/ipa-test-config
index 3d1ff757b8..b03a2bb7dc 100755
--- a/ipatests/ipa-test-config
+++ b/ipatests/ipa-test-config
@@ -105,7 +105,7 @@ def get_object(conf, args):
         try:
             return conf.host_by_name(args.host)
         except LookupError:
-            exit('Host %s not found in config. Try --global' % args.host)
+            sys.exit('Host %s not found in config. Try --global' % args.host)
     else:
         if args.domain:
             try:
@@ -113,18 +113,18 @@ def get_object(conf, args):
             except ValueError:
                 domains = [d for d in conf.domains if d.name == args.domain]
                 if not domains:
-                    exit('Domain %s not found' % args.domain)
+                    sys.exit('Domain %s not found' % args.domain)
                 domain = domains[0]
             else:
                 try:
                     domain = conf.domains[num]
                 except LookupError:
-                    exit('Domain %s not found.' % args.domain)
+                    sys.exit('Domain %s not found.' % args.domain)
         else:
             try:
                 domain = conf.domains[0]
             except IndexError:
-                exit('No domains are configured.')
+                sys.exit('No domains are configured.')
         if args.master:
             return domain.master
 
@@ -133,23 +133,29 @@ def get_object(conf, args):
             try:
                 return domain.replicas[args.replica]
             except LookupError:
-                exit('Domain %s not found in domain %s' % (args.replica,
-                                                           domain.name))
+                sys.exit(
+                    'Domain %s not found in domain %s'
+                    % (args.replica, domain.name)
+                )
 
         elif args.client:
             num = int(args.client) - 1
             try:
                 return domain.replicas[args.client]
             except LookupError:
-                exit('Client %s not found in domain %s' % (args.client,
-                                                           domain.name))
+                sys.exit(
+                    'Client %s not found in domain %s'
+                    % (args.client, domain.name)
+                )
 
         elif args.role:
             try:
                 return domain.get_host_by_role(args.role)
             except LookupError:
-                exit('No host with role %s not found in domain %s'
-                     % (args.role, domain.name))
+                sys.exit(
+                    'No host with role %s not found in domain %s'
+                    % (args.role, domain.name)
+                )
 
         else:
             return domain
diff --git a/ipatests/ipa-test-task b/ipatests/ipa-test-task
index 3342233894..987149609f 100755
--- a/ipatests/ipa-test-task
+++ b/ipatests/ipa-test-task
@@ -453,4 +453,4 @@ class TaskRunner(object):
         tasks.add_a_record(master, host)
 
 if __name__ == '__main__':
-    exit(TaskRunner().main(sys.argv[1:]))
+    sys.exit(TaskRunner().main(sys.argv[1:]))
diff --git a/makeaci b/makeaci
index ff46f1ec09..4d2c75bcbc 100755
--- a/makeaci
+++ b/makeaci
@@ -121,7 +121,7 @@ def main(options):
             print(file=sys.stderr)
             print('Managed permission ACI validation failed.', file=sys.stderr)
             print('Re-check permission changes and run `makeaci`.', file=sys.stderr)
-            exit('%s validation failed' % options.filename)
+            sys.exit('%s validation failed' % options.filename)
     else:
         with open(options.filename, 'w') as file:
             file.writelines(output_lines)

From bf67f7725ad3ec034be5e86f82d10a6281b27c7e Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 10:14:03 +0100
Subject: [PATCH 03/10] LGTM: Name unused variable in loop

For loop variable '_nothing' is not used in the loop body. The name
'unused' is used to indicate that a variable is unused.

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipapython/install/common.py | 4 ++--
 pylintrc                    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipapython/install/common.py b/ipapython/install/common.py
index 3ec14141ae..05dfadfcb1 100644
--- a/ipapython/install/common.py
+++ b/ipapython/install/common.py
@@ -63,7 +63,7 @@ def parent(self):
         raise AttributeError('parent')
 
     def _install(self):
-        for _nothing in self._installer(self.parent):
+        for unused in self._installer(self.parent):
             yield from_(super(Step, self)._install())
 
     @staticmethod
@@ -71,7 +71,7 @@ def _installer(obj):
         yield
 
     def _uninstall(self):
-        for _nothing in self._uninstaller(self.parent):
+        for unused in self._uninstaller(self.parent):
             yield from_(super(Step, self)._uninstall())
 
     @staticmethod
diff --git a/pylintrc b/pylintrc
index 8cd7c870bf..9fad4d9472 100644
--- a/pylintrc
+++ b/pylintrc
@@ -110,7 +110,7 @@ msg-template='{path}:{line}: [{msg_id}({symbol}), {obj}] {msg})'
 
 
 [VARIABLES]
-dummy-variables-rgx=_.+
+dummy-variables-rgx=(_.+|unused)
 
 
 [IPA]

From 2e69284ce044b68fd686aac83592c87ea7a05ef1 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 10:17:40 +0100
Subject: [PATCH 04/10] LGTM: Membership test with a non-container

Silence false positive by using isinstance(value, dict).

Also clean up and optimize most common cases.

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipalib/rpc.py | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index d868518aff..539d4cfc9f 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -223,25 +223,29 @@ def xml_unwrap(value, encoding='UTF-8'):
     :param value: The value to unwrap.
     :param encoding: The Unicode encoding to use (defaults to ``'UTF-8'``).
     """
-    if type(value) in (list, tuple):
+    if isinstance(value, (unicode, int, float, bool)):
+        # most common first
+        return value
+    elif value is None:
+        return value
+    elif isinstance(value, bytes):
+        return value.decode(encoding)
+    elif isinstance(value, (list, tuple)):
         return tuple(xml_unwrap(v, encoding) for v in value)
-    if type(value) is dict:
+    elif isinstance(value, dict):
         if '__dns_name__' in value:
             return DNSName(value['__dns_name__'])
         else:
             return dict(
                 (k, xml_unwrap(v, encoding)) for (k, v) in value.items()
             )
-    if isinstance(value, bytes):
-        return value.decode(encoding)
-    if isinstance(value, Binary):
+    elif isinstance(value, Binary):
         assert type(value.data) is bytes
         return value.data
-    if isinstance(value, DateTime):
+    elif isinstance(value, DateTime):
         # xmlprc DateTime is converted to string of %Y%m%dT%H:%M:%S format
         return datetime.datetime.strptime(str(value), "%Y%m%dT%H:%M:%S")
-    assert type(value) in (unicode, int, float, bool, type(None))
-    return value
+    raise TypeError(value)
 
 
 def xml_dumps(params, version, methodname=None, methodresponse=False,

From 2cd692521622cd87e1f1cfd22b48ec6ac7c87a77 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 10:21:46 +0100
Subject: [PATCH 05/10] LGTM: Fix exception in permission_del

Instantiating an exception, but not raising it, has no effect.

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaserver/plugins/permission.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/plugins/permission.py b/ipaserver/plugins/permission.py
index a91426afd2..49ac7cfcad 100644
--- a/ipaserver/plugins/permission.py
+++ b/ipaserver/plugins/permission.py
@@ -1072,7 +1072,7 @@ def pre_callback(self, ldap, dn, *keys, **options):
         try:
             self.obj.remove_aci(entry)
         except errors.NotFound:
-            errors.NotFound(
+            raise errors.NotFound(
                 reason=_('ACI of permission %s was not found') % keys[0])
 
         return dn

From 496ae584e9bb3827968325070cd9a3c0a78d1b0a Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 10:22:56 +0100
Subject: [PATCH 06/10] LGTM: Remove redundant assignment

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipapython/certdb.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ipapython/certdb.py b/ipapython/certdb.py
index 6d4415b53b..ef0097f339 100644
--- a/ipapython/certdb.py
+++ b/ipapython/certdb.py
@@ -225,8 +225,6 @@ def __init__(self, nssdir=None, dbtype='auto'):
             self._is_temporary = True
             if dbtype == 'auto':
                 dbtype = constants.NSS_DEFAULT_DBTYPE
-            else:
-                dbtype = dbtype
         else:
             self.secdir = nssdir
             self._is_temporary = False

From 97571df703433e62d934d3e23d83e05bd66d1042 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 11:09:41 +0100
Subject: [PATCH 07/10] LGTM: Fix multiple use before assignment

- Move assignment before try/finally block
- Add raise to indicate control flow change
- Add default value

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 install/tools/ipa-custodia-check             | 26 ++++++++++++++++++--------
 ipaclient/install/client.py                  | 16 +++++++++-------
 ipaserver/install/ipa_cacert_manage.py       |  6 ++----
 ipaserver/install/ipa_replica_prepare.py     |  2 ++
 ipaserver/install/server/install.py          |  2 ++
 ipaserver/install/server/replicainstall.py   |  3 +++
 ipaserver/plugins/trust.py                   |  8 +++++---
 ipaserver/secrets/store.py                   |  4 ++--
 ipatests/pytest_plugins/integration/tasks.py |  3 ++-
 9 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/install/tools/ipa-custodia-check b/install/tools/ipa-custodia-check
index 4e46b7dc2c..c94bc67eb0 100755
--- a/install/tools/ipa-custodia-check
+++ b/install/tools/ipa-custodia-check
@@ -191,14 +191,16 @@ class IPACustodiaTester(object):
             pkey = JWK(**dictkeys[usage_id])
             local_pubkey = json_decode(pkey.export_public())
         except Exception:
-            self.error("Failed to load and parse local JWK.", fatal=True)
+            raise self.error(
+                "Failed to load and parse local JWK.", fatal=True
+            )
         else:
             self.info("Loaded key for usage '{}' from '{}'.".format(
                 usage, IPA_CUSTODIA_KEYFILE
             ))
 
         if pkey.key_id != self.host_spn:
-            self.error(
+            raise self.error(
                 "KID '{}' != host service principal name '{}' "
                 "(usage: {})".format(pkey.key_id, self.host_spn, usage),
                 fatal=True
@@ -215,8 +217,11 @@ class IPACustodiaTester(object):
         try:
             host_pubkey = json_decode(find_key(self.host_spn, usage_id))
         except Exception:
-            self.error("Fetching host keys {} (usage: {}) failed.".format(
-                self.host_spn, usage), fatal=True)
+            raise self.error(
+                "Fetching host keys {} (usage: {}) failed.".format(
+                    self.host_spn, usage),
+                fatal=True
+            )
         else:
             self.info("Checked host LDAP keys '{}' for usage {}.".format(
                 self.host_spn, usage
@@ -225,8 +230,10 @@ class IPACustodiaTester(object):
         if host_pubkey != local_pubkey:
             self.debug("LDAP: '{}'".format(host_pubkey))
             self.debug("Local: '{}'".format(local_pubkey))
-            self.error(
-                "Host key in LDAP does not match local key.", fatal=True)
+            raise self.error(
+                "Host key in LDAP does not match local key.",
+                fatal=True
+            )
         else:
             self.info(
                 "Local key for usage '{}' matches key in LDAP.".format(usage)
@@ -235,8 +242,11 @@ class IPACustodiaTester(object):
         try:
             server_pubkey = json_decode(find_key(self.server_spn, usage_id))
         except Exception:
-            self.error("Fetching server keys {} (usage: {}) failed.".format(
-                self.server_spn, usage), fatal=True)
+            raise self.error(
+                "Fetching server keys {} (usage: {}) failed.".format(
+                    self.server_spn, usage),
+                fatal=True
+            )
         else:
             self.info("Checked server LDAP keys '{}' for usage {}.".format(
                 self.server_spn, usage
diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index 6fa42e3665..c565871b9c 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -1275,9 +1275,11 @@ def update_dns(server, hostname, options):
         ips = get_local_ipaddresses()
     except CalledProcessError as e:
         logger.error("Cannot update DNS records. %s", e)
-        logger.debug("Unable to get local IP addresses.")
+        ips = None
 
     if options.all_ip_addresses:
+        if ips is None:
+            raise RuntimeError("Unable to get local IP addresses.")
         update_ips = ips
     elif options.ip_addresses:
         update_ips = []
@@ -1777,8 +1779,8 @@ def http_url():
                                       override)
         else:
             # Auth with user credentials
+            url = ldap_url()
             try:
-                url = ldap_url()
                 ca_certs = get_ca_certs_from_ldap(server, basedn, realm)
                 validate_new_ca_certs(existing_ca_certs, ca_certs, interactive)
             except errors.FileError as e:
@@ -1821,7 +1823,7 @@ def http_url():
 
         if ca_certs is None and existing_ca_certs is None:
             raise errors.InternalError(u"expected CA cert file '%s' to "
-                                       u"exist, but it's absent" % (ca_file))
+                                       u"exist, but it's absent" % ca_file)
 
     if ca_certs is not None:
         try:
@@ -2427,9 +2429,10 @@ def _install(options):
     if not options.on_master:
         nolog = tuple()
         # First test out the kerberos configuration
+        fd, krb_name = tempfile.mkstemp()
+        os.close(fd)
+        ccache_dir = tempfile.mkdtemp(prefix='krbcc')
         try:
-            (krb_fd, krb_name) = tempfile.mkstemp()
-            os.close(krb_fd)
             configure_krb5_conf(
                 cli_realm=cli_realm,
                 cli_domain=cli_domain,
@@ -2442,7 +2445,6 @@ def _install(options):
                 configure_sssd=options.sssd,
                 force=options.force)
             env['KRB5_CONFIG'] = krb_name
-            ccache_dir = tempfile.mkdtemp(prefix='krbcc')
             ccache_name = os.path.join(ccache_dir, 'ccache')
             join_args = [paths.SBIN_IPA_JOIN,
                          "-s", cli_server[0],
@@ -2799,7 +2801,7 @@ def _install(options):
     nscd = services.knownservices.nscd
     if nscd.is_installed():
         save_state(nscd, statestore)
-
+        nscd_service_action = None
         try:
             if options.sssd:
                 nscd_service_action = 'stop'
diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py
index 0ac0c5c0a1..c87e8048af 100644
--- a/ipaserver/install/ipa_cacert_manage.py
+++ b/ipaserver/install/ipa_cacert_manage.py
@@ -123,14 +123,12 @@ def run(self):
 
         try:
             if command == 'renew':
-                rc = self.renew()
+                return self.renew()
             elif command == 'install':
-                rc = self.install()
+                return self.install()
         finally:
             api.Backend.ldap2.disconnect()
 
-        return rc
-
     def ldap_connect(self):
         password = self.options.password
         if not password:
diff --git a/ipaserver/install/ipa_replica_prepare.py b/ipaserver/install/ipa_replica_prepare.py
index 0be0302ddc..6872cefec1 100644
--- a/ipaserver/install/ipa_replica_prepare.py
+++ b/ipaserver/install/ipa_replica_prepare.py
@@ -197,6 +197,8 @@ def load_pkcs12(self, cert_files, key_password, key_nickname):
     def ask_for_options(self):
         options = self.options
         super(ReplicaPrepare, self).ask_for_options()
+        http_ca_cert = None
+        dirsrv_ca_cert = None
 
         # get the directory manager password
         self.dirman_password = options.password
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 98577447d2..4bf9e5681b 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -305,6 +305,8 @@ def install_check(installer):
     external_cert_file = installer._external_cert_file
     external_ca_file = installer._external_ca_file
     http_ca_cert = installer._ca_cert
+    dirsrv_ca_cert = None
+    pkinit_ca_cert = None
 
     tasks.check_ipv6_stack_enabled()
     tasks.check_selinux_status()
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 39c776c881..82c40a18af 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1021,10 +1021,13 @@ def promote_check(installer):
 
     http_pkcs12_file = None
     http_pkcs12_info = None
+    http_ca_cert = None
     dirsrv_pkcs12_file = None
     dirsrv_pkcs12_info = None
+    dirsrv_ca_cert = None
     pkinit_pkcs12_file = None
     pkinit_pkcs12_info = None
+    pkinit_ca_cert = None
 
     if options.http_cert_files:
         if options.http_pin is None:
diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py
index 835c6405b4..41c7f3d748 100644
--- a/ipaserver/plugins/trust.py
+++ b/ipaserver/plugins/trust.py
@@ -1808,7 +1808,8 @@ def execute(self, *keys, **options):
             trust_dn = self.obj.get_dn(keys[0], trust_type=u'ad')
             trust_entry = ldap.get_entry(trust_dn)
         except errors.NotFound:
-            self.api.Object[self.obj.parent_object].handle_not_found(keys[0])
+            self.api.Object[self.obj.parent_object].handle_not_found(
+                keys[0])
 
         dn = self.obj.get_dn(keys[0], keys[1], trust_type=u'ad')
         try:
@@ -1849,13 +1850,14 @@ def execute(self, *keys, **options):
             trust_dn = self.obj.get_dn(keys[0], trust_type=u'ad')
             trust_entry = ldap.get_entry(trust_dn)
         except errors.NotFound:
-            self.api.Object[self.obj.parent_object].handle_not_found(keys[0])
+            self.api.Object[self.obj.parent_object].handle_not_found(
+                keys[0])
 
         dn = self.obj.get_dn(keys[0], keys[1], trust_type=u'ad')
         try:
             entry = ldap.get_entry(dn)
             sid = entry.single_value.get('ipanttrusteddomainsid', None)
-            if not (sid in trust_entry['ipantsidblacklistincoming']):
+            if sid not in trust_entry['ipantsidblacklistincoming']:
                 trust_entry['ipantsidblacklistincoming'].append(sid)
                 ldap.update_entry(trust_entry)
             else:
diff --git a/ipaserver/secrets/store.py b/ipaserver/secrets/store.py
index 364b748c06..444666f01f 100644
--- a/ipaserver/secrets/store.py
+++ b/ipaserver/secrets/store.py
@@ -225,9 +225,9 @@ def import_key(self, value):
         v = json_decode(value)
         data = b64decode(v['pkcs12 data'])
         password = v['export password']
+        fd, tmpdata = tempfile.mkstemp(dir=paths.TMP)
+        os.close(fd)
         try:
-            _fd, tmpdata = tempfile.mkstemp(dir=paths.TMP)
-            os.close(_fd)
             with open(tmpdata, 'wb') as f:
                 f.write(data)
 
diff --git a/ipatests/pytest_plugins/integration/tasks.py b/ipatests/pytest_plugins/integration/tasks.py
index b407145ace..a6fdca39e6 100644
--- a/ipatests/pytest_plugins/integration/tasks.py
+++ b/ipatests/pytest_plugins/integration/tasks.py
@@ -600,8 +600,9 @@ def modify_sssd_conf(host, domain, mod_dict, provider='ipa',
     :param provider_subtype: backend subtype (e.g. id or sudo), will be added
         to the domain config if not present
     """
+    fd, temp_config_file = tempfile.mkstemp()
+    os.close(fd)
     try:
-        temp_config_file = tempfile.mkstemp()[1]
         current_config = host.transport.get_file_contents(paths.SSSD_CONF)
 
         with open(temp_config_file, 'wb') as f:

From b2b88283d74fe61aa2908977448f6e80f7ddcd1e Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Wed, 3 Jan 2018 12:11:15 +0100
Subject: [PATCH 08/10] LGTM: raise handle_not_found()

Turn calls "handle_not_found()" into "raise handle_not_found()" to
indicate control flow chance. It makes the code easier to understand,
the control flow more obvious and helps static analyzers.

It's OK to raise here because handle_not_found() always raises an
exception.

https://pagure.io/freeipa/issue/7344

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaserver/plugins/aci.py            |  2 +-
 ipaserver/plugins/automember.py     |  2 +-
 ipaserver/plugins/baseldap.py       | 35 +++++++++++++---------
 ipaserver/plugins/baseuser.py       |  2 +-
 ipaserver/plugins/caacl.py          | 16 +++++-----
 ipaserver/plugins/certmap.py        |  4 +--
 ipaserver/plugins/config.py         |  2 +-
 ipaserver/plugins/dns.py            | 44 ++++++++++++++--------------
 ipaserver/plugins/group.py          | 24 ++++++++++-----
 ipaserver/plugins/hbacrule.py       | 46 +++++++++++++++++------------
 ipaserver/plugins/host.py           | 10 +++----
 ipaserver/plugins/idrange.py        |  4 +--
 ipaserver/plugins/idviews.py        |  8 ++---
 ipaserver/plugins/netgroup.py       | 12 ++++++--
 ipaserver/plugins/otptoken.py       | 10 +++++--
 ipaserver/plugins/permission.py     |  4 +--
 ipaserver/plugins/pwpolicy.py       |  2 +-
 ipaserver/plugins/selinuxusermap.py | 58 +++++++++++++++++++++++--------------
 ipaserver/plugins/server.py         |  4 +--
 ipaserver/plugins/serverrole.py     |  2 +-
 ipaserver/plugins/service.py        |  2 +-
 ipaserver/plugins/stageuser.py      |  2 +-
 ipaserver/plugins/sudorule.py       | 31 ++++++++++----------
 ipaserver/plugins/trust.py          | 16 +++++-----
 ipaserver/plugins/user.py           |  8 ++---
 25 files changed, 205 insertions(+), 145 deletions(-)

diff --git a/ipaserver/plugins/aci.py b/ipaserver/plugins/aci.py
index f775d16a67..bc766c38cc 100644
--- a/ipaserver/plugins/aci.py
+++ b/ipaserver/plugins/aci.py
@@ -282,7 +282,7 @@ def _make_aci(ldap, current, aciname, kw):
             try:
                 api.Object['group'].get_dn_if_exists(kw['memberof'])
             except errors.NotFound:
-                api.Object['group'].handle_not_found(kw['memberof'])
+                raise api.Object['group'].handle_not_found(kw['memberof'])
             groupdn = _group_from_memberof(kw['memberof'])
             a.set_target_filter('memberOf=%s' % groupdn)
         if valid['filter']:
diff --git a/ipaserver/plugins/automember.py b/ipaserver/plugins/automember.py
index 8e9356a9d3..1e29f36578 100644
--- a/ipaserver/plugins/automember.py
+++ b/ipaserver/plugins/automember.py
@@ -769,7 +769,7 @@ def execute(self, *keys, **options):
                 try:
                     obj.get_dn_if_exists(name)
                 except errors.NotFound:
-                    obj.handle_not_found(name)
+                    raise obj.handle_not_found(name)
             search_filter = ldap.make_filter_from_attr(
                 obj.primary_key.name,
                 names,
diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 9050a18b4a..2da4ea3a14 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -753,6 +753,10 @@ def get_password_attributes(self, ldap, dn, entry_attrs):
                 entry_attrs[attr] = False
 
     def handle_not_found(self, *keys):
+        """Handle NotFound exception
+
+        Must raise errors.NotFound again.
+        """
         pkey = ''
         if self.primary_key:
             pkey = keys[-1]
@@ -1015,7 +1019,7 @@ def process_attr_options(self, entry_attrs, dn, keys, options):
                     dn, needldapattrs
                 )
             except errors.NotFound:
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
 
             # Provide a nice error message when user tries to delete an
             # attribute that does not exist on the entry (and user is not
@@ -1220,7 +1224,7 @@ def execute(self, *keys, **options):
                 entry_attrs = self._exc_wrapper(keys, options, ldap.get_entry)(
                     entry_attrs.dn, attrs_list)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         self.obj.get_indirect_members(entry_attrs, attrs_list)
 
@@ -1320,7 +1324,7 @@ def execute(self, *keys, **options):
                 dn, attrs_list
             )
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         self.obj.get_indirect_members(entry_attrs, attrs_list)
 
@@ -1450,7 +1454,7 @@ def execute(self, *keys, **options):
                     # Attempt to rename to the current name, ignore
                     pass
                 except errors.NotFound:
-                    self.obj.handle_not_found(*keys)
+                    raise self.obj.handle_not_found(*keys)
                 finally:
                     # Delete the primary_key from entry_attrs either way
                     del entry_attrs[self.obj.primary_key.name]
@@ -1469,7 +1473,7 @@ def execute(self, *keys, **options):
             if not rdnupdate:
                 raise e
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         try:
             entry_attrs = self._exc_wrapper(keys, options, ldap.get_entry)(
@@ -1548,14 +1552,16 @@ def delete_subtree(base_dn):
                         for entry_attrs in subentries:
                             delete_subtree(entry_attrs.dn)
                 try:
-                    self._exc_wrapper(nkeys, options, ldap.delete_entry)(base_dn)
+                    self._exc_wrapper(nkeys, options, ldap.delete_entry)(
+                        base_dn
+                    )
                 except errors.NotFound:
-                    self.obj.handle_not_found(*nkeys)
+                    raise self.obj.handle_not_found(*nkeys)
 
             try:
                 self._exc_wrapper(nkeys, options, ldap.delete_entry)(dn)
             except errors.NotFound:
-                self.obj.handle_not_found(*nkeys)
+                raise self.obj.handle_not_found(*nkeys)
             except errors.NotAllowedOnNonLeaf:
                 if not self.subtree_delete:
                     raise
@@ -1712,7 +1718,7 @@ def execute(self, *keys, **options):
                 dn, attrs_list
             )
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         self.obj.get_indirect_members(entry_attrs, attrs_list)
 
@@ -1813,7 +1819,7 @@ def execute(self, *keys, **options):
                 dn, attrs_list
             )
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         self.obj.get_indirect_members(entry_attrs, attrs_list)
 
@@ -2055,10 +2061,13 @@ def execute(self, *args, **options):
         except errors.EmptyResult:
             (entries, truncated) = ([], False)
         except errors.NotFound:
-            self.api.Object[self.obj.parent_object].handle_not_found(*keys)
+            return self.api.Object[self.obj.parent_object].handle_not_found(
+                *keys)
 
         for callback in self.get_callbacks('post'):
-            truncated = callback(self, ldap, entries, truncated, *args, **options)
+            truncated = callback(
+                self, ldap, entries, truncated, *args, **options
+            )
 
         if self.sort_result_entries:
             if self.obj.primary_key:
@@ -2370,7 +2379,7 @@ def execute(self, *keys, **options):
 
             self._exc_wrapper(keys, options, ldap.update_entry)(update)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         try:
             entry_attrs = self._exc_wrapper(keys, options, ldap.get_entry)(
diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py
index ea4cd90996..58c3332d2f 100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -529,7 +529,7 @@ def preserve_krbprincipalname_pre(self, ldap, entry_attrs, *keys, **options):
             if 'krbcanonicalname' not in old_entry:
                 return
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         self.context.krbprincipalname = old_entry.get(
             'krbprincipalname', [])
diff --git a/ipaserver/plugins/caacl.py b/ipaserver/plugins/caacl.py
index 43a397d8c7..edb8eabc48 100644
--- a/ipaserver/plugins/caacl.py
+++ b/ipaserver/plugins/caacl.py
@@ -278,7 +278,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             entry_attrs = ldap.get_entry(dn, attrs_list)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if is_all(options, 'ipacacategory') and 'ipamemberca' in entry_attrs:
             raise errors.MutuallyExclusiveError(reason=_(
@@ -332,7 +332,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['TRUE']
 
@@ -361,7 +361,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['FALSE']
 
@@ -391,7 +391,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if is_all(entry_attrs, 'usercategory'):
             raise errors.MutuallyExclusiveError(
                 reason=_("users cannot be added when user category='all'"))
@@ -423,7 +423,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if is_all(entry_attrs, 'hostcategory'):
             raise errors.MutuallyExclusiveError(
                 reason=_("hosts cannot be added when host category='all'"))
@@ -453,7 +453,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if is_all(entry_attrs, 'servicecategory'):
             raise errors.MutuallyExclusiveError(reason=_(
                 "services cannot be added when service category='all'"))
@@ -493,7 +493,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if is_all(entry_attrs, 'ipacertprofilecategory'):
             raise errors.MutuallyExclusiveError(reason=_(
                 "profiles cannot be added when profile category='all'"))
@@ -525,7 +525,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if is_all(entry_attrs, 'ipacacategory'):
             raise errors.MutuallyExclusiveError(reason=_(
                 "CAs cannot be added when CA category='all'"))
diff --git a/ipaserver/plugins/certmap.py b/ipaserver/plugins/certmap.py
index 6b44d376c8..843a0fd08a 100644
--- a/ipaserver/plugins/certmap.py
+++ b/ipaserver/plugins/certmap.py
@@ -349,7 +349,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['TRUE']
 
@@ -378,7 +378,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['FALSE']
 
diff --git a/ipaserver/plugins/config.py b/ipaserver/plugins/config.py
index c9033fa8e7..3437cde97b 100644
--- a/ipaserver/plugins/config.py
+++ b/ipaserver/plugins/config.py
@@ -532,7 +532,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             try:
                 self.api.Object.server.get_dn_if_exists(new_master)
             except errors.NotFound:
-                self.api.Object.server.handle_not_found(new_master)
+                raise self.api.Object.server.handle_not_found(new_master)
 
             backend = self.api.Backend.serverroles
             backend.config_update(ca_renewal_master_server=new_master)
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index 21af740955..01bd689d60 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -2164,7 +2164,7 @@ class DNSZoneBase_del(LDAPDelete):
     def pre_callback(self, ldap, dn, *nkeys, **options):
         assert isinstance(dn, DN)
         if not _check_DN_objectclass(ldap, dn, self.obj.object_class):
-            self.obj.handle_not_found(*nkeys)
+            raise self.obj.handle_not_found(*nkeys)
         return dn
 
     def post_callback(self, ldap, dn, *keys, **options):
@@ -2227,7 +2227,7 @@ class DNSZoneBase_show(LDAPRetrieve):
     def pre_callback(self, ldap, dn, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
         if not _check_DN_objectclass(ldap, dn, self.obj.object_class):
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         return dn
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -2246,10 +2246,10 @@ def execute(self, *keys, **options):
         try:
             entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if not _check_entry_objectclass(entry, self.obj.object_class):
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         entry['idnszoneactive'] = ['FALSE']
 
@@ -2271,10 +2271,10 @@ def execute(self, *keys, **options):
         try:
             entry = ldap.get_entry(dn, ['idnszoneactive', 'objectclass'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if not _check_entry_objectclass(entry, self.obj.object_class):
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         entry['idnszoneactive'] = ['TRUE']
 
@@ -2297,10 +2297,11 @@ def execute(self, *keys, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['objectclass'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         else:
-            if not _check_entry_objectclass(entry_attrs, self.obj.object_class):
-                self.obj.handle_not_found(*keys)
+            if not _check_entry_objectclass(
+                    entry_attrs, self.obj.object_class):
+                raise self.obj.handle_not_found(*keys)
 
         permission_name = self.obj.permission_name(keys[-1])
 
@@ -2353,10 +2354,10 @@ def execute(self, *keys, **options):
         try:
             entry = ldap.get_entry(dn, ['managedby', 'objectclass'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         else:
             if not _check_entry_objectclass(entry, self.obj.object_class):
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
 
         entry['managedby'] = None
 
@@ -2863,13 +2864,13 @@ class dnszone_mod(DNSZoneBase_mod):
     takes_options = DNSZoneBase_mod.takes_options + (
         Flag('force',
              label=_('Force'),
-             doc=_('Force nameserver change even if nameserver not in DNS'),
-        ),
+             doc=_('Force nameserver change even if nameserver not in DNS')),
     )
 
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list,  *keys, **options):
+    def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
+                     *keys, **options):
         if not _check_DN_objectclass(ldap, dn, self.obj.object_class):
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if 'idnssoamname' in entry_attrs:
             nameserver = entry_attrs['idnssoamname']
             if nameserver:
@@ -3146,10 +3147,11 @@ def check_zone(self, zone, **options):
         try:
             entry = ldap.get_entry(dn, ['objectclass'])
         except errors.NotFound:
-            parent_object.handle_not_found(zone)
+            raise parent_object.handle_not_found(zone)
         else:
             # only master zones can contain records
-            if 'idnszone' not in [x.lower() for x in entry.get('objectclass', [])]:
+            if 'idnszone' not in [x.lower()
+                                  for x in entry.get('objectclass', [])]:
                 raise errors.ValidationError(
                     name='dnszoneidnsname',
                     error=_(u'only master zones can contain records')
@@ -3751,7 +3753,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list,  *keys, **options):
         try:
             old_entry = ldap.get_entry(dn, _record_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if updated_attrs:
             for attr in updated_attrs:
@@ -3876,7 +3878,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             old_entry = ldap.get_entry(dn, _record_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         for attr in entry_attrs.keys():
             if attr not in _record_attributes:
@@ -4407,10 +4409,10 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             entry = ldap.get_entry(dn)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if not _check_entry_objectclass(entry, self.obj.object_class):
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         policy = self.obj.default_forward_policy
         forwarders = []
diff --git a/ipaserver/plugins/group.py b/ipaserver/plugins/group.py
index 5e94272396..2d6aba1495 100644
--- a/ipaserver/plugins/group.py
+++ b/ipaserver/plugins/group.py
@@ -659,17 +659,27 @@ def execute(self, *keys, **options):
         try:
             user_attrs = ldap.get_entry(user_dn)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        is_managed = self.obj.has_objectclass(user_attrs['objectclass'], 'mepmanagedentry')
+            raise self.obj.handle_not_found(*keys)
+        is_managed = self.obj.has_objectclass(
+            user_attrs['objectclass'], 'mepmanagedentry'
+        )
         if (not ldap.can_write(user_dn, "objectclass") or
-            not (ldap.can_write(user_dn, "mepManagedEntry")) and is_managed):
-            raise errors.ACIError(info=_('not allowed to modify user entries'))
+                not ldap.can_write(user_dn, "mepManagedEntry")
+                and is_managed):
+            raise errors.ACIError(
+                info=_('not allowed to modify user entries')
+            )
 
         group_attrs = ldap.get_entry(group_dn)
-        is_managed = self.obj.has_objectclass(group_attrs['objectclass'], 'mepmanagedby')
+        is_managed = self.obj.has_objectclass(
+            group_attrs['objectclass'], 'mepmanagedby'
+        )
         if (not ldap.can_write(group_dn, "objectclass") or
-            not (ldap.can_write(group_dn, "mepManagedBy")) and is_managed):
-            raise errors.ACIError(info=_('not allowed to modify group entries'))
+                not ldap.can_write(group_dn, "mepManagedBy")
+                and is_managed):
+            raise errors.ACIError(
+                info=_('not allowed to modify group entries')
+            )
 
         objectclasses = user_attrs['objectclass']
         try:
diff --git a/ipaserver/plugins/hbacrule.py b/ipaserver/plugins/hbacrule.py
index 2495702e87..5b77a6c79d 100644
--- a/ipaserver/plugins/hbacrule.py
+++ b/ipaserver/plugins/hbacrule.py
@@ -339,14 +339,24 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             entry_attrs = ldap.get_entry(dn, attrs_list)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if is_all(options, 'usercategory') and 'memberuser' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason=_("user category cannot be set to 'all' while there are allowed users"))
+            raise errors.MutuallyExclusiveError(
+                reason=_("user category cannot be set to 'all' while there "
+                         "are allowed users")
+            )
         if is_all(options, 'hostcategory') and 'memberhost' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason=_("host category cannot be set to 'all' while there are allowed hosts"))
-        if is_all(options, 'servicecategory') and 'memberservice' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason=_("service category cannot be set to 'all' while there are allowed services"))
+            raise errors.MutuallyExclusiveError(
+                reason=_("host category cannot be set to 'all' while there "
+                         "are allowed hosts")
+            )
+        if (is_all(options, 'servicecategory')
+                and 'memberservice' in entry_attrs):
+            raise errors.MutuallyExclusiveError(
+                reason=_("service category cannot be set to 'all' while "
+                         "there are allowed services")
+            )
         return dn
 
 
@@ -381,7 +391,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['TRUE']
 
@@ -411,7 +421,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['FALSE']
 
@@ -453,7 +463,7 @@ def execute(self, cn, **options):
         except errors.EmptyModlist:
             pass
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         return dict(result=True)
 
@@ -484,7 +494,7 @@ def execute(self, cn, **options):
         except (ValueError, errors.EmptyModlist):
             pass
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         return dict(result=True)
 
@@ -502,9 +512,9 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        if 'usercategory' in entry_attrs and \
-            entry_attrs['usercategory'][0].lower() == 'all':
+            raise self.obj.handle_not_found(*keys)
+        if ('usercategory' in entry_attrs and
+                entry_attrs['usercategory'][0].lower() == 'all'):
             raise errors.MutuallyExclusiveError(
                 reason=_("users cannot be added when user category='all'"))
         return dn
@@ -533,9 +543,9 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        if 'hostcategory' in entry_attrs and \
-            entry_attrs['hostcategory'][0].lower() == 'all':
+            raise self.obj.handle_not_found(*keys)
+        if ('hostcategory' in entry_attrs and
+                entry_attrs['hostcategory'][0].lower() == 'all'):
             raise errors.MutuallyExclusiveError(
                 reason=_("hosts cannot be added when host category='all'"))
         return dn
@@ -588,9 +598,9 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        if 'servicecategory' in entry_attrs and \
-            entry_attrs['servicecategory'][0].lower() == 'all':
+            raise self.obj.handle_not_found(*keys)
+        if ('servicecategory' in entry_attrs and
+                entry_attrs['servicecategory'][0].lower() == 'all'):
             raise errors.MutuallyExclusiveError(reason=_(
                 "services cannot be added when service category='all'"))
         return dn
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index d6a8dcf58c..6487cd612d 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -899,7 +899,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             try:
                 entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
             except errors.NotFound:
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
             old_certs = entry_attrs_old.get('usercertificate', [])
             removed_certs = set(old_certs) - set(certs)
             for cert in removed_certs:
@@ -931,7 +931,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
                 result = api.Command['dnszone_show'](domain)['result']
                 domain = result['idnsname'][0]
             except errors.NotFound:
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
             update_sshfp_record(domain, unicode(parts[0]), entry_attrs)
 
         if 'ipasshpubkey' in entry_attrs:
@@ -1020,7 +1020,7 @@ def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **option
                     try:
                         entry_attrs = ldap.get_entry(dn, ['managedby'])
                     except errors.NotFound:
-                        self.obj.handle_not_found(pkey)
+                        raise self.obj.handle_not_found(pkey)
                     hosts.append(set(entry_attrs.get('managedby', '')))
                 hosts = list(reduce(lambda s1, s2: s1 & s2, hosts))
 
@@ -1037,7 +1037,7 @@ def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **option
                     try:
                         entry_attrs = ldap.get_entry(dn, ['managedby'])
                     except errors.NotFound:
-                        self.obj.handle_not_found(pkey)
+                        raise self.obj.handle_not_found(pkey)
                     not_hosts += entry_attrs.get('managedby', [])
                 not_hosts = list(set(not_hosts))
 
@@ -1191,7 +1191,7 @@ def execute(self, *keys, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['usercertificate'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if self.api.Command.ca_is_enabled()['result']:
             certs = self.api.Command.cert_find(host=keys)['result']
 
diff --git a/ipaserver/plugins/idrange.py b/ipaserver/plugins/idrange.py
index 49d98d2c0a..b67ed31082 100644
--- a/ipaserver/plugins/idrange.py
+++ b/ipaserver/plugins/idrange.py
@@ -535,7 +535,7 @@ def pre_callback(self, ldap, dn, *keys, **options):
                                             'ipaidrangesize',
                                             'ipanttrusteddomainsid'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         # Check whether we leave any object with id in deleted range
         old_base_id = int(old_attrs.get('ipabaseid', [0])[0])
@@ -645,7 +645,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             old_attrs = ldap.get_entry(dn, ['*'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if old_attrs['iparangetype'][0] == 'ipa-local':
             raise errors.ExecutionError(
diff --git a/ipaserver/plugins/idviews.py b/ipaserver/plugins/idviews.py
index a68b03915b..cadeb1453f 100644
--- a/ipaserver/plugins/idviews.py
+++ b/ipaserver/plugins/idviews.py
@@ -153,7 +153,7 @@ def ensure_possible_objectclasses(self, ldap, dn, entry_attrs, *keys):
         try:
             orig_entry_attrs = ldap.get_entry(dn, ['objectclass'])
         except errors.NotFound:
-            self.handle_not_found(*keys)
+            raise self.handle_not_found(*keys)
 
         orig_objectclasses = {
             o.lower() for o in orig_entry_attrs.get('objectclass', [])}
@@ -587,7 +587,7 @@ def resolve_object_to_anchor(ldap, obj_type, obj, fallback_to_ldap):
         pass
 
     # No acceptable object was found
-    api.Object[obj_type].handle_not_found(obj)
+    raise api.Object[obj_type].handle_not_found(obj)
 
 
 def resolve_anchor_to_object_name(ldap, obj_type, anchor):
@@ -789,12 +789,12 @@ def pre_callback(self, ldap, dn, *keys, **options):
         try:
             entry = ldap.get_entry(dn, ['objectclass'])
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         # If not, treat it as a failed search
         for required_oc in self.obj.object_class:
             if not self.obj.has_objectclass(entry['objectclass'], required_oc):
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
 
         return dn
 
diff --git a/ipaserver/plugins/netgroup.py b/ipaserver/plugins/netgroup.py
index 11fec0aad8..511c512f43 100644
--- a/ipaserver/plugins/netgroup.py
+++ b/ipaserver/plugins/netgroup.py
@@ -315,11 +315,17 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             entry_attrs = ldap.get_entry(dn, attrs_list)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if is_all(options, 'usercategory') and 'memberuser' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason=_("user category cannot be set to 'all' while there are allowed users"))
+            raise errors.MutuallyExclusiveError(
+                reason=_("user category cannot be set to 'all' while there "
+                         "are allowed users")
+            )
         if is_all(options, 'hostcategory') and 'memberhost' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason=_("host category cannot be set to 'all' while there are allowed hosts"))
+            raise errors.MutuallyExclusiveError(
+                reason=_("host category cannot be set to 'all' while there "
+                         "are allowed hosts")
+            )
         return dn
 
 
diff --git a/ipaserver/plugins/otptoken.py b/ipaserver/plugins/otptoken.py
index c66f0980f0..24815c108f 100644
--- a/ipaserver/plugins/otptoken.py
+++ b/ipaserver/plugins/otptoken.py
@@ -99,19 +99,24 @@ def _convert_owner(userobj, entry_attrs, options):
         entry_attrs['ipatokenowner'] = [userobj.get_primary_key_from_dn(o)
                                         for o in entry_attrs['ipatokenowner']]
 
+
 def _normalize_owner(userobj, entry_attrs):
     owner = entry_attrs.get('ipatokenowner', None)
     if owner:
         try:
-            entry_attrs['ipatokenowner'] = userobj._normalize_manager(owner)[0]
+            entry_attrs['ipatokenowner'] = userobj._normalize_manager(
+                owner
+            )[0]
         except NotFound:
-            userobj.handle_not_found(owner)
+            raise userobj.handle_not_found(owner)
+
 
 def _check_interval(not_before, not_after):
     if not_before and not_after:
         return not_before <= not_after
     return True
 
+
 def _set_token_type(entry_attrs, **options):
     klasses = [x.lower() for x in entry_attrs.get('objectclass', [])]
     for ttype in TOKEN_TYPES:
@@ -122,6 +127,7 @@ def _set_token_type(entry_attrs, **options):
     if not options.get('all', False) or options.get('pkey_only', False):
         entry_attrs.pop('objectclass', None)
 
+
 @register()
 class otptoken(LDAPObject):
     """
diff --git a/ipaserver/plugins/permission.py b/ipaserver/plugins/permission.py
index 49ac7cfcad..0a3873b40e 100644
--- a/ipaserver/plugins/permission.py
+++ b/ipaserver/plugins/permission.py
@@ -1061,7 +1061,7 @@ def pre_callback(self, ldap, dn, *keys, **options):
         try:
             entry = ldap.get_entry(dn, attrs_list=self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if not options.get('force'):
             self.obj.reject_system(entry)
@@ -1105,7 +1105,7 @@ def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
             attrs_list = self.obj.default_attributes
             old_entry = ldap.get_entry(dn, attrs_list=attrs_list)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         self.obj.reject_system(old_entry)
         self.obj.upgrade_permission(old_entry)
diff --git a/ipaserver/plugins/pwpolicy.py b/ipaserver/plugins/pwpolicy.py
index d8e6ed8c74..5534906a57 100644
--- a/ipaserver/plugins/pwpolicy.py
+++ b/ipaserver/plugins/pwpolicy.py
@@ -179,7 +179,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             result = ldap.get_entry(group_dn, ['objectclass'])
         except errors.NotFound:
-            self.api.Object.group.handle_not_found(keys[-1])
+            raise self.api.Object.group.handle_not_found(keys[-1])
 
         oc = [x.lower() for x in result['objectclass']]
         if 'mepmanagedentry' in oc:
diff --git a/ipaserver/plugins/selinuxusermap.py b/ipaserver/plugins/selinuxusermap.py
index ec23153de5..2fad418493 100644
--- a/ipaserver/plugins/selinuxusermap.py
+++ b/ipaserver/plugins/selinuxusermap.py
@@ -355,16 +355,24 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             _entry_attrs = ldap.get_entry(dn, attrs_list)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
-        is_to_be_deleted = lambda x: (x in _entry_attrs and x in entry_attrs) and \
-                                     entry_attrs[x] == None
+        def is_to_be_deleted(x):
+            return (
+                (x in _entry_attrs and x in entry_attrs)
+                and entry_attrs[x] is None
+            )
 
         # makes sure the local members and hbacrule is not set at the same time
         # memberuser or memberhost could have been set using --setattr
-        is_to_be_set = lambda x: ((x in _entry_attrs and _entry_attrs[x] != None) or \
-                                 (x in entry_attrs and entry_attrs[x] != None)) and \
-                                 not is_to_be_deleted(x)
+        def is_to_be_set(x):
+            return (
+                (
+                    (x in _entry_attrs and _entry_attrs[x] is not None) or
+                    (x in entry_attrs and entry_attrs[x] is not None)
+                )
+                and not is_to_be_deleted(x)
+            )
 
         are_local_members_to_be_set = any(is_to_be_set(attr)
                                           for attr in ('usercategory',
@@ -379,18 +387,26 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         if are_local_members_to_be_set and is_hbacrule_to_be_set:
             raise errors.MutuallyExclusiveError(reason=notboth_err)
 
-        if is_all(entry_attrs, 'usercategory') and 'memberuser' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason="user category "
-                 "cannot be set to 'all' while there are allowed users")
-        if is_all(entry_attrs, 'hostcategory') and 'memberhost' in entry_attrs:
-            raise errors.MutuallyExclusiveError(reason="host category "
-                 "cannot be set to 'all' while there are allowed hosts")
+        if (is_all(entry_attrs, 'usercategory')
+                and 'memberuser' in entry_attrs):
+            raise errors.MutuallyExclusiveError(
+                reason="user category cannot be set to 'all' while there "
+                       "are allowed users"
+            )
+        if (is_all(entry_attrs, 'hostcategory')
+                and 'memberhost' in entry_attrs):
+            raise errors.MutuallyExclusiveError(
+                reason="host category cannot be set to 'all' while there "
+                       "are allowed hosts"
+            )
 
         if 'ipaselinuxuser' in entry_attrs:
             validate_selinuxuser_inlist(ldap, entry_attrs['ipaselinuxuser'])
 
         if 'seealso' in entry_attrs:
-            entry_attrs['seealso'] = self.obj._normalize_seealso(entry_attrs['seealso'])
+            entry_attrs['seealso'] = self.obj._normalize_seealso(
+                entry_attrs['seealso']
+            )
         return dn
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -457,7 +473,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['TRUE']
 
@@ -487,7 +503,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['FALSE']
 
@@ -516,9 +532,9 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        if 'usercategory' in entry_attrs and \
-            entry_attrs['usercategory'][0].lower() == 'all':
+            raise self.obj.handle_not_found(*keys)
+        if ('usercategory' in entry_attrs and
+                entry_attrs['usercategory'][0].lower() == 'all'):
             raise errors.MutuallyExclusiveError(
                 reason=_("users cannot be added when user category='all'"))
         if 'seealso' in entry_attrs:
@@ -549,9 +565,9 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
             entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
             dn = entry_attrs.dn
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        if 'hostcategory' in entry_attrs and \
-            entry_attrs['hostcategory'][0].lower() == 'all':
+            raise self.obj.handle_not_found(*keys)
+        if ('hostcategory' in entry_attrs and
+                entry_attrs['hostcategory'][0].lower() == 'all'):
             raise errors.MutuallyExclusiveError(
                 reason=_("hosts cannot be added when host category='all'"))
         if 'seealso' in entry_attrs:
diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index 2cee976f29..94ada8b9da 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -227,7 +227,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
 
         if entry_attrs.get('ipalocation'):
             if not ldap.entry_exists(entry_attrs['ipalocation'][0]):
-                self.api.Object.location.handle_not_found(
+                raise self.api.Object.location.handle_not_found(
                     options['ipalocation_location'])
 
         if 'ipalocation' in entry_attrs or 'ipaserviceweight' in entry_attrs:
@@ -893,7 +893,7 @@ def execute(self, *keys, **options):
         try:
             self.obj.get_dn_if_exists(*keys[:-1])
         except errors.NotFound:
-            self.obj.handle_not_found(keys[-2])
+            raise self.obj.handle_not_found(keys[-2])
 
         # the user must have the Replication Administrators privilege
         privilege = u'Replication Administrators'
diff --git a/ipaserver/plugins/serverrole.py b/ipaserver/plugins/serverrole.py
index b5781b0dff..1b19c7e867 100644
--- a/ipaserver/plugins/serverrole.py
+++ b/ipaserver/plugins/serverrole.py
@@ -76,7 +76,7 @@ def ensure_master_exists(self, fqdn):
         try:
             server_obj.get_dn_if_exists(fqdn)
         except NotFound:
-            server_obj.handle_not_found(fqdn)
+            raise server_obj.handle_not_found(fqdn)
 
 
 @register()
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 9ff361fbbc..be31f81027 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -698,7 +698,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             try:
                 entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
             except errors.NotFound:
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
             old_certs = entry_attrs_old.get('usercertificate', [])
             removed_certs = set(old_certs) - set(certs)
             for cert in removed_certs:
diff --git a/ipaserver/plugins/stageuser.py b/ipaserver/plugins/stageuser.py
index 1fc591e851..59c66276cb 100644
--- a/ipaserver/plugins/stageuser.py
+++ b/ipaserver/plugins/stageuser.py
@@ -671,7 +671,7 @@ def execute(self, *args, **options):
                 staging_dn, ['*']
             )
         except errors.NotFound:
-            self.obj.handle_not_found(*args)
+            raise self.obj.handle_not_found(*args)
         entry_attrs = dict((k.lower(), v) for (k, v) in entry_attrs.items())
 
         # Check it does not exist an active entry with the same RDN
diff --git a/ipaserver/plugins/sudorule.py b/ipaserver/plugins/sudorule.py
index 28c3f21f11..6037938330 100644
--- a/ipaserver/plugins/sudorule.py
+++ b/ipaserver/plugins/sudorule.py
@@ -417,7 +417,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         error = _("%(type)s category cannot be set to 'all' "
                   "while there are allowed %(objects)s")
@@ -487,7 +487,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['TRUE']
 
@@ -510,7 +510,7 @@ def execute(self, cn, **options):
         try:
             entry_attrs = ldap.get_entry(dn, ['ipaenabledflag'])
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         entry_attrs['ipaenabledflag'] = ['FALSE']
 
@@ -535,7 +535,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if is_all(_entry_attrs, 'cmdcategory'):
             raise errors.MutuallyExclusiveError(
@@ -586,7 +586,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if is_all(_entry_attrs, 'usercategory'):
             raise errors.MutuallyExclusiveError(
@@ -640,7 +640,7 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, **options):
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if is_all(_entry_attrs, 'hostcategory'):
             raise errors.MutuallyExclusiveError(
@@ -654,10 +654,11 @@ def post_callback(self, ldap, completed, failed, dn, entry_attrs,
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if 'hostmask' in options:
-            norm = lambda x: unicode(netaddr.IPNetwork(x).cidr)
+            def norm(x):
+                return unicode(netaddr.IPNetwork(x).cidr)
 
             old_masks = set(norm(m) for m in _entry_attrs.get('hostmask', []))
             new_masks = set(norm(m) for m in options['hostmask'])
@@ -699,7 +700,7 @@ def post_callback(self, ldap, completed, failed, dn, entry_attrs,
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if 'hostmask' in options:
             def norm(x):
@@ -745,7 +746,7 @@ def check_validity(runas):
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         if any((is_all(_entry_attrs, 'ipasudorunasusercategory'),
                 is_all(_entry_attrs, 'ipasudorunasgroupcategory'))):
@@ -860,9 +861,9 @@ def check_validity(runas):
         try:
             _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-        if is_all(_entry_attrs, 'ipasudorunasusercategory') or \
-          is_all(_entry_attrs, 'ipasudorunasgroupcategory'):
+            raise self.obj.handle_not_found(*keys)
+        if (is_all(_entry_attrs, 'ipasudorunasusercategory') or
+                is_all(_entry_attrs, 'ipasudorunasgroupcategory')):
             raise errors.MutuallyExclusiveError(
                 reason=_("users cannot be added when runAs user or runAs "
                          "group category='all'"))
@@ -943,7 +944,7 @@ def execute(self, cn, **options):
         except errors.EmptyModlist:
             pass
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         attrs_list = self.obj.default_attributes
         entry_attrs = ldap.get_entry(dn, attrs_list)
@@ -993,7 +994,7 @@ def execute(self, cn, **options):
                     value=options['ipasudoopt']
                     )
         except errors.NotFound:
-            self.obj.handle_not_found(cn)
+            raise self.obj.handle_not_found(cn)
 
         attrs_list = self.obj.default_attributes
         entry_attrs = ldap.get_entry(dn, attrs_list)
diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py
index 41c7f3d748..978738e20a 100644
--- a/ipaserver/plugins/trust.py
+++ b/ipaserver/plugins/trust.py
@@ -590,7 +590,7 @@ def get_dn(self, *keys, **kwargs):
                     ldap.SCOPE_SUBTREE, trustfilter, ['']
                 )
             except errors.NotFound:
-                self.handle_not_found(keys[-1])
+                raise self.handle_not_found(keys[-1])
 
             if len(result) > 1:
                 raise errors.OnlyOneValueAllowed(attr='trust domain')
@@ -1273,7 +1273,7 @@ def _normalize_groupdn(self, entry_attrs):
             try:
                 self.backend.get_entry(dn)
             except errors.NotFound:
-                self.api.Object['group'].handle_not_found(group)
+                raise self.api.Object['group'].handle_not_found(group)
             # DN is valid, we can just return
             return
         except ValueError:
@@ -1288,7 +1288,7 @@ def _normalize_groupdn(self, entry_attrs):
                     [''],
                     DN(self.api.env.container_group, self.api.env.basedn))
             except errors.NotFound:
-                self.api.Object['group'].handle_not_found(group)
+                raise self.api.Object['group'].handle_not_found(group)
             else:
                 entry_attrs['ipantfallbackprimarygroup'] = [group_entry.dn]
 
@@ -1645,7 +1645,7 @@ def execute(self, *keys, **options):
                         name='domain',
                         error=_("cannot delete root domain of the trust, "
                                 "use trust-del to delete the trust itself"))
-                self.obj.handle_not_found(keys[0], domain)
+                raise self.obj.handle_not_found(keys[0], domain)
 
             try:
                 self.api.Command.trustdomain_enable(keys[0], domain)
@@ -1808,7 +1808,7 @@ def execute(self, *keys, **options):
             trust_dn = self.obj.get_dn(keys[0], trust_type=u'ad')
             trust_entry = ldap.get_entry(trust_dn)
         except errors.NotFound:
-            self.api.Object[self.obj.parent_object].handle_not_found(
+            raise self.api.Object[self.obj.parent_object].handle_not_found(
                 keys[0])
 
         dn = self.obj.get_dn(keys[0], keys[1], trust_type=u'ad')
@@ -1821,7 +1821,7 @@ def execute(self, *keys, **options):
             else:
                 raise errors.AlreadyActive()
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         return dict(
             result=True,
@@ -1850,7 +1850,7 @@ def execute(self, *keys, **options):
             trust_dn = self.obj.get_dn(keys[0], trust_type=u'ad')
             trust_entry = ldap.get_entry(trust_dn)
         except errors.NotFound:
-            self.api.Object[self.obj.parent_object].handle_not_found(
+            raise self.api.Object[self.obj.parent_object].handle_not_found(
                 keys[0])
 
         dn = self.obj.get_dn(keys[0], keys[1], trust_type=u'ad')
@@ -1863,7 +1863,7 @@ def execute(self, *keys, **options):
             else:
                 raise errors.AlreadyInactive()
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
 
         return dict(
             result=True,
diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py
index fe899e4649..9e8abf869d 100644
--- a/ipaserver/plugins/user.py
+++ b/ipaserver/plugins/user.py
@@ -654,7 +654,7 @@ def _preserve_user(self, pkey, delete_container, **options):
             original_entry_attrs = self._exc_wrapper(
                 pkey, options, ldap.get_entry)(dn, ['dn'])
         except errors.NotFound:
-            self.obj.handle_not_found(pkey)
+            raise self.obj.handle_not_found(pkey)
 
         for callback in self.get_callbacks('pre'):
             dn = callback(self, ldap, dn, pkey, **options)
@@ -710,7 +710,7 @@ def pre_callback(self, ldap, dn, *keys, **options):
             try:
                 remove_ipaobject_overrides(self.obj.backend, self.obj.api, dn)
             except errors.NotFound:
-                self.obj.handle_not_found(*keys)
+                raise self.obj.handle_not_found(*keys)
 
         if dn.endswith(DN(self.obj.delete_container_dn, api.env.basedn)):
             return dn
@@ -878,7 +878,7 @@ def execute(self, *keys, **options):
         try:
             self._exc_wrapper(keys, options, ldap.get_entry)(delete_dn)
         except errors.NotFound:
-            self.obj.handle_not_found(*keys)
+            raise self.obj.handle_not_found(*keys)
         if delete_dn.endswith(DN(self.obj.active_container_dn,
                                  api.env.basedn)):
             raise errors.InvocationError(
@@ -1160,7 +1160,7 @@ def execute(self, *keys, **options):
                 entries.append(newresult)
                 count += 1
             except errors.NotFound:
-                self.api.Object.user.handle_not_found(*keys)
+                raise self.api.Object.user.handle_not_found(*keys)
             except Exception as e:
                 logger.error("user_status: Retrieving status for %s failed "
                              "with %s", dn, str(e))

From 79e6a44e90c249b47773c49fcff0ab049bd64f2d Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Thu, 4 Jan 2018 13:36:36 +0100
Subject: [PATCH 09/10] LGTM: Use explicit string concatenation

Implicit string concatenation is technically correct, too. But when
combined in list, it's confusing for both human eye and static code
analysis.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipalib/plugable.py              | 12 ++++++------
 ipaserver/install/cainstance.py | 30 ++++++++++++++++--------------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index b1fba368db..758b67cd71 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -532,15 +532,15 @@ def config_file_callback(option, opt, value, parser):
                 formatter=IPAHelpFormatter(),
                 usage='%prog [global-options] COMMAND [command-options]',
                 description='Manage an IPA domain',
-                version=('VERSION: %s, API_VERSION: %s'
-                                % (VERSION, API_VERSION)),
+                version=('VERSION: %s, API_VERSION: %s' %
+                            (VERSION, API_VERSION)),
                 epilog='\n'.join([
                     'See "ipa help topics" for available help topics.',
-                    'See "ipa help <TOPIC>" for more information on a '
-                        'specific topic.',
+                    'See "ipa help <TOPIC>" for more information on '
+                    + 'a specific topic.',
                     'See "ipa help commands" for the full list of commands.',
-                    'See "ipa <COMMAND> --help" for more information on a '
-                        'specific command.',
+                    'See "ipa <COMMAND> --help" for more information on '
+                    + 'a specific command.',
                 ]))
             parser.disable_interspersed_args()
             parser.add_option("-h", "--help", action="help",
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 20bca078c0..d2126a1b1e 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1541,12 +1541,14 @@ def ensure_entry(dn, **attrs):
 def configure_profiles_acl():
     """Allow the Certificate Manager Agents group to modify profiles."""
     new_rules = [
-        'certServer.profile.configuration:read,modify:allow (read,modify) '
-        'group="Certificate Manager Agents":'
-        'Certificate Manager agents may modify (create/update/delete) and read profiles',
-
-        'certServer.ca.account:login,logout:allow (login,logout) '
-        'user="anybody":Anybody can login and logout',
+        'certServer.profile.configuration:read,modify' +
+        ':allow (read,modify) group="Certificate Manager Agents"' +
+        ':Certificate Manager agents may modify (create/update/delete) ' +
+        'and read profiles',
+
+        'certServer.ca.account:login,logout' +
+        ':allow (login,logout) user="anybody"' +
+        ':Anybody can login and logout',
     ]
     return __add_acls(new_rules)
 
@@ -1554,20 +1556,20 @@ def configure_profiles_acl():
 def configure_lightweight_ca_acls():
     """Allow Certificate Manager Agents to manage lightweight CAs."""
     new_rules = [
-        'certServer.ca.authorities:list,read'
-        ':allow (list,read) user="anybody"'
+        'certServer.ca.authorities:list,read' +
+        ':allow (list,read) user="anybody"' +
         ':Anybody may list and read lightweight authorities',
 
-        'certServer.ca.authorities:create,modify'
-        ':allow (create,modify) group="Administrators"'
+        'certServer.ca.authorities:create,modify' +
+        ':allow (create,modify) group="Administrators"' +
         ':Administrators may create and modify lightweight authorities',
 
-        'certServer.ca.authorities:delete'
-        ':allow (delete) group="Administrators"'
+        'certServer.ca.authorities:delete' +
+        ':allow (delete) group="Administrators"' +
         ':Administrators may delete lightweight authorities',
 
-        'certServer.ca.authorities:create,modify,delete'
-        ':allow (create,modify,delete) group="Certificate Manager Agents"'
+        'certServer.ca.authorities:create,modify,delete' +
+        ':allow (create,modify,delete) group="Certificate Manager Agents"' +
         ':Certificate Manager Agents may manage lightweight authorities',
     ]
     return __add_acls(new_rules)

From 71f913fd91c9f20e11ae27330684fc3ef0670dcc Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Thu, 4 Jan 2018 13:51:39 +0100
Subject: [PATCH 10/10] LGTM: unnecessary else in for loop

for/else makes only sense when the for loop uses break, too. If the for
loop simply returns on success, then else is not necessary.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipaclient/install/client.py                       | 15 +++++++--------
 ipaserver/plugins/dns.py                          |  3 +--
 ipaserver/plugins/permission.py                   | 13 +++++++------
 ipatests/pytest_plugins/integration/env_config.py |  4 ++--
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index c565871b9c..5173d90bfe 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -1194,8 +1194,7 @@ def get_iface_from_ip(ip_addr):
             for ip in if_addrs.get(family, []):
                 if ip['addr'] == ip_addr:
                     return interface
-    else:
-        raise RuntimeError("IP %s not assigned to any interface." % ip_addr)
+    raise RuntimeError("IP %s not assigned to any interface." % ip_addr)
 
 
 def get_local_ipaddresses(iface=None):
@@ -1390,6 +1389,7 @@ def verify_dns_update(fqdn, ips):
 def get_server_connection_interface(server):
     """Connect to IPA server, get all ip addresses of interface used to connect
     """
+    last_error = None
     for res in socket.getaddrinfo(
             server, 389, socket.AF_UNSPEC, socket.SOCK_STREAM):
         af, socktype, proto, _canonname, sa = res
@@ -1397,7 +1397,6 @@ def get_server_connection_interface(server):
             s = socket.socket(af, socktype, proto)
         except socket.error as e:
             last_error = e
-            s = None
             continue
         try:
             s.connect(sa)
@@ -1413,11 +1412,11 @@ def get_server_connection_interface(server):
             return get_iface_from_ip(ip)
         except (CalledProcessError, RuntimeError) as e:
             last_error = e
-    else:
-        msg = "Cannot get server connection interface"
-        if last_error:
-            msg += ": %s" % (last_error)
-        raise RuntimeError(msg)
+
+    msg = "Cannot get server connection interface"
+    if last_error:
+        msg += ": %s" % last_error
+    raise RuntimeError(msg)
 
 
 def client_dns(server, hostname, options):
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index 01bd689d60..e6a1f580e9 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -3426,8 +3426,7 @@ def wait_for_modified_attr(self, ldap_rrset, rdtype, dns_name):
             time.sleep(period)
 
         # Maximum number of attempts was reached
-        else:
-            raise errors.DNSDataMismatch(expected=ldap_rrset, got=dns_rrset)
+        raise errors.DNSDataMismatch(expected=ldap_rrset, got=dns_rrset)
 
     def wait_for_modified_attrs(self, entry_attrs, dns_name, dns_domain):
         '''Wait until DNS resolver returns up-to-date answer for given entry
diff --git a/ipaserver/plugins/permission.py b/ipaserver/plugins/permission.py
index 0a3873b40e..30f8663c9f 100644
--- a/ipaserver/plugins/permission.py
+++ b/ipaserver/plugins/permission.py
@@ -694,6 +694,7 @@ def _get_aci_entry_and_string(self, permission_entry, name=None,
                 acientry = ldap.get_entry(location, ['aci'])
             except errors.NotFound:
                 acientry = ldap.make_entry(location)
+
         acis = acientry.get('aci', ())
         for acistring in acis:
             try:
@@ -704,12 +705,12 @@ def _get_aci_entry_and_string(self, permission_entry, name=None,
                 continue
             if aci.name == wanted_aciname:
                 return acientry, acistring
-        else:
-            if notfound_ok:
-                return acientry, None
-            raise errors.NotFound(
-                reason=_('The ACI for permission %(name)s was not found '
-                         'in %(dn)s ') % {'name': name, 'dn': location})
+
+        if notfound_ok:
+            return acientry, None
+        raise errors.NotFound(
+            reason=_('The ACI for permission %(name)s was not found '
+                     'in %(dn)s ') % {'name': name, 'dn': location})
 
     def upgrade_permission(self, entry, target_entry=None,
                            output_only=False, cached_acientry=None):
diff --git a/ipatests/pytest_plugins/integration/env_config.py b/ipatests/pytest_plugins/integration/env_config.py
index d140aa9df6..1bf1dc7d07 100644
--- a/ipatests/pytest_plugins/integration/env_config.py
+++ b/ipatests/pytest_plugins/integration/env_config.py
@@ -251,8 +251,8 @@ def coalesce(name, *other_names):
                     pass
                 else:
                     return
-            else:
-                env[name] = ''
+            env[name] = ''
+
     coalesce('MASTER_env1', 'MASTER')
     coalesce('REPLICA_env1', 'REPLICA', 'SLAVE')
     coalesce('CLIENT_env1', 'CLIENT')
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to