Skip to content

Commit

Permalink
refactor: Unify startup scripts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
maringuu committed Sep 19, 2024
1 parent 279d266 commit 535b0f4
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 75 deletions.
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions docker/entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/bash

python -m fact_extractor \
--config_file /app/config/main.cfg \
"$@"
5 changes: 4 additions & 1 deletion extract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 13 additions & 6 deletions fact_extractor/fact_extract.py → fact_extractor/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,29 @@
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


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())
61 changes: 0 additions & 61 deletions fact_extractor/docker_extraction.py

This file was deleted.

4 changes: 2 additions & 2 deletions fact_extractor/helperFunctions/file_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ def change_owner_of_output_files(files_dir: Path, owner: str) -> int:
logging.error('ownership string should have the format <user id>:<group id>')
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


Expand Down
2 changes: 1 addition & 1 deletion fact_extractor/helperFunctions/program_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion fact_extractor/unpacker/unpackBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 535b0f4

Please sign in to comment.