Skip to content

Commit bd314b2

Browse files
committed
[cleaner] Make cleaner concurrent
Draft version adding to the traditional sequential backend also sqlite3 and file based concurrent ones. Closes: #3097 Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
1 parent 6299806 commit bd314b2

10 files changed

Lines changed: 332 additions & 174 deletions

File tree

sos/cleaner/__init__.py

Lines changed: 63 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import tempfile
1717
import fnmatch
1818

19-
from concurrent.futures import ThreadPoolExecutor
19+
from concurrent.futures import ProcessPoolExecutor
2020
from datetime import datetime
2121
from pwd import getpwuid
2222

@@ -84,6 +84,7 @@ class SoSCleaner(SoSComponent):
8484
'domains': [],
8585
'disable_parsers': [],
8686
'skip_cleaning_files': [],
87+
'concur_backend': 'files',
8788
'jobs': 4,
8889
'keywords': [],
8990
'keyword_file': None,
@@ -128,14 +129,19 @@ def __init__(self, parser=None, args=None, cmdline=None, in_place=False,
128129

129130
self.cleaner_md = self.manifest.components.add_section('cleaner')
130131

131-
skip_cleaning_files = self.opts.skip_cleaning_files
132+
parser_args = [
133+
self.cleaner_mapping,
134+
self.opts.skip_cleaning_files,
135+
self.opts.concur_backend,
136+
os.path.dirname(self.opts.map_file) if self.opts.map_file else None # TODO: is it safe to pass None..?
137+
]
132138
self.parsers = [
133-
SoSHostnameParser(self.cleaner_mapping, skip_cleaning_files),
134-
SoSIPParser(self.cleaner_mapping, skip_cleaning_files),
135-
SoSIPv6Parser(self.cleaner_mapping, skip_cleaning_files),
136-
SoSMacParser(self.cleaner_mapping, skip_cleaning_files),
137-
SoSKeywordParser(self.cleaner_mapping, skip_cleaning_files),
138-
SoSUsernameParser(self.cleaner_mapping, skip_cleaning_files)
139+
SoSHostnameParser(*parser_args),
140+
SoSIPParser(*parser_args),
141+
SoSIPv6Parser(*parser_args),
142+
SoSMacParser(*parser_args),
143+
SoSKeywordParser(*parser_args),
144+
SoSUsernameParser(*parser_args),
139145
]
140146

141147
for _parser in self.opts.disable_parsers:
@@ -264,6 +270,10 @@ def add_parser_options(cls, parser):
264270
dest='skip_cleaning_files',
265271
help=('List of files to skip/ignore during '
266272
'cleaning. Globs are supported.'))
273+
clean_grp.add_argument('--concurrency-backend', default='files',
274+
choices=['files', 'sql', 'sequential'],
275+
help=('Backend for concurrent process cleaner. sequential enforces --jobs=1'), # TODO: add check for this enforcing
276+
dest='concur_backend')
267277
clean_grp.add_argument('-j', '--jobs', default=4, type=int,
268278
help='Number of concurrent archives to clean')
269279
clean_grp.add_argument('--keywords', action='extend', default=[],
@@ -307,11 +317,11 @@ def inspect_target_archive(self):
307317
check_type = self.opts.archive_type.replace('-', '_')
308318
for archive in self.archive_types:
309319
if archive.type_name == check_type:
310-
_arc = archive(self.opts.target, self.tmpdir)
320+
_arc = archive(self.opts.target, self.tmpdir, self.opts.keep_binary_files)
311321
else:
312322
for arc in self.archive_types:
313323
if arc.check_is_type(self.opts.target):
314-
_arc = arc(self.opts.target, self.tmpdir)
324+
_arc = arc(self.opts.target, self.tmpdir, self.opts.keep_binary_files)
315325
break
316326
if not _arc:
317327
return
@@ -521,7 +531,7 @@ def write_cleaner_log(self, archive=False):
521531
logfile.write(line)
522532

523533
if archive:
524-
self.obfuscate_file(log_name)
534+
self.archive.obfuscate_file([log_name])
525535
self.archive.add_file(log_name, dest="sos_logs/cleaner.log")
526536

527537
def get_new_checksum(self, archive_path):
@@ -552,17 +562,18 @@ def obfuscate_report_paths(self):
552562
try:
553563
msg = (
554564
f"Found {len(self.report_paths)} total reports to obfuscate, "
555-
f"processing up to {self.opts.jobs} concurrently\n"
565+
f"processing up to {self.opts.jobs} concurrently within one "
566+
"archive\n"
556567
)
557568
self.ui_log.info(msg)
558569
if self.opts.keep_binary_files:
559570
self.ui_log.warning(
560571
"WARNING: binary files that potentially contain sensitive "
561572
"information will NOT be removed from the final archive\n"
562573
)
563-
pool = ThreadPoolExecutor(self.opts.jobs)
564-
pool.map(self.obfuscate_report, self.report_paths, chunksize=1)
565-
pool.shutdown(wait=True)
574+
for report_path in self.report_paths:
575+
self.ui_log.info(f"Obfuscating {report_path.archive_path}")
576+
self.obfuscate_report(report_path)
566577
# finally, obfuscate the nested archive if one exists
567578
if self.nested_archive:
568579
self._replace_obfuscated_archives()
@@ -633,6 +644,10 @@ def _prepare_archive_with_prepper(self, archive, prepper):
633644

634645
for ritem in prepper.regex_items[pname]:
635646
_parser.mapping.add_regex_item(ritem)
647+
# we can't pass sqlite connection/cursor in constructor of child processes as that fails on the spawn
648+
# so we must unset it now and initiate inside (cloned processes') archive
649+
archive.set_parsers(self.parsers)
650+
archive.unload_parser_entries()
636651

637652
def get_preppers(self):
638653
"""
@@ -666,8 +681,9 @@ def obfuscate_report(self, archive): # pylint: disable=too-many-branches
666681
667682
Positional arguments:
668683
669-
:param report str: Filepath to the directory or archive
684+
:param archive str: Filepath to the directory or archive
670685
"""
686+
671687
try:
672688
arc_md = self.cleaner_md.add_section(archive.archive_name)
673689
start_time = datetime.now()
@@ -677,21 +693,32 @@ def obfuscate_report(self, archive): # pylint: disable=too-many-branches
677693
archive.extract()
678694
archive.report_msg("Beginning obfuscation...")
679695

680-
for fname in archive.get_file_list():
681-
short_name = fname.split(archive.archive_name + '/')[1]
682-
if archive.should_skip_file(short_name):
683-
continue
684-
if (not self.opts.keep_binary_files and
685-
archive.should_remove_file(short_name)):
686-
archive.remove_file(short_name)
687-
continue
688-
try:
689-
count = self.obfuscate_file(fname, short_name,
690-
archive.archive_name)
691-
if count:
692-
archive.update_sub_count(short_name, count)
693-
except Exception as err:
694-
self.log_debug(f"Unable to parse file {short_name}: {err}")
696+
file_list = [f for f in archive.get_file_list()]
697+
# we can't call simple executor.map(archive.obfuscate_arc_files, archive.get_file_list()) because
698+
# a child process does not carry forward internal changes (e.g. mappings' datasets) from one call
699+
# of obfuscate_arc_files method to another. Each obfuscate_arc_files method starts with vanilla
700+
# parent archive, that is initialised *once* at its beginning via initializer=archive.load_parser_entries
701+
# - but not afterwards..
702+
#
703+
# so we must pass list of all files for each worker at the beginning
704+
# this means less granularity of the child processes work (one worker can finish much sooner than the other)
705+
# but it is the best we can have (or best I found out)
706+
#
707+
# At least, the "file_list[i::self.opts.jobs]" means subsequent files (speculativelly of similar size and content)
708+
# are distributed to different processes, which attempts to split the load evenly. Yet better approach might be
709+
# reorderig file_list based on files' sizes.
710+
711+
files_obfuscated_count = total_sub_count = removed_file_count = 0
712+
with ProcessPoolExecutor(max_workers=self.opts.jobs, initializer=archive.load_parser_entries) as executor:
713+
futures = executor.map(archive.obfuscate_arc_files, [file_list[i::self.opts.jobs] for i in range(self.opts.jobs)])
714+
for (foc, tsc, rfc) in futures:
715+
files_obfuscated_count += foc
716+
total_sub_count += tsc
717+
removed_file_count += rfc
718+
# as there is no easy way to get dataset dicts from child processes' mappings, we can reload our own parent-process
719+
# archive from the disk/sqlite. The trick is that sequence of files/entries is the source of truth of *sequence*
720+
# of calling *all* mapping.all(item) methods - so replying this will generate the right datasets!
721+
archive.load_parser_entries()
695722

696723
try:
697724
self.obfuscate_directory_names(archive)
@@ -726,96 +753,18 @@ def obfuscate_report(self, archive): # pylint: disable=too-many-branches
726753
end_time = datetime.now()
727754
arc_md.add_field('end_time', end_time)
728755
arc_md.add_field('run_time', end_time - start_time)
729-
arc_md.add_field('files_obfuscated', len(archive.file_sub_list))
730-
arc_md.add_field('total_substitutions', archive.total_sub_count)
756+
arc_md.add_field('files_obfuscated', files_obfuscated_count)
757+
arc_md.add_field('total_substitutions', total_sub_count)
731758
rmsg = ''
732-
if archive.removed_file_count:
759+
if removed_file_count:
733760
rmsg = " [removed %s unprocessable files]"
734-
rmsg = rmsg % archive.removed_file_count
761+
rmsg = rmsg % removed_file_count
735762
archive.report_msg(f"Obfuscation completed{rmsg}")
736763

737764
except Exception as err:
738765
self.ui_log.info("Exception while processing "
739766
f"{archive.archive_name}: {err}")
740767

741-
def obfuscate_file(self, filename, short_name=None, arc_name=None):
742-
# pylint: disable=too-many-locals
743-
"""Obfuscate and individual file, line by line.
744-
745-
Lines processed, even if no substitutions occur, are then written to a
746-
temp file without our own tmpdir. Once the file has been completely
747-
iterated through, if there have been substitutions then the temp file
748-
overwrites the original file. If there are no substitutions, then the
749-
original file is left in place.
750-
751-
Positional arguments:
752-
753-
:param filename str: Filename relative to the extracted
754-
archive root
755-
"""
756-
if not filename:
757-
# the requested file doesn't exist in the archive
758-
return None
759-
subs = 0
760-
if not short_name:
761-
short_name = filename.split('/')[-1]
762-
if not os.path.islink(filename):
763-
# don't run the obfuscation on the link, but on the actual file
764-
# at some other point.
765-
_parsers = [
766-
_p for _p in self.parsers if not
767-
any(
768-
_skip.match(short_name) for _skip in _p.skip_patterns
769-
)
770-
]
771-
if not _parsers:
772-
self.log_debug(
773-
f"Skipping obfuscation of {short_name or filename} due to "
774-
f"matching file skip pattern"
775-
)
776-
return 0
777-
self.log_debug(f"Obfuscating {short_name or filename}",
778-
caller=arc_name)
779-
with tempfile.NamedTemporaryFile(mode='w', dir=self.tmpdir) \
780-
as tfile:
781-
with open(filename, 'r', encoding='utf-8',
782-
errors='replace') as fname:
783-
for line in fname:
784-
try:
785-
line, count = self.obfuscate_line(line, _parsers)
786-
subs += count
787-
tfile.write(line)
788-
except Exception as err:
789-
self.log_debug(f"Unable to obfuscate {short_name}:"
790-
f"{err}", caller=arc_name)
791-
tfile.seek(0)
792-
if subs:
793-
shutil.copyfile(tfile.name, filename)
794-
795-
_ob_short_name = self.obfuscate_string(short_name.split('/')[-1])
796-
_ob_filename = short_name.replace(short_name.split('/')[-1],
797-
_ob_short_name)
798-
799-
if _ob_filename != short_name:
800-
arc_path = filename.split(short_name)[0]
801-
_ob_path = os.path.join(arc_path, _ob_filename)
802-
# ensure that any plugin subdirs that contain obfuscated strings
803-
# get created with obfuscated counterparts
804-
if not os.path.islink(filename):
805-
os.rename(filename, _ob_path)
806-
else:
807-
# generate the obfuscated name of the link target
808-
_target_ob = self.obfuscate_string(os.readlink(filename))
809-
# remove the unobfuscated original symlink first, in case the
810-
# symlink name hasn't changed but the target has
811-
os.remove(filename)
812-
# create the newly obfuscated symlink, pointing to the
813-
# obfuscated target name, which may not exist just yet, but
814-
# when the actual file is obfuscated, will be created
815-
os.symlink(_target_ob, _ob_path)
816-
817-
return subs
818-
819768
def obfuscate_symlinks(self, archive):
820769
"""Iterate over symlinks in the archive and obfuscate their names.
821770
The content of the link target will have already been cleaned, and this
@@ -883,6 +832,7 @@ def obfuscate_directory_names(self, archive):
883832
)
884833
os.rename(_dirname, _ob_arc_dir)
885834

835+
# TODO: this is a dup method in SoSObfuscationArchive but we cant easily remove either of them..?
886836
def obfuscate_string(self, string_data):
887837
for parser in self.parsers:
888838
try:
@@ -891,34 +841,6 @@ def obfuscate_string(self, string_data):
891841
self.log_info(f"Error obfuscating string data: {err}")
892842
return string_data
893843

894-
def obfuscate_line(self, line, parsers=None):
895-
"""Run a line through each of the obfuscation parsers, keeping a
896-
cumulative total of substitutions done on that particular line.
897-
898-
Positional arguments:
899-
900-
:param line str: The raw line as read from the file being
901-
processed
902-
:param parsers: A list of parser objects to obfuscate
903-
with. If None, use all.
904-
905-
Returns the fully obfuscated line and the number of substitutions made
906-
"""
907-
# don't iterate over blank lines, but still write them to the tempfile
908-
# to maintain the same structure when we write a scrubbed file back
909-
count = 0
910-
if not line.strip():
911-
return line, count
912-
if parsers is None:
913-
parsers = self.parsers
914-
for parser in parsers:
915-
try:
916-
line, _count = parser.parse_line(line)
917-
count += _count
918-
except Exception as err:
919-
self.log_debug(f"failed to parse line: {err}", parser.name)
920-
return line, count
921-
922844
def write_stats_to_manifest(self):
923845
"""Write some cleaner-level, non-report-specific stats to the manifest
924846
"""

0 commit comments

Comments
 (0)