From 49315adf792db51a2f545432659195a4ba3b748d Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Mon, 28 Nov 2016 19:49:50 +0100 Subject: [PATCH] Do not recurse scan for images in standard image directory Fix https://github.com/GNS3/gns3-gui/issues/1680 --- gns3server/compute/base_manager.py | 21 +--- gns3server/compute/dynamips/__init__.py | 41 -------- gns3server/compute/iou/__init__.py | 6 -- gns3server/compute/qemu/__init__.py | 6 -- gns3server/controller/compute.py | 18 +--- gns3server/controller/topology.py | 1 + gns3server/utils/images.py | 103 ++++++++++++++++---- tests/compute/test_manager.py | 21 ++-- tests/controller/test_compute.py | 2 +- tests/handlers/api/compute/test_dynamips.py | 23 +++-- tests/utils/test_images.py | 49 ++++++++-- 11 files changed, 156 insertions(+), 135 deletions(-) diff --git a/gns3server/compute/base_manager.py b/gns3server/compute/base_manager.py index db10d5b2..0ede64d9 100644 --- a/gns3server/compute/base_manager.py +++ b/gns3server/compute/base_manager.py @@ -39,7 +39,7 @@ from .port_manager import PortManager from .nios.nio_udp import NIOUDP from .nios.nio_tap import NIOTAP from .nios.nio_ethernet import NIOEthernet -from ..utils.images import md5sum, remove_checksum, images_directories +from ..utils.images import md5sum, remove_checksum, images_directories, default_images_directory, list_images from .error import NodeError, ImageMissingError @@ -477,27 +477,14 @@ class BaseManager: :returns: Array of hash """ - images = [] - img_dir = self.get_images_directory() - for root, dirs, files in os.walk(img_dir): - for filename in files: - if filename[0] != "." and not filename.endswith(".md5sum"): - path = os.path.relpath(os.path.join(root, filename), img_dir) - try: - images.append({ - "filename": filename, - "path": path, - "md5sum": md5sum(os.path.join(root, filename)), - "filesize": os.stat(os.path.join(root, filename)).st_size}) - except OSError as e: - log.warn("Can't add image {}: {}".format(path, str(e))) - return images + return list_images(self._NODE_TYPE) def get_images_directory(self): """ Get the image directory on disk """ - + if hasattr(self, "_NODE_TYPE"): + return default_images_directory(self._NODE_TYPE) raise NotImplementedError @asyncio.coroutine diff --git a/gns3server/compute/dynamips/__init__.py b/gns3server/compute/dynamips/__init__.py index 36f98008..4b05661d 100644 --- a/gns3server/compute/dynamips/__init__.py +++ b/gns3server/compute/dynamips/__init__.py @@ -678,44 +678,3 @@ class Dynamips(BaseManager): if was_auto_started: yield from vm.stop() return validated_idlepc - - def get_images_directory(self): - """ - Return the full path of the images directory on disk - """ - return os.path.join(os.path.expanduser(self.config.get_section_config("Server").get("images_path", "~/GNS3/images")), "IOS") - - @asyncio.coroutine - def list_images(self): - """ - Return the list of available IOS images. - - :returns: Array of hash - """ - - image_dir = self.get_images_directory() - if not os.path.exists(image_dir): - return [] - try: - files = os.listdir(image_dir) - except OSError as e: - raise DynamipsError("Can not list {}: {}".format(image_dir, str(e))) - files.sort() - images = [] - for filename in files: - if filename[0] != "." and not filename.endswith(".md5sum"): - try: - path = os.path.join(image_dir, filename) - with open(path, "rb") as f: - # read the first 7 bytes of the file. - elf_header_start = f.read(7) - except OSError as e: - continue - # valid IOS images must start with the ELF magic number, be 32-bit, big endian and have an ELF version of 1 - if elf_header_start == b'\x7fELF\x01\x02\x01': - images.append({"filename": filename, - "path": os.path.relpath(path, image_dir), - "md5sum": md5sum(path), - "filesize": os.stat(path).st_size - }) - return images diff --git a/gns3server/compute/iou/__init__.py b/gns3server/compute/iou/__init__.py index d101e0d1..acc994df 100644 --- a/gns3server/compute/iou/__init__.py +++ b/gns3server/compute/iou/__init__.py @@ -95,9 +95,3 @@ class IOU(BaseManager): """ return os.path.join("iou", "device-{}".format(legacy_vm_id)) - - def get_images_directory(self): - """ - Return the full path of the images directory on disk - """ - return os.path.join(os.path.expanduser(self.config.get_section_config("Server").get("images_path", "~/GNS3/images")), "IOU") diff --git a/gns3server/compute/qemu/__init__.py b/gns3server/compute/qemu/__init__.py index 68dd0020..2bf9f04b 100644 --- a/gns3server/compute/qemu/__init__.py +++ b/gns3server/compute/qemu/__init__.py @@ -229,12 +229,6 @@ class Qemu(BaseManager): return os.path.join("qemu", "vm-{}".format(legacy_vm_id)) - def get_images_directory(self): - """ - Return the full path of the images directory on disk - """ - return os.path.join(os.path.expanduser(self.config.get_section_config("Server").get("images_path", "~/GNS3/images")), "QEMU") - @asyncio.coroutine def create_disk(self, qemu_img, path, options): """ diff --git a/gns3server/controller/compute.py b/gns3server/controller/compute.py index 7d42c0a7..f3aa4b49 100644 --- a/gns3server/controller/compute.py +++ b/gns3server/controller/compute.py @@ -26,7 +26,7 @@ import os import io from ..utils import parse_version -from ..utils.images import scan_for_images, md5sum +from ..utils.images import list_images, md5sum from ..utils.asyncio import locked_coroutine from ..controller.controller_error import ControllerError from ..config import Config @@ -563,18 +563,10 @@ class Compute: res = yield from self.http_query("GET", "/{}/images".format(type), timeout=None) images = res.json - try: - if type in ["qemu", "dynamips", "iou"]: - for path in scan_for_images(type): - image = os.path.basename(path) - if image not in [i['filename'] for i in images]: - images.append({"filename": image, - "path": image, - "md5sum": md5sum(path), - "filesize": os.stat(path).st_size - }) - except OSError as e: - raise aiohttp.web.HTTPConflict(text="Can't scan for images: {}".format(str(e))) + if type in ["qemu", "dynamips", "iou"]: + for local_image in list_images(type): + if local_image['filename'] not in [i['filename'] for i in images]: + images.append(local_image) return images @asyncio.coroutine diff --git a/gns3server/controller/topology.py b/gns3server/controller/topology.py index 43877d9d..84bdc4c2 100644 --- a/gns3server/controller/topology.py +++ b/gns3server/controller/topology.py @@ -22,6 +22,7 @@ import uuid import shutil import zipfile import aiohttp +import platform import jsonschema diff --git a/gns3server/utils/images.py b/gns3server/utils/images.py index ce64fe40..03bb3d16 100644 --- a/gns3server/utils/images.py +++ b/gns3server/utils/images.py @@ -26,28 +26,96 @@ import logging log = logging.getLogger(__name__) -def scan_for_images(type): +def list_images(type): """ Scan directories for available image for a type :param type: emulator type (dynamips, qemu, iou) """ files = set() - paths = [] + images = [] + + server_config = Config.instance().get_section_config("Server") + general_images_directory = os.path.expanduser(server_config.get("images_path", "~/GNS3/images")) + + # Subfolder of the general_images_directory specific to this VM type + default_directory = default_images_directory(type) + for directory in images_directories(type): + + # We limit recursion to path outside the default images directory + # the reason is in the default directory manage file organization and + # it should be flatten to keep things simple + recurse = True + if os.path.commonprefix([directory, general_images_directory]) == general_images_directory: + recurse = False + directory = os.path.normpath(directory) - for root, _, filenames in os.walk(directory): - for file in filenames: - path = os.path.join(root, file) - if file not in files: - if file.endswith(".md5sum") or file.startswith("."): + for root, _, filenames in _os_walk(directory, recurse=recurse): + for filename in filenames: + path = os.path.join(root, filename) + if filename not in files: + if filename.endswith(".md5sum") or filename.startswith("."): continue - elif (file.endswith(".image") and type == "dynamips") \ - or (file.endswith(".bin") and type == "iou") \ - or (not file.endswith(".bin") and not file.endswith(".image") and type == "qemu"): - files.add(file) - paths.append(force_unix_path(path)) - return paths + elif ((filename.endswith(".image") or filename.endswith(".bin")) and type == "dynamips") \ + or (filename.endswith(".bin") and type == "iou") \ + or (not filename.endswith(".bin") and not filename.endswith(".image") and type == "qemu"): + files.add(filename) + + # It the image is located in the standard directory the path is relative + if os.path.commonprefix([root, default_directory]) != default_directory: + path = os.path.join(root, filename) + else: + path = os.path.relpath(os.path.join(root, filename), default_directory) + + try: + if type in ["dynamips", "iou"]: + with open(os.path.join(root, filename), "rb") as f: + # read the first 7 bytes of the file. + elf_header_start = f.read(7) + # valid IOS images must start with the ELF magic number, be 32-bit, big endian and have an ELF version of 1 + if not elf_header_start == b'\x7fELF\x01\x02\x01' and not elf_header_start == b'\x7fELF\x01\x01\x01': + continue + + images.append({ + "filename": filename, + "path": path, + "md5sum": md5sum(os.path.join(root, filename)), + "filesize": os.stat(os.path.join(root, filename)).st_size}) + except OSError as e: + log.warn("Can't add image {}: {}".format(path, str(e))) + return images + + +def _os_walk(directory, recurse=True, **kwargs): + """ + Work like os.walk but if recurse is False just list current directory + """ + if recurse: + for root, dirs, files in os.walk(directory, **kwargs): + yield root, dirs, files + else: + files = [] + for filename in os.listdir(directory): + if os.path.isfile(os.path.join(directory, filename)): + files.append(filename) + yield directory, [], files + + +def default_images_directory(type): + """ + :returns: Return the default directory for a node type + """ + server_config = Config.instance().get_section_config("Server") + img_dir = os.path.expanduser(server_config.get("images_path", "~/GNS3/images")) + if type == "qemu": + return os.path.join(img_dir, "QEMU") + elif type == "iou": + return os.path.join(img_dir, "IOU") + elif type == "dynamips": + return os.path.join(img_dir, "IOS") + else: + raise NotImplementedError("%s node type is not supported", type) def images_directories(type): @@ -61,14 +129,7 @@ def images_directories(type): paths = [] img_dir = os.path.expanduser(server_config.get("images_path", "~/GNS3/images")) - if type == "qemu": - type_img_directory = os.path.join(img_dir, "QEMU") - elif type == "iou": - type_img_directory = os.path.join(img_dir, "IOU") - elif type == "dynamips": - type_img_directory = os.path.join(img_dir, "IOS") - else: - raise NotImplementedError("%s is not supported", type) + type_img_directory = default_images_directory(type) os.makedirs(type_img_directory, exist_ok=True) paths.append(type_img_directory) for directory in server_config.get("additional_images_path", "").split(";"): diff --git a/tests/compute/test_manager.py b/tests/compute/test_manager.py index 4a5b6402..43cf48c2 100644 --- a/tests/compute/test_manager.py +++ b/tests/compute/test_manager.py @@ -222,35 +222,36 @@ def test_get_relative_image_path_ova(qemu, tmpdir, config): def test_list_images(loop, qemu, tmpdir): - fake_images = ["a.bin", "b.bin", ".blu.bin", "a.bin.md5sum"] + fake_images = ["a.qcow2", "b.qcow2", ".blu.qcow2", "a.qcow2.md5sum"] for image in fake_images: with open(str(tmpdir / image), "w+") as f: f.write("1") - with patch("gns3server.compute.Qemu.get_images_directory", return_value=str(tmpdir)): + with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)): assert loop.run_until_complete(qemu.list_images()) == [ - {"filename": "a.bin", "path": "a.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}, - {"filename": "b.bin", "path": "b.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1} + {"filename": "a.qcow2", "path": "a.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}, + {"filename": "b.qcow2", "path": "b.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1} ] def test_list_images_recursives(loop, qemu, tmpdir): - fake_images = ["a.bin", "b.bin", ".blu.bin", "a.bin.md5sum"] + fake_images = ["a.qcow2", "b.qcow2", ".blu.qcow2", "a.qcow2.md5sum"] for image in fake_images: with open(str(tmpdir / image), "w+") as f: f.write("1") os.makedirs(str(tmpdir / "c")) - fake_images = ["c.bin", "c.bin.md5sum"] + fake_images = ["c.qcow2", "c.qcow2.md5sum"] for image in fake_images: with open(str(tmpdir / "c" / image), "w+") as f: f.write("1") - with patch("gns3server.compute.Qemu.get_images_directory", return_value=str(tmpdir)): + with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)): + assert loop.run_until_complete(qemu.list_images()) == [ - {"filename": "a.bin", "path": "a.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}, - {"filename": "b.bin", "path": "b.bin", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}, - {"filename": "c.bin", "path": os.path.sep.join(["c", "c.bin"]), "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1} + {"filename": "a.qcow2", "path": "a.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}, + {"filename": "b.qcow2", "path": "b.qcow2", "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1}, + {"filename": "c.qcow2", "path": os.path.sep.join(["c", "c.qcow2"]), "md5sum": "c4ca4238a0b923820dcc509a6f75849b", "filesize": 1} ] diff --git a/tests/controller/test_compute.py b/tests/controller/test_compute.py index 5af58274..c0f4c134 100644 --- a/tests/controller/test_compute.py +++ b/tests/controller/test_compute.py @@ -355,7 +355,7 @@ def test_images(compute, async_run, images_dir): "path": "linux.qcow2", "md5sum": "d41d8cd98f00b204e9800998ecf8427e", "filesize": 0}]).encode()) - open(os.path.join(images_dir, "asa.qcow2"), "w+").close() + open(os.path.join(images_dir, "QEMU", "asa.qcow2"), "w+").close() with asyncio_patch("aiohttp.ClientSession.request", return_value=response) as mock: images = async_run(compute.images("qemu")) mock.assert_called_with("GET", "https://example.com:84/v2/compute/qemu/images", auth=None, data=None, headers={'content-type': 'application/json'}, chunked=False, timeout=None) diff --git a/tests/handlers/api/compute/test_dynamips.py b/tests/handlers/api/compute/test_dynamips.py index c7666fad..d9d4097f 100644 --- a/tests/handlers/api/compute/test_dynamips.py +++ b/tests/handlers/api/compute/test_dynamips.py @@ -132,7 +132,6 @@ from tests.utils import asyncio_patch @pytest.fixture def fake_dynamips(tmpdir): """Create a fake Dynamips image on disk""" - path = str(tmpdir / "7200.bin") with open(path, "wb+") as f: f.write(b'\x7fELF\x01\x02\x01') @@ -153,7 +152,7 @@ def fake_file(tmpdir): def test_images(http_compute, tmpdir, fake_dynamips, fake_file): - with patch("gns3server.compute.Dynamips.get_images_directory", return_value=str(tmpdir), example=True): + with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)): response = http_compute.get("/dynamips/images") assert response.status == 200 assert response.json == [{"filename": "7200.bin", @@ -163,24 +162,24 @@ def test_images(http_compute, tmpdir, fake_dynamips, fake_file): }] -def test_upload_image(http_compute, tmpdir): - with patch("gns3server.compute.Dynamips.get_images_directory", return_value=str(tmpdir),): - response = http_compute.post("/dynamips/images/test2", body="TEST", raw=True) - assert response.status == 204 +def test_upload_image(http_compute, tmpdir, images_dir): + response = http_compute.post("/dynamips/images/test2", body="TEST", raw=True) + assert response.status == 204 - with open(str(tmpdir / "test2")) as f: + with open(os.path.join(images_dir, "IOS", "test2")) as f: assert f.read() == "TEST" - with open(str(tmpdir / "test2.md5sum")) as f: + with open(os.path.join(images_dir, "IOS", "test2.md5sum")) as f: checksum = f.read() assert checksum == "033bd94b1168d7e4f0d644c3c95e35bf" -def test_upload_image_permission_denied(http_compute, tmpdir): - with open(str(tmpdir / "test2.tmp"), "w+") as f: +def test_upload_image_permission_denied(http_compute, tmpdir, images_dir): + os.makedirs(os.path.join(images_dir, "IOS"), exist_ok=True) + with open(os.path.join(images_dir, "IOS", "test2.tmp"), "w+") as f: f.write("") - os.chmod(str(tmpdir / "test2.tmp"), 0) + os.chmod(os.path.join(images_dir, "IOS", "test2.tmp"), 0) - with patch("gns3server.compute.Dynamips.get_images_directory", return_value=str(tmpdir),): + with patch("gns3server.utils.images.default_images_directory", return_value=str(tmpdir)): response = http_compute.post("/dynamips/images/test2", body="TEST", raw=True) assert response.status == 409 diff --git a/tests/utils/test_images.py b/tests/utils/test_images.py index 42bf6faa..8b9b5c24 100644 --- a/tests/utils/test_images.py +++ b/tests/utils/test_images.py @@ -20,7 +20,7 @@ from unittest.mock import patch from gns3server.utils import force_unix_path -from gns3server.utils.images import md5sum, remove_checksum, images_directories, scan_for_images +from gns3server.utils.images import md5sum, remove_checksum, images_directories, list_images def test_images_directories(tmpdir): @@ -92,17 +92,21 @@ def test_remove_checksum(tmpdir): remove_checksum(str(tmpdir / 'not_exists')) -def test_scan_for_images(tmpdir): +def test_list_images(tmpdir): path1 = tmpdir / "images1" / "IOS" / "test1.image" - path1.write("1", ensure=True) + path1.write(b'\x7fELF\x01\x02\x01', ensure=True) path1 = force_unix_path(str(path1)) path2 = tmpdir / "images2" / "test2.image" - path2.write("1", ensure=True) + path2.write(b'\x7fELF\x01\x02\x01', ensure=True) path2 = force_unix_path(str(path2)) + # Invalid image because not a valid elf file + path = tmpdir / "images2" / "test_invalid.image" + path.write(b'NOTANELF', ensure=True) + path3 = tmpdir / "images1" / "IOU" / "test3.bin" - path3.write("1", ensure=True) + path3.write(b'\x7fELF\x01\x02\x01', ensure=True) path3 = force_unix_path(str(path3)) path4 = tmpdir / "images1" / "QEMU" / "test4.qcow2" @@ -118,6 +122,35 @@ def test_scan_for_images(tmpdir): "additional_images_path": "/tmp/null24564;{}".format(str(tmpdir / "images2")), "local": False}): - assert scan_for_images("dynamips") == [str(path1), str(path2)] - assert scan_for_images("iou") == [str(path3)] - assert scan_for_images("qemu") == [str(path4)] + assert list_images("dynamips") == [ + { + 'filename': 'test1.image', + 'filesize': 7, + 'md5sum': 'b0d5aa897d937aced5a6b1046e8f7e2e', + 'path': 'test1.image' + }, + { + 'filename': 'test2.image', + 'filesize': 7, + 'md5sum': 'b0d5aa897d937aced5a6b1046e8f7e2e', + 'path': str(path2) + } + ] + + assert list_images("iou") == [ + { + 'filename': 'test3.bin', + 'filesize': 7, + 'md5sum': 'b0d5aa897d937aced5a6b1046e8f7e2e', + 'path': 'test3.bin' + } + ] + + assert list_images("qemu") == [ + { + 'filename': 'test4.qcow2', + 'filesize': 1, + 'md5sum': 'c4ca4238a0b923820dcc509a6f75849b', + 'path': 'test4.qcow2' + } + ]