launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05548
[Merge] lp:~lifeless/python-oops-datedir-repo/bug-891251 into lp:python-oops-datedir-repo
Robert Collins has proposed merging lp:~lifeless/python-oops-datedir-repo/bug-891251 into lp:python-oops-datedir-repo.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #891251 in Python OOPS Date-dir repository: "txlongpoll oopses are recorded with the wrong permission, causing oops loader script to fail with a permission denied error"
https://bugs.launchpad.net/python-oops-datedir-repo/+bug/891251
For more details, see:
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/bug-891251/+merge/82469
Allow txlongpoll oopses to be analysed and viewed.
--
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/bug-891251/+merge/82469
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-datedir-repo/bug-891251 into lp:python-oops-datedir-repo.
=== modified file 'NEWS'
--- NEWS 2011-11-16 03:44:36 +0000
+++ NEWS 2011-11-17 00:02:23 +0000
@@ -6,6 +6,13 @@
NEXT
----
+0.0.13
+------
+
+* The date directory created when writing an OOPS with hashed names has its
+ permissions chmodded in the same way that the OOPS report file itself does.
+ (Robert Collins, #891251)
+
0.0.12
------
=== modified file 'oops_datedir_repo/__init__.py'
--- oops_datedir_repo/__init__.py 2011-11-16 03:44:36 +0000
+++ oops_datedir_repo/__init__.py 2011-11-17 00:02:23 +0000
@@ -25,7 +25,7 @@
# established at this point, and setup.py will use a version of next-$(revno).
# If the releaselevel is 'final', then the tarball will be major.minor.micro.
# Otherwise it is major.minor.micro~$(revno).
-__version__ = (0, 0, 12, 'beta', 0)
+__version__ = (0, 0, 13, 'beta', 0)
__all__ = [
'DateDirRepo',
=== modified file 'oops_datedir_repo/repository.py'
--- oops_datedir_repo/repository.py 2011-11-14 03:20:31 +0000
+++ oops_datedir_repo/repository.py 2011-11-17 00:02:23 +0000
@@ -103,6 +103,11 @@
:param now: The datetime to use as the current time. Will be
determined if not supplied. Useful for testing.
"""
+ # We set file permission to: rw-r--r-- (so that reports from
+ # umask-restricted services can be gathered by a tool running as
+ # another user).
+ wanted_file_permission = (
+ stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
if now is not None:
now = now.astimezone(utc)
else:
@@ -118,6 +123,10 @@
prefix = os.path.join(self.root, now.strftime('%Y-%m-%d'))
if not os.path.isdir(prefix):
os.makedirs(prefix)
+ # For directories we need to set the x bits too.
+ os.chmod(
+ prefix, wanted_file_permission | stat.S_IXUSR | stat.S_IXGRP |
+ stat.S_IXOTH)
filename = os.path.join(prefix, oopsid)
if self.inherit_id:
oopsid = report.get('id') or oopsid
@@ -126,12 +135,7 @@
os.rename(filename + '.tmp', filename)
if self.stash_path:
original_report['datedir_repo_filepath'] = filename
- # Set file permission to: rw-r--r-- (so that reports from
- # umask-restricted services can be gathered by a tool running as
- # another user).
- wanted_permission = (
- stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
- os.chmod(filename, wanted_permission)
+ os.chmod(filename, wanted_file_permission)
return report['id']
def republish(self, publisher):
=== modified file 'oops_datedir_repo/tests/test_repository.py'
--- oops_datedir_repo/tests/test_repository.py 2011-11-14 03:20:31 +0000
+++ oops_datedir_repo/tests/test_repository.py 2011-11-17 00:02:23 +0000
@@ -22,11 +22,17 @@
import os.path
import stat
-from fixtures import TempDir
+from fixtures import (
+ Fixture,
+ TempDir,
+ )
import bson
from pytz import utc
import testtools
-from testtools.matchers import raises
+from testtools.matchers import (
+ Equals,
+ raises,
+ )
from oops_datedir_repo import (
DateDirRepo,
@@ -35,9 +41,39 @@
from oops_datedir_repo.uniquefileallocator import UniqueFileAllocator
+class HasUnixPermissions:
+
+ def __init__(self, wanted_permission):
+ self.wanted_permission = wanted_permission
+
+ def match(self, path):
+ st = os.stat(path)
+ # Get only the permission bits from this mode.
+ file_permission = stat.S_IMODE(st.st_mode)
+ return Equals(self.wanted_permission).match(file_permission)
+
+ def __str__(self):
+ # TODO: might be nice to split out the bits in future, format nicely
+ # etc. Should also move this into testtools.
+ return "HasUnixPermissions(0%o)" % self.wanted_permission
+
+
+class UMaskFixture(Fixture):
+ """Set a umask temporarily."""
+
+ def __init__(self, mask):
+ super(UMaskFixture, self).__init__()
+ self.umask_permission = mask
+
+ def setUp(self):
+ super(UMaskFixture, self).setUp()
+ old_umask = os.umask(self.umask_permission)
+ self.addCleanup(os.umask, old_umask)
+
+
class TestDateDirRepo(testtools.TestCase):
- def test_publish_permissions(self):
+ def test_publish_permissions_lognamer(self):
errordir = self.useFixture(TempDir()).path
repo = DateDirRepo(errordir, 'T')
report = {'id': 'OOPS-91T1'}
@@ -45,21 +81,36 @@
# Set up default file creation mode to rwx------ as some restrictive
# servers do.
- umask_permission = stat.S_IRWXG | stat.S_IRWXO
- old_umask = os.umask(umask_permission)
- self.addCleanup(os.umask, old_umask)
+ self.useFixture(UMaskFixture(stat.S_IRWXG | stat.S_IRWXO))
repo.publish(report, now)
errorfile = os.path.join(repo.log_namer.output_dir(now), '01800.T1')
- self.assertTrue(os.path.exists(errorfile))
-
- # Check errorfile is set with the correct permission: rw-r--r--
- st = os.stat(errorfile)
- wanted_permission = (
- stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
- # Get only the permission bits for this file.
- file_permission = stat.S_IMODE(st.st_mode)
- self.assertEqual(file_permission, wanted_permission)
+ # Check errorfile and directory are set with the correct permission:
+ # rw-r--r-- and rwxr-xr-x accordingly
+ file_perms = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
+ has_file_permission = HasUnixPermissions(file_perms)
+ has_dir_permission = HasUnixPermissions(
+ file_perms | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
+ self.assertThat(errorfile, has_file_permission)
+ self.assertThat(repo.log_namer.output_dir(now), has_dir_permission)
+
+ def test_publish_permissions_hashnames(self):
+ repo = DateDirRepo(self.useFixture(TempDir()).path, stash_path=True)
+ report = {'id': 'OOPS-91T1'}
+ now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=utc)
+
+ # Set up default file creation mode to rwx------ as some restrictive
+ # servers do.
+ self.useFixture(UMaskFixture(stat.S_IRWXG | stat.S_IRWXO))
+ repo.publish(report, now)
+ # Check errorfile and directory are set with the correct permission:
+ # rw-r--r--
+ file_perms = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
+ has_file_permission = HasUnixPermissions(file_perms)
+ has_dir_permission = HasUnixPermissions(
+ file_perms | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
+ self.assertThat(report['datedir_repo_filepath'], has_file_permission)
+ self.assertThat(repo.root + '/2006-04-01', has_dir_permission)
def test_sets_log_namer_to_a_UniqueFileAllocator(self):
repo = DateDirRepo(self.useFixture(TempDir()).path, 'T')
=== modified file 'setup.py'
--- setup.py 2011-11-16 03:44:36 +0000
+++ setup.py 2011-11-17 00:02:23 +0000
@@ -22,7 +22,7 @@
description = file(os.path.join(os.path.dirname(__file__), 'README'), 'rb').read()
setup(name="oops_datedir_repo",
- version="0.0.12",
+ version="0.0.13",
description="OOPS disk serialisation and repository management.",
long_description=description,
maintainer="Launchpad Developers",
