click

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

  1. binapparmor
  2. Merge into 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
=== modified file 'click/build.py'
--- click/build.py 2014-12-04 14:12:41 +0000
+++ click/build.py 2014-12-05 21:56:27 +0000
@@ -62,7 +62,7 @@
62)62)
6363
6464
65SNAPPY_APPARMOR_DEFAULT_TEMPLATE = """\65SNAPPY_SERVICE_APPARMOR_DEFAULT_TEMPLATE = u"""\
66{66{
67 "template": "default",67 "template": "default",
68 "policy_groups": [68 "policy_groups": [
@@ -73,6 +73,10 @@
73}73}
74"""74"""
7575
76# XXX For now, these are the same templates.
77SNAPPY_BINARY_APPARMOR_DEFAULT_TEMPLATE = \
78 SNAPPY_SERVICE_APPARMOR_DEFAULT_TEMPLATE
79
7680
77@contextlib.contextmanager81@contextlib.contextmanager
78def make_temp_dir():82def make_temp_dir():
@@ -170,7 +174,8 @@
170 template_path = os.path.join(174 template_path = os.path.join(
171 'meta', '{}.apparmor'.format(name))175 'meta', '{}.apparmor'.format(name))
172 current_hook['apparmor'] = template_path176 current_hook['apparmor'] = template_path
173 self.autogen_apparmor.append(template_path)177 self.autogen_apparmor.append(
178 (SNAPPY_SERVICE_APPARMOR_DEFAULT_TEMPLATE, template_path))
174 self.manifest["hooks"] = hooks179 self.manifest["hooks"] = hooks
175 # cleanup180 # cleanup
176 if "services" in self.manifest:181 if "services" in self.manifest:
@@ -183,6 +188,15 @@
183 current_hook = hooks.get(base_name, {})188 current_hook = hooks.get(base_name, {})
184 current_hook["bin-path"] = binary["name"]189 current_hook["bin-path"] = binary["name"]
185 hooks[base_name] = current_hook190 hooks[base_name] = current_hook
191 # Is there an explicit apparmor hook? If so, use it otherwise
192 # generate a default one.
193 if ( 'apparmor' not in current_hook and
194 'apparmor-profile' not in current_hook):
195 template_path = os.path.join(
196 'meta', '{}.apparmor'.format(base_name))
197 current_hook['apparmor'] = template_path
198 self.autogen_apparmor.append(
199 (SNAPPY_BINARY_APPARMOR_DEFAULT_TEMPLATE, template_path))
186 self.manifest["hooks"] = hooks200 self.manifest["hooks"] = hooks
187 # cleanup201 # cleanup
188 if "binaries" in self.manifest:202 if "binaries" in self.manifest:
@@ -342,10 +356,10 @@
342 if "framework" in self.manifest:356 if "framework" in self.manifest:
343 self._validate_framework(self.manifest["framework"])357 self._validate_framework(self.manifest["framework"])
344358
345 for template_path in self.autogen_apparmor:359 for template, template_path in self.autogen_apparmor:
346 path = os.path.join(root_path, template_path)360 path = os.path.join(root_path, template_path)
347 with io.open(path, 'w', encoding='utf-8') as fp:361 with io.open(path, 'w', encoding='utf-8') as fp:
348 fp.write(SNAPPY_APPARMOR_DEFAULT_TEMPLATE)362 fp.write(template)
349363
350 du_output = subprocess.check_output(364 du_output = subprocess.check_output(
351 ["du", "-k", "-s", "--apparent-size", "."],365 ["du", "-k", "-s", "--apparent-size", "."],
352366
=== modified file 'click/tests/test_build.py'
--- click/tests/test_build.py 2014-12-04 14:08:55 +0000
+++ click/tests/test_build.py 2014-12-05 21:56:27 +0000
@@ -24,6 +24,7 @@
24 ]24 ]
2525
2626
27import io
27import json28import json
28import os29import os
29import stat30import stat
@@ -409,6 +410,38 @@
409 - name: path/to/cli1410 - name: path/to/cli1
410"""411"""
411412
413APPARMOR_3_TEMPLATE = """\
414name: test-yaml
415icon: data/icon.svg
416version: 1.0
417maintainer: Foo Bar <foo@example.org>
418architecture: all
419framework: ubuntu-core-15.04
420integration:
421 cli1:
422 hookname: hook-file-name
423 apparmor: path/to/cli1.apparmor-profile
424binaries:
425 - name: path/to/cli1
426 - name: path/to/cli2
427"""
428
429APPARMOR_4_TEMPLATE = """\
430name: test-yaml
431icon: data/icon.svg
432version: 1.0
433maintainer: Foo Bar <foo@example.org>
434architecture: all
435framework: ubuntu-core-15.04
436integration:
437 cli1:
438 hookname: hook-file-name
439 apparmor-profile: path/to/cli1.apparmor-profile
440binaries:
441 - name: path/to/cli1
442 - name: path/to/cli2
443"""
444
412445
413class TestClickBuildYaml(TestCase):446class TestClickBuildYaml(TestCase):
414 def setUp(self):447 def setUp(self):
@@ -453,7 +486,7 @@
453 ["dpkg-deb", "-c", actual_path], universal_newlines=True)486 ["dpkg-deb", "-c", actual_path], universal_newlines=True)
454 for file_path in ("./data/icon.svg", "./meta/package.yaml",487 for file_path in ("./data/icon.svg", "./meta/package.yaml",
455 "./meta/readme.md", "./bin/foo",488 "./meta/readme.md", "./bin/foo",
456 "./meta/app1.apparmor"):489 "./meta/app1.apparmor", "./meta/cli1.apparmor"):
457 self.assertIn(file_path, click_contents)490 self.assertIn(file_path, click_contents)
458491
459 def test_read_manifest(self):492 def test_read_manifest(self):
@@ -475,6 +508,9 @@
475 "path/to/cli1",508 "path/to/cli1",
476 self.builder.manifest["hooks"]["cli1"]["bin-path"])509 self.builder.manifest["hooks"]["cli1"]["bin-path"])
477 self.assertEqual(510 self.assertEqual(
511 "meta/cli1.apparmor",
512 self.builder.manifest["hooks"]["cli1"]["apparmor"])
513 self.assertEqual(
478 "meta/app1.apparmor",514 "meta/app1.apparmor",
479 self.builder.manifest["hooks"]["app1"]["apparmor"])515 self.builder.manifest["hooks"]["app1"]["apparmor"])
480516
@@ -488,6 +524,9 @@
488 self.assertEqual(524 self.assertEqual(
489 "meta/app2.apparmor",525 "meta/app2.apparmor",
490 self.builder.manifest["hooks"]["app2"]["apparmor"])526 self.builder.manifest["hooks"]["app2"]["apparmor"])
527 self.assertEqual(
528 "meta/cli1.apparmor",
529 self.builder.manifest["hooks"]["cli1"]["apparmor"])
491530
492 def test_one_explicit_apparmor_profile(self):531 def test_one_explicit_apparmor_profile(self):
493 self.use_temp_dir()532 self.use_temp_dir()
@@ -501,6 +540,9 @@
501 self.assertEqual(540 self.assertEqual(
502 "meta/app2.apparmor",541 "meta/app2.apparmor",
503 self.builder.manifest["hooks"]["app2"]["apparmor"])542 self.builder.manifest["hooks"]["app2"]["apparmor"])
543 self.assertEqual(
544 "meta/cli1.apparmor",
545 self.builder.manifest["hooks"]["cli1"]["apparmor"])
504546
505 def test_one_explicit_apparmor_file(self):547 def test_one_explicit_apparmor_file(self):
506 self.use_temp_dir()548 self.use_temp_dir()
@@ -509,8 +551,8 @@
509 touch(os.path.join(scratch, "data", "icon.svg"))551 touch(os.path.join(scratch, "data", "icon.svg"))
510 apparmor_path = os.path.join(scratch, 'path', 'to', 'app1.apparmor')552 apparmor_path = os.path.join(scratch, 'path', 'to', 'app1.apparmor')
511 os.makedirs(os.path.dirname(apparmor_path))553 os.makedirs(os.path.dirname(apparmor_path))
512 with open(apparmor_path, 'w', encoding='utf-8') as fp:554 with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
513 fp.write(dedent("""\555 fp.write(dedent(u"""\
514 {556 {
515 "template": "default",557 "template": "default",
516 "policy_groups": [558 "policy_groups": [
@@ -535,8 +577,8 @@
535 self.assertEqual(577 self.assertEqual(
536 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),578 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
537 ['app1.apparmor'])579 ['app1.apparmor'])
538 with open(os.path.join(contents, 'path', 'to', 'app1.apparmor'),580 with io.open(os.path.join(contents, 'path', 'to', 'app1.apparmor'),
539 encoding='utf-8') as fp:581 encoding='utf-8') as fp:
540 data = fp.read()582 data = fp.read()
541 self.assertEqual(data, dedent("""\583 self.assertEqual(data, dedent("""\
542 {584 {
@@ -548,8 +590,8 @@
548 "policy_version": 99.99590 "policy_version": 99.99
549 }591 }
550 """))592 """))
551 with open(os.path.join(contents, 'meta', 'app2.apparmor'),593 with io.open(os.path.join(contents, 'meta', 'app2.apparmor'),
552 encoding='utf-8') as fp:594 encoding='utf-8') as fp:
553 data = fp.read()595 data = fp.read()
554 self.assertEqual(data, dedent("""\596 self.assertEqual(data, dedent("""\
555 {597 {
@@ -570,8 +612,8 @@
570 apparmor_path = os.path.join(612 apparmor_path = os.path.join(
571 scratch, 'path', 'to', 'app1.apparmor-profile')613 scratch, 'path', 'to', 'app1.apparmor-profile')
572 os.makedirs(os.path.dirname(apparmor_path))614 os.makedirs(os.path.dirname(apparmor_path))
573 with open(apparmor_path, 'w', encoding='utf-8') as fp:615 with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
574 fp.write(dedent("""\616 fp.write(dedent(u"""\
575 {617 {
576 "template": "default",618 "template": "default",
577 "policy_groups": [619 "policy_groups": [
@@ -596,7 +638,7 @@
596 self.assertEqual(638 self.assertEqual(
597 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),639 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
598 ['app1.apparmor-profile'])640 ['app1.apparmor-profile'])
599 with open(641 with io.open(
600 os.path.join(contents, 'path', 'to', 'app1.apparmor-profile'),642 os.path.join(contents, 'path', 'to', 'app1.apparmor-profile'),
601 encoding='utf-8') as fp:643 encoding='utf-8') as fp:
602 data = fp.read()644 data = fp.read()
@@ -610,7 +652,129 @@
610 "policy_version": 88.88652 "policy_version": 88.88
611 }653 }
612 """))654 """))
613 with open(os.path.join(contents, 'meta', 'app2.apparmor'),655 with io.open(os.path.join(contents, 'meta', 'app2.apparmor'),
656 encoding='utf-8') as fp:
657 data = fp.read()
658 self.assertEqual(data, dedent("""\
659 {
660 "template": "default",
661 "policy_groups": [
662 "networking"
663 ],
664 "policy_vendor": "ubuntu-snappy",
665 "policy_version": 1.3
666 }
667 """))
668
669 def test_one_explicit_binary_apparmor_file(self):
670 self.use_temp_dir()
671 scratch = os.path.join(self.temp_dir, "scratch")
672 touch(os.path.join(scratch, "bin", "foo"))
673 touch(os.path.join(scratch, "data", "icon.svg"))
674 apparmor_path = os.path.join(scratch, 'path', 'to', 'cli1.apparmor')
675 os.makedirs(os.path.dirname(apparmor_path))
676 with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
677 fp.write(dedent(u"""\
678 {
679 "template": "default",
680 "policy_groups": [
681 "networking"
682 ],
683 "policy_vendor": "ubuntu-snappy",
684 "policy_version": 99.99
685 }
686 """))
687 self._make_metadata(scratch, APPARMOR_3_TEMPLATE)
688 self.builder.add_file(scratch, "./")
689 # FIXME: make it a .snap
690 expected_path = os.path.join(self.temp_dir, "test-yaml_1.0_all.click")
691 actual_path = self.builder.build(self.temp_dir, "meta/package.yaml")
692 self.assertEqual(expected_path, actual_path)
693 self.assertTrue(os.path.exists(actual_path))
694 contents = os.path.join(self.temp_dir, 'contents')
695 subprocess.check_output(
696 ["dpkg-deb", "-R", actual_path, contents], universal_newlines=True)
697 self.assertEqual(sorted(os.listdir(contents)),
698 ['DEBIAN', 'bin', 'data', 'meta', 'path'])
699 self.assertEqual(
700 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
701 ['cli1.apparmor'])
702 with io.open(os.path.join(contents, 'path', 'to', 'cli1.apparmor'),
703 encoding='utf-8') as fp:
704 data = fp.read()
705 self.assertEqual(data, dedent("""\
706 {
707 "template": "default",
708 "policy_groups": [
709 "networking"
710 ],
711 "policy_vendor": "ubuntu-snappy",
712 "policy_version": 99.99
713 }
714 """))
715 with io.open(os.path.join(contents, 'meta', 'cli2.apparmor'),
716 encoding='utf-8') as fp:
717 data = fp.read()
718 self.assertEqual(data, dedent("""\
719 {
720 "template": "default",
721 "policy_groups": [
722 "networking"
723 ],
724 "policy_vendor": "ubuntu-snappy",
725 "policy_version": 1.3
726 }
727 """))
728
729 def test_one_explicit_binary_apparmor_profile_file(self):
730 self.use_temp_dir()
731 scratch = os.path.join(self.temp_dir, "scratch")
732 touch(os.path.join(scratch, "bin", "foo"))
733 touch(os.path.join(scratch, "data", "icon.svg"))
734 apparmor_path = os.path.join(
735 scratch, 'path', 'to', 'cli1.apparmor-profile')
736 os.makedirs(os.path.dirname(apparmor_path))
737 with io.open(apparmor_path, 'w', encoding='utf-8') as fp:
738 fp.write(dedent(u"""\
739 {
740 "template": "default",
741 "policy_groups": [
742 "networking"
743 ],
744 "policy_vendor": "ubuntu-snappy",
745 "policy_version": 88.88
746 }
747 """))
748 self._make_metadata(scratch, APPARMOR_4_TEMPLATE)
749 self.builder.add_file(scratch, "./")
750 # FIXME: make it a .snap
751 expected_path = os.path.join(self.temp_dir, "test-yaml_1.0_all.click")
752 actual_path = self.builder.build(self.temp_dir, "meta/package.yaml")
753 self.assertEqual(expected_path, actual_path)
754 self.assertTrue(os.path.exists(actual_path))
755 contents = os.path.join(self.temp_dir, 'contents')
756 subprocess.check_output(
757 ["dpkg-deb", "-R", actual_path, contents], universal_newlines=True)
758 self.assertEqual(sorted(os.listdir(contents)),
759 ['DEBIAN', 'bin', 'data', 'meta', 'path'])
760 self.assertEqual(
761 sorted(os.listdir(os.path.join(contents, 'path', 'to'))),
762 ['cli1.apparmor-profile'])
763 with io.open(
764 os.path.join(contents, 'path', 'to', 'cli1.apparmor-profile'),
765 encoding='utf-8') as fp:
766 data = fp.read()
767 self.assertEqual(data, dedent("""\
768 {
769 "template": "default",
770 "policy_groups": [
771 "networking"
772 ],
773 "policy_vendor": "ubuntu-snappy",
774 "policy_version": 88.88
775 }
776 """))
777 with io.open(os.path.join(contents, 'meta', 'cli2.apparmor'),
614 encoding='utf-8') as fp:778 encoding='utf-8') as fp:
615 data = fp.read()779 data = fp.read()
616 self.assertEqual(data, dedent("""\780 self.assertEqual(data, dedent("""\
617781
=== modified file 'click/tests/test_database.py'
--- click/tests/test_database.py 2014-09-10 11:54:14 +0000
+++ click/tests/test_database.py 2014-12-05 21:56:27 +0000
@@ -29,6 +29,7 @@
29from itertools import takewhile29from itertools import takewhile
30import json30import json
31import os31import os
32import six
32import unittest33import unittest
3334
34from gi.repository import Click, GLib35from gi.repository import Click, GLib
@@ -55,7 +56,7 @@
55 "bar", "1.0", "/path/to/foo/1.0", False)56 "bar", "1.0", "/path/to/foo/1.0", False)
5657
57 def test_hash(self):58 def test_hash(self):
58 self.assertIsInstance(self.foo.hash(), int)59 self.assertIsInstance(self.foo.hash(), six.integer_types)
59 self.assertEqual(self.foo.hash(), self.foo_clone.hash())60 self.assertEqual(self.foo.hash(), self.foo_clone.hash())
60 self.assertNotEqual(self.foo.hash(), self.foo_different_version.hash())61 self.assertNotEqual(self.foo.hash(), self.foo_different_version.hash())
61 self.assertNotEqual(self.foo.hash(), self.foo_different_path.hash())62 self.assertNotEqual(self.foo.hash(), self.foo_different_path.hash())
6263
=== modified file 'debian/changelog'
--- debian/changelog 2014-12-05 15:17:16 +0000
+++ debian/changelog 2014-12-05 21:56:27 +0000
@@ -1,3 +1,10 @@
1click (0.4.35+ppa27) vivid; urgency=medium
2
3 * Give binaries default apparmor and apparmor-profile keys in their
4 manifest when explicit profiles are not present in packages.yaml
5
6 -- Barry Warsaw <barry@ubuntu.com> Fri, 05 Dec 2014 16:53:51 -0500
7
1click (0.4.35+ppa26) vivid; urgency=low8click (0.4.35+ppa26) vivid; urgency=low
29
3 * lp:~mvo/click/lp1394256-run-user-hooks-on-remove-too:10 * lp:~mvo/click/lp1394256-run-user-hooks-on-remove-too:

Subscribers

People subscribed via source and target branches

to all changes: