From 75b969b1adb02811a0aa82188d64626ba0be29e9 Mon Sep 17 00:00:00 2001 From: DavHau Date: Thu, 11 Jul 2024 15:34:41 +0700 Subject: [PATCH] clan-cli: improve runtime dependency management Many dependencies of clan-cli are currently dynamically loaded via nix-shell on each execution. This is nice, as it reduces the initial closure size of clan, but the overhead introduced by nix-shell piles up quickly, as some commands shell out many times during their lifetime. For example, when adding a secret git is called 10+ times. This reduces the time of a test which adds a secret from around 50 seconds to 15 seconds. - add run_cmd() as an alternative to nix_shell() - introduce the concept of static dependencies which do not need to go through nix-shell - static dependencies are defined at build time and included into the wrapper for clan-cli - add package: clan-cli-full which statically ships all required dependencies TODO: deprecate nix_shell() in favor of run_cmd() --- pkgs/clan-cli/clan_cli/git.py | 14 +-- .../clan_cli/{nix.py => nix/__init__.py} | 50 ++++++++++- .../clan_cli/nix/allowed-programs.json | 15 ++++ pkgs/clan-cli/default.nix | 86 +++++++++---------- pkgs/clan-cli/flake-module.nix | 11 ++- pkgs/clan-cli/shell.nix | 6 ++ pkgs/clan-cli/tests/test_vars.py | 6 +- 7 files changed, 132 insertions(+), 56 deletions(-) rename pkgs/clan-cli/clan_cli/{nix.py => nix/__init__.py} (62%) create mode 100644 pkgs/clan-cli/clan_cli/nix/allowed-programs.json diff --git a/pkgs/clan-cli/clan_cli/git.py b/pkgs/clan-cli/clan_cli/git.py index b812004f..ec9e693f 100644 --- a/pkgs/clan-cli/clan_cli/git.py +++ b/pkgs/clan-cli/clan_cli/git.py @@ -2,7 +2,7 @@ from pathlib import Path # from clan_cli.dirs import find_git_repo_root from clan_cli.errors import ClanError -from clan_cli.nix import nix_shell +from clan_cli.nix import run_cmd from .cmd import Log, run from .locked_open import locked_open @@ -60,8 +60,8 @@ def _commit_file_to_git( """ with locked_open(repo_dir / ".git" / "clan.lock", "w+"): for file_path in file_paths: - cmd = nix_shell( - ["nixpkgs#git"], + cmd = run_cmd( + ["git"], ["git", "-C", str(repo_dir), "add", str(file_path)], ) # add the file to the git index @@ -73,8 +73,8 @@ def _commit_file_to_git( ) # check if there is a diff - cmd = nix_shell( - ["nixpkgs#git"], + cmd = run_cmd( + ["git"], ["git", "-C", str(repo_dir), "diff", "--cached", "--exit-code"] + [str(file_path) for file_path in file_paths], ) @@ -84,8 +84,8 @@ def _commit_file_to_git( return # commit only that file - cmd = nix_shell( - ["nixpkgs#git"], + cmd = run_cmd( + ["git"], [ "git", "-C", diff --git a/pkgs/clan-cli/clan_cli/nix.py b/pkgs/clan-cli/clan_cli/nix/__init__.py similarity index 62% rename from pkgs/clan-cli/clan_cli/nix.py rename to pkgs/clan-cli/clan_cli/nix/__init__.py index 0d191577..e09be891 100644 --- a/pkgs/clan-cli/clan_cli/nix.py +++ b/pkgs/clan-cli/clan_cli/nix/__init__.py @@ -4,8 +4,8 @@ import tempfile from pathlib import Path from typing import Any -from .cmd import run, run_no_stdout -from .dirs import nixpkgs_flake, nixpkgs_source +from ..cmd import run, run_no_stdout +from ..dirs import nixpkgs_flake, nixpkgs_source def nix_command(flags: list[str]) -> list[str]: @@ -111,3 +111,49 @@ def nix_shell(packages: list[str], cmd: list[str]) -> list[str]: "-c", *cmd, ] + + +# lazy loads list of allowed and static programs +class Programs: + allowed_programs = None + static_programs = None + + @classmethod + def is_allowed(cls: type["Programs"], program: str) -> bool: + if cls.allowed_programs is None: + with open(Path(__file__).parent / "allowed-programs.json") as f: + cls.allowed_programs = json.load(f) + return program in cls.allowed_programs + + @classmethod + def is_static(cls: type["Programs"], program: str) -> bool: + """ + Determines if a program is statically shipped with this clan distribution + """ + if cls.static_programs is None: + cls.static_programs = os.environ.get("CLAN_STATIC_PROGRAMS", "").split(":") + return program in cls.static_programs + + +# Alternative implementation of nix_shell() to replace nix_shell() at some point +# Features: +# - allow list for programs (need to be specified in allowed-programs.json) +# - be abe to compute a closure of all deps for testing +# - build clan distributions that ship some or all packages (eg. clan-cli-full) +def run_cmd(programs: list[str], cmd: list[str]) -> list[str]: + for program in programs: + if not Programs.is_allowed(program): + raise ValueError(f"Program not allowed: {program}") + if os.environ.get("IN_NIX_SANDBOX"): + return cmd + missing_packages = [ + f"nixpkgs#{program}" for program in programs if not Programs.is_static(program) + ] + if not missing_packages: + return cmd + return [ + *nix_command(["shell", "--inputs-from", f"{nixpkgs_flake()!s}"]), + *missing_packages, + "-c", + *cmd, + ] diff --git a/pkgs/clan-cli/clan_cli/nix/allowed-programs.json b/pkgs/clan-cli/clan_cli/nix/allowed-programs.json new file mode 100644 index 00000000..f429db13 --- /dev/null +++ b/pkgs/clan-cli/clan_cli/nix/allowed-programs.json @@ -0,0 +1,15 @@ +[ + "age", + "bash", + "e2fsprogs", + "git", + "mypy", + "nix", + "openssh", + "qemu", + "rsync", + "sops", + "sshpass", + "tor", + "zbar" +] diff --git a/pkgs/clan-cli/default.nix b/pkgs/clan-cli/default.nix index a78bfe57..56f9512d 100644 --- a/pkgs/clan-cli/default.nix +++ b/pkgs/clan-cli/default.nix @@ -1,54 +1,40 @@ { - age, - lib, + # callPackage args argcomplete, + gitMinimal, + gnupg, installShellFiles, + lib, nix, - openssh, - pytest, + pkgs, pytest-cov, - pytest-xdist, pytest-subprocess, pytest-timeout, + pytest-xdist, + pytest, python3, runCommand, setuptools, - sops, stdenv, - rsync, - bash, - sshpass, - zbar, - tor, - git, - qemu, - gnupg, - e2fsprogs, - mypy, - nixpkgs, + + # custom args clan-core-path, - gitMinimal, + nixpkgs, + includedRuntimeDeps, }: let pythonDependencies = [ argcomplete # Enables shell completions ]; - runtimeDependencies = [ - bash - nix - openssh - sshpass - zbar - tor - age - rsync - sops - git - mypy - qemu - e2fsprogs - ]; + # load nixpkgs runtime dependencies from a json file + # This file represents an allow list at the same time that is checked by the run_cmd + # implementation in nix.py + runtimeDependenciesAsSet = lib.genAttrs (lib.importJSON ./clan_cli/nix/allowed-programs.json) ( + name: pkgs.${name} + ); + + runtimeDependencies = lib.attrValues runtimeDependenciesAsSet; testDependencies = runtimeDependencies @@ -65,10 +51,6 @@ let pytest-timeout # Add timeouts to your tests ]; - runtimeDependenciesAsSet = builtins.listToAttrs ( - builtins.map (p: lib.nameValuePair (lib.getName p.name) p) runtimeDependencies - ); - # Setup Python environment with all dependencies for running tests pythonWithTestDeps = python3.withPackages (_ps: testDependencies); @@ -106,13 +88,28 @@ python3.pkgs.buildPythonApplication { format = "pyproject"; # Arguments for the wrapper to unset LD_LIBRARY_PATH to avoid glibc version issues - makeWrapperArgs = [ - "--unset LD_LIBRARY_PATH" - "--suffix" - "PATH" - ":" - "${gitMinimal}/bin/git" - ]; + makeWrapperArgs = + [ + "--unset LD_LIBRARY_PATH" + + # TODO: remove gitMinimal here and use the one from runtimeDependencies + "--suffix" + "PATH" + ":" + "${gitMinimal}/bin/git" + ] + # include selected runtime dependencies in the PATH + ++ lib.concatMap (p: [ + "--prefix" + "PATH" + ":" + p + ]) includedRuntimeDeps + ++ [ + "--set" + "CLAN_STATIC_PROGRAMS" + (lib.concatStringsSep ":" includedRuntimeDeps) + ]; nativeBuildInputs = [ setuptools @@ -165,6 +162,7 @@ python3.pkgs.buildPythonApplication { passthru.testDependencies = testDependencies; passthru.pythonWithTestDeps = pythonWithTestDeps; passthru.runtimeDependencies = runtimeDependencies; + passthru.runtimeDependenciesAsSet = runtimeDependenciesAsSet; postInstall = '' cp -r ${nixpkgs'} $out/${python3.sitePackages}/clan_cli/nixpkgs diff --git a/pkgs/clan-cli/flake-module.nix b/pkgs/clan-cli/flake-module.nix index c41143a3..b5645105 100644 --- a/pkgs/clan-cli/flake-module.nix +++ b/pkgs/clan-cli/flake-module.nix @@ -40,13 +40,22 @@ { devShells.clan-cli = pkgs.callPackage ./shell.nix { - inherit (self'.packages) clan-cli; + inherit (self'.packages) clan-cli clan-cli-full; inherit self'; }; packages = { clan-cli = pkgs.python3.pkgs.callPackage ./default.nix { inherit (inputs) nixpkgs; clan-core-path = clanCoreWithVendoredDeps; + includedRuntimeDeps = [ + "age" + "git" + ]; + }; + clan-cli-full = pkgs.python3.pkgs.callPackage ./default.nix { + inherit (inputs) nixpkgs; + clan-core-path = clanCoreWithVendoredDeps; + includedRuntimeDeps = lib.importJSON ./clan_cli/nix/allowed-programs.json; }; clan-cli-docs = pkgs.stdenv.mkDerivation { name = "clan-cli-docs"; diff --git a/pkgs/clan-cli/shell.nix b/pkgs/clan-cli/shell.nix index 53f92602..fdd8a321 100644 --- a/pkgs/clan-cli/shell.nix +++ b/pkgs/clan-cli/shell.nix @@ -1,6 +1,8 @@ { + lib, nix-unit, clan-cli, + clan-cli-full, mkShell, ruff, python3, @@ -27,6 +29,10 @@ mkShell { PYTHONBREAKPOINT = "ipdb.set_trace"; + CLAN_STATIC_PROGRAMS = lib.concatStringsSep ":" ( + lib.attrNames clan-cli-full.passthru.runtimeDependenciesAsSet + ); + shellHook = '' export GIT_ROOT="$(git rev-parse --show-toplevel)" export PKG_ROOT="$GIT_ROOT/pkgs/clan-cli" diff --git a/pkgs/clan-cli/tests/test_vars.py b/pkgs/clan-cli/tests/test_vars.py index 0bcfa264..73670ca0 100644 --- a/pkgs/clan-cli/tests/test_vars.py +++ b/pkgs/clan-cli/tests/test_vars.py @@ -40,9 +40,11 @@ def test_generate_public_var( ) monkeypatch.chdir(flake.path) cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"]) - assert ( + secret_path = ( flake.path / "machines" / "my_machine" / "vars" / "my_generator" / "my_secret" - ).is_file() + ) + assert secret_path.is_file() + assert secret_path.read_text() == "hello\n" @pytest.mark.impure