From 1b77bf2e16e452ceb98e496106a899a44a5d2e01 Mon Sep 17 00:00:00 2001 From: Jonathan Kamens Date: Wed, 24 May 2017 09:41:12 -0400 Subject: [PATCH 1/3] Fix incorrect variable reference --- afs/pts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/afs/pts.py b/afs/pts.py index e9f3c98..19828c7 100644 --- a/afs/pts.py +++ b/afs/pts.py @@ -378,7 +378,7 @@ def getEntry(self, ident): if isinstance(ident, PTEntry): if ident._pts is not self: raise TypeError("Entry '%s' is from a different cell." % - elt) + ident) return ident elif isinstance(ident, basestring): From 618758669028db82f5d0ae938c40491a8b7e704e Mon Sep 17 00:00:00 2001 From: Jonathan Kamens Date: Wed, 24 May 2017 09:41:21 -0400 Subject: [PATCH 2/3] Flake8 --- afs/__init__.py | 2 +- afs/acl.py | 30 +++++++++++++++++++++--------- afs/fs.py | 13 +++++++++---- afs/pts.py | 13 ++++++++++++- afs/tests/test__pts.py | 10 +++++++++- afs/tests/test_acl.py | 15 +++++++++++---- 6 files changed, 63 insertions(+), 20 deletions(-) diff --git a/afs/__init__.py b/afs/__init__.py index d717c3d..edd9511 100644 --- a/afs/__init__.py +++ b/afs/__init__.py @@ -3,7 +3,7 @@ # you type "import afs" import afs.fs import afs.acl -import afs.pts +import afs.pts # noqa # Basic logging support for this package. # Add a NullHandler to avoid "No handlers could be found" error diff --git a/afs/acl.py b/afs/acl.py index 20a416f..4d31b18 100644 --- a/afs/acl.py +++ b/afs/acl.py @@ -1,7 +1,6 @@ from afs import _acl from afs._acl import READ, WRITE, INSERT, LOOKUP, DELETE, LOCK, ADMINISTER, \ USR0, USR1, USR2, USR3, USR4, USR5, USR6, USR7 -from afs._acl import getCallerAccess _canonical = { "read": "rl", @@ -41,18 +40,23 @@ def rightsToEnglish(s): else: return '' + def readRights(s): """Canonicalizes string rights to bitmask""" - if s in _canonical: s = _canonical[s] + if s in _canonical: + s = _canonical[s] return _parseRights(s) + def showRights(r): """Takes a bitmask and returns a rwlidka string""" s = "" - for char,mask in _charBitAssoc: - if r & mask == mask: s += char + for char, mask in _charBitAssoc: + if r & mask == mask: + s += char return s + def _parseRights(s): """Parses a rwlid... rights tring to bitmask""" r = 0 @@ -63,13 +67,15 @@ def _parseRights(s): raise ValueError return r + def _parseAcl(inp): lines = inp.split("\n") npos = int(lines[0].split(" ")[0]) pos = {} neg = {} for l in lines[2:]: - if l == "": continue + if l == "": + continue name, acl = l.split() if npos: npos -= 1 @@ -79,6 +85,7 @@ def _parseAcl(inp): neg[name] = int(acl) return (pos, neg) + def _unparseAcl(pos, neg): npos = len(pos) nneg = len(neg) @@ -89,6 +96,7 @@ def _unparseAcl(pos, neg): acl += "%s\t%d\n" % n return acl + class ACL(object): def __init__(self, pos, neg): """ @@ -99,32 +107,36 @@ def __init__(self, pos, neg): """ self.pos = pos self.neg = neg + @staticmethod def retrieve(dir, follow=True): """Retrieve the ACL for an AFS directory""" pos, neg = _parseAcl(_acl.getAcl(dir, follow)) return ACL(pos, neg) + def apply(self, dir, follow=True): """Apply the ACL to a directory""" self._clean() _acl.setAcl(dir, _unparseAcl(self.pos, self.neg), follow) + def _clean(self): """Clean an ACL by removing any entries whose bitmask is 0""" - for n,a in self.pos.items(): + for n, a in self.pos.items(): if a == 0: del self.pos[n] - for n,a in self.neg.items(): + for n, a in self.neg.items(): if a == 0: del self.neg[n] + def set(self, user, bitmask, negative=False): """Set the bitmask for a given user""" if bitmask < 0 or bitmask > max(_char2bit.values()): - raise ValueError, "Invalid bitmask" + raise ValueError("Invalid bitmask") if negative: self.neg[user] = bitmask else: self.pos[user] = bitmask + def remove(self, user, negative=False): """Convenience function to removeSet the bitmask for a given user""" self.set(user, 0, negative) - diff --git a/afs/fs.py b/afs/fs.py index 4d254b6..248ffed 100644 --- a/afs/fs.py +++ b/afs/fs.py @@ -2,26 +2,26 @@ import os.path import socket import logging -import math -import sys from afs import _fs log = logging.getLogger('afs.fs') + class AFSVolumeStatus(object): def __init__(self, volstat): if type(volstat) is not tuple or len(volstat) != 3: raise TypeError("AFSVolumeStatus takes a tuple of (str,str,dict)") - if [type(x) for x in volstat] != [str,str,dict]: + if [type(x) for x in volstat] != [str, str, dict]: raise TypeError("AFSVolumeStatus takes a tuple of (str,str,dict)") self.name = volstat[0] self.offline_message = volstat[1] - for k,v in volstat[2].items(): + for k, v in volstat[2].items(): setattr(self, k, v) def __repr__(self): return self.__class__.__name__ + ': ' + repr(self.__dict__) + def whichcell(path): """Return the cell name or None if the path is not in AFS""" try: @@ -32,6 +32,7 @@ def whichcell(path): else: raise + def inafs(path): """Return True if a path is in AFS.""" try: @@ -42,6 +43,7 @@ def inafs(path): return True + def lsmount(path): """Return a volume name for a mountpoint.""" # os.path.realpath will take care of ensuring we don't @@ -56,6 +58,7 @@ def lsmount(path): else: raise + def examine(path): """ Given a path in AFS, tells you about the path and volume that @@ -80,6 +83,7 @@ def examine(path): pass return (AFSVolumeStatus(_volstat), _fid) + def _reverse_lookup(ip): """ Convenience function to provide best-effort reverse-resolution @@ -93,6 +97,7 @@ def _reverse_lookup(ip): log.warning("IndexError while reverse-resolving IP, shouldn't happen") return ip + def whereis(path, dns=True): """ Return a list of hostnames and/or IP addresses representing diff --git a/afs/pts.py b/afs/pts.py index 19828c7..e6678fd 100644 --- a/afs/pts.py +++ b/afs/pts.py @@ -6,6 +6,7 @@ except AttributeError: SetMixin = object + class PTRelationSet(SetMixin): """Collection class for the groups/members of a PTEntry. @@ -251,6 +252,7 @@ def __repr__(self): def _get_id(self): return self._id + def _set_id(self, val): del self._pts._cache[self._id] self._pts._ChangeEntry(self.id, newid=val) @@ -260,6 +262,7 @@ def _set_id(self, val): def _get_name(self): return self._name + def _set_name(self, val): self._pts._ChangeEntry(self.id, newname=val) self._name = val @@ -267,6 +270,7 @@ def _set_name(self, val): def _get_krbname(self): return self._pts._AfsToKrb5(self.name) + def _set_krbname(self, val): self.name = self._pts._Krb5ToAfs(val) krbname = property(_get_krbname, _set_krbname) @@ -279,6 +283,7 @@ def _get_count(self): def _get_flags(self): self._loadEntry() return self._flags + def _set_flags(self, val): self._pts._SetFields(self.id, access=val) self._flags = val @@ -287,6 +292,7 @@ def _set_flags(self, val): def _get_ngroups(self): self._loadEntry() return self._ngroups + def _set_ngroups(self, val): self._pts._SetFields(self.id, groups=val) self._ngroups = val @@ -295,6 +301,7 @@ def _set_ngroups(self, val): def _get_nusers(self): self._loadEntry() return self._nusers + def _set_nusers(self, val): self._pts._SetFields(self.id, users=val) self._nusers = val @@ -303,6 +310,7 @@ def _set_nusers(self, val): def _get_owner(self): self._loadEntry() return self._owner + def _set_owner(self, val): self._pts._ChangeEntry(self.id, newoid=self._pts.getEntry(val).id) self._owner = val @@ -319,7 +327,8 @@ def _loadEntry(self): for field in self._attrs: setattr(self, '_%s' % field, getattr(info, field)) for field in self._entry_attrs: - setattr(self, '_%s' % field, self._pts.getEntry(getattr(info, field))) + setattr(self, '_%s' % field, self._pts.getEntry( + getattr(info, field))) PTS_UNAUTH = 0 @@ -406,12 +415,14 @@ def expire(self): def _get_umax(self): return self._ListMax()[0] + def _set_umax(self, val): self._SetMaxUserId(val) umax = property(_get_umax, _set_umax) def _get_gmax(self): return self._ListMax()[1] + def _set_gmax(self, val): self._SetMaxGroupId(val) gmax = property(_get_gmax, _set_gmax) diff --git a/afs/tests/test__pts.py b/afs/tests/test__pts.py index 64297b1..cd6a9b8 100644 --- a/afs/tests/test__pts.py +++ b/afs/tests/test__pts.py @@ -2,6 +2,7 @@ from afs._pts import PTS import nose + def get_this_cell(): # Feel free to add more places ThisCell might show up to_try = ['/private/var/db/openafs/etc/ThisCell', @@ -11,15 +12,19 @@ def get_this_cell(): if os.path.isfile(f): return open(f).read().strip() + def test_init_home_cell(): p = PTS() - assert p.cell == get_this_cell(), "PTS doesn't initialize to ThisCell when none specified." + assert p.cell == get_this_cell(), \ + "PTS doesn't initialize to ThisCell when none specified." + def test_init_other_cell(): cell = 'zone.mit.edu' p = PTS('zone.mit.edu') assert p.cell == cell, "PTS doesn't initialize to provided cell." + def test_user_name_to_id(): p = PTS() name = 'broder' @@ -27,6 +32,7 @@ def test_user_name_to_id(): assert id == 41803, "PTS can't convert user name to ID." assert p._IdToName(id) == name, "PTS can't convert user ID to name." + def test_group_name_to_id(): p = PTS() name = 'system:administrators' @@ -34,6 +40,7 @@ def test_group_name_to_id(): assert id == -204, "PTS can't convert group name to ID." assert p._IdToName(id) == name, "PTS can't convert group ID to name." + def test_name_or_id(): p = PTS() name = 'system:administrators' @@ -41,5 +48,6 @@ def test_name_or_id(): assert p._NameOrId(name) == id, "PTS._NameOrId can't identify name." assert p._NameOrId(id) == id, "PTS._NameOrId can't identify ID." + if __name__ == '__main__': nose.main() diff --git a/afs/tests/test_acl.py b/afs/tests/test_acl.py index aa4d1ae..6c3485d 100644 --- a/afs/tests/test_acl.py +++ b/afs/tests/test_acl.py @@ -1,21 +1,28 @@ import nose import afs.acl as acl + def test_showRights(): assert acl.showRights(acl.READ | acl.WRITE) == "rw" + def test_readRights(): assert acl.readRights('read') & acl.READ assert acl.readRights('read') & acl.LOOKUP assert not acl.readRights('read') & acl.WRITE + def test_retrieve(): - assert acl.ACL.retrieve('/afs/athena.mit.edu/contrib/bitbucket2').pos['system:anyuser'] & acl.WRITE - assert acl.ACL.retrieve('/afs/athena.mit.edu/user/t/a/tabbott').neg['yuranlu'] & acl.USR0 + assert acl.ACL.retrieve('/afs/athena.mit.edu/contrib/bitbucket2').\ + pos['system:anyuser'] & acl.WRITE + assert acl.ACL.retrieve('/afs/athena.mit.edu/user/t/a/tabbott').\ + neg['yuranlu'] & acl.USR0 + def test_getCallerAccess(): - assert acl.getCallerAccess('/afs/athena.mit.edu/contrib/bitbucket2') & acl.WRITE + assert acl.getCallerAccess('/afs/athena.mit.edu/contrib/bitbucket2') & \ + acl.WRITE + if __name__ == '__main__': nose.main() - From d055a0989ec2415a59f906debf65c999a06572f9 Mon Sep 17 00:00:00 2001 From: Jonathan Kamens Date: Wed, 24 May 2017 09:47:57 -0400 Subject: [PATCH 3/3] ENOSYS means AFS isn't enabled in the kernel If we get ENOSYS when attempting to treat something as an AFS path, then that means AFS isn't enabled in the kernel, so obviously it's not an AFS path and that's how we should treat it. Fixes https://debathena.mit.edu/trac/ticket/1584. --- afs/_fs.pyx | 2 +- afs/fs.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/afs/_fs.pyx b/afs/_fs.pyx index 0ee3359..622403d 100644 --- a/afs/_fs.pyx +++ b/afs/_fs.pyx @@ -93,7 +93,7 @@ def _whereis(char* path): _whereis(path) -> list() Low-level implementation of the "whereis" command. Raises - OSError, and EINVAL usually indicates the path isn't in AFS. + OSError, and EINVAL or ENOSYS usually indicates the path isn't in AFS. Returns a list of IP addresses. It is the caller's responsibility to get hostnames if that's desired. If anything goes wrong converting the 32-bit network numbers into IP addresses, that network number is diff --git a/afs/fs.py b/afs/fs.py index 248ffed..6e487dd 100644 --- a/afs/fs.py +++ b/afs/fs.py @@ -27,7 +27,7 @@ def whichcell(path): try: return _fs.whichcell(path) except OSError as e: - if e.errno == errno.EINVAL: + if e.errno in (errno.EINVAL, errno.ENOSYS): return None else: raise @@ -38,7 +38,7 @@ def inafs(path): try: _fs.whichcell(path) except OSError as e: - if e.errno in (errno.EINVAL, errno.ENOENT): + if e.errno in (errno.EINVAL, errno.ENOENT, errno.ENOSYS): return False return True @@ -53,7 +53,7 @@ def lsmount(path): try: return _fs._lsmount(dirname, basename) except OSError as e: - if e.errno == errno.EINVAL: + if e.errno in (errno.EINVAL, errno.ENOSYS): return None else: raise @@ -72,7 +72,7 @@ def examine(path): try: _volstat = _fs._volume_status(path) except OSError as e: - if e.errno == errno.EINVAL: + if e.errno in (errno.EINVAL, errno.ENOSYS): return None else: raise @@ -113,7 +113,7 @@ def whereis(path, dns=True): else: return addrs except OSError as e: - if e.errno == errno.EINVAL: + if e.errno in (errno.EINVAL, errno.ENOSYS): return None else: raise