diff -Nru click-reviewers-tools-0.24/bin/click-check-bin-path click-reviewers-tools-0.25/bin/click-check-bin-path --- click-reviewers-tools-0.24/bin/click-check-bin-path 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-bin-path 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-bin-path: perform click bin-path checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_bin_path as cr_bin_path if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_bin_path.ClickReviewBinPath(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_bin_path.ClickReviewBinPath) diff -Nru click-reviewers-tools-0.24/bin/click-check-content-hub click-reviewers-tools-0.25/bin/click-check-content-hub --- click-reviewers-tools-0.24/bin/click-check-content-hub 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-content-hub 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-content-hub: perform click content-hub checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_content_hub as cr_content_hub if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_content_hub.ClickReviewContentHub(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_content_hub.ClickReviewContentHub) diff -Nru click-reviewers-tools-0.24/bin/click-check-desktop click-reviewers-tools-0.25/bin/click-check-desktop --- click-reviewers-tools-0.24/bin/click-check-desktop 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-desktop 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-desktop: perform click desktop checks''' # -# Copyright (C) 2013 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys from clickreviews import cr_common from clickreviews import cr_desktop if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_desktop.ClickReviewDesktop(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_desktop.ClickReviewDesktop) diff -Nru click-reviewers-tools-0.24/bin/click-check-framework click-reviewers-tools-0.25/bin/click-check-framework --- click-reviewers-tools-0.24/bin/click-check-framework 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-framework 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-framework: perform click framework checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_framework as cr_framework if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_framework.ClickReviewFramework(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_framework.ClickReviewFramework) diff -Nru click-reviewers-tools-0.24/bin/click-check-functional click-reviewers-tools-0.25/bin/click-check-functional --- click-reviewers-tools-0.24/bin/click-check-functional 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-functional 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-functional: perform functional checks on click packages''' # -# Copyright (C) 2013 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_functional as cr_functional if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_functional.ClickReviewFunctional(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_functional.ClickReviewFunctional) diff -Nru click-reviewers-tools-0.24/bin/click-check-lint click-reviewers-tools-0.25/bin/click-check-lint --- click-reviewers-tools-0.24/bin/click-check-lint 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-lint 2015-04-13 14:18:56.000000000 +0000 @@ -16,24 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import json -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_lint as cr_lint if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - # extract args - fn = sys.argv[1] - if len(sys.argv) > 2: - overrides = json.loads(sys.argv[2]) - else: - overrides = None - - review = cr_lint.ClickReviewLint(fn, overrides=overrides) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_lint.ClickReviewLint) diff -Nru click-reviewers-tools-0.24/bin/click-check-online-accounts click-reviewers-tools-0.25/bin/click-check-online-accounts --- click-reviewers-tools-0.24/bin/click-check-online-accounts 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-online-accounts 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-online-accounts: perform click online accounts checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_online_accounts as cr_online_accounts if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_online_accounts.ClickReviewAccounts(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_online_accounts.ClickReviewAccounts) diff -Nru click-reviewers-tools-0.24/bin/click-check-push-helper click-reviewers-tools-0.25/bin/click-check-push-helper --- click-reviewers-tools-0.24/bin/click-check-push-helper 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-push-helper 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-push-helper: perform click push-helper checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_push_helper as cr_push_helper if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_push_helper.ClickReviewPushHelper(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_push_helper.ClickReviewPushHelper) diff -Nru click-reviewers-tools-0.24/bin/click-check-scope click-reviewers-tools-0.25/bin/click-check-scope --- click-reviewers-tools-0.24/bin/click-check-scope 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-scope 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-scope: perform click scope checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_scope as cr_scope if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_scope.ClickReviewScope(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_scope.ClickReviewScope) diff -Nru click-reviewers-tools-0.24/bin/click-check-security click-reviewers-tools-0.25/bin/click-check-security --- click-reviewers-tools-0.24/bin/click-check-security 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-security 2015-04-13 14:18:56.000000000 +0000 @@ -16,24 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import json -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_security as cr_security if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - # extract args - fn = sys.argv[1] - if len(sys.argv) > 2: - overrides = json.loads(sys.argv[2]) - else: - overrides = None - - review = cr_security.ClickReviewSecurity(fn, overrides=overrides) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_security.ClickReviewSecurity) diff -Nru click-reviewers-tools-0.24/bin/click-check-skeleton click-reviewers-tools-0.25/bin/click-check-skeleton --- click-reviewers-tools-0.24/bin/click-check-skeleton 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-skeleton 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-skeleton: perform click skeleton checks''' # -# Copyright (C) 2013 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_skeleton as cr_skeleton if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_skeleton.ClickReviewSkeleton(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_skeleton.ClickReviewSkeleton) diff -Nru click-reviewers-tools-0.24/bin/click-check-systemd click-reviewers-tools-0.25/bin/click-check-systemd --- click-reviewers-tools-0.24/bin/click-check-systemd 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-systemd 2015-04-13 14:18:56.000000000 +0000 @@ -0,0 +1,24 @@ +#!/usr/bin/python3 +'''click-check-systemd: perform click systemd checks''' +# +# Copyright (C) 2013-2015 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function + +import clickreviews.cr_common as cr_common +import clickreviews.cr_systemd as cr_systemd + +if __name__ == "__main__": + cr_common.run_click_check(cr_systemd.ClickReviewSystemd) diff -Nru click-reviewers-tools-0.24/bin/click-check-url-dispatcher click-reviewers-tools-0.25/bin/click-check-url-dispatcher --- click-reviewers-tools-0.24/bin/click-check-url-dispatcher 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-check-url-dispatcher 2015-04-13 14:18:56.000000000 +0000 @@ -1,7 +1,7 @@ #!/usr/bin/python3 '''click-check-url_dispatcher: perform click url dispatcher checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,16 +16,9 @@ # along with this program. If not, see . from __future__ import print_function -import sys import clickreviews.cr_common as cr_common import clickreviews.cr_url_dispatcher as cr_url_dispatcher if __name__ == "__main__": - if len(sys.argv) < 2: - cr_common.error("Must give path to click package") - - review = cr_url_dispatcher.ClickReviewUrlDispatcher(sys.argv[1]) - review.do_checks() - rc = review.do_report() - sys.exit(rc) + cr_common.run_click_check(cr_url_dispatcher.ClickReviewUrlDispatcher) diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_bin_path.py click-reviewers-tools-0.25/bin/clickreviews/cr_bin_path.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_bin_path.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_bin_path.py 2015-04-13 14:18:56.000000000 +0000 @@ -16,31 +16,63 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview +from clickreviews.cr_common import ClickReview, error import os class ClickReviewBinPath(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'bin-path' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['required'] = ["apparmor"] peer_hooks[my_hook]['allowed'] = peer_hooks[my_hook]['required'] - ClickReview.__init__(self, fn, "bin-path", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "bin-path", peer_hooks=peer_hooks, + overrides=overrides) + # snappy yaml currently only allows specifying: + # - exec (optional) + # - description (optional) + # - TODO: caps (optional) + self.required_keys = [] + self.optional_keys = ['description', 'exec'] + + self.bin_paths_files = dict() self.bin_paths = dict() + + if self.is_snap and 'binaries' in self.pkg_yaml: + for binary in self.pkg_yaml['binaries']: + if 'name' not in binary: + error("package.yaml malformed: required 'name' not found " + "for entry in %s" % self.pkg_yaml['binaries']) + elif not isinstance(binary['name'], str): + error("package.yaml malformed: required 'name' is not str" + "for entry in %s" % self.pkg_yaml['binaries']) + + app = os.path.basename(binary['name']) + if 'exec' in binary: + rel = binary['exec'] + else: + rel = binary['name'] + self.bin_paths[app] = rel + self.bin_paths_files[app] = self._extract_bin_path(app) + + # Now verify click manifest for app in self.manifest['hooks']: - if 'bin-path' not in self.manifest['hooks'][app]: + if not self.is_snap and \ + 'bin-path' not in self.manifest['hooks'][app]: + # non-snappy clicks don't need bin-path hook # msg("Skipped missing bin-path hook for '%s'" % app) continue - self.bin_paths[app] = self._extract_bin_path(app) + elif 'bin-path' in self.manifest['hooks'][app] and \ + not isinstance(self.manifest['hooks'][app]['bin-path'], str): + error("manifest malformed: hooks/%s/bin-path is not str" % app) def _extract_bin_path(self, app): '''Get bin-path for app''' - rel = self.manifest['hooks'][app]['bin-path'] + rel = self.bin_paths[app] fn = os.path.join(self.unpack_dir, rel) if not os.path.exists(fn): error("Could not find '%s'" % rel) @@ -48,24 +80,137 @@ def _check_bin_path_executable(self, app): '''Check that the provided path exists''' - rel = self.manifest['hooks'][app]['bin-path'] - fn = os.path.join(self.unpack_dir, rel) + fn = self.bin_paths_files[app] return os.access(fn, os.X_OK) + def check_click_hooks(self): + '''Check that the click hooks match the package.yaml''' + t = 'info' + n = 'manifest_matches_yaml' + s = "OK" + extra = [] + for app in self.manifest['hooks']: + if 'bin-path' in self.manifest['hooks'][app] and \ + app not in self.bin_paths: + extra.append(app) + if len(extra) > 0: + t = 'error' + s = 'manifest has extra bin-path hooks: %s' % ",".join(extra) + self._add_result(t, n, s) + + t = 'info' + n = 'yaml_matches_manifest' + s = "OK" + missing = [] + for app in self.bin_paths: + if app not in self.manifest['hooks'] or \ + 'bin-path' not in self.manifest['hooks'][app]: + missing.append(app) + if len(missing) > 0: + t = 'error' + s = 'manifest has missing bin-path hooks: %s' % ",".join(missing) + self._add_result(t, n, s) + + def _verify_required(self, my_dict, test_str): + for app in sorted(my_dict): + for r in self.required_keys: + found = False + t = 'info' + n = '%s_required_key_%s_%s' % (test_str, r, app) + s = "OK" + if r in my_dict[app]: + if not isinstance(my_dict[app][r], str): + t = 'error' + s = "'%s' is not a string" % r + elif my_dict[app][r] == "": + t = 'error' + s = "'%s' is empty" % r + else: + found = True + if not found and t != 'error': + t = 'error' + s = "Missing required field '%s'" % r + self._add_result(t, n, s) + + def check_snappy_required(self): + '''Check for package.yaml required fields''' + if not self.is_snap or 'binaries' not in self.pkg_yaml: + return + self._verify_required(self._create_dict(self.pkg_yaml['binaries']), + 'package_yaml') + + def _verify_optional(self, my_dict, test_str): + for app in sorted(my_dict): + for o in self.optional_keys: + found = False + t = 'info' + n = '%s_optional_key_%s_%s' % (test_str, o, app) + s = "OK" + if o in my_dict[app]: + if o == 'stop-timeout' and \ + not isinstance(my_dict[app][o], int): + t = 'error' + s = "'%s' is not an integer" % o + elif not isinstance(my_dict[app][o], str): + t = 'error' + s = "'%s' is not a string" % o + elif my_dict[app][o] == "": + t = 'error' + s = "'%s' is empty" % o + else: + found = True + if not found and t != 'error': + s = "OK (skip missing)" + self._add_result(t, n, s) + + def check_snappy_optional(self): + '''Check snappy packate.yaml optional fields''' + if not self.is_snap or 'binaries' not in self.pkg_yaml: + return + self._verify_optional(self._create_dict(self.pkg_yaml['binaries']), + 'package_yaml') + + def _verify_unknown(self, my_dict, test_str): + for app in sorted(my_dict): + unknown = [] + t = 'info' + n = '%s_unknown_key_%s' % (test_str, app) + s = "OK" + + for f in my_dict[app].keys(): + if f not in self.required_keys and \ + f not in self.optional_keys: + unknown.append(f) + + if len(unknown) == 1: + t = 'warn' + s = "Unknown field '%s'" % unknown[0] + elif len(unknown) > 1: + t = 'warn' + s = "Unknown fields '%s'" % ", ".join(unknown) + self._add_result(t, n, s) + + def check_snappy_unknown(self): + '''Check snappy package.yaml unknown fields''' + if not self.is_snap or 'binaries' not in self.pkg_yaml: + return + self._verify_unknown(self._create_dict(self.pkg_yaml['binaries']), + 'package_yaml') + def check_path(self): '''Check path exists''' t = 'info' n = 'path exists' s = "OK" - for app in sorted(self.bin_paths): + for app in sorted(self.bin_paths_files): t = 'info' n = 'path executable' s = "OK" if not self._check_bin_path_executable(app): t = 'error' s = "'%s' is not executable" % \ - (self.manifest['hooks'][app]['bin-path']) + os.path.relpath(self.bin_paths_files[app], self.unpack_dir) self._add_result(t, n, s) def check_binary_description(self): diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_common.py click-reviewers-tools-0.25/bin/clickreviews/cr_common.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_common.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_common.py 2015-04-13 14:18:56.000000000 +0000 @@ -60,7 +60,7 @@ class ClickReview(object): '''This class represents click reviews''' # Convenience to break out common types of clicks (eg, app, scope, - # framework, click service) + # click service) app_allowed_peer_hooks = ["account-application", "account-service", "apparmor", @@ -75,8 +75,6 @@ "scope", ] # FIXME: when apparmor-policy is implemented, use this - # framework_allowed_peer_hooks = ["apparmor-policy"] - framework_allowed_peer_hooks = [] service_allowed_peer_hooks = ["apparmor", "bin-path", "snappy-systemd", @@ -86,16 +84,19 @@ # optional snappy fields here (may be required by appstore) snappy_optional = ["architecture", "binaries", + "config", "frameworks", "icon", + "immutable-config", "integration", + "oem", "services", "source", "type", "vendor", # replaces maintainer ] - def __init__(self, fn, review_type, peer_hooks=None): + def __init__(self, fn, review_type, peer_hooks=None, overrides=None): self.click_package = fn self._check_path_exists() if not self.click_package.endswith(".click") and \ @@ -144,11 +145,20 @@ if pkg_yaml is not None: try: self.pkg_yaml = yaml.safe_load(pkg_yaml) - except Exception as e: + except Exception: error("Could not load package.yaml. Is it properly formatted?") self._verify_package_yaml_structure() self.is_snap = True + # default to 'app' + if 'type' not in self.pkg_yaml: + self.pkg_yaml['type'] = 'app' + + self.is_snap_oem = False + if self.is_snap and 'type' in self.pkg_yaml and \ + self.pkg_yaml['type'] == 'oem': + self.is_snap_oem = True + # Get a list of all unpacked files, except DEBIAN/ self.pkg_files = [] self._list_all_files() @@ -164,6 +174,7 @@ self.valid_frameworks = self._extract_click_frameworks() self.peer_hooks = peer_hooks + self.overrides = overrides if overrides is not None else {} def _extract_click_frameworks(self): '''Extract installed click frameworks''' @@ -251,6 +262,14 @@ error("manifest malformed: '%s' is not str or list:\n%s" % (f, mp)) + # FIXME: this is kinda gross but the best we can do while we are trying + # to support clicks and native snaps + if 'type' in self.manifest and self.manifest['type'] == 'oem': + if 'hooks' in self.manifest: + error("'hooks' present in manifest with type 'oem'") + # mock up something for other tests + self.manifest['hooks'] = {'oem': {'reviewtools': True}} + # Not required by click, but required by appstore. 'hooks' is assumed # to be present in other checks if 'hooks' not in self.manifest: @@ -300,7 +319,7 @@ (isinstance(self.pkg_yaml[f], str) or isinstance(self.pkg_yaml[f], list)): error("yaml malformed: '%s' is not str or list:\n%s" % - (f, mp)) + (f, yp)) elif f in ["binaries", "services"] and not \ isinstance(self.pkg_yaml[f], list): error("yaml malformed: '%s' is not list:\n%s" % (f, yp)) @@ -609,3 +628,20 @@ recursive_rm(path) if contents_only is False: os.rmdir(dirPath) + + +def run_click_check(cls): + if len(sys.argv) < 2: + error("Must give path to click package") + + # extract args + fn = sys.argv[1] + if len(sys.argv) > 2: + overrides = json.loads(sys.argv[2]) + else: + overrides = None + + review = cls(fn, overrides=overrides) + review.do_checks() + rc = review.do_report() + sys.exit(rc) diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_content_hub.py click-reviewers-tools-0.25/bin/clickreviews/cr_content_hub.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_content_hub.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_content_hub.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_content_hub.py: click content-hub checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,21 +16,22 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import json import os class ClickReviewContentHub(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'content-hub' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks peer_hooks[my_hook]['required'] = [] - ClickReview.__init__(self, fn, "content_hub", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "content_hub", peer_hooks=peer_hooks, + overrides=overrides) self.valid_keys = ['destination', 'share', 'source'] diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_desktop.py click-reviewers-tools-0.25/bin/clickreviews/cr_desktop.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_desktop.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_desktop.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_desktop.py: click desktop checks''' # -# Copyright (C) 2013-2014 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import glob import json import os @@ -28,14 +28,15 @@ class ClickReviewDesktop(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'desktop' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks peer_hooks[my_hook]['required'] = ["apparmor"] - ClickReview.__init__(self, fn, "desktop", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "desktop", peer_hooks=peer_hooks, + overrides=overrides) self.desktop_files = dict() # click-show-files and a couple tests self.desktop_entries = dict() @@ -318,15 +319,20 @@ found_named_webapp = False urls = [] for i in de.getExec().split(): + if i == "webbrowser-app" or i == "webapp-container": + continue if i.startswith('--webappUrlPatterns'): found_url_patterns = True if i.startswith('--webappModelSearchPath'): found_model_search_path = True if i.startswith('--webapp='): found_model_search_path = True - if not i.startswith('--'): + # consider potential Exec field codes as 'non urls' + if not i.startswith('--') and not i.startswith('%'): urls.append(i) is_launching_local_app = True + if len(urls) == 0: + is_launching_local_app = False for url in urls: parts = urlsplit(url) if parts.scheme in ['http', 'https']: diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_framework.py click-reviewers-tools-0.25/bin/clickreviews/cr_framework.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_framework.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_framework.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_framework.py: click framework''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,19 +16,16 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, open_file_read +from clickreviews.cr_common import ClickReview, error, open_file_read +import glob import os +import re class ClickReviewFramework(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): - peer_hooks = dict() - my_hook = 'framework' - peer_hooks[my_hook] = dict() - peer_hooks[my_hook]['allowed'] = ClickReview.framework_allowed_peer_hooks - peer_hooks[my_hook]['required'] = peer_hooks[my_hook]['allowed'] - ClickReview.__init__(self, fn, "framework", peer_hooks=peer_hooks) + def __init__(self, fn, overrides=None): + ClickReview.__init__(self, fn, "framework", overrides=overrides) self.frameworks_file = dict() self.frameworks = dict() @@ -43,6 +40,11 @@ self.frameworks_file[app] = full_fn self.frameworks[app] = data + self.framework_policy_dirs = ['apparmor', 'seccomp'] + self.framework_policy_subdirs = ['templates', 'policygroups'] + (self.framework_policy, self.framework_policy_unknown) = \ + self._extract_framework_policy() + def _extract_framework(self, app): '''Get framework for app''' rel = self.manifest['hooks'][app]['framework'] @@ -61,70 +63,186 @@ return (fn, data) - def check_single_framework(self): - '''Check only have one framework in the click''' + def _extract_framework_policy(self): + '''Get framework policy files''' + policy_dict = dict() + unknown_dict = dict() + fpdir = os.path.join(self.unpack_dir, "meta", "framework-policy") + for i in glob.glob("%s/*" % fpdir): + rel_i = os.path.basename(i) + if not os.path.isdir(i) or rel_i not in self.framework_policy_dirs: + unknown_dict.append(os.path.relpath(i, self.unpack_dir)) + continue + + policy_dict[rel_i] = dict() + for j in glob.glob("%s/*" % i): + rel_j = os.path.basename(j) + if not os.path.isdir(j) or \ + rel_j not in self.framework_policy_subdirs: + unknown_dict.append(os.path.relpath(j, self.unpack_dir)) + continue + + policy_dict[rel_i][rel_j] = dict() + for k in glob.glob("%s/*" % j): + rel_k = os.path.basename(k) + if not os.path.isfile(k): + unknown_dict.append(os.path.relpath(k, + self.unpack_dir)) + continue + + fh = open_file_read(k) + policy_dict[rel_i][rel_j][rel_k] = fh.read() + fh.close() + return (policy_dict, unknown_dict) + + def _has_framework_in_metadir(self): + '''Check if snap has meta/.framework''' + if not self.is_snap: + return False + + return os.path.exists(os.path.join(self.unpack_dir, 'meta', + '%s.framework' % + self.pkg_yaml['name'])) + + def check_framework_hook_obsolete(self): + '''Check manifest doesn't specify 'framework' hook''' t = 'info' - n = 'single_framework' + n = "obsolete_declaration" s = "OK" - if len(self.frameworks.keys()) > 1: + if len(self.frameworks) > 0: t = 'error' - s = 'framework hook specified multiple times' + s = "framework hook found for '%s'" % \ + ",".join(sorted(self.frameworks)) self._add_result(t, n, s) - def check_framework_base_name(self): - '''Check framework Base-Name''' - for app in sorted(self.frameworks): - t = 'info' - n = "base_name_present '%s'" % app - s = "OK" - if 'Base-Name' not in self.frameworks[app]: - t = 'error' - s = "Could not find 'Base-Name' in '%s'" % \ - (self.frameworks_file[app]) - self._add_result(t, n, s) - continue - self._add_result(t, n, s) + def check_snappy_framework_file_obsolete(self): + '''Check snap doesn't ship .framework file''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + t = 'info' + n = "obsolete_framework_file" + s = "OK" + if self._has_framework_in_metadir(): + t = 'warn' + s = "found '%s.framework' (safe to remove)" % self.pkg_yaml['name'] + self._add_result(t, n, s) - t = 'info' - n = "base_name_namespacing '%s'" % app - s = "OK" - if self.frameworks[app]['Base-Name'] != self.manifest['name']: - t = 'error' - s = "'%s' != '%s'" % (self.frameworks[app]['Base-Name'], - self.manifest['name']) - self._add_result(t, n, s) + def check_snappy_framework_depends(self): + '''Check framework doesn't depend on other frameworks''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + t = 'info' + n = "framework_dependency" + s = "OK" + if "frameworks" in self.pkg_yaml: + t = 'error' + s = "'type: framework' may not specify 'frameworks'" + self._add_result(t, n, s) - def check_framework_base_version(self): - '''Check framework Base-Version''' - for app in sorted(self.frameworks): - t = 'info' - n = "base_version_present '%s'" % app - s = "OK" - if 'Base-Version' not in self.frameworks[app]: - t = 'error' - s = "Could not find 'Base-Version' in '%s'" % \ - (self.frameworks_file[app]) - self._add_result(t, n, s) - continue - self._add_result(t, n, s) + def check_snappy_framework_policy(self): + '''Check framework ships at least some policy''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return - v = self.frameworks[app]['Base-Version'] - t = 'info' - n = "base_version_number '%s'" % app - s = "OK" - try: - float(v) - except ValueError: - t = 'error' - s = "'Base-Version' malformed: '%s' is not a number" % v - self._add_result(t, n, s) + t = 'info' + n = "framework_policies" + s = "OK" + found = False + for i in self.framework_policy_dirs: + if i not in self.framework_policy: continue - self._add_result(t, n, s) + for j in self.framework_policy_subdirs: + if j not in self.framework_policy[i]: + continue + if len(self.framework_policy[i][j].keys()) > 0: + found = True + if not found: + t = 'warn' + s = "security policy not found" + self._add_result(t, n, s) - t = 'info' - n = "base_version_positive '%s'" % app - s = "OK" - if float(v) < 0: - t = 'error' - s = "'Base-Version' malformed: '%s' is negative" % v + t = 'info' + n = "framework_policy_unknown" + s = "OK" + if len(self.framework_policy_unknown) > 0: + t = 'warn' + s = "framework policy has unexpected entries: '%s'" % \ + ",".join(self.framework_policy_unknown) + self._add_result(t, n, s) + + def check_snappy_framework_policy_metadata(self): + '''Check framework policy has expected meta data''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + + t = 'info' + n = "framework_policy_metadata" + s = "OK" + msgs = [] + for term in ["# Description: ", "# Usage: "]: + for i in self.framework_policy_dirs: + if i not in self.framework_policy: + continue + for j in self.framework_policy_subdirs: + if j not in self.framework_policy[i]: + continue + for k in self.framework_policy[i][j].keys(): + found = False + for l in self.framework_policy[i][j][k].splitlines(): + if l.startswith(term): + found = True + if not found: + msgs.append("'%s' in '%s/%s/%s'" % (term, i, j, k)) + if len(msgs) > 0: + t = 'error' + s = "Could not find meta data: %s" % ",".join(msgs) + self._add_result(t, n, s) + + def check_snappy_framework_policy_matching(self): + '''Check framework policy ships apparmor and seccomp for each''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + + t = 'info' + n = "framework_has_all_policy" + s = "OK" + if len(self.framework_policy.keys()) == 0: + s = "OK (skipped missing policy)" self._add_result(t, n, s) + return + + for i in self.framework_policy: + for j in self.framework_policy[i]: + for k in self.framework_policy[i][j]: + for other in self.framework_policy_dirs: + if t == i: + continue + t = 'info' + n = "framework_policy_%s/%s/%s" % (i, j, k) + s = "OK" + if j not in self.framework_policy[other] or \ + k not in self.framework_policy[other][j]: + t = 'error' + s = "Could not find mathcing '%s/%s/%s'" % (other, + j, k) + self._add_result(t, n, s) + + def check_snappy_framework_policy_filenames(self): + '''Check framework policy file names''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + + for i in self.framework_policy: + for j in self.framework_policy[i]: + for k in self.framework_policy[i][j]: + f = "%s/%s/%s" % (i, j, k) + t = 'info' + n = "framework_policy_valid_name_%s" % f + s = "OK" + if not re.search(r'^[a-z0-9][a-z0-9+\.-]+$', k): + t = 'error' + s = "'%s' should match '^[a-z0-9][a-z0-9+\.-]+$'" % f + elif k.startswith(self.pkg_yaml['name']): + t = 'warn' + s = "'%s' should not begin with package name" % f + self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_functional.py click-reviewers-tools-0.25/bin/clickreviews/cr_functional.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_functional.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_functional.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_functional.py: click functional''' # -# Copyright (C) 2013-2014 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -27,8 +27,8 @@ class ClickReviewFunctional(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): - ClickReview.__init__(self, fn, "functional") + def __init__(self, fn, overrides=None): + ClickReview.__init__(self, fn, "functional", overrides=overrides) self.qml_files = [] for i in self.pkg_files: diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_lint.py click-reviewers-tools-0.25/bin/clickreviews/cr_lint.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_lint.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_lint.py 2015-04-13 14:18:56.000000000 +0000 @@ -24,7 +24,7 @@ from clickreviews.frameworks import Frameworks from clickreviews.cr_common import ClickReview, open_file_read, cmd -CONTROL_FILE_NAMES = ["control", "manifest", "md5sums", "preinst"] +CONTROL_FILE_NAMES = ["control", "manifest", "preinst"] MINIMUM_CLICK_FRAMEWORK_VERSION = "0.4" @@ -33,7 +33,9 @@ def __init__(self, fn, overrides=None): '''Set up the class.''' - ClickReview.__init__(self, fn, "lint") + ClickReview.__init__(self, fn, "lint", overrides=overrides) + if not self.is_snap and "md5sums" not in CONTROL_FILE_NAMES: + CONTROL_FILE_NAMES.append("md5sums") self.control_files = dict() self._list_control_files() # Valid values for Architecture in DEBIAN/control. Note: @@ -93,15 +95,13 @@ 'bin-path', 'content-hub', 'desktop', - 'framework', 'pay-ui', 'push-helper', 'scope', 'snappy-systemd', 'urls'] - self.redflagged_hooks = ['framework', - 'pay-ui', + self.redflagged_hooks = ['pay-ui', 'apparmor-profile', ] @@ -111,17 +111,12 @@ # - oem self.snappy_valid_types = ['app', 'framework', - # 'framework-policy', # TBI - # 'oem', # TBI + 'oem', ] self.snappy_redflagged_types = ['framework', - # 'oem', # TBI + 'oem', # TBD ] - if overrides is None: - overrides = {} - self.overrides = overrides - def _list_control_files(self): '''List all control files with their full path.''' for i in CONTROL_FILE_NAMES: @@ -135,8 +130,11 @@ n = 'DEBIAN_has_%s' % os.path.basename(f) s = "OK" if not os.path.isfile(self.control_files[os.path.basename(f)]): - t = 'error' - s = "'%s' not found in DEBIAN/" % os.path.basename(f) + if self.is_snap and os.path.basename(f) == 'md5sums': + s = "OK (skip md5sums with snap)" + else: + t = 'error' + s = "'%s' not found in DEBIAN/" % os.path.basename(f) self._add_result(t, n, s) found = [] @@ -260,7 +258,8 @@ n = 'control_description_match' s = 'OK' if 'title' in self.manifest: - if control['Description'] != self.manifest['title']: + if control['Description'].strip() != \ + self.manifest['title'].strip(): t = 'error' s = "Description=%s does not match manifest title=%s" % \ (control['Description'], self.manifest['title']) @@ -294,6 +293,8 @@ def check_md5sums(self): '''Check md5sums()''' + if self.is_snap: + return curdir = os.getcwd() fh = open_file_read(self.control_files["md5sums"]) badsums = [] @@ -338,6 +339,10 @@ def check_hooks(self): '''Check click manifest hooks''' + # oem snaps don't have a hooks entry + if self.is_snap_oem: + return + # Some checks are already handled in # cr_common.py:_verify_manifest_structure() @@ -431,6 +436,10 @@ def check_hooks_unknown(self): '''Check if have any unknown hooks''' + # oem snaps don't have a hooks entry + if self.is_snap_oem: + return + t = 'info' n = 'unknown hooks' s = 'OK' @@ -785,9 +794,13 @@ else: arch = tmp[2].partition('.click')[0] if arch != self.click_arch: - t = 'error' - s = "'%s' != '%s' from DEBIAN/control" % (arch, - self.click_arch) + if arch == 'all' and self.click_arch == 'multi': + # The store creates filenames for fat packages with _all + pass + else: + t = 'error' + s = "'%s' != '%s' from DEBIAN/control" % (arch, + self.click_arch) else: t = 'warn' s = "could not determine architecture from '%s'" % \ diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_online_accounts.py click-reviewers-tools-0.25/bin/clickreviews/cr_online_accounts.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_online_accounts.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_online_accounts.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_online_accounts.py: click online accounts''' # -# Copyright (C) 2013 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error import os # http://lxml.de/tutorial.html import lxml.etree as etree @@ -24,7 +24,7 @@ class ClickReviewAccounts(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() peer_hooks['account-application'] = dict() peer_hooks['account-application']['allowed'] = \ @@ -54,7 +54,8 @@ peer_hooks['account-qml-plugin']['allowed'] = \ peer_hooks['account-qml-plugin']['required'] - ClickReview.__init__(self, fn, "online_accounts", peer_hooks) + ClickReview.__init__(self, fn, "online_accounts", peer_hooks=peer_hooks, + overrides=overrides) self.accounts_files = dict() self.accounts = dict() @@ -100,8 +101,7 @@ tree = etree.parse(fn) xml = tree.getroot() except Exception as e: - error("accounts xml unparseable: %s (%s):\n%s" % (bn, str(e), - contents)) + error("accounts xml unparseable: %s (%s)" % (bn, str(e))) return (fn, xml) def check_application(self): @@ -126,7 +126,6 @@ t = 'info' n = '%s_%s_id' % (app, account_type) s = "OK" - expected_id = "%s_%s" % (self.manifest["name"], app) if "id" in self.accounts[app][account_type].keys(): t = 'warn' s = "Found 'id' in application tag" @@ -174,7 +173,6 @@ t = 'info' n = '%s_%s_id' % (app, account_type) s = "OK" - expected_id = "%s_%s" % (self.manifest["name"], app) if "id" in self.accounts[app][account_type].keys(): t = 'warn' s = "Found 'id' in service tag" @@ -223,7 +221,6 @@ t = 'info' n = '%s_%s_id' % (app, account_type) s = "OK" - expected_id = "%s_%s" % (self.manifest["name"], app) if "id" in self.accounts[app][account_type].keys(): t = 'warn' s = "Found 'id' in provider tag" diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_push_helper.py click-reviewers-tools-0.25/bin/clickreviews/cr_push_helper.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_push_helper.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_push_helper.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_push_helper.py: click push-helper checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,21 +16,22 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import json import os class ClickReviewPushHelper(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'push-helper' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ['apparmor'] peer_hooks[my_hook]['required'] = ['apparmor'] - ClickReview.__init__(self, fn, "push_helper", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "push_helper", peer_hooks=peer_hooks, + overrides=overrides) self.required_keys = ['exec'] self.optional_keys = ['app_id'] diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_scope.py click-reviewers-tools-0.25/bin/clickreviews/cr_scope.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_scope.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_scope.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_scope.py: click scope''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, msg +from clickreviews.cr_common import ClickReview, error import codecs import configparser import os @@ -30,14 +30,15 @@ class ClickReviewScope(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'scope' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.scope_allowed_peer_hooks peer_hooks[my_hook]['required'] = ['apparmor'] - ClickReview.__init__(self, fn, "scope", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "scope", peer_hooks=peer_hooks, + overrides=overrides) self.scopes = dict() for app in self.manifest['hooks']: diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_security.py click-reviewers-tools-0.25/bin/clickreviews/cr_security.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_security.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_security.py 2015-04-13 14:18:56.000000000 +0000 @@ -43,7 +43,8 @@ ClickReview.service_allowed_peer_hooks peer_hooks[my_hook2]['required'] = [] - ClickReview.__init__(self, fn, "security", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "security", peer_hooks=peer_hooks, + overrides=overrides) local_copy = os.path.join(os.path.dirname(__file__), '../data/apparmor-easyprof-ubuntu.json') @@ -111,9 +112,7 @@ 'policy_version': 1.3, }, } - if overrides is None: - overrides = {} - framework_overrides = overrides.get('framework', {}) + framework_overrides = self.overrides.get('framework', {}) self._override_framework_policies(framework_overrides) self.security_manifests = dict() diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_skeleton.py click-reviewers-tools-0.25/bin/clickreviews/cr_skeleton.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_skeleton.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_skeleton.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_skeleton.py: click skeleton''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -21,7 +21,7 @@ class ClickReviewSkeleton(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): # Many test classes are for verify click hooks. 'peer_hooks' is used # to declare what hooks may be use with my_hook. When using this # mechanism, ClickReview.check_peer_hooks() is run for you. @@ -31,7 +31,8 @@ peer_hooks[my_hook]['allowed'] = ["desktop", "apparmor", "urls"] peer_hooks[my_hook]['required'] = ["desktop", "apparmor"] - ClickReview.__init__(self, fn, "skeleton", peer_hooks) + ClickReview.__init__(self, fn, "skeleton", peer_hooks=peer_hooks, + overrides=overrides) # If not a hooks test, skip the above and omit peer_hooks like so: # ClickReview.__init__(self, fn, "skeleton") diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_systemd.py click-reviewers-tools-0.25/bin/clickreviews/cr_systemd.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_systemd.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_systemd.py 2015-04-13 14:18:56.000000000 +0000 @@ -16,21 +16,22 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import yaml import os class ClickReviewSystemd(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'snappy-systemd' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['required'] = ["apparmor"] peer_hooks[my_hook]['allowed'] = peer_hooks[my_hook]['required'] - ClickReview.__init__(self, fn, "snappy-systemd", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "snappy-systemd", peer_hooks=peer_hooks, + overrides=overrides) # snappy-systemd currently only allows specifying: # - start (required) @@ -38,27 +39,43 @@ # - stop # - poststop # - stop-timeout + # - TODO: caps self.required_keys = ['start', 'description'] self.optional_keys = ['stop', 'poststop', 'stop-timeout'] self.systemd_files = dict() # click-show-files and tests self.systemd = dict() + + if self.is_snap and 'services' in self.pkg_yaml: + for service in self.pkg_yaml['services']: + if 'name' not in service: + error("package.yaml malformed: required 'name' not found " + "for entry in %s" % self.pkg_yaml['services']) + elif not isinstance(service['name'], str): + error("package.yaml malformed: required 'name' is not str" + "for entry in %s" % self.pkg_yaml['services']) + + app = service['name'] + (full_fn, yd) = self._extract_systemd(app) + self.systemd_files[app] = full_fn + self.systemd[app] = yd + + # Now verify click manifest for app in self.manifest['hooks']: - if 'snappy-systemd' not in self.manifest['hooks'][app]: + if not self.is_snap and \ + 'snappy-systemd' not in self.manifest['hooks'][app]: + # non-snappy clicks don't need snappy-systemd hook # msg("Skipped missing systemd hook for '%s'" % app) continue - if not isinstance(self.manifest['hooks'][app]['snappy-systemd'], - str): + elif 'snappy-systemd' in self.manifest['hooks'][app] and \ + not isinstance(self.manifest['hooks'][app]['snappy-systemd'], + str): error("manifest malformed: hooks/%s/snappy-systemd is not str" % app) - (full_fn, jd) = self._extract_systemd(app) - self.systemd_files[app] = full_fn - self.systemd[app] = jd def _extract_systemd(self, app): '''Get systemd yaml''' - u = self.manifest['hooks'][app]['snappy-systemd'] - fn = os.path.join(self.unpack_dir, u) + fn = os.path.join(self.unpack_dir, "meta", "%s.snappy-systemd" % app) bn = os.path.basename(fn) if not os.path.exists(fn): diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_tests.py click-reviewers-tools-0.25/bin/clickreviews/cr_tests.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_tests.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_tests.py 2015-04-13 14:18:56.000000000 +0000 @@ -46,6 +46,8 @@ TEST_PUSH_HELPER = dict() TEST_BIN_PATH = dict() TEST_FRAMEWORK = dict() +TEST_FRAMEWORK_POLICY = dict() +TEST_FRAMEWORK_POLICY_UNKNOWN = [] TEST_SNAPPY_SYSTEMD = dict() @@ -198,6 +200,16 @@ return ("%s.framework" % app, TEST_FRAMEWORK[app]) +def _extract_framework_policy(self): + '''Pretend we found the framework policy files''' + return (TEST_FRAMEWORK_POLICY, TEST_FRAMEWORK_POLICY_UNKNOWN) + + +def _has_framework_in_metadir(self): + '''Pretend we found the framework file''' + return True + + def _extract_systemd(self, app): '''Pretend we found the systemd file''' return ("%s.snappy-systemd" % app, TEST_SNAPPY_SYSTEMD[app]) @@ -312,6 +324,12 @@ patches.append(patch( 'clickreviews.cr_framework.ClickReviewFramework._extract_framework', _extract_framework)) +patches.append(patch( + 'clickreviews.cr_framework.ClickReviewFramework._extract_framework_policy', + _extract_framework_policy)) +patches.append(patch( + 'clickreviews.cr_framework.ClickReviewFramework._has_framework_in_metadir', + _has_framework_in_metadir)) # systemd overrides patches.append(patch( @@ -399,6 +417,8 @@ self.test_push_helper = dict() self.test_bin_path = dict() self.test_framework = dict() + self.test_framework_policy = dict() + self.test_framework_policy_unknown = [] self.test_systemd = dict() for app in self.test_manifest["hooks"].keys(): # setup security manifest for each app @@ -444,6 +464,10 @@ # Reset to no framework entries in manifest self.set_test_framework(app, None, None) + # Reset to no framework entries in manifest + self.set_test_framework_policy(None) + self.set_test_framework_policy_unknown([]) + # Reset to no systemd entries in manifest self.set_test_systemd(app, None, None) @@ -463,6 +487,8 @@ self._update_test_push_helper() self._update_test_bin_path() self._update_test_framework() + self._update_test_framework_policy() + self._update_test_framework_policy_unknown() self._update_test_systemd() # webapps manifests (leave empty for now) @@ -617,6 +643,14 @@ "%s.framework" % TEST_FRAMEWORK[app] self._update_test_manifest() + def _update_test_framework_policy(self): + global TEST_FRAMEWORK_POLICY + TEST_FRAMEWORK_POLICY = self.test_framework_policy + + def _update_test_framework_policy_unknown(self): + global TEST_FRAMEWORK_POLICY_UNKNOWN + TEST_FRAMEWORK_POLICY_UNKNOWN = self.test_framework_policy_unknown + def _update_test_systemd(self): global TEST_SNAPPY_SYSTEMD TEST_SNAPPY_SYSTEMD = dict() @@ -851,7 +885,44 @@ def set_test_bin_path(self, app, value): '''Set bin-path entries. If value is None, remove bin-path from - manifest''' + manifest and yaml. If app != value, set 'exec' in the yaml + + Note the click manifest and the package.yaml use different + storage types. pkg_yaml['binaries'] is a list of dictionaries where + manifest['hooks'] is a dictionary of dictionaries. This function + sets the manifest entry and then a yaml entry with 'name' and 'exec' + fields. + + manifest['hooks'][app]['bin-path'] = value + pkg_yaml['binaries'][*]['name'] = app + pkg_yaml['binaries'][*]['exec'] = value + ''' + + # Update the package.yaml + if value is None: + if 'binaries' in self.test_pkg_yaml: + for b in self.test_pkg_yaml['binaries']: + if 'name' in b and b['name'] == app: + self.test_pkg_yaml['binaries'].remove(b) + break + else: + found = False + if 'binaries' in self.test_pkg_yaml: + for b in self.test_pkg_yaml['binaries']: + if 'name' in b and b['name'] == app: + found = True + break + if not found: + if 'binaries' not in self.test_pkg_yaml: + self.test_pkg_yaml['binaries'] = [] + if value == app: + self.test_pkg_yaml['binaries'].append({'name': app}) + else: + self.test_pkg_yaml['binaries'].append({'name': app, + 'exec': value}) + self._update_test_pkg_yaml() + + # Update the click manifest (we still support click format) if value is None: if app in self.test_bin_path: self.test_bin_path.pop(app) @@ -859,6 +930,8 @@ if app not in self.test_bin_path: self.test_bin_path[app] = dict() self.test_bin_path[app] = value + + # Now update TEST_BIN_PATH self._update_test_bin_path() def set_test_framework(self, app, key, value): @@ -876,9 +949,68 @@ self.test_framework[app][key] = value self._update_test_framework() + def set_test_framework_policy(self, policy_dict=None): + '''Set framework policy''' + if policy_dict is None: # Reset + self.test_framework_policy = dict() + for i in ['apparmor', 'seccomp']: + self.test_framework_policy[i] = dict() + for j in ['templates', 'policygroups']: + self.test_framework_policy[i][j] = dict() + for k in ['-common', '-reserved']: + n = "%s%s" % (j.rstrip('s'), k) + self.test_framework_policy[i][j][n] = \ + '''# Description: %s +# Usage: %s +''' % (n, k.lstrip('-')) + else: + self.test_framework_policy = policy_dict + self._update_test_framework_policy() + + def set_test_framework_policy_unknown(self, unknown=[]): + '''Set framework policy unknown''' + self.test_framework_policy_unknown = unknown + self._update_test_framework_policy_unknown() + def set_test_systemd(self, app, key, value): - '''Set systemd entries. If key is None, remove hook, if value is None, - remove key''' + '''Set systemd entries. If key is None, snappy-systemd from manifest + and yaml. + + Note the click manifest and the package.yaml use different + storage types. pkg_yaml['services'] is a list of dictionaries where + manifest['hooks'] is a dictionary of dictionaries. This function + sets the manifest entry and then a yaml entry with 'name' field. + + manifest['hooks'][app]['snappy-systemd'] = + pkg_yaml['services'][*]['name'] = app + pkg_yaml['services'][*][key] = value + ''' + + # Update the package.yaml + if key is None: + if 'services' in self.test_pkg_yaml: + for s in self.test_pkg_yaml['services']: + if 'name' in s and s['name'] == app: + self.test_pkg_yaml['services'].remove(s) + break + else: + found = False + if 'services' in self.test_pkg_yaml: + for s in self.test_pkg_yaml['services']: + if 'name' in s and s['name'] == app: + # Found the entry, so update key/value + s[key] = value + found = True + break + # Did not find the entry, so create one + if not found: + if 'services' not in self.test_pkg_yaml: + self.test_pkg_yaml['services'] = [] + self.test_pkg_yaml['services'].append({'name': app, + key: value}) + self._update_test_pkg_yaml() + + # Update the click manifest (we still support click format) if key is None: if app in self.test_systemd: self.test_systemd.pop(app) @@ -891,6 +1023,8 @@ del(self.test_systemd[app][key]) else: self.test_systemd[app][key] = value + + # Now update TEST_SNAPPY_SYSTEMD self._update_test_systemd() def setUp(self): @@ -935,6 +1069,10 @@ TEST_BIN_PATH = dict() global TEST_FRAMEWORK TEST_FRAMEWORK = dict() + global TEST_FRAMEWORK_POLICY + TEST_FRAMEWORK_POLICY = dict() + global TEST_FRAMEWORK_POLICY_UNKNOWN + TEST_FRAMEWORK_POLICY_UNKNOWN = [] global TEST_SNAPPY_SYSTEMD TEST_SNAPPY_SYSTEMD = dict() diff -Nru click-reviewers-tools-0.24/bin/clickreviews/cr_url_dispatcher.py click-reviewers-tools-0.25/bin/clickreviews/cr_url_dispatcher.py --- click-reviewers-tools-0.24/bin/clickreviews/cr_url_dispatcher.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/cr_url_dispatcher.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_url dispatcher.py: click url_dispatcher''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import json import os @@ -25,14 +25,15 @@ class ClickReviewUrlDispatcher(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'urls' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks peer_hooks[my_hook]['required'] = [] - ClickReview.__init__(self, fn, "url_dispatcher", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "url_dispatcher", peer_hooks=peer_hooks, + overrides=overrides) self.required_keys = ['protocol'] self.optional_keys = ['domain-suffix'] diff -Nru click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_bin_path.py click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_bin_path.py --- click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_bin_path.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_bin_path.py 2015-04-13 14:18:56.000000000 +0000 @@ -26,15 +26,21 @@ cr_tests.mock_patch() super() - def _set_binary(self, key, value, name=None): + def _set_binary(self, entries, name=None): d = dict() if name is None: - d['name'] = 'foo' + d['name'] = self.default_appname else: d['name'] = name - d[key] = value + for (key, value) in entries: + d[key] = value self.set_test_pkg_yaml("binaries", [d]) + if 'exec' in d: + self.set_test_bin_path(d['name'], d['exec']) + else: + self.set_test_bin_path(d['name'], d['name']) + def test_check_path(self): '''Test check_path()''' self.set_test_bin_path(self.default_appname, "bin/foo.exe") @@ -51,12 +57,12 @@ c = ClickReviewBinPath(self.test_name) c.check_path() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': 0, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) def test_check_peer_hooks(self): '''Test check_peer_hooks()''' + self.set_test_bin_path(self.default_appname, self.default_appname) c = ClickReviewBinPath(self.test_name) # create a new hooks database for our peer hooks tests @@ -80,6 +86,7 @@ def test_check_peer_hooks_disallowed(self): '''Test check_peer_hooks() - disallowed''' + self.set_test_bin_path(self.default_appname, self.default_appname) c = ClickReviewBinPath(self.test_name) # create a new hooks database for our peer hooks tests @@ -106,6 +113,7 @@ def test_check_peer_hooks_required(self): '''Test check_peer_hooks() - required''' + self.set_test_bin_path(self.default_appname, self.default_appname) c = ClickReviewBinPath(self.test_name) # create a new hooks database for our peer hooks tests @@ -125,9 +133,96 @@ expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + def test_check_required(self): + '''Test check_snappy_required()''' + self._set_binary([("exec", "bin/foo")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_required_empty_value(self): + '''Test check_snappy_required() - empty exec''' + self._set_binary([("exec", "")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_required_bad_value(self): + '''Test check_snappy_required() - bad exec''' + self._set_binary([("exec", [])]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_required_multiple(self): + '''Test check_snappy_required() - multiple''' + self._set_binary([("exec", "bin/foo"), + ("description", "foo desc")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_snappy_optional(self): + '''Test check_snappy_optional()''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_optional() + r = c.click_report + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_snappy_optional_empty_value(self): + '''Test check_snappy_optional() - empty description''' + self._set_binary([("description", "")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_optional() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_snappy_optional_bad_value(self): + '''Test check_snappy_optional() - bad description''' + self._set_binary([("description", [])]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_optional() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_snappy_unknown(self): + '''Test check_snappy_unknown()''' + self._set_binary([("nonexistent", "foo")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_unknown() + r = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_snappy_unknown_multiple(self): + '''Test check_snappy_unknown() - multiple''' + self._set_binary([("exec", "bin/foo"), + ("nonexistent", "foo")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_unknown() + r = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(r, expected_counts) + def test_check_binary_description(self): '''Test check_binary_description()''' - self._set_binary("description", "some description") + self._set_binary([("description", "some description")]) c = ClickReviewBinPath(self.test_name) c.check_binary_description() r = c.click_report @@ -136,7 +231,7 @@ def test_check_binary_description_unspecified(self): '''Test check_binary_description() - unspecified''' - self._set_binary("name", "foo") + self._set_binary([("exec", "foo")]) c = ClickReviewBinPath(self.test_name) c.check_binary_description() r = c.click_report @@ -145,9 +240,40 @@ def test_check_binary_description_empty(self): '''Test check_binary_description() - empty''' - self._set_binary("description", "") + self._set_binary([("description", "")]) c = ClickReviewBinPath(self.test_name) c.check_binary_description() r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_click_hooks(self): + '''Test check_click_hooks()''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + c.check_click_hooks() + r = c.click_report + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_click_hooks_missing_hook(self): + '''Test check_click_hooks() - missing''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + del c.manifest['hooks'][self.default_appname]['bin-path'] + c.check_click_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_click_hooks_added_hook(self): + '''Test check_click_hooks() - added''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + tmp = c.manifest['hooks'] + tmp['extra-app'] = c.manifest['hooks'][self.default_appname] + c.manifest['hooks'] = tmp + c.check_click_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_desktop.py click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_desktop.py --- click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_desktop.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_desktop.py 2015-04-13 14:18:56.000000000 +0000 @@ -726,6 +726,42 @@ expected_counts = {'info': None, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) + def test_check_desktop_exec_webbrowser_no_homepage(self): + '''Test check_desktop_exec_webbrowser_no_homepage() not local app''' + c = ClickReviewDesktop(self.test_name) + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "name", "foo") + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "includes", + ['https?://mobile.twitter.net/*']) + ex = "webapp-container --webapp='Zm9v' " + \ + "--enable-back-forward --webappModelSearchPath=." + self.set_test_desktop(self.default_appname, + "Exec", + ex) + c.check_desktop_exec_webapp_args() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_desktop_exec_webbrowser_field_code(self): + '''Test check_desktop_exec_webbrowser_field_code() with field code''' + c = ClickReviewDesktop(self.test_name) + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "name", "foo") + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "includes", + ['https?://mobile.twitter.net/*']) + ex = "webapp-container --webapp='Zm9v' " + \ + "--enable-back-forward --webappModelSearchPath=. %u" + self.set_test_desktop(self.default_appname, + "Exec", + ex) + c.check_desktop_exec_webapp_args() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + def test_check_desktop_exec_webbrowser_local_pattern(self): '''Test test_check_desktop_exec_webbrowser_local_pattern() invalid pattern''' c = ClickReviewDesktop(self.test_name) @@ -751,7 +787,7 @@ ex) c.check_desktop_exec_webapp_args() r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 1, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) def test_check_desktop_exec_webbrowser_local_model(self): diff -Nru click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_framework.py click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_framework.py --- click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_framework.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_framework.py 2015-04-13 14:18:56.000000000 +0000 @@ -26,184 +26,178 @@ cr_tests.mock_patch() super() - def test_single_framework(self): - '''Test check_single_framework()''' + def test_framework_hook_obsolete(self): + '''Test check_framework_hook_obsolete()''' self.set_test_framework(self.default_appname, "", "") c = ClickReviewFramework(self.test_name) - c.check_single_framework() + c.check_framework_hook_obsolete() + r = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_snappy_framework_file_obsolete(self): + '''Test check_snappy_framework_file_obsolete()''' + self.set_test_pkg_yaml("type", "framework") + c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_file_obsolete() + r = c.click_report + expected_counts = {'info': 0, 'warn': 1, 'error': 0} + self.check_results(r, expected_counts) + + def test_snappy_framework_depends(self): + '''Test check_snappy_framework_depends()''' + self.set_test_pkg_yaml("type", "framework") + c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_depends() r = c.click_report - # We should end up with 1 info expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_single_framework_multiple(self): - '''Test check_single_framework() - multiple''' - self.set_test_framework(self.default_appname, "", "") - self.set_test_framework("test-alt", "", "") + def test_snappy_framework_depends_bad(self): + '''Test check_snappy_framework_depends() - bad''' + self.set_test_pkg_yaml("type", "framework") + self.set_test_pkg_yaml("frameworks", ['foo']) c = ClickReviewFramework(self.test_name) - c.check_single_framework() + c.check_snappy_framework_depends() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 0, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_framework_base_name(self): - '''Test check_framework_base_name()''' - self.set_test_framework(self.default_appname, "Base-Name", - self.test_manifest["name"]) + def test_snappy_framework_policy(self): + '''Test check_snappy_framework_policy()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) - c.check_framework_base_name() + c.check_snappy_framework_policy() r = c.click_report - # We should end up with 2 info expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_name_missing(self): - '''Test check_framework_base_name() - missing''' - self.set_test_framework(self.default_appname, "Base-Version", "") + def test_snappy_framework_policy_missing(self): + '''Test check_snappy_framework_policy() - missing''' + self.set_test_pkg_yaml("type", "framework") + self.set_test_framework_policy({}) c = ClickReviewFramework(self.test_name) - c.check_framework_base_name() + c.check_snappy_framework_policy() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_name_empty(self): - '''Test check_framework_base_name() - empty''' - self.set_test_framework(self.default_appname, "Base-Name", "") + def test_snappy_framework_policy_unknown(self): + '''Test check_snappy_framework_policy() - unknown''' + self.set_test_pkg_yaml("type", "framework") + self.set_test_framework_policy_unknown(['foo/bar/baz']) c = ClickReviewFramework(self.test_name) - c.check_framework_base_name() + c.check_snappy_framework_policy() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_version(self): - '''Test check_framework_base_version()''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_metadata(self): + '''Test check_snappy_framework_policy_metadata()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_metadata() r = c.click_report - # We should end up with 3 info - expected_counts = {'info': 3, 'warn': 0, 'error': 0} + expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_version_missing(self): - '''Test check_framework_base_version() - missing''' - self.set_test_framework(self.default_appname, "Base-Name", "") + def test_snappy_framework_policy_metadata_template(self): + '''Test check_snappy_framework_policy_metadata() - template missing + data + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + tmp['apparmor']['templates']['template-common'] = 'missing data' + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_metadata() r = c.click_report - # We should end up with 1 info expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_framework_base_version_empty(self): - '''Test check_framework_base_version() - empty''' - self.set_test_framework(self.default_appname, "Base-Version", "") + def test_snappy_framework_policy_metadata_policygroup(self): + '''Test check_snappy_framework_policy_metadata() - policygroup missing + data + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + tmp['seccomp']['policygroups']['policygroup-reserved'] = 'missing data' + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_metadata() r = c.click_report - # We should end up with 1 info expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_framework_base_version_str(self): - '''Test check_framework_base_version() - str''' - self.set_test_framework(self.default_appname, "Base-Version", "abc") + def test_snappy_framework_policy_matching(self): + '''Test check_snappy_framework_policy_matching()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_matching() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 8, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_version_negative(self): - '''Test check_framework_base_version() - negative''' - self.set_test_framework(self.default_appname, "Base-Version", -1) + def test_snappy_framework_policy_matching_missing_aa_template(self): + '''Test check_snappy_framework_policy_matching() - missing aa template + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + del tmp['apparmor']['templates']['template-common'] + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_matching() r = c.click_report - # We should end up with 1 info expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_check_peer_hooks(self): - '''Test check_peer_hooks()''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_matching_missing_sc_policygroup(self): + '''Test check_snappy_framework_policy_matching() - missing sc policy + group + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + del tmp['seccomp']['policygroups']['policygroup-reserved'] + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - - # create a new hooks database for our peer hooks tests - tmp = dict() - - # add our hook - tmp["framework"] = "foo.framework" - - # add any required peer hooks - # FIXME: - # tmp["apparmor-policy"] = "apparmor/" - - # update the manifest and test_manifest - c.manifest["hooks"][self.default_appname] = tmp - self._update_test_manifest() - - # do the test - c.check_peer_hooks() + c.check_snappy_framework_policy_matching() r = c.click_report - # We should end up with 2 info - expected_counts = {'info': 2, 'warn': 0, 'error': 0} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_check_peer_hooks_disallowed(self): - '''Test check_peer_hooks() - disallowed''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_filenames(self): + '''Test check_snappy_framework_policy_filenames()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_policy_filenames() + r = c.click_report + expected_counts = {'info': 8, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) - # create a new hooks database for our peer hooks tests - tmp = dict() - - # add our hook - tmp["framework"] = "foo.framework" - - # add any required peer hooks - # FIXME: - # tmp["apparmor-policy"] = "apparmor/" - - # add disallowed framework - tmp["nonexistent"] = "nonexistent-hook" - - # update the manifest and test_manifest - c.manifest["hooks"][self.default_appname] = tmp - self._update_test_manifest() - - # do the test - c.check_peer_hooks() + def test_snappy_framework_policy_filenames_bad(self): + '''Test check_snappy_framework_policy_filenames() - bad name''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + tmp['seccomp']['policygroups']['policygroup-res_erved'] = "foo" + self.set_test_framework_policy(tmp) + c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_policy_filenames() r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_check_peer_hooks_required(self): - '''Test check_peer_hooks() - required''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_filenames_bad2(self): + '''Test check_snappy_framework_policy_filenames() - starts with + package name + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + n = self.test_name.split('_')[0] + tmp['seccomp']['policygroups']['%s-group' % n] = "foo" + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - - # create a new hooks database for our peer hooks tests - tmp = dict() - - # add our hook - tmp["framework"] = "foo.framework" - - # skip adding required hooks - - # update the manifest and test_manifest - c.manifest["hooks"][self.default_appname] = tmp - self._update_test_manifest() - - # do the test - c.check_peer_hooks() + c.check_snappy_framework_policy_filenames() r = c.click_report - # FIXME: apparmor-policy is not defined yet, so no error, when it is - # adjust to 'error': 1 - expected_counts = {'info': None, 'warn': 0, 'error': 0} + expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_lint.py click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_lint.py --- click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_lint.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_lint.py 2015-04-13 14:18:56.000000000 +0000 @@ -801,6 +801,17 @@ expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + def test_check_hooks_type_oem(self): + '''Test check_hooks() - type: oem''' + self.set_test_manifest("framework", "ubuntu-sdk-13.10") + c = ClickReviewLint(self.test_name) + c.is_snap_oem = True + c.check_hooks() + r = c.click_report + # oem type has no hooks so these should all be '0' + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + def test_check_hooks_unknown_nonexistent(self): '''Test check_hooks_unknown() - nonexistent''' self.set_test_manifest("framework", "ubuntu-sdk-13.10") @@ -820,6 +831,17 @@ expected_counts = {'info': None, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) + def test_check_hooks_unknown_type_oem(self): + '''Test check_hooks_unknown() - type: oem''' + self.set_test_manifest("framework", "ubuntu-sdk-13.10") + c = ClickReviewLint(self.test_name) + c.is_snap_oem = True + c.check_hooks_unknown() + r = c.click_report + # oem type has no hooks so these should all be '0' + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + def test_check_hooks_redflagged_payui(self): '''Test check_hooks_redflagged() - pay-ui''' self.set_test_manifest("framework", "ubuntu-sdk-13.10") @@ -1013,24 +1035,13 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_snappy_type_framework_policy(self): - '''Test check_snappy_type - framework-policy''' - self.set_test_pkg_yaml("type", "framework-policy") - c = ClickReviewLint(self.test_name) - c.check_snappy_type() - r = c.click_report - # TODO: update this when 'framework-policy' is implemented - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - def test_snappy_type_oem(self): '''Test check_snappy_type - oem''' self.set_test_pkg_yaml("type", "oem") c = ClickReviewLint(self.test_name) c.check_snappy_type() r = c.click_report - # TODO: update this when 'oem' is implemented - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) def test_snappy_type_redflagged(self): @@ -1114,7 +1125,7 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_icon_empty(self): + def test_check_snappy_icon_empty(self): '''Test check_snappy_icon() - empty''' self.set_test_pkg_yaml("icon", "") c = ClickReviewLint(self.test_name) diff -Nru click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_systemd.py click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_systemd.py --- click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_systemd.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_systemd.py 2015-04-13 14:18:56.000000000 +0000 @@ -26,14 +26,16 @@ cr_tests.mock_patch() super() - def _set_service(self, key, value, name=None): + def _set_service(self, entries, name=None): d = dict() if name is None: - d['name'] = 'foo' + d['name'] = self.default_appname else: d['name'] = name - d[key] = value + for (key, value) in entries: + d[key] = value self.set_test_pkg_yaml("services", [d]) + self.set_test_systemd(d['name'], 'name', d['name']) def test_check_required(self): '''Test check_required() - has start and description''' @@ -85,13 +87,16 @@ self.set_test_systemd(self.default_appname, key="description", value="something") + self.set_test_systemd(self.default_appname, + key="stop", + value="/bin/foo-stop") c = ClickReviewSystemd(self.test_name) c.check_required() r = c.click_report - expected_counts = {'info': -1, 'warn': 0, 'error': 0} + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_required_multiple(self): + def test_check_required_multiple2(self): '''Test check_required() - multiple with nonexistent''' self.set_test_systemd(self.default_appname, key="start", @@ -579,7 +584,7 @@ def test_check_snappy_service_description(self): '''Test check_snappy_service_description()''' - self._set_service("description", "some description") + self._set_service([("description", "some description")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_description() @@ -589,7 +594,7 @@ def test_check_snappy_service_description_unspecified(self): '''Test check_snappy_service_description() - unspecified''' - # self._set_service("description", None) + # self._set_service([("description", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_description() @@ -600,7 +605,7 @@ def test_check_snappy_service_description_empty(self): '''Test check_snappy_service_description() - empty''' - self._set_service("description", "") + self._set_service([("description", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_description() @@ -610,7 +615,7 @@ def test_check_snappy_service_start(self): '''Test check_snappy_service_start()''' - self._set_service("start", "some/start") + self._set_service([("start", "some/start")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -620,7 +625,7 @@ def test_check_snappy_service_start_unspecified(self): '''Test check_snappy_service_start() - unspecified''' - # self._set_service("start", None) + # self._set_service([("start", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -630,7 +635,7 @@ def test_check_snappy_service_start_empty(self): '''Test check_snappy_service_start() - empty''' - self._set_service("start", "") + self._set_service([("start", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -640,7 +645,7 @@ def test_check_snappy_service_start_absolute_path(self): '''Test check_snappy_service_start() - absolute path''' - self._set_service("start", "/foo/bar/some/start") + self._set_service([("start", "/foo/bar/some/start")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -650,7 +655,7 @@ def test_check_snappy_service_stop(self): '''Test check_snappy_service_stop()''' - self._set_service("stop", "some/stop") + self._set_service([("stop", "some/stop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -660,7 +665,7 @@ def test_check_snappy_service_stop_unspecified(self): '''Test check_snappy_service_stop() - unspecified''' - # self._set_service("stop", None) + # self._set_service([("stop", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -670,7 +675,7 @@ def test_check_snappy_service_stop_empty(self): '''Test check_snappy_service_stop() - empty''' - self._set_service("stop", "") + self._set_service([("stop", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -680,7 +685,7 @@ def test_check_snappy_service_stop_absolute_path(self): '''Test check_snappy_service_stop() - absolute path''' - self._set_service("stop", "/foo/bar/some/stop") + self._set_service([("stop", "/foo/bar/some/stop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -690,7 +695,7 @@ def test_check_snappy_service_poststop(self): '''Test check_snappy_service_poststop()''' - self._set_service("poststop", "some/poststop") + self._set_service([("poststop", "some/poststop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -700,7 +705,7 @@ def test_check_snappy_service_poststop_unspecified(self): '''Test check_snappy_service_poststop() - unspecified''' - # self._set_service("poststop", None) + # self._set_service([("poststop", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -710,7 +715,7 @@ def test_check_snappy_service_poststop_empty(self): '''Test check_snappy_service_poststop() - empty''' - self._set_service("poststop", "") + self._set_service([("poststop", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -720,7 +725,7 @@ def test_check_snappy_service_poststop_absolute_path(self): '''Test check_snappy_service_poststop() - absolute path''' - self._set_service("poststop", "/foo/bar/some/poststop") + self._set_service([("poststop", "/foo/bar/some/poststop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -730,9 +735,9 @@ def test_check_snappy_service_stop_timeout(self): '''Test check_snappy_service_stop_timeout()''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value=30) + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", 30)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -742,9 +747,9 @@ def test_check_snappy_service_stop_timeout_empty(self): '''Test check_snappy_service_stop_timeout() - empty''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value="") + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -754,9 +759,9 @@ def test_check_snappy_service_stop_timeout_bad(self): '''Test check_snappy_service_stop_timeout() - bad''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value="a") + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", "a")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -766,9 +771,9 @@ def test_check_snappy_service_stop_timeout_range_low(self): '''Test check_snappy_service_stop_timeout() - out of range (low)''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value=-1) + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", -1)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -778,9 +783,9 @@ def test_check_snappy_service_stop_timeout_range_high(self): '''Test check_snappy_service_stop_timeout() - out of range (high)''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value=61) + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", 61)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() diff -Nru click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_url_dispatcher.py click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_url_dispatcher.py --- click-reviewers-tools-0.24/bin/clickreviews/tests/test_cr_url_dispatcher.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/clickreviews/tests/test_cr_url_dispatcher.py 2015-04-13 14:18:56.000000000 +0000 @@ -74,7 +74,7 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_required_multiple(self): + def test_check_required_multiple_with_nonexiting(self): '''Test check_required() - multiple with nonexistent''' self.set_test_url_dispatcher(self.default_appname, key="protocol", diff -Nru click-reviewers-tools-0.24/bin/click-run-checks click-reviewers-tools-0.25/bin/click-run-checks --- click-reviewers-tools-0.24/bin/click-run-checks 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-run-checks 2015-04-13 14:18:56.000000000 +0000 @@ -11,8 +11,10 @@ fi # prefer local script for testing, otherwise use system location +CLICK_CHECKS_BIN_PATH="$(dirname $(readlink -f "$0"))" + prefer_local() { - path="$(dirname $(readlink -f "$0"))/$1" + path="${CLICK_CHECKS_BIN_PATH}/$1" if [ -x $path ]; then "$path" "$2" || set_rc "$?" return @@ -35,49 +37,14 @@ prefer_local click-show-files "$c" -echo "" -echo "= click-check-lint =" -prefer_local click-check-lint "$c" - -echo "" -echo "= click-check-content-hub =" -prefer_local click-check-content-hub "$c" - -echo "" -echo "= click-check-desktop =" -prefer_local click-check-desktop "$c" - -echo "" -echo "= click-check-functional =" -prefer_local click-check-functional "$c" - -echo "" -echo "= click-check-online-accounts =" -prefer_local click-check-online-accounts "$c" - -echo "" -echo "= click-check-push-helper =" -prefer_local click-check-push-helper "$c" - -echo "" -echo "= click-check-scope =" -prefer_local click-check-scope "$c" - -echo "" -echo "= click-check-security =" -prefer_local click-check-security "$c" - -echo "" -echo "= click-check-url-dispatcher =" -prefer_local click-check-url-dispatcher "$c" - -echo "" -echo "= click-check-bin-path =" -prefer_local click-check-bin-path "$c" - -echo "" -echo "= click-check-framework =" -prefer_local click-check-framework "$c" +CLICK_CHECKS=$(ls ${CLICK_CHECKS_BIN_PATH}/click-check-* | grep -v click-check-skeleton | grep -v click-check-lint) +CLICK_CHECKS="click-check-lint ${CLICK_CHECKS}" +for check_path in ${CLICK_CHECKS}; do + check=$(basename ${check_path} ${CLICK_CHECKS_BIN_PATH}) + echo "" + echo "=" $check "=" + prefer_local $check "$c" +done echo "" echo "" diff -Nru click-reviewers-tools-0.24/bin/click-show-files click-reviewers-tools-0.25/bin/click-show-files --- click-reviewers-tools-0.24/bin/click-show-files 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/bin/click-show-files 2015-04-13 14:18:56.000000000 +0000 @@ -48,6 +48,15 @@ fh.close() print("") + fn = os.path.join(review.unpack_dir, "meta", "package.yaml") + if os.path.exists(fn): + print("= %s =" % os.path.basename(fn)) + fh = cr_common.open_file_read(fn) + for line in fh.readlines(): + print(line, end="") + fh.close() + print("") + print("= hooks =") review_content_hub = cr_content_hub.ClickReviewContentHub(sys.argv[1]) diff -Nru click-reviewers-tools-0.24/clickreviews/cr_bin_path.py click-reviewers-tools-0.25/clickreviews/cr_bin_path.py --- click-reviewers-tools-0.24/clickreviews/cr_bin_path.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_bin_path.py 2015-04-13 14:18:56.000000000 +0000 @@ -16,31 +16,63 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview +from clickreviews.cr_common import ClickReview, error import os class ClickReviewBinPath(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'bin-path' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['required'] = ["apparmor"] peer_hooks[my_hook]['allowed'] = peer_hooks[my_hook]['required'] - ClickReview.__init__(self, fn, "bin-path", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "bin-path", peer_hooks=peer_hooks, + overrides=overrides) + # snappy yaml currently only allows specifying: + # - exec (optional) + # - description (optional) + # - TODO: caps (optional) + self.required_keys = [] + self.optional_keys = ['description', 'exec'] + + self.bin_paths_files = dict() self.bin_paths = dict() + + if self.is_snap and 'binaries' in self.pkg_yaml: + for binary in self.pkg_yaml['binaries']: + if 'name' not in binary: + error("package.yaml malformed: required 'name' not found " + "for entry in %s" % self.pkg_yaml['binaries']) + elif not isinstance(binary['name'], str): + error("package.yaml malformed: required 'name' is not str" + "for entry in %s" % self.pkg_yaml['binaries']) + + app = os.path.basename(binary['name']) + if 'exec' in binary: + rel = binary['exec'] + else: + rel = binary['name'] + self.bin_paths[app] = rel + self.bin_paths_files[app] = self._extract_bin_path(app) + + # Now verify click manifest for app in self.manifest['hooks']: - if 'bin-path' not in self.manifest['hooks'][app]: + if not self.is_snap and \ + 'bin-path' not in self.manifest['hooks'][app]: + # non-snappy clicks don't need bin-path hook # msg("Skipped missing bin-path hook for '%s'" % app) continue - self.bin_paths[app] = self._extract_bin_path(app) + elif 'bin-path' in self.manifest['hooks'][app] and \ + not isinstance(self.manifest['hooks'][app]['bin-path'], str): + error("manifest malformed: hooks/%s/bin-path is not str" % app) def _extract_bin_path(self, app): '''Get bin-path for app''' - rel = self.manifest['hooks'][app]['bin-path'] + rel = self.bin_paths[app] fn = os.path.join(self.unpack_dir, rel) if not os.path.exists(fn): error("Could not find '%s'" % rel) @@ -48,24 +80,137 @@ def _check_bin_path_executable(self, app): '''Check that the provided path exists''' - rel = self.manifest['hooks'][app]['bin-path'] - fn = os.path.join(self.unpack_dir, rel) + fn = self.bin_paths_files[app] return os.access(fn, os.X_OK) + def check_click_hooks(self): + '''Check that the click hooks match the package.yaml''' + t = 'info' + n = 'manifest_matches_yaml' + s = "OK" + extra = [] + for app in self.manifest['hooks']: + if 'bin-path' in self.manifest['hooks'][app] and \ + app not in self.bin_paths: + extra.append(app) + if len(extra) > 0: + t = 'error' + s = 'manifest has extra bin-path hooks: %s' % ",".join(extra) + self._add_result(t, n, s) + + t = 'info' + n = 'yaml_matches_manifest' + s = "OK" + missing = [] + for app in self.bin_paths: + if app not in self.manifest['hooks'] or \ + 'bin-path' not in self.manifest['hooks'][app]: + missing.append(app) + if len(missing) > 0: + t = 'error' + s = 'manifest has missing bin-path hooks: %s' % ",".join(missing) + self._add_result(t, n, s) + + def _verify_required(self, my_dict, test_str): + for app in sorted(my_dict): + for r in self.required_keys: + found = False + t = 'info' + n = '%s_required_key_%s_%s' % (test_str, r, app) + s = "OK" + if r in my_dict[app]: + if not isinstance(my_dict[app][r], str): + t = 'error' + s = "'%s' is not a string" % r + elif my_dict[app][r] == "": + t = 'error' + s = "'%s' is empty" % r + else: + found = True + if not found and t != 'error': + t = 'error' + s = "Missing required field '%s'" % r + self._add_result(t, n, s) + + def check_snappy_required(self): + '''Check for package.yaml required fields''' + if not self.is_snap or 'binaries' not in self.pkg_yaml: + return + self._verify_required(self._create_dict(self.pkg_yaml['binaries']), + 'package_yaml') + + def _verify_optional(self, my_dict, test_str): + for app in sorted(my_dict): + for o in self.optional_keys: + found = False + t = 'info' + n = '%s_optional_key_%s_%s' % (test_str, o, app) + s = "OK" + if o in my_dict[app]: + if o == 'stop-timeout' and \ + not isinstance(my_dict[app][o], int): + t = 'error' + s = "'%s' is not an integer" % o + elif not isinstance(my_dict[app][o], str): + t = 'error' + s = "'%s' is not a string" % o + elif my_dict[app][o] == "": + t = 'error' + s = "'%s' is empty" % o + else: + found = True + if not found and t != 'error': + s = "OK (skip missing)" + self._add_result(t, n, s) + + def check_snappy_optional(self): + '''Check snappy packate.yaml optional fields''' + if not self.is_snap or 'binaries' not in self.pkg_yaml: + return + self._verify_optional(self._create_dict(self.pkg_yaml['binaries']), + 'package_yaml') + + def _verify_unknown(self, my_dict, test_str): + for app in sorted(my_dict): + unknown = [] + t = 'info' + n = '%s_unknown_key_%s' % (test_str, app) + s = "OK" + + for f in my_dict[app].keys(): + if f not in self.required_keys and \ + f not in self.optional_keys: + unknown.append(f) + + if len(unknown) == 1: + t = 'warn' + s = "Unknown field '%s'" % unknown[0] + elif len(unknown) > 1: + t = 'warn' + s = "Unknown fields '%s'" % ", ".join(unknown) + self._add_result(t, n, s) + + def check_snappy_unknown(self): + '''Check snappy package.yaml unknown fields''' + if not self.is_snap or 'binaries' not in self.pkg_yaml: + return + self._verify_unknown(self._create_dict(self.pkg_yaml['binaries']), + 'package_yaml') + def check_path(self): '''Check path exists''' t = 'info' n = 'path exists' s = "OK" - for app in sorted(self.bin_paths): + for app in sorted(self.bin_paths_files): t = 'info' n = 'path executable' s = "OK" if not self._check_bin_path_executable(app): t = 'error' s = "'%s' is not executable" % \ - (self.manifest['hooks'][app]['bin-path']) + os.path.relpath(self.bin_paths_files[app], self.unpack_dir) self._add_result(t, n, s) def check_binary_description(self): diff -Nru click-reviewers-tools-0.24/clickreviews/cr_common.py click-reviewers-tools-0.25/clickreviews/cr_common.py --- click-reviewers-tools-0.24/clickreviews/cr_common.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_common.py 2015-04-13 14:18:56.000000000 +0000 @@ -60,7 +60,7 @@ class ClickReview(object): '''This class represents click reviews''' # Convenience to break out common types of clicks (eg, app, scope, - # framework, click service) + # click service) app_allowed_peer_hooks = ["account-application", "account-service", "apparmor", @@ -75,8 +75,6 @@ "scope", ] # FIXME: when apparmor-policy is implemented, use this - # framework_allowed_peer_hooks = ["apparmor-policy"] - framework_allowed_peer_hooks = [] service_allowed_peer_hooks = ["apparmor", "bin-path", "snappy-systemd", @@ -86,16 +84,19 @@ # optional snappy fields here (may be required by appstore) snappy_optional = ["architecture", "binaries", + "config", "frameworks", "icon", + "immutable-config", "integration", + "oem", "services", "source", "type", "vendor", # replaces maintainer ] - def __init__(self, fn, review_type, peer_hooks=None): + def __init__(self, fn, review_type, peer_hooks=None, overrides=None): self.click_package = fn self._check_path_exists() if not self.click_package.endswith(".click") and \ @@ -144,11 +145,20 @@ if pkg_yaml is not None: try: self.pkg_yaml = yaml.safe_load(pkg_yaml) - except Exception as e: + except Exception: error("Could not load package.yaml. Is it properly formatted?") self._verify_package_yaml_structure() self.is_snap = True + # default to 'app' + if 'type' not in self.pkg_yaml: + self.pkg_yaml['type'] = 'app' + + self.is_snap_oem = False + if self.is_snap and 'type' in self.pkg_yaml and \ + self.pkg_yaml['type'] == 'oem': + self.is_snap_oem = True + # Get a list of all unpacked files, except DEBIAN/ self.pkg_files = [] self._list_all_files() @@ -164,6 +174,7 @@ self.valid_frameworks = self._extract_click_frameworks() self.peer_hooks = peer_hooks + self.overrides = overrides if overrides is not None else {} def _extract_click_frameworks(self): '''Extract installed click frameworks''' @@ -251,6 +262,14 @@ error("manifest malformed: '%s' is not str or list:\n%s" % (f, mp)) + # FIXME: this is kinda gross but the best we can do while we are trying + # to support clicks and native snaps + if 'type' in self.manifest and self.manifest['type'] == 'oem': + if 'hooks' in self.manifest: + error("'hooks' present in manifest with type 'oem'") + # mock up something for other tests + self.manifest['hooks'] = {'oem': {'reviewtools': True}} + # Not required by click, but required by appstore. 'hooks' is assumed # to be present in other checks if 'hooks' not in self.manifest: @@ -300,7 +319,7 @@ (isinstance(self.pkg_yaml[f], str) or isinstance(self.pkg_yaml[f], list)): error("yaml malformed: '%s' is not str or list:\n%s" % - (f, mp)) + (f, yp)) elif f in ["binaries", "services"] and not \ isinstance(self.pkg_yaml[f], list): error("yaml malformed: '%s' is not list:\n%s" % (f, yp)) @@ -609,3 +628,20 @@ recursive_rm(path) if contents_only is False: os.rmdir(dirPath) + + +def run_click_check(cls): + if len(sys.argv) < 2: + error("Must give path to click package") + + # extract args + fn = sys.argv[1] + if len(sys.argv) > 2: + overrides = json.loads(sys.argv[2]) + else: + overrides = None + + review = cls(fn, overrides=overrides) + review.do_checks() + rc = review.do_report() + sys.exit(rc) diff -Nru click-reviewers-tools-0.24/clickreviews/cr_content_hub.py click-reviewers-tools-0.25/clickreviews/cr_content_hub.py --- click-reviewers-tools-0.24/clickreviews/cr_content_hub.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_content_hub.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_content_hub.py: click content-hub checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,21 +16,22 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import json import os class ClickReviewContentHub(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'content-hub' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks peer_hooks[my_hook]['required'] = [] - ClickReview.__init__(self, fn, "content_hub", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "content_hub", peer_hooks=peer_hooks, + overrides=overrides) self.valid_keys = ['destination', 'share', 'source'] diff -Nru click-reviewers-tools-0.24/clickreviews/cr_desktop.py click-reviewers-tools-0.25/clickreviews/cr_desktop.py --- click-reviewers-tools-0.24/clickreviews/cr_desktop.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_desktop.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_desktop.py: click desktop checks''' # -# Copyright (C) 2013-2014 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import glob import json import os @@ -28,14 +28,15 @@ class ClickReviewDesktop(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'desktop' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks peer_hooks[my_hook]['required'] = ["apparmor"] - ClickReview.__init__(self, fn, "desktop", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "desktop", peer_hooks=peer_hooks, + overrides=overrides) self.desktop_files = dict() # click-show-files and a couple tests self.desktop_entries = dict() @@ -318,15 +319,20 @@ found_named_webapp = False urls = [] for i in de.getExec().split(): + if i == "webbrowser-app" or i == "webapp-container": + continue if i.startswith('--webappUrlPatterns'): found_url_patterns = True if i.startswith('--webappModelSearchPath'): found_model_search_path = True if i.startswith('--webapp='): found_model_search_path = True - if not i.startswith('--'): + # consider potential Exec field codes as 'non urls' + if not i.startswith('--') and not i.startswith('%'): urls.append(i) is_launching_local_app = True + if len(urls) == 0: + is_launching_local_app = False for url in urls: parts = urlsplit(url) if parts.scheme in ['http', 'https']: diff -Nru click-reviewers-tools-0.24/clickreviews/cr_framework.py click-reviewers-tools-0.25/clickreviews/cr_framework.py --- click-reviewers-tools-0.24/clickreviews/cr_framework.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_framework.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_framework.py: click framework''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,19 +16,16 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, open_file_read +from clickreviews.cr_common import ClickReview, error, open_file_read +import glob import os +import re class ClickReviewFramework(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): - peer_hooks = dict() - my_hook = 'framework' - peer_hooks[my_hook] = dict() - peer_hooks[my_hook]['allowed'] = ClickReview.framework_allowed_peer_hooks - peer_hooks[my_hook]['required'] = peer_hooks[my_hook]['allowed'] - ClickReview.__init__(self, fn, "framework", peer_hooks=peer_hooks) + def __init__(self, fn, overrides=None): + ClickReview.__init__(self, fn, "framework", overrides=overrides) self.frameworks_file = dict() self.frameworks = dict() @@ -43,6 +40,11 @@ self.frameworks_file[app] = full_fn self.frameworks[app] = data + self.framework_policy_dirs = ['apparmor', 'seccomp'] + self.framework_policy_subdirs = ['templates', 'policygroups'] + (self.framework_policy, self.framework_policy_unknown) = \ + self._extract_framework_policy() + def _extract_framework(self, app): '''Get framework for app''' rel = self.manifest['hooks'][app]['framework'] @@ -61,70 +63,186 @@ return (fn, data) - def check_single_framework(self): - '''Check only have one framework in the click''' + def _extract_framework_policy(self): + '''Get framework policy files''' + policy_dict = dict() + unknown_dict = dict() + fpdir = os.path.join(self.unpack_dir, "meta", "framework-policy") + for i in glob.glob("%s/*" % fpdir): + rel_i = os.path.basename(i) + if not os.path.isdir(i) or rel_i not in self.framework_policy_dirs: + unknown_dict.append(os.path.relpath(i, self.unpack_dir)) + continue + + policy_dict[rel_i] = dict() + for j in glob.glob("%s/*" % i): + rel_j = os.path.basename(j) + if not os.path.isdir(j) or \ + rel_j not in self.framework_policy_subdirs: + unknown_dict.append(os.path.relpath(j, self.unpack_dir)) + continue + + policy_dict[rel_i][rel_j] = dict() + for k in glob.glob("%s/*" % j): + rel_k = os.path.basename(k) + if not os.path.isfile(k): + unknown_dict.append(os.path.relpath(k, + self.unpack_dir)) + continue + + fh = open_file_read(k) + policy_dict[rel_i][rel_j][rel_k] = fh.read() + fh.close() + return (policy_dict, unknown_dict) + + def _has_framework_in_metadir(self): + '''Check if snap has meta/.framework''' + if not self.is_snap: + return False + + return os.path.exists(os.path.join(self.unpack_dir, 'meta', + '%s.framework' % + self.pkg_yaml['name'])) + + def check_framework_hook_obsolete(self): + '''Check manifest doesn't specify 'framework' hook''' t = 'info' - n = 'single_framework' + n = "obsolete_declaration" s = "OK" - if len(self.frameworks.keys()) > 1: + if len(self.frameworks) > 0: t = 'error' - s = 'framework hook specified multiple times' + s = "framework hook found for '%s'" % \ + ",".join(sorted(self.frameworks)) self._add_result(t, n, s) - def check_framework_base_name(self): - '''Check framework Base-Name''' - for app in sorted(self.frameworks): - t = 'info' - n = "base_name_present '%s'" % app - s = "OK" - if 'Base-Name' not in self.frameworks[app]: - t = 'error' - s = "Could not find 'Base-Name' in '%s'" % \ - (self.frameworks_file[app]) - self._add_result(t, n, s) - continue - self._add_result(t, n, s) + def check_snappy_framework_file_obsolete(self): + '''Check snap doesn't ship .framework file''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + t = 'info' + n = "obsolete_framework_file" + s = "OK" + if self._has_framework_in_metadir(): + t = 'warn' + s = "found '%s.framework' (safe to remove)" % self.pkg_yaml['name'] + self._add_result(t, n, s) - t = 'info' - n = "base_name_namespacing '%s'" % app - s = "OK" - if self.frameworks[app]['Base-Name'] != self.manifest['name']: - t = 'error' - s = "'%s' != '%s'" % (self.frameworks[app]['Base-Name'], - self.manifest['name']) - self._add_result(t, n, s) + def check_snappy_framework_depends(self): + '''Check framework doesn't depend on other frameworks''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + t = 'info' + n = "framework_dependency" + s = "OK" + if "frameworks" in self.pkg_yaml: + t = 'error' + s = "'type: framework' may not specify 'frameworks'" + self._add_result(t, n, s) - def check_framework_base_version(self): - '''Check framework Base-Version''' - for app in sorted(self.frameworks): - t = 'info' - n = "base_version_present '%s'" % app - s = "OK" - if 'Base-Version' not in self.frameworks[app]: - t = 'error' - s = "Could not find 'Base-Version' in '%s'" % \ - (self.frameworks_file[app]) - self._add_result(t, n, s) - continue - self._add_result(t, n, s) + def check_snappy_framework_policy(self): + '''Check framework ships at least some policy''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return - v = self.frameworks[app]['Base-Version'] - t = 'info' - n = "base_version_number '%s'" % app - s = "OK" - try: - float(v) - except ValueError: - t = 'error' - s = "'Base-Version' malformed: '%s' is not a number" % v - self._add_result(t, n, s) + t = 'info' + n = "framework_policies" + s = "OK" + found = False + for i in self.framework_policy_dirs: + if i not in self.framework_policy: continue - self._add_result(t, n, s) + for j in self.framework_policy_subdirs: + if j not in self.framework_policy[i]: + continue + if len(self.framework_policy[i][j].keys()) > 0: + found = True + if not found: + t = 'warn' + s = "security policy not found" + self._add_result(t, n, s) - t = 'info' - n = "base_version_positive '%s'" % app - s = "OK" - if float(v) < 0: - t = 'error' - s = "'Base-Version' malformed: '%s' is negative" % v + t = 'info' + n = "framework_policy_unknown" + s = "OK" + if len(self.framework_policy_unknown) > 0: + t = 'warn' + s = "framework policy has unexpected entries: '%s'" % \ + ",".join(self.framework_policy_unknown) + self._add_result(t, n, s) + + def check_snappy_framework_policy_metadata(self): + '''Check framework policy has expected meta data''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + + t = 'info' + n = "framework_policy_metadata" + s = "OK" + msgs = [] + for term in ["# Description: ", "# Usage: "]: + for i in self.framework_policy_dirs: + if i not in self.framework_policy: + continue + for j in self.framework_policy_subdirs: + if j not in self.framework_policy[i]: + continue + for k in self.framework_policy[i][j].keys(): + found = False + for l in self.framework_policy[i][j][k].splitlines(): + if l.startswith(term): + found = True + if not found: + msgs.append("'%s' in '%s/%s/%s'" % (term, i, j, k)) + if len(msgs) > 0: + t = 'error' + s = "Could not find meta data: %s" % ",".join(msgs) + self._add_result(t, n, s) + + def check_snappy_framework_policy_matching(self): + '''Check framework policy ships apparmor and seccomp for each''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + + t = 'info' + n = "framework_has_all_policy" + s = "OK" + if len(self.framework_policy.keys()) == 0: + s = "OK (skipped missing policy)" self._add_result(t, n, s) + return + + for i in self.framework_policy: + for j in self.framework_policy[i]: + for k in self.framework_policy[i][j]: + for other in self.framework_policy_dirs: + if t == i: + continue + t = 'info' + n = "framework_policy_%s/%s/%s" % (i, j, k) + s = "OK" + if j not in self.framework_policy[other] or \ + k not in self.framework_policy[other][j]: + t = 'error' + s = "Could not find mathcing '%s/%s/%s'" % (other, + j, k) + self._add_result(t, n, s) + + def check_snappy_framework_policy_filenames(self): + '''Check framework policy file names''' + if not self.is_snap or self.pkg_yaml['type'] != 'framework': + return + + for i in self.framework_policy: + for j in self.framework_policy[i]: + for k in self.framework_policy[i][j]: + f = "%s/%s/%s" % (i, j, k) + t = 'info' + n = "framework_policy_valid_name_%s" % f + s = "OK" + if not re.search(r'^[a-z0-9][a-z0-9+\.-]+$', k): + t = 'error' + s = "'%s' should match '^[a-z0-9][a-z0-9+\.-]+$'" % f + elif k.startswith(self.pkg_yaml['name']): + t = 'warn' + s = "'%s' should not begin with package name" % f + self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.24/clickreviews/cr_functional.py click-reviewers-tools-0.25/clickreviews/cr_functional.py --- click-reviewers-tools-0.24/clickreviews/cr_functional.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_functional.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_functional.py: click functional''' # -# Copyright (C) 2013-2014 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -27,8 +27,8 @@ class ClickReviewFunctional(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): - ClickReview.__init__(self, fn, "functional") + def __init__(self, fn, overrides=None): + ClickReview.__init__(self, fn, "functional", overrides=overrides) self.qml_files = [] for i in self.pkg_files: diff -Nru click-reviewers-tools-0.24/clickreviews/cr_lint.py click-reviewers-tools-0.25/clickreviews/cr_lint.py --- click-reviewers-tools-0.24/clickreviews/cr_lint.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_lint.py 2015-04-13 14:18:56.000000000 +0000 @@ -24,7 +24,7 @@ from clickreviews.frameworks import Frameworks from clickreviews.cr_common import ClickReview, open_file_read, cmd -CONTROL_FILE_NAMES = ["control", "manifest", "md5sums", "preinst"] +CONTROL_FILE_NAMES = ["control", "manifest", "preinst"] MINIMUM_CLICK_FRAMEWORK_VERSION = "0.4" @@ -33,7 +33,9 @@ def __init__(self, fn, overrides=None): '''Set up the class.''' - ClickReview.__init__(self, fn, "lint") + ClickReview.__init__(self, fn, "lint", overrides=overrides) + if not self.is_snap and "md5sums" not in CONTROL_FILE_NAMES: + CONTROL_FILE_NAMES.append("md5sums") self.control_files = dict() self._list_control_files() # Valid values for Architecture in DEBIAN/control. Note: @@ -93,15 +95,13 @@ 'bin-path', 'content-hub', 'desktop', - 'framework', 'pay-ui', 'push-helper', 'scope', 'snappy-systemd', 'urls'] - self.redflagged_hooks = ['framework', - 'pay-ui', + self.redflagged_hooks = ['pay-ui', 'apparmor-profile', ] @@ -111,17 +111,12 @@ # - oem self.snappy_valid_types = ['app', 'framework', - # 'framework-policy', # TBI - # 'oem', # TBI + 'oem', ] self.snappy_redflagged_types = ['framework', - # 'oem', # TBI + 'oem', # TBD ] - if overrides is None: - overrides = {} - self.overrides = overrides - def _list_control_files(self): '''List all control files with their full path.''' for i in CONTROL_FILE_NAMES: @@ -135,8 +130,11 @@ n = 'DEBIAN_has_%s' % os.path.basename(f) s = "OK" if not os.path.isfile(self.control_files[os.path.basename(f)]): - t = 'error' - s = "'%s' not found in DEBIAN/" % os.path.basename(f) + if self.is_snap and os.path.basename(f) == 'md5sums': + s = "OK (skip md5sums with snap)" + else: + t = 'error' + s = "'%s' not found in DEBIAN/" % os.path.basename(f) self._add_result(t, n, s) found = [] @@ -260,7 +258,8 @@ n = 'control_description_match' s = 'OK' if 'title' in self.manifest: - if control['Description'] != self.manifest['title']: + if control['Description'].strip() != \ + self.manifest['title'].strip(): t = 'error' s = "Description=%s does not match manifest title=%s" % \ (control['Description'], self.manifest['title']) @@ -294,6 +293,8 @@ def check_md5sums(self): '''Check md5sums()''' + if self.is_snap: + return curdir = os.getcwd() fh = open_file_read(self.control_files["md5sums"]) badsums = [] @@ -338,6 +339,10 @@ def check_hooks(self): '''Check click manifest hooks''' + # oem snaps don't have a hooks entry + if self.is_snap_oem: + return + # Some checks are already handled in # cr_common.py:_verify_manifest_structure() @@ -431,6 +436,10 @@ def check_hooks_unknown(self): '''Check if have any unknown hooks''' + # oem snaps don't have a hooks entry + if self.is_snap_oem: + return + t = 'info' n = 'unknown hooks' s = 'OK' @@ -785,9 +794,13 @@ else: arch = tmp[2].partition('.click')[0] if arch != self.click_arch: - t = 'error' - s = "'%s' != '%s' from DEBIAN/control" % (arch, - self.click_arch) + if arch == 'all' and self.click_arch == 'multi': + # The store creates filenames for fat packages with _all + pass + else: + t = 'error' + s = "'%s' != '%s' from DEBIAN/control" % (arch, + self.click_arch) else: t = 'warn' s = "could not determine architecture from '%s'" % \ diff -Nru click-reviewers-tools-0.24/clickreviews/cr_online_accounts.py click-reviewers-tools-0.25/clickreviews/cr_online_accounts.py --- click-reviewers-tools-0.24/clickreviews/cr_online_accounts.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_online_accounts.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_online_accounts.py: click online accounts''' # -# Copyright (C) 2013 Canonical Ltd. +# Copyright (C) 2013-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error import os # http://lxml.de/tutorial.html import lxml.etree as etree @@ -24,7 +24,7 @@ class ClickReviewAccounts(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() peer_hooks['account-application'] = dict() peer_hooks['account-application']['allowed'] = \ @@ -54,7 +54,8 @@ peer_hooks['account-qml-plugin']['allowed'] = \ peer_hooks['account-qml-plugin']['required'] - ClickReview.__init__(self, fn, "online_accounts", peer_hooks) + ClickReview.__init__(self, fn, "online_accounts", peer_hooks=peer_hooks, + overrides=overrides) self.accounts_files = dict() self.accounts = dict() @@ -100,8 +101,7 @@ tree = etree.parse(fn) xml = tree.getroot() except Exception as e: - error("accounts xml unparseable: %s (%s):\n%s" % (bn, str(e), - contents)) + error("accounts xml unparseable: %s (%s)" % (bn, str(e))) return (fn, xml) def check_application(self): @@ -126,7 +126,6 @@ t = 'info' n = '%s_%s_id' % (app, account_type) s = "OK" - expected_id = "%s_%s" % (self.manifest["name"], app) if "id" in self.accounts[app][account_type].keys(): t = 'warn' s = "Found 'id' in application tag" @@ -174,7 +173,6 @@ t = 'info' n = '%s_%s_id' % (app, account_type) s = "OK" - expected_id = "%s_%s" % (self.manifest["name"], app) if "id" in self.accounts[app][account_type].keys(): t = 'warn' s = "Found 'id' in service tag" @@ -223,7 +221,6 @@ t = 'info' n = '%s_%s_id' % (app, account_type) s = "OK" - expected_id = "%s_%s" % (self.manifest["name"], app) if "id" in self.accounts[app][account_type].keys(): t = 'warn' s = "Found 'id' in provider tag" diff -Nru click-reviewers-tools-0.24/clickreviews/cr_push_helper.py click-reviewers-tools-0.25/clickreviews/cr_push_helper.py --- click-reviewers-tools-0.24/clickreviews/cr_push_helper.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_push_helper.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_push_helper.py: click push-helper checks''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,21 +16,22 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import json import os class ClickReviewPushHelper(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'push-helper' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ['apparmor'] peer_hooks[my_hook]['required'] = ['apparmor'] - ClickReview.__init__(self, fn, "push_helper", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "push_helper", peer_hooks=peer_hooks, + overrides=overrides) self.required_keys = ['exec'] self.optional_keys = ['app_id'] diff -Nru click-reviewers-tools-0.24/clickreviews/cr_scope.py click-reviewers-tools-0.25/clickreviews/cr_scope.py --- click-reviewers-tools-0.24/clickreviews/cr_scope.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_scope.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_scope.py: click scope''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, msg +from clickreviews.cr_common import ClickReview, error import codecs import configparser import os @@ -30,14 +30,15 @@ class ClickReviewScope(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'scope' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.scope_allowed_peer_hooks peer_hooks[my_hook]['required'] = ['apparmor'] - ClickReview.__init__(self, fn, "scope", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "scope", peer_hooks=peer_hooks, + overrides=overrides) self.scopes = dict() for app in self.manifest['hooks']: diff -Nru click-reviewers-tools-0.24/clickreviews/cr_security.py click-reviewers-tools-0.25/clickreviews/cr_security.py --- click-reviewers-tools-0.24/clickreviews/cr_security.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_security.py 2015-04-13 14:18:56.000000000 +0000 @@ -43,7 +43,8 @@ ClickReview.service_allowed_peer_hooks peer_hooks[my_hook2]['required'] = [] - ClickReview.__init__(self, fn, "security", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "security", peer_hooks=peer_hooks, + overrides=overrides) local_copy = os.path.join(os.path.dirname(__file__), '../data/apparmor-easyprof-ubuntu.json') @@ -111,9 +112,7 @@ 'policy_version': 1.3, }, } - if overrides is None: - overrides = {} - framework_overrides = overrides.get('framework', {}) + framework_overrides = self.overrides.get('framework', {}) self._override_framework_policies(framework_overrides) self.security_manifests = dict() diff -Nru click-reviewers-tools-0.24/clickreviews/cr_skeleton.py click-reviewers-tools-0.25/clickreviews/cr_skeleton.py --- click-reviewers-tools-0.24/clickreviews/cr_skeleton.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_skeleton.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_skeleton.py: click skeleton''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -21,7 +21,7 @@ class ClickReviewSkeleton(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): # Many test classes are for verify click hooks. 'peer_hooks' is used # to declare what hooks may be use with my_hook. When using this # mechanism, ClickReview.check_peer_hooks() is run for you. @@ -31,7 +31,8 @@ peer_hooks[my_hook]['allowed'] = ["desktop", "apparmor", "urls"] peer_hooks[my_hook]['required'] = ["desktop", "apparmor"] - ClickReview.__init__(self, fn, "skeleton", peer_hooks) + ClickReview.__init__(self, fn, "skeleton", peer_hooks=peer_hooks, + overrides=overrides) # If not a hooks test, skip the above and omit peer_hooks like so: # ClickReview.__init__(self, fn, "skeleton") diff -Nru click-reviewers-tools-0.24/clickreviews/cr_systemd.py click-reviewers-tools-0.25/clickreviews/cr_systemd.py --- click-reviewers-tools-0.24/clickreviews/cr_systemd.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_systemd.py 2015-04-13 14:18:56.000000000 +0000 @@ -16,21 +16,22 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import yaml import os class ClickReviewSystemd(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'snappy-systemd' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['required'] = ["apparmor"] peer_hooks[my_hook]['allowed'] = peer_hooks[my_hook]['required'] - ClickReview.__init__(self, fn, "snappy-systemd", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "snappy-systemd", peer_hooks=peer_hooks, + overrides=overrides) # snappy-systemd currently only allows specifying: # - start (required) @@ -38,27 +39,43 @@ # - stop # - poststop # - stop-timeout + # - TODO: caps self.required_keys = ['start', 'description'] self.optional_keys = ['stop', 'poststop', 'stop-timeout'] self.systemd_files = dict() # click-show-files and tests self.systemd = dict() + + if self.is_snap and 'services' in self.pkg_yaml: + for service in self.pkg_yaml['services']: + if 'name' not in service: + error("package.yaml malformed: required 'name' not found " + "for entry in %s" % self.pkg_yaml['services']) + elif not isinstance(service['name'], str): + error("package.yaml malformed: required 'name' is not str" + "for entry in %s" % self.pkg_yaml['services']) + + app = service['name'] + (full_fn, yd) = self._extract_systemd(app) + self.systemd_files[app] = full_fn + self.systemd[app] = yd + + # Now verify click manifest for app in self.manifest['hooks']: - if 'snappy-systemd' not in self.manifest['hooks'][app]: + if not self.is_snap and \ + 'snappy-systemd' not in self.manifest['hooks'][app]: + # non-snappy clicks don't need snappy-systemd hook # msg("Skipped missing systemd hook for '%s'" % app) continue - if not isinstance(self.manifest['hooks'][app]['snappy-systemd'], - str): + elif 'snappy-systemd' in self.manifest['hooks'][app] and \ + not isinstance(self.manifest['hooks'][app]['snappy-systemd'], + str): error("manifest malformed: hooks/%s/snappy-systemd is not str" % app) - (full_fn, jd) = self._extract_systemd(app) - self.systemd_files[app] = full_fn - self.systemd[app] = jd def _extract_systemd(self, app): '''Get systemd yaml''' - u = self.manifest['hooks'][app]['snappy-systemd'] - fn = os.path.join(self.unpack_dir, u) + fn = os.path.join(self.unpack_dir, "meta", "%s.snappy-systemd" % app) bn = os.path.basename(fn) if not os.path.exists(fn): diff -Nru click-reviewers-tools-0.24/clickreviews/cr_tests.py click-reviewers-tools-0.25/clickreviews/cr_tests.py --- click-reviewers-tools-0.24/clickreviews/cr_tests.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_tests.py 2015-04-13 14:18:56.000000000 +0000 @@ -46,6 +46,8 @@ TEST_PUSH_HELPER = dict() TEST_BIN_PATH = dict() TEST_FRAMEWORK = dict() +TEST_FRAMEWORK_POLICY = dict() +TEST_FRAMEWORK_POLICY_UNKNOWN = [] TEST_SNAPPY_SYSTEMD = dict() @@ -198,6 +200,16 @@ return ("%s.framework" % app, TEST_FRAMEWORK[app]) +def _extract_framework_policy(self): + '''Pretend we found the framework policy files''' + return (TEST_FRAMEWORK_POLICY, TEST_FRAMEWORK_POLICY_UNKNOWN) + + +def _has_framework_in_metadir(self): + '''Pretend we found the framework file''' + return True + + def _extract_systemd(self, app): '''Pretend we found the systemd file''' return ("%s.snappy-systemd" % app, TEST_SNAPPY_SYSTEMD[app]) @@ -312,6 +324,12 @@ patches.append(patch( 'clickreviews.cr_framework.ClickReviewFramework._extract_framework', _extract_framework)) +patches.append(patch( + 'clickreviews.cr_framework.ClickReviewFramework._extract_framework_policy', + _extract_framework_policy)) +patches.append(patch( + 'clickreviews.cr_framework.ClickReviewFramework._has_framework_in_metadir', + _has_framework_in_metadir)) # systemd overrides patches.append(patch( @@ -399,6 +417,8 @@ self.test_push_helper = dict() self.test_bin_path = dict() self.test_framework = dict() + self.test_framework_policy = dict() + self.test_framework_policy_unknown = [] self.test_systemd = dict() for app in self.test_manifest["hooks"].keys(): # setup security manifest for each app @@ -444,6 +464,10 @@ # Reset to no framework entries in manifest self.set_test_framework(app, None, None) + # Reset to no framework entries in manifest + self.set_test_framework_policy(None) + self.set_test_framework_policy_unknown([]) + # Reset to no systemd entries in manifest self.set_test_systemd(app, None, None) @@ -463,6 +487,8 @@ self._update_test_push_helper() self._update_test_bin_path() self._update_test_framework() + self._update_test_framework_policy() + self._update_test_framework_policy_unknown() self._update_test_systemd() # webapps manifests (leave empty for now) @@ -617,6 +643,14 @@ "%s.framework" % TEST_FRAMEWORK[app] self._update_test_manifest() + def _update_test_framework_policy(self): + global TEST_FRAMEWORK_POLICY + TEST_FRAMEWORK_POLICY = self.test_framework_policy + + def _update_test_framework_policy_unknown(self): + global TEST_FRAMEWORK_POLICY_UNKNOWN + TEST_FRAMEWORK_POLICY_UNKNOWN = self.test_framework_policy_unknown + def _update_test_systemd(self): global TEST_SNAPPY_SYSTEMD TEST_SNAPPY_SYSTEMD = dict() @@ -851,7 +885,44 @@ def set_test_bin_path(self, app, value): '''Set bin-path entries. If value is None, remove bin-path from - manifest''' + manifest and yaml. If app != value, set 'exec' in the yaml + + Note the click manifest and the package.yaml use different + storage types. pkg_yaml['binaries'] is a list of dictionaries where + manifest['hooks'] is a dictionary of dictionaries. This function + sets the manifest entry and then a yaml entry with 'name' and 'exec' + fields. + + manifest['hooks'][app]['bin-path'] = value + pkg_yaml['binaries'][*]['name'] = app + pkg_yaml['binaries'][*]['exec'] = value + ''' + + # Update the package.yaml + if value is None: + if 'binaries' in self.test_pkg_yaml: + for b in self.test_pkg_yaml['binaries']: + if 'name' in b and b['name'] == app: + self.test_pkg_yaml['binaries'].remove(b) + break + else: + found = False + if 'binaries' in self.test_pkg_yaml: + for b in self.test_pkg_yaml['binaries']: + if 'name' in b and b['name'] == app: + found = True + break + if not found: + if 'binaries' not in self.test_pkg_yaml: + self.test_pkg_yaml['binaries'] = [] + if value == app: + self.test_pkg_yaml['binaries'].append({'name': app}) + else: + self.test_pkg_yaml['binaries'].append({'name': app, + 'exec': value}) + self._update_test_pkg_yaml() + + # Update the click manifest (we still support click format) if value is None: if app in self.test_bin_path: self.test_bin_path.pop(app) @@ -859,6 +930,8 @@ if app not in self.test_bin_path: self.test_bin_path[app] = dict() self.test_bin_path[app] = value + + # Now update TEST_BIN_PATH self._update_test_bin_path() def set_test_framework(self, app, key, value): @@ -876,9 +949,68 @@ self.test_framework[app][key] = value self._update_test_framework() + def set_test_framework_policy(self, policy_dict=None): + '''Set framework policy''' + if policy_dict is None: # Reset + self.test_framework_policy = dict() + for i in ['apparmor', 'seccomp']: + self.test_framework_policy[i] = dict() + for j in ['templates', 'policygroups']: + self.test_framework_policy[i][j] = dict() + for k in ['-common', '-reserved']: + n = "%s%s" % (j.rstrip('s'), k) + self.test_framework_policy[i][j][n] = \ + '''# Description: %s +# Usage: %s +''' % (n, k.lstrip('-')) + else: + self.test_framework_policy = policy_dict + self._update_test_framework_policy() + + def set_test_framework_policy_unknown(self, unknown=[]): + '''Set framework policy unknown''' + self.test_framework_policy_unknown = unknown + self._update_test_framework_policy_unknown() + def set_test_systemd(self, app, key, value): - '''Set systemd entries. If key is None, remove hook, if value is None, - remove key''' + '''Set systemd entries. If key is None, snappy-systemd from manifest + and yaml. + + Note the click manifest and the package.yaml use different + storage types. pkg_yaml['services'] is a list of dictionaries where + manifest['hooks'] is a dictionary of dictionaries. This function + sets the manifest entry and then a yaml entry with 'name' field. + + manifest['hooks'][app]['snappy-systemd'] = + pkg_yaml['services'][*]['name'] = app + pkg_yaml['services'][*][key] = value + ''' + + # Update the package.yaml + if key is None: + if 'services' in self.test_pkg_yaml: + for s in self.test_pkg_yaml['services']: + if 'name' in s and s['name'] == app: + self.test_pkg_yaml['services'].remove(s) + break + else: + found = False + if 'services' in self.test_pkg_yaml: + for s in self.test_pkg_yaml['services']: + if 'name' in s and s['name'] == app: + # Found the entry, so update key/value + s[key] = value + found = True + break + # Did not find the entry, so create one + if not found: + if 'services' not in self.test_pkg_yaml: + self.test_pkg_yaml['services'] = [] + self.test_pkg_yaml['services'].append({'name': app, + key: value}) + self._update_test_pkg_yaml() + + # Update the click manifest (we still support click format) if key is None: if app in self.test_systemd: self.test_systemd.pop(app) @@ -891,6 +1023,8 @@ del(self.test_systemd[app][key]) else: self.test_systemd[app][key] = value + + # Now update TEST_SNAPPY_SYSTEMD self._update_test_systemd() def setUp(self): @@ -935,6 +1069,10 @@ TEST_BIN_PATH = dict() global TEST_FRAMEWORK TEST_FRAMEWORK = dict() + global TEST_FRAMEWORK_POLICY + TEST_FRAMEWORK_POLICY = dict() + global TEST_FRAMEWORK_POLICY_UNKNOWN + TEST_FRAMEWORK_POLICY_UNKNOWN = [] global TEST_SNAPPY_SYSTEMD TEST_SNAPPY_SYSTEMD = dict() diff -Nru click-reviewers-tools-0.24/clickreviews/cr_url_dispatcher.py click-reviewers-tools-0.25/clickreviews/cr_url_dispatcher.py --- click-reviewers-tools-0.24/clickreviews/cr_url_dispatcher.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/cr_url_dispatcher.py 2015-04-13 14:18:56.000000000 +0000 @@ -1,6 +1,6 @@ '''cr_url dispatcher.py: click url_dispatcher''' # -# Copyright (C) 2014 Canonical Ltd. +# Copyright (C) 2014-2015 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -16,7 +16,7 @@ from __future__ import print_function -from clickreviews.cr_common import ClickReview, error, open_file_read, msg +from clickreviews.cr_common import ClickReview, error, open_file_read import json import os @@ -25,14 +25,15 @@ class ClickReviewUrlDispatcher(ClickReview): '''This class represents click lint reviews''' - def __init__(self, fn): + def __init__(self, fn, overrides=None): peer_hooks = dict() my_hook = 'urls' peer_hooks[my_hook] = dict() peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks peer_hooks[my_hook]['required'] = [] - ClickReview.__init__(self, fn, "url_dispatcher", peer_hooks=peer_hooks) + ClickReview.__init__(self, fn, "url_dispatcher", peer_hooks=peer_hooks, + overrides=overrides) self.required_keys = ['protocol'] self.optional_keys = ['domain-suffix'] diff -Nru click-reviewers-tools-0.24/clickreviews/tests/test_cr_bin_path.py click-reviewers-tools-0.25/clickreviews/tests/test_cr_bin_path.py --- click-reviewers-tools-0.24/clickreviews/tests/test_cr_bin_path.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/tests/test_cr_bin_path.py 2015-04-13 14:18:56.000000000 +0000 @@ -26,15 +26,21 @@ cr_tests.mock_patch() super() - def _set_binary(self, key, value, name=None): + def _set_binary(self, entries, name=None): d = dict() if name is None: - d['name'] = 'foo' + d['name'] = self.default_appname else: d['name'] = name - d[key] = value + for (key, value) in entries: + d[key] = value self.set_test_pkg_yaml("binaries", [d]) + if 'exec' in d: + self.set_test_bin_path(d['name'], d['exec']) + else: + self.set_test_bin_path(d['name'], d['name']) + def test_check_path(self): '''Test check_path()''' self.set_test_bin_path(self.default_appname, "bin/foo.exe") @@ -51,12 +57,12 @@ c = ClickReviewBinPath(self.test_name) c.check_path() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': 0, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) def test_check_peer_hooks(self): '''Test check_peer_hooks()''' + self.set_test_bin_path(self.default_appname, self.default_appname) c = ClickReviewBinPath(self.test_name) # create a new hooks database for our peer hooks tests @@ -80,6 +86,7 @@ def test_check_peer_hooks_disallowed(self): '''Test check_peer_hooks() - disallowed''' + self.set_test_bin_path(self.default_appname, self.default_appname) c = ClickReviewBinPath(self.test_name) # create a new hooks database for our peer hooks tests @@ -106,6 +113,7 @@ def test_check_peer_hooks_required(self): '''Test check_peer_hooks() - required''' + self.set_test_bin_path(self.default_appname, self.default_appname) c = ClickReviewBinPath(self.test_name) # create a new hooks database for our peer hooks tests @@ -125,9 +133,96 @@ expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + def test_check_required(self): + '''Test check_snappy_required()''' + self._set_binary([("exec", "bin/foo")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_required_empty_value(self): + '''Test check_snappy_required() - empty exec''' + self._set_binary([("exec", "")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_required_bad_value(self): + '''Test check_snappy_required() - bad exec''' + self._set_binary([("exec", [])]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_required_multiple(self): + '''Test check_snappy_required() - multiple''' + self._set_binary([("exec", "bin/foo"), + ("description", "foo desc")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_required() + r = c.click_report + # Only 'name' is required at this time so 0s for all + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_snappy_optional(self): + '''Test check_snappy_optional()''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_optional() + r = c.click_report + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_snappy_optional_empty_value(self): + '''Test check_snappy_optional() - empty description''' + self._set_binary([("description", "")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_optional() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_snappy_optional_bad_value(self): + '''Test check_snappy_optional() - bad description''' + self._set_binary([("description", [])]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_optional() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_snappy_unknown(self): + '''Test check_snappy_unknown()''' + self._set_binary([("nonexistent", "foo")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_unknown() + r = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_snappy_unknown_multiple(self): + '''Test check_snappy_unknown() - multiple''' + self._set_binary([("exec", "bin/foo"), + ("nonexistent", "foo")]) + c = ClickReviewBinPath(self.test_name) + c.check_snappy_unknown() + r = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(r, expected_counts) + def test_check_binary_description(self): '''Test check_binary_description()''' - self._set_binary("description", "some description") + self._set_binary([("description", "some description")]) c = ClickReviewBinPath(self.test_name) c.check_binary_description() r = c.click_report @@ -136,7 +231,7 @@ def test_check_binary_description_unspecified(self): '''Test check_binary_description() - unspecified''' - self._set_binary("name", "foo") + self._set_binary([("exec", "foo")]) c = ClickReviewBinPath(self.test_name) c.check_binary_description() r = c.click_report @@ -145,9 +240,40 @@ def test_check_binary_description_empty(self): '''Test check_binary_description() - empty''' - self._set_binary("description", "") + self._set_binary([("description", "")]) c = ClickReviewBinPath(self.test_name) c.check_binary_description() r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_click_hooks(self): + '''Test check_click_hooks()''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + c.check_click_hooks() + r = c.click_report + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_click_hooks_missing_hook(self): + '''Test check_click_hooks() - missing''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + del c.manifest['hooks'][self.default_appname]['bin-path'] + c.check_click_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_click_hooks_added_hook(self): + '''Test check_click_hooks() - added''' + self._set_binary([("description", "some description")]) + c = ClickReviewBinPath(self.test_name) + tmp = c.manifest['hooks'] + tmp['extra-app'] = c.manifest['hooks'][self.default_appname] + c.manifest['hooks'] = tmp + c.check_click_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.24/clickreviews/tests/test_cr_desktop.py click-reviewers-tools-0.25/clickreviews/tests/test_cr_desktop.py --- click-reviewers-tools-0.24/clickreviews/tests/test_cr_desktop.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/tests/test_cr_desktop.py 2015-04-13 14:18:56.000000000 +0000 @@ -726,6 +726,42 @@ expected_counts = {'info': None, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) + def test_check_desktop_exec_webbrowser_no_homepage(self): + '''Test check_desktop_exec_webbrowser_no_homepage() not local app''' + c = ClickReviewDesktop(self.test_name) + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "name", "foo") + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "includes", + ['https?://mobile.twitter.net/*']) + ex = "webapp-container --webapp='Zm9v' " + \ + "--enable-back-forward --webappModelSearchPath=." + self.set_test_desktop(self.default_appname, + "Exec", + ex) + c.check_desktop_exec_webapp_args() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_desktop_exec_webbrowser_field_code(self): + '''Test check_desktop_exec_webbrowser_field_code() with field code''' + c = ClickReviewDesktop(self.test_name) + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "name", "foo") + self.set_test_webapp_manifest("unity-webapps-foo/manifest.json", + "includes", + ['https?://mobile.twitter.net/*']) + ex = "webapp-container --webapp='Zm9v' " + \ + "--enable-back-forward --webappModelSearchPath=. %u" + self.set_test_desktop(self.default_appname, + "Exec", + ex) + c.check_desktop_exec_webapp_args() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + def test_check_desktop_exec_webbrowser_local_pattern(self): '''Test test_check_desktop_exec_webbrowser_local_pattern() invalid pattern''' c = ClickReviewDesktop(self.test_name) @@ -751,7 +787,7 @@ ex) c.check_desktop_exec_webapp_args() r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 1, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) def test_check_desktop_exec_webbrowser_local_model(self): diff -Nru click-reviewers-tools-0.24/clickreviews/tests/test_cr_framework.py click-reviewers-tools-0.25/clickreviews/tests/test_cr_framework.py --- click-reviewers-tools-0.24/clickreviews/tests/test_cr_framework.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/tests/test_cr_framework.py 2015-04-13 14:18:56.000000000 +0000 @@ -26,184 +26,178 @@ cr_tests.mock_patch() super() - def test_single_framework(self): - '''Test check_single_framework()''' + def test_framework_hook_obsolete(self): + '''Test check_framework_hook_obsolete()''' self.set_test_framework(self.default_appname, "", "") c = ClickReviewFramework(self.test_name) - c.check_single_framework() + c.check_framework_hook_obsolete() + r = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_snappy_framework_file_obsolete(self): + '''Test check_snappy_framework_file_obsolete()''' + self.set_test_pkg_yaml("type", "framework") + c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_file_obsolete() + r = c.click_report + expected_counts = {'info': 0, 'warn': 1, 'error': 0} + self.check_results(r, expected_counts) + + def test_snappy_framework_depends(self): + '''Test check_snappy_framework_depends()''' + self.set_test_pkg_yaml("type", "framework") + c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_depends() r = c.click_report - # We should end up with 1 info expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_single_framework_multiple(self): - '''Test check_single_framework() - multiple''' - self.set_test_framework(self.default_appname, "", "") - self.set_test_framework("test-alt", "", "") + def test_snappy_framework_depends_bad(self): + '''Test check_snappy_framework_depends() - bad''' + self.set_test_pkg_yaml("type", "framework") + self.set_test_pkg_yaml("frameworks", ['foo']) c = ClickReviewFramework(self.test_name) - c.check_single_framework() + c.check_snappy_framework_depends() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 0, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_framework_base_name(self): - '''Test check_framework_base_name()''' - self.set_test_framework(self.default_appname, "Base-Name", - self.test_manifest["name"]) + def test_snappy_framework_policy(self): + '''Test check_snappy_framework_policy()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) - c.check_framework_base_name() + c.check_snappy_framework_policy() r = c.click_report - # We should end up with 2 info expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_name_missing(self): - '''Test check_framework_base_name() - missing''' - self.set_test_framework(self.default_appname, "Base-Version", "") + def test_snappy_framework_policy_missing(self): + '''Test check_snappy_framework_policy() - missing''' + self.set_test_pkg_yaml("type", "framework") + self.set_test_framework_policy({}) c = ClickReviewFramework(self.test_name) - c.check_framework_base_name() + c.check_snappy_framework_policy() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_name_empty(self): - '''Test check_framework_base_name() - empty''' - self.set_test_framework(self.default_appname, "Base-Name", "") + def test_snappy_framework_policy_unknown(self): + '''Test check_snappy_framework_policy() - unknown''' + self.set_test_pkg_yaml("type", "framework") + self.set_test_framework_policy_unknown(['foo/bar/baz']) c = ClickReviewFramework(self.test_name) - c.check_framework_base_name() + c.check_snappy_framework_policy() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_version(self): - '''Test check_framework_base_version()''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_metadata(self): + '''Test check_snappy_framework_policy_metadata()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_metadata() r = c.click_report - # We should end up with 3 info - expected_counts = {'info': 3, 'warn': 0, 'error': 0} + expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_version_missing(self): - '''Test check_framework_base_version() - missing''' - self.set_test_framework(self.default_appname, "Base-Name", "") + def test_snappy_framework_policy_metadata_template(self): + '''Test check_snappy_framework_policy_metadata() - template missing + data + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + tmp['apparmor']['templates']['template-common'] = 'missing data' + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_metadata() r = c.click_report - # We should end up with 1 info expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_framework_base_version_empty(self): - '''Test check_framework_base_version() - empty''' - self.set_test_framework(self.default_appname, "Base-Version", "") + def test_snappy_framework_policy_metadata_policygroup(self): + '''Test check_snappy_framework_policy_metadata() - policygroup missing + data + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + tmp['seccomp']['policygroups']['policygroup-reserved'] = 'missing data' + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_metadata() r = c.click_report - # We should end up with 1 info expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_framework_base_version_str(self): - '''Test check_framework_base_version() - str''' - self.set_test_framework(self.default_appname, "Base-Version", "abc") + def test_snappy_framework_policy_matching(self): + '''Test check_snappy_framework_policy_matching()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_matching() r = c.click_report - # We should end up with 1 info - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 8, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_framework_base_version_negative(self): - '''Test check_framework_base_version() - negative''' - self.set_test_framework(self.default_appname, "Base-Version", -1) + def test_snappy_framework_policy_matching_missing_aa_template(self): + '''Test check_snappy_framework_policy_matching() - missing aa template + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + del tmp['apparmor']['templates']['template-common'] + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - c.check_framework_base_version() + c.check_snappy_framework_policy_matching() r = c.click_report - # We should end up with 1 info expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_check_peer_hooks(self): - '''Test check_peer_hooks()''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_matching_missing_sc_policygroup(self): + '''Test check_snappy_framework_policy_matching() - missing sc policy + group + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + del tmp['seccomp']['policygroups']['policygroup-reserved'] + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - - # create a new hooks database for our peer hooks tests - tmp = dict() - - # add our hook - tmp["framework"] = "foo.framework" - - # add any required peer hooks - # FIXME: - # tmp["apparmor-policy"] = "apparmor/" - - # update the manifest and test_manifest - c.manifest["hooks"][self.default_appname] = tmp - self._update_test_manifest() - - # do the test - c.check_peer_hooks() + c.check_snappy_framework_policy_matching() r = c.click_report - # We should end up with 2 info - expected_counts = {'info': 2, 'warn': 0, 'error': 0} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_check_peer_hooks_disallowed(self): - '''Test check_peer_hooks() - disallowed''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_filenames(self): + '''Test check_snappy_framework_policy_filenames()''' + self.set_test_pkg_yaml("type", "framework") c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_policy_filenames() + r = c.click_report + expected_counts = {'info': 8, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) - # create a new hooks database for our peer hooks tests - tmp = dict() - - # add our hook - tmp["framework"] = "foo.framework" - - # add any required peer hooks - # FIXME: - # tmp["apparmor-policy"] = "apparmor/" - - # add disallowed framework - tmp["nonexistent"] = "nonexistent-hook" - - # update the manifest and test_manifest - c.manifest["hooks"][self.default_appname] = tmp - self._update_test_manifest() - - # do the test - c.check_peer_hooks() + def test_snappy_framework_policy_filenames_bad(self): + '''Test check_snappy_framework_policy_filenames() - bad name''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + tmp['seccomp']['policygroups']['policygroup-res_erved'] = "foo" + self.set_test_framework_policy(tmp) + c = ClickReviewFramework(self.test_name) + c.check_snappy_framework_policy_filenames() r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - def test_check_peer_hooks_required(self): - '''Test check_peer_hooks() - required''' - self.set_test_framework(self.default_appname, "Base-Version", 0.1) + def test_snappy_framework_policy_filenames_bad2(self): + '''Test check_snappy_framework_policy_filenames() - starts with + package name + ''' + self.set_test_pkg_yaml("type", "framework") + tmp = self.test_framework_policy + n = self.test_name.split('_')[0] + tmp['seccomp']['policygroups']['%s-group' % n] = "foo" + self.set_test_framework_policy(tmp) c = ClickReviewFramework(self.test_name) - - # create a new hooks database for our peer hooks tests - tmp = dict() - - # add our hook - tmp["framework"] = "foo.framework" - - # skip adding required hooks - - # update the manifest and test_manifest - c.manifest["hooks"][self.default_appname] = tmp - self._update_test_manifest() - - # do the test - c.check_peer_hooks() + c.check_snappy_framework_policy_filenames() r = c.click_report - # FIXME: apparmor-policy is not defined yet, so no error, when it is - # adjust to 'error': 1 - expected_counts = {'info': None, 'warn': 0, 'error': 0} + expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.24/clickreviews/tests/test_cr_lint.py click-reviewers-tools-0.25/clickreviews/tests/test_cr_lint.py --- click-reviewers-tools-0.24/clickreviews/tests/test_cr_lint.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/tests/test_cr_lint.py 2015-04-13 14:18:56.000000000 +0000 @@ -801,6 +801,17 @@ expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + def test_check_hooks_type_oem(self): + '''Test check_hooks() - type: oem''' + self.set_test_manifest("framework", "ubuntu-sdk-13.10") + c = ClickReviewLint(self.test_name) + c.is_snap_oem = True + c.check_hooks() + r = c.click_report + # oem type has no hooks so these should all be '0' + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + def test_check_hooks_unknown_nonexistent(self): '''Test check_hooks_unknown() - nonexistent''' self.set_test_manifest("framework", "ubuntu-sdk-13.10") @@ -820,6 +831,17 @@ expected_counts = {'info': None, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) + def test_check_hooks_unknown_type_oem(self): + '''Test check_hooks_unknown() - type: oem''' + self.set_test_manifest("framework", "ubuntu-sdk-13.10") + c = ClickReviewLint(self.test_name) + c.is_snap_oem = True + c.check_hooks_unknown() + r = c.click_report + # oem type has no hooks so these should all be '0' + expected_counts = {'info': 0, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + def test_check_hooks_redflagged_payui(self): '''Test check_hooks_redflagged() - pay-ui''' self.set_test_manifest("framework", "ubuntu-sdk-13.10") @@ -1013,24 +1035,13 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_snappy_type_framework_policy(self): - '''Test check_snappy_type - framework-policy''' - self.set_test_pkg_yaml("type", "framework-policy") - c = ClickReviewLint(self.test_name) - c.check_snappy_type() - r = c.click_report - # TODO: update this when 'framework-policy' is implemented - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - def test_snappy_type_oem(self): '''Test check_snappy_type - oem''' self.set_test_pkg_yaml("type", "oem") c = ClickReviewLint(self.test_name) c.check_snappy_type() r = c.click_report - # TODO: update this when 'oem' is implemented - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) def test_snappy_type_redflagged(self): @@ -1114,7 +1125,7 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_icon_empty(self): + def test_check_snappy_icon_empty(self): '''Test check_snappy_icon() - empty''' self.set_test_pkg_yaml("icon", "") c = ClickReviewLint(self.test_name) diff -Nru click-reviewers-tools-0.24/clickreviews/tests/test_cr_systemd.py click-reviewers-tools-0.25/clickreviews/tests/test_cr_systemd.py --- click-reviewers-tools-0.24/clickreviews/tests/test_cr_systemd.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/tests/test_cr_systemd.py 2015-04-13 14:18:56.000000000 +0000 @@ -26,14 +26,16 @@ cr_tests.mock_patch() super() - def _set_service(self, key, value, name=None): + def _set_service(self, entries, name=None): d = dict() if name is None: - d['name'] = 'foo' + d['name'] = self.default_appname else: d['name'] = name - d[key] = value + for (key, value) in entries: + d[key] = value self.set_test_pkg_yaml("services", [d]) + self.set_test_systemd(d['name'], 'name', d['name']) def test_check_required(self): '''Test check_required() - has start and description''' @@ -85,13 +87,16 @@ self.set_test_systemd(self.default_appname, key="description", value="something") + self.set_test_systemd(self.default_appname, + key="stop", + value="/bin/foo-stop") c = ClickReviewSystemd(self.test_name) c.check_required() r = c.click_report - expected_counts = {'info': -1, 'warn': 0, 'error': 0} + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_required_multiple(self): + def test_check_required_multiple2(self): '''Test check_required() - multiple with nonexistent''' self.set_test_systemd(self.default_appname, key="start", @@ -579,7 +584,7 @@ def test_check_snappy_service_description(self): '''Test check_snappy_service_description()''' - self._set_service("description", "some description") + self._set_service([("description", "some description")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_description() @@ -589,7 +594,7 @@ def test_check_snappy_service_description_unspecified(self): '''Test check_snappy_service_description() - unspecified''' - # self._set_service("description", None) + # self._set_service([("description", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_description() @@ -600,7 +605,7 @@ def test_check_snappy_service_description_empty(self): '''Test check_snappy_service_description() - empty''' - self._set_service("description", "") + self._set_service([("description", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_description() @@ -610,7 +615,7 @@ def test_check_snappy_service_start(self): '''Test check_snappy_service_start()''' - self._set_service("start", "some/start") + self._set_service([("start", "some/start")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -620,7 +625,7 @@ def test_check_snappy_service_start_unspecified(self): '''Test check_snappy_service_start() - unspecified''' - # self._set_service("start", None) + # self._set_service([("start", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -630,7 +635,7 @@ def test_check_snappy_service_start_empty(self): '''Test check_snappy_service_start() - empty''' - self._set_service("start", "") + self._set_service([("start", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -640,7 +645,7 @@ def test_check_snappy_service_start_absolute_path(self): '''Test check_snappy_service_start() - absolute path''' - self._set_service("start", "/foo/bar/some/start") + self._set_service([("start", "/foo/bar/some/start")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_start() @@ -650,7 +655,7 @@ def test_check_snappy_service_stop(self): '''Test check_snappy_service_stop()''' - self._set_service("stop", "some/stop") + self._set_service([("stop", "some/stop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -660,7 +665,7 @@ def test_check_snappy_service_stop_unspecified(self): '''Test check_snappy_service_stop() - unspecified''' - # self._set_service("stop", None) + # self._set_service([("stop", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -670,7 +675,7 @@ def test_check_snappy_service_stop_empty(self): '''Test check_snappy_service_stop() - empty''' - self._set_service("stop", "") + self._set_service([("stop", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -680,7 +685,7 @@ def test_check_snappy_service_stop_absolute_path(self): '''Test check_snappy_service_stop() - absolute path''' - self._set_service("stop", "/foo/bar/some/stop") + self._set_service([("stop", "/foo/bar/some/stop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop() @@ -690,7 +695,7 @@ def test_check_snappy_service_poststop(self): '''Test check_snappy_service_poststop()''' - self._set_service("poststop", "some/poststop") + self._set_service([("poststop", "some/poststop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -700,7 +705,7 @@ def test_check_snappy_service_poststop_unspecified(self): '''Test check_snappy_service_poststop() - unspecified''' - # self._set_service("poststop", None) + # self._set_service([("poststop", None)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -710,7 +715,7 @@ def test_check_snappy_service_poststop_empty(self): '''Test check_snappy_service_poststop() - empty''' - self._set_service("poststop", "") + self._set_service([("poststop", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -720,7 +725,7 @@ def test_check_snappy_service_poststop_absolute_path(self): '''Test check_snappy_service_poststop() - absolute path''' - self._set_service("poststop", "/foo/bar/some/poststop") + self._set_service([("poststop", "/foo/bar/some/poststop")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_poststop() @@ -730,9 +735,9 @@ def test_check_snappy_service_stop_timeout(self): '''Test check_snappy_service_stop_timeout()''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value=30) + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", 30)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -742,9 +747,9 @@ def test_check_snappy_service_stop_timeout_empty(self): '''Test check_snappy_service_stop_timeout() - empty''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value="") + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", "")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -754,9 +759,9 @@ def test_check_snappy_service_stop_timeout_bad(self): '''Test check_snappy_service_stop_timeout() - bad''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value="a") + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", "a")]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -766,9 +771,9 @@ def test_check_snappy_service_stop_timeout_range_low(self): '''Test check_snappy_service_stop_timeout() - out of range (low)''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value=-1) + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", -1)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() @@ -778,9 +783,9 @@ def test_check_snappy_service_stop_timeout_range_high(self): '''Test check_snappy_service_stop_timeout() - out of range (high)''' - self._set_service(key="start", value="bin/foo") - self._set_service(key="description", value="something") - self._set_service(key="stop-timeout", value=61) + self._set_service([("start", "bin/foo"), + ("description", "something"), + ("stop-timeout", 61)]) c = ClickReviewSystemd(self.test_name) c.systemd_files['foo'] = "meta/foo.snappy-systemd" c.check_snappy_service_stop_timeout() diff -Nru click-reviewers-tools-0.24/clickreviews/tests/test_cr_url_dispatcher.py click-reviewers-tools-0.25/clickreviews/tests/test_cr_url_dispatcher.py --- click-reviewers-tools-0.24/clickreviews/tests/test_cr_url_dispatcher.py 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/clickreviews/tests/test_cr_url_dispatcher.py 2015-04-13 14:18:56.000000000 +0000 @@ -74,7 +74,7 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_required_multiple(self): + def test_check_required_multiple_with_nonexiting(self): '''Test check_required() - multiple with nonexistent''' self.set_test_url_dispatcher(self.default_appname, key="protocol", diff -Nru click-reviewers-tools-0.24/data/apparmor-easyprof-ubuntu.json click-reviewers-tools-0.25/data/apparmor-easyprof-ubuntu.json --- click-reviewers-tools-0.24/data/apparmor-easyprof-ubuntu.json 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/data/apparmor-easyprof-ubuntu.json 2015-04-13 14:18:56.000000000 +0000 @@ -175,8 +175,25 @@ "1.3": { "templates": { "common": [ - "default", - "service" + "default" + ], + "reserved": [ + "unconfined" + ] + }, + "policy_groups": { + "common": [ + "networking" + ], + "reserved": [] + } + } + }, + "ubuntu-core": { + "15.04": { + "templates": { + "common": [ + "default" ], "reserved": [ "unconfined" diff -Nru click-reviewers-tools-0.24/debian/bzr-builder.manifest click-reviewers-tools-0.25/debian/bzr-builder.manifest --- click-reviewers-tools-0.24/debian/bzr-builder.manifest 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/debian/bzr-builder.manifest 2015-04-13 14:18:56.000000000 +0000 @@ -1,2 +1,2 @@ -# bzr-builder format 0.3 deb-version {debupstream}-0~407 -lp:click-reviewers-tools revid:jamie@ubuntu.com-20150309212251-98sbnjvyci0u90md +# bzr-builder format 0.3 deb-version {debupstream}-0~436 +lp:click-reviewers-tools revid:daniel.holbach@canonical.com-20150413141015-4ozvo4xsl1aiet5m diff -Nru click-reviewers-tools-0.24/debian/changelog click-reviewers-tools-0.25/debian/changelog --- click-reviewers-tools-0.24/debian/changelog 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/debian/changelog 2015-04-13 14:18:56.000000000 +0000 @@ -1,14 +1,53 @@ -click-reviewers-tools (0.24-0~407~ubuntu14.04.1) trusty; urgency=low +click-reviewers-tools (0.25-0~436~ubuntu14.04.1) trusty; urgency=low * Auto build. - -- Daniel Holbach Tue, 10 Mar 2015 15:16:29 +0000 + -- Daniel Holbach Mon, 13 Apr 2015 14:18:56 +0000 -click-reviewers-tools (0.24) UNRELEASED; urgency=medium +click-reviewers-tools (0.25) UNRELEASED; urgency=medium - * + [ Michael Vogt ] + * Fixed a number of issues raised by pyflakes. - -- Jamie Strandboge Mon, 09 Mar 2015 16:22:37 -0500 + [ Ricardo Kirkner ] + * support overrides in all click-check scripts + * refactored click checks to avoid duplication + * handle checks from branch as well as installed system-wide when running + all checks + + [ Jamie Strandboge ] + * update bin-path tests for new binaries yaml + * 'oem' is a valid type + * handle missing 'hooks' in manifest with oem snaps (LP: #1434279) + * cr_common.py: add config, immutable-config and oem in support of oem snaps + * obsolete framework click hook and meta/*.framework + * don't allow 'type: framework' to specify 'frameworks' + * fix click-show-files with native snaps + * click-show-files should show package.yaml + * add framework policy checks + * update systemd tests to check package.yaml + * .strip() whitespace in control_description_match + * check_package_filename() store downloads packages with _all instead of + _multi. Account for that. We may want to remove this check entirely. + + [ Fabian Ezequiel Gallina ] + * fix missing import on clickreviews/cr_framework.py + * add test for non-string framework + + [ Alex Abreu ] + * fix webapp exec with no homepage url or with exec field code (LP: + #1441185) + + -- Jamie Strandboge Fri, 10 Apr 2015 11:20:59 -0500 + +click-reviewers-tools (0.24) vivid; urgency=medium + + * don't fail if DEBIAN/md5sums doesn't exist with snap packages. The snap + package format uses a different method for integrity checking + * add bin/click-check-systemd + * adjust bin/click-run-checks to call click-check-systemd + + -- Jamie Strandboge Wed, 18 Mar 2015 14:27:51 -0500 click-reviewers-tools (0.23) vivid; urgency=medium diff -Nru click-reviewers-tools-0.24/debian/control click-reviewers-tools-0.25/debian/control --- click-reviewers-tools-0.24/debian/control 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/debian/control 2015-04-13 14:18:56.000000000 +0000 @@ -4,6 +4,7 @@ Maintainer: Ubuntu Appstore Developers Build-Depends: debhelper (>= 9~), pep8, + pyflakes, python3-all (>= 3.2~), python3-apt, python3-debian, diff -Nru click-reviewers-tools-0.24/debian/rules click-reviewers-tools-0.25/debian/rules --- click-reviewers-tools-0.24/debian/rules 2015-03-10 15:16:29.000000000 +0000 +++ click-reviewers-tools-0.25/debian/rules 2015-04-13 14:18:56.000000000 +0000 @@ -43,4 +43,5 @@ $$python setup.py test; \ done ./run-pep8 + ./run-pyflakes endif diff -Nru click-reviewers-tools-0.24/run-pyflakes click-reviewers-tools-0.25/run-pyflakes --- click-reviewers-tools-0.24/run-pyflakes 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.25/run-pyflakes 2015-04-13 14:18:56.000000000 +0000 @@ -0,0 +1,9 @@ +#!/bin/sh +set -e + +echo "= pyflakes3 =" +for i in ./bin/update-* ./bin/click-check-* ./bin/click-show-files ./bin/click-review \ + ./clickreviews/*py ./clickreviews/tests/*py ; do + echo "Checking $i" + pyflakes3 $i +done