Merge lp:~jdstrand/click-apparmor/click-apparmor.lstat into lp:click-apparmor

Proposed by Jamie Strandboge
Status: Superseded
Proposed branch: lp:~jdstrand/click-apparmor/click-apparmor.lstat
Merge into: lp:click-apparmor
Diff against target: 101 lines (+60/-2)
4 files modified
apparmor/click.py (+9/-2)
debian/changelog (+9/-0)
debian/control (+3/-0)
test-clicktool.py (+39/-0)
To merge this branch: bzr merge lp:~jdstrand/click-apparmor/click-apparmor.lstat
Reviewer Review Type Date Requested Status
Steve Beattie Approve
Review via email: mp+210671@code.launchpad.net

This proposal has been superseded by a proposal from 2014-03-13.

Commit message

regenerate policy if hook symlink is newer than the profile (LP: #1291549)
debian/control: update for CI Train
- Set X-Auto-Uploader to no-rewrite-version
- Set Vcs-Bzr to the new target branch

Description of the change

High priority fix for bug #1291549. In essence, apps that were installed via the store had policy generated using CLICK_DIR="/top/click.ubuntu.com". Then when a preinstalled app used the same version in the store, click did not update the symlinks in the hooks directory to point to /usr/share/click/preinstalled. Click was then updated to update the symlinks to prefer the lowest overlay (practically speaking, to use /usr/share/click/preinstalled when it and /opt/click.ubuntu.com have the same version). However, while click was updated to update the symlinks, click-apparmor did not check to see if the hook symlink was newer than the generated profile. This branch fixes that.

Checklist:
 * Is your branch in sync with latest trunk (e.g. bzr pull lp:click-apparmor -> no changes): yes

 * Did you build your software in a clean sbuild/pbuilder chroot or ppa? yes

 * Did you build your software in a clean sbuild/pbuilder chroot or ppa on armhf? yes

 * Does the package's autopkgtests pass with exit status '0'? yes

 * Has your component TestPlan been executed successfully on emulator/supported device? yes

 * Has a 5 minute exploratory testing run been executed on emulator/supported device? yes

 * If you changed the packaging (debian), did you subscribe a core-dev to this MP? n/a

 * What components might get impacted by your changes? none

 * Have you requested review by the teams of these owning components? n/a

To post a comment you must log in.
Revision history for this message
Steve Beattie (sbeattie) wrote :

This change looks correct. Thanks!

review: Approve
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Set to approved based on Steve's review.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

MP Review Checklist
 * Are any changes against your component pending/needed to land the MP under review in a functional state and are those called out explicitly by the submitter? no

 * Did you do exploratory testing related to the component you own with the MP changeset included? yes

 * Has the submitter requested review by all the relevant teams/reviewers? yes

 * If you are the reviewer owning the component the MP is against, have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut? I requested and reviewed. I doubled checked I didn't miss anything

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Note, I plan to update the comment in the test script to not reference symlinks since it isn't actually creating them, but I will do that in click-apparmor 0.2 rather than here to not delay on re-testing.

98. By Jamie Strandboge

* debian/control: update for CI Train
  - Set X-Auto-Uploader to no-rewrite-version
  - Set Vcs-Bzr to the new target branch

99. By Jamie Strandboge

update debian/changelog

100. By Jamie Strandboge

rev version in an attempt to get build going in CI Train

101. By Jamie Strandboge

rev version to 0.1.15.2 and set distribution name to UNTRUSTED

102. By Jamie Strandboge

fix typo in last commit

103. By Jamie Strandboge

rev version

104. By Jamie Strandboge

adjust urgency to low

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'apparmor/click.py'
2--- apparmor/click.py 2014-02-05 22:39:37 +0000
3+++ apparmor/click.py 2014-03-13 01:54:51 +0000
4@@ -570,8 +570,15 @@
5 result = []
6 for hook in os.listdir(hooksdir):
7 name = AppName(click_name=hook)
8- if not os.path.exists(os.path.join(profilesdir,
9- name.profile_filename)):
10+ profile = os.path.join(profilesdir, name.profile_filename)
11+ hook_full = os.path.join(hooksdir, hook)
12+ if not os.path.exists(profile):
13+ # If profile doesn't exist, we need to generate it
14+ result.append(name.click_name)
15+ elif os.lstat(hook_full).st_mtime > os.stat(profile).st_mtime:
16+ # If the profile exists, but the hook symlink is newer, we need to
17+ # regenerate it. Click may update the symlink from time to time, so
18+ # we need to handle this (LP: #1291549)
19 result.append(name.click_name)
20 return result
21
22
23=== modified file 'debian/changelog'
24--- debian/changelog 2014-02-05 22:39:37 +0000
25+++ debian/changelog 2014-03-13 01:54:51 +0000
26@@ -1,3 +1,12 @@
27+click-apparmor (0.1.15.1) trusty; urgency=medium
28+
29+ * regenerate policy if hook symlink is newer than the profile (LP: #1291549)
30+ * debian/control: update for CI Train
31+ - Set X-Auto-Uploader to no-rewrite-version
32+ - Set Vcs-Bzr to the new target branch
33+
34+ -- Jamie Strandboge <jamie@ubuntu.com> Wed, 12 Mar 2014 20:16:48 -0500
35+
36 click-apparmor (0.1.14) trusty; urgency=medium
37
38 * implement autopkgtests
39
40=== modified file 'debian/control'
41--- debian/control 2014-02-05 22:39:37 +0000
42+++ debian/control 2014-03-13 01:54:51 +0000
43@@ -12,6 +12,9 @@
44 libnih-dbus1,
45 apparmor-easyprof-ubuntu
46 Standards-Version: 3.9.4
47+Vcs-Bzr: https://code.launchpad.net/~ubuntu-security/click-apparmor/trunk
48+Vcs-Browser: http://bazaar.launchpad.net/~ubuntu-security/click-apparmor/trunk/files
49+X-Auto-Uploader: no-rewrite-version
50 X-Python3-Version: >= 3.3
51 XS-Testsuite: autopkgtest
52
53
54=== modified file 'test-clicktool.py'
55--- test-clicktool.py 2014-02-05 22:39:37 +0000
56+++ test-clicktool.py 2014-03-13 01:54:51 +0000
57@@ -1347,6 +1347,45 @@
58 self.assertEquals(expected, result,
59 "Expected to get no click hooks, got %s" % (result))
60
61+ def test_two_equal_directories_new_symlink(self):
62+ '''Test two equal directories with new symlink (LP: #1291549)'''
63+ c = self.clickstate
64+ clicks = ["alpha_beta_gamma", "click_click_version",
65+ "wat_no-really_wat"]
66+ for cname in clicks:
67+ with open(os.path.join(c.click_dir, '%s.json' %
68+ (cname)), 'w+') as f:
69+ f.write('invalid json here')
70+ with open(os.path.join(c.profiles_dir, 'click_%s' %
71+ (cname)), 'w+') as f:
72+ f.write('profile %s { }' % (cname))
73+
74+ # No symlinks update yet, so everything should be the same
75+ expected = []
76+ result = click.get_missing_profiles(c.click_dir, c.profiles_dir)
77+ self.assertEquals(expected, result,
78+ "Expected to get no profiles, got %s" % (result))
79+ result = click.get_missing_clickhooks(c.click_dir, c.profiles_dir)
80+ self.assertEquals(expected, result,
81+ "Expected to get no click hooks, got %s" % (result))
82+
83+ time.sleep(1)
84+ clicks = ["alpha_beta_gamma", "click_click_version"]
85+ for cname in clicks:
86+ with open(os.path.join(c.click_dir, '%s.json' %
87+ (cname)), 'w+') as f:
88+ f.write('invalid json here')
89+
90+ expected = []
91+ result = click.get_missing_clickhooks(c.click_dir, c.profiles_dir)
92+ self.assertEquals(expected, result,
93+ "Expected to get no click hooks, got %s" % (result))
94+ expected = len(clicks)
95+ result = click.get_missing_profiles(c.click_dir, c.profiles_dir)
96+ self.assertEquals(expected, len(result),
97+ "Expected to get %d profiles, got %s:\n" %
98+ (expected, len(result)))
99+
100
101 class AppArmorPolicyModificationTests(unittest.TestCase):
102

Subscribers

People subscribed via source and target branches