From 535b0f4546ebbf9e4f2c3ee5b71cabd862516b02 Mon Sep 17 00:00:00 2001 From: Marten Ringwelski Date: Thu, 19 Sep 2024 10:24:04 +0200 Subject: [PATCH] refactor: Unify startup scripts Previously we had three different startup scripts: - The docker entrypoint (fact_extractor/docker_extract.py) - The python module entrypoint (fact_extractor/fact_extract.py) - The docker wrapper (extract.py) This commit removes the docker entrypoint and moves the module entrypoint to __main__.py while changing as little code as possible. This commit removes two usages of `sudo` that were used to chown the extracted files. Using sudo in a python module is bad practice. Also, one could argue that the --chown functionality should be replaced by proper container namespace mapping as its usecase is to change back permissions from root to the user that called docker. --- Dockerfile | 4 +- README.md | 2 +- docker/entrypoint.sh | 5 ++ extract.py | 5 +- .../{fact_extract.py => __main__.py} | 19 ++++-- fact_extractor/docker_extraction.py | 61 ------------------- fact_extractor/helperFunctions/file_system.py | 4 +- .../helperFunctions/program_setup.py | 2 +- fact_extractor/unpacker/unpackBase.py | 2 +- 9 files changed, 29 insertions(+), 75 deletions(-) create mode 100755 docker/entrypoint.sh rename fact_extractor/{fact_extract.py => __main__.py} (73%) delete mode 100755 fact_extractor/docker_extraction.py diff --git a/Dockerfile b/Dockerfile index 2fa30dcc..01481b3e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -17,10 +17,10 @@ RUN --mount=type=cache,target=/var/cache/apt \ --mount=type=cache,target=/root/.cache/pip \ /app/fact_extractor/install/pre_install.sh -ADD . /app +ADD . /app/ RUN --mount=type=cache,target=/var/cache/apt \ --mount=type=cache,target=/root/.cache/pip \ /app/install.py -ENTRYPOINT ["/app/fact_extractor/docker_extraction.py"] +ENTRYPOINT ["/app/docker/entrypoint.py"] diff --git a/README.md b/README.md index b04eb0a2..15786593 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ sudo apt install linux-modules-extra-$(uname -r) The tool can then be run with ```bash -fact_extractor/fact_extract.py [OPTIONS] PATH_TO_FIRMWARE +python -m fact_extractor [OPTIONS] PATH_TO_FIRMWARE ``` The tool is build with docker in mind. To that end it extracts all files into a directory specified in the config. diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh new file mode 100755 index 00000000..e9ab983a --- /dev/null +++ b/docker/entrypoint.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +python -m fact_extractor \ + --config_file /app/config/main.cfg \ + "$@" diff --git a/extract.py b/extract.py index dd065af6..f72f691c 100755 --- a/extract.py +++ b/extract.py @@ -18,7 +18,10 @@ def parse_arguments(): parser = argparse.ArgumentParser( - description='Command line interface for FACT_extractor.\nExtract arbitrary container or compression formats with one utility.' + description=( + 'Command line wrapper for FACT_extractor docker container.\n' + ' Extract arbitrary container or compression formats with one utility.' + ) ) parser.add_argument('-v', '--version', action='version', version=set_version()) parser.add_argument('-c', '--container', help='docker container', default=DEFAULT_CONTAINER) diff --git a/fact_extractor/fact_extract.py b/fact_extractor/__main__.py similarity index 73% rename from fact_extractor/fact_extract.py rename to fact_extractor/__main__.py index 86721f7e..5a260d72 100755 --- a/fact_extractor/fact_extract.py +++ b/fact_extractor/__main__.py @@ -20,7 +20,8 @@ import sys from pathlib import Path -from fact_extractor.helperFunctions.program_setup import setup_argparser, setup_logging, load_config +from fact_extractor.helperFunctions.file_system import change_owner_of_output_files +from fact_extractor.helperFunctions.program_setup import setup_argparser, setup_logging, load_config, check_ulimits from fact_extractor.unpacker.unpack import unpack @@ -28,14 +29,20 @@ def main(): arguments = setup_argparser('FACT extractor', 'Standalone extraction utility', sys.argv) config = load_config(arguments.config_file) setup_logging(arguments.debug, log_file=arguments.log_file, log_level=arguments.log_level) + check_ulimits() + + data_dir = Path(config.get('unpack', 'data_folder')) + report_dir = data_dir / 'reports' + files_dir = data_dir / 'files' # Make sure report folder exists some meta.json can be written - report_folder = Path(config.get('unpack', 'data_folder'), 'reports') - report_folder.mkdir(parents=True, exist_ok=True) + report_dir.mkdir(parents=True, exist_ok=True) + unpack(arguments.FILE_PATH, config) - return 0 + if arguments.chown is not None: + change_owner_of_output_files(files_dir, arguments.chown) + return 0 -if __name__ == '__main__': - exit(main()) +sys.exit(main()) diff --git a/fact_extractor/docker_extraction.py b/fact_extractor/docker_extraction.py deleted file mode 100755 index 58dd260b..00000000 --- a/fact_extractor/docker_extraction.py +++ /dev/null @@ -1,61 +0,0 @@ -#!/usr/bin/env python3 -''' - fact_extractor - Copyright (C) 2015-2019 Fraunhofer FKIE - - 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 argparse -from pathlib import Path -import sys - -from fact_extractor.helperFunctions.config import get_config_dir -from fact_extractor.helperFunctions.file_system import change_owner_of_output_files -from fact_extractor.helperFunctions.program_setup import check_ulimits, load_config, setup_logging -from fact_extractor.unpacker.unpack import unpack - - -def _parse_args(): - parser = argparse.ArgumentParser() - parser.add_argument( - '--chown', type=str, default='', help='change back ownership of output files to :' - ) - parser.add_argument( - '--extract_everything', - action='store_true', - default=False, - help='change the behavior of the extractor: extract also empty files', - ) - return parser.parse_args() - - -def main(args): - config = load_config(f'{get_config_dir()}/main.cfg') - setup_logging(debug=False) - check_ulimits() - - input_dir = Path(config.get('unpack', 'data_folder'), 'input') - input_file = list(input_dir.iterdir())[0] - - unpack(input_file, config, args.extract_everything) - - if args.chown: - output_dir = Path(config.get('unpack', 'data_folder'), 'files') - return change_owner_of_output_files(output_dir, args.chown) - - return 0 - - -if __name__ == '__main__': - sys.exit(main(_parse_args())) diff --git a/fact_extractor/helperFunctions/file_system.py b/fact_extractor/helperFunctions/file_system.py index 4c078e14..58a26863 100644 --- a/fact_extractor/helperFunctions/file_system.py +++ b/fact_extractor/helperFunctions/file_system.py @@ -54,8 +54,8 @@ def change_owner_of_output_files(files_dir: Path, owner: str) -> int: logging.error('ownership string should have the format :') return 1 - _, return_code_chown = execute_shell_command_get_return_code(f'sudo chown -R {owner} {files_dir}') - _, return_code_chmod = execute_shell_command_get_return_code(f'sudo chmod -R u+rw {files_dir}') + _, return_code_chown = execute_shell_command_get_return_code(f'chown -R {owner} {files_dir}') + _, return_code_chmod = execute_shell_command_get_return_code(f'chmod -R u+rw {files_dir}') return return_code_chmod | return_code_chown diff --git a/fact_extractor/helperFunctions/program_setup.py b/fact_extractor/helperFunctions/program_setup.py index f031e343..e0c39f87 100644 --- a/fact_extractor/helperFunctions/program_setup.py +++ b/fact_extractor/helperFunctions/program_setup.py @@ -10,7 +10,7 @@ def setup_argparser(name, description, command_line_options, version=__VERSION__): - parser = argparse.ArgumentParser(description='{} - {}'.format(name, description)) + parser = argparse.ArgumentParser(prog=name, description='{} - {}'.format(name, description)) parser.add_argument('-V', '--version', action='version', version='{} {}'.format(name, version)) parser.add_argument('-l', '--log_file', help='path to log file', default=None) parser.add_argument('-L', '--log_level', help='define the log level', choices=['DEBUG', 'INFO', 'WARNING', 'ERROR'], default=None) diff --git a/fact_extractor/unpacker/unpackBase.py b/fact_extractor/unpacker/unpackBase.py index 59d9cc20..e76e2377 100644 --- a/fact_extractor/unpacker/unpackBase.py +++ b/fact_extractor/unpacker/unpackBase.py @@ -102,7 +102,7 @@ def _extract_files_from_file_using_specific_unpacker(self, file_path: str, tmp_d return out, meta_data def change_owner_back_to_me(self, directory: str = None, permissions: str = 'u+r'): - with Popen(f'sudo chown -R {getuid()}:{getgid()} {directory}', shell=True, stdout=PIPE, stderr=PIPE) as pl: + with Popen(f'chown -R {getuid()}:{getgid()} {directory}', shell=True, stdout=PIPE, stderr=PIPE) as pl: pl.communicate() self.grant_read_permission(directory, permissions)