Merge lp:~snappy-dev/click/binapparmor into lp:~snappy-dev/click/snappy

Proposed by Barry Warsaw
Status: Merged
Merged at revision: 583
Proposed branch: lp:~snappy-dev/click/binapparmor
Merge into: lp:~snappy-dev/click/snappy
Diff against target: 372 lines (+202/-16)
4 files modified
click/build.py (+18/-4)
click/tests/test_build.py (+175/-11)
click/tests/test_database.py (+2/-1)
debian/changelog (+7/-0)
To merge this branch: bzr merge lp:~snappy-dev/click/binapparmor
Reviewer Review Type Date Requested Status
James Hunt Pending
Jamie Strandboge Pending
Michael Vogt Pending
Review via email: mp+243875@code.launchpad.net

Description of the change

click (0.4.35+ppa27) vivid; urgency=medium

  * Give binaries default apparmor and apparmor-profile keys in their
    manifest when explicit profiles are not present in packages.yaml

 -- Barry Warsaw <email address hidden> Fri, 05 Dec 2014 16:53:51 -0500

With test cases! :)

I also repaired the test suite for Python 2.7 (tox) as bonus.

To post a comment you must log in.
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

The changelog entry is confusing, but it looks like the code DTRT. We should not be autogenerating the apparmor-profile key, only apparmor when apparmor and apparmor-profile aren't already present.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'click/build.py'
2--- click/build.py 2014-12-04 14:12:41 +0000
3+++ click/build.py 2014-12-05 21:56:27 +0000
4@@ -62,7 +62,7 @@
5 )
6
7
8-SNAPPY_APPARMOR_DEFAULT_TEMPLATE = """\
9+SNAPPY_SERVICE_APPARMOR_DEFAULT_TEMPLATE = u"""\
10 {
11 "template": "default",
12 "policy_groups": [
13@@ -73,6 +73,10 @@
14 }
15 """
16
17+# XXX For now, these are the same templates.
18+SNAPPY_BINARY_APPARMOR_DEFAULT_TEMPLATE = \
19+ SNAPPY_SERVICE_APPARMOR_DEFAULT_TEMPLATE
20+
21
22 @contextlib.contextmanager
23 def make_temp_dir():
24@@ -170,7 +174,8 @@
25 template_path = os.path.join(
26 'meta', '{}.apparmor'.format(name))
27 current_hook['apparmor'] = template_path
28- self.autogen_apparmor.append(template_path)
29+ self.autogen_apparmor.append(
30+ (SNAPPY_SERVICE_APPARMOR_DEFAULT_TEMPLATE, template_path))
31 self.manifest["hooks"] = hooks
32 # cleanup
33 if "services" in self.manifest:
34@@ -183,6 +188,15 @@
35 current_hook = hooks.get(base_name, {})
36 current_hook["bin-path"] = binary["name"]
37 hooks[base_name] = current_hook
38+ # Is there an explicit apparmor hook? If so, use it otherwise
39+ # generate a default one.
40+ if ( 'apparmor' not in current_hook and
41+ 'apparmor-profile' not in current_hook):
42+ template_path = os.path.join(
43+ 'meta', '{}.apparmor'.format(base_name))
44+ current_hook['apparmor'] = template_path
45+ self.autogen_apparmor.append(
46+ (SNAPPY_BINARY_APPARMOR_DEFAULT_TEMPLATE, template_path))
47 self.manifest["hooks"] = hooks
48 # cleanup
49 if "binaries" in self.manifest:
50@@ -342,10 +356,10 @@
51 if "framework" in self.manifest:
52 self._validate_framework(self.manifest["framework"])
53
54- for template_path in self.autogen_apparmor:
55+ for template, template_path in self.autogen_apparmor:
56 path = os.path.join(root_path, template_path)
57 with io.open(path, 'w', encoding='utf-8') as fp:
58- fp.write(SNAPPY_APPARMOR_DEFAULT_TEMPLATE)
59+ fp.write(template)
60
61 du_output = subprocess.check_output(
62 ["du", "-k", "-s", "--apparent-size", "."],
63
64=== modified file 'click/tests/test_build.py'
65--- click/tests/test_build.py 2014-12-04 14:08:55 +0000
66+++ click/tests/test_build.py 2014-12-05 21:56:27 +0000
67@@ -24,6 +24,7 @@
68 ]
69
70
71+import io
72 import json
73 import os
74 import stat
75@@ -409,6 +410,38 @@
76 - name: path/to/cli1
77 """
78
79+APPARMOR_3_TEMPLATE = """\
80+name: test-yaml
81+icon: data/icon.svg
82+version: 1.0
83+maintainer: Foo Bar <foo@example.org>
84+architecture: all
85+framework: ubuntu-core-15.04
86+integration:
87+ cli1:
88+ hookname: hook-file-name
89+ apparmor: path/to/cli1.apparmor-profile
90+binaries:
91+ - name: path/to/cli1
92+ - name: path/to/cli2
93+"""
94+
95+APPARMOR_4_TEMPLATE = """\
96+name: test-yaml
97+icon: data/icon.svg
98+version: 1.0
99+maintainer: Foo Bar <foo@example.org>
100+architecture: all
101+framework: ubuntu-core-15.04
102+integration:
103+ cli1:
104+ hookname: hook-file-name
105+ apparmor-profile: path/to/cli1.apparmor-profile
106+binaries:
107+ - name: path/to/cli1
108+ - name: path/to/cli2
109+"""
110+
111
112 class TestClickBuildYaml(TestCase):
113 def setUp(self):
114@@ -453,7 +486,7 @@
115 ["dpkg-deb", "-c", actual_path], universal_newlines=True)
116 for file_path in ("./data/icon.svg", "./meta/package.yaml",
117 "./meta/readme.md", "./bin/foo",
118- "./meta/app1.apparmor"):
119+ "./meta/app1.apparmor", "./meta/cli1.apparmor"):
120 self.assertIn(file_path, click_contents)
121
122 def test_read_manifest(self):
123@@ -475,6 +508,9 @@
124 "path/to/cli1",
125 self.builder.manifest["hooks"]["cli1"]["bin-path"])
126 self.assertEqual(
127+ "meta/cli1.apparmor",
128+ self.builder.manifest["hooks"]["cli1"]["apparmor"])
129+ self.assertEqual(
130 "meta/app1.apparmor",
131 self.builder.manifest["hooks"]["app1"]["apparmor"])
132
133@@ -488,6 +524,9 @@
134 self.assertEqual(
135 "meta/app2.apparmor",
136 self.builder.manifest["hooks"]["app2"]["apparmor"])
137+ self.assertEqual(
138+ "meta/cli1.apparmor",
139+ self.builder.manifest["hooks"]["cli1"]["apparmor"])
140
141 def test_one_explicit_apparmor_profile(self):
142 self.use_temp_dir()
143@@ -501,6 +540,9 @@
144 self.assertEqual(
145 "meta/app2.apparmor",
146 self.builder.manifest["hooks"]["app2"]["apparmor"])
147+ self.assertEqual(
148+ "meta/cli1.apparmor",
149+ self.builder.manifest["hooks"]["cli1"]["apparmor"])
150
151 def test_one_explicit_apparmor_file(self):
152 self.use_temp_dir()
153@@ -509,8 +551,8 @@
154 touch(os.path.join(scratch, "data", "icon.svg"))
155 apparmor_path = os.path.join(scratch, 'path', 'to', 'app1.apparmor')
156 os.makedirs(os.path.dirname(apparmor_path))
157- with open(apparmor_path, 'w', encoding='utf-8') as fp:
158- fp.write(dedent("""\
159+ with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
160+ fp.write(dedent(u"""\
161 {
162 "template": "default",
163 "policy_groups": [
164@@ -535,8 +577,8 @@
165 self.assertEqual(
166 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
167 ['app1.apparmor'])
168- with open(os.path.join(contents, 'path', 'to', 'app1.apparmor'),
169- encoding='utf-8') as fp:
170+ with io.open(os.path.join(contents, 'path', 'to', 'app1.apparmor'),
171+ encoding='utf-8') as fp:
172 data = fp.read()
173 self.assertEqual(data, dedent("""\
174 {
175@@ -548,8 +590,8 @@
176 "policy_version": 99.99
177 }
178 """))
179- with open(os.path.join(contents, 'meta', 'app2.apparmor'),
180- encoding='utf-8') as fp:
181+ with io.open(os.path.join(contents, 'meta', 'app2.apparmor'),
182+ encoding='utf-8') as fp:
183 data = fp.read()
184 self.assertEqual(data, dedent("""\
185 {
186@@ -570,8 +612,8 @@
187 apparmor_path = os.path.join(
188 scratch, 'path', 'to', 'app1.apparmor-profile')
189 os.makedirs(os.path.dirname(apparmor_path))
190- with open(apparmor_path, 'w', encoding='utf-8') as fp:
191- fp.write(dedent("""\
192+ with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
193+ fp.write(dedent(u"""\
194 {
195 "template": "default",
196 "policy_groups": [
197@@ -596,7 +638,7 @@
198 self.assertEqual(
199 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
200 ['app1.apparmor-profile'])
201- with open(
202+ with io.open(
203 os.path.join(contents, 'path', 'to', 'app1.apparmor-profile'),
204 encoding='utf-8') as fp:
205 data = fp.read()
206@@ -610,7 +652,129 @@
207 "policy_version": 88.88
208 }
209 """))
210- with open(os.path.join(contents, 'meta', 'app2.apparmor'),
211+ with io.open(os.path.join(contents, 'meta', 'app2.apparmor'),
212+ encoding='utf-8') as fp:
213+ data = fp.read()
214+ self.assertEqual(data, dedent("""\
215+ {
216+ "template": "default",
217+ "policy_groups": [
218+ "networking"
219+ ],
220+ "policy_vendor": "ubuntu-snappy",
221+ "policy_version": 1.3
222+ }
223+ """))
224+
225+ def test_one_explicit_binary_apparmor_file(self):
226+ self.use_temp_dir()
227+ scratch = os.path.join(self.temp_dir, "scratch")
228+ touch(os.path.join(scratch, "bin", "foo"))
229+ touch(os.path.join(scratch, "data", "icon.svg"))
230+ apparmor_path = os.path.join(scratch, 'path', 'to', 'cli1.apparmor')
231+ os.makedirs(os.path.dirname(apparmor_path))
232+ with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
233+ fp.write(dedent(u"""\
234+ {
235+ "template": "default",
236+ "policy_groups": [
237+ "networking"
238+ ],
239+ "policy_vendor": "ubuntu-snappy",
240+ "policy_version": 99.99
241+ }
242+ """))
243+ self._make_metadata(scratch, APPARMOR_3_TEMPLATE)
244+ self.builder.add_file(scratch, "./")
245+ # FIXME: make it a .snap
246+ expected_path = os.path.join(self.temp_dir, "test-yaml_1.0_all.click")
247+ actual_path = self.builder.build(self.temp_dir, "meta/package.yaml")
248+ self.assertEqual(expected_path, actual_path)
249+ self.assertTrue(os.path.exists(actual_path))
250+ contents = os.path.join(self.temp_dir, 'contents')
251+ subprocess.check_output(
252+ ["dpkg-deb", "-R", actual_path, contents], universal_newlines=True)
253+ self.assertEqual(sorted(os.listdir(contents)),
254+ ['DEBIAN', 'bin', 'data', 'meta', 'path'])
255+ self.assertEqual(
256+ sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
257+ ['cli1.apparmor'])
258+ with io.open(os.path.join(contents, 'path', 'to', 'cli1.apparmor'),
259+ encoding='utf-8') as fp:
260+ data = fp.read()
261+ self.assertEqual(data, dedent("""\
262+ {
263+ "template": "default",
264+ "policy_groups": [
265+ "networking"
266+ ],
267+ "policy_vendor": "ubuntu-snappy",
268+ "policy_version": 99.99
269+ }
270+ """))
271+ with io.open(os.path.join(contents, 'meta', 'cli2.apparmor'),
272+ encoding='utf-8') as fp:
273+ data = fp.read()
274+ self.assertEqual(data, dedent("""\
275+ {
276+ "template": "default",
277+ "policy_groups": [
278+ "networking"
279+ ],
280+ "policy_vendor": "ubuntu-snappy",
281+ "policy_version": 1.3
282+ }
283+ """))
284+
285+ def test_one_explicit_binary_apparmor_profile_file(self):
286+ self.use_temp_dir()
287+ scratch = os.path.join(self.temp_dir, "scratch")
288+ touch(os.path.join(scratch, "bin", "foo"))
289+ touch(os.path.join(scratch, "data", "icon.svg"))
290+ apparmor_path = os.path.join(
291+ scratch, 'path', 'to', 'cli1.apparmor-profile')
292+ os.makedirs(os.path.dirname(apparmor_path))
293+ with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
294+ fp.write(dedent(u"""\
295+ {
296+ "template": "default",
297+ "policy_groups": [
298+ "networking"
299+ ],
300+ "policy_vendor": "ubuntu-snappy",
301+ "policy_version": 88.88
302+ }
303+ """))
304+ self._make_metadata(scratch, APPARMOR_4_TEMPLATE)
305+ self.builder.add_file(scratch, "./")
306+ # FIXME: make it a .snap
307+ expected_path = os.path.join(self.temp_dir, "test-yaml_1.0_all.click")
308+ actual_path = self.builder.build(self.temp_dir, "meta/package.yaml")
309+ self.assertEqual(expected_path, actual_path)
310+ self.assertTrue(os.path.exists(actual_path))
311+ contents = os.path.join(self.temp_dir, 'contents')
312+ subprocess.check_output(
313+ ["dpkg-deb", "-R", actual_path, contents], universal_newlines=True)
314+ self.assertEqual(sorted(os.listdir(contents)),
315+ ['DEBIAN', 'bin', 'data', 'meta', 'path'])
316+ self.assertEqual(
317+ sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
318+ ['cli1.apparmor-profile'])
319+ with io.open(
320+ os.path.join(contents, 'path', 'to', 'cli1.apparmor-profile'),
321+ encoding='utf-8') as fp:
322+ data = fp.read()
323+ self.assertEqual(data, dedent("""\
324+ {
325+ "template": "default",
326+ "policy_groups": [
327+ "networking"
328+ ],
329+ "policy_vendor": "ubuntu-snappy",
330+ "policy_version": 88.88
331+ }
332+ """))
333+ with io.open(os.path.join(contents, 'meta', 'cli2.apparmor'),
334 encoding='utf-8') as fp:
335 data = fp.read()
336 self.assertEqual(data, dedent("""\
337
338=== modified file 'click/tests/test_database.py'
339--- click/tests/test_database.py 2014-09-10 11:54:14 +0000
340+++ click/tests/test_database.py 2014-12-05 21:56:27 +0000
341@@ -29,6 +29,7 @@
342 from itertools import takewhile
343 import json
344 import os
345+import six
346 import unittest
347
348 from gi.repository import Click, GLib
349@@ -55,7 +56,7 @@
350 "bar", "1.0", "/path/to/foo/1.0", False)
351
352 def test_hash(self):
353- self.assertIsInstance(self.foo.hash(), int)
354+ self.assertIsInstance(self.foo.hash(), six.integer_types)
355 self.assertEqual(self.foo.hash(), self.foo_clone.hash())
356 self.assertNotEqual(self.foo.hash(), self.foo_different_version.hash())
357 self.assertNotEqual(self.foo.hash(), self.foo_different_path.hash())
358
359=== modified file 'debian/changelog'
360--- debian/changelog 2014-12-05 15:17:16 +0000
361+++ debian/changelog 2014-12-05 21:56:27 +0000
362@@ -1,3 +1,10 @@
363+click (0.4.35+ppa27) vivid; urgency=medium
364+
365+ * Give binaries default apparmor and apparmor-profile keys in their
366+ manifest when explicit profiles are not present in packages.yaml
367+
368+ -- Barry Warsaw <barry@ubuntu.com> Fri, 05 Dec 2014 16:53:51 -0500
369+
370 click (0.4.35+ppa26) vivid; urgency=low
371
372 * lp:~mvo/click/lp1394256-run-user-hooks-on-remove-too:

Subscribers

People subscribed via source and target branches

to all changes: