Merge ~jslarraz/review-tools:add-doc-about-libmagic-warning into review-tools:master
- Git
- lp:~jslarraz/review-tools
- add-doc-about-libmagic-warning
- Merge into master
Proposed by
Jorge Sancho Larraz
Status: | Merged |
---|---|
Merged at revision: | f0fe85e3345eb4d0bae3657222b8d4974286a357 |
Proposed branch: | ~jslarraz/review-tools:add-doc-about-libmagic-warning |
Merge into: | review-tools:master |
Diff against target: |
947 lines (+492/-169) 9 files modified
reviewtools/available.py (+7/-2) reviewtools/common.py (+0/-142) reviewtools/containers/base_container.py (+160/-13) reviewtools/containers/snap_container.py (+44/-0) reviewtools/containers/squashfs_container.py (+111/-0) reviewtools/sr_common.py (+2/-2) reviewtools/sr_tests.py (+1/-2) reviewtools/tests/containers/test_base_container.py (+94/-8) reviewtools/tests/containers/test_squashfs_container.py (+73/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alex Murray | Needs Fixing | ||
Review via email:
|
Commit message
rt/c/base_
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 | diff --git a/reviewtools/available.py b/reviewtools/available.py | |||
2 | index eafa366..5b0dd3c 100755 | |||
3 | --- a/reviewtools/available.py | |||
4 | +++ b/reviewtools/available.py | |||
5 | @@ -19,6 +19,7 @@ import shutil | |||
6 | 19 | import tempfile | 19 | import tempfile |
7 | 20 | 20 | ||
8 | 21 | from reviewtools.common import ( | 21 | from reviewtools.common import ( |
9 | 22 | error, | ||
10 | 22 | debug, | 23 | debug, |
11 | 23 | warn, | 24 | warn, |
12 | 24 | open_file_write, | 25 | open_file_write, |
13 | @@ -26,7 +27,6 @@ from reviewtools.common import ( | |||
14 | 26 | MKDTEMP_PREFIX, | 27 | MKDTEMP_PREFIX, |
15 | 27 | _add_error, # make a class | 28 | _add_error, # make a class |
16 | 28 | get_rock_manifest, | 29 | get_rock_manifest, |
17 | 29 | get_snap_manifest, | ||
18 | 30 | ) | 30 | ) |
19 | 31 | import reviewtools.email as email | 31 | import reviewtools.email as email |
20 | 32 | 32 | ||
21 | @@ -37,6 +37,7 @@ from reviewtools.store import ( | |||
22 | 37 | get_faked_build_and_stage_packages, | 37 | get_faked_build_and_stage_packages, |
23 | 38 | ) | 38 | ) |
24 | 39 | from reviewtools.usn import read_usn_db | 39 | from reviewtools.usn import read_usn_db |
25 | 40 | from reviewtools.containers.snap_container import SnapContainer, ContainerException | ||
26 | 40 | 41 | ||
27 | 41 | email_update_required_text = """A scan of this %s shows that it was built with packages from the Ubuntu | 42 | email_update_required_text = """A scan of this %s shows that it was built with packages from the Ubuntu |
28 | 42 | archive that have since received security updates. """ | 43 | archive that have since received security updates. """ |
29 | @@ -484,7 +485,11 @@ def scan_store( | |||
30 | 484 | def scan_snap(secnot_db_fn, snap_fn, with_cves=False, ignore_pockets=list()): | 485 | def scan_snap(secnot_db_fn, snap_fn, with_cves=False, ignore_pockets=list()): |
31 | 485 | """Scan snap for packages with security notices""" | 486 | """Scan snap for packages with security notices""" |
32 | 486 | out = "" | 487 | out = "" |
34 | 487 | (man, dpkg) = get_snap_manifest(snap_fn) | 488 | try: |
35 | 489 | snap = SnapContainer(snap_fn) | ||
36 | 490 | (man, dpkg) = snap.get_snap_manifest() | ||
37 | 491 | except ContainerException as e: | ||
38 | 492 | error(str(e)) | ||
39 | 488 | man = get_faked_build_and_stage_packages(man) | 493 | man = get_faked_build_and_stage_packages(man) |
40 | 489 | 494 | ||
41 | 490 | # Use dpkg.list when the snap ships it in a spot we honor. This is limited | 495 | # Use dpkg.list when the snap ships it in a spot we honor. This is limited |
42 | diff --git a/reviewtools/common.py b/reviewtools/common.py | |||
43 | index e2e80ff..d2c15b8 100644 | |||
44 | --- a/reviewtools/common.py | |||
45 | +++ b/reviewtools/common.py | |||
46 | @@ -424,30 +424,6 @@ def cmdIgnoreErrorStrings(command, ignoreErrorStrings): | |||
47 | 424 | return [rc, out] | 424 | return [rc, out] |
48 | 425 | 425 | ||
49 | 426 | 426 | ||
50 | 427 | def _unpack_cmd(cmd_args, d, dest): | ||
51 | 428 | """Low level unpack helper""" | ||
52 | 429 | curdir = os.getcwd() | ||
53 | 430 | os.chdir(d) | ||
54 | 431 | |||
55 | 432 | (rc, out) = cmdIgnoreErrorStrings(cmd_args, UNSQUASHFS_IGNORED_ERRORS) | ||
56 | 433 | |||
57 | 434 | os.chdir(curdir) | ||
58 | 435 | |||
59 | 436 | if rc != 0: | ||
60 | 437 | if os.path.isdir(d): | ||
61 | 438 | recursive_rm(d) | ||
62 | 439 | error("unpacking failed with '%d':\n%s" % (rc, out)) | ||
63 | 440 | |||
64 | 441 | if dest is None: | ||
65 | 442 | dest = d | ||
66 | 443 | else: | ||
67 | 444 | # Recursively move unpacked content to original dir, keeping | ||
68 | 445 | # permissions | ||
69 | 446 | move_dir_content(d, dest) | ||
70 | 447 | |||
71 | 448 | return dest | ||
72 | 449 | |||
73 | 450 | |||
74 | 451 | # The original directory might have restrictive permissions, so save | 427 | # The original directory might have restrictive permissions, so save |
75 | 452 | # them off, chmod the dir, do the move and reapply | 428 | # them off, chmod the dir, do the move and reapply |
76 | 453 | def move_dir_content(d, dest): | 429 | def move_dir_content(d, dest): |
77 | @@ -621,24 +597,6 @@ def unsquashfs_lln_parse(lln_out): | |||
78 | 621 | return hdr, entries | 597 | return hdr, entries |
79 | 622 | 598 | ||
80 | 623 | 599 | ||
81 | 624 | def _calculate_snap_unsquashfs_uncompressed_size(snap_pkg): | ||
82 | 625 | """Calculate size of the uncompressed snap""" | ||
83 | 626 | (rc, out) = cmd(["unsquashfs", "-lln", snap_pkg]) | ||
84 | 627 | if rc != 0: | ||
85 | 628 | error("unsquashfs -lln '%s' failed: %s" % (snap_pkg, out)) | ||
86 | 629 | |||
87 | 630 | size = 0 | ||
88 | 631 | for line in out.splitlines(): | ||
89 | 632 | if not line.startswith("-"): # skip non-regular files | ||
90 | 633 | continue | ||
91 | 634 | try: | ||
92 | 635 | size += int(line.split()[2]) | ||
93 | 636 | except ValueError: # skip non-numbers | ||
94 | 637 | continue | ||
95 | 638 | |||
96 | 639 | return size | ||
97 | 640 | |||
98 | 641 | |||
99 | 642 | def _calculate_rock_untar_uncompressed_size(rock_pkg): | 600 | def _calculate_rock_untar_uncompressed_size(rock_pkg): |
100 | 643 | """Calculate size of the uncompressed tar""" | 601 | """Calculate size of the uncompressed tar""" |
101 | 644 | size = 0 | 602 | size = 0 |
102 | @@ -656,34 +614,6 @@ def unsquashfs_supports_ignore_errors(): | |||
103 | 656 | return "-ig[nore-errors]" in out | 614 | return "-ig[nore-errors]" in out |
104 | 657 | 615 | ||
105 | 658 | 616 | ||
106 | 659 | def _unpack_snap_squashfs(snap_pkg, dest, items=[]): | ||
107 | 660 | """Unpack a squashfs based snap package to dest""" | ||
108 | 661 | size = _calculate_snap_unsquashfs_uncompressed_size(snap_pkg) | ||
109 | 662 | |||
110 | 663 | snap_max_size = MAX_UNCOMPRESSED_SIZE * 1024 * 1024 * 1024 | ||
111 | 664 | valid_size, error_msg = is_pkg_uncompressed_size_valid( | ||
112 | 665 | snap_max_size, size, snap_pkg | ||
113 | 666 | ) | ||
114 | 667 | if valid_size: | ||
115 | 668 | global MKDTEMP_PREFIX | ||
116 | 669 | global MKDTEMP_DIR | ||
117 | 670 | d = tempfile.mkdtemp(prefix=MKDTEMP_PREFIX, dir=MKDTEMP_DIR) | ||
118 | 671 | cmd = ["unsquashfs", "-no-progress", "-f", "-d", d] | ||
119 | 672 | |||
120 | 673 | # If unsquashfs supports it, pass "-ignore-errors -quiet" | ||
121 | 674 | if unsquashfs_supports_ignore_errors(): | ||
122 | 675 | cmd.append("-ignore-errors") | ||
123 | 676 | cmd.append("-quiet") | ||
124 | 677 | |||
125 | 678 | cmd.append(os.path.abspath(snap_pkg)) | ||
126 | 679 | |||
127 | 680 | if len(items) != 0: | ||
128 | 681 | cmd += items | ||
129 | 682 | return _unpack_cmd(cmd, d, dest) | ||
130 | 683 | else: | ||
131 | 684 | error(error_msg) | ||
132 | 685 | |||
133 | 686 | |||
134 | 687 | def is_pkg_uncompressed_size_valid(pkg_max_size, size, pkg): | 617 | def is_pkg_uncompressed_size_valid(pkg_max_size, size, pkg): |
135 | 688 | st = os.statvfs(pkg) | 618 | st = os.statvfs(pkg) |
136 | 689 | avail = st.f_bsize * st.f_bavail * 0.9 # 90% of available space | 619 | avail = st.f_bsize * st.f_bavail * 0.9 # 90% of available space |
137 | @@ -751,21 +681,6 @@ def _unpack_rock_tar(rock_pkg, dest): | |||
138 | 751 | error(error_msg) | 681 | error(error_msg) |
139 | 752 | 682 | ||
140 | 753 | 683 | ||
141 | 754 | def unpack_pkg(fn, dest=None, items=[]): | ||
142 | 755 | """Unpack package""" | ||
143 | 756 | pkg = check_fn(fn) | ||
144 | 757 | check_dir(dest) | ||
145 | 758 | |||
146 | 759 | # Limit the maximimum size of the package | ||
147 | 760 | check_max_pkg_size(pkg) | ||
148 | 761 | |||
149 | 762 | # check if its a squashfs based snap | ||
150 | 763 | if is_squashfs(pkg): | ||
151 | 764 | return _unpack_snap_squashfs(fn, dest, items) | ||
152 | 765 | |||
153 | 766 | error("Unsupported package format (not squashfs)") | ||
154 | 767 | |||
155 | 768 | |||
156 | 769 | def check_dir(dest): | 684 | def check_dir(dest): |
157 | 770 | if dest is not None and os.path.exists(dest): | 685 | if dest is not None and os.path.exists(dest): |
158 | 771 | error("'%s' exists. Aborting." % dest) | 686 | error("'%s' exists. Aborting." % dest) |
159 | @@ -790,13 +705,6 @@ def check_max_pkg_size(pkg): | |||
160 | 790 | ) | 705 | ) |
161 | 791 | 706 | ||
162 | 792 | 707 | ||
163 | 793 | def is_squashfs(filename): | ||
164 | 794 | """Return true if the given filename as a squashfs header""" | ||
165 | 795 | with open(filename, "rb") as f: | ||
166 | 796 | header = f.read(10) | ||
167 | 797 | return header.startswith(b"hsqs") | ||
168 | 798 | |||
169 | 799 | |||
170 | 800 | def unpack_rock(fn, dest=None): | 708 | def unpack_rock(fn, dest=None): |
171 | 801 | """Unpack rock""" | 709 | """Unpack rock""" |
172 | 802 | pkg = check_fn(fn) | 710 | pkg = check_fn(fn) |
173 | @@ -1078,56 +986,6 @@ def read_file_as_json_dict(fn): | |||
174 | 1078 | return raw | 986 | return raw |
175 | 1079 | 987 | ||
176 | 1080 | 988 | ||
177 | 1081 | def get_snap_manifest(fn): | ||
178 | 1082 | if "SNAP_USER_COMMON" in os.environ and os.path.exists( | ||
179 | 1083 | os.environ["SNAP_USER_COMMON"] | ||
180 | 1084 | ): | ||
181 | 1085 | MKDTEMP_DIR = os.environ["SNAP_USER_COMMON"] | ||
182 | 1086 | else: | ||
183 | 1087 | MKDTEMP_DIR = tempfile.gettempdir() | ||
184 | 1088 | |||
185 | 1089 | man = "snap/manifest.yaml" | ||
186 | 1090 | os_dpkg = "usr/share/snappy/dpkg.list" | ||
187 | 1091 | snap_dpkg = "snap/dpkg.list" | ||
188 | 1092 | # unpack_pkg() fails if this exists, so this is safe | ||
189 | 1093 | dir = tempfile.mktemp(prefix=MKDTEMP_PREFIX, dir=MKDTEMP_DIR) | ||
190 | 1094 | unpack_pkg(fn, dir, [man, os_dpkg, snap_dpkg]) | ||
191 | 1095 | |||
192 | 1096 | man_fn = os.path.join(dir, man) | ||
193 | 1097 | if not os.path.isfile(man_fn): | ||
194 | 1098 | recursive_rm(dir) | ||
195 | 1099 | error("%s not in %s" % (man, fn)) | ||
196 | 1100 | |||
197 | 1101 | with open_file_read(man_fn) as fd: | ||
198 | 1102 | try: | ||
199 | 1103 | man_yaml = yaml.safe_load(fd) | ||
200 | 1104 | except Exception: | ||
201 | 1105 | recursive_rm(dir) | ||
202 | 1106 | error("Could not load %s. Is it properly formatted?" % man) | ||
203 | 1107 | |||
204 | 1108 | os_dpkg_fn = os.path.join(dir, os_dpkg) | ||
205 | 1109 | snap_dpkg_fn = os.path.join(dir, snap_dpkg) | ||
206 | 1110 | dpkg_list = None | ||
207 | 1111 | if os.path.isfile(os_dpkg_fn): | ||
208 | 1112 | with open_file_read(os_dpkg_fn) as fd: | ||
209 | 1113 | try: | ||
210 | 1114 | dpkg_list = fd.readlines() | ||
211 | 1115 | except Exception: | ||
212 | 1116 | recursive_rm(dir) | ||
213 | 1117 | error("Could not load %s. Is it properly formatted?" % os_dpkg) | ||
214 | 1118 | elif os.path.isfile(snap_dpkg_fn): | ||
215 | 1119 | with open_file_read(snap_dpkg_fn) as fd: | ||
216 | 1120 | try: | ||
217 | 1121 | dpkg_list = fd.readlines() | ||
218 | 1122 | except Exception: | ||
219 | 1123 | recursive_rm(dir) | ||
220 | 1124 | error("Could not load %s. Is it properly formatted?" % snap_dpkg) | ||
221 | 1125 | |||
222 | 1126 | recursive_rm(dir) | ||
223 | 1127 | |||
224 | 1128 | return (man_yaml, dpkg_list) | ||
225 | 1129 | |||
226 | 1130 | |||
227 | 1131 | def get_rock_manifest(fn): | 989 | def get_rock_manifest(fn): |
228 | 1132 | if "SNAP_USER_COMMON" in os.environ and os.path.exists( | 990 | if "SNAP_USER_COMMON" in os.environ and os.path.exists( |
229 | 1133 | os.environ["SNAP_USER_COMMON"] | 991 | os.environ["SNAP_USER_COMMON"] |
230 | diff --git a/reviewtools/containers/base_container.py b/reviewtools/containers/base_container.py | |||
231 | index 9e6f104..0d51021 100644 | |||
232 | --- a/reviewtools/containers/base_container.py | |||
233 | +++ b/reviewtools/containers/base_container.py | |||
234 | @@ -7,12 +7,19 @@ from tempfile import mkdtemp | |||
235 | 7 | from reviewtools.common import ( | 7 | from reviewtools.common import ( |
236 | 8 | MKDTEMP_PREFIX, | 8 | MKDTEMP_PREFIX, |
237 | 9 | MKDTEMP_DIR, | 9 | MKDTEMP_DIR, |
238 | 10 | debug, | ||
239 | 11 | unpack_pkg, | ||
240 | 12 | recursive_rm, | 10 | recursive_rm, |
241 | 13 | ) | 11 | ) |
242 | 14 | 12 | ||
243 | 15 | 13 | ||
244 | 14 | # Add base_container private attributes to documentation | ||
245 | 15 | __pdoc__ = { | ||
246 | 16 | "BaseContainer._min_packed_size": True, | ||
247 | 17 | "BaseContainer._max_packed_size": True, | ||
248 | 18 | "BaseContainer._min_unpacked_size": True, | ||
249 | 19 | "BaseContainer._max_unpacked_size": True, | ||
250 | 20 | } | ||
251 | 21 | |||
252 | 22 | |||
253 | 16 | class ContainerException(Exception): | 23 | class ContainerException(Exception): |
254 | 17 | """This class represents exceptions""" | 24 | """This class represents exceptions""" |
255 | 18 | 25 | ||
256 | @@ -32,16 +39,36 @@ class BaseContainer: | |||
257 | 32 | """ | 39 | """ |
258 | 33 | 40 | ||
259 | 34 | magic_binary_file_descriptions = [ | 41 | magic_binary_file_descriptions = [ |
260 | 35 | "application/x-executable; charset=binary", | ||
261 | 36 | "application/x-sharedlib; charset=binary", | ||
262 | 37 | "application/x-object; charset=binary", | ||
263 | 38 | # 18.04 and higher doesn't show the charset | ||
264 | 39 | "application/x-executable", | 42 | "application/x-executable", |
265 | 40 | "application/x-sharedlib", | 43 | "application/x-sharedlib", |
266 | 41 | "application/x-object", | 44 | "application/x-object", |
267 | 42 | "application/x-pie-executable", | 45 | "application/x-pie-executable", |
268 | 43 | ] | 46 | ] |
269 | 44 | 47 | ||
270 | 48 | _min_packed_size = None | ||
271 | 49 | """ | ||
272 | 50 | Minimum size of the container file in bytes. It will be used to check the | ||
273 | 51 | container size during initialization. | ||
274 | 52 | """ | ||
275 | 53 | |||
276 | 54 | _max_packed_size = None | ||
277 | 55 | """ | ||
278 | 56 | Maximum size of the container file in bytes. It will be used to check the | ||
279 | 57 | container size during initialization. | ||
280 | 58 | """ | ||
281 | 59 | |||
282 | 60 | _min_unpacked_size = None | ||
283 | 61 | """ | ||
284 | 62 | Minimum size of the unpacked directory in bytes. It will be used to check the | ||
285 | 63 | container size during initialization. | ||
286 | 64 | """ | ||
287 | 65 | |||
288 | 66 | _max_unpacked_size = None | ||
289 | 67 | """ | ||
290 | 68 | Maximum size of the unpacked directory in bytes. It will be used to check the | ||
291 | 69 | container size during initialization. | ||
292 | 70 | """ | ||
293 | 71 | |||
294 | 45 | _unpack_dir = None | 72 | _unpack_dir = None |
295 | 46 | _files = None | 73 | _files = None |
296 | 47 | _bin_files = None | 74 | _bin_files = None |
297 | @@ -59,6 +86,9 @@ class BaseContainer: | |||
298 | 59 | else: | 86 | else: |
299 | 60 | raise ContainerException("Could not find '%s'" % fn) | 87 | raise ContainerException("Could not find '%s'" % fn) |
300 | 61 | 88 | ||
301 | 89 | # Check max/min size if defined | ||
302 | 90 | self.check_container_size() | ||
303 | 91 | |||
304 | 62 | @property | 92 | @property |
305 | 63 | def unpack_dir(self) -> str: | 93 | def unpack_dir(self) -> str: |
306 | 64 | """ | 94 | """ |
307 | @@ -67,8 +97,7 @@ class BaseContainer: | |||
308 | 67 | will be returned. | 97 | will be returned. |
309 | 68 | """ | 98 | """ |
310 | 69 | if self._unpack_dir is None: | 99 | if self._unpack_dir is None: |
313 | 70 | self._unpack_dir = ".".join(self.filename.split(".")[:-1]) | 100 | self._unpack_dir = self.unpack() |
312 | 71 | unpack_pkg(self.filename, dest=self._unpack_dir) | ||
314 | 72 | return self._unpack_dir | 101 | return self._unpack_dir |
315 | 73 | 102 | ||
316 | 74 | @property | 103 | @property |
317 | @@ -104,6 +133,117 @@ class BaseContainer: | |||
318 | 104 | if os.path.exists(self.tmp_dir): | 133 | if os.path.exists(self.tmp_dir): |
319 | 105 | recursive_rm(self.tmp_dir) | 134 | recursive_rm(self.tmp_dir) |
320 | 106 | 135 | ||
321 | 136 | def check_container_format(self): | ||
322 | 137 | """ | ||
323 | 138 | Checks that the container file has the expected format. | ||
324 | 139 | |||
325 | 140 | Raises: | ||
326 | 141 | NotImplementedError: Calling this method will raise an exception as | ||
327 | 142 | format is specific to the container type. This method needs to be | ||
328 | 143 | implemented by the specific subclasses. | ||
329 | 144 | """ | ||
330 | 145 | raise NotImplementedError("Must be implemented by subclasses") | ||
331 | 146 | |||
332 | 147 | def get_container_size(self) -> int: | ||
333 | 148 | """ | ||
334 | 149 | Uses os.path.getsize() to return the size of the container file. | ||
335 | 150 | |||
336 | 151 | Retruns: | ||
337 | 152 | int: Size in bytes of the container file | ||
338 | 153 | """ | ||
339 | 154 | return os.path.getsize(self.filename) | ||
340 | 155 | |||
341 | 156 | def check_container_size(self): | ||
342 | 157 | """ | ||
343 | 158 | Checks that the container size is within the range specified by | ||
344 | 159 | min_packed_size and max_packed_size. If any of those values is not specified | ||
345 | 160 | (i.e. it is None), the respective check will be skipped. | ||
346 | 161 | |||
347 | 162 | Raises: | ||
348 | 163 | ContainerException: If container size is less than minSize or greater | ||
349 | 164 | than maxSize, a ContainerException will be raised. | ||
350 | 165 | """ | ||
351 | 166 | size = self.get_container_size() | ||
352 | 167 | if self._min_packed_size and (size < self._min_packed_size): | ||
353 | 168 | raise ContainerException( | ||
354 | 169 | "container file is too small (%dM < %dM)" | ||
355 | 170 | % (size / 1024 / 1024, self._min_packed_size / 1024 / 1024) | ||
356 | 171 | ) | ||
357 | 172 | if self._max_packed_size and (size > self._max_packed_size): | ||
358 | 173 | raise ContainerException( | ||
359 | 174 | "container file is too large (%dM > %dM)" | ||
360 | 175 | % (size / 1024 / 1024, self._max_packed_size / 1024 / 1024) | ||
361 | 176 | ) | ||
362 | 177 | |||
363 | 178 | def calculate_unpacked_size(self) -> int: | ||
364 | 179 | """ | ||
365 | 180 | Estimates the space that the unpacked directory will use without unpacking. | ||
366 | 181 | |||
367 | 182 | Returns: | ||
368 | 183 | int: Estimated size in bytes of the unpacked directory | ||
369 | 184 | |||
370 | 185 | Raises: | ||
371 | 186 | NotImplementedError: Calling this method will raise an exception as the | ||
372 | 187 | estimation method is specific to the container type. This method | ||
373 | 188 | needs to be implemented by the specific subclasses. | ||
374 | 189 | """ | ||
375 | 190 | raise NotImplementedError("Must be implemented by subclasses") | ||
376 | 191 | |||
377 | 192 | def check_unpacked_size(self): | ||
378 | 193 | """ | ||
379 | 194 | Checks that the container size is within the range specified by | ||
380 | 195 | min_unpacked_size and max_unpacked_size. If any of those values is not | ||
381 | 196 | specified (i.e. it is None), the respective check will be skipped. | ||
382 | 197 | |||
383 | 198 | Raises: | ||
384 | 199 | ContainerException: If unpacked size is less than minSize or greater | ||
385 | 200 | than maxSize, a ContainerException will be raised. | ||
386 | 201 | """ | ||
387 | 202 | size = self.calculate_unpacked_size() | ||
388 | 203 | if self._min_unpacked_size and (size < self._min_unpacked_size): | ||
389 | 204 | raise ContainerException( | ||
390 | 205 | "unpacked directory is too small (%dM < %dM)" | ||
391 | 206 | % (size / 1024 / 1024, self._min_unpacked_size / 1024 / 1024) | ||
392 | 207 | ) | ||
393 | 208 | if self._max_unpacked_size and (size > self._max_unpacked_size): | ||
394 | 209 | raise ContainerException( | ||
395 | 210 | "unpacked directory is too large (%dM > %dM)" | ||
396 | 211 | % (size / 1024 / 1024, self._max_unpacked_size / 1024 / 1024) | ||
397 | 212 | ) | ||
398 | 213 | |||
399 | 214 | def check_disk_space(self): | ||
400 | 215 | """ | ||
401 | 216 | Checks that there is sufficient disk space available for the container to be | ||
402 | 217 | unpacked, i.e. that it will not consume more than 90% of the available disk | ||
403 | 218 | |||
404 | 219 | Raises: | ||
405 | 220 | ContainerException: If estimated unpacked size exceed the 90% of the | ||
406 | 221 | available space, an exception will be raised. | ||
407 | 222 | """ | ||
408 | 223 | size = self.calculate_unpacked_size() | ||
409 | 224 | st = os.statvfs(self.tmp_dir) | ||
410 | 225 | avail = st.f_bsize * st.f_bavail * 0.9 # 90% of available space | ||
411 | 226 | if size > avail * 0.9: | ||
412 | 227 | raise ContainerException( | ||
413 | 228 | "insufficient available disk space (%dM > %dM)" | ||
414 | 229 | % (size / 1024 / 1024, avail / 1024 / 1024) | ||
415 | 230 | ) | ||
416 | 231 | |||
417 | 232 | def unpack(self) -> str: | ||
418 | 233 | """ | ||
419 | 234 | Extracts the content of the container file and returns the absolute path to | ||
420 | 235 | the newly unpacked directory. | ||
421 | 236 | |||
422 | 237 | Returns: | ||
423 | 238 | str: absolute path to the newly created unpack dir | ||
424 | 239 | |||
425 | 240 | Raises: | ||
426 | 241 | NotImplementedError: Calling this method will raise an exception as the | ||
427 | 242 | estimation method is specific to the container type. This method | ||
428 | 243 | needs to be implemented by the specific subclasses. | ||
429 | 244 | """ | ||
430 | 245 | raise NotImplementedError("Must be implemented by subclasses") | ||
431 | 246 | |||
432 | 107 | def get_file(self, fn: str) -> Union[str, None]: | 247 | def get_file(self, fn: str) -> Union[str, None]: |
433 | 108 | """ | 248 | """ |
434 | 109 | Reads and return the raw content of the specified file. | 249 | Reads and return the raw content of the specified file. |
435 | @@ -117,7 +257,7 @@ class BaseContainer: | |||
436 | 117 | 257 | ||
437 | 118 | Raises: | 258 | Raises: |
438 | 119 | ContainerException: Calling this method will raise an exception if | 259 | ContainerException: Calling this method will raise an exception if |
440 | 120 | the requested file is not found. | 260 | the requested file is not found. |
441 | 121 | """ | 261 | """ |
442 | 122 | if not os.path.exists(os.path.join(self.unpack_dir, fn)): | 262 | if not os.path.exists(os.path.join(self.unpack_dir, fn)): |
443 | 123 | raise ContainerException("Could not find '%s'" % fn) | 263 | raise ContainerException("Could not find '%s'" % fn) |
444 | @@ -156,15 +296,22 @@ class BaseContainer: | |||
445 | 156 | Returns: | 296 | Returns: |
446 | 157 | list(str): list of files present in the container | 297 | list(str): list of files present in the container |
447 | 158 | """ | 298 | """ |
449 | 159 | mime = magic.open(magic.MAGIC_MIME) | 299 | mime = magic.compat.open(magic.MAGIC_MIME) |
450 | 160 | mime.load() | 300 | mime.load() |
451 | 161 | bin_files = [] | 301 | bin_files = [] |
452 | 162 | for file in self.get_files_list(abs=True): | 302 | for file in self.get_files_list(abs=True): |
453 | 163 | try: | 303 | try: |
454 | 304 | # There are two different magic libraries for python, python-magic | ||
455 | 305 | # and python-libmagic. We are installing python-magic (which is the | ||
456 | 306 | # only one distributed via apt). However, python-magic seems to hang | ||
457 | 307 | # when it hits a pipe file. | ||
458 | 308 | # python-magic also bundles the python-libmagic module and exposes | ||
459 | 309 | # it through the following compatibility API. Using this bundled | ||
460 | 310 | # python-libmagic module will generate a deprecation warning. See | ||
461 | 311 | # https://github.com/ahupp/python-magic/blob/master/magic/__init__.py#L458 | ||
462 | 164 | res = mime.file(file) | 312 | res = mime.file(file) |
466 | 165 | except Exception: # pragma: nocover | 313 | except OSError: |
467 | 166 | # workaround for zesty python3-magic | 314 | # TODO: file does not exists. Only hit for external symlinks |
465 | 167 | debug("could not detemine mime type of '%s'" % file) | ||
468 | 168 | continue | 315 | continue |
469 | 169 | 316 | ||
470 | 170 | if not abs: | 317 | if not abs: |
471 | diff --git a/reviewtools/containers/snap_container.py b/reviewtools/containers/snap_container.py | |||
472 | 171 | new file mode 100644 | 318 | new file mode 100644 |
473 | index 0000000..ff1ee57 | |||
474 | --- /dev/null | |||
475 | +++ b/reviewtools/containers/snap_container.py | |||
476 | @@ -0,0 +1,44 @@ | |||
477 | 1 | import yaml | ||
478 | 2 | from reviewtools.containers.squashfs_container import ( | ||
479 | 3 | SquashfsContainer, | ||
480 | 4 | ContainerException, | ||
481 | 5 | ) | ||
482 | 6 | |||
483 | 7 | |||
484 | 8 | class SnapContainer(SquashfsContainer): | ||
485 | 9 | # TODO: review-tools was not checking for it. We still have some snaps in | ||
486 | 10 | # "tests/" that doesn't fit this limit and will need to be regenerated before | ||
487 | 11 | # this check can be enforced. | ||
488 | 12 | # _min_packed_size = 16 * 1024 | ||
489 | 13 | _max_packed_size = 5 * 1024 * 1024 * 1024 | ||
490 | 14 | _max_unpacked_size = 25 * 1024 * 1024 * 1024 | ||
491 | 15 | |||
492 | 16 | def get_snap_manifest(self): | ||
493 | 17 | man = "snap/manifest.yaml" | ||
494 | 18 | os_dpkg = "usr/share/snappy/dpkg.list" | ||
495 | 19 | snap_dpkg = "snap/dpkg.list" | ||
496 | 20 | |||
497 | 21 | if self._unpack_dir is None: | ||
498 | 22 | self.unpack(items=[man, os_dpkg, snap_dpkg]) | ||
499 | 23 | |||
500 | 24 | man_content = self.get_file(man).decode() | ||
501 | 25 | try: | ||
502 | 26 | man_yaml = yaml.safe_load(man_content) | ||
503 | 27 | except Exception: | ||
504 | 28 | raise ContainerException( | ||
505 | 29 | "Could not load %s. Is it properly formatted?" % man | ||
506 | 30 | ) | ||
507 | 31 | |||
508 | 32 | dpkg_list = None | ||
509 | 33 | try: | ||
510 | 34 | dpkg_list = self.get_file(os_dpkg).decode().split("\n") | ||
511 | 35 | return (man_yaml, dpkg_list) | ||
512 | 36 | except ContainerException: | ||
513 | 37 | pass | ||
514 | 38 | |||
515 | 39 | try: | ||
516 | 40 | dpkg_list = self.get_file(snap_dpkg).decode().split("\n") | ||
517 | 41 | except ContainerException: | ||
518 | 42 | pass | ||
519 | 43 | |||
520 | 44 | return (man_yaml, dpkg_list) | ||
521 | diff --git a/reviewtools/containers/squashfs_container.py b/reviewtools/containers/squashfs_container.py | |||
522 | 0 | new file mode 100644 | 45 | new file mode 100644 |
523 | index 0000000..631fe76 | |||
524 | --- /dev/null | |||
525 | +++ b/reviewtools/containers/squashfs_container.py | |||
526 | @@ -0,0 +1,111 @@ | |||
527 | 1 | import os | ||
528 | 2 | import shutil | ||
529 | 3 | from reviewtools.common import cmd, cmdIgnoreErrorStrings, UNSQUASHFS_IGNORED_ERRORS | ||
530 | 4 | from reviewtools.containers.base_container import BaseContainer, ContainerException | ||
531 | 5 | |||
532 | 6 | |||
533 | 7 | class SquashfsContainer(BaseContainer): | ||
534 | 8 | """ | ||
535 | 9 | A container is a file that encapsulates a file system. This class extends | ||
536 | 10 | BaseContainer to handle functionality that is specific to Squashfs containers. | ||
537 | 11 | """ | ||
538 | 12 | |||
539 | 13 | def __init__(self, fn): | ||
540 | 14 | """ | ||
541 | 15 | If SquashfsContainer is initialized with a container file, it will also | ||
542 | 16 | verify that the provided file is actually a squashfs, in addition to the | ||
543 | 17 | checks performed by the BaseContainer. | ||
544 | 18 | """ | ||
545 | 19 | super().__init__(fn) | ||
546 | 20 | |||
547 | 21 | # Check is squash file | ||
548 | 22 | self.check_container_format() | ||
549 | 23 | |||
550 | 24 | def check_container_format(self): | ||
551 | 25 | """ | ||
552 | 26 | Checks that the container file has the expected format. | ||
553 | 27 | |||
554 | 28 | Raises: | ||
555 | 29 | ContainerException: This method will raise an exception if the | ||
556 | 30 | container file is not a squashfs container. | ||
557 | 31 | """ | ||
558 | 32 | with open(self.filename, "rb") as f: | ||
559 | 33 | header = f.read(10) | ||
560 | 34 | if not header.startswith(b"hsqs"): | ||
561 | 35 | raise ContainerException("Unsupported package format (not squashfs)") | ||
562 | 36 | |||
563 | 37 | def calculate_unpacked_size(self) -> int: | ||
564 | 38 | """ | ||
565 | 39 | Estimates the space that the unpacked directory will use by adding the | ||
566 | 40 | individual files size reported by "unsquashfs -lln" command. It may not | ||
567 | 41 | be completely accurate, as this command does not consider that each folder | ||
568 | 42 | requires at least one complete block | ||
569 | 43 | |||
570 | 44 | Returns: | ||
571 | 45 | int: Estimated size in bytes of the unpacked directory | ||
572 | 46 | """ | ||
573 | 47 | (rc, out) = cmd(["unsquashfs", "-lln", self.filename]) | ||
574 | 48 | if rc != 0: | ||
575 | 49 | raise RuntimeError("unsquashfs -lln '%s' failed: %s" % (self.filename, out)) | ||
576 | 50 | |||
577 | 51 | size = 0 | ||
578 | 52 | for line in out.splitlines(): | ||
579 | 53 | if not line.startswith("-"): # skip non-regular files | ||
580 | 54 | continue | ||
581 | 55 | try: | ||
582 | 56 | size += int(line.split()[2]) | ||
583 | 57 | except ValueError: # skip non-numbers | ||
584 | 58 | continue | ||
585 | 59 | |||
586 | 60 | return size | ||
587 | 61 | |||
588 | 62 | def unpack(self, items: list = [], force: bool = False) -> str: | ||
589 | 63 | """ | ||
590 | 64 | Extracts the content of the container file and returns the absolute path to | ||
591 | 65 | the newly unpacked directory. | ||
592 | 66 | |||
593 | 67 | Args: | ||
594 | 68 | force (bool, optional): If ``True``, the unpack_dir will be overwritten | ||
595 | 69 | if it already exists. Defaults to ``False`` | ||
596 | 70 | |||
597 | 71 | Returns: | ||
598 | 72 | str: absolute path to the newly created unpack dir | ||
599 | 73 | |||
600 | 74 | Raises: | ||
601 | 75 | ContainerException: This method will raise an ContainerException if the | ||
602 | 76 | unpack_dir already exists and this method was called without the | ||
603 | 77 | force argument set to True | ||
604 | 78 | RuntimeError: This method will raise a RuntimeError under the execution | ||
605 | 79 | of the "unsquashfs" command returns with an error code other than 0. | ||
606 | 80 | """ | ||
607 | 81 | if self._unpack_dir is not None and not force: | ||
608 | 82 | raise ContainerException("'%s' exists. Aborting." % self._unpack_dir) | ||
609 | 83 | |||
610 | 84 | # Check disk space and space available in disk | ||
611 | 85 | self.check_unpacked_size() | ||
612 | 86 | self.check_disk_space() | ||
613 | 87 | |||
614 | 88 | # Unpack the container | ||
615 | 89 | stage_dir = os.path.join(self.tmp_dir, "stage") | ||
616 | 90 | cmd = [ | ||
617 | 91 | "unsquashfs", | ||
618 | 92 | "-no-progress", | ||
619 | 93 | "-ignore-errors", | ||
620 | 94 | "-quiet", | ||
621 | 95 | "-f", | ||
622 | 96 | "-d", | ||
623 | 97 | stage_dir, | ||
624 | 98 | self.filename, | ||
625 | 99 | ] + items | ||
626 | 100 | (rc, out) = cmdIgnoreErrorStrings(cmd, UNSQUASHFS_IGNORED_ERRORS) | ||
627 | 101 | if rc != 0: | ||
628 | 102 | if os.path.isdir(stage_dir): | ||
629 | 103 | shutil.rmtree(stage_dir) | ||
630 | 104 | raise RuntimeError("unpacking failed with '%d':\n%s" % (rc, out)) | ||
631 | 105 | |||
632 | 106 | # Update on success | ||
633 | 107 | if self._unpack_dir is not None and os.path.exists(self._unpack_dir): | ||
634 | 108 | shutil.rmtree(self._unpack_dir) | ||
635 | 109 | self._unpack_dir = ".".join(self.filename.split(".")[:-1]) | ||
636 | 110 | shutil.move(stage_dir, self._unpack_dir) | ||
637 | 111 | return self._unpack_dir | ||
638 | diff --git a/reviewtools/sr_common.py b/reviewtools/sr_common.py | |||
639 | index 16790cc..9b961e2 100644 | |||
640 | --- a/reviewtools/sr_common.py | |||
641 | +++ b/reviewtools/sr_common.py | |||
642 | @@ -30,7 +30,7 @@ from reviewtools.common import ( | |||
643 | 30 | unsquashfs_lln_parse, | 30 | unsquashfs_lln_parse, |
644 | 31 | verify_type, | 31 | verify_type, |
645 | 32 | ) | 32 | ) |
647 | 33 | from reviewtools.containers.base_container import BaseContainer, ContainerException | 33 | from reviewtools.containers.snap_container import SnapContainer, ContainerException |
648 | 34 | from reviewtools.overrides import interfaces_attribs_addons | 34 | from reviewtools.overrides import interfaces_attribs_addons |
649 | 35 | 35 | ||
650 | 36 | 36 | ||
651 | @@ -398,7 +398,7 @@ class SnapReview(ReviewBase): | |||
652 | 398 | ReviewBase.__init__(self, review_type, overrides=overrides) | 398 | ReviewBase.__init__(self, review_type, overrides=overrides) |
653 | 399 | 399 | ||
654 | 400 | try: | 400 | try: |
656 | 401 | self.snap = BaseContainer(fn) | 401 | self.snap = SnapContainer(fn) |
657 | 402 | except ContainerException as e: | 402 | except ContainerException as e: |
658 | 403 | error(str(e)) | 403 | error(str(e)) |
659 | 404 | # TODO: update code to directly use self.snap.xxx instead of assigning here | 404 | # TODO: update code to directly use self.snap.xxx instead of assigning here |
660 | diff --git a/reviewtools/sr_tests.py b/reviewtools/sr_tests.py | |||
661 | index 6caae54..bfbf375 100644 | |||
662 | --- a/reviewtools/sr_tests.py | |||
663 | +++ b/reviewtools/sr_tests.py | |||
664 | @@ -87,7 +87,7 @@ def create_patches(): | |||
665 | 87 | # child. | 87 | # child. |
666 | 88 | patches = [] | 88 | patches = [] |
667 | 89 | patches.append( | 89 | patches.append( |
669 | 90 | patch("reviewtools.sr_common.BaseContainer.__init__", _BaseContainer.__init__) | 90 | patch("reviewtools.sr_common.SnapContainer.__init__", _BaseContainer.__init__) |
670 | 91 | ) | 91 | ) |
671 | 92 | patches.append( | 92 | patches.append( |
672 | 93 | patch("reviewtools.sr_common.SnapReview._extract_snap_yaml", _extract_snap_yaml) | 93 | patch("reviewtools.sr_common.SnapReview._extract_snap_yaml", _extract_snap_yaml) |
673 | @@ -98,7 +98,6 @@ def create_patches(): | |||
674 | 98 | _extract_snap_manifest_yaml, | 98 | _extract_snap_manifest_yaml, |
675 | 99 | ) | 99 | ) |
676 | 100 | ) | 100 | ) |
677 | 101 | patches.append(patch("reviewtools.common.unpack_pkg", _mock_func)) | ||
678 | 102 | # sr_common | 101 | # sr_common |
679 | 103 | patches.append( | 102 | patches.append( |
680 | 104 | patch("reviewtools.sr_common.SnapReview._get_unpack_dir", __get_unpack_dir) | 103 | patch("reviewtools.sr_common.SnapReview._get_unpack_dir", __get_unpack_dir) |
681 | diff --git a/reviewtools/tests/containers/test_base_container.py b/reviewtools/tests/containers/test_base_container.py | |||
682 | index 740e942..1b4e022 100644 | |||
683 | --- a/reviewtools/tests/containers/test_base_container.py | |||
684 | +++ b/reviewtools/tests/containers/test_base_container.py | |||
685 | @@ -3,29 +3,43 @@ import shutil | |||
686 | 3 | import unittest | 3 | import unittest |
687 | 4 | from tempfile import mkdtemp | 4 | from tempfile import mkdtemp |
688 | 5 | from reviewtools.containers.base_container import BaseContainer, ContainerException | 5 | from reviewtools.containers.base_container import BaseContainer, ContainerException |
689 | 6 | from reviewtools.tests.utils import make_snap2 | ||
690 | 7 | 6 | ||
691 | 8 | 7 | ||
692 | 9 | class TestBaseContainer(unittest.TestCase): | 8 | class TestBaseContainer(unittest.TestCase): |
693 | 10 | """Tests for reviewtools.containers.base_container.BaseContainer""" | 9 | """Tests for reviewtools.containers.base_container.BaseContainer""" |
694 | 11 | 10 | ||
697 | 12 | expected_files = ["meta/snap.yaml", "meta/icon.png", "nonexecstack.bin"] | 11 | expected_files = ["meta/snap.yaml", "meta/icon.png", "bin/ls"] |
698 | 13 | expected_binaries = ["nonexecstack.bin"] | 12 | expected_binaries = ["bin/ls"] |
699 | 14 | 13 | ||
700 | 15 | @classmethod | 14 | @classmethod |
701 | 16 | def setUpClass(cls): | 15 | def setUpClass(cls): |
702 | 17 | cls.tmp_dir = mkdtemp() | 16 | cls.tmp_dir = mkdtemp() |
708 | 18 | cls.fn = make_snap2( | 17 | cls.fn = cls._create_container(cls) |
709 | 19 | output_dir=cls.tmp_dir, | 18 | cls.unpack_dir = cls._create_unpackdir(cls) |
705 | 20 | yaml="Test", | ||
706 | 21 | extra_files=["/bin/ls:nonexecstack.bin"], | ||
707 | 22 | ) | ||
710 | 23 | 19 | ||
711 | 24 | @classmethod | 20 | @classmethod |
712 | 25 | def tearDownClass(cls): | 21 | def tearDownClass(cls): |
713 | 26 | if os.path.exists(cls.tmp_dir): | 22 | if os.path.exists(cls.tmp_dir): |
714 | 27 | shutil.rmtree(cls.tmp_dir) | 23 | shutil.rmtree(cls.tmp_dir) |
715 | 28 | 24 | ||
716 | 25 | def _create_container(self, container_size: int = 10000): | ||
717 | 26 | fn = os.path.join(self.tmp_dir, "container.test") | ||
718 | 27 | with open(fn, "w") as f: | ||
719 | 28 | f.truncate(container_size) | ||
720 | 29 | return fn | ||
721 | 30 | |||
722 | 31 | def _create_unpackdir(self): | ||
723 | 32 | unpack_dir = os.path.join(self.tmp_dir, "unpack_dir") | ||
724 | 33 | for file in self.expected_files: | ||
725 | 34 | unpacked_file = os.path.join(unpack_dir, file) | ||
726 | 35 | os.makedirs(os.path.dirname(unpacked_file), exist_ok=True) | ||
727 | 36 | if file not in self.expected_binaries: | ||
728 | 37 | with open(unpacked_file, "w+b") as fd: | ||
729 | 38 | fd.write(b"Test") | ||
730 | 39 | else: | ||
731 | 40 | shutil.copyfile(os.path.join("/", file), unpacked_file) | ||
732 | 41 | return unpack_dir | ||
733 | 42 | |||
734 | 29 | # Test class initialization | 43 | # Test class initialization |
735 | 30 | def test_initialization_container__happy(self): | 44 | def test_initialization_container__happy(self): |
736 | 31 | try: | 45 | try: |
737 | @@ -43,10 +57,75 @@ class TestBaseContainer(unittest.TestCase): | |||
738 | 43 | in context.exception.value | 57 | in context.exception.value |
739 | 44 | ) | 58 | ) |
740 | 45 | 59 | ||
741 | 60 | # Test deletion | ||
742 | 61 | def test_check_cleanup(self): | ||
743 | 62 | try: | ||
744 | 63 | container = BaseContainer(self.fn) | ||
745 | 64 | tmp_dir = container.tmp_dir | ||
746 | 65 | del container | ||
747 | 66 | self.assertFalse(os.path.exists(tmp_dir)) | ||
748 | 67 | except Exception: | ||
749 | 68 | self.fail("An unexpected error occurred during deletion test") | ||
750 | 69 | |||
751 | 70 | # Test check container size | ||
752 | 71 | def test_check_container_size__happy(self): | ||
753 | 72 | container = BaseContainer(self.fn) | ||
754 | 73 | container._min_packed_size = 0 | ||
755 | 74 | container._max_packed_size = 10**6 | ||
756 | 75 | try: | ||
757 | 76 | container.check_container_size() | ||
758 | 77 | except ContainerException: | ||
759 | 78 | self.fail("An unexpected error occurred while checking file size") | ||
760 | 79 | |||
761 | 80 | def test_check_container_size__too_big(self): | ||
762 | 81 | container = BaseContainer(self.fn) | ||
763 | 82 | container._max_packed_size = 1 | ||
764 | 83 | with self.assertRaises(ContainerException) as context: | ||
765 | 84 | container.check_container_size() | ||
766 | 85 | self.assertTrue("container file is too large " in context.exception.value) | ||
767 | 86 | |||
768 | 87 | def test_check_container_size__too_small(self): | ||
769 | 88 | container = BaseContainer(self.fn) | ||
770 | 89 | container._min_packed_size = 10**6 | ||
771 | 90 | with self.assertRaises(ContainerException) as context: | ||
772 | 91 | container.check_container_size() | ||
773 | 92 | self.assertTrue("container file is too small " in context.exception.value) | ||
774 | 93 | |||
775 | 94 | # Test check unpacked size | ||
776 | 95 | def _mock_calculate_unpacked_size(self): | ||
777 | 96 | return 10**3 | ||
778 | 97 | |||
779 | 98 | def test_check_unpacked_size__happy(self): | ||
780 | 99 | container = BaseContainer(self.fn) | ||
781 | 100 | container.calculate_unpacked_size = self._mock_calculate_unpacked_size | ||
782 | 101 | container._min_unpacked_size = 0 | ||
783 | 102 | container._max_unpacked_size = 10**6 | ||
784 | 103 | try: | ||
785 | 104 | container.check_unpacked_size() | ||
786 | 105 | except ContainerException: | ||
787 | 106 | self.fail("An unexpected error occurred while checking file size") | ||
788 | 107 | |||
789 | 108 | def test_check_unpacked_size__too_big(self): | ||
790 | 109 | container = BaseContainer(self.fn) | ||
791 | 110 | container.calculate_unpacked_size = self._mock_calculate_unpacked_size | ||
792 | 111 | container._max_unpacked_size = 1 | ||
793 | 112 | with self.assertRaises(ContainerException) as context: | ||
794 | 113 | container.check_unpacked_size() | ||
795 | 114 | self.assertTrue("container file is too large " in context.exception.value) | ||
796 | 115 | |||
797 | 116 | def test_check_unpacked_size__too_small(self): | ||
798 | 117 | container = BaseContainer(self.fn) | ||
799 | 118 | container.calculate_unpacked_size = self._mock_calculate_unpacked_size | ||
800 | 119 | container._min_unpacked_size = 10**6 | ||
801 | 120 | with self.assertRaises(ContainerException) as context: | ||
802 | 121 | container.check_unpacked_size() | ||
803 | 122 | self.assertTrue("container file is too small " in context.exception.value) | ||
804 | 123 | |||
805 | 46 | # Test get file | 124 | # Test get file |
806 | 47 | def test_get_file__happy(self): | 125 | def test_get_file__happy(self): |
807 | 48 | try: | 126 | try: |
808 | 49 | container = BaseContainer(self.fn) | 127 | container = BaseContainer(self.fn) |
809 | 128 | container._unpack_dir = self.unpack_dir | ||
810 | 50 | snap_yaml = container.get_file("meta/snap.yaml") | 129 | snap_yaml = container.get_file("meta/snap.yaml") |
811 | 51 | self.assertEqual(snap_yaml.decode(), "Test") | 130 | self.assertEqual(snap_yaml.decode(), "Test") |
812 | 52 | except ContainerException: | 131 | except ContainerException: |
813 | @@ -55,6 +134,7 @@ class TestBaseContainer(unittest.TestCase): | |||
814 | 55 | def test_get_file__missing_file(self): | 134 | def test_get_file__missing_file(self): |
815 | 56 | with self.assertRaises(ContainerException) as context: | 135 | with self.assertRaises(ContainerException) as context: |
816 | 57 | container = BaseContainer(self.fn) | 136 | container = BaseContainer(self.fn) |
817 | 137 | container._unpack_dir = self.unpack_dir | ||
818 | 58 | container.get_file("non/existent/file") | 138 | container.get_file("non/existent/file") |
819 | 59 | self.assertTrue("Could not find" in context.exception.value) | 139 | self.assertTrue("Could not find" in context.exception.value) |
820 | 60 | 140 | ||
821 | @@ -62,6 +142,7 @@ class TestBaseContainer(unittest.TestCase): | |||
822 | 62 | def test_get_files_list__happy(self): | 142 | def test_get_files_list__happy(self): |
823 | 63 | try: | 143 | try: |
824 | 64 | container = BaseContainer(self.fn) | 144 | container = BaseContainer(self.fn) |
825 | 145 | container._unpack_dir = self.unpack_dir | ||
826 | 65 | self.assertCountEqual( | 146 | self.assertCountEqual( |
827 | 66 | container.get_files_list(), | 147 | container.get_files_list(), |
828 | 67 | self.expected_files, | 148 | self.expected_files, |
829 | @@ -72,6 +153,7 @@ class TestBaseContainer(unittest.TestCase): | |||
830 | 72 | def test_get_files_list_abs__happy(self): | 153 | def test_get_files_list_abs__happy(self): |
831 | 73 | try: | 154 | try: |
832 | 74 | container = BaseContainer(self.fn) | 155 | container = BaseContainer(self.fn) |
833 | 156 | container._unpack_dir = self.unpack_dir | ||
834 | 75 | self.assertCountEqual( | 157 | self.assertCountEqual( |
835 | 76 | container.get_files_list(abs=True), | 158 | container.get_files_list(abs=True), |
836 | 77 | [ | 159 | [ |
837 | @@ -85,6 +167,7 @@ class TestBaseContainer(unittest.TestCase): | |||
838 | 85 | def test_files__happy(self): | 167 | def test_files__happy(self): |
839 | 86 | try: | 168 | try: |
840 | 87 | container = BaseContainer(self.fn) | 169 | container = BaseContainer(self.fn) |
841 | 170 | container._unpack_dir = self.unpack_dir | ||
842 | 88 | self.assertCountEqual( | 171 | self.assertCountEqual( |
843 | 89 | container.files, | 172 | container.files, |
844 | 90 | self.expected_files, | 173 | self.expected_files, |
845 | @@ -96,6 +179,7 @@ class TestBaseContainer(unittest.TestCase): | |||
846 | 96 | def test_get_compiled_binaries_list__happy(self): | 179 | def test_get_compiled_binaries_list__happy(self): |
847 | 97 | try: | 180 | try: |
848 | 98 | container = BaseContainer(self.fn) | 181 | container = BaseContainer(self.fn) |
849 | 182 | container._unpack_dir = self.unpack_dir | ||
850 | 99 | self.assertCountEqual( | 183 | self.assertCountEqual( |
851 | 100 | container.get_compiled_binaries_list(), self.expected_binaries | 184 | container.get_compiled_binaries_list(), self.expected_binaries |
852 | 101 | ) | 185 | ) |
853 | @@ -107,6 +191,7 @@ class TestBaseContainer(unittest.TestCase): | |||
854 | 107 | def test_get_compiled_binaries_list_abs__happy(self): | 191 | def test_get_compiled_binaries_list_abs__happy(self): |
855 | 108 | try: | 192 | try: |
856 | 109 | container = BaseContainer(self.fn) | 193 | container = BaseContainer(self.fn) |
857 | 194 | container._unpack_dir = self.unpack_dir | ||
858 | 110 | self.assertCountEqual( | 195 | self.assertCountEqual( |
859 | 111 | container.get_compiled_binaries_list(abs=True), | 196 | container.get_compiled_binaries_list(abs=True), |
860 | 112 | [ | 197 | [ |
861 | @@ -122,6 +207,7 @@ class TestBaseContainer(unittest.TestCase): | |||
862 | 122 | def test_bin_files__happy(self): | 207 | def test_bin_files__happy(self): |
863 | 123 | try: | 208 | try: |
864 | 124 | container = BaseContainer(self.fn) | 209 | container = BaseContainer(self.fn) |
865 | 210 | container._unpack_dir = self.unpack_dir | ||
866 | 125 | self.assertCountEqual(container.bin_files, self.expected_binaries) | 211 | self.assertCountEqual(container.bin_files, self.expected_binaries) |
867 | 126 | except ContainerException: | 212 | except ContainerException: |
868 | 127 | self.fail( | 213 | self.fail( |
869 | diff --git a/reviewtools/tests/containers/test_squashfs_container.py b/reviewtools/tests/containers/test_squashfs_container.py | |||
870 | 128 | new file mode 100644 | 214 | new file mode 100644 |
871 | index 0000000..7d19a77 | |||
872 | --- /dev/null | |||
873 | +++ b/reviewtools/tests/containers/test_squashfs_container.py | |||
874 | @@ -0,0 +1,73 @@ | |||
875 | 1 | import os | ||
876 | 2 | import unittest | ||
877 | 3 | from tempfile import mkstemp, mkdtemp | ||
878 | 4 | from reviewtools.containers.squashfs_container import ( | ||
879 | 5 | SquashfsContainer, | ||
880 | 6 | ContainerException, | ||
881 | 7 | ) | ||
882 | 8 | from reviewtools.tests.utils import make_snap2 | ||
883 | 9 | |||
884 | 10 | |||
885 | 11 | class TestSquashfsContainer(unittest.TestCase): | ||
886 | 12 | """Tests for reviewtools.containers.squashfs_container""" | ||
887 | 13 | |||
888 | 14 | @classmethod | ||
889 | 15 | def setUpClass(cls): | ||
890 | 16 | cls.tmp_dir = mkdtemp() | ||
891 | 17 | cls.fn = make_snap2( | ||
892 | 18 | output_dir=cls.tmp_dir, | ||
893 | 19 | yaml="Test", | ||
894 | 20 | extra_files=["/bin/ls:bin/ls"], | ||
895 | 21 | ) | ||
896 | 22 | |||
897 | 23 | # Test initialization | ||
898 | 24 | def test_check_initialization__happy(self): | ||
899 | 25 | try: | ||
900 | 26 | SquashfsContainer(self.fn) | ||
901 | 27 | except Exception: | ||
902 | 28 | self.fail( | ||
903 | 29 | "An unexpected error occurred during SquashfsContainer initialization with container file" | ||
904 | 30 | ) | ||
905 | 31 | |||
906 | 32 | def test_check_initialization__invalid_format(self): | ||
907 | 33 | with self.assertRaises(ContainerException) as context: | ||
908 | 34 | fd, plain_file = mkstemp() | ||
909 | 35 | SquashfsContainer(plain_file) | ||
910 | 36 | self.assertEqual( | ||
911 | 37 | "unsupported package format (not squashfs)", context.exception.value | ||
912 | 38 | ) | ||
913 | 39 | os.unlink(plain_file) | ||
914 | 40 | |||
915 | 41 | # Test check format | ||
916 | 42 | def test_calculate_unpacked_size__happy(self): | ||
917 | 43 | try: | ||
918 | 44 | # unpacked size calculated is a bit smaller than actual size as it does not consider that | ||
919 | 45 | # folders require at least one complete block | ||
920 | 46 | container = SquashfsContainer(self.fn) | ||
921 | 47 | size = container.calculate_unpacked_size() | ||
922 | 48 | self.assertTrue(isinstance(size, int)) | ||
923 | 49 | except Exception: | ||
924 | 50 | self.fail("An unexpected error occurred during unpacked size calculation") | ||
925 | 51 | |||
926 | 52 | # Test unpack | ||
927 | 53 | def test_unpack__happy(self): | ||
928 | 54 | try: | ||
929 | 55 | container = SquashfsContainer(self.fn) | ||
930 | 56 | container.unpack() | ||
931 | 57 | except Exception: | ||
932 | 58 | self.fail("An unexpected error occurred during unpack") | ||
933 | 59 | |||
934 | 60 | def test_unpack__force(self): | ||
935 | 61 | try: | ||
936 | 62 | container = SquashfsContainer(self.fn) | ||
937 | 63 | container.unpack() | ||
938 | 64 | container.unpack(force=True) | ||
939 | 65 | except Exception: | ||
940 | 66 | self.fail("An unexpected error occurred during unpack") | ||
941 | 67 | |||
942 | 68 | def test_unpack__no_force(self): | ||
943 | 69 | with self.assertRaises(ContainerException) as context: | ||
944 | 70 | container = SquashfsContainer(self.fn) | ||
945 | 71 | container.unpack() | ||
946 | 72 | container.unpack() | ||
947 | 73 | self.assertTrue(" exists. Aborting." in context.exception.value) |
LGTM, but can you please update the commit description to also describe how and why you are removing the 3 entries from magic_binary_ file_descriptio ns as well? Thanks. A commit message more like
rt/c/base_ container: use magic compat API
...
would perhaps make more sense here.