Skip to content

Commit

Permalink
Changes after code review
Browse files Browse the repository at this point in the history
- rpm -V --nomtime and .pyc files filtering
- track_custom_modifications actor renamed to scan_custom_modifications
- Using library function get_leapp_packages instead of custom list of
  rpms.
- Changed the wording from 'your system' to 'the system' in checker.
  • Loading branch information
fernflower committed Dec 6, 2023
1 parent 1dc9922 commit 55a6791
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def _create_report(msgs):
['- {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 your system in leapp directories'
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'
'Please consult the list of discovered files for more information:\n'
'{discovered_files}')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ def test_report_any_modifications(current_actor_context):
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'] ==
'Custom files were discovered on your system in leapp directories')
'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 your system in leapp directories')
'Modified files were discovered on the system in leapp directories')
assert 'some/changed/leapp/actor/file (an_actor Actor)' in modified_files_report['summary']
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
from leapp.actors import Actor
from leapp.libraries.actor.trackcustommodifications import check_for_modifications
from leapp.libraries.actor import scancustommodifications
from leapp.models import CustomModifications
from leapp.tags import FactsPhaseTag, IPUWorkflowTag


class TrackCustomModificationsActor(Actor):
class ScanCustomModificationsActor(Actor):
"""
Collects information about files in leapp directories that have been modified or newly added.
"""

name = 'track_custom_modifications_actor'
name = 'scan_custom_modifications_actor'
produces = (CustomModifications,)
tags = (IPUWorkflowTag, FactsPhaseTag)

def process(self):
changed, custom = check_for_modifications()
changed, custom = scancustommodifications.check_for_modifications()
for msg in changed + custom:
self.produce(msg)
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@
import os

from leapp.exceptions import StopActorExecution
from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version
from leapp.libraries.common import dnfconfig
from leapp.libraries.stdlib import api, CalledProcessError, run
from leapp.models import CustomModifications

RPMS_TO_CHECK = ['leapp-upgrade-el{source}toel{target}']
DIRS_TO_SCAN = ['/usr/share/leapp-repository']
LEAPP_PACKAGES_TO_IGNORE = ['snactor']


def _get_rpms_to_check():
return [rpm.format(source=get_source_major_version(), target=get_target_major_version()) for rpm in RPMS_TO_CHECK]
# 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))


def deduce_actor_name(a_file):
Expand Down Expand Up @@ -80,13 +84,16 @@ def check_for_modifications(dirs=None):
# Now let's check for modifications
modified_files = []
for rpm in rpms:
res = _run_command(['rpm', '-V', rpm], 'Could not check authenticity of the files from {}'.format(rpm),
# NOTE(ivasilev) check is False here as in case of any changes found exit code will be 1
checked=False)
res = _run_command(
['rpm', '-V', '--nomtime', rpm], 'Could not check authenticity of the files from {}'.format(rpm),
# NOTE(ivasilev) check is False here as in case of any changes found exit code will be 1
checked=False)
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])
for f in modified_files],
# 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])
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from leapp.libraries.actor import trackcustommodifications
from leapp.libraries.actor import scancustommodifications
from leapp.libraries.common.testutils import CurrentActorMocked, produce_mocked
from leapp.libraries.stdlib import api

Expand Down Expand Up @@ -33,7 +33,7 @@
('repos/system_upgrade/el7toel8/models/authselect.py', ''),
])
def test_deduce_actor_name_from_file(a_file, name):
assert trackcustommodifications.deduce_actor_name(a_file) == name
assert scancustommodifications.deduce_actor_name(a_file) == name


def mocked__run_command(list_of_args, log_message, checked=True):
Expand All @@ -51,8 +51,8 @@ def mocked__run_command(list_of_args, log_message, checked=True):

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(trackcustommodifications, '_run_command', mocked__run_command)
modified, custom = trackcustommodifications.check_for_modifications()
monkeypatch.setattr(scancustommodifications, '_run_command', mocked__run_command)
modified, custom = scancustommodifications.check_for_modifications()
assert len(modified) == 2
assert modified[0].filename == 'repos/system_upgrade/el8toel9/actors/xorgdrvfact/libraries/xorgdriverlib.py'
assert modified[0].rpm_checks_str == '.......T.'
Expand Down

0 comments on commit 55a6791

Please sign in to comment.