upgrade data files loading: update error msgs and repors + minor changes

pirat89 · pirat89 · commit 500967badf8d · 2024-01-17T22:17:59.000+01:00
Updated number of error messages and reports to be sure that users
know what files are actually problematic. Original errors and reports
usually didn't contain the full path to an upgrade data file due to
possibility to download the file from a server. However, the posibility
to download fresh data files from a requested server is not expected
to support in the current state as the data files are part of provided
packages.

I've been thinking quite long time whether to actually drop
or deprecate bigger part of the code to simplify the whole solution,
as currently it's not planned to have a possibility to download
some data files from servers in future. However, thinking about
upcoming challenges, I am not totally persuaded that we will not
revive that functionality in future, or that we will not want to
use it for something little bit different. From that POV (and late
phase of development prior the planned release) I think it will be
better to preserve it for now and raise a discussion about it later.

Other changes in this PR:
 * drop hardcoded name of the leapp-upgrade-elXtoelY rpm and use
   the leapp.libraries.common.rpms.get_leapp_packages() function
 * replace REPOSITORY group by SANITY; it was originally a mixture
   of both and SANITY fits better to me from this point
 * the check of consumed data sets could produce report with empty
   links, as the original article(s) we referred to have been obsoleted;
   so added filter for missing URLs
diff --git a/repos/system_upgrade/common/actors/checkconsumedassets/libraries/check_consumed_assets.py b/repos/system_upgrade/common/actors/checkconsumedassets/libraries/check_consumed_assets.py
@@ -4,10 +4,27 @@
 from leapp import reporting
 from leapp.libraries.common.config import get_consumed_data_stream_id
 from leapp.libraries.common.fetch import ASSET_PROVIDED_DATA_STREAMS_FIELD
+from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents
 from leapp.libraries.stdlib import api
 from leapp.models import ConsumedDataAsset
 
 
+def _get_hint():
+    hint = (
+        'All official assets (data files) are nowadays part of the installed rpms.'
+        ' This issue is usually encountered when the data files are incorrectly'
+        ' customized, replaced, or removed. '
+        ' In case you want to recover original files, remove them (if still exist)'
+        ' and reinstall following rpms: {rpms}.\n'
+        'The listed assets (data files) are usually inside the /etc/leapp/files/'
+        ' directory.'
+        .format(
+            rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY))
+        )
+    )
+    return hint
+
+
 def compose_summary_for_incompatible_assets(assets, incompatibility_reason):
     if not assets:
         return []
@@ -69,13 +86,16 @@ def report_incompatible_assets(assets):
         summary_lines += compose_summary_for_incompatible_assets(incompatible_assets, reason)
 
         for asset in incompatible_assets:
-            doc_url_to_title[asset.docs_url].append(asset.docs_title)
+            if asset.docs_url:
+                # Add URLs only when they are specified. docs_url could be empty string
+                doc_url_to_title[asset.docs_url].append(asset.docs_title)
 
     report_parts = [
         reporting.Title(title),
         reporting.Summary('\n'.join(summary_lines)),
         reporting.Severity(reporting.Severity.HIGH),
-        reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.REPOSITORY]),
+        reporting.Remediation(hint=_get_hint()),
+        reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.SANITY]),
     ]
 
     report_parts += make_report_entries_with_unique_urls(docs_url_to_title_map)
@@ -101,13 +121,16 @@ def report_malformed_assets(malformed_assets):
             details = ' - The asset file {filename} contains invalid value in its "{data_streams_field}"'
             details = details.format(filename=asset.filename, data_streams_field=ASSET_PROVIDED_DATA_STREAMS_FIELD)
         summary_lines.append(details)
-        docs_url_to_title_map[asset.docs_url].append(asset.docs_title)
+        if asset.docs_url:
+            # Add URLs only when they are specified. docs_url could be empty string
+            docs_url_to_title_map[asset.docs_url].append(asset.docs_title)
 
     report_parts = [
         reporting.Title(title),
         reporting.Summary('\n'.join(summary_lines)),
+        reporting.Remediation(hint=_get_hint()),
         reporting.Severity(reporting.Severity.HIGH),
-        reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.REPOSITORY]),
+        reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.SANITY]),
     ]
 
     report_parts += make_report_entries_with_unique_urls(docs_url_to_title_map)
diff --git a/repos/system_upgrade/common/actors/loaddevicedriverdeprecationdata/libraries/deviceanddriverdeprecationdataload.py b/repos/system_upgrade/common/actors/loaddevicedriverdeprecationdata/libraries/deviceanddriverdeprecationdataload.py
@@ -13,6 +13,9 @@ def process():
     supported_device_types = set(DeviceDriverDeprecationEntry.device_type.serialize()['choices'])
 
     data_file_name = 'device_driver_deprecation_data.json'
+    # NOTE(pstodulk): load_data_assert raises StopActorExecutionError, see
+    # the code for more info. Keeping the handling on the framework in such
+    # a case as we have no work to do in such a case here.
     deprecation_data = fetch.load_data_asset(api.current_actor(),
                                              data_file_name,
                                              asset_fulltext_name='Device driver deprecation data',
diff --git a/repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_event_parsing.py b/repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_event_parsing.py
@@ -8,7 +8,7 @@
 from leapp.exceptions import StopActorExecution
 from leapp.libraries.common import fetch
 from leapp.libraries.common.config import architecture
-from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version
+from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents
 from leapp.libraries.stdlib import api
 
 # NOTE(mhecko): The modulestream field contains a set of modulestreams until the very end when we generate a Package
@@ -67,6 +67,9 @@ def get_pes_events(pes_json_directory, pes_json_filename):
     :return: List of Event tuples, where each event contains event type and input/output pkgs
     """
     try:
+        # NOTE(pstodulk): load_data_assert raises StopActorExecutionError, see
+        # the code for more info. Keeping the handling on the framework in such
+        # a case as we have no work to do in such a case here.
         events_data = fetch.load_data_asset(api.current_actor(),
                                             pes_json_filename,
                                             asset_fulltext_name='PES events file',
@@ -83,22 +86,27 @@ def get_pes_events(pes_json_directory, pes_json_filename):
         events_matching_arch = [e for e in all_events if not e.architectures or arch in e.architectures]
         return events_matching_arch
     except (ValueError, KeyError):
-        rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version())
-        title = 'Missing/Invalid PES data file ({}/{})'.format(pes_json_directory, pes_json_filename)
+        local_path = os.path.join(pes_json_directory, pes_json_filename)
+        title = 'Missing/Invalid PES data file ({})'.format(local_path)
         summary = (
             'All official data files are nowadays part of the installed rpms.'
             ' This issue is usually encountered when the data files are incorrectly customized, replaced, or removed'
             ' (e.g. by custom scripts).'
-            ' In case you want to recover the original file, remove it (if still exists)'
-            ' and reinstall the {} rpm.'
-            .format(rpmname)
+        )
+        hint = (
+            ' In case you want to recover the original {lp} file, remove it (if still exists)'
+            ' and reinstall following rpms: {rpms}.'
+            .format(
+                lp=local_path,
+                rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY))
+            )
         )
         reporting.create_report([
             reporting.Title(title),
             reporting.Summary(summary),
+            reporting.Remediation(hint=hint),
             reporting.Severity(reporting.Severity.HIGH),
-            reporting.Groups([reporting.Groups.SANITY]),
-            reporting.Groups([reporting.Groups.INHIBITOR]),
+            reporting.Groups([reporting.Groups.SANITY, reporting.Groups.INHIBITOR]),
             reporting.RelatedResource('file', os.path.join(pes_json_directory, pes_json_filename))
         ])
         raise StopActorExecution()
diff --git a/repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py b/repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py
@@ -4,6 +4,7 @@
 from leapp.exceptions import StopActorExecutionError
 from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version
 from leapp.libraries.common.fetch import load_data_asset
+from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents
 from leapp.libraries.stdlib import api
 from leapp.models import PESIDRepositoryEntry, RepoMapEntry, RepositoriesMapping
 from leapp.models.fields import ModelViolationError
@@ -130,29 +131,31 @@ def load_from_dict(data):
 
 
 def _inhibit_upgrade(msg):
-    rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version())
+    local_path = os.path.join('/etc/leapp/file', REPOMAP_FILE)
     hint = (
         'All official data files are nowadays part of the installed rpms.'
         ' This issue is usually encountered when the data files are incorrectly customized, replaced, or removed'
         ' (e.g. by custom scripts).'
-        ' In case you want to recover the original file, remove it (if still exists)'
-        ' and reinstall the {} rpm.'
-        .format(rpmname)
+        ' In case you want to recover the original {lp} file, remove the current one (if still exists)'
+        ' and reinstall following packages: {rpms}.'
+        .format(
+            lp=local_path,
+            rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY))
+        )
     )
     raise StopActorExecutionError(msg, details={'hint': hint})
 
 
 def _read_repofile(repofile):
-    # NOTE: what about catch StopActorExecution error when the file cannot be
-    # obtained -> then check whether old_repomap file exists and in such a case
-    # inform user they have to provide the new repomap.json file (we have the
-    # warning now only which could be potentially overlooked)
+    # NOTE(pstodulk): load_data_assert raises StopActorExecutionError, see
+    # the code for more info. Keeping the handling on the framework in such
+    # a case as we have no work to do in such a case here.
     repofile_data = load_data_asset(api.current_actor(),
                                     repofile,
                                     asset_fulltext_name='Repositories mapping',
                                     docs_url='',
                                     docs_title='')
-    return repofile_data  # If the file does not contain a valid json then load_asset will do a stop actor execution
+    return repofile_data
 
 
 def scan_repositories(read_repofile_func=_read_repofile):
diff --git a/repos/system_upgrade/common/libraries/fetch.py b/repos/system_upgrade/common/libraries/fetch.py
@@ -7,7 +7,7 @@
 from leapp import models
 from leapp.exceptions import StopActorExecutionError
 from leapp.libraries.common.config import get_consumed_data_stream_id, get_env
-from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version
+from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents
 from leapp.libraries.stdlib import api
 
 SERVICE_HOST_DEFAULT = "https://cert.cloud.redhat.com"
@@ -16,16 +16,18 @@
 ASSET_PROVIDED_DATA_STREAMS_FIELD = 'provided_data_streams'
 
 
-def _get_hint():
-    rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version())
+def _get_hint(local_path):
     hint = (
         'All official data files are nowadays part of the installed rpms.'
         ' That is the only official resource of actual official data files for in-place upgrades.'
         ' This issue is usually encountered when the data files are incorrectly customized, replaced, or removed'
         ' (e.g. by custom scripts).'
-        ' In case you want to recover the original file, remove it (if still exists)'
-        ' and reinstall the {} rpm.'
-        .format(rpmname)
+        ' In case you want to recover the original {lp} file, remove the current one (if still exists)'
+        ' and reinstall following packages: {rpms}.'
+        .format(
+            lp=local_path,
+            rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY))
+        )
     )
     return hint
 
@@ -34,10 +36,9 @@ def _raise_error(local_path, details):
     """
     If the file acquisition fails in any way, throw an informative error to stop the actor.
     """
-    rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version())
     summary = 'Data file {lp} is missing or invalid.'.format(lp=local_path)
 
-    raise StopActorExecutionError(summary, details={'details': details, 'hint': _get_hint()})
+    raise StopActorExecutionError(summary, details={'details': details, 'hint': _get_hint(local_path)})
 
 
 def _request_data(service_path, cert, proxies, timeout=REQUEST_TIMEOUT):
@@ -148,6 +149,7 @@ def load_data_asset(actor_requesting_asset,
                     docs_title):
     """
     Load the content of the data asset with given asset_filename
+    and produce :class:`leapp.model.ConsumedDataAsset` message.
 
     :param Actor actor_requesting_asset: The actor instance requesting the asset file. It is necessary for the actor
                                          to be able to produce ConsumedDataAsset message in order for leapp to be able
@@ -157,6 +159,10 @@ def load_data_asset(actor_requesting_asset,
     :param str docs_url: Docs url to provide if an asset is malformed or outdated.
     :param str docs_title: Title of the documentation to where `docs_url` points to.
     :returns: A dict with asset contents (a parsed JSON), or None if the asset was outdated.
+    :raises StopActorExecutionError: In following cases:
+        * ConsumedDataAsset is not specified in the produces tuple of the actor_requesting_asset actor
+        * The content of the required data file is not valid JSON format
+        * The required data cannot be obtained (e.g. due to missing file)
     """
 
     # Check that the actor that is attempting to obtain the asset meets the contract to call this function
@@ -167,7 +173,7 @@ def load_data_asset(actor_requesting_asset,
         error_hint = {'hint': ('Read documentation at the following link for more information about how to retrieve '
                                'the valid file: {0}'.format(docs_url))}
     else:
-        error_hint = {'hint': _get_hint()}
+        error_hint = {'hint': _get_hint(os.path.join('/etc/leapp/files', asset_filename))}
 
     data_stream_id = get_consumed_data_stream_id()
     data_stream_major = data_stream_id.split('.', 1)[0]
