From 8780751330871033b13fc8fbb7eb0588baf4417d Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 11 Aug 2009 17:08:09 -0400 Subject: Clean up some problems discovered with pylint and pychecker Much of this is formatting to make pylint happy but it also fixes some real bugs. --- ipalib/errors.py | 16 ++++++++++ ipapython/entity.py | 4 --- ipapython/ipasslfile.py | 26 +++++++++------- ipapython/ipautil.py | 14 ++++----- ipapython/nsslib.py | 1 - ipapython/sysrestore.py | 5 ++- ipaserver/install/bindinstance.py | 6 +--- ipaserver/install/cainstance.py | 6 +--- ipaserver/install/dsinstance.py | 7 ++--- ipaserver/install/httpinstance.py | 10 ++---- ipaserver/install/installutils.py | 17 +++++------ ipaserver/install/krbinstance.py | 8 ++--- ipaserver/install/ntpinstance.py | 1 - ipaserver/install/replication.py | 14 +++++---- ipaserver/ipautil.py | 64 ++++++++++++++++++++------------------- ipaserver/plugins/ldap2.py | 4 --- ipaserver/servercore.py | 3 -- 17 files changed, 99 insertions(+), 107 deletions(-) diff --git a/ipalib/errors.py b/ipalib/errors.py index 82905b391..caecdc382 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -992,6 +992,22 @@ class RemoteRetrieveError(ExecutionError): errno = 4016 format = _('%(reason)s') +class SameGroupError(ExecutionError): + """ + **4017** Raised when adding a group as a member of itself + + For example: + + >>> raise SameGroupError() + Traceback (most recent call last): + ... + SameGroupError: A group may not be added as a member of itself + + """ + + errno = 4017 + format = _('A group may not be added as a member of itself') + class BuiltinError(ExecutionError): """ **4100** Base class for builtin execution errors (*4100 - 4199*). diff --git a/ipapython/entity.py b/ipapython/entity.py index 580cbd00b..1274f8a13 100644 --- a/ipapython/entity.py +++ b/ipapython/entity.py @@ -15,10 +15,6 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # -import ldap -import ldif -import re -import cStringIO import copy import ipapython.ipautil diff --git a/ipapython/ipasslfile.py b/ipapython/ipasslfile.py index 5525ab9b5..5f4ea2a7b 100644 --- a/ipapython/ipasslfile.py +++ b/ipapython/ipasslfile.py @@ -15,6 +15,10 @@ # the reference counting and SharedSocketClient provides an constructor # and close() method that call incref() and decref() correctly. +import socket +import errno +from httplib import UnimplementedFileMode + class SharedSocket: def __init__(self, sock): self.sock = sock @@ -91,13 +95,13 @@ class SSLFile(SharedSocketClient): break L.append(s) avail += len(s) - all = "".join(L) + alldata = "".join(L) if size is None: self._buf = '' - return all + return alldata else: - self._buf = all[size:] - return all[:size] + self._buf = alldata[size:] + return alldata[:size] def readline(self): L = [self._buf] @@ -114,25 +118,25 @@ class SSLFile(SharedSocketClient): # loop exited because there is no more data return "".join(L) else: - all = "".join(L) + alldata = "".join(L) # XXX could do enough bookkeeping not to do a 2nd search - i = all.find("\n") + 1 - line = all[:i] - self._buf = all[i:] + i = alldata.find("\n") + 1 + line = alldata[:i] + self._buf = alldata[i:] return line def readlines(self, sizehint=0): total = 0 - list = [] + inlist = [] while True: line = self.readline() if not line: break - list.append(line) + inlist.append(line) total += len(line) if sizehint and total >= sizehint: break - return list + return inlist def fileno(self): return self._sock.fileno() diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index df721a518..2fa26fef8 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -57,7 +57,7 @@ def get_domain_name(): try: config.init_config() domain_name = config.config.get_domain() - except Exception, e: + except Exception: return None return domain_name @@ -362,13 +362,13 @@ class GeneralizedTimeZone(datetime.tzinfo): if self.houroffset < 0: self.minoffset *= -1 - def utcoffset(self, dt): + def utcoffset(self): return datetime.timedelta(hours=self.houroffset, minutes=self.minoffset) - def dst(self, dt): + def dst(self): return datetime.timedelta(0) - def tzname(self, dt): + def tzname(self): return self.name @@ -748,7 +748,7 @@ class AttributeValueCompleter: return self.default_value.get(self.lhs, None) elif default_value_type is FunctionType: return self.default_value(self.lhs) - elif default_value_type is StringsType: + elif default_value_type is StringType: return self.default_value else: return None @@ -759,7 +759,7 @@ class AttributeValueCompleter: else: self.completions = self.lhs_names - def complete(self, text, state): + def complete(self, state): self.line_buffer= readline.get_line_buffer() self.parse_input() if not self.lhs_complete: @@ -905,7 +905,7 @@ class ItemCompleter: else: self.completions = self.items - def complete(self, text, state): + def complete(self, state): self.line_buffer= readline.get_line_buffer() if state == 0: beg = readline.get_begidx() diff --git a/ipapython/nsslib.py b/ipapython/nsslib.py index 1b678305b..05b45c14a 100644 --- a/ipapython/nsslib.py +++ b/ipapython/nsslib.py @@ -20,7 +20,6 @@ import httplib import getpass import socket -import errno from nss.error import NSPRError import nss.io as io diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py index 503f38b24..78c9b6283 100644 --- a/ipapython/sysrestore.py +++ b/ipapython/sysrestore.py @@ -25,7 +25,6 @@ import os import os.path -import errno import shutil import logging import ConfigParser @@ -110,13 +109,13 @@ class FileStore: logging.debug(" -> Not backing up - '%s' doesn't exist", path) return - (reldir, file) = os.path.split(path) + (reldir, backupfile) = os.path.split(path) filename = "" for i in range(8): h = "%02x" % self.random.randint(0,255) filename += h - filename += "-"+file + filename += "-"+backupfile backup_path = os.path.join(self._path, filename) if os.path.exists(backup_path): diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index cadab10e3..f5a704614 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -17,12 +17,9 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # -import string import tempfile -import shutil import os import pwd -import socket import logging import installutils @@ -73,7 +70,7 @@ class BindInstance(service.Service): tmp = ip_address.split(".") tmp.reverse() - + self.reverse_host = tmp.pop(0) self.reverse_subnet = ".".join(tmp) @@ -232,7 +229,6 @@ class BindInstance(service.Service): def uninstall(self): running = self.restore_state("running") enabled = self.restore_state("enabled") - domain = self.restore_state("domain") if not running is None: self.stop() diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index a8a1a774c..5ade47160 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -575,7 +575,7 @@ class CAInstance(service.Service): def __restart_instance(self): try: self.restart() - except Exception, e: + except Exception: # TODO: roll back here? logging.critical("Failed to restart the certificate server. See the installation log for details.") @@ -625,8 +625,6 @@ class CAInstance(service.Service): params['submit'] = 'submit' params['requestNotes'] = '' params = urllib.urlencode(params) - headers = {"Content-type": "application/x-www-form-urlencoded", - "Accept": "text/plain"} # Now issue the RA certificate. args = [ @@ -826,8 +824,6 @@ class CAInstance(service.Service): os.chown(self.ra_agent_pwd, pent.pw_uid, pent.pw_gid) def __setup_sign_profile(self): - caconfig = "/var/lib/pki-ca/conf/CS.cfg" - # Tell the profile to automatically issue certs for RAs installutils.set_directive('/var/lib/pki-ca/profiles/ca/caJarSigningCert.cfg', 'auth.instance_id', 'raCertAuth', quotes=False, separator='=') diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index 76eb8dd95..f66959bc1 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -26,7 +26,6 @@ import sys import os import re import time -import stat from ipapython import ipautil @@ -36,7 +35,7 @@ import certs import ldap from ipaserver import ipaldap from ipaserver.install import ldapupdate -from ipalib import util +from ipalib import util, errors SERVER_ROOT_64 = "/usr/lib64/dirsrv" SERVER_ROOT_32 = "/usr/lib/dirsrv" @@ -319,7 +318,7 @@ class DsInstance(service.Service): dsdb.create_from_pkcs12(self.pkcs12_info[0], self.pkcs12_info[1]) server_certs = dsdb.find_server_certs() if len(server_certs) == 0: - raise RuntimeError("Could not find a suitable server cert in import in %s" % pkcs12_info[0]) + raise RuntimeError("Could not find a suitable server cert in import in %s" % self.pkcs12_info[0]) # We only handle one server cert nickname = server_certs[0][0] @@ -453,7 +452,7 @@ class DsInstance(service.Service): status = True try: certdb.load_cacert(cacert_fname) - except ipalib.CalledProcessError, e: + except errors.CalledProcessError, e: logging.critical("Error importing CA cert file named [%s]: %s" % (cacert_fname, str(e))) status = False diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py index ba9de6138..b6483fb3d 100644 --- a/ipaserver/install/httpinstance.py +++ b/ipaserver/install/httpinstance.py @@ -19,13 +19,9 @@ import os import os.path -import subprocess -import string import tempfile import logging import pwd -import fileinput -import sys import shutil import service @@ -105,8 +101,8 @@ class HTTPInstance(service.Service): if selinux: try: # returns e.g. "httpd_can_network_connect --> off" - (stdout, stderr) = ipautils.run(["/usr/sbin/getsebool", - "httpd_can_network_connect"]) + (stdout, stderr) = ipautil.run(["/usr/sbin/getsebool", + "httpd_can_network_connect"]) self.backup_state("httpd_can_network_connect", stdout.split()[2]) except: pass @@ -170,7 +166,7 @@ class HTTPInstance(service.Service): db.create_from_pkcs12(self.pkcs12_info[0], self.pkcs12_info[1], passwd="") server_certs = db.find_server_certs() if len(server_certs) == 0: - raise RuntimeError("Could not find a suitable server cert in import in %s" % pkcs12_info[0]) + raise RuntimeError("Could not find a suitable server cert in import in %s" % self.pkcs12_info[0]) db.create_password_conf() # We only handle one server cert diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index c12023719..a41176cc7 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -25,7 +25,6 @@ import os import re import fileinput import sys -import time import struct import fcntl @@ -217,28 +216,28 @@ def set_directive(filename, directive, value, quotes=True, separator=' '): """ valueset = False fd = open(filename) - file = [] + newfile = [] for line in fd: if directive in line: valueset = True if quotes: - file.append('%s%s"%s"\n' % (directive, separator, value)) + newfile.append('%s%s"%s"\n' % (directive, separator, value)) else: - file.append('%s%s%s\n' % (directive, separator, value)) + newfile.append('%s%s%s\n' % (directive, separator, value)) else: - file.append(line) + newfile.append(line) fd.close() if not valueset: if quotes: - file.append('%s%s"%s"\n' % (directive, separator, value)) + newfile.append('%s%s"%s"\n' % (directive, separator, value)) else: - file.append('%s%s%s\n' % (directive, separator, value)) + newfile.append('%s%s%s\n' % (directive, separator, value)) fd = open(filename, "w") - fd.write("".join(file)) + fd.write("".join(newfile)) fd.close() -def get_directive(filename, directive, strip_quotes=True, separator=' '): +def get_directive(filename, directive, separator=' '): """ A rather inefficient way to get a configuration directive. """ diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 1c3489725..a6caa81eb 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -17,8 +17,6 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # -import subprocess -import string import shutil import logging import fileinput @@ -27,7 +25,6 @@ import sys import os import pwd import socket -import shutil import service import installutils @@ -46,7 +43,6 @@ from pyasn1.type import univ, namedtype import pyasn1.codec.ber.encoder import pyasn1.codec.ber.decoder import struct -import base64 KRBMKEY_DENY_ACI = """ (targetattr = "krbMKey")(version 3.0; acl "No external access"; deny (all) userdn != "ldap:///uid=kdc,cn=sysaccounts,cn=etc,$SUFFIX";) @@ -225,7 +221,7 @@ class KrbInstance(service.Service): msgid = self.conn.search("cn=mapping,cn=sasl,cn=config", ldap.SCOPE_ONELEVEL, "(objectclass=nsSaslMapping)") res = self.conn.result(msgid) for r in res[1]: - mid = self.conn.delete_s(r[0]) + self.conn.delete_s(r[0]) #except LDAPError, e: # logging.critical("Error during SASL mapping removal: %s" % str(e)) except Exception, e: @@ -301,7 +297,7 @@ class KrbInstance(service.Service): def __write_stash_from_ds(self): try: entry = self.conn.getEntry("cn=%s, cn=kerberos, %s" % (self.realm, self.suffix), ldap.SCOPE_SUBTREE) - except errors.NotFound: + except errors.NotFound, e: logging.critical("Could not find master key in DS") raise e diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py index 3f76551e8..2c53b5611 100644 --- a/ipaserver/install/ntpinstance.py +++ b/ipaserver/install/ntpinstance.py @@ -17,7 +17,6 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # -import shutil import logging import service diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py index 653ec3c4e..242352975 100644 --- a/ipaserver/install/replication.py +++ b/ipaserver/install/replication.py @@ -27,11 +27,11 @@ from ipalib import util from ipalib import errors DIRMAN_CN = "cn=directory manager" -CACERT="/usr/share/ipa/html/ca.crt" +CACERT = "/usr/share/ipa/html/ca.crt" # the default container used by AD for user entries -WIN_USER_CONTAINER="cn=Users" +WIN_USER_CONTAINER = "cn=Users" # the default container used by IPA for user entries -IPA_USER_CONTAINER="cn=users,cn=accounts" +IPA_USER_CONTAINER = "cn=users,cn=accounts" PORT = 636 TIMEOUT = 120 @@ -351,9 +351,11 @@ class ReplicationManager: def check_repl_init(self, conn, agmtdn): done = False hasError = 0 - attrlist = ['cn', 'nsds5BeginReplicaRefresh', 'nsds5replicaUpdateInProgress', - 'nsds5ReplicaLastInitStatus', 'nsds5ReplicaLastInitStart', - 'nsds5ReplicaLastInitEnd'] + attrlist = ['cn', 'nsds5BeginReplicaRefresh', + 'nsds5replicaUpdateInProgress', + 'nsds5ReplicaLastInitStatus', + 'nsds5ReplicaLastInitStart', + 'nsds5ReplicaLastInitEnd'] entry = conn.getEntry(agmtdn, ldap.SCOPE_BASE, "(objectclass=*)", attrlist) if not entry: print "Error reading status from agreement", agmtdn diff --git a/ipaserver/ipautil.py b/ipaserver/ipautil.py index 6422fe5a6..7042c1ce2 100644 --- a/ipaserver/ipautil.py +++ b/ipaserver/ipautil.py @@ -17,11 +17,13 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # -import string import xmlrpclib import re def realm_to_suffix(realm_name): + """ + Convert a kerberos realm into the IPA suffix. + """ s = realm_name.split(".") terms = ["dc=" + x.lower() for x in s] return ",".join(terms) @@ -37,32 +39,32 @@ class CIDict(dict): If you extend UserDict, isinstance(foo, dict) returns false. """ - def __init__(self,default=None): + def __init__(self, default=None): super(CIDict, self).__init__() self._keys = {} self.update(default or {}) - def __getitem__(self,key): - return super(CIDict,self).__getitem__(string.lower(key)) + def __getitem__(self, key): + return super(CIDict, self).__getitem__(key.lower()) - def __setitem__(self,key,value): - lower_key = string.lower(key) + def __setitem__(self, key, value): + lower_key = key.lower() self._keys[lower_key] = key - return super(CIDict,self).__setitem__(string.lower(key),value) + return super(CIDict, self).__setitem__(lower_key, value) - def __delitem__(self,key): - lower_key = string.lower(key) + def __delitem__(self, key): + lower_key = key.lower() del self._keys[lower_key] - return super(CIDict,self).__delitem__(string.lower(key)) + return super(CIDict, self).__delitem__(key.lower()) - def update(self,dict): + def update(self, dict): for key in dict.keys(): self[key] = dict[key] - def has_key(self,key): - return super(CIDict, self).has_key(string.lower(key)) + def has_key(self, key): + return super(CIDict, self).has_key(key.lower()) - def get(self,key,failobj=None): + def get(self, key, failobj=None): try: return self[key] except KeyError: @@ -74,7 +76,7 @@ class CIDict(dict): def items(self): result = [] for k in self._keys.values(): - result.append((k,self[k])) + result.append((k, self[k])) return result def copy(self): @@ -89,7 +91,7 @@ class CIDict(dict): def iterkeys(self): return self.copy().iterkeys() - def setdefault(self,key,value=None): + def setdefault(self, key, value=None): try: return self[key] except KeyError: @@ -107,11 +109,11 @@ class CIDict(dict): raise def popitem(self): - (lower_key,value) = super(CIDict,self).popitem() + (lower_key, value) = super(CIDict, self).popitem() key = self._keys[lower_key] del self._keys[lower_key] - return (key,value) + return (key, value) # @@ -127,10 +129,10 @@ SAFE_STRING_PATTERN = '(^(\000|\n|\r| |:|<)|[\000\n\r\200-\377]+|[ ]+$)' safe_string_re = re.compile(SAFE_STRING_PATTERN) def needs_base64(s): - """ - returns 1 if s has to be base-64 encoded because of special chars - """ - return not safe_string_re.search(s) is None + """ + returns 1 if s has to be base-64 encoded because of special chars + """ + return not safe_string_re.search(s) is None def wrap_binary_data(data): @@ -148,7 +150,7 @@ def wrap_binary_data(data): return retval elif isinstance(data, dict): retval = {} - for (k,v) in data.iteritems(): + for (k, v) in data.iteritems(): retval[k] = wrap_binary_data(v) return retval else: @@ -170,7 +172,7 @@ def unwrap_binary_data(data): return retval elif isinstance(data, dict): retval = {} - for (k,v) in data.iteritems(): + for (k, v) in data.iteritems(): retval[k] = unwrap_binary_data(v) return retval else: @@ -181,21 +183,21 @@ def get_gsserror(e): in python 2.5, deal with it.""" try: - primary = e[0] - secondary = e[1] - except: - primary = e[0][0] - secondary = e[0][1] + primary = e[0] + secondary = e[1] + except Exception: + primary = e[0][0] + secondary = e[0][1] return (primary[0], secondary[0]) def utf8_encode_value(value): - if isinstance(value,unicode): + if isinstance(value, unicode): return value.encode('utf-8') return value def utf8_encode_values(values): - if isinstance(values,list) or isinstance(values,tuple): + if isinstance(values, list) or isinstance(values, tuple): return map(utf8_encode_value, values) else: return utf8_encode_value(values) diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index 4df288dfc..9587cbe2e 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -28,16 +28,12 @@ Backend plugin for LDAP. import copy import os -import re import socket import string -import krbV import ldap as _ldap import ldap.filter as _ldap_filter import ldap.sasl as _ldap_sasl -from ldap.controls import LDAPControl -from ldap.ldapobject import SimpleLDAPObject # for backward compatibility from ldap.functions import explode_dn diff --git a/ipaserver/servercore.py b/ipaserver/servercore.py index 74014c758..ed490312b 100644 --- a/ipaserver/servercore.py +++ b/ipaserver/servercore.py @@ -18,11 +18,8 @@ # import ldap -import string -import re from ipalib.request import context from ipaserver import ipaldap -import ipautil from ipalib import errors from ipalib import api -- cgit