Merge lp://staging/~gandelman-a/ubuntu/precise/keystone/lp900553 into lp://staging/~ubuntu-server-dev/keystone/essex

Proposed by Adam Gandelman
Status: Merged
Merged at revision: 48
Proposed branch: lp://staging/~gandelman-a/ubuntu/precise/keystone/lp900553
Merge into: lp://staging/~ubuntu-server-dev/keystone/essex
Diff against target: 29 lines (+9/-3)
2 files modified
debian/changelog (+7/-0)
debian/keystone.postinst (+2/-3)
To merge this branch: bzr merge lp://staging/~gandelman-a/ubuntu/precise/keystone/lp900553
Reviewer Review Type Date Requested Status
Chuck Short (community) Approve
Review via email: mp+84663@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Adam Gandelman (gandelman-a) wrote :

Resubmitting this to the correct packaging branch.

Revision history for this message
Chuck Short (zulcss) wrote :

lgtm

review: Approve
48. By Chuck Short

debian/keystone.postinst: Remove redundant chgrp. Also tighten permissions
on /var/lib/keystone and /etc/keystone (LP: #900553)

Revision history for this message
Scott Moser (smoser) wrote :

the only comment i have is really on the previous code that was here:
If the user had set up for one of these directories to be symlinked
elsewhere, the chown will not correctly affect the target.

example:
$ mkdir /tmp/tmpx
$ touch /tmp/tmpx/foo
$ ls -l /tmp/tmpx/foo
-rw-rw-r-- 1 smoser smoser 0 Dec 6 15:16 /tmp/tmpx/foo
$ ln -snf /tmp/tmpx /tmp/tmpx.link
$ ls -l /tmp/tmpx/foo /tmp/tmpx.link
-rw-rw-r-- 1 smoser smoser 0 Dec 6 15:16 /tmp/tmpx/foo
lrwxrwxrwx 1 smoser smoser 9 Dec 6 15:17 /tmp/tmpx.link -> /tmp/tmpx
$ sudo chown -R root:root /tmp/tmpx.link
$ ls -l /tmp/tmpx/foo /tmp/tmpx.link
-rw-rw-r-- 1 smoser smoser 0 Dec 6 15:16 /tmp/tmpx/foo
lrwxrwxrwx 1 root root 9 Dec 6 15:17 /tmp/tmpx.link -> /tmp/tmpx
$ sudo chown -R root:root /tmp/tmpx.link/
$ ls -l /tmp/tmpx/foo /tmp/tmpx.link
-rw-rw-r-- 1 root root 0 Dec 6 15:16 /tmp/tmpx/foo
lrwxrwxrwx 1 root root 9 Dec 6 15:17 /tmp/tmpx.link -> /tmp/tmpx

Basically, I think you should have the trailing / on the directories.

Revision history for this message
Adam Gandelman (gandelman-a) wrote :

Ah, good catch. Thanks, Scott. Pushed an update containing the trailing /'s

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches