Skip to content

Commit 04f27ef

Browse files
committed
[cleaner] Make cleaner concurrent
Allow running cleaner concurrently via child processes. They synchronize on the ordering of items added to dataset of each mapper by creating numbered files in a directory specific for each mapper. Together with deterministic generation of obfuscated values, this ensures the individual processes end up with identical mappings. Resolves: #3097 Closes: #3988 Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
1 parent 1ea4c38 commit 04f27ef

16 files changed

Lines changed: 440 additions & 265 deletions

File tree

sos/cleaner/__init__.py

Lines changed: 89 additions & 154 deletions
Large diffs are not rendered by default.

sos/cleaner/archives/__init__.py

Lines changed: 151 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import shutil
1414
import stat
1515
import tarfile
16+
import tempfile
1617
import re
1718

1819
from concurrent.futures import ProcessPoolExecutor
@@ -54,15 +55,15 @@ class SoSObfuscationArchive():
5455
class. All report-level operations should be contained within this class.
5556
"""
5657

57-
file_sub_list = []
58+
files_obfuscated_count = 0
5859
total_sub_count = 0
5960
removed_file_count = 0
6061
type_name = 'undetermined'
6162
description = 'undetermined'
6263
is_nested = False
6364
prep_files = {}
6465

65-
def __init__(self, archive_path, tmpdir):
66+
def __init__(self, archive_path, tmpdir, keep_binary_files):
6667
self.archive_path = archive_path
6768
self.final_archive_path = self.archive_path
6869
self.tmpdir = tmpdir
@@ -74,10 +75,144 @@ def __init__(self, archive_path, tmpdir):
7475
self.is_extracted = False
7576
self._load_self()
7677
self.archive_root = ''
78+
self.keep_binary_files = keep_binary_files
79+
self.parsers = ()
7780
self.log_info(
7881
f"Loaded {self.archive_path} as type {self.description}"
7982
)
8083

84+
def obfuscate_string(self, string_data):
85+
for parser in self.parsers:
86+
try:
87+
string_data = parser.parse_string_for_keys(string_data)
88+
except Exception as err:
89+
self.log_info(f"Error obfuscating string data: {err}")
90+
return string_data
91+
92+
# TODO: merge content to obfuscate_arc_files as that is the only place we
93+
# call obfuscate_filename ?
94+
def obfuscate_filename(self, short_name, filename):
95+
_ob_short_name = self.obfuscate_string(short_name.split('/')[-1])
96+
_ob_filename = short_name.replace(short_name.split('/')[-1],
97+
_ob_short_name)
98+
99+
if _ob_filename != short_name:
100+
arc_path = filename.split(short_name)[0]
101+
_ob_path = os.path.join(arc_path, _ob_filename)
102+
# ensure that any plugin subdirs that contain obfuscated strings
103+
# get created with obfuscated counterparts
104+
if not os.path.islink(filename):
105+
os.rename(filename, _ob_path)
106+
else:
107+
# generate the obfuscated name of the link target
108+
_target_ob = self.obfuscate_string(os.readlink(filename))
109+
# remove the unobfuscated original symlink first, in case the
110+
# symlink name hasn't changed but the target has
111+
os.remove(filename)
112+
# create the newly obfuscated symlink, pointing to the
113+
# obfuscated target name, which may not exist just yet, but
114+
# when the actual file is obfuscated, will be created
115+
os.symlink(_target_ob, _ob_path)
116+
117+
def set_parsers(self, parsers):
118+
self.parsers = parsers # TODO: include this in __init__?
119+
120+
def load_parser_entries(self):
121+
for parser in self.parsers:
122+
parser.load_map_entries()
123+
124+
def obfuscate_line(self, line, parsers=None):
125+
"""Run a line through each of the obfuscation parsers, keeping a
126+
cumulative total of substitutions done on that particular line.
127+
128+
Positional arguments:
129+
130+
:param line str: The raw line as read from the file being
131+
processed
132+
:param parsers: A list of parser objects to obfuscate
133+
with. If None, use all.
134+
135+
Returns the fully obfuscated line and the number of substitutions made
136+
"""
137+
# don't iterate over blank lines, but still write them to the tempfile
138+
# to maintain the same structure when we write a scrubbed file back
139+
count = 0
140+
if not line.strip():
141+
return line, count
142+
if parsers is None:
143+
parsers = self.parsers
144+
for parser in parsers:
145+
try:
146+
line, _count = parser.parse_line(line)
147+
count += _count
148+
except Exception as err:
149+
self.log_debug(f"failed to parse line: {err}", parser.name)
150+
return line, count
151+
152+
def obfuscate_arc_files(self, flist):
153+
for filename in flist:
154+
self.log_debug(f" pid={os.getpid()}: obfuscating {filename}")
155+
try:
156+
short_name = filename.split(self.archive_name + '/')[1]
157+
if self.should_skip_file(short_name):
158+
continue
159+
if (not self.keep_binary_files and
160+
self.should_remove_file(short_name)):
161+
# We reach this case if the option --keep-binary-files
162+
# was not used, and the file is in a list to be removed
163+
self.remove_file(short_name)
164+
continue
165+
if (self.keep_binary_files and
166+
(file_is_binary(filename) or
167+
self.should_remove_file(short_name))):
168+
# We reach this case if the option --keep-binary-files
169+
# is used. In this case we want to make sure
170+
# the cleaner doesn't try to clean a binary file
171+
continue
172+
if os.path.islink(filename):
173+
# don't run the obfuscation on the link, but on the actual
174+
# file at some other point.
175+
continue
176+
_parsers = [
177+
_p for _p in self.parsers if not
178+
any(
179+
_skip.match(short_name) for _skip in _p.skip_patterns
180+
)
181+
]
182+
if not _parsers:
183+
self.log_debug(
184+
f"Skipping obfuscation of {short_name or filename} "
185+
f"due to matching file skip pattern"
186+
)
187+
continue
188+
self.log_debug(f"Obfuscating {short_name or filename}")
189+
subs = 0
190+
with tempfile.NamedTemporaryFile(mode='w', dir=self.tmpdir) \
191+
as tfile:
192+
with open(filename, 'r', encoding='utf-8',
193+
errors='replace') as fname:
194+
for line in fname:
195+
try:
196+
line, cnt = self.obfuscate_line(line, _parsers)
197+
subs += cnt
198+
tfile.write(line)
199+
except Exception as err:
200+
self.log_debug(f"Unable to obfuscate "
201+
f"{short_name}: {err}")
202+
tfile.seek(0)
203+
if subs:
204+
shutil.copyfile(tfile.name, filename)
205+
self.update_sub_count(subs)
206+
207+
self.obfuscate_filename(short_name, filename)
208+
209+
except Exception as err:
210+
self.log_debug(f" pid={os.getpid()}: caught exception on "
211+
f"obfuscating file {filename}: {err}")
212+
213+
return (self.files_obfuscated_count, self.total_sub_count,
214+
self.removed_file_count)
215+
81216
@classmethod
82217
def check_is_type(cls, arc_path):
83218
"""Check if the archive is a well-known type we directly support"""
@@ -120,14 +255,18 @@ def report_msg(self, msg):
120255
"""Helper to easily format ui messages on a per-report basis"""
121256
self.ui_log.info(f"{self.ui_name + ' :':<50} {msg}")
122257

123-
def _fmt_log_msg(self, msg):
124-
return f"[cleaner:{self.archive_name}] {msg}"
258+
def _fmt_log_msg(self, msg, caller=None):
259+
return f"[cleaner{f':{caller}' if caller else ''}" \
260+
f"[{self.archive_name}]] {msg}"
261+
262+
def log_debug(self, msg, caller=None):
263+
self.soslog.debug(self._fmt_log_msg(msg, caller))
125264

126-
def log_debug(self, msg):
127-
self.soslog.debug(self._fmt_log_msg(msg))
265+
def log_info(self, msg, caller=None):
266+
self.soslog.info(self._fmt_log_msg(msg, caller))
128267

129-
def log_info(self, msg):
130-
self.soslog.info(self._fmt_log_msg(msg))
268+
def log_error(self, msg, caller=None):
269+
self.soslog.error(self._fmt_log_msg(msg, caller))
131270

132271
def _load_skip_list(self):
133272
"""Provide a list of files and file regexes to skip obfuscation on
@@ -201,6 +340,7 @@ def extract(self, quiet=False):
201340
self.report_msg("Extracting...")
202341
self.extracted_path = self.extract_self()
203342
self.is_extracted = True
343+
self.tarobj = None # we can't pickle this & not further needed
204344
else:
205345
self.extracted_path = self.archive_path
206346
# if we're running as non-root (e.g. collector), then we can have a
@@ -326,7 +466,7 @@ def get_symlinks(self):
326466
if os.path.islink(_fname):
327467
yield _fname
328468

329-
def get_file_list(self):
469+
def get_files(self):
330470
"""Iterator for a list of files in the archive, to allow clean to
331471
iterate over.
332472
@@ -345,11 +485,11 @@ def get_directory_list(self):
345485
dir_list.append(dirname)
346486
return dir_list
347487

348-
def update_sub_count(self, fname, count):
488+
def update_sub_count(self, count):
349489
"""Called when a file has finished being parsed and used to track
350490
total substitutions made and number of files that had changes made
351491
"""
352-
self.file_sub_list.append(fname)
492+
self.files_obfuscated_count += 1
353493
self.total_sub_count += count
354494

355495
def get_file_path(self, fname):

sos/cleaner/archives/sos.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ def get_nested_archives(self):
6969
for fname in os.listdir(_path):
7070
arc_name = os.path.join(_path, fname)
7171
if 'sosreport-' in fname and tarfile.is_tarfile(arc_name):
72-
archives.append(SoSReportArchive(arc_name, self.tmpdir))
72+
archives.append(SoSReportArchive(arc_name, self.tmpdir,
73+
self.keep_binary_files))
7374
return archives
7475

7576

sos/cleaner/mappings/__init__.py

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99
# See the LICENSE file in the source distribution for further information.
1010

1111
import re
12-
13-
from threading import Lock
12+
import os
13+
import tempfile
14+
from pathlib import Path
1415

1516

1617
class SoSMap():
@@ -28,11 +29,33 @@ class SoSMap():
2829
ignore_short_items = False
2930
match_full_words_only = False
3031

31-
def __init__(self):
32+
def __init__(self, workdir):
3233
self.dataset = {}
3334
self._regexes_made = set()
3435
self.compiled_regexes = []
35-
self.lock = Lock()
36+
self.cname = self.__class__.__name__.lower()
37+
# workdir's default value '/tmp' is used just by avocado tests,
38+
# otherwise we override it to /etc/sos/cleaner (or map_file dir)
39+
self.workdir = workdir
40+
self.cache_dir = os.path.join(self.workdir, 'cleaner_cache',
41+
self.cname)
42+
self.load_entries()
43+
44+
def load_entries(self):
45+
""" Load cached entries from the disk. This method must be called when
46+
we initialize a Map instance and whenever we want to retrieve
47+
self.dataset (e.g. to store default_mapping file). The later is
48+
essential since a concurrent Map can add more objects to the cache,
49+
so we need to update self.dataset up to date.
50+
51+
Keep in mind that size of self.dataset is usually bigger than number
52+
of files in the corresponding cleaner's directory: directory contains
53+
just whole items (e.g. IP addresses) while dataset contains more
54+
derived objects (e.g. subnets).
55+
"""
56+
57+
Path(self.cache_dir).mkdir(parents=True, exist_ok=True)
58+
self.load_new_entries_from_dir(1)
3659

3760
def ignore_item(self, item):
3861
"""Some items need to be completely ignored, for example link-local or
@@ -46,6 +69,36 @@ def ignore_item(self, item):
4669
return True
4770
return False
4871

72+
def add_sanitised_item_to_dataset(self, item):
73+
try:
74+
self.dataset[item] = self.sanitize_item(item)
75+
except Exception:
76+
self.dataset[item] = item
77+
if self.compile_regexes:
78+
self.add_regex_item(item)
79+
80+
def load_new_entries_from_dir(self, counter):
81+
# this is a performance hack; there can be gaps in counter values as
82+
# e.g. sanitised item #14 is an IP address (in file) while item #15
83+
# is its network (in dataset but not in files). So the next file
84+
# number is 16. The diffs should be at most 2, the above is so far
85+
# the only type of "underneath dataset growth". But let be
86+
# conservative and test next 5 numbers "only".
87+
no_files_cnt = 5
88+
while no_files_cnt > 0:
89+
fname = os.path.join(self.cache_dir, f"{counter}")
90+
while os.path.isfile(fname):
91+
no_files_cnt = 5
92+
with open(fname, 'r', encoding='utf-8') as f:
93+
item = f.read()
94+
if not self.dataset.get(item, False):
95+
self.add_sanitised_item_to_dataset(item)
96+
counter += 1
97+
fname = os.path.join(self.cache_dir, f"{counter}")
98+
# no next file, but try a new next ones until no_files_cnt==0
99+
no_files_cnt -= 1
100+
counter += 1
101+
49102
def add(self, item):
50103
"""Add a particular item to the map, generating an obfuscated pair
51104
for it.
@@ -56,11 +109,23 @@ def add(self, item):
56109
"""
57110
if self.ignore_item(item):
58111
return item
59-
with self.lock:
60-
self.dataset[item] = self.sanitize_item(item)
61-
if self.compile_regexes:
62-
self.add_regex_item(item)
63-
return self.dataset[item]
112+
113+
tmpfile = None
114+
while not self.dataset.get(item, False):
115+
if not tmpfile:
116+
# pylint: disable=consider-using-with
117+
tmpfile = tempfile.NamedTemporaryFile(dir=self.cache_dir)
118+
with open(tmpfile.name, 'w', encoding='utf-8') as f:
119+
f.write(item)
120+
try:
121+
counter = len(self.dataset) + 1
122+
os.link(tmpfile.name, os.path.join(self.cache_dir,
123+
f"{counter}"))
124+
self.add_sanitised_item_to_dataset(item)
125+
except FileExistsError:
126+
self.load_new_entries_from_dir(counter)
127+
128+
return self.dataset[item]
64129

65130
def add_regex_item(self, item):
66131
"""Add an item to the regexes dict and then re-sort the list that the

sos/cleaner/mappings/ip_map.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
# See the LICENSE file in the source distribution for further information.
1010

1111
import ipaddress
12-
import random
1312

1413
from sos.cleaner.mappings import SoSMap
1514

@@ -45,6 +44,11 @@ class SoSIPMap(SoSMap):
4544
network_first_octet = 100
4645
skip_network_octets = ['127', '169', '172', '192']
4746
compile_regexes = False
47+
# counter for obfuscating a single IP address; the value stands for
48+
# 172.17.0.0; we use a private block of IP addresses and ignore
49+
# 172.16.0.0/16 block as those addresses are more often used in real
50+
# (an attempt to prevent confusion)
51+
_saddr_cnt = 2886795264
4852

4953
def ip_in_dataset(self, ipaddr):
5054
"""There are multiple ways in which an ip address could be handed to us
@@ -162,13 +166,12 @@ def sanitize_ipaddr(self, addr):
162166
return self._new_obfuscated_single_address()
163167

164168
def _new_obfuscated_single_address(self):
165-
def _gen_address():
166-
_octets = []
167-
for _ in range(0, 4):
168-
_octets.append(random.randint(11, 99))
169-
return f"{_octets[0]}.{_octets[1]}.{_octets[2]}.{_octets[3]}"
170-
171-
_addr = _gen_address()
169+
# increment the counter and ignore *.0 and *.255 addresses
170+
self._saddr_cnt += 1
171+
while self._saddr_cnt % 256 in (0, 255):
172+
self._saddr_cnt += 1
173+
_addr = f"{self._saddr_cnt >> 24}.{(self._saddr_cnt >> 16) % 256}." \
174+
f"{(self._saddr_cnt >> 8) % 256}.{self._saddr_cnt % 256}"
172175
if _addr in self.dataset.values():
173176
return self._new_obfuscated_single_address()
174177
return _addr

0 commit comments

Comments
 (0)