Merge lp:~ricardokirkner/click-reviewers-tools/override-frameworks into lp:click-reviewers-tools
- override-frameworks
- Merge into trunk
Proposed by
Ricardo Kirkner
Status: | Merged |
---|---|
Merged at revision: | 292 |
Proposed branch: | lp:~ricardokirkner/click-reviewers-tools/override-frameworks |
Merge into: | lp:click-reviewers-tools |
Diff against target: |
271 lines (+104/-24) 7 files modified
bin/click-check-lint (+9/-1) bin/click-check-security (+9/-1) clickreviews/cr_lint.py (+4/-2) clickreviews/cr_security.py (+35/-9) clickreviews/frameworks.py (+16/-9) clickreviews/tests/test_cr_lint.py (+15/-1) clickreviews/tests/test_cr_security.py (+16/-1) |
To merge this branch: | bzr merge lp:~ricardokirkner/click-reviewers-tools/override-frameworks |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jamie Strandboge (community) | Approve | ||
Review via email: mp+241998@code.launchpad.net |
Commit message
allow specifying overrides for framework checks
- support overriding frameworks for lint check
- support overriding framework policy data for security checks
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'bin/click-check-lint' | |||
2 | --- bin/click-check-lint 2013-09-26 12:24:10 +0000 | |||
3 | +++ bin/click-check-lint 2014-11-17 17:14:19 +0000 | |||
4 | @@ -16,6 +16,7 @@ | |||
5 | 16 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | 16 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
6 | 17 | 17 | ||
7 | 18 | from __future__ import print_function | 18 | from __future__ import print_function |
8 | 19 | import json | ||
9 | 19 | import sys | 20 | import sys |
10 | 20 | 21 | ||
11 | 21 | import clickreviews.cr_common as cr_common | 22 | import clickreviews.cr_common as cr_common |
12 | @@ -25,7 +26,14 @@ | |||
13 | 25 | if len(sys.argv) < 2: | 26 | if len(sys.argv) < 2: |
14 | 26 | cr_common.error("Must give path to click package") | 27 | cr_common.error("Must give path to click package") |
15 | 27 | 28 | ||
17 | 28 | review = cr_lint.ClickReviewLint(sys.argv[1]) | 29 | # extract args |
18 | 30 | fn = sys.argv[1] | ||
19 | 31 | if len(sys.argv) > 2: | ||
20 | 32 | overrides = json.loads(sys.argv[2]) | ||
21 | 33 | else: | ||
22 | 34 | overrides = None | ||
23 | 35 | |||
24 | 36 | review = cr_lint.ClickReviewLint(fn, overrides=overrides) | ||
25 | 29 | review.do_checks() | 37 | review.do_checks() |
26 | 30 | rc = review.do_report() | 38 | rc = review.do_report() |
27 | 31 | sys.exit(rc) | 39 | sys.exit(rc) |
28 | 32 | 40 | ||
29 | === modified file 'bin/click-check-security' | |||
30 | --- bin/click-check-security 2013-09-26 12:24:10 +0000 | |||
31 | +++ bin/click-check-security 2014-11-17 17:14:19 +0000 | |||
32 | @@ -16,6 +16,7 @@ | |||
33 | 16 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | 16 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
34 | 17 | 17 | ||
35 | 18 | from __future__ import print_function | 18 | from __future__ import print_function |
36 | 19 | import json | ||
37 | 19 | import sys | 20 | import sys |
38 | 20 | 21 | ||
39 | 21 | import clickreviews.cr_common as cr_common | 22 | import clickreviews.cr_common as cr_common |
40 | @@ -25,7 +26,14 @@ | |||
41 | 25 | if len(sys.argv) < 2: | 26 | if len(sys.argv) < 2: |
42 | 26 | cr_common.error("Must give path to click package") | 27 | cr_common.error("Must give path to click package") |
43 | 27 | 28 | ||
45 | 28 | review = cr_security.ClickReviewSecurity(sys.argv[1]) | 29 | # extract args |
46 | 30 | fn = sys.argv[1] | ||
47 | 31 | if len(sys.argv) > 2: | ||
48 | 32 | overrides = json.loads(sys.argv[2]) | ||
49 | 33 | else: | ||
50 | 34 | overrides = None | ||
51 | 35 | |||
52 | 36 | review = cr_security.ClickReviewSecurity(fn, overrides=overrides) | ||
53 | 29 | review.do_checks() | 37 | review.do_checks() |
54 | 30 | rc = review.do_report() | 38 | rc = review.do_report() |
55 | 31 | sys.exit(rc) | 39 | sys.exit(rc) |
56 | 32 | 40 | ||
57 | === modified file 'clickreviews/cr_lint.py' | |||
58 | --- clickreviews/cr_lint.py 2014-11-11 15:19:59 +0000 | |||
59 | +++ clickreviews/cr_lint.py 2014-11-17 17:14:19 +0000 | |||
60 | @@ -31,7 +31,7 @@ | |||
61 | 31 | class ClickReviewLint(ClickReview): | 31 | class ClickReviewLint(ClickReview): |
62 | 32 | '''This class represents click lint reviews''' | 32 | '''This class represents click lint reviews''' |
63 | 33 | 33 | ||
65 | 34 | def __init__(self, fn): | 34 | def __init__(self, fn, overrides=None): |
66 | 35 | '''Set up the class.''' | 35 | '''Set up the class.''' |
67 | 36 | ClickReview.__init__(self, fn, "lint") | 36 | ClickReview.__init__(self, fn, "lint") |
68 | 37 | self.control_files = dict() | 37 | self.control_files = dict() |
69 | @@ -87,6 +87,7 @@ | |||
70 | 87 | 'urls'] | 87 | 'urls'] |
71 | 88 | 88 | ||
72 | 89 | self.redflagged_hooks = ['pay-ui'] | 89 | self.redflagged_hooks = ['pay-ui'] |
73 | 90 | self.overrides = overrides if overrides is not None else {} | ||
74 | 90 | 91 | ||
75 | 91 | def _list_control_files(self): | 92 | def _list_control_files(self): |
76 | 92 | '''List all control files with their full path.''' | 93 | '''List all control files with their full path.''' |
77 | @@ -641,7 +642,8 @@ | |||
78 | 641 | '''Check framework()''' | 642 | '''Check framework()''' |
79 | 642 | n = 'framework' | 643 | n = 'framework' |
80 | 643 | l = "http://askubuntu.com/questions/460512/what-framework-should-i-use-in-my-manifest-file" | 644 | l = "http://askubuntu.com/questions/460512/what-framework-should-i-use-in-my-manifest-file" |
82 | 644 | frameworks = Frameworks() | 645 | framework_overrides = self.overrides.get('framework') |
83 | 646 | frameworks = Frameworks(overrides=framework_overrides) | ||
84 | 645 | if self.manifest['framework'] in frameworks.AVAILABLE_FRAMEWORKS: | 647 | if self.manifest['framework'] in frameworks.AVAILABLE_FRAMEWORKS: |
85 | 646 | t = 'info' | 648 | t = 'info' |
86 | 647 | s = 'OK' | 649 | s = 'OK' |
87 | 648 | 650 | ||
88 | === modified file 'clickreviews/cr_security.py' | |||
89 | --- clickreviews/cr_security.py 2014-10-14 13:14:05 +0000 | |||
90 | +++ clickreviews/cr_security.py 2014-11-17 17:14:19 +0000 | |||
91 | @@ -25,7 +25,7 @@ | |||
92 | 25 | 25 | ||
93 | 26 | class ClickReviewSecurity(ClickReview): | 26 | class ClickReviewSecurity(ClickReview): |
94 | 27 | '''This class represents click lint reviews''' | 27 | '''This class represents click lint reviews''' |
96 | 28 | def __init__(self, fn): | 28 | def __init__(self, fn, overrides=None): |
97 | 29 | ClickReview.__init__(self, fn, "security") | 29 | ClickReview.__init__(self, fn, "security") |
98 | 30 | 30 | ||
99 | 31 | local_copy = os.path.join(os.path.dirname(__file__), | 31 | local_copy = os.path.join(os.path.dirname(__file__), |
100 | @@ -78,10 +78,19 @@ | |||
101 | 78 | # framework policy is based on major framework version. In 13.10, there | 78 | # framework policy is based on major framework version. In 13.10, there |
102 | 79 | # was only 'ubuntu-sdk-13.10', but in 14.04, there will be several, | 79 | # was only 'ubuntu-sdk-13.10', but in 14.04, there will be several, |
103 | 80 | # like 'ubuntu-sdk-14.04-html5', 'ubuntu-sdk-14.04-platform', etc | 80 | # like 'ubuntu-sdk-14.04-html5', 'ubuntu-sdk-14.04-platform', etc |
108 | 81 | self.major_framework_policy = {'ubuntu-sdk-13.10': 1.0, | 81 | self.major_framework_policy = { |
109 | 82 | 'ubuntu-sdk-14.04': 1.1, | 82 | 'ubuntu-sdk-13.10': { |
110 | 83 | 'ubuntu-sdk-14.10': 1.2, | 83 | 'policy_version': 1.0, |
111 | 84 | } | 84 | }, |
112 | 85 | 'ubuntu-sdk-14.04': { | ||
113 | 86 | 'policy_version': 1.1, | ||
114 | 87 | }, | ||
115 | 88 | 'ubuntu-sdk-14.10': { | ||
116 | 89 | 'policy_version': 1.2, | ||
117 | 90 | }, | ||
118 | 91 | } | ||
119 | 92 | framework_overrides = overrides.get('framework') if overrides else {} | ||
120 | 93 | self._override_framework_policies(framework_overrides) | ||
121 | 85 | 94 | ||
122 | 86 | self.security_manifests = dict() | 95 | self.security_manifests = dict() |
123 | 87 | self.security_apps = [] | 96 | self.security_apps = [] |
124 | @@ -102,6 +111,23 @@ | |||
125 | 102 | self._extract_security_manifest(app) | 111 | self._extract_security_manifest(app) |
126 | 103 | self.security_apps.append(app) | 112 | self.security_apps.append(app) |
127 | 104 | 113 | ||
128 | 114 | def _override_framework_policies(self, overrides): | ||
129 | 115 | # override major framework policies | ||
130 | 116 | self.major_framework_policy.update(overrides) | ||
131 | 117 | |||
132 | 118 | # override apparmor policies | ||
133 | 119 | for name, data in overrides.items(): | ||
134 | 120 | vendor = data.get('policy_vendor') | ||
135 | 121 | version = str(data.get('policy_version')) | ||
136 | 122 | |||
137 | 123 | if vendor not in self.aa_policy: | ||
138 | 124 | self.aa_policy[vendor] = {} | ||
139 | 125 | |||
140 | 126 | if version not in self.aa_policy[vendor]: | ||
141 | 127 | # just ensure the version is defined | ||
142 | 128 | # TODO: add support to override templates and policy groups | ||
143 | 129 | self.aa_policy[vendor][version] = {} | ||
144 | 130 | |||
145 | 105 | def _extract_security_manifest(self, app): | 131 | def _extract_security_manifest(self, app): |
146 | 106 | '''Extract security manifest and verify it has the expected | 132 | '''Extract security manifest and verify it has the expected |
147 | 107 | structure''' | 133 | structure''' |
148 | @@ -269,15 +295,15 @@ | |||
149 | 269 | n = 'policy_version_matches_framework (%s)' % (f) | 295 | n = 'policy_version_matches_framework (%s)' % (f) |
150 | 270 | s = "OK" | 296 | s = "OK" |
151 | 271 | found_major = False | 297 | found_major = False |
153 | 272 | for k in self.major_framework_policy.keys(): | 298 | for name, data in self.major_framework_policy.items(): |
154 | 273 | # TODO: use libclick when it is available | 299 | # TODO: use libclick when it is available |
156 | 274 | if not self.manifest['framework'].startswith(k): | 300 | if not self.manifest['framework'].startswith(name): |
157 | 275 | continue | 301 | continue |
158 | 276 | found_major = True | 302 | found_major = True |
160 | 277 | if m['policy_version'] != self.major_framework_policy[k]: | 303 | if m['policy_version'] != data['policy_version']: |
161 | 278 | t = 'error' | 304 | t = 'error' |
162 | 279 | s = '%s != %s (%s)' % (str(m['policy_version']), | 305 | s = '%s != %s (%s)' % (str(m['policy_version']), |
164 | 280 | self.major_framework_policy[k], | 306 | data['policy_version'], |
165 | 281 | self.manifest['framework']) | 307 | self.manifest['framework']) |
166 | 282 | if not found_major: | 308 | if not found_major: |
167 | 283 | t = 'error' | 309 | t = 'error' |
168 | 284 | 310 | ||
169 | === modified file 'clickreviews/frameworks.py' | |||
170 | --- clickreviews/frameworks.py 2014-11-11 16:12:57 +0000 | |||
171 | +++ clickreviews/frameworks.py 2014-11-17 17:14:19 +0000 | |||
172 | @@ -32,14 +32,21 @@ | |||
173 | 32 | OBSOLETE_FRAMEWORKS = [] | 32 | OBSOLETE_FRAMEWORKS = [] |
174 | 33 | AVAILABLE_FRAMEWORKS = [] | 33 | AVAILABLE_FRAMEWORKS = [] |
175 | 34 | 34 | ||
177 | 35 | def __init__(self): | 35 | def __init__(self, overrides=None): |
178 | 36 | self.FRAMEWORKS = clickreviews.remote.read_cr_file(USER_DATA_FILE, | 36 | self.FRAMEWORKS = clickreviews.remote.read_cr_file(USER_DATA_FILE, |
179 | 37 | FRAMEWORKS_DATA_URL) | 37 | FRAMEWORKS_DATA_URL) |
188 | 38 | 38 | if overrides is not None: | |
189 | 39 | for k, v in self.FRAMEWORKS.items(): | 39 | self.FRAMEWORKS.update(overrides) |
190 | 40 | if v == 'deprecated': | 40 | |
191 | 41 | self.DEPRECATED_FRAMEWORKS.append(k) | 41 | for name, data in self.FRAMEWORKS.items(): |
192 | 42 | elif v == 'obsolete': | 42 | if type(data) is dict: |
193 | 43 | self.OBSOLETE_FRAMEWORKS.append(k) | 43 | state = data.get('state') |
194 | 44 | elif v == 'available': | 44 | else: |
195 | 45 | self.AVAILABLE_FRAMEWORKS.append(k) | 45 | state = data |
196 | 46 | |||
197 | 47 | if state == 'deprecated': | ||
198 | 48 | self.DEPRECATED_FRAMEWORKS.append(name) | ||
199 | 49 | elif state == 'obsolete': | ||
200 | 50 | self.OBSOLETE_FRAMEWORKS.append(name) | ||
201 | 51 | elif state == 'available': | ||
202 | 52 | self.AVAILABLE_FRAMEWORKS.append(name) | ||
203 | 46 | 53 | ||
204 | === modified file 'clickreviews/tests/test_cr_lint.py' | |||
205 | --- clickreviews/tests/test_cr_lint.py 2014-11-11 16:12:57 +0000 | |||
206 | +++ clickreviews/tests/test_cr_lint.py 2014-11-17 17:14:19 +0000 | |||
207 | @@ -31,7 +31,7 @@ | |||
208 | 31 | super() | 31 | super() |
209 | 32 | 32 | ||
210 | 33 | def patch_frameworks(self): | 33 | def patch_frameworks(self): |
212 | 34 | def _mock_frameworks(self): | 34 | def _mock_frameworks(self, overrides=None): |
213 | 35 | self.FRAMEWORKS = { | 35 | self.FRAMEWORKS = { |
214 | 36 | 'ubuntu-sdk-14.10-qml-dev2': 'available', | 36 | 'ubuntu-sdk-14.10-qml-dev2': 'available', |
215 | 37 | 'ubuntu-sdk-13.10': 'deprecated', | 37 | 'ubuntu-sdk-13.10': 'deprecated', |
216 | @@ -752,6 +752,20 @@ | |||
217 | 752 | expected_counts = {'info': None, 'warn': 0, 'error': 1} | 752 | expected_counts = {'info': None, 'warn': 0, 'error': 1} |
218 | 753 | self.check_results(r, expected_counts) | 753 | self.check_results(r, expected_counts) |
219 | 754 | 754 | ||
220 | 755 | @patch('clickreviews.remote.read_cr_file') | ||
221 | 756 | def test_check_framework_with_overrides(self, mock_read_cr_file): | ||
222 | 757 | '''Test check_framework() - using overrides''' | ||
223 | 758 | mock_read_cr_file.return_value = { | ||
224 | 759 | 'ubuntu-sdk-14.10-qml-dev2': 'available', | ||
225 | 760 | } | ||
226 | 761 | self.set_test_manifest("framework", "nonexistent") | ||
227 | 762 | overrides = {'framework': {'nonexistent': {'state': 'available'}}} | ||
228 | 763 | c = ClickReviewLint(self.test_name, overrides=overrides) | ||
229 | 764 | c.check_framework() | ||
230 | 765 | r = c.click_report | ||
231 | 766 | expected_counts = {'info': 1, 'warn': 0, 'error': 0} | ||
232 | 767 | self.check_results(r, expected_counts) | ||
233 | 768 | |||
234 | 755 | def test_check_hooks(self): | 769 | def test_check_hooks(self): |
235 | 756 | '''Test check_hooks()''' | 770 | '''Test check_hooks()''' |
236 | 757 | self.set_test_manifest("framework", "ubuntu-sdk-13.10") | 771 | self.set_test_manifest("framework", "ubuntu-sdk-13.10") |
237 | 758 | 772 | ||
238 | === modified file 'clickreviews/tests/test_cr_security.py' | |||
239 | --- clickreviews/tests/test_cr_security.py 2014-10-08 20:08:44 +0000 | |||
240 | +++ clickreviews/tests/test_cr_security.py 2014-11-17 17:14:19 +0000 | |||
241 | @@ -136,7 +136,7 @@ | |||
242 | 136 | policy_version = 0 | 136 | policy_version = 0 |
243 | 137 | for k in tmp.major_framework_policy.keys(): | 137 | for k in tmp.major_framework_policy.keys(): |
244 | 138 | if f.startswith(k): | 138 | if f.startswith(k): |
246 | 139 | policy_version = tmp.major_framework_policy[k] | 139 | policy_version = tmp.major_framework_policy[k]['policy_version'] |
247 | 140 | self.set_test_security_manifest(self.default_appname, | 140 | self.set_test_security_manifest(self.default_appname, |
248 | 141 | "policy_version", | 141 | "policy_version", |
249 | 142 | policy_version) | 142 | policy_version) |
250 | @@ -209,6 +209,21 @@ | |||
251 | 209 | {"text": "Invalid framework 'nonexistent'"} | 209 | {"text": "Invalid framework 'nonexistent'"} |
252 | 210 | self.check_results(report, expected=expected) | 210 | self.check_results(report, expected=expected) |
253 | 211 | 211 | ||
254 | 212 | def test_check_policy_version_framework_with_overrides(self): | ||
255 | 213 | '''Test check_policy_version() - override framework (nonexistent)''' | ||
256 | 214 | self.set_test_manifest("framework", "nonexistent") | ||
257 | 215 | self.set_test_security_manifest(self.default_appname, | ||
258 | 216 | "policy_version", 1.3) | ||
259 | 217 | overrides = {'framework': {'nonexistent': {'state': 'available', | ||
260 | 218 | 'policy_vendor': 'ubuntu', | ||
261 | 219 | 'policy_version': 1.3}}} | ||
262 | 220 | c = ClickReviewSecurity(self.test_name, overrides=overrides) | ||
263 | 221 | c.check_policy_version() | ||
264 | 222 | report = c.click_report | ||
265 | 223 | |||
266 | 224 | expected_counts = {'info': 3, 'warn': 0, 'error': 0} | ||
267 | 225 | self.check_results(report, expected_counts) | ||
268 | 226 | |||
269 | 212 | def test_check_policy_vendor_unspecified(self): | 227 | def test_check_policy_vendor_unspecified(self): |
270 | 213 | '''Test check_policy_vendor() - unspecified''' | 228 | '''Test check_policy_vendor() - unspecified''' |
271 | 214 | c = ClickReviewSecurity(self.test_name) | 229 | c = ClickReviewSecurity(self.test_name) |
Looks great, thanks!