Merge lp:~ricardokirkner/click-reviewers-tools/override-frameworks into lp:click-reviewers-tools

[Ricardo Kirkner]

allow specifying overrides for framework checks

- support overriding frameworks for lint check
- support overriding framework policy data for security checks

[Jamie Strandboge (jdstrand)]

Looks great, thanks!

review: Approve

=== modified file 'bin/click-check-lint'
--- bin/click-check-lint	2013-09-26 12:24:10 +0000
+++ bin/click-check-lint	2014-11-17 17:14:19 +0000
@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import print_function
+import json
 import sys
 
 import clickreviews.cr_common as cr_common
@@ -25,7 +26,14 @@
     if len(sys.argv) < 2:
         cr_common.error("Must give path to click package")
 
-    review = cr_lint.ClickReviewLint(sys.argv[1])
+    # 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)

=== modified file 'bin/click-check-security'
--- bin/click-check-security	2013-09-26 12:24:10 +0000
+++ bin/click-check-security	2014-11-17 17:14:19 +0000
@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import print_function
+import json
 import sys
 
 import clickreviews.cr_common as cr_common
@@ -25,7 +26,14 @@
     if len(sys.argv) < 2:
         cr_common.error("Must give path to click package")
 
-    review = cr_security.ClickReviewSecurity(sys.argv[1])
+    # 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)

=== modified file 'clickreviews/cr_lint.py'
--- clickreviews/cr_lint.py	2014-11-11 15:19:59 +0000
+++ clickreviews/cr_lint.py	2014-11-17 17:14:19 +0000
@@ -31,7 +31,7 @@
 class ClickReviewLint(ClickReview):
     '''This class represents click lint reviews'''
 
-    def __init__(self, fn):
+    def __init__(self, fn, overrides=None):
         '''Set up the class.'''
         ClickReview.__init__(self, fn, "lint")
         self.control_files = dict()
@@ -87,6 +87,7 @@
                             'urls']
 
         self.redflagged_hooks = ['pay-ui']
+        self.overrides = overrides if overrides is not None else {}
 
     def _list_control_files(self):
         '''List all control files with their full path.'''
@@ -641,7 +642,8 @@
         '''Check framework()'''
         n = 'framework'
         l = "http://askubuntu.com/questions/460512/what-framework-should-i-use-in-my-manifest-file"
-        frameworks = Frameworks()
+        framework_overrides = self.overrides.get('framework')
+        frameworks = Frameworks(overrides=framework_overrides)
         if self.manifest['framework'] in frameworks.AVAILABLE_FRAMEWORKS:
             t = 'info'
             s = 'OK'

=== modified file 'clickreviews/cr_security.py'
--- clickreviews/cr_security.py	2014-10-14 13:14:05 +0000
+++ clickreviews/cr_security.py	2014-11-17 17:14:19 +0000
@@ -25,7 +25,7 @@
 
 class ClickReviewSecurity(ClickReview):
     '''This class represents click lint reviews'''
-    def __init__(self, fn):
+    def __init__(self, fn, overrides=None):
         ClickReview.__init__(self, fn, "security")
 
         local_copy = os.path.join(os.path.dirname(__file__),
@@ -78,10 +78,19 @@
         # framework policy is based on major framework version. In 13.10, there
         # was only 'ubuntu-sdk-13.10', but in 14.04, there will be several,
         # like 'ubuntu-sdk-14.04-html5', 'ubuntu-sdk-14.04-platform', etc
-        self.major_framework_policy = {'ubuntu-sdk-13.10': 1.0,
-                                       'ubuntu-sdk-14.04': 1.1,
-                                       'ubuntu-sdk-14.10': 1.2,
-                                       }
+        self.major_framework_policy = {
+            'ubuntu-sdk-13.10': {
+                'policy_version': 1.0,
+            },
+            'ubuntu-sdk-14.04': {
+                'policy_version': 1.1,
+            },
+            'ubuntu-sdk-14.10': {
+                'policy_version': 1.2,
+            },
+        }
+        framework_overrides = overrides.get('framework') if overrides else {}
+        self._override_framework_policies(framework_overrides)
 
         self.security_manifests = dict()
         self.security_apps = []
@@ -102,6 +111,23 @@
                 self._extract_security_manifest(app)
             self.security_apps.append(app)
 
+    def _override_framework_policies(self, overrides):
+        # override major framework policies
+        self.major_framework_policy.update(overrides)
+
+        # override apparmor policies
+        for name, data in overrides.items():
+            vendor = data.get('policy_vendor')
+            version = str(data.get('policy_version'))
+
+            if vendor not in self.aa_policy:
+                self.aa_policy[vendor] = {}
+
+            if version not in self.aa_policy[vendor]:
+                # just ensure the version is defined
+                # TODO: add support to override templates and policy groups
+                self.aa_policy[vendor][version] = {}
+
     def _extract_security_manifest(self, app):
         '''Extract security manifest and verify it has the expected
            structure'''
@@ -269,15 +295,15 @@
             n = 'policy_version_matches_framework (%s)' % (f)
             s = "OK"
             found_major = False
-            for k in self.major_framework_policy.keys():
+            for name, data in self.major_framework_policy.items():
                 # TODO: use libclick when it is available
-                if not self.manifest['framework'].startswith(k):
+                if not self.manifest['framework'].startswith(name):
                     continue
                 found_major = True
-                if m['policy_version'] != self.major_framework_policy[k]:
+                if m['policy_version'] != data['policy_version']:
                     t = 'error'
                     s = '%s != %s (%s)' % (str(m['policy_version']),
-                                           self.major_framework_policy[k],
+                                           data['policy_version'],
                                            self.manifest['framework'])
             if not found_major:
                 t = 'error'

=== modified file 'clickreviews/frameworks.py'
--- clickreviews/frameworks.py	2014-11-11 16:12:57 +0000
+++ clickreviews/frameworks.py	2014-11-17 17:14:19 +0000
@@ -32,14 +32,21 @@
     OBSOLETE_FRAMEWORKS = []
     AVAILABLE_FRAMEWORKS = []
 
-    def __init__(self):
+    def __init__(self, overrides=None):
         self.FRAMEWORKS = clickreviews.remote.read_cr_file(USER_DATA_FILE,
                                                            FRAMEWORKS_DATA_URL)
-
-        for k, v in self.FRAMEWORKS.items():
-            if v == 'deprecated':
-                self.DEPRECATED_FRAMEWORKS.append(k)
-            elif v == 'obsolete':
-                self.OBSOLETE_FRAMEWORKS.append(k)
-            elif v == 'available':
-                self.AVAILABLE_FRAMEWORKS.append(k)
+        if overrides is not None:
+            self.FRAMEWORKS.update(overrides)
+
+        for name, data in self.FRAMEWORKS.items():
+            if type(data) is dict:
+                state = data.get('state')
+            else:
+                state = data
+
+            if state == 'deprecated':
+                self.DEPRECATED_FRAMEWORKS.append(name)
+            elif state == 'obsolete':
+                self.OBSOLETE_FRAMEWORKS.append(name)
+            elif state == 'available':
+                self.AVAILABLE_FRAMEWORKS.append(name)

=== modified file 'clickreviews/tests/test_cr_lint.py'
--- clickreviews/tests/test_cr_lint.py	2014-11-11 16:12:57 +0000
+++ clickreviews/tests/test_cr_lint.py	2014-11-17 17:14:19 +0000
@@ -31,7 +31,7 @@
         super()
 
     def patch_frameworks(self):
-        def _mock_frameworks(self):
+        def _mock_frameworks(self, overrides=None):
             self.FRAMEWORKS = {
                 'ubuntu-sdk-14.10-qml-dev2': 'available',
                 'ubuntu-sdk-13.10': 'deprecated',
@@ -752,6 +752,20 @@
         expected_counts = {'info': None, 'warn': 0, 'error': 1}
         self.check_results(r, expected_counts)
 
+    @patch('clickreviews.remote.read_cr_file')
+    def test_check_framework_with_overrides(self, mock_read_cr_file):
+        '''Test check_framework() - using overrides'''
+        mock_read_cr_file.return_value = {
+            'ubuntu-sdk-14.10-qml-dev2': 'available',
+        }
+        self.set_test_manifest("framework", "nonexistent")
+        overrides = {'framework': {'nonexistent': {'state': 'available'}}}
+        c = ClickReviewLint(self.test_name, overrides=overrides)
+        c.check_framework()
+        r = c.click_report
+        expected_counts = {'info': 1, 'warn': 0, 'error': 0}
+        self.check_results(r, expected_counts)
+
     def test_check_hooks(self):
         '''Test check_hooks()'''
         self.set_test_manifest("framework", "ubuntu-sdk-13.10")

=== modified file 'clickreviews/tests/test_cr_security.py'
--- clickreviews/tests/test_cr_security.py	2014-10-08 20:08:44 +0000
+++ clickreviews/tests/test_cr_security.py	2014-11-17 17:14:19 +0000
@@ -136,7 +136,7 @@
             policy_version = 0
             for k in tmp.major_framework_policy.keys():
                 if f.startswith(k):
-                    policy_version = tmp.major_framework_policy[k]
+                    policy_version = tmp.major_framework_policy[k]['policy_version']
             self.set_test_security_manifest(self.default_appname,
                                             "policy_version",
                                             policy_version)
@@ -209,6 +209,21 @@
             {"text": "Invalid framework 'nonexistent'"}
         self.check_results(report, expected=expected)
 
+    def test_check_policy_version_framework_with_overrides(self):
+        '''Test check_policy_version() - override framework (nonexistent)'''
+        self.set_test_manifest("framework", "nonexistent")
+        self.set_test_security_manifest(self.default_appname,
+                                        "policy_version", 1.3)
+        overrides = {'framework': {'nonexistent': {'state': 'available',
+                                                   'policy_vendor': 'ubuntu',
+                                                   'policy_version': 1.3}}}
+        c = ClickReviewSecurity(self.test_name, overrides=overrides)
+        c.check_policy_version()
+        report = c.click_report
+
+        expected_counts = {'info': 3, 'warn': 0, 'error': 0}
+        self.check_results(report, expected_counts)
+
     def test_check_policy_vendor_unspecified(self):
         '''Test check_policy_vendor() - unspecified'''
         c = ClickReviewSecurity(self.test_name)