From 5f61e582fd92c241efe4eea82b9c538641594bc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Nikoli=C4=87?= <57295098+n-dusan@users.noreply.github.com> Date: Sat, 28 Sep 2024 01:39:03 +0200 Subject: [PATCH] Ndusan/lifecycle handlers executable fixes (#535) * feat: add to install additional dependencies into taf executable * fchore: formatting * chore: disable tests * fix: fixes to get executables to work Since we introduced LazyGroup, lots of modules are getting dynamically loaded. Add --collect-submodules to pyinstaller to resolve. * chore(revert): turn on script from local disk for testing * feat: add `taf scripts execute` command necessary for taf executable Used to execute arbitrary python scripts using a pre-built taf executable with pyinstaller. Lots of fixes were needed to get pyinstaller to correctly run a python script from subprocess. See [1] for pyinstaller discussion and the reasoning behind this. [1] - https://github.com/orgs/pyinstaller/discussions/5978 * chore: changelog --- .github/workflows/ci.yml | 7 +++-- CHANGELOG.md | 4 +++ setup.py | 3 ++ taf/tools/cli/taf.py | 3 +- taf/tools/scripts/__init__.py | 50 +++++++++++++++++++++++++++++++ taf/updater/lifecycle_handlers.py | 33 ++++++++++++++------ 6 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 taf/tools/scripts/__init__.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac8151a1..18b48624 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -159,25 +159,26 @@ jobs: - name: Install dependencies run: | pip install .[yubikey] + pip install .[executable] pip install pyinstaller - name: Build and test standalone executable (Linux) if: matrix.os == 'ubuntu-latest' run: | - pyinstaller --onefile --hidden-import=yubikey_manager --name taf-linux -y taf/tools/cli/taf.py + pyinstaller --onefile --hidden-import=yubikey_manager --hidden-import=lxml --collect-submodules taf.tools --name taf-linux -y taf/tools/cli/taf.py chmod +x dist/taf-linux ./dist/taf-linux --help | grep "TAF Command Line Interface" || { echo "Error: Expected text not found in the executable output"; exit 1; } - name: Build and test standalone executable (Windows) if: matrix.os == 'windows-latest' run: | - pyinstaller --onefile --hidden-import=yubikey_manager --name taf-windows.exe -y taf/tools/cli/taf.py + pyinstaller --onefile --hidden-import=yubikey_manager --hidden-import=lxml --collect-submodules taf.tools --name taf-windows.exe -y taf/tools/cli/taf.py ./dist/taf-windows.exe --help | Select-String "TAF Command Line Interface" -quiet - name: Build and test standalone executable (macOS) if: matrix.os == 'macos-latest' run: | - pyinstaller --onefile --hidden-import=yubikey_manager --name taf-macos -y taf/tools/cli/taf.py + pyinstaller --onefile --hidden-import=yubikey_manager --hidden-import=lxml --collect-submodules taf.tools --name taf-macos -y taf/tools/cli/taf.py ./dist/taf-macos --help | grep "TAF Command Line Interface" || { echo "Error: Expected text not found in the executable output"; exit 1; } - name: Upload standalone executable (Linux) diff --git a/CHANGELOG.md b/CHANGELOG.md index d849bcb3..d2e721f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning][semver]. ### Added +- Added lxml to taf pyinstaller to execute arbitrary python scripts ([535]) - Added support for execution of executable files within the scripts directories ([529]) - Added yubikey_present parameter to keys description (Can be specified when generating keys) ([508]) - Removed 2048-bit key restriction [494] @@ -41,6 +42,8 @@ and this project adheres to [Semantic Versioning][semver]. ### Fixed +- Fixes to executing taf handler scripts from a pyinstaller executable ([535]) +- Fix `persisent` and `transient` NoneType error when running taf handlers ([535]) - Fix update status when a target repo was updated and the auth repo was not ([532]) - Fix merge-commit which wasn't updating the remote-tracking branch ([532]) - Fix removal of additional local commits ([532]) @@ -48,6 +51,7 @@ and this project adheres to [Semantic Versioning][semver]. - Fix setup role when specifying public keys in keys-description ([511]) - `check_if_repositories_clean` error now returns a list of repositories which aren't clean, instead of a single repository ([525]) +[535]: https://github.com/openlawlibrary/taf/pull/535 [532]: https://github.com/openlawlibrary/taf/pull/532 [529]: https://github.com/openlawlibrary/taf/pull/529 [528]: https://github.com/openlawlibrary/taf/pull/528 diff --git a/setup.py b/setup.py index bfe5fd5d..01cb1376 100644 --- a/setup.py +++ b/setup.py @@ -24,6 +24,8 @@ "freezegun==0.3.15", ] +executable_require = ["lxml"] + dev_require = ["bandit>=1.6.0", "black>=19.3b0", "pre-commit>=1.18.3"] tests_require = [ @@ -72,6 +74,7 @@ "test": tests_require, "dev": dev_require, "yubikey": yubikey_require, + "executable": executable_require, }, "tests_require": tests_require, "entry_points": { diff --git a/taf/tools/cli/taf.py b/taf/tools/cli/taf.py index 9a2d60f9..530437fc 100644 --- a/taf/tools/cli/taf.py +++ b/taf/tools/cli/taf.py @@ -1,5 +1,5 @@ import click -from .lazy_group import LazyGroup +from taf.tools.cli.lazy_group import LazyGroup @click.group(cls=LazyGroup, lazy_subcommands={ @@ -11,6 +11,7 @@ "metadata": "taf.tools.metadata.attach_to_group", "roles": "taf.tools.roles.attach_to_group", "yubikey": "taf.tools.yubikey.attach_to_group", + "scripts": "taf.tools.scripts.attach_to_group", }) @click.version_option() def taf(): diff --git a/taf/tools/scripts/__init__.py b/taf/tools/scripts/__init__.py new file mode 100644 index 00000000..917c60d0 --- /dev/null +++ b/taf/tools/scripts/__init__.py @@ -0,0 +1,50 @@ +import click + +from pathlib import Path +from taf.log import taf_logger + +import ast + + +def extract_global_variables(filename): + """ + Utility function to extract global variables from a Python file. + This is necessary because we want to execute the Python file in a separate process, and we need to pass the global variables to it. + TAF currently uses this when executing lifecycle handler scripts from an executable built from pyinstaller, which uses a frozen sys module. + """ + with open(filename, "r") as f: + tree = ast.parse(f.read(), filename=filename) + + global_vars = {} + + for node in ast.walk(tree): + try: + if isinstance(node, ast.Assign): + for target in node.targets: + if isinstance(target, ast.Name): + # We only extract simple variable assignments, not function definitions or imports + if isinstance(node.value, (ast.Constant, ast.List, ast.Dict, ast.Tuple, ast.Constant)): + # Convert the AST expression to Python objects + global_vars[target.id] = ast.literal_eval(node.value) + except Exception as e: + taf_logger.debug(f"Error extracting global variables from {filename}: {e}") + pass + + global_vars["__file__"] = filename + + return global_vars + + +def execute_command(): + @click.command(help="Executes an arbitrary python script") + @click.argument("script_path") + def execute(script_path): + script_path = Path(script_path).resolve() + global_scopes = extract_global_variables(script_path) + with open(script_path, "r") as f: + exec(f.read(), global_scopes) # nosec: B102 + return execute + + +def attach_to_group(group): + group.add_command(execute_command(), name='execute') diff --git a/taf/updater/lifecycle_handlers.py b/taf/updater/lifecycle_handlers.py index e8991fc4..2af8cf47 100644 --- a/taf/updater/lifecycle_handlers.py +++ b/taf/updater/lifecycle_handlers.py @@ -190,21 +190,38 @@ def execute_scripts(auth_repo, last_commit, scripts_rel_path, data, scripts_root script_paths = [] for script_path in sorted(script_paths): - taf_logger.info("Executing script {}", script_path) + taf_logger.log("NOTICE", f"Executing script {script_path}") json_data = json.dumps(data) try: if Path(script_path).suffix == ".py": - output = run(sys.executable, script_path, input=json_data) + if getattr(sys, "frozen", False): + # we are running in a pyinstaller bundle + output = run( + f"{sys.executable}", + "scripts", + "execute", + script_path, + input=json_data, + ) + else: + output = run(f"{sys.executable}", script_path, input=json_data) # assume that all other types of files are non-OS-specific executables of some kind else: output = run_subprocess([script_path]) - except subprocess.CalledProcessError as e: - taf_logger.error( - "An error occurred while executing {}: {}", script_path, e.output - ) - raise ScriptExecutionError(script_path, e.output) + except Exception as e: + if type(e) is subprocess.CalledProcessError: + taf_logger.error( + f"An error occurred while executing {script_path}: {e.output}" + ) + raise ScriptExecutionError(script_path, e.output) + else: + taf_logger.error( + f"An error occurred while executing {script_path}: {str(e)}" + ) + raise ScriptExecutionError(script_path, str(e)) if type(output) is bytes: output = output.decode() + transient_data = persistent_data = {} if output is not None and output != "": # if the script contains print statements other than the final # print which outputs transient and persistent data @@ -215,8 +232,6 @@ def execute_scripts(auth_repo, last_commit, scripts_rel_path, data, scripts_root persistent_data = json_data.get("persistent") if transient_data is not None or persistent_data is not None: break - else: - transient_data = persistent_data = {} taf_logger.debug("Persistent data: {}", persistent_data) taf_logger.debug("Transient data: {}", transient_data) # overwrite current persistent and transient data