From 1cb433c5bcbc2d7a4ee2070448957319c51eeaa4 Mon Sep 17 00:00:00 2001 From: grossmj Date: Thu, 10 Aug 2023 22:44:37 +1000 Subject: [PATCH 1/8] New packaging relying only pyproject.toml --- .github/workflows/testing.yml | 3 -- AUTHORS | 2 - MANIFEST.in | 8 +--- README.md | 2 +- dev-requirements.txt | 4 +- gns3server/compute/docker/__init__.py | 23 +++++++++ gns3server/compute/docker/docker_vm.py | 5 +- .../config_samples}/gns3_server.conf | 0 gns3server/controller/appliance_manager.py | 5 -- gns3server/server.py | 5 +- pyproject.toml | 15 ++---- requirements.txt | 1 - setup.py | 47 ------------------- 13 files changed, 38 insertions(+), 82 deletions(-) delete mode 100644 AUTHORS rename {conf => gns3server/config_samples}/gns3_server.conf (100%) delete mode 100644 setup.py diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 8456ec06..3e95e800 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -31,9 +31,6 @@ jobs: run: | python -m pip install --upgrade pip python -m pip install -r dev-requirements.txt - - name: Install Windows dependencies - if: matrix.os == 'windows-latest' - run: python -m pip install -r win-requirements.txt - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names diff --git a/AUTHORS b/AUTHORS deleted file mode 100644 index 783e04bb..00000000 --- a/AUTHORS +++ /dev/null @@ -1,2 +0,0 @@ -Jeremy Grossmann -Julien Duponchelle \ No newline at end of file diff --git a/MANIFEST.in b/MANIFEST.in index 6fa63e63..e8286093 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,11 +1,7 @@ include README.md -include AUTHORS include LICENSE -include MANIFEST.in -include requirements.txt -include conf/*.conf -recursive-include tests * -recursive-exclude docs * +include CHANGELOG recursive-include gns3server * +recursive-exclude docs * recursive-exclude * __pycache__ recursive-exclude * *.py[co] diff --git a/README.md b/README.md index 5246d74c..f4761960 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ cd gns3-server git checkout 3.0 python3 -m venv venv-gns3server source venv-gns3server/bin/activate -python3 setup.py install +python3 -m pip install . python3 -m gns3server ``` diff --git a/dev-requirements.txt b/dev-requirements.txt index 3dc8c8d0..5e29ebc7 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,8 +1,6 @@ --r requirements.txt - pytest==7.4.0 flake8==5.0.4 # v5.0.4 is the last to support Python 3.7 pytest-timeout==2.1.0 pytest-asyncio==0.21.1 requests==2.31.0 -httpx==0.24.1 \ No newline at end of file +httpx==0.24.1 diff --git a/gns3server/compute/docker/__init__.py b/gns3server/compute/docker/__init__.py index 6ed9ecb6..2801d93c 100644 --- a/gns3server/compute/docker/__init__.py +++ b/gns3server/compute/docker/__init__.py @@ -18,11 +18,15 @@ Docker server module. """ +import os import sys import json import asyncio import logging import aiohttp +import shutil +import subprocess + from gns3server.utils import parse_version from gns3server.utils.asyncio import locking from gns3server.compute.base_manager import BaseManager @@ -54,6 +58,25 @@ class Docker(BaseManager): self._session = None self._api_version = DOCKER_MINIMUM_API_VERSION + @staticmethod + def install_busybox(): + + if not sys.platform.startswith("linux"): + return + dst_busybox = os.path.join(os.path.dirname(os.path.abspath(__file__)), "resources", "bin", "busybox") + if os.path.isfile(dst_busybox): + return + for busybox_exec in ("busybox-static", "busybox.static", "busybox"): + busybox_path = shutil.which(busybox_exec) + if busybox_path: + log.info(f"Installing busybox from '{busybox_path}'") + try: + shutil.copy2(busybox_path, dst_busybox, follow_symlinks=True) + return + except OSError as e: + raise DockerError(f"Could not install busybox: {e}") + raise DockerError("No busybox executable could be found") + async def _check_connection(self): if not self._connected: diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index b77bc00d..0e8cf243 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -523,7 +523,7 @@ class DockerVM(BaseNode): async def update(self): """ - Destroy an recreate the container with the new settings + Destroy and recreate the container with the new settings """ # We need to save the console and state and restore it @@ -544,6 +544,9 @@ class DockerVM(BaseNode): Starts this Docker container. """ + # make sure busybox is installed + self.manager.install_busybox() + try: state = await self._get_container_state() except DockerHttp404Error: diff --git a/conf/gns3_server.conf b/gns3server/config_samples/gns3_server.conf similarity index 100% rename from conf/gns3_server.conf rename to gns3server/config_samples/gns3_server.conf diff --git a/gns3server/controller/appliance_manager.py b/gns3server/controller/appliance_manager.py index 14beab27..831aa70f 100644 --- a/gns3server/controller/appliance_manager.py +++ b/gns3server/controller/appliance_manager.py @@ -22,11 +22,6 @@ import asyncio import aiofiles import shutil -try: - import importlib_resources -except ImportError: - from importlib import resources as importlib_resources - from typing import Tuple, List from aiohttp.client_exceptions import ClientError diff --git a/gns3server/server.py b/gns3server/server.py index e17f2603..f6bd7431 100644 --- a/gns3server/server.py +++ b/gns3server/server.py @@ -167,8 +167,9 @@ class Server: config.Server.allow_remote_console = args.allow config.Server.host = args.host config.Server.port = args.port - config.Server.certfile = args.certfile - config.Server.certkey = args.certkey + #FIXME + #config.Server.certfile = args.certfile + #config.Server.certkey = args.certkey config.Server.enable_ssl = args.ssl def _signal_handling(self): diff --git a/pyproject.toml b/pyproject.toml index f50442c5..5c2cfc6a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ name = "gns3-server" description = "GNS3 graphical interface for the GNS3 server." license = {file = "LICENSE"} authors = [ - { name="Jeremy Grossmann" } + { name = "Jeremy Grossmann", email = "developers@gns3.com" } ] readme = "README.md" requires-python = ">=3.7" @@ -29,7 +29,7 @@ classifiers = [ "Programming Language :: Python :: Implementation :: CPython" ] -dynamic = ["version", "dependencies"] +dynamic = ["version", "dependencies", "optional-dependencies"] [tool.setuptools] packages = ["gns3server"] @@ -38,15 +38,8 @@ packages = ["gns3server"] version = {attr = "gns3server.version.__version__"} dependencies = {file = "requirements.txt"} -[project.optional-dependencies] -test = [ - "pytest==7.2.2", - "flake8==5.0.4", # v5.0.4 is the last to support Python 3.7 - "pytest-timeout==2.1.0", - "pytest-asyncio==0.20.3", - "requests==2.28.2", - "httpx==0.23.3" -] +[tool.setuptools.dynamic.optional-dependencies] +development = {file = ['dev-requirements.txt']} [project.urls] "Homepage" = "http://gns3.com" diff --git a/requirements.txt b/requirements.txt index 1f96abe9..2f5dfc1b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -19,4 +19,3 @@ email-validator==2.0.0.post2 watchfiles==0.19.0 zstandard==0.21.0 importlib_resources>=1.3 -setuptools>=60.8.1 diff --git a/setup.py b/setup.py deleted file mode 100644 index 87d5c711..00000000 --- a/setup.py +++ /dev/null @@ -1,47 +0,0 @@ -# -*- coding: utf-8 -*- -# -# Copyright (C) 2013 GNS3 Technologies Inc. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -import sys -import os -import shutil -import subprocess - -from setuptools import setup - -BUSYBOX_PATH = "gns3server/compute/docker/resources/bin/busybox" - - -def copy_busybox(): - if not sys.platform.startswith("linux"): - return - if os.path.isfile(BUSYBOX_PATH): - return - for bb_cmd in ("busybox-static", "busybox.static", "busybox"): - bb_path = shutil.which(bb_cmd) - if bb_path: - if subprocess.call(["ldd", bb_path], - stdin=subprocess.DEVNULL, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL): - shutil.copy2(bb_path, BUSYBOX_PATH, follow_symlinks=True) - break - else: - raise SystemExit("No static busybox found") - - -copy_busybox() # TODO: this should probably be done when the first time the server is started -setup() # required with setuptools below version 64.0.0 From f3d43aeb39ef84d1cc9293c6a5fd73105c895a1a Mon Sep 17 00:00:00 2001 From: grossmj Date: Thu, 10 Aug 2023 22:52:35 +1000 Subject: [PATCH 2/8] Fix testing.yml --- .github/workflows/testing.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 3e95e800..236a9985 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -30,7 +30,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install -r dev-requirements.txt + python -m pip install .[development] - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names From 483db91851dfc9ea62ea55141cd1b2a0985fe0ec Mon Sep 17 00:00:00 2001 From: grossmj Date: Thu, 10 Aug 2023 23:16:57 +1000 Subject: [PATCH 3/8] Use dev for optional development dependencies --- .github/workflows/testing.yml | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 236a9985..aa78d289 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -30,7 +30,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install .[development] + python -m pip install .[dev] - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names diff --git a/pyproject.toml b/pyproject.toml index 5c2cfc6a..9aa67ac5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,7 @@ version = {attr = "gns3server.version.__version__"} dependencies = {file = "requirements.txt"} [tool.setuptools.dynamic.optional-dependencies] -development = {file = ['dev-requirements.txt']} +dev = {file = ['dev-requirements.txt']} [project.urls] "Homepage" = "http://gns3.com" From 719458764f961aed9f4ec66c44ebb76857a04988 Mon Sep 17 00:00:00 2001 From: grossmj Date: Thu, 10 Aug 2023 23:23:11 +1000 Subject: [PATCH 4/8] Fix tests --- gns3server/server.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gns3server/server.py b/gns3server/server.py index f6bd7431..e17f2603 100644 --- a/gns3server/server.py +++ b/gns3server/server.py @@ -167,9 +167,8 @@ class Server: config.Server.allow_remote_console = args.allow config.Server.host = args.host config.Server.port = args.port - #FIXME - #config.Server.certfile = args.certfile - #config.Server.certkey = args.certkey + config.Server.certfile = args.certfile + config.Server.certkey = args.certkey config.Server.enable_ssl = args.ssl def _signal_handling(self): From f3b6825e4062da41149f6381b8434cacaa38139e Mon Sep 17 00:00:00 2001 From: grossmj Date: Fri, 11 Aug 2023 14:10:25 +1000 Subject: [PATCH 5/8] Test if busybox is not dynamically linked --- gns3server/compute/docker/__init__.py | 22 ++++++++++++++++++---- gns3server/compute/docker/docker_vm.py | 2 +- gns3server/utils/asyncio/__init__.py | 2 +- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/gns3server/compute/docker/__init__.py b/gns3server/compute/docker/__init__.py index 2801d93c..9e9e72b1 100644 --- a/gns3server/compute/docker/__init__.py +++ b/gns3server/compute/docker/__init__.py @@ -59,7 +59,7 @@ class Docker(BaseManager): self._api_version = DOCKER_MINIMUM_API_VERSION @staticmethod - def install_busybox(): + async def install_busybox(): if not sys.platform.startswith("linux"): return @@ -69,10 +69,24 @@ class Docker(BaseManager): for busybox_exec in ("busybox-static", "busybox.static", "busybox"): busybox_path = shutil.which(busybox_exec) if busybox_path: - log.info(f"Installing busybox from '{busybox_path}'") try: - shutil.copy2(busybox_path, dst_busybox, follow_symlinks=True) - return + # check that busybox is statically linked + # (dynamically linked busybox will fail to run in a container) + proc = await asyncio.create_subprocess_exec( + "ldd", + busybox_path, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.DEVNULL + ) + stdout, _ = await proc.communicate() + if proc.returncode == 1: + # ldd returns 1 if the file is not a dynamic executable + log.info(f"Installing busybox from '{busybox_path}' to '{dst_busybox}'") + shutil.copy2(busybox_path, dst_busybox, follow_symlinks=True) + return + else: + log.warning(f"Busybox '{busybox_path}' is dynamically linked\n" + f"{stdout.decode('utf-8', errors='ignore').strip()}") except OSError as e: raise DockerError(f"Could not install busybox: {e}") raise DockerError("No busybox executable could be found") diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 0e8cf243..bd94e86e 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -545,7 +545,7 @@ class DockerVM(BaseNode): """ # make sure busybox is installed - self.manager.install_busybox() + await self.manager.install_busybox() try: state = await self._get_container_state() diff --git a/gns3server/utils/asyncio/__init__.py b/gns3server/utils/asyncio/__init__.py index 46937b1b..562a3df6 100644 --- a/gns3server/utils/asyncio/__init__.py +++ b/gns3server/utils/asyncio/__init__.py @@ -77,7 +77,7 @@ async def subprocess_check_output(*args, cwd=None, env=None, stderr=False): if output is None: return "" # If we received garbage we ignore invalid characters - # it should happens only when user try to use another binary + # it should happen only when user try to use another binary # and the code of VPCS, dynamips... Will detect it's not the correct binary return output.decode("utf-8", errors="ignore") From 1fd8444d22f94d249ecf176d9ec6130381935a91 Mon Sep 17 00:00:00 2001 From: grossmj Date: Fri, 11 Aug 2023 17:32:05 +1000 Subject: [PATCH 6/8] Add tests for install_busybox() --- tests/compute/docker/test_docker.py | 47 +++++++++++++++++++++++++++++ tests/conftest.py | 16 ++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/tests/compute/docker/test_docker.py b/tests/compute/docker/test_docker.py index 5155d4eb..98096deb 100644 --- a/tests/compute/docker/test_docker.py +++ b/tests/compute/docker/test_docker.py @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import asyncio import pytest import pytest_asyncio from unittest.mock import MagicMock, patch @@ -209,3 +210,49 @@ async def test_docker_check_connection_docker_preferred_version_against_older(vm vm._connected = False await vm._check_connection() assert vm._api_version == DOCKER_MINIMUM_API_VERSION + + +@pytest.mark.asyncio +async def test_install_busybox(): + + mock_process = MagicMock(spec=asyncio.subprocess.Process) + mock_process.communicate.return_value = (b"", b"not a dynamic executable") + mock_process.returncode = 1 # means that busybox is not dynamically linked + + with patch("os.path.isfile", return_value=False): + with patch("shutil.which", return_value="/usr/bin/busybox"): + with patch("asyncio.create_subprocess_exec", return_value=mock_process) as create_subprocess_mock: + with patch("shutil.copy2") as copy2_mock: + await Docker.install_busybox() + create_subprocess_mock.assert_called_with( + "ldd", + "/usr/bin/busybox", + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.DEVNULL, + ) + assert copy2_mock.called + + +@pytest.mark.asyncio +async def test_install_busybox_dynamic_linked(): + + mock_process = MagicMock(spec=asyncio.subprocess.Process) + mock_process.communicate.return_value = (b"Dynamically linked library", b"") + mock_process.returncode = 0 # means that busybox is dynamically linked + + with patch("os.path.isfile", return_value=False): + with patch("shutil.which", return_value="/usr/bin/busybox"): + with patch("asyncio.create_subprocess_exec", return_value=mock_process): + with pytest.raises(DockerError) as e: + await Docker.install_busybox() + assert str(e.value) == "No busybox executable could be found" + + +@pytest.mark.asyncio +async def test_install_busybox_no_executables(): + + with patch("os.path.isfile", return_value=False): + with patch("shutil.which", return_value=None): + with pytest.raises(DockerError) as e: + await Docker.install_busybox() + assert str(e.value) == "No busybox executable could be found" diff --git a/tests/conftest.py b/tests/conftest.py index 579c4f78..40dd9604 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,7 @@ import os import uuid import configparser import base64 +import stat from fastapi import FastAPI from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine @@ -405,13 +406,24 @@ def run_around_tests(monkeypatch, config, port_manager): monkeypatch.setattr("gns3server.utils.path.get_default_project_directory", lambda *args: os.path.join(tmppath, 'projects')) - # Force sys.platform to the original value. Because it seem not be restore correctly at each tests + # Force sys.platform to the original value. Because it seems not be restored correctly after each test sys.platform = sys.original_platform yield - # An helper should not raise Exception + # A helper should not raise Exception try: shutil.rmtree(tmppath) except BaseException: pass + + +@pytest.fixture +def fake_executable(monkeypatch, tmpdir) -> str: + + monkeypatch.setenv("PATH", str(tmpdir)) + executable_path = os.path.join(os.environ["PATH"], "fake_executable") + with open(executable_path, "w+") as f: + f.write("1") + os.chmod(executable_path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) + return executable_path From d9eb61efc435f3b73e036e19e664b77b33d6fa49 Mon Sep 17 00:00:00 2001 From: grossmj Date: Fri, 11 Aug 2023 17:37:11 +1000 Subject: [PATCH 7/8] Fix tests with asyncio_patch --- tests/compute/docker/test_docker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/compute/docker/test_docker.py b/tests/compute/docker/test_docker.py index 98096deb..cb6f2e4d 100644 --- a/tests/compute/docker/test_docker.py +++ b/tests/compute/docker/test_docker.py @@ -221,7 +221,7 @@ async def test_install_busybox(): with patch("os.path.isfile", return_value=False): with patch("shutil.which", return_value="/usr/bin/busybox"): - with patch("asyncio.create_subprocess_exec", return_value=mock_process) as create_subprocess_mock: + with asyncio_patch("asyncio.create_subprocess_exec", return_value=mock_process) as create_subprocess_mock: with patch("shutil.copy2") as copy2_mock: await Docker.install_busybox() create_subprocess_mock.assert_called_with( @@ -242,7 +242,7 @@ async def test_install_busybox_dynamic_linked(): with patch("os.path.isfile", return_value=False): with patch("shutil.which", return_value="/usr/bin/busybox"): - with patch("asyncio.create_subprocess_exec", return_value=mock_process): + with asyncio_patch("asyncio.create_subprocess_exec", return_value=mock_process): with pytest.raises(DockerError) as e: await Docker.install_busybox() assert str(e.value) == "No busybox executable could be found" From 2f2aabeb5a2a0be3aa6912dba4fa8c4cdc3138f4 Mon Sep 17 00:00:00 2001 From: grossmj Date: Fri, 11 Aug 2023 17:58:00 +1000 Subject: [PATCH 8/8] Fix tests when running Python 3.7 --- tests/compute/docker/test_docker.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/compute/docker/test_docker.py b/tests/compute/docker/test_docker.py index cb6f2e4d..04b144c5 100644 --- a/tests/compute/docker/test_docker.py +++ b/tests/compute/docker/test_docker.py @@ -215,9 +215,9 @@ async def test_docker_check_connection_docker_preferred_version_against_older(vm @pytest.mark.asyncio async def test_install_busybox(): - mock_process = MagicMock(spec=asyncio.subprocess.Process) - mock_process.communicate.return_value = (b"", b"not a dynamic executable") - mock_process.returncode = 1 # means that busybox is not dynamically linked + mock_process = MagicMock() + mock_process.returncode = 1 # means that busybox is not dynamically linked + mock_process.communicate = AsyncioMagicMock(return_value=(b"", b"not a dynamic executable")) with patch("os.path.isfile", return_value=False): with patch("shutil.which", return_value="/usr/bin/busybox"): @@ -236,9 +236,9 @@ async def test_install_busybox(): @pytest.mark.asyncio async def test_install_busybox_dynamic_linked(): - mock_process = MagicMock(spec=asyncio.subprocess.Process) - mock_process.communicate.return_value = (b"Dynamically linked library", b"") + mock_process = MagicMock() mock_process.returncode = 0 # means that busybox is dynamically linked + mock_process.communicate = AsyncioMagicMock(return_value=(b"Dynamically linked library", b"")) with patch("os.path.isfile", return_value=False): with patch("shutil.which", return_value="/usr/bin/busybox"):