Track framework changes as well

Also some minor refactoring done to the scanner and tests.

repos/system_upgrade/common/actors/checkcustommodifications/libraries/checkcustommodifications.py

@@ -3,29 +3,35 @@
 from leapp.models import CustomModifications
 
 
-def _create_report(msgs):
+def _create_report(msgs, report_type, component=None, hint=None):
     if not msgs:
         # Nothing to report
         return
-    report_type = msgs[0].type
+    filtered_msgs = [m for m in msgs if m.type == report_type and (not component or m.component == component)]
     discovered_files = '\n'.join(
             ['- {filename}{actor}'.format(filename=m.filename,
                                           actor=' ({} Actor)'.format(m.actor_name) if m.actor_name else '')
-             for m in msgs])
-    title = '{report_type} files were discovered on the system in leapp directories'
-    summary = ('Apparently some {report_type} files have been found in leapp installation directories.\n'
+             for m in filtered_msgs])
+    component_str = 'leapp {}'.format(component) if component else 'leapp'
+    title = '{report_type} files were discovered on the system in {component_str} directories'
+    summary = ('Apparently some {report_type} files have been found in {component_str} installation directories.\n'
                'Please consult the list of discovered files for more information:\n'
                '{discovered_files}')
     report_parts = [
-        reporting.Title(title.format(report_type=report_type.capitalize())),
-        reporting.Summary(summary.format(report_type=report_type, discovered_files=discovered_files)),
+        reporting.Title(title.format(report_type=report_type.capitalize(), component_str=component_str)),
+        reporting.Summary(summary.format(report_type=report_type, discovered_files=discovered_files,
+                                         component_str=component_str)),
         reporting.Severity(reporting.Severity.HIGH),
         reporting.Groups([reporting.Groups.REPOSITORY]),
     ]
+    if hint:
+        report_parts.append(reporting.RemediationHint(hint))
     reporting.create_report(report_parts)
 
 
 def report_any_modifications():
     modifications = list(api.consume(CustomModifications))
-    _create_report([m for m in modifications if m.type == 'custom'])
-    _create_report([m for m in modifications if m.type == 'modified'])
+    _create_report(modifications, report_type='custom')
+    _create_report(modifications, report_type='modified', component='repository')
+    _create_report(modifications, report_type='modified', component='framework',
+                   hint='Reinstall leapp to get rid of these changes.')

repos/system_upgrade/common/actors/checkcustommodifications/tests/test_checkcustommodifications.py

@@ -5,22 +5,27 @@
 def test_report_any_modifications(current_actor_context):
     discovered_msgs = [CustomModifications(filename='some/changed/leapp/actor/file',
                                            type='modified',
-                                           actor_name='an_actor'),
+                                           actor_name='an_actor',
+                                           component='repository'),
                        CustomModifications(filename='some/new/actor/in/leapp/dir',
                                            type='custom',
-                                           actor_name='a_new_actor'),
-                       CustomModifications(filename='some/new/file/unrelated/to/any/actor', type='custom',
-                                           actor_name='')]
+                                           actor_name='a_new_actor',
+                                           component='repository'),
+                       CustomModifications(filename='some/changed/file/in/framework',
+                                           type='modified',
+                                           actor_name='',
+                                           component='framework')]
     for msg in discovered_msgs:
         current_actor_context.feed(msg)
     current_actor_context.run()
     reports = current_actor_context.consume(Report)
-    custom_files_report = next((r.report for r in reports if 'Custom' in r.report['title']), None)
-    modified_files_report = next((r.report for r in reports if 'Modified' in r.report['title']), None)
-    assert (custom_files_report['title'] ==
+    assert len(reports) == 3
+    assert (reports[1].report['title'] ==
+            'Modified files were discovered on the system in leapp repository directories')
+    assert 'some/changed/leapp/actor/file (an_actor Actor)' in reports[1].report['summary']
+    assert (reports[0].report['title'] ==
             'Custom files were discovered on the system in leapp directories')
-    assert 'some/new/actor/in/leapp/dir (a_new_actor Actor)' in custom_files_report['summary']
-    assert 'some/new/file/unrelated/to/any/actor' in custom_files_report['summary']
-    assert (modified_files_report['title'] ==
-            'Modified files were discovered on the system in leapp directories')
-    assert 'some/changed/leapp/actor/file (an_actor Actor)' in modified_files_report['summary']
+    assert 'some/new/actor/in/leapp/dir (a_new_actor Actor)' in reports[0].report['summary']
+    assert (reports[2].report['title'] ==
+            'Modified files were discovered on the system in leapp framework directories')
+    assert 'some/changed/file/in/framework' in reports[2].report['summary']

repos/system_upgrade/common/actors/scancustommodifications/actor.py

@@ -14,6 +14,5 @@ class ScanCustomModificationsActor(Actor):
     tags = (IPUWorkflowTag, FactsPhaseTag)
 
     def process(self):
-        changed, custom = scancustommodifications.check_for_modifications()
-        for msg in changed + custom:
+        for msg in scancustommodifications.scan():
             self.produce(msg)

repos/system_upgrade/common/actors/scancustommodifications/libraries/scancustommodifications.py

@@ -3,19 +3,32 @@
 
 from leapp.exceptions import StopActorExecution
 from leapp.libraries.common import dnfconfig
+from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version
 from leapp.libraries.stdlib import api, CalledProcessError, run
 from leapp.models import CustomModifications
 
-DIRS_TO_SCAN = ['/usr/share/leapp-repository']
+LEAPP_REPO_DIRS = ['/usr/share/leapp-repository']
 LEAPP_PACKAGES_TO_IGNORE = ['snactor']
 
 
-def _get_rpms_to_check():
+def _get_dirs_to_check(component):
+    if component == 'repository':
+        return LEAPP_REPO_DIRS
+    return []
+
+
+def _get_rpms_to_check(component=None):
     # NOTE(ivasilev) Still not sure that library function is the way to go as looks like leapp file changes have
     # to be handled differently, and configuration change has to be tracked only for leapp-repository as well.
     # Let's keep it here for now but I am probably switching to LEAPP_PKG and LEAPP_REPO_PKG in the next commit.
-    all_leapp_packages = dnfconfig.get_leapp_packages()
-    return list(set(all_leapp_packages) - set(LEAPP_PACKAGES_TO_IGNORE))
+    leapp_installation = set(dnfconfig.get_leapp_packages()) - set(LEAPP_PACKAGES_TO_IGNORE)
+    leapp_repo = ['leapp-upgrade-el{source}toel{target}'.format(source=get_source_major_version(),
+                                                                target=get_target_major_version())]
+    if component == 'repository':
+        return leapp_repo
+    if component == 'framework':
+        return list(leapp_installation - set(leapp_repo))
+    return list(leapp_installation)
 
 
 def deduce_actor_name(a_file):
@@ -81,14 +94,21 @@ def _run_command(cmd, warning_to_log, checked=True):
         raise StopActorExecution()
 
 
-def check_for_modifications(dirs=None):
+def _modification_model(filename, change_type, component, rpm_checks_str=''):
+    # XXX FIXME(ivasilev) Actively thinking if different model classes inheriting from CustomModifications
+    # are needed or let's get away with one model for everything (as is implemented now).
+    # The only difference atm is that actor_name makes sense only for repository modifications.
+    return CustomModifications(filename=filename, type=change_type, component=component,
+                               actor_name=deduce_actor_name(filename), rpm_checks_str=rpm_checks_str)
+
+
+def check_for_modifications(component):
     """
-    This will return a tuple (changes, custom) is case any untypical files or changes to shipped leapp files are
-    discovered.
-    (None, None) means that no modifications have been found.
+    This will return a list of any untypical files or changes to shipped leapp files discovered on the system.
+    An empty list means that no modifications have been found.
     """
-    dirs = dirs if dirs else DIRS_TO_SCAN
-    rpms = _get_rpms_to_check()
+    rpms = _get_rpms_to_check(component)
+    dirs = _get_dirs_to_check(component)
     source_of_truth = []
     leapp_files = []
     # Let's collect data about what should have been installed from rpm
@@ -112,9 +132,13 @@ def check_for_modifications(dirs=None):
         if res:
             api.current_logger().warning('Modifications to leapp files detected!\n%s', res)
             modified_files.extend([tuple(x.split()) for x in res])
-    return ([CustomModifications(actor_name=deduce_actor_name(f[1]), filename=f[1], type='modified',
-                                 rpm_checks_str=f[0])
+    return ([_modification_model(filename=f[1], component=component, rpm_checks_str=f[0], change_type='modified')
              # Let's filter out pyc files not to clutter the output as pyc will be present even in case of
-             # a plain open&save-not-changed
-             for f in modified_files if not f[1].endswith('.pyc')],
-            [CustomModifications(actor_name=deduce_actor_name(f), filename=f, type='custom') for f in custom_files])
+             # a plain open & save-not-changed that we agreed not to react upon.
+             for f in modified_files if not f[1].endswith('.pyc')] +
+            [_modification_model(filename=f, component=component, change_type='custom')
+             for f in custom_files])
+
+
+def scan():
+    return check_for_modifications('framework') + check_for_modifications('repository')

repos/system_upgrade/common/actors/scancustommodifications/tests/test_scancustommodifications.py

@@ -58,22 +58,24 @@ def test_deduce_actor_name_from_file(a_file, name):
 
 
 def mocked__run_command(list_of_args, log_message, checked=True):
-    if list_of_args == ['rpm', '-ql', 'leapp-upgrade-el7toel8']:
+    if list_of_args == ['rpm', '-ql', 'leapp-upgrade-el8toel9']:
         # get source of truth
         return FILES_FROM_RPM.strip().split('\n')
     if list_of_args and list_of_args[0] == 'find':
         # listing files in directory
         return FILES_ON_SYSTEM.strip().split('\n')
-    if list_of_args == ['rpm', '-V', '--nomtime', 'leapp-upgrade-el7toel8']:
+    if list_of_args == ['rpm', '-V', '--nomtime', 'leapp-upgrade-el8toel9']:
         # checking authenticity
         return VERIFIED_FILES.strip().split('\n')
     return []
 
 
 def test_check_for_modifications(monkeypatch):
-    monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(arch='x86_64', src_ver='7.9', dst_ver='8.4'))
+    monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(arch='x86_64', src_ver='8.9', dst_ver='9.3'))
     monkeypatch.setattr(scancustommodifications, '_run_command', mocked__run_command)
-    modified, custom = scancustommodifications.check_for_modifications()
+    modifications = scancustommodifications.check_for_modifications('repository')
+    modified = [m for m in modifications if m.type == 'modified']
+    custom = [m for m in modifications if m.type == 'custom']
     assert modified[0].filename == 'repos/system_upgrade/el8toel9/actors/xorgdrvfact/libraries/xorgdriverlib.py'
     assert modified[0].rpm_checks_str == '.......T.'
     assert len(custom) == 3

repos/system_upgrade/common/models/custommodifications.py

@@ -10,3 +10,4 @@ class CustomModifications(Model):
     actor_name = fields.String()
     type = fields.StringEnum(choices=['custom', 'modified'])
     rpm_checks_str = fields.String(default='')
+    component = fields.String()