From d0e55530a71f52b099e7932e95c6c9ec6ad913a8 Mon Sep 17 00:00:00 2001 From: Yash Gorana Date: Fri, 1 Sep 2023 14:21:25 +0530 Subject: [PATCH 1/4] [syftcli] fix subprocess error bubbling --- packages/syftcli/syftcli/bundle/create.py | 59 +++++++--- .../syftcli/syftcli/core/container_engine.py | 106 +++++++++++------- packages/syftcli/syftcli/core/proc.py | 69 +++++++++--- 3 files changed, 164 insertions(+), 70 deletions(-) diff --git a/packages/syftcli/syftcli/bundle/create.py b/packages/syftcli/syftcli/bundle/create.py index fa33a661cbc..13f9cb34daa 100644 --- a/packages/syftcli/syftcli/bundle/create.py +++ b/packages/syftcli/syftcli/bundle/create.py @@ -13,6 +13,7 @@ # relative from ..core.container_engine import ContainerEngine +from ..core.container_engine import ContainerEngineError from ..core.container_engine import Docker from ..core.container_engine import Podman from ..core.syft_repo import SyftRepo @@ -53,7 +54,7 @@ def create( bundle_path = Path(out_path, f"syft-{ver.release_tag}-{engine.value}.tar") # Prepare container engine & images - engine_sdk = get_container_engine(engine) + engine_sdk = get_container_engine(engine, dryrun=dryrun) image_tags = get_syft_images(ver) # Begin bundling @@ -63,14 +64,10 @@ def create( ) print("\n[bold cyan]Pulling images...") - engine_sdk.pull( - image_tags, - stream_output={"cb_stdout": fn_print_std, "cb_stderr": fn_print_std}, - dryrun=dryrun, - ) + pull_images(engine_sdk, image_tags, dryrun=dryrun) print("\n[bold cyan]Creating image archive...") - engine_sdk.save(image_tags, tarfile=img_path, dryrun=dryrun) + archive_images(engine_sdk, image_tags, img_path, dryrun=dryrun) print(f"\n[bold cyan]Downloading {engine} config...") asset_path = get_engine_config(engine, ver, temp_path, dryrun=dryrun) @@ -84,10 +81,6 @@ def create( print("\n[bold green]Done!") -def fn_print_std(line: str) -> None: - print(f"[bright_black]{line}", end="", sep="") - - def validate_version(version: str) -> SyftVersion: _ver: SyftVersion @@ -108,7 +101,9 @@ def validate_version(version: str) -> SyftVersion: return _ver -def get_container_engine(engine_name: ContainerEngineType) -> ContainerEngine: +def get_container_engine( + engine_name: ContainerEngineType, dryrun: bool = False +) -> ContainerEngine: engine: ContainerEngine if engine_name == ContainerEngineType.Docker: @@ -116,13 +111,49 @@ def get_container_engine(engine_name: ContainerEngineType) -> ContainerEngine: elif engine_name == ContainerEngineType.Podman: engine = Podman() - if not engine.is_installed(): - print(f"[bold red]'{engine_name}' is not running or not installed") + if not dryrun and not engine.is_available(): + print( + f"[bold red]Error: '{engine_name}' is unavailable. Make sure it is installed and running." + ) raise Exit(1) return engine +def pull_images( + engine_sdk: ContainerEngine, + image_tags: List[str], + dryrun: bool = False, +) -> None: + def fn_print_std(line: str) -> None: + print(f"[bright_black]{line}", end="", sep="") + + try: + results = engine_sdk.pull( + image_tags, + stream_output={"cb_stdout": fn_print_std, "cb_stderr": fn_print_std}, + dryrun=dryrun, + ) + dryrun and [print(f"[grey70]{result.args}") for result in results] + except ContainerEngineError as e: + print("[bold red]Error:", e) + raise Exit(e.returncode) + + +def archive_images( + engine_sdk: ContainerEngine, + image_tags: List[str], + archive_path: Path, + dryrun: bool = False, +) -> None: + try: + result = engine_sdk.save(image_tags, archive_path, dryrun=dryrun) + dryrun and print(f"[grey70]{result.args}") + except ContainerEngineError as e: + print("[bold red]Error:", e) + raise Exit(e.returncode) + + def get_syft_images(syft_ver: SyftVersion) -> List[str]: manifest = SyftRepo.get_manifest(syft_ver.release_tag) return manifest["images"] diff --git a/packages/syftcli/syftcli/core/container_engine.py b/packages/syftcli/syftcli/core/container_engine.py index 96011a9f4ce..e1430a379c6 100644 --- a/packages/syftcli/syftcli/core/container_engine.py +++ b/packages/syftcli/syftcli/core/container_engine.py @@ -1,4 +1,6 @@ # stdlib +from abc import ABC +from abc import abstractmethod from typing import List from typing import Optional @@ -6,75 +8,103 @@ from rich.progress import track # relative -from .proc import handle_error +from .proc import CalledProcessError +from .proc import CompletedProcess from .proc import run_command -class ContainerEngine: +class ContainerEngineError(CalledProcessError): pass +class ContainerEngine(ABC): + @abstractmethod + def is_available(self) -> bool: + raise NotImplementedError() + + @abstractmethod + def pull( + self, images: List[str], dryrun: bool, stream_output: Optional[dict] + ) -> List[CompletedProcess]: + raise NotImplementedError() + + @abstractmethod + def save( + self, images: List[str], archive_path: str, dryrun: bool + ) -> CompletedProcess: + raise NotImplementedError() + + def check_returncode(self, result: CompletedProcess) -> None: + try: + result.check_returncode() + except CalledProcessError as e: + raise ContainerEngineError(e.returncode, e.cmd) from e + + class Podman(ContainerEngine): - def is_installed(self) -> bool: + def is_available(self) -> bool: result = run_command("podman version") - return result[-1] == 0 + return result.returncode == 0 def pull( self, images: List[str], dryrun: bool = False, stream_output: Optional[dict] = None, - ) -> None: + ) -> List[CompletedProcess]: + results = [] + for image in track(images, description=""): command = f"podman pull {image} --quiet" + result = run_command(command, stream_output=stream_output, dryrun=dryrun) + self.check_returncode(result) + results.append(result) - if dryrun: - print(command) - continue - - result = run_command(command, stream_output=stream_output) - handle_error(result) + return results - def save(self, images: List[str], tarfile: str, dryrun: bool = False) -> None: + def save( + self, + images: List[str], + archive_path: str, + dryrun: bool = False, + ) -> CompletedProcess: + # -m works only with --format=docker-archive images_str = " ".join(images) - command = f"podman save -o {tarfile} {images_str}" - - if dryrun: - print(command) - return - - result = run_command(command) - handle_error(result) + command = f"podman save -m -o {archive_path} {images_str}" + result = run_command(command, dryrun=dryrun) + self.check_returncode(result) + return result class Docker(ContainerEngine): - def is_installed(self) -> bool: + def is_available(self) -> bool: result = run_command("docker version") - return result[-1] == 0 + return result.returncode == 0 def pull( self, images: List[str], dryrun: bool = False, stream_output: Optional[dict] = None, - ) -> None: + ) -> List[CompletedProcess]: + results = [] + for image in track(images, description=""): command = f"docker pull {image} --quiet" + result = run_command(command, stream_output=stream_output, dryrun=dryrun) + self.check_returncode(result) + results.append(result) - if dryrun: - print(command) - continue - - result = run_command(command, stream_output=stream_output) - handle_error(result) + return results - def save(self, images: List[str], tarfile: str, dryrun: bool = False) -> None: + def save( + self, + images: List[str], + archive_path: str, + dryrun: bool = False, + ) -> CompletedProcess: images_str = " ".join(images) - command = f"docker save -o {tarfile} {images_str}" - - if dryrun: - print(command) - return - - result = run_command(command) - handle_error(result) + command = f"docker save -o {archive_path} {images_str}" + result = run_command(command, dryrun=dryrun) + self.check_returncode(result) + return result diff --git a/packages/syftcli/syftcli/core/proc.py b/packages/syftcli/syftcli/core/proc.py index ce3c4ed7a51..196efc3bd43 100644 --- a/packages/syftcli/syftcli/core/proc.py +++ b/packages/syftcli/syftcli/core/proc.py @@ -1,14 +1,15 @@ # stdlib -import subprocess -import sys +from functools import wraps +from subprocess import CalledProcessError +from subprocess import CompletedProcess +from subprocess import PIPE +from subprocess import Popen import threading from typing import Any from typing import Callable from typing import Optional -from typing import Tuple -from typing import TypeAlias -CmdResult: TypeAlias = Tuple[str, str, int] +__all__ = ["run_command", "check_returncode", "CalledProcessError", "CompletedProcess"] def NOOP(x: Any) -> None: @@ -18,12 +19,37 @@ def NOOP(x: Any) -> None: def run_command( command: str, working_dir: Optional[str] = None, - stdout: int = subprocess.PIPE, - stderr: int = subprocess.PIPE, + stdout: int = PIPE, + stderr: int = PIPE, stream_output: Optional[dict] = None, -) -> CmdResult: + dryrun: bool = False, +) -> CompletedProcess: + """ + Run a command in a subprocess. + + Args: + command (str): The command to run. + working_dir (str): The working directory to run the command in. + stdout (int): The stdout file descriptor. Defaults to subprocess.PIPE. + stderr (int): The stderr file descriptor. Defaults to subprocess.PIPE. + stream_output (dict): A dict containing callbacks to process stdout and stderr in real-time. + dryrun (bool): If True, the command will not be executed. + + Returns: + A CompletedProcess object. + + Example: + >>> from syftcli.core.proc import run_command + >>> result = run_command("echo 'hello world'") + >>> result.check_returncode() + >>> result.stdout + """ + + if dryrun: + return CompletedProcess(command, 0, stdout="", stderr="") + try: - process = subprocess.Popen( + process = Popen( command, shell=True, cwd=working_dir, @@ -66,16 +92,23 @@ def process_output(stream: str, callback: Callable) -> None: stdout_thread.join() stderr_thread.join() - return _out, _err, process.returncode else: _out, _err = process.communicate() - return (_out, _err, process.returncode) - except subprocess.CalledProcessError as e: - return (e.stdout, e.stderr, e.returncode) + return CompletedProcess(command, process.returncode, _out, _err) + except CalledProcessError as e: + return CompletedProcess(command, e.returncode, e.stdout, e.stderr) + + +def check_returncode( + func: Callable[..., CompletedProcess] +) -> Callable[..., CompletedProcess]: + """A decorator to wrap run_command and check the return code.""" + + @wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> CompletedProcess: + result = func(*args, **kwargs) + result.check_returncode() + return result -def handle_error(result: CmdResult, exit_on_error: bool = False) -> None: - stdout, stderr, code = result - if code != 0: - print("Error:", stderr) - exit_on_error and sys.exit(-1) + return wrapper From 5e70628a316eb2f3a4e13091c4e3f42acdf3c426 Mon Sep 17 00:00:00 2001 From: Yash Gorana Date: Fri, 1 Sep 2023 15:15:19 +0530 Subject: [PATCH 2/4] [syftcli] raise exception on http error --- packages/syftcli/syftcli/core/syft_repo.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/syftcli/syftcli/core/syft_repo.py b/packages/syftcli/syftcli/core/syft_repo.py index b3a43b2218c..33102ede743 100644 --- a/packages/syftcli/syftcli/core/syft_repo.py +++ b/packages/syftcli/syftcli/core/syft_repo.py @@ -24,7 +24,9 @@ class Assets: @lru_cache(maxsize=None) def releases() -> List[dict]: url = REPO_API_URL + "/releases" - releases = requests.get(url).json() + response = requests.get(url) + response.raise_for_status() + releases = response.json() return [rel for rel in releases if rel.get("tag_name", "").startswith("v")] @staticmethod @@ -76,4 +78,6 @@ def download_asset(asset_name: str, rel_ver: str, dl_dir: str) -> Path: @staticmethod def get_asset(rel_ver: str, asset_name: str, **kwargs: Any) -> requests.Response: url = REPO_DL_URL + f"/{rel_ver}/{asset_name}" - return requests.get(url, **kwargs) + response = requests.get(url, **kwargs) + response.raise_for_status() + return response From f55d9299751da7a24ab69acc0433b29269848921 Mon Sep 17 00:00:00 2001 From: Yash Gorana Date: Fri, 1 Sep 2023 15:33:37 +0530 Subject: [PATCH 3/4] [syftcli] simplify printing --- packages/syftcli/syftcli/bundle/create.py | 43 ++++++++++++----------- packages/syftcli/syftcli/core/console.py | 30 ++++++++++++++-- 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/packages/syftcli/syftcli/bundle/create.py b/packages/syftcli/syftcli/bundle/create.py index 13f9cb34daa..eb1e9ad162a 100644 --- a/packages/syftcli/syftcli/bundle/create.py +++ b/packages/syftcli/syftcli/bundle/create.py @@ -6,12 +6,15 @@ from typing import List # third party -from rich import print from typer import Exit from typer import Option from typing_extensions import Annotated # relative +from ..core.console import debug +from ..core.console import error +from ..core.console import info +from ..core.console import success from ..core.container_engine import ContainerEngine from ..core.container_engine import ContainerEngineError from ..core.container_engine import Docker @@ -58,27 +61,24 @@ def create( image_tags = get_syft_images(ver) # Begin bundling - print( - f"[bold green]" - f"Creating Syft {ver.release_tag} {engine} bundle at '{bundle_path}'" - ) + info(f"Creating Syft {ver.release_tag} {engine} bundle at '{bundle_path}'") - print("\n[bold cyan]Pulling images...") + info("\nPulling images...") pull_images(engine_sdk, image_tags, dryrun=dryrun) - print("\n[bold cyan]Creating image archive...") + info("\nCreating image archive...") archive_images(engine_sdk, image_tags, img_path, dryrun=dryrun) - print(f"\n[bold cyan]Downloading {engine} config...") + info(f"\nDownloading {engine} config...") asset_path = get_engine_config(engine, ver, temp_path, dryrun=dryrun) - print("\n[bold cyan]Creating final bundle...") + info("\nCreating final bundle...") create_syft_bundle(bundle_path, images=img_path, assets=asset_path, dryrun=dryrun) - print("\n[bold cyan]Cleaning up...") + info("\nCleaning up...") cleanup_dir(temp_path) - print("\n[bold green]Done!") + success("\nDone!") def validate_version(version: str) -> SyftVersion: @@ -87,15 +87,15 @@ def validate_version(version: str) -> SyftVersion: try: _ver = SyftVersion(version) except InvalidVersion: - print(f"[bold red]Error: '{version}' is not a valid version") + error(f"Error: '{version}' is not a valid version") raise Exit(1) if _ver.match("<0.8.2b27"): - print(f"[bold red]Error: Minimum supported version is 0.8.2. Got: {_ver}") + error(f"Error: Minimum supported version is '0.8.2' Got: '{_ver}'") raise Exit(1) if not _ver.valid_version(): - print(f"[bold red]Error: Version '{_ver}' is not a valid Syft release") + error(f"Error: Version '{_ver}' is not a valid Syft release") raise Exit(1) return _ver @@ -112,8 +112,9 @@ def get_container_engine( engine = Podman() if not dryrun and not engine.is_available(): - print( - f"[bold red]Error: '{engine_name}' is unavailable. Make sure it is installed and running." + error( + f"Error: '{engine_name}' is unavailable. " + "Make sure it is installed and running." ) raise Exit(1) @@ -126,7 +127,7 @@ def pull_images( dryrun: bool = False, ) -> None: def fn_print_std(line: str) -> None: - print(f"[bright_black]{line}", end="", sep="") + debug(line, end="", sep="") try: results = engine_sdk.pull( @@ -134,9 +135,9 @@ def fn_print_std(line: str) -> None: stream_output={"cb_stdout": fn_print_std, "cb_stderr": fn_print_std}, dryrun=dryrun, ) - dryrun and [print(f"[grey70]{result.args}") for result in results] + dryrun and [debug(result.args) for result in results] # type: ignore[func-returns-value] except ContainerEngineError as e: - print("[bold red]Error:", e) + error("Error:", e) raise Exit(e.returncode) @@ -148,9 +149,9 @@ def archive_images( ) -> None: try: result = engine_sdk.save(image_tags, archive_path, dryrun=dryrun) - dryrun and print(f"[grey70]{result.args}") + dryrun and debug(result.args) # type: ignore[func-returns-value] except ContainerEngineError as e: - print("[bold red]Error:", e) + error("Error:", e) raise Exit(e.returncode) diff --git a/packages/syftcli/syftcli/core/console.py b/packages/syftcli/syftcli/core/console.py index f66eed4e5fa..19b232dc272 100644 --- a/packages/syftcli/syftcli/core/console.py +++ b/packages/syftcli/syftcli/core/console.py @@ -1,4 +1,30 @@ +# stdlib +from typing import Any + # third party -from rich.console import Console +from rich import get_console +from rich import print + +__all__ = ["console", "info", "warn", "success", "error", "debug", "print"] + +console = get_console() + + +def info(*args: Any, **kwargs: Any) -> None: + console.print(*args, style="bold cyan", **kwargs) + + +def debug(*args: Any, **kwargs: Any) -> None: + console.print(*args, style="grey50", highlight=False, **kwargs) + + +def warn(*args: Any, **kwargs: Any) -> None: + console.print(*args, style="bold yellow", **kwargs) + + +def success(*args: Any, **kwargs: Any) -> None: + console.print(*args, style="bold green", **kwargs) + -console = Console() +def error(*args: Any, **kwargs: Any) -> None: + console.print(*args, style="bold red", **kwargs) From 4e57f7fd9c3da2307a9d3323c09553cc7d97b7f2 Mon Sep 17 00:00:00 2001 From: Yash Gorana Date: Fri, 1 Sep 2023 16:03:18 +0530 Subject: [PATCH 4/4] [syftcli] refactor some code --- packages/syftcli/syftcli/bundle/create.py | 70 +++++++++++++---------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/syftcli/syftcli/bundle/create.py b/packages/syftcli/syftcli/bundle/create.py index eb1e9ad162a..92e84a483f6 100644 --- a/packages/syftcli/syftcli/bundle/create.py +++ b/packages/syftcli/syftcli/bundle/create.py @@ -28,20 +28,24 @@ DEFAULT_OUTPUT_DIR = Path("~/.syft") -class ContainerEngineType(str, Enum): +class Engine(str, Enum): Docker = "docker" Podman = "podman" +VersionOpts = Annotated[str, Option("--version", "-v")] +EngineOpts = Annotated[Engine, Option("--engine", "-e")] +DryrunOpts = Annotated[bool, Option("--dryrun")] +OutdirOpts = Annotated[ + Path, Option("--outdir", "-d", dir_okay=True, file_okay=False, writable=True) +] + + def create( - version: Annotated[str, Option("--version", "-v")] = "latest", - outdir: Annotated[ - Path, Option("--outdir", "-d", dir_okay=True, file_okay=False, writable=True) - ] = DEFAULT_OUTPUT_DIR, - engine: Annotated[ - ContainerEngineType, Option("--engine", "-e") - ] = ContainerEngineType.Docker, - dryrun: bool = False, + version: VersionOpts = "latest", + outdir: OutdirOpts = DEFAULT_OUTPUT_DIR, + engine: EngineOpts = Engine.Docker, + dryrun: DryrunOpts = False, ) -> None: """Create an offline deployment bundle for Syft.""" @@ -51,7 +55,7 @@ def create( # Prepare temp paths out_path = prepare_output_dir(outdir) temp_path = prepare_tmp_dir(out_path) - img_path = Path(temp_path, "images.tar") + archive_path = Path(temp_path, "images.tar") # prepare output paths bundle_path = Path(out_path, f"syft-{ver.release_tag}-{engine.value}.tar") @@ -67,13 +71,13 @@ def create( pull_images(engine_sdk, image_tags, dryrun=dryrun) info("\nCreating image archive...") - archive_images(engine_sdk, image_tags, img_path, dryrun=dryrun) + archive_images(engine_sdk, image_tags, archive_path, dryrun=dryrun) - info(f"\nDownloading {engine} config...") - asset_path = get_engine_config(engine, ver, temp_path, dryrun=dryrun) + info(f"\nDownloading {engine.value} config...") + config_path = get_engine_config(engine, ver, temp_path, dryrun=dryrun) info("\nCreating final bundle...") - create_syft_bundle(bundle_path, images=img_path, assets=asset_path, dryrun=dryrun) + create_syft_bundle(bundle_path, archive_path, config_path, dryrun=dryrun) info("\nCleaning up...") cleanup_dir(temp_path) @@ -101,14 +105,12 @@ def validate_version(version: str) -> SyftVersion: return _ver -def get_container_engine( - engine_name: ContainerEngineType, dryrun: bool = False -) -> ContainerEngine: +def get_container_engine(engine_name: Engine, dryrun: bool = False) -> ContainerEngine: engine: ContainerEngine - if engine_name == ContainerEngineType.Docker: + if engine_name == Engine.Docker: engine = Docker() - elif engine_name == ContainerEngineType.Podman: + elif engine_name == Engine.Podman: engine = Podman() if not dryrun and not engine.is_available(): @@ -184,40 +186,46 @@ def cleanup_dir(path: Path) -> None: def get_engine_config( - engine: ContainerEngineType, + engine: Engine, ver: SyftVersion, dl_dir: Path, dryrun: bool = False, ) -> Path: asset_name = ( SyftRepo.Assets.PODMAN_CONFIG - if engine == ContainerEngineType.Podman + if engine == Engine.Podman else SyftRepo.Assets.DOCKER_CONFIG ) if dryrun: + debug(f"Download: '{ver.release_tag}/{asset_name}' to '{dl_dir}'") return Path(dl_dir, asset_name) return SyftRepo.download_asset(asset_name, ver.release_tag, dl_dir) def create_syft_bundle( - path: Path, - images: Path, - assets: Path, + bundle_path: Path, + archive_path: Path, + config_path: Path, dryrun: bool = False, ) -> None: if dryrun: + debug( + f"Bundle: {bundle_path}\n" + f"+ Image: {archive_path}\n" + f"+ Deployment Config: {config_path}\n" + ) return - if path.exists(): - path.unlink() + if bundle_path.exists(): + bundle_path.unlink() - with tarfile.open(str(path), "w") as bundle: - # extract assets as-is into bundle root - with tarfile.open(str(assets), "r:gz") as asset: + with tarfile.open(str(bundle_path), "w") as bundle: + # extract assets config as-is into bundle root + with tarfile.open(str(config_path), "r:gz") as asset: for member in asset.getmembers(): bundle.addfile(member, asset.extractfile(member)) - # add images archive into the bundle - bundle.add(images, arcname=images.name) + # add image archive into the bundle + bundle.add(archive_path, arcname=archive_path.name)